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

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