From 30780a596140ea5cd67970c69f148e198a981c87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Orange=F0=9F=8D=8A?= Date: Mon, 29 Jun 2026 03:59:44 +0800 Subject: [PATCH] refactor(api): scope controllers through current_resource_owner (#2414) Follow-up to #2405. Replace remaining API controller reads of Current.user, Current.family, and Current.session with current_resource_owner. Add an architecture guard test to prevent regression. Scope is limited to the Current sweep only: - Revert balance_sheet user-scoping (moves to account-auth PR B). - Revert provider_connections DebugLogEntry logging (separate PR). - Remove UsersController#destroy attempt to destroy unsaved API session. --- app/controllers/api/v1/chats_controller.rb | 6 +-- .../api/v1/import_sessions_controller.rb | 4 +- app/controllers/api/v1/messages_controller.rb | 2 +- .../api/v1/provider_connections_controller.rb | 2 +- .../api/v1/rule_runs_controller.rb | 2 +- app/controllers/api/v1/syncs_controller.rb | 2 +- app/controllers/api/v1/users_controller.rb | 1 - test/architecture/api_current_usage_test.rb | 49 +++++++++++++++++++ 8 files changed, 58 insertions(+), 10 deletions(-) create mode 100644 test/architecture/api_current_usage_test.rb diff --git a/app/controllers/api/v1/chats_controller.rb b/app/controllers/api/v1/chats_controller.rb index ad1a2b228..a739000c3 100644 --- a/app/controllers/api/v1/chats_controller.rb +++ b/app/controllers/api/v1/chats_controller.rb @@ -8,7 +8,7 @@ class Api::V1::ChatsController < Api::V1::BaseController before_action :set_chat, only: [ :show, :update, :destroy ] def index - @pagy, @chats = pagy(Current.user.chats.ordered, items: 20) + @pagy, @chats = pagy(current_resource_owner.chats.ordered, items: 20) end def show @@ -17,7 +17,7 @@ class Api::V1::ChatsController < Api::V1::BaseController end def create - @chat = Current.user.chats.build(title: chat_params[:title]) + @chat = current_resource_owner.chats.build(title: chat_params[:title]) if @chat.save if chat_params[:message].present? @@ -74,7 +74,7 @@ class Api::V1::ChatsController < Api::V1::BaseController end def set_chat - @chat = Current.user.chats.find(params[:id]) + @chat = current_resource_owner.chats.find(params[:id]) rescue ActiveRecord::RecordNotFound render json: { error: "Chat not found" }, status: :not_found end diff --git a/app/controllers/api/v1/import_sessions_controller.rb b/app/controllers/api/v1/import_sessions_controller.rb index f749124d7..212c97c2a 100644 --- a/app/controllers/api/v1/import_sessions_controller.rb +++ b/app/controllers/api/v1/import_sessions_controller.rb @@ -7,7 +7,7 @@ class Api::V1::ImportSessionsController < Api::V1::BaseController def create @import_session = ImportSession.create_or_find_for!( - family: Current.family, + family: current_resource_owner.family, import_type: params[:type].to_s, client_session_id: params[:client_session_id].presence, expected_chunks: expected_chunks_param @@ -68,7 +68,7 @@ class Api::V1::ImportSessionsController < Api::V1::BaseController private def set_import_session - @import_session = Current.family.import_sessions.find(params[:id]) + @import_session = current_resource_owner.family.import_sessions.find(params[:id]) end def ensure_read_scope diff --git a/app/controllers/api/v1/messages_controller.rb b/app/controllers/api/v1/messages_controller.rb index f9f3b8388..5076dde0b 100644 --- a/app/controllers/api/v1/messages_controller.rb +++ b/app/controllers/api/v1/messages_controller.rb @@ -49,7 +49,7 @@ class Api::V1::MessagesController < Api::V1::BaseController end def set_chat - @chat = Current.user.chats.find(params[:chat_id]) + @chat = current_resource_owner.chats.find(params[:chat_id]) rescue ActiveRecord::RecordNotFound render json: { error: "Chat not found" }, status: :not_found end diff --git a/app/controllers/api/v1/provider_connections_controller.rb b/app/controllers/api/v1/provider_connections_controller.rb index 11e0c6f7b..d8ac2c240 100644 --- a/app/controllers/api/v1/provider_connections_controller.rb +++ b/app/controllers/api/v1/provider_connections_controller.rb @@ -4,7 +4,7 @@ class Api::V1::ProviderConnectionsController < Api::V1::BaseController before_action :ensure_read_scope def index - @provider_connections = ProviderConnectionStatus.for_family(Current.family) + @provider_connections = ProviderConnectionStatus.for_family(current_resource_owner.family) render :index rescue StandardError => e Rails.logger.error "ProviderConnectionsController#index error: #{e.message}" diff --git a/app/controllers/api/v1/rule_runs_controller.rb b/app/controllers/api/v1/rule_runs_controller.rb index 4aeae3a62..79120c9d6 100644 --- a/app/controllers/api/v1/rule_runs_controller.rb +++ b/app/controllers/api/v1/rule_runs_controller.rb @@ -44,7 +44,7 @@ class Api::V1::RuleRunsController < Api::V1::BaseController def rule_runs_scope RuleRun .joins(:rule) - .where(rules: { family_id: Current.family.id }) + .where(rules: { family_id: current_resource_owner.family.id }) .includes(:rule) end diff --git a/app/controllers/api/v1/syncs_controller.rb b/app/controllers/api/v1/syncs_controller.rb index 401362392..4f44df55b 100644 --- a/app/controllers/api/v1/syncs_controller.rb +++ b/app/controllers/api/v1/syncs_controller.rb @@ -41,6 +41,6 @@ class Api::V1::SyncsController < Api::V1::BaseController end def family_syncs_query - Sync.for_family(Current.family, resource_owner: Current.user) + Sync.for_family(current_resource_owner.family, resource_owner: current_resource_owner) end end diff --git a/app/controllers/api/v1/users_controller.rb b/app/controllers/api/v1/users_controller.rb index fa1c934a9..291304914 100644 --- a/app/controllers/api/v1/users_controller.rb +++ b/app/controllers/api/v1/users_controller.rb @@ -45,7 +45,6 @@ class Api::V1::UsersController < Api::V1::BaseController user = current_resource_owner if user.deactivate - Current.session&.destroy render json: { message: "Account has been deleted" } else render json: { error: "Failed to delete account", details: user.errors.full_messages }, status: :unprocessable_entity diff --git a/test/architecture/api_current_usage_test.rb b/test/architecture/api_current_usage_test.rb new file mode 100644 index 000000000..b58ece19c --- /dev/null +++ b/test/architecture/api_current_usage_test.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +require "test_helper" + +class ApiCurrentUsageTest < ActiveSupport::TestCase + API_CONTROLLER_GLOB = Rails.root.join("app/controllers/api/**/*.rb").to_s + BASE_CONTROLLER = Rails.root.join("app/controllers/api/v1/base_controller.rb").to_s + DISALLOWED_CURRENT_REFERENCES = [ + "Current.user", + "Current.family", + "Current.session" + ].freeze + + # Api::V1::BaseController may set an unsaved session as a compatibility bridge + # for code paths that still derive Current.user from Current.session.user. + # Add new entries only when they are part of that bridge and document why. + ALLOWED_BASE_CONTROLLER_REFERENCES = [ + "Current.session = @current_user.sessions.build(", + "Current.session.active_impersonator_session = nil" + ].freeze + + test "api controllers scope through current_resource_owner instead of Current" do + violations = [] + + Dir.glob(API_CONTROLLER_GLOB).sort.each do |path| + File.readlines(path).each.with_index(1) do |line, line_number| + next unless DISALLOWED_CURRENT_REFERENCES.any? { |reference| line.include?(reference) } + next if allowed_base_controller_reference?(path, line) + + relative_path = Pathname.new(path).relative_path_from(Rails.root) + violations << "#{relative_path}:#{line_number}: #{line.strip}" + end + end + + assert_empty violations, <<~MESSAGE + API controllers should not read Current.user, Current.family, or Current.session. + + Use current_resource_owner/current_resource_owner.family for API auth scoping. + The only allowed Current.session usage is the compatibility bridge in Api::V1::BaseController. + + #{violations.join("\n")} + MESSAGE + end + + private + def allowed_base_controller_reference?(path, line) + path == BASE_CONTROLLER && ALLOWED_BASE_CONTROLLER_REFERENCES.any? { |reference| line.include?(reference) } + end +end