From cc043b5caf098bafdfd3ba9b71037bd833891c26 Mon Sep 17 00:00:00 2001 From: ghost <49853598+JSONbored@users.noreply.github.com> Date: Fri, 1 May 2026 07:22:28 -0600 Subject: [PATCH] feat(api): expose complete account export state (#1597) * feat(api): expose complete account export state * fix(api): handle malformed account identifiers * fix(api): tighten account export contracts * fix(api): correct account id OpenAPI format * fix(api): tighten account docs auth contracts * docs(api): document balance sheet auth errors * docs(api): clarify account scope fixture --- app/controllers/api/v1/accounts_controller.rb | 111 ++-- .../api/v1/accounts/_account.json.jbuilder | 20 + app/views/api/v1/accounts/index.json.jbuilder | 7 +- app/views/api/v1/accounts/show.json.jbuilder | 3 + docs/api/openapi.yaml | 567 ++++++++++++++---- spec/requests/api/v1/accounts_spec.rb | 62 ++ spec/requests/api/v1/auth_spec.rb | 5 +- spec/requests/api/v1/balance_sheet_spec.rb | 2 + spec/swagger_helper.rb | 24 +- .../api/v1/accounts_controller_test.rb | 272 ++++++--- 10 files changed, 808 insertions(+), 265 deletions(-) create mode 100644 app/views/api/v1/accounts/_account.json.jbuilder create mode 100644 app/views/api/v1/accounts/show.json.jbuilder diff --git a/app/controllers/api/v1/accounts_controller.rb b/app/controllers/api/v1/accounts_controller.rb index 07b654112..5e849a2c4 100644 --- a/app/controllers/api/v1/accounts_controller.rb +++ b/app/controllers/api/v1/accounts_controller.rb @@ -7,53 +7,86 @@ class Api::V1::AccountsController < Api::V1::BaseController before_action :ensure_read_scope def index - # Test with Pagy pagination - family = current_resource_owner.family - accounts_query = family.accounts.accessible_by(current_resource_owner).visible.alphabetically - - # Handle pagination with Pagy - @pagy, @accounts = pagy( - accounts_query, - page: safe_page_param, - limit: safe_per_page_param - ) - @per_page = safe_per_page_param - # Rails will automatically use app/views/api/v1/accounts/index.json.jbuilder + @pagy, @accounts = pagy( + accounts_scope.alphabetically, + page: safe_page_param, + limit: @per_page + ) + render :index rescue => e - Rails.logger.error "AccountsController error: #{e.message}" + Rails.logger.error "AccountsController#index error: #{e.message}" Rails.logger.error e.backtrace.join("\n") render json: { error: "internal_server_error", message: "An unexpected error occurred" }, status: :internal_server_error -end - - private - - def ensure_read_scope - authorize_scope!(:read) - end - - - - def safe_page_param - page = params[:page].to_i - page > 0 ? page : 1 - end - - def safe_per_page_param - per_page = params[:per_page].to_i - - # Default to 25, max 100 - case per_page - when 1..100 - per_page - else - 25 - end - end + end + + def show + unless valid_uuid?(params[:id]) + render json: { + error: "not_found", + message: "Account not found" + }, status: :not_found + return + end + + @account = accounts_scope.find(params[:id]) + + render :show + rescue ActiveRecord::RecordNotFound + render json: { + error: "not_found", + message: "Account not found" + }, status: :not_found + rescue => e + Rails.logger.error "AccountsController#show error: #{e.message}" + Rails.logger.error e.backtrace.join("\n") + + render json: { + error: "internal_server_error", + message: "An unexpected error occurred" + }, status: :internal_server_error + end + + private + + def ensure_read_scope + authorize_scope!(:read) + end + + def accounts_scope + scope = current_resource_owner.family.accounts + .accessible_by(current_resource_owner) + .includes(:accountable, account_providers: :provider) + include_disabled_accounts? ? scope : scope.visible + end + + def include_disabled_accounts? + ActiveModel::Type::Boolean.new.cast(params[:include_disabled]) + end + + def valid_uuid?(value) + value.to_s.match?(/\A[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\z/i) + end + + def safe_page_param + page = params[:page].to_i + page > 0 ? page : 1 + end + + def safe_per_page_param + per_page = params[:per_page].to_i + + case per_page + when 1..100 + per_page + else + 25 + end + end end diff --git a/app/views/api/v1/accounts/_account.json.jbuilder b/app/views/api/v1/accounts/_account.json.jbuilder new file mode 100644 index 000000000..b38c3b1a6 --- /dev/null +++ b/app/views/api/v1/accounts/_account.json.jbuilder @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +balance_money = account.balance_money +cash_balance_money = account.cash_balance_money + +json.id account.id +json.name account.name +json.balance balance_money.format +json.balance_cents((balance_money.amount * balance_money.currency.minor_unit_conversion).round(0).to_i) +json.cash_balance cash_balance_money.format +json.cash_balance_cents((cash_balance_money.amount * cash_balance_money.currency.minor_unit_conversion).round(0).to_i) +json.currency account.currency +json.classification account.classification +json.account_type account.accountable_type&.underscore +json.subtype account.subtype +json.status account.status +json.institution_name account.institution_name +json.institution_domain account.institution_domain +json.created_at account.created_at.iso8601 +json.updated_at account.updated_at.iso8601 diff --git a/app/views/api/v1/accounts/index.json.jbuilder b/app/views/api/v1/accounts/index.json.jbuilder index e6e8bfeec..889e2e5d4 100644 --- a/app/views/api/v1/accounts/index.json.jbuilder +++ b/app/views/api/v1/accounts/index.json.jbuilder @@ -1,12 +1,7 @@ # frozen_string_literal: true json.accounts @accounts do |account| - json.id account.id - json.name account.name - json.balance account.balance_money.format - json.currency account.currency - json.classification account.classification - json.account_type account.accountable_type.underscore + json.partial! "account", account: account end json.pagination do diff --git a/app/views/api/v1/accounts/show.json.jbuilder b/app/views/api/v1/accounts/show.json.jbuilder new file mode 100644 index 000000000..8a29ac174 --- /dev/null +++ b/app/views/api/v1/accounts/show.json.jbuilder @@ -0,0 +1,3 @@ +# frozen_string_literal: true + +json.partial! "account", account: @account diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index e71c1568c..5b9000e51 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -61,6 +61,16 @@ components: nullable: true description: Validation error messages (alternative to details used by trades, valuations, etc.) + MfaRequiredResponse: + type: object + required: + - error + - mfa_required + properties: + error: + type: string + mfa_required: + type: boolean ToolCall: type: object required: @@ -230,15 +240,24 @@ components: type: string account_type: type: string + nullable: true + status: + type: string AccountDetail: type: object required: - id - name - balance + - balance_cents + - cash_balance + - cash_balance_cents - currency - classification - account_type + - status + - created_at + - updated_at properties: id: type: string @@ -247,12 +266,43 @@ components: type: string balance: type: string + balance_cents: + type: integer + description: Signed balance in minor currency units + cash_balance: + type: string + cash_balance_cents: + type: integer + description: Signed cash balance in minor currency units currency: type: string classification: type: string account_type: type: string + nullable: true + subtype: + type: string + nullable: true + status: + type: string + enum: + - active + - draft + - disabled + - pending_deletion + institution_name: + type: string + nullable: true + institution_domain: + type: string + nullable: true + created_at: + type: string + format: date-time + updated_at: + type: string + format: date-time AccountCollection: type: object required: @@ -270,7 +320,6 @@ components: required: - id - name - - classification - color - icon properties: @@ -279,8 +328,6 @@ components: format: uuid name: type: string - classification: - type: string color: type: string icon: @@ -301,7 +348,6 @@ components: required: - id - name - - classification - color - icon - subcategories_count @@ -313,11 +359,6 @@ components: format: uuid name: type: string - classification: - type: string - enum: - - income - - expense color: type: string icon: @@ -545,13 +586,6 @@ components: properties: message: type: string - SuccessMessage: - type: object - required: - - message - properties: - message: - type: string ImportConfiguration: type: object properties: @@ -879,50 +913,47 @@ components: "$ref": "#/components/schemas/Holding" pagination: "$ref": "#/components/schemas/Pagination" + Money: + type: object + required: + - amount + - currency + - formatted + properties: + amount: + type: string + description: Numeric amount as string + currency: + type: string + description: ISO 4217 currency code + formatted: + type: string + description: Locale-formatted money string + BalanceSheet: + type: object + required: + - currency + - net_worth + - assets + - liabilities + properties: + currency: + type: string + description: Family primary currency + net_worth: + "$ref": "#/components/schemas/Money" + assets: + "$ref": "#/components/schemas/Money" + liabilities: + "$ref": "#/components/schemas/Money" + SuccessMessage: + type: object + required: + - message + properties: + message: + type: string paths: - "/api/v1/merchants": - get: - summary: List merchants - tags: - - Merchants - security: - - apiKeyAuth: [] - responses: - '200': - description: merchants listed - content: - application/json: - schema: - type: array - items: - "$ref": "#/components/schemas/MerchantDetail" - "/api/v1/merchants/{id}": - parameters: - - name: id - in: path - required: true - description: Merchant ID - schema: - type: string - get: - summary: Retrieve a merchant - tags: - - Merchants - security: - - apiKeyAuth: [] - responses: - '200': - description: merchant retrieved - content: - application/json: - schema: - "$ref": "#/components/schemas/MerchantDetail" - '404': - description: merchant not found - content: - application/json: - schema: - "$ref": "#/components/schemas/ErrorResponse" "/api/v1/accounts": get: summary: List accounts @@ -943,6 +974,12 @@ paths: description: 'Items per page (default: 25, max: 100)' schema: type: integer + - name: include_disabled + in: query + required: false + description: Include disabled accounts in the response. Defaults to false. + schema: + type: boolean responses: '200': description: accounts paginated @@ -950,6 +987,53 @@ paths: application/json: schema: "$ref": "#/components/schemas/AccountCollection" + "/api/v1/accounts/{id}": + parameters: + - name: id + in: path + required: true + description: Account ID + schema: + type: string + format: uuid + get: + summary: Retrieve an account + tags: + - Accounts + security: + - apiKeyAuth: [] + parameters: + - name: include_disabled + in: query + required: false + description: Allow retrieving a disabled account. Defaults to false. + schema: + type: boolean + responses: + '200': + description: account retrieved + content: + application/json: + schema: + "$ref": "#/components/schemas/AccountDetail" + '401': + description: unauthorized + content: + application/json: + schema: + "$ref": "#/components/schemas/ErrorResponse" + '403': + description: insufficient scope + content: + application/json: + schema: + "$ref": "#/components/schemas/ErrorResponse" + '404': + description: account not found + content: + application/json: + schema: + "$ref": "#/components/schemas/ErrorResponse" "/api/v1/auth/signup": post: summary: Sign up a new user @@ -1267,6 +1351,181 @@ paths: - refresh_token - device required: true + "/api/v1/auth/sso_link": + post: + summary: Link an existing account via SSO + tags: + - Auth + description: Authenticates with email/password and links the SSO identity from + a previously issued linking code. Creates an OidcIdentity, logs the link via + SsoAuditLog, and issues mobile OAuth tokens. + parameters: [] + responses: + '200': + description: account linked and tokens issued + content: + application/json: + schema: + type: object + properties: + access_token: + type: string + refresh_token: + type: string + token_type: + type: string + expires_in: + type: integer + created_at: + type: integer + user: + type: object + properties: + id: + type: string + format: uuid + email: + type: string + first_name: + type: string + last_name: + type: string + ui_layout: + type: string + enum: + - dashboard + - intro + ai_enabled: + type: boolean + '400': + description: missing linking code + content: + application/json: + schema: + "$ref": "#/components/schemas/ErrorResponse" + '401': + description: invalid credentials or expired linking code + content: + application/json: + schema: + oneOf: + - "$ref": "#/components/schemas/ErrorResponse" + - "$ref": "#/components/schemas/MfaRequiredResponse" + requestBody: + content: + application/json: + schema: + type: object + properties: + linking_code: + type: string + description: One-time linking code from mobile SSO onboarding redirect + email: + type: string + format: email + description: Email of the existing account to link + password: + type: string + description: Password for the existing account + required: + - linking_code + - email + - password + required: true + "/api/v1/auth/sso_create_account": + post: + summary: Create a new account via SSO + tags: + - Auth + description: Creates a new user and family from a previously issued linking + code. Links the SSO identity via OidcIdentity, logs the JIT account creation + via SsoAuditLog, and issues mobile OAuth tokens. The linking code must have + allow_account_creation enabled. + parameters: [] + responses: + '200': + description: account created and tokens issued + content: + application/json: + schema: + type: object + properties: + access_token: + type: string + refresh_token: + type: string + token_type: + type: string + expires_in: + type: integer + created_at: + type: integer + user: + type: object + properties: + id: + type: string + format: uuid + email: + type: string + first_name: + type: string + last_name: + type: string + ui_layout: + type: string + enum: + - dashboard + - intro + ai_enabled: + type: boolean + '400': + description: missing linking code + content: + application/json: + schema: + "$ref": "#/components/schemas/ErrorResponse" + '401': + description: invalid or expired linking code + content: + application/json: + schema: + "$ref": "#/components/schemas/ErrorResponse" + '403': + description: account creation disabled + content: + application/json: + schema: + "$ref": "#/components/schemas/ErrorResponse" + '422': + description: user validation error + content: + application/json: + schema: + type: object + properties: + errors: + type: array + items: + type: string + requestBody: + content: + application/json: + schema: + type: object + properties: + linking_code: + type: string + description: One-time linking code from mobile SSO onboarding redirect + first_name: + type: string + description: First name (overrides value from SSO provider if provided) + last_name: + type: string + description: Last name (overrides value from SSO provider if provided) + required: + - linking_code + required: true "/api/v1/auth/enable_ai": patch: summary: Enable AI features for the authenticated user @@ -1309,6 +1568,34 @@ paths: application/json: schema: "$ref": "#/components/schemas/ErrorResponse" + '403': + description: insufficient scope + content: + application/json: + schema: + "$ref": "#/components/schemas/ErrorResponse" + "/api/v1/balance_sheet": + get: + summary: Show balance sheet + tags: + - Balance Sheet + description: Returns the family balance sheet including net worth, total assets, + and total liabilities with amounts converted to the family's primary currency. + security: + - apiKeyAuth: [] + responses: + '200': + description: balance sheet returned + content: + application/json: + schema: + "$ref": "#/components/schemas/BalanceSheet" + '401': + description: unauthorized + content: + application/json: + schema: + "$ref": "#/components/schemas/ErrorResponse" "/api/v1/categories": get: summary: List categories @@ -1329,15 +1616,6 @@ paths: description: 'Items per page (default: 25, max: 100)' schema: type: integer - - name: classification - in: query - required: false - description: Filter by classification (income or expense) - schema: - type: string - enum: - - income - - expense - name: roots_only in: query required: false @@ -1883,6 +2161,49 @@ paths: application/json: schema: "$ref": "#/components/schemas/ErrorResponse" + "/api/v1/merchants": + get: + summary: List merchants + tags: + - Merchants + security: + - apiKeyAuth: [] + responses: + '200': + description: merchants listed + content: + application/json: + schema: + type: array + items: + "$ref": "#/components/schemas/MerchantDetail" + "/api/v1/merchants/{id}": + parameters: + - name: id + in: path + required: true + description: Merchant ID + schema: + type: string + get: + summary: Retrieve a merchant + tags: + - Merchants + security: + - apiKeyAuth: [] + responses: + '200': + description: merchant retrieved + content: + application/json: + schema: + "$ref": "#/components/schemas/MerchantDetail" + '404': + description: merchant not found + content: + application/json: + schema: + "$ref": "#/components/schemas/ErrorResponse" "/api/v1/tags": get: summary: List tags @@ -2583,6 +2904,53 @@ paths: application/json: schema: "$ref": "#/components/schemas/ErrorResponse" + "/api/v1/users/reset": + delete: + summary: Reset account + tags: + - Users + description: Resets all financial data (accounts, categories, merchants, tags, + etc.) for the current user's family while keeping the user account intact. + The reset runs asynchronously in the background. Requires admin role. + security: + - apiKeyAuth: [] + responses: + '200': + description: account reset initiated + content: + application/json: + schema: + "$ref": "#/components/schemas/SuccessMessage" + '401': + description: unauthorized + '403': + description: forbidden - requires read_write scope and admin role + "/api/v1/users/me": + delete: + summary: Delete account + tags: + - Users + description: Permanently deactivates the current user account and all associated + data. This action cannot be undone. + security: + - apiKeyAuth: [] + responses: + '200': + description: account deleted + content: + application/json: + schema: + "$ref": "#/components/schemas/SuccessMessage" + '401': + description: unauthorized + '403': + description: insufficient scope + '422': + description: deactivation failed + content: + application/json: + schema: + "$ref": "#/components/schemas/ErrorResponse" "/api/v1/valuations": post: summary: Create valuation @@ -2725,66 +3093,3 @@ paths: type: string description: Additional notes required: true - "/api/v1/users/reset": - delete: - summary: Reset account - tags: - - Users - description: Resets all financial data (accounts, categories, merchants, tags, - etc.) for the current user's family while keeping the user account intact. - The reset runs asynchronously in the background. Requires admin role. - security: - - apiKeyAuth: [] - responses: - '200': - description: account reset initiated - content: - application/json: - schema: - "$ref": "#/components/schemas/SuccessMessage" - '401': - description: unauthorized - content: - application/json: - schema: - "$ref": "#/components/schemas/ErrorResponse" - '403': - description: "forbidden \u2014 requires read_write scope and admin role" - content: - application/json: - schema: - "$ref": "#/components/schemas/ErrorResponse" - "/api/v1/users/me": - delete: - summary: Delete account - tags: - - Users - description: Permanently deactivates the current user account and all associated - data. This action cannot be undone. - security: - - apiKeyAuth: [] - responses: - '200': - description: account deleted - content: - application/json: - schema: - "$ref": "#/components/schemas/SuccessMessage" - '401': - description: unauthorized - content: - application/json: - schema: - "$ref": "#/components/schemas/ErrorResponse" - '403': - description: insufficient scope - content: - application/json: - schema: - "$ref": "#/components/schemas/ErrorResponse" - '422': - description: deactivation failed - content: - application/json: - schema: - "$ref": "#/components/schemas/ErrorResponse" diff --git a/spec/requests/api/v1/accounts_spec.rb b/spec/requests/api/v1/accounts_spec.rb index 80927388d..91686840b 100644 --- a/spec/requests/api/v1/accounts_spec.rb +++ b/spec/requests/api/v1/accounts_spec.rb @@ -31,6 +31,19 @@ RSpec.describe 'API V1 Accounts', type: :request do ) end + let(:api_key_without_read_scope) do + key = ApiKey.generate_secure_key + # Valid persisted API keys can only be read/read_write; this intentionally + # bypasses validations to document the runtime insufficient-scope response. + ApiKey.new( + user: user, + name: 'No Read Docs Key', + key: key, + scopes: %w[write], + source: 'web' + ).tap { |api_key| api_key.save!(validate: false) } + end + let(:'X-Api-Key') { api_key.plain_key } let!(:checking_account) do @@ -72,6 +85,8 @@ RSpec.describe 'API V1 Accounts', type: :request do description: 'Page number (default: 1)' parameter name: :per_page, in: :query, type: :integer, required: false, description: 'Items per page (default: 25, max: 100)' + parameter name: :include_disabled, in: :query, type: :boolean, required: false, + description: 'Include disabled accounts in the response. Defaults to false.' response '200', 'accounts listed' do schema '$ref' => '#/components/schemas/AccountCollection' @@ -89,4 +104,51 @@ RSpec.describe 'API V1 Accounts', type: :request do end end end + + path '/api/v1/accounts/{id}' do + parameter name: :id, in: :path, required: true, description: 'Account ID', + schema: { type: :string, format: :uuid } + + get 'Retrieve an account' do + tags 'Accounts' + security [ { apiKeyAuth: [] } ] + produces 'application/json' + parameter name: :include_disabled, in: :query, type: :boolean, required: false, + description: 'Allow retrieving a disabled account. Defaults to false.' + + let(:id) { checking_account.id } + + response '200', 'account retrieved' do + schema '$ref' => '#/components/schemas/AccountDetail' + + run_test! + end + + response '401', 'unauthorized' do + schema '$ref' => '#/components/schemas/ErrorResponse' + + let(:id) { checking_account.id } + let(:'X-Api-Key') { nil } + + run_test! + end + + response '403', 'insufficient scope' do + schema '$ref' => '#/components/schemas/ErrorResponse' + + let(:id) { checking_account.id } + let(:'X-Api-Key') { api_key_without_read_scope.plain_key } + + run_test! + end + + response '404', 'account not found' do + schema '$ref' => '#/components/schemas/ErrorResponse' + + let(:id) { SecureRandom.uuid } + + run_test! + end + end + end end diff --git a/spec/requests/api/v1/auth_spec.rb b/spec/requests/api/v1/auth_spec.rb index 278ec7348..11366acf8 100644 --- a/spec/requests/api/v1/auth_spec.rb +++ b/spec/requests/api/v1/auth_spec.rb @@ -261,7 +261,10 @@ RSpec.describe 'API V1 Auth', type: :request do end response '401', 'invalid credentials or expired linking code' do - schema '$ref' => '#/components/schemas/ErrorResponse' + schema oneOf: [ + { '$ref' => '#/components/schemas/ErrorResponse' }, + { '$ref' => '#/components/schemas/MfaRequiredResponse' } + ] run_test! end end diff --git a/spec/requests/api/v1/balance_sheet_spec.rb b/spec/requests/api/v1/balance_sheet_spec.rb index 72132871a..f42e079f9 100644 --- a/spec/requests/api/v1/balance_sheet_spec.rb +++ b/spec/requests/api/v1/balance_sheet_spec.rb @@ -48,6 +48,8 @@ RSpec.describe 'API V1 Balance Sheet', type: :request do end response '401', 'unauthorized' do + schema '$ref' => '#/components/schemas/ErrorResponse' + let(:'X-Api-Key') { 'invalid-key' } run_test! diff --git a/spec/swagger_helper.rb b/spec/swagger_helper.rb index ad99750c1..d788ba8e5 100644 --- a/spec/swagger_helper.rb +++ b/spec/swagger_helper.rb @@ -64,6 +64,14 @@ RSpec.configure do |config| } } }, + MfaRequiredResponse: { + type: :object, + required: %w[error mfa_required], + properties: { + error: { type: :string }, + mfa_required: { type: :boolean } + } + }, ToolCall: { type: :object, required: %w[id function_name function_arguments created_at], @@ -175,19 +183,29 @@ RSpec.configure do |config| properties: { id: { type: :string, format: :uuid }, name: { type: :string }, - account_type: { type: :string } + account_type: { type: :string, nullable: true }, + status: { type: :string } } }, AccountDetail: { type: :object, - required: %w[id name balance currency classification account_type], + required: %w[id name balance balance_cents cash_balance cash_balance_cents currency classification account_type status created_at updated_at], properties: { id: { type: :string, format: :uuid }, name: { type: :string }, balance: { type: :string }, + balance_cents: { type: :integer, description: 'Signed balance in minor currency units' }, + cash_balance: { type: :string }, + cash_balance_cents: { type: :integer, description: 'Signed cash balance in minor currency units' }, currency: { type: :string }, classification: { type: :string }, - account_type: { type: :string } + account_type: { type: :string, nullable: true }, + subtype: { type: :string, nullable: true }, + status: { type: :string, enum: %w[active draft disabled pending_deletion] }, + institution_name: { type: :string, nullable: true }, + institution_domain: { type: :string, nullable: true }, + created_at: { type: :string, format: :'date-time' }, + updated_at: { type: :string, format: :'date-time' } } }, AccountCollection: { diff --git a/test/controllers/api/v1/accounts_controller_test.rb b/test/controllers/api/v1/accounts_controller_test.rb index 9d95b4548..65e1715cd 100644 --- a/test/controllers/api/v1/accounts_controller_test.rb +++ b/test/controllers/api/v1/accounts_controller_test.rb @@ -8,10 +8,22 @@ class Api::V1::AccountsControllerTest < ActionDispatch::IntegrationTest @other_family_user = users(:family_member) @other_family_user.update!(family: families(:empty)) - @oauth_app = Doorkeeper::Application.create!( - name: "Test API App", - redirect_uri: "https://example.com/callback", - scopes: "read read_write" + @user.api_keys.active.destroy_all + @api_key = ApiKey.create!( + user: @user, + name: "Test Read Key", + scopes: [ "read" ], + source: "web", + display_key: "test_read_#{SecureRandom.hex(8)}" + ) + + @other_family_user.api_keys.active.destroy_all + @other_family_api_key = ApiKey.create!( + user: @other_family_user, + name: "Other Family Read Key", + scopes: [ "read" ], + source: "web", + display_key: "other_family_read_#{SecureRandom.hex(8)}" ) end @@ -24,37 +36,28 @@ class Api::V1::AccountsControllerTest < ActionDispatch::IntegrationTest end test "should require read_accounts scope" do - # TODO: Re-enable this test after fixing scope checking - skip "Scope checking temporarily disabled - needs configuration fix" + api_key_without_read = ApiKey.new( + user: @user, + name: "No Read Key", + scopes: [], + source: "web", + display_key: "no_read_#{SecureRandom.hex(8)}" + ) + # Valid persisted API keys can only be read/read_write; this intentionally + # bypasses validations to exercise the runtime insufficient-scope guard. + api_key_without_read.save!(validate: false) - # Create token with wrong scope - using a non-existent scope to test rejection - access_token = Doorkeeper::AccessToken.create!( - application: @oauth_app, - resource_owner_id: @user.id, - scopes: "invalid_scope" # Wrong scope - ) + get "/api/v1/accounts", params: {}, headers: api_headers(api_key_without_read) - get "/api/v1/accounts", params: {}, headers: { - "Authorization" => "Bearer #{access_token.token}" - } - - assert_response :forbidden - - # Doorkeeper returns a standard OAuth error response - response_body = JSON.parse(response.body) - assert_equal "insufficient_scope", response_body["error"] -end + assert_response :forbidden + response_body = JSON.parse(response.body) + assert_equal "insufficient_scope", response_body["error"] + ensure + api_key_without_read&.destroy + end test "should return user's family accounts successfully" do - access_token = Doorkeeper::AccessToken.create!( - application: @oauth_app, - resource_owner_id: @user.id, - scopes: "read" - ) - - get "/api/v1/accounts", params: {}, headers: { - "Authorization" => "Bearer #{access_token.token}" - } + get "/api/v1/accounts", params: {}, headers: api_headers(@api_key) assert_response :success response_body = JSON.parse(response.body) @@ -83,15 +86,7 @@ end inactive_account = accounts(:depository) inactive_account.disable! - access_token = Doorkeeper::AccessToken.create!( - application: @oauth_app, - resource_owner_id: @user.id, - scopes: "read" - ) - - get "/api/v1/accounts", params: {}, headers: { - "Authorization" => "Bearer #{access_token.token}" - } + get "/api/v1/accounts", params: {}, headers: api_headers(@api_key) assert_response :success response_body = JSON.parse(response.body) @@ -101,17 +96,140 @@ end assert_not_includes account_names, inactive_account.name end - test "should not return other family's accounts" do - access_token = Doorkeeper::AccessToken.create!( - application: @oauth_app, - resource_owner_id: @other_family_user.id, # User from different family - scopes: "read" - ) + test "should include disabled accounts when requested" do + inactive_account = accounts(:depository) + inactive_account.disable! - get "/api/v1/accounts", params: {}, headers: { - "Authorization" => "Bearer #{access_token.token}" + get "/api/v1/accounts", params: { include_disabled: true }, headers: api_headers(@api_key) + + assert_response :success + response_body = JSON.parse(response.body) + + account = response_body["accounts"].find { |account_data| account_data["id"] == inactive_account.id } + assert_not_nil account + assert_equal "disabled", account["status"] + end + + test "should show active account" do + account = accounts(:depository) + + get "/api/v1/accounts/#{account.id}", headers: api_headers(@api_key) + + assert_response :success + response_body = JSON.parse(response.body) + assert_equal account.id, response_body["id"] + assert_equal account.status, response_body["status"] + assert_equal account.balance_money.format, response_body["balance"] + assert_equal money_cents(account.balance_money), response_body["balance_cents"] + assert_equal account.cash_balance_money.format, response_body["cash_balance"] + assert_equal money_cents(account.cash_balance_money), response_body["cash_balance_cents"] + assert_nullable_equal account.subtype, response_body["subtype"] + assert response_body.key?("institution_name") + assert response_body.key?("institution_domain") + assert_nullable_equal account.institution_name, response_body["institution_name"] + assert_nullable_equal account.institution_domain, response_body["institution_domain"] + assert_equal account.created_at.iso8601, response_body["created_at"] + assert_equal account.updated_at.iso8601, response_body["updated_at"] + end + + test "should return 404 for unknown account on show" do + get "/api/v1/accounts/#{SecureRandom.uuid}", headers: api_headers(@api_key) + + assert_response :not_found + response_body = JSON.parse(response.body) + assert_equal "not_found", response_body["error"] + end + + test "should return 404 for malformed account id on show" do + get "/api/v1/accounts/not-a-uuid", headers: api_headers(@api_key) + + assert_response :not_found + response_body = JSON.parse(response.body) + assert_equal "not_found", response_body["error"] + assert_equal "Account not found", response_body["message"] + end + + test "should require authentication on show" do + account = accounts(:depository) + + get "/api/v1/accounts/#{account.id}" + + assert_response :unauthorized + response_body = JSON.parse(response.body) + assert_equal "unauthorized", response_body["error"] + end + + test "should require read scope on show" do + account = accounts(:depository) + api_key_without_read = ApiKey.new( + user: @user, + name: "No Read Show Key", + scopes: [], + source: "web", + display_key: "no_read_show_#{SecureRandom.hex(8)}" + ) + # Valid persisted API keys can only be read/read_write; this intentionally + # bypasses validations to exercise the runtime insufficient-scope guard. + api_key_without_read.save!(validate: false) + + get "/api/v1/accounts/#{account.id}", headers: api_headers(api_key_without_read) + + assert_response :forbidden + response_body = JSON.parse(response.body) + assert_equal "insufficient_scope", response_body["error"] + ensure + api_key_without_read&.destroy + end + + test "should hide disabled account by default on show" do + inactive_account = accounts(:depository) + inactive_account.disable! + + get "/api/v1/accounts/#{inactive_account.id}", headers: api_headers(@api_key) + + assert_response :not_found + end + + test "should show disabled account when requested" do + inactive_account = accounts(:depository) + inactive_account.disable! + + get "/api/v1/accounts/#{inactive_account.id}", + params: { include_disabled: true }, + headers: api_headers(@api_key) + + assert_response :success + response_body = JSON.parse(response.body) + assert_equal inactive_account.id, response_body["id"] + assert_equal "disabled", response_body["status"] + end + + test "should expose subtype across account types" do + expected_subtypes = { + accounts(:depository) => "checking", + accounts(:credit_card) => "credit_card", + accounts(:investment) => "brokerage", + accounts(:loan) => "mortgage", + accounts(:property) => "single_family_home", + accounts(:vehicle) => "sedan", + accounts(:crypto) => "exchange", + accounts(:other_asset) => "collectible", + accounts(:other_liability) => "personal_debt" } + expected_subtypes.each { |account, subtype| account.accountable.update!(subtype: subtype) } + + expected_subtypes.each do |account, subtype| + get "/api/v1/accounts/#{account.id}", headers: api_headers(@api_key) + + assert_response :success + assert_equal subtype, JSON.parse(response.body)["subtype"] + end + end + + test "should not return other family's accounts" do + get "/api/v1/accounts", params: {}, headers: api_headers(@other_family_api_key) + assert_response :success response_body = JSON.parse(response.body) @@ -121,16 +239,8 @@ end end test "should handle pagination parameters" do - access_token = Doorkeeper::AccessToken.create!( - application: @oauth_app, - resource_owner_id: @user.id, - scopes: "read" - ) - # Test with pagination params - get "/api/v1/accounts", params: { page: 1, per_page: 2 }, headers: { - "Authorization" => "Bearer #{access_token.token}" - } + get "/api/v1/accounts", params: { page: 1, per_page: 2 }, headers: api_headers(@api_key) assert_response :success response_body = JSON.parse(response.body) @@ -142,15 +252,7 @@ end end test "should return proper account data structure" do - access_token = Doorkeeper::AccessToken.create!( - application: @oauth_app, - resource_owner_id: @user.id, - scopes: "read" - ) - - get "/api/v1/accounts", params: {}, headers: { - "Authorization" => "Bearer #{access_token.token}" - } + get "/api/v1/accounts", params: {}, headers: api_headers(@api_key) assert_response :success response_body = JSON.parse(response.body) @@ -161,7 +263,7 @@ end account = response_body["accounts"].first # Check required fields are present - required_fields = %w[id name balance currency classification account_type] + required_fields = %w[id name balance balance_cents cash_balance cash_balance_cents currency classification account_type] required_fields.each do |field| assert account.key?(field), "Account should have #{field} field" end @@ -170,21 +272,15 @@ end assert account["id"].is_a?(String), "ID should be string (UUID)" assert account["name"].is_a?(String), "Name should be string" assert account["balance"].is_a?(String), "Balance should be string (money)" + assert account["balance_cents"].is_a?(Integer), "Balance cents should be integer" + assert account["cash_balance_cents"].is_a?(Integer), "Cash balance cents should be integer" assert account["currency"].is_a?(String), "Currency should be string" assert %w[asset liability].include?(account["classification"]), "Classification should be asset or liability" end test "should handle invalid pagination parameters gracefully" do - access_token = Doorkeeper::AccessToken.create!( - application: @oauth_app, - resource_owner_id: @user.id, - scopes: "read" - ) - # Test with invalid page number - get "/api/v1/accounts", params: { page: -1, per_page: "invalid" }, headers: { - "Authorization" => "Bearer #{access_token.token}" - } + get "/api/v1/accounts", params: { page: -1, per_page: "invalid" }, headers: api_headers(@api_key) # Should still return success with default pagination assert_response :success @@ -197,15 +293,7 @@ end end test "should sort accounts alphabetically" do - access_token = Doorkeeper::AccessToken.create!( - application: @oauth_app, - resource_owner_id: @user.id, - scopes: "read" - ) - - get "/api/v1/accounts", params: {}, headers: { - "Authorization" => "Bearer #{access_token.token}" - } + get "/api/v1/accounts", params: {}, headers: api_headers(@api_key) assert_response :success response_body = JSON.parse(response.body) @@ -214,4 +302,18 @@ end account_names = response_body["accounts"].map { |a| a["name"] } assert_equal account_names.sort, account_names end + + private + + def api_headers(api_key) + { "X-Api-Key" => api_key.plain_key } + end + + def money_cents(money) + (money.amount * money.currency.minor_unit_conversion).round(0).to_i + end + + def assert_nullable_equal(expected, actual) + expected.nil? ? assert_nil(actual) : assert_equal(expected, actual) + end end