diff --git a/app/controllers/api/v1/base_controller.rb b/app/controllers/api/v1/base_controller.rb index f1631ec1c..f69e6f4be 100644 --- a/app/controllers/api/v1/base_controller.rb +++ b/app/controllers/api/v1/base_controller.rb @@ -310,23 +310,15 @@ class Api::V1::BaseController < ApplicationController # Set up Current context for API requests since we don't use session-based auth def setup_current_context_for_api - # For API requests, we need to create a minimal session-like object - # or find/create an actual session for this user to make Current.user work - if @current_user - # Try to find an existing session for this user, or create a temporary one - session = @current_user.sessions.first - if session - Current.session = session - else - # Create a temporary session for this API request - # This won't be persisted but will allow Current.user to work - session = @current_user.sessions.build( - user_agent: request.user_agent, - ip_address: request.ip - ) - Current.session = session - end - end + return unless @current_user + + # Build a fresh unsaved session so API requests never reuse an existing + # web session that may already carry impersonation state. + Current.session = @current_user.sessions.build( + user_agent: request.user_agent, + ip_address: request.ip + ) + Current.session.active_impersonator_session = nil end # Check if AI features are enabled for the current user diff --git a/app/controllers/reports_controller.rb b/app/controllers/reports_controller.rb index d1b76204e..f1159b28c 100644 --- a/app/controllers/reports_controller.rb +++ b/app/controllers/reports_controller.rb @@ -1060,14 +1060,18 @@ class ReportsController < ApplicationController return false end - # Find or create a session for this API request - # We need to find or create a persisted session so that Current.user delegation works properly - session = @current_user.sessions.first_or_create!( + unless @current_user.active? + render plain: "Invalid or expired API key", status: :unauthorized + return false + end + + # Build a fresh unsaved session so API key exports never reuse an existing + # web session that may already carry impersonation state. + Current.session = @current_user.sessions.build( user_agent: request.user_agent, ip_address: request.ip ) - - Current.session = session + Current.session.active_impersonator_session = nil # Verify the delegation chain works unless Current.user diff --git a/test/controllers/api/v1/chats_controller_test.rb b/test/controllers/api/v1/chats_controller_test.rb index f56ed3536..c7dbcdf76 100644 --- a/test/controllers/api/v1/chats_controller_test.rb +++ b/test/controllers/api/v1/chats_controller_test.rb @@ -125,6 +125,68 @@ class Api::V1::ChatsControllerTest < ActionDispatch::IntegrationTest assert_response :success end + test "API key authentication should not inherit web session impersonation" do + support_user = users(:sure_support_staff) + support_user.update!(ai_enabled: true) + support_user.api_keys.active.destroy_all + support_chat = support_user.chats.create!(title: "Support Chat") + + token_value = ApiKey.generate_secure_key + credential = support_user.api_keys.build( + name: "Impersonation Guard Credential", + scopes: [ "read" ] + ) + credential.key = token_value + credential.save! + + impersonation_session = impersonation_sessions(:in_progress) + impersonated_chat = impersonation_session.impersonated.chats.create!(title: "Impersonated Chat") + support_user.sessions.destroy_all + support_user.sessions.create!( + user_agent: "Browser session", + ip_address: "127.0.0.1", + active_impersonator_session: impersonation_session + ) + + get "/api/v1/chats", headers: { "X-Api-Key" => token_value } + + assert_response :success + response_body = JSON.parse(response.body) + chat_ids = response_body["chats"].pluck("id") + + assert_includes chat_ids, support_chat.id + assert_not_includes chat_ids, impersonated_chat.id + end + + test "OAuth authentication should not inherit web session impersonation" do + support_user = users(:sure_support_staff) + support_user.update!(ai_enabled: true) + support_chat = support_user.chats.create!(title: "Support Chat") + support_token = Doorkeeper::AccessToken.create!( + application: @oauth_app, + resource_owner_id: support_user.id, + scopes: "read" + ) + + impersonation_session = impersonation_sessions(:in_progress) + impersonated_chat = impersonation_session.impersonated.chats.create!(title: "Impersonated Chat") + support_user.sessions.destroy_all + support_user.sessions.create!( + user_agent: "Browser session", + ip_address: "127.0.0.1", + active_impersonator_session: impersonation_session + ) + + get "/api/v1/chats", headers: bearer_auth_header(support_token) + + assert_response :success + response_body = JSON.parse(response.body) + chat_ids = response_body["chats"].pluck("id") + + assert_includes chat_ids, support_chat.id + assert_not_includes chat_ids, impersonated_chat.id + end + private def bearer_auth_header(token) diff --git a/test/controllers/reports_controller_test.rb b/test/controllers/reports_controller_test.rb index 4c75de2c3..01baf8b4f 100644 --- a/test/controllers/reports_controller_test.rb +++ b/test/controllers/reports_controller_test.rb @@ -247,6 +247,79 @@ class ReportsControllerTest < ActionDispatch::IntegrationTest assert_match /Category/, @response.body end + test "export transactions with API key does not inherit web session impersonation" do + support_user = users(:sure_support_staff) + support_user.api_keys.active.destroy_all + token_value = ApiKey.generate_secure_key + export_credential = support_user.api_keys.build( + name: "Support Export Credential", + scopes: [ "read" ], + source: "web" + ) + export_credential.key = token_value + export_credential.save! + + impersonation_session = impersonation_sessions(:in_progress) + impersonated_user = impersonation_session.impersonated + leaked_category = impersonated_user.family.categories.create!( + name: "Impersonated Export Leak Probe" + ) + impersonated_account = impersonated_user.finance_accounts.first + assert_not_nil impersonated_account, "Test setup failed: impersonated user has no finance account" + + create_transaction( + account: impersonated_account, + name: "Impersonated export leak probe", + date: Date.current, + amount: 123.45, + category: leaked_category + ) + + support_user.sessions.destroy_all + support_user.sessions.create!( + user_agent: "Browser session", + ip_address: "127.0.0.1", + active_impersonator_session: impersonation_session + ) + + get export_transactions_reports_path( + format: :csv, + period_type: :ytd, + start_date: Date.current.beginning_of_year, + end_date: Date.current, + api_key: token_value + ) + + assert_response :ok + assert_equal "text/csv", @response.media_type + assert_no_match /Impersonated Export Leak Probe/, @response.body + end + + test "export transactions with API key rejects deactivated user" do + user = users(:family_admin) + user.api_keys.active.destroy_all + token_value = ApiKey.generate_secure_key + export_access = user.api_keys.build( + name: "Inactive User Export Credential", + scopes: [ "read" ], + source: "web" + ) + export_access.key = token_value + export_access.save! + user.update_column(:active, false) + + get export_transactions_reports_path( + format: :csv, + period_type: :ytd, + api_key: token_value + ) + + assert_response :unauthorized + assert_match /Invalid or expired API key/, @response.body + ensure + user&.update_column(:active, true) + end + test "export transactions with invalid API key" do get export_transactions_reports_path( format: :csv,