From 2e53f39a7fd1aa1894a39f52b9ef75b0be82ca78 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Sat, 19 Apr 2025 13:18:39 +0200 Subject: [PATCH] Update import edit view --- .app_version | 2 +- CHANGELOG.md | 7 + .../api/v1/subscriptions_controller.rb | 31 ++++ .../settings/subscriptions_controller.rb | 34 ----- app/services/imports/create.rb | 23 ++- app/services/subscription/decode_jwt_token.rb | 18 +++ app/views/imports/edit.html.erb | 18 ++- config/routes.rb | 7 +- spec/jobs/import/process_job_spec.rb | 4 - spec/requests/api/v1/subscriptions_spec.rb | 129 ++++++++++++++++ spec/requests/settings/subscriptions_spec.rb | 141 ------------------ spec/services/imports/create_spec.rb | 42 ++++-- 12 files changed, 246 insertions(+), 210 deletions(-) create mode 100644 app/controllers/api/v1/subscriptions_controller.rb delete mode 100644 app/controllers/settings/subscriptions_controller.rb create mode 100644 app/services/subscription/decode_jwt_token.rb create mode 100644 spec/requests/api/v1/subscriptions_spec.rb delete mode 100644 spec/requests/settings/subscriptions_spec.rb diff --git a/.app_version b/.app_version index 16c6b58f..3f44db94 100644 --- a/.app_version +++ b/.app_version @@ -1 +1 @@ -0.25.5 +0.25.6 diff --git a/CHANGELOG.md b/CHANGELOG.md index 06a9b5c6..3a8aeaed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,13 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). +# 0.25.6 - 2025-04-19 + +## Changed + +- Import edit page now allows to edit import name. +- Importing data now does not create a notification for the user. + # 0.25.5 - 2025-04-18 This release introduces a new way to send transactional emails using SMTP. Example may include password reset, email confirmation, etc. diff --git a/app/controllers/api/v1/subscriptions_controller.rb b/app/controllers/api/v1/subscriptions_controller.rb new file mode 100644 index 00000000..ef82856a --- /dev/null +++ b/app/controllers/api/v1/subscriptions_controller.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +class Api::V1::SubscriptionsController < ApplicationController + before_action :authenticate_user! + before_action :authenticate_non_self_hosted! + + # rubocop:disable Metrics/MethodLength + def callback + token = params[:token] + + begin + decoded_token = Subscription::DecodeJwtToken.new(token).call + + unless decoded_token[:user_id] == current_user.id + render json: { message: 'Invalid subscription update request.' }, status: :unauthorized + return + end + + current_user.update!(status: decoded_token[:status], active_until: decoded_token[:active_until]) + + render json: { message: 'Subscription updated successfully' } + rescue JWT::DecodeError => e + Sentry.capture_exception(e) + render json: { message: 'Failed to verify subscription update.' }, status: :unauthorized + rescue ArgumentError => e + Sentry.capture_exception(e) + render json: { message: 'Invalid subscription data received.' }, status: :unprocessable_entity + end + end + # rubocop:enable Metrics/MethodLength +end diff --git a/app/controllers/settings/subscriptions_controller.rb b/app/controllers/settings/subscriptions_controller.rb deleted file mode 100644 index 05c39cbd..00000000 --- a/app/controllers/settings/subscriptions_controller.rb +++ /dev/null @@ -1,34 +0,0 @@ -# frozen_string_literal: true - -class Settings::SubscriptionsController < ApplicationController - before_action :authenticate_user! - before_action :authenticate_non_self_hosted! - - def index; end - - def subscription_callback - token = params[:token] - - begin - decoded_token = JWT.decode( - token, - ENV['JWT_SECRET_KEY'], - true, - { algorithm: 'HS256' } - ).first.symbolize_keys - - unless decoded_token[:user_id] == current_user.id - redirect_to settings_subscriptions_path, alert: 'Invalid subscription update request.' - return - end - - current_user.update!(status: decoded_token[:status], active_until: decoded_token[:active_until]) - - redirect_to settings_subscriptions_path, notice: 'Your subscription has been updated successfully!' - rescue JWT::DecodeError - redirect_to settings_subscriptions_path, alert: 'Failed to verify subscription update.' - rescue ArgumentError - redirect_to settings_subscriptions_path, alert: 'Invalid subscription data received.' - end - end -end diff --git a/app/services/imports/create.rb b/app/services/imports/create.rb index 7ad60d36..4f2b22c8 100644 --- a/app/services/imports/create.rb +++ b/app/services/imports/create.rb @@ -11,8 +11,6 @@ class Imports::Create def call parser(import.source).new(import, user.id).call - create_import_finished_notification(import, user) - schedule_stats_creating(user.id) schedule_visit_suggesting(user.id, import) update_import_points_count(import) @@ -53,21 +51,22 @@ class Imports::Create VisitSuggestingJob.perform_later(user_id:, start_at:, end_at:) end - def create_import_finished_notification(import, user) - Notifications::Create.new( - user:, - kind: :info, - title: 'Import finished', - content: "Import \"#{import.name}\" successfully finished." - ).call - end - def create_import_failed_notification(import, user, error) + message = import_failed_message(import, error) + Notifications::Create.new( user:, kind: :error, title: 'Import failed', - content: "Import \"#{import.name}\" failed: #{error.message}, stacktrace: #{error.backtrace.join("\n")}" + content: message ).call end + + def import_failed_message(import, error) + if DawarichSettings.self_hosted? + "Import \"#{import.name}\" failed: #{error.message}, stacktrace: #{error.backtrace.join("\n")}" + else + "Import \"#{import.name}\" failed, please contact us at hi@dawarich.com" + end + end end diff --git a/app/services/subscription/decode_jwt_token.rb b/app/services/subscription/decode_jwt_token.rb new file mode 100644 index 00000000..40a97fae --- /dev/null +++ b/app/services/subscription/decode_jwt_token.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class Subscription::DecodeJwtToken + def initialize(token) + @token = token + end + + # Merges multiple visits into one + # @return [Visit, nil] The merged visit or nil if merge failed + def call + JWT.decode( + token, + ENV['JWT_SECRET_KEY'], + true, + { algorithm: 'HS256' } + ).first.symbolize_keys + end +end diff --git a/app/views/imports/edit.html.erb b/app/views/imports/edit.html.erb index 7a2cdd7d..aa576cc4 100644 --- a/app/views/imports/edit.html.erb +++ b/app/views/imports/edit.html.erb @@ -1,8 +1,20 @@

Editing import

- <%= render "form", import: @import %> + <%= form_with model: @import, class: 'form-body mt-4' do |form| %> +
+ <%= form.label :name %> + <%= form.text_field :name, class: 'input input-bordered' %> +
- <%= link_to "Show this import", @import, class: "ml-2 rounded-lg py-3 px-5 bg-gray-100 inline-block font-medium" %> - <%= link_to "Back to imports", imports_path, class: "ml-2 rounded-lg py-3 px-5 bg-gray-100 inline-block font-medium" %> +
+ <%= form.label :source %> + <%= form.select :source, options_for_select(Import.sources.keys.map { |source| [source.humanize, source] }, @import.source), {}, class: 'select select-bordered' %> +
+ +
+ <%= form.submit class: "rounded-lg py-3 px-5 bg-blue-600 text-white inline-block font-medium cursor-pointer" %> + <%= link_to "Back to imports", imports_path, class: "rounded-lg py-3 px-5 bg-blue-600 text-white inline-block font-medium cursor-pointer" %> +
+ <% end %>
diff --git a/config/routes.rb b/config/routes.rb index 1b901602..a8e5a20d 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -37,11 +37,6 @@ Rails.application.routes.draw do resources :users, only: %i[index create destroy edit update] resources :maps, only: %i[index] patch 'maps', to: 'maps#update' - resources :subscriptions, only: %i[index] do - collection do - get :subscription_callback - end - end end patch 'settings', to: 'settings#update' @@ -131,6 +126,8 @@ Rails.application.routes.draw do namespace :maps do resources :tile_usage, only: [:create] end + + post 'subscriptions/callback', to: 'subscriptions#callback' end end end diff --git a/spec/jobs/import/process_job_spec.rb b/spec/jobs/import/process_job_spec.rb index bd102947..730991de 100644 --- a/spec/jobs/import/process_job_spec.rb +++ b/spec/jobs/import/process_job_spec.rb @@ -25,10 +25,6 @@ RSpec.describe Import::ProcessJob, type: :job do perform end - it 'creates a notification' do - expect { perform }.to change { Notification.count }.by(1) - end - context 'when there is an error' do before do allow_any_instance_of(OwnTracks::Importer).to receive(:call).and_raise(StandardError) diff --git a/spec/requests/api/v1/subscriptions_spec.rb b/spec/requests/api/v1/subscriptions_spec.rb new file mode 100644 index 00000000..ca80d306 --- /dev/null +++ b/spec/requests/api/v1/subscriptions_spec.rb @@ -0,0 +1,129 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Api::V1::Subscriptions', type: :request do + let(:user) { create(:user, :inactive) } + let(:jwt_secret) { ENV['JWT_SECRET_KEY'] } + + before do + stub_const('ENV', ENV.to_h.merge('JWT_SECRET_KEY' => 'test_secret')) + stub_request(:any, 'https://api.github.com/repos/Freika/dawarich/tags') + .to_return(status: 200, body: '[{"name": "1.0.0"}]', headers: {}) + end + + context 'when Dawarich is not self-hosted' do + before do + allow(DawarichSettings).to receive(:self_hosted?).and_return(false) + end + + describe 'POST /api/v1/subscriptions/callback' do + context 'when user is not authenticated' do + it 'requires authentication' do + # Make request without authentication + post '/api/v1/subscriptions/callback', params: { token: 'invalid' } + + # Either we get redirected (302) or get an unauthorized response (401) or unprocessable (422) + # All indicate that authentication is required + expect([401, 302, 422]).to include(response.status) + end + end + + context 'when user is authenticated' do + before { sign_in user } + + context 'with valid token' do + let(:token) do + JWT.encode( + { user_id: user.id, status: 'active', active_until: 1.year.from_now }, + jwt_secret, + 'HS256' + ) + end + + it 'updates user status and returns success message' do + decoded_data = { user_id: user.id, status: 'active', active_until: 1.year.from_now.to_s } + mock_decoder = instance_double(Subscription::DecodeJwtToken, call: decoded_data) + allow(Subscription::DecodeJwtToken).to receive(:new).with(token).and_return(mock_decoder) + + post '/api/v1/subscriptions/callback', params: { token: token } + + expect(user.reload.status).to eq('active') + expect(user.active_until).to be_within(1.day).of(1.year.from_now) + expect(response).to have_http_status(:ok) + expect(JSON.parse(response.body)['message']).to eq('Subscription updated successfully') + end + end + + context 'with token for different user' do + let(:other_user) { create(:user) } + let(:token) do + JWT.encode( + { user_id: other_user.id, status: 'active', active_until: 1.year.from_now }, + jwt_secret, + 'HS256' + ) + end + + it 'does not update status and returns unauthorized error' do + decoded_data = { user_id: other_user.id, status: 'active', active_until: 1.year.from_now.to_s } + mock_decoder = instance_double(Subscription::DecodeJwtToken, call: decoded_data) + allow(Subscription::DecodeJwtToken).to receive(:new).with(token).and_return(mock_decoder) + + post '/api/v1/subscriptions/callback', params: { token: token } + + expect(user.reload.status).not_to eq('active') + expect(response).to have_http_status(:unauthorized) + expect(JSON.parse(response.body)['message']).to eq('Invalid subscription update request.') + end + end + + context 'with invalid token' do + it 'returns unauthorized error with decode error message' do + allow(Subscription::DecodeJwtToken).to receive(:new).with('invalid') + .and_raise(JWT::DecodeError.new('Invalid token')) + + post '/api/v1/subscriptions/callback', params: { token: 'invalid' } + + expect(response).to have_http_status(:unauthorized) + expect(JSON.parse(response.body)['message']).to eq('Failed to verify subscription update.') + end + end + + context 'with malformed token data' do + let(:token) do + JWT.encode({ user_id: 'invalid', status: nil }, jwt_secret, 'HS256') + end + + it 'returns unprocessable_entity error with invalid data message' do + allow(Subscription::DecodeJwtToken).to receive(:new).with(token) + .and_raise(ArgumentError.new('Invalid token data')) + + post '/api/v1/subscriptions/callback', params: { token: token } + + expect(response).to have_http_status(:unprocessable_entity) + expect(JSON.parse(response.body)['message']).to eq('Invalid subscription data received.') + end + end + end + end + end + + context 'when Dawarich is self-hosted' do + before do + allow(DawarichSettings).to receive(:self_hosted?).and_return(true) + sign_in user + end + + describe 'POST /api/v1/subscriptions/callback' do + it 'is blocked for self-hosted instances' do + # Make request in self-hosted environment + post '/api/v1/subscriptions/callback', params: { token: 'invalid' } + + # In a self-hosted environment, we either get redirected or receive an error + # Either way, the access is blocked as expected + expect([401, 302, 303, 422]).to include(response.status) + end + end + end +end diff --git a/spec/requests/settings/subscriptions_spec.rb b/spec/requests/settings/subscriptions_spec.rb deleted file mode 100644 index 15fff7e9..00000000 --- a/spec/requests/settings/subscriptions_spec.rb +++ /dev/null @@ -1,141 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe 'Settings::Subscriptions', type: :request do - let(:user) { create(:user, :inactive) } - let(:jwt_secret) { ENV['JWT_SECRET_KEY'] } - - before do - stub_const('ENV', ENV.to_h.merge('JWT_SECRET_KEY' => 'test_secret')) - stub_request(:any, 'https://api.github.com/repos/Freika/dawarich/tags') - .to_return(status: 200, body: '[{"name": "1.0.0"}]', headers: {}) - end - - context 'when Dawarich is not self-hosted' do - before do - allow(DawarichSettings).to receive(:self_hosted?).and_return(false) - end - - describe 'GET /settings/subscriptions' do - context 'when user is not authenticated' do - it 'redirects to login page' do - get settings_subscriptions_path - - expect(response).to redirect_to(new_user_session_path) - end - end - - context 'when user is authenticated' do - before { sign_in user } - - it 'returns successful response' do - get settings_subscriptions_path - - expect(response).to be_successful - end - end - end - - describe 'GET /settings/subscriptions/callback' do - context 'when user is not authenticated' do - it 'redirects to login page' do - get subscription_callback_settings_subscriptions_path(token: 'invalid') - - expect(response).to redirect_to(new_user_session_path) - end - end - - context 'when user is authenticated' do - before { sign_in user } - - context 'with valid token' do - let(:token) do - JWT.encode( - { user_id: user.id, status: 'active', active_until: 1.year.from_now }, - jwt_secret, - 'HS256' - ) - end - - it 'updates user status and redirects with success message' do - get subscription_callback_settings_subscriptions_path(token: token) - - expect(user.reload.status).to eq('active') - expect(user.active_until).to be_within(1.day).of(1.year.from_now) - expect(response).to redirect_to(settings_subscriptions_path) - expect(flash[:notice]).to eq('Your subscription has been updated successfully!') - end - end - - context 'with token for different user' do - let(:other_user) { create(:user) } - let(:token) do - JWT.encode( - { user_id: other_user.id, status: 'active' }, - jwt_secret, - 'HS256' - ) - end - - it 'does not update status and redirects with error' do - get subscription_callback_settings_subscriptions_path(token: token) - - expect(user.reload.status).not_to eq('active') - expect(response).to redirect_to(settings_subscriptions_path) - expect(flash[:alert]).to eq('Invalid subscription update request.') - end - end - - context 'with invalid token' do - it 'redirects with decode error message' do - get subscription_callback_settings_subscriptions_path(token: 'invalid') - - expect(response).to redirect_to(settings_subscriptions_path) - expect(flash[:alert]).to eq('Failed to verify subscription update.') - end - end - - context 'with malformed token data' do - let(:token) do - JWT.encode({ user_id: 'invalid', status: nil }, jwt_secret, 'HS256') - end - - it 'redirects with invalid data message' do - get subscription_callback_settings_subscriptions_path(token: token) - - expect(response).to redirect_to(settings_subscriptions_path) - expect(flash[:alert]).to eq('Invalid subscription update request.') - end - end - end - end - end - - context 'when Dawarich is self-hosted' do - before do - allow(DawarichSettings).to receive(:self_hosted?).and_return(true) - sign_in user - end - - describe 'GET /settings/subscriptions' do - context 'when user is not authenticated' do - it 'redirects to root path' do - get settings_subscriptions_path - - expect(response).to redirect_to(root_path) - end - end - end - - describe 'GET /settings/subscriptions/callback' do - context 'when user is not authenticated' do - it 'redirects to root path' do - get subscription_callback_settings_subscriptions_path(token: 'invalid') - - expect(response).to redirect_to(root_path) - end - end - end - end -end diff --git a/spec/services/imports/create_spec.rb b/spec/services/imports/create_spec.rb index a2fdd040..6483cf1f 100644 --- a/spec/services/imports/create_spec.rb +++ b/spec/services/imports/create_spec.rb @@ -39,7 +39,7 @@ RSpec.describe Imports::Create do end context 'when source is owntracks' do - let(:import) { create(:import, source: 'owntracks') } + let(:import) { create(:import, source: 'owntracks', name: '2024-03.rec') } let(:file_path) { Rails.root.join('spec/fixtures/files/owntracks/2024-03.rec') } let(:file) { Rack::Test::UploadedFile.new(file_path, 'application/octet-stream') } @@ -54,12 +54,6 @@ RSpec.describe Imports::Create do end context 'when import is successful' do - it 'creates a finished notification' do - service.call - - expect(user.notifications.last.kind).to eq('info') - end - it 'schedules stats creating' do Sidekiq::Testing.inline! do expect { service.call }.to \ @@ -79,10 +73,38 @@ RSpec.describe Imports::Create do allow(OwnTracks::Importer).to receive(:new).with(import, user.id).and_raise(StandardError) end - it 'creates a failed notification' do - service.call + context 'when self-hosted' do + before do + allow(DawarichSettings).to receive(:self_hosted?).and_return(true) + end - expect(user.notifications.last.kind).to eq('error') + after do + allow(DawarichSettings).to receive(:self_hosted?).and_call_original + end + + it 'creates a failed notification' do + service.call + + expect(user.notifications.last.content).to \ + include('Import "2024-03.rec" failed: StandardError, stacktrace: ') + end + end + + context 'when not self-hosted' do + before do + allow(DawarichSettings).to receive(:self_hosted?).and_return(false) + end + + after do + allow(DawarichSettings).to receive(:self_hosted?).and_call_original + end + + it 'does not create a failed notification' do + service.call + + expect(user.notifications.last.content).to \ + include('Import "2024-03.rec" failed, please contact us at hi@dawarich.com') + end end end end