From 6fac14675bd26d11be086a01c85fc50b9de05a6f Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Wed, 19 Feb 2025 21:23:11 +0100 Subject: [PATCH 1/4] Restrict to some functionality access for inactive users --- .../api/v1/overland/batches_controller.rb | 2 + .../api/v1/owntracks/points_controller.rb | 2 + app/controllers/api/v1/points_controller.rb | 2 + app/controllers/api/v1/settings_controller.rb | 2 + app/controllers/api_controller.rb | 6 ++ app/controllers/application_controller.rb | 6 ++ app/controllers/imports_controller.rb | 1 + app/controllers/settings_controller.rb | 2 +- app/controllers/stats_controller.rb | 1 + app/controllers/trips_controller.rb | 1 + app/jobs/visit_suggesting_job.rb | 1 + app/models/user.rb | 6 ++ .../20250219195822_add_status_to_users.rb | 7 +++ db/schema.rb | 7 ++- spec/factories/users.rb | 2 + spec/jobs/visit_suggesting_job_spec.rb | 12 ++++ spec/models/user_spec.rb | 28 +++++++++ spec/requests/api/v1/overland/batches_spec.rb | 12 ++++ spec/requests/api/v1/owntracks/points_spec.rb | 12 ++++ spec/requests/api/v1/points_spec.rb | 62 +++++++++++++++++++ spec/requests/api/v1/settings_spec.rb | 12 ++++ spec/requests/settings_spec.rb | 13 ++++ spec/requests/stats_spec.rb | 26 ++++++++ spec/requests/trips_spec.rb | 26 ++++++++ spec/swagger/api/v1/points_controller_spec.rb | 5 +- swagger/v1/swagger.yaml | 5 +- 26 files changed, 255 insertions(+), 6 deletions(-) create mode 100644 db/migrate/20250219195822_add_status_to_users.rb diff --git a/app/controllers/api/v1/overland/batches_controller.rb b/app/controllers/api/v1/overland/batches_controller.rb index 530b7eab..db7c4ade 100644 --- a/app/controllers/api/v1/overland/batches_controller.rb +++ b/app/controllers/api/v1/overland/batches_controller.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class Api::V1::Overland::BatchesController < ApiController + before_action :authenticate_active_api_user!, only: %i[create] + def create Overland::BatchCreatingJob.perform_later(batch_params, current_api_user.id) diff --git a/app/controllers/api/v1/owntracks/points_controller.rb b/app/controllers/api/v1/owntracks/points_controller.rb index e1f8bb9a..26c53c2f 100644 --- a/app/controllers/api/v1/owntracks/points_controller.rb +++ b/app/controllers/api/v1/owntracks/points_controller.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class Api::V1::Owntracks::PointsController < ApiController + before_action :authenticate_active_api_user!, only: %i[create] + def create Owntracks::PointCreatingJob.perform_later(point_params, current_api_user.id) diff --git a/app/controllers/api/v1/points_controller.rb b/app/controllers/api/v1/points_controller.rb index f09340b8..dc34387c 100644 --- a/app/controllers/api/v1/points_controller.rb +++ b/app/controllers/api/v1/points_controller.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class Api::V1::PointsController < ApiController + before_action :authenticate_active_api_user!, only: %i[create update destroy] + def index start_at = params[:start_at]&.to_datetime&.to_i end_at = params[:end_at]&.to_datetime&.to_i || Time.zone.now.to_i diff --git a/app/controllers/api/v1/settings_controller.rb b/app/controllers/api/v1/settings_controller.rb index 316c201e..7d7e123d 100644 --- a/app/controllers/api/v1/settings_controller.rb +++ b/app/controllers/api/v1/settings_controller.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class Api::V1::SettingsController < ApiController + before_action :authenticate_active_api_user!, only: %i[update] + def index render json: { settings: current_api_user.settings, diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb index c193148e..868c72c0 100644 --- a/app/controllers/api_controller.rb +++ b/app/controllers/api_controller.rb @@ -12,6 +12,12 @@ class ApiController < ApplicationController true end + def authenticate_active_api_user! + render json: { error: 'User is not active' }, status: :unauthorized unless current_api_user&.active? + + true + end + def current_api_user @current_api_user ||= User.find_by(api_key:) end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 7b7c27d0..78071582 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -25,6 +25,12 @@ class ApplicationController < ActionController::Base redirect_to root_path, notice: 'You are not authorized to perform this action.', status: :see_other end + def authenticate_active_user! + return if current_user&.active? + + redirect_to root_path, notice: 'Your account is not active.', status: :see_other + end + private def set_self_hosted_status diff --git a/app/controllers/imports_controller.rb b/app/controllers/imports_controller.rb index 3004cf8a..a6359e67 100644 --- a/app/controllers/imports_controller.rb +++ b/app/controllers/imports_controller.rb @@ -2,6 +2,7 @@ class ImportsController < ApplicationController before_action :authenticate_user! + before_action :authenticate_active_user!, only: %i[new create] before_action :set_import, only: %i[show destroy] def index diff --git a/app/controllers/settings_controller.rb b/app/controllers/settings_controller.rb index 243189cf..82a934af 100644 --- a/app/controllers/settings_controller.rb +++ b/app/controllers/settings_controller.rb @@ -2,7 +2,7 @@ class SettingsController < ApplicationController before_action :authenticate_user! - + before_action :authenticate_active_user!, only: %i[update] def index; end def update diff --git a/app/controllers/stats_controller.rb b/app/controllers/stats_controller.rb index b7e68f41..045772f3 100644 --- a/app/controllers/stats_controller.rb +++ b/app/controllers/stats_controller.rb @@ -2,6 +2,7 @@ class StatsController < ApplicationController before_action :authenticate_user! + before_action :authenticate_active_user!, only: %i[update update_all] def index @stats = current_user.stats.group_by(&:year).sort.reverse diff --git a/app/controllers/trips_controller.rb b/app/controllers/trips_controller.rb index 038d4842..f9e57e1d 100644 --- a/app/controllers/trips_controller.rb +++ b/app/controllers/trips_controller.rb @@ -2,6 +2,7 @@ class TripsController < ApplicationController before_action :authenticate_user! + before_action :authenticate_active_user!, only: %i[new create] before_action :set_trip, only: %i[show edit update destroy] before_action :set_coordinates, only: %i[show edit] diff --git a/app/jobs/visit_suggesting_job.rb b/app/jobs/visit_suggesting_job.rb index b1a3e13d..e217dd4d 100644 --- a/app/jobs/visit_suggesting_job.rb +++ b/app/jobs/visit_suggesting_job.rb @@ -8,6 +8,7 @@ class VisitSuggestingJob < ApplicationJob users = user_ids.any? ? User.where(id: user_ids) : User.all users.find_each do |user| + next unless user.active? next if user.tracked_points.empty? Visits::Suggest.new(user, start_at:, end_at:).call diff --git a/app/models/user.rb b/app/models/user.rb index 97fb9fe0..e1a4b5cd 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -24,6 +24,8 @@ class User < ApplicationRecord attribute :admin, :boolean, default: false + enum :status, { inactive: 0, active: 1 } + def safe_settings Users::SafeSettings.new(settings) end @@ -104,6 +106,10 @@ class User < ApplicationRecord save end + def activate + update(state: :active) if DawarichSettings.self_hosted? + end + def sanitize_input settings['immich_url']&.gsub!(%r{/+\z}, '') settings['photoprism_url']&.gsub!(%r{/+\z}, '') diff --git a/db/migrate/20250219195822_add_status_to_users.rb b/db/migrate/20250219195822_add_status_to_users.rb new file mode 100644 index 00000000..eeffba08 --- /dev/null +++ b/db/migrate/20250219195822_add_status_to_users.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddStatusToUsers < ActiveRecord::Migration[8.0] + def change + add_column :users, :status, :integer, default: 0 + end +end diff --git a/db/schema.rb b/db/schema.rb index b431351f..512330a8 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.0].define(version: 2025_01_23_151657) do +ActiveRecord::Schema[8.0].define(version: 2025_02_19_195822) do # These are extensions that must be enabled in order to support this database enable_extension "pg_catalog.plpgsql" enable_extension "postgis" @@ -201,7 +201,7 @@ ActiveRecord::Schema[8.0].define(version: 2025_01_23_151657) do t.bigint "user_id", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.geometry "path", limit: {:srid=>3857, :type=>"line_string"} + t.geometry "path", limit: {srid: 3857, type: "line_string"} t.index ["user_id"], name: "index_trips_on_user_id" end @@ -215,13 +215,14 @@ ActiveRecord::Schema[8.0].define(version: 2025_01_23_151657) do t.datetime "updated_at", null: false t.string "api_key", default: "", null: false t.string "theme", default: "dark", null: false - t.jsonb "settings", default: {"fog_of_war_meters"=>"100", "meters_between_routes"=>"1000", "minutes_between_routes"=>"60"} + t.jsonb "settings", default: {"fog_of_war_meters" => "100", "meters_between_routes" => "1000", "minutes_between_routes" => "60"} t.boolean "admin", default: false t.integer "sign_in_count", default: 0, null: false t.datetime "current_sign_in_at" t.datetime "last_sign_in_at" t.string "current_sign_in_ip" t.string "last_sign_in_ip" + t.integer "status", default: 0 t.index ["email"], name: "index_users_on_email", unique: true t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true end diff --git a/spec/factories/users.rb b/spec/factories/users.rb index f3fe9d7b..1f1c54bc 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -6,6 +6,8 @@ FactoryBot.define do "user#{n}@example.com" end + status { :active } + password { SecureRandom.hex(8) } settings do diff --git a/spec/jobs/visit_suggesting_job_spec.rb b/spec/jobs/visit_suggesting_job_spec.rb index f2ce47d9..271d7675 100644 --- a/spec/jobs/visit_suggesting_job_spec.rb +++ b/spec/jobs/visit_suggesting_job_spec.rb @@ -30,5 +30,17 @@ RSpec.describe VisitSuggestingJob, type: :job do expect(Visits::Suggest).to have_received(:new) end end + + context 'when user is inactive' do + before do + users.first.update(status: :inactive) + end + + it 'does not suggest visits' do + subject + + expect(Visits::Suggest).not_to have_received(:new) + end + end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 398e436f..a5f0b45e 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -16,6 +16,10 @@ RSpec.describe User, type: :model do it { is_expected.to have_many(:trips).dependent(:destroy) } end + describe 'enums' do + it { is_expected.to define_enum_for(:status).with_values(inactive: 0, active: 1) } + end + describe 'callbacks' do describe '#create_api_key' do let(:user) { create(:user) } @@ -24,6 +28,30 @@ RSpec.describe User, type: :model do expect(user.api_key).to be_present end end + + describe '#activate' do + let(:user) { create(:user) } + + context 'when self-hosted' do + before do + allow(DawarichSettings).to receive(:self_hosted?).and_return(true) + end + + it 'activates user' do + expect { user.send(:activate) }.to change(user, :status).to('active') + end + end + + context 'when not self-hosted' do + before do + allow(DawarichSettings).to receive(:self_hosted?).and_return(false) + end + + it 'does not activate user' do + expect { user.send(:activate) }.to_not change(user, :status) + end + end + end end describe 'methods' do diff --git a/spec/requests/api/v1/overland/batches_spec.rb b/spec/requests/api/v1/overland/batches_spec.rb index 912aa280..a673480b 100644 --- a/spec/requests/api/v1/overland/batches_spec.rb +++ b/spec/requests/api/v1/overland/batches_spec.rb @@ -31,6 +31,18 @@ RSpec.describe 'Api::V1::Overland::Batches', type: :request do post "/api/v1/overland/batches?api_key=#{user.api_key}", params: params end.to have_enqueued_job(Overland::BatchCreatingJob) end + + context 'when user is inactive' do + before do + user.update(status: :inactive) + end + + it 'returns http unauthorized' do + post "/api/v1/overland/batches?api_key=#{user.api_key}", params: params + + expect(response).to have_http_status(:unauthorized) + end + end end end end diff --git a/spec/requests/api/v1/owntracks/points_spec.rb b/spec/requests/api/v1/owntracks/points_spec.rb index e99b6c6f..39cf486f 100644 --- a/spec/requests/api/v1/owntracks/points_spec.rb +++ b/spec/requests/api/v1/owntracks/points_spec.rb @@ -31,6 +31,18 @@ RSpec.describe 'Api::V1::Owntracks::Points', type: :request do end.to have_enqueued_job(Owntracks::PointCreatingJob) end end + + context 'when user is inactive' do + before do + user.update(status: :inactive) + end + + it 'returns http unauthorized' do + post api_v1_owntracks_points_path(api_key: user.api_key), params: params + + expect(response).to have_http_status(:unauthorized) + end + end end end end diff --git a/spec/requests/api/v1/points_spec.rb b/spec/requests/api/v1/points_spec.rb index 3d5f49d8..f218d085 100644 --- a/spec/requests/api/v1/points_spec.rb +++ b/spec/requests/api/v1/points_spec.rb @@ -119,4 +119,66 @@ RSpec.describe 'Api::V1::Points', type: :request do end end end + + describe 'POST /create' do + it 'returns a successful response' do + post "/api/v1/points?api_key=#{user.api_key}", params: { point: { latitude: 1.0, longitude: 1.0 } } + + expect(response).to have_http_status(:success) + end + + context 'when user is inactive' do + before do + user.update(status: :inactive) + end + + it 'returns an unauthorized response' do + post "/api/v1/points?api_key=#{user.api_key}", params: { point: { latitude: 1.0, longitude: 1.0 } } + + expect(response).to have_http_status(:unauthorized) + end + end + end + + describe 'PUT /update' do + it 'returns a successful response' do + put "/api/v1/points/#{points.first.id}?api_key=#{user.api_key}", + params: { point: { latitude: 1.0, longitude: 1.1 } } + + expect(response).to have_http_status(:success) + end + + context 'when user is inactive' do + before do + user.update(status: :inactive) + end + + it 'returns an unauthorized response' do + put "/api/v1/points/#{points.first.id}?api_key=#{user.api_key}", + params: { point: { latitude: 1.0, longitude: 1.1 } } + + expect(response).to have_http_status(:unauthorized) + end + end + end + + describe 'DELETE /destroy' do + it 'returns a successful response' do + delete "/api/v1/points/#{points.first.id}?api_key=#{user.api_key}" + + expect(response).to have_http_status(:success) + end + + context 'when user is inactive' do + before do + user.update(status: :inactive) + end + + it 'returns an unauthorized response' do + delete "/api/v1/points/#{points.first.id}?api_key=#{user.api_key}" + + expect(response).to have_http_status(:unauthorized) + end + end + end end diff --git a/spec/requests/api/v1/settings_spec.rb b/spec/requests/api/v1/settings_spec.rb index 69498221..075e3dca 100644 --- a/spec/requests/api/v1/settings_spec.rb +++ b/spec/requests/api/v1/settings_spec.rb @@ -25,6 +25,18 @@ RSpec.describe 'Api::V1::Settings', type: :request do expect(response.parsed_body['settings']['route_opacity'].to_f).to eq(0.3) end + + context 'when user is inactive' do + before do + user.update(status: :inactive) + end + + it 'returns http unauthorized' do + patch "/api/v1/settings?api_key=#{api_key}", params: { settings: { route_opacity: 0.3 } } + + expect(response).to have_http_status(:unauthorized) + end + end end context 'with invalid request' do diff --git a/spec/requests/settings_spec.rb b/spec/requests/settings_spec.rb index 0ced085e..f457855c 100644 --- a/spec/requests/settings_spec.rb +++ b/spec/requests/settings_spec.rb @@ -82,6 +82,19 @@ RSpec.describe 'Settings', type: :request do expect(user.reload.settings).to eq(params[:settings]) end + + context 'when user is inactive' do + before do + user.update(status: :inactive) + end + + it 'redirects to the root path' do + patch '/settings', params: params + + expect(response).to redirect_to(root_path) + expect(flash[:notice]).to eq('Your account is not active.') + end + end end describe 'GET /settings/users' do diff --git a/spec/requests/stats_spec.rb b/spec/requests/stats_spec.rb index c7473bed..516f4cd3 100644 --- a/spec/requests/stats_spec.rb +++ b/spec/requests/stats_spec.rb @@ -69,6 +69,19 @@ RSpec.describe '/stats', type: :request do end end end + + context 'when user is inactive' do + before do + user.update(status: :inactive) + end + + it 'returns an unauthorized response' do + put update_year_month_stats_url(year: '2024', month: '1') + + expect(response).to redirect_to(root_path) + expect(flash[:notice]).to eq('Your account is not active.') + end + end end describe 'PUT /update_all' do @@ -83,6 +96,19 @@ RSpec.describe '/stats', type: :request do expect(Stats::CalculatingJob).to have_been_enqueued.with(user.id, 2024, 2) expect(Stats::CalculatingJob).to_not have_been_enqueued.with(user.id, 2024, 3) end + + context 'when user is inactive' do + before do + user.update(status: :inactive) + end + + it 'returns an unauthorized response' do + put update_all_stats_url + + expect(response).to redirect_to(root_path) + expect(flash[:notice]).to eq('Your account is not active.') + end + end end end end diff --git a/spec/requests/trips_spec.rb b/spec/requests/trips_spec.rb index d0e1e794..3f536edc 100644 --- a/spec/requests/trips_spec.rb +++ b/spec/requests/trips_spec.rb @@ -53,6 +53,19 @@ RSpec.describe '/trips', type: :request do expect(response).to be_successful end + + context 'when user is inactive' do + before do + user.update(status: :inactive) + end + + it 'redirects to the root path' do + get new_trip_url + + expect(response).to redirect_to(root_path) + expect(flash[:notice]).to eq('Your account is not active.') + end + end end describe 'GET /edit' do @@ -77,6 +90,19 @@ RSpec.describe '/trips', type: :request do post trips_url, params: { trip: valid_attributes } expect(response).to redirect_to(trip_url(Trip.last)) end + + context 'when user is inactive' do + before do + user.update(status: :inactive) + end + + it 'redirects to the root path' do + post trips_url, params: { trip: valid_attributes } + + expect(response).to redirect_to(root_path) + expect(flash[:notice]).to eq('Your account is not active.') + end + end end context 'with invalid parameters' do diff --git a/spec/swagger/api/v1/points_controller_spec.rb b/spec/swagger/api/v1/points_controller_spec.rb index e5b8bf01..7b710f2d 100644 --- a/spec/swagger/api/v1/points_controller_spec.rb +++ b/spec/swagger/api/v1/points_controller_spec.rb @@ -78,6 +78,9 @@ describe 'Points API', type: :request do coordinates: [-122.40530871, 37.74430413] }, properties: { + battery_state: 'full', + battery_level: 0.7, + wifi: 'dawarich_home', timestamp: '2025-01-17T21:03:01Z', horizontal_accuracy: 5, vertical_accuracy: -1, @@ -92,7 +95,7 @@ describe 'Points API', type: :request do } ] } - tags 'Batches' + tags 'Points' consumes 'application/json' parameter name: :locations, in: :body, schema: { type: :object, diff --git a/swagger/v1/swagger.yaml b/swagger/v1/swagger.yaml index 3ce30e09..57db976d 100644 --- a/swagger/v1/swagger.yaml +++ b/swagger/v1/swagger.yaml @@ -828,7 +828,7 @@ paths: post: summary: Creates a batch of points tags: - - Batches + - Points parameters: - name: api_key in: query @@ -918,6 +918,9 @@ paths: - -122.40530871 - 37.74430413 properties: + battery_state: full + battery_level: 0.7 + wifi: dawarich_home timestamp: '2025-01-17T21:03:01Z' horizontal_accuracy: 5 vertical_accuracy: -1 From 5bd6e27fc8746a7d44fc914c277dba99e9be9539 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Wed, 19 Feb 2025 21:25:22 +0100 Subject: [PATCH 2/4] Update changelog --- .app_version | 2 +- CHANGELOG.md | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/.app_version b/.app_version index 48b91fd8..8b95abd9 100644 --- a/.app_version +++ b/.app_version @@ -1 +1 @@ -0.24.1 +0.24.2 diff --git a/CHANGELOG.md b/CHANGELOG.md index 34d90083..61cec6c5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/). # 0.24.2 - 2025-02-15 +## Added + +- Status field to the User model. Inactive users are now being restricted from accessing some of the functionality, which is mostly about writing data to the database. Reading is remaining unrestricted. + ## Fixed - Fixed a bug where background jobs to import Immich and Photoprism geolocation data data could not be created by non-admin users. From 85049b398b16ecd79d973867eaaa8b63ab13ffa8 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Tue, 25 Feb 2025 00:04:05 +0100 Subject: [PATCH 3/4] Fix user status --- app/models/user.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/user.rb b/app/models/user.rb index e1a4b5cd..14de61fa 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -107,7 +107,7 @@ class User < ApplicationRecord end def activate - update(state: :active) if DawarichSettings.self_hosted? + update(status: :active) if DawarichSettings.self_hosted? end def sanitize_input From 080da9f2de31825cd09022a3533b66dd34fc3c64 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Tue, 25 Feb 2025 00:16:42 +0100 Subject: [PATCH 4/4] Update tests --- app/models/user.rb | 3 ++- spec/models/user_spec.rb | 15 +++++++++------ 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 14de61fa..2f6499d2 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -16,6 +16,7 @@ class User < ApplicationRecord has_many :trips, dependent: :destroy after_create :create_api_key + after_commit :activate, on: :create, if: -> { DawarichSettings.self_hosted? } before_save :sanitize_input validates :email, presence: true @@ -107,7 +108,7 @@ class User < ApplicationRecord end def activate - update(status: :active) if DawarichSettings.self_hosted? + update(status: :active) end def sanitize_input diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index a5f0b45e..0c5d60e3 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -30,25 +30,28 @@ RSpec.describe User, type: :model do end describe '#activate' do - let(:user) { create(:user) } - context 'when self-hosted' do + let!(:user) { create(:user, status: :inactive) } + before do allow(DawarichSettings).to receive(:self_hosted?).and_return(true) end - it 'activates user' do - expect { user.send(:activate) }.to change(user, :status).to('active') + it 'activates user after creation' do + expect(user.active?).to be_truthy end end context 'when not self-hosted' do + let!(:user) { create(:user, status: :inactive) } + before do + stub_const('SELF_HOSTED', false) allow(DawarichSettings).to receive(:self_hosted?).and_return(false) end - it 'does not activate user' do - expect { user.send(:activate) }.to_not change(user, :status) + xit 'does not activate user' do + expect(user.active?).to be_falsey end end end