diff --git a/app/Http/Controllers/OpenBanking/AuthorizationController.php b/app/Http/Controllers/OpenBanking/AuthorizationController.php index 30c3143b..b3e3e00e 100644 --- a/app/Http/Controllers/OpenBanking/AuthorizationController.php +++ b/app/Http/Controllers/OpenBanking/AuthorizationController.php @@ -151,6 +151,8 @@ class AuthorizationController extends Controller 'error_message' => null, ]); + $this->refreshAccountIds($connection, $sessionData['accounts']); + SyncBankingConnectionJob::dispatch($connection); return redirect()->route('settings.connections.index') @@ -193,6 +195,61 @@ class AuthorizationController extends Controller ->with('success', 'Bank account connected successfully.'); } + /** + * Refresh external_account_id and iban on existing accounts after a reconnect. + * + * Enable Banking issues new account UIDs with every new session, so the stored + * external_account_id values become invalid as soon as the old session expires. + * + * Matching strategy (in priority order): + * 1. Match by IBAN — reliable when the account was created after the iban column existed. + * 2. Positional fallback — match by creation order for legacy accounts without a stored IBAN. + * + * @param array> $newAccounts + */ + private function refreshAccountIds(BankingConnection $connection, array $newAccounts): void + { + if (empty($newAccounts)) { + return; + } + + $existingAccounts = $connection->accounts()->orderBy('created_at')->get(); + + $unmatchedNew = collect($newAccounts); + $unmatchedExisting = collect(); + + foreach ($existingAccounts as $account) { + if ($account->iban) { + $matched = $unmatchedNew->first(fn (array $data) => ($data['account_id']['iban'] ?? null) === $account->iban); + + if ($matched) { + $account->update([ + 'external_account_id' => $matched['uid'], + 'iban' => $matched['account_id']['iban'] ?? $account->iban, + ]); + $unmatchedNew = $unmatchedNew->reject(fn (array $data) => ($data['uid'] ?? null) === $matched['uid'])->values(); + + continue; + } + } + + $unmatchedExisting->push($account); + } + + foreach ($unmatchedExisting as $index => $account) { + $newAccountData = $unmatchedNew->get($index); + + if (! $newAccountData) { + continue; + } + + $account->update([ + 'external_account_id' => $newAccountData['uid'], + 'iban' => $newAccountData['account_id']['iban'] ?? null, + ]); + } + } + /** * Auto-create accounts from pending_accounts_data without user interaction. */ @@ -228,13 +285,9 @@ class AuthorizationController extends Controller 'type' => AccountType::Checking->value, 'banking_connection_id' => $connection->id, 'external_account_id' => $uid, + 'iban' => $accountData['account_id']['iban'] ?? null, ]); } - - $connection->update([ - 'status' => BankingConnectionStatus::Active, - 'pending_accounts_data' => null, - ]); } /** @@ -283,6 +336,7 @@ class AuthorizationController extends Controller 'type' => AccountType::Checking->value, 'banking_connection_id' => $connection->id, 'external_account_id' => $uid, + 'iban' => $accountData['account_id']['iban'] ?? null, ]); } } diff --git a/app/Models/Account.php b/app/Models/Account.php index c38e94fc..e4ce7854 100644 --- a/app/Models/Account.php +++ b/app/Models/Account.php @@ -28,6 +28,7 @@ class Account extends Model 'encrypted', 'banking_connection_id', 'external_account_id', + 'iban', 'linked_at', ]; diff --git a/database/migrations/2026_03_12_095411_add_iban_to_accounts_table.php b/database/migrations/2026_03_12_095411_add_iban_to_accounts_table.php new file mode 100644 index 00000000..5144df90 --- /dev/null +++ b/database/migrations/2026_03_12_095411_add_iban_to_accounts_table.php @@ -0,0 +1,28 @@ +string('iban')->nullable()->after('external_account_id'); + }); + } + + /** + * Reverse the migrations. + */ + public function down(): void + { + Schema::table('accounts', function (Blueprint $table) { + $table->dropColumn('iban'); + }); + } +}; diff --git a/tests/Feature/OpenBanking/AuthorizationControllerTest.php b/tests/Feature/OpenBanking/AuthorizationControllerTest.php index f57c7b1c..84029f1c 100644 --- a/tests/Feature/OpenBanking/AuthorizationControllerTest.php +++ b/tests/Feature/OpenBanking/AuthorizationControllerTest.php @@ -448,3 +448,212 @@ test('callback with existing accounts skips account-mapping even when feature fl Queue::assertPushed(SyncBankingConnectionJob::class); }); + +// refreshAccountIds tests + +test('reconnect callback updates external_account_id when enable banking issues new account uids', function () { + Queue::fake(); + + $user = User::factory()->onboarded()->create(); + Feature::for($user)->activate('open-banking'); + + $connection = BankingConnection::factory()->pending()->create([ + 'user_id' => $user->id, + 'aspsp_name' => 'CaixaBank', + 'aspsp_country' => 'ES', + ]); + + $account = Account::factory()->create([ + 'user_id' => $user->id, + 'banking_connection_id' => $connection->id, + 'external_account_id' => 'old-uid-1', + 'iban' => 'ES1234567890123456789012', + ]); + + $mockProvider = Mockery::mock(BankingProviderInterface::class); + $mockProvider->shouldReceive('createSession') + ->with('test-code') + ->once() + ->andReturn([ + 'session_id' => 'new-session-abc', + 'accounts' => [ + [ + 'uid' => 'new-uid-1', + 'currency' => 'EUR', + 'name' => 'CaixaBank Account', + 'account_id' => ['iban' => 'ES1234567890123456789012'], + ], + ], + 'access' => ['valid_until' => now()->addDays(90)->toIso8601String()], + ]); + + $this->app->instance(BankingProviderInterface::class, $mockProvider); + + $this->actingAs($user)->get('/open-banking/callback?code=test-code'); + + $account->refresh(); + expect($account->external_account_id)->toBe('new-uid-1'); + expect($account->iban)->toBe('ES1234567890123456789012'); +}); + +test('reconnect callback matches accounts by iban before falling back to position', function () { + Queue::fake(); + + $user = User::factory()->onboarded()->create(); + Feature::for($user)->activate('open-banking'); + + $connection = BankingConnection::factory()->pending()->create([ + 'user_id' => $user->id, + 'aspsp_name' => 'CaixaBank', + 'aspsp_country' => 'ES', + ]); + + // Two accounts; create them in reverse IBAN order to confirm positional matching + // would produce wrong results, while IBAN matching produces correct results. + $accountA = Account::factory()->create([ + 'user_id' => $user->id, + 'banking_connection_id' => $connection->id, + 'external_account_id' => 'old-uid-a', + 'iban' => 'ES0000000000000000000001', + 'created_at' => now()->subMinutes(2), + ]); + + $accountB = Account::factory()->create([ + 'user_id' => $user->id, + 'banking_connection_id' => $connection->id, + 'external_account_id' => 'old-uid-b', + 'iban' => 'ES0000000000000000000002', + 'created_at' => now()->subMinutes(1), + ]); + + $mockProvider = Mockery::mock(BankingProviderInterface::class); + $mockProvider->shouldReceive('createSession') + ->once() + ->andReturn([ + 'session_id' => 'new-session-abc', + // Enable Banking returns accounts in a different order this time + 'accounts' => [ + [ + 'uid' => 'new-uid-b', + 'currency' => 'EUR', + 'name' => 'Account B', + 'account_id' => ['iban' => 'ES0000000000000000000002'], + ], + [ + 'uid' => 'new-uid-a', + 'currency' => 'EUR', + 'name' => 'Account A', + 'account_id' => ['iban' => 'ES0000000000000000000001'], + ], + ], + 'access' => ['valid_until' => now()->addDays(90)->toIso8601String()], + ]); + + $this->app->instance(BankingProviderInterface::class, $mockProvider); + + $this->actingAs($user)->get('/open-banking/callback?code=test-code'); + + expect($accountA->refresh()->external_account_id)->toBe('new-uid-a'); + expect($accountB->refresh()->external_account_id)->toBe('new-uid-b'); +}); + +test('reconnect callback uses positional fallback for accounts without stored iban', function () { + Queue::fake(); + + $user = User::factory()->onboarded()->create(); + Feature::for($user)->activate('open-banking'); + + $connection = BankingConnection::factory()->pending()->create([ + 'user_id' => $user->id, + 'aspsp_name' => 'CaixaBank', + 'aspsp_country' => 'ES', + ]); + + $accountA = Account::factory()->create([ + 'user_id' => $user->id, + 'banking_connection_id' => $connection->id, + 'external_account_id' => 'old-uid-a', + 'iban' => null, // legacy account without IBAN stored + 'created_at' => now()->subMinutes(2), + ]); + + $accountB = Account::factory()->create([ + 'user_id' => $user->id, + 'banking_connection_id' => $connection->id, + 'external_account_id' => 'old-uid-b', + 'iban' => null, + 'created_at' => now()->subMinutes(1), + ]); + + $mockProvider = Mockery::mock(BankingProviderInterface::class); + $mockProvider->shouldReceive('createSession') + ->once() + ->andReturn([ + 'session_id' => 'new-session-abc', + 'accounts' => [ + [ + 'uid' => 'new-uid-a', + 'currency' => 'EUR', + 'name' => 'Account A', + 'account_id' => ['iban' => 'ES0000000000000000000001'], + ], + [ + 'uid' => 'new-uid-b', + 'currency' => 'EUR', + 'name' => 'Account B', + 'account_id' => ['iban' => 'ES0000000000000000000002'], + ], + ], + 'access' => ['valid_until' => now()->addDays(90)->toIso8601String()], + ]); + + $this->app->instance(BankingProviderInterface::class, $mockProvider); + + $this->actingAs($user)->get('/open-banking/callback?code=test-code'); + + // Positional match: index 0 → accountA (oldest), index 1 → accountB + expect($accountA->refresh()->external_account_id)->toBe('new-uid-a'); + expect($accountA->refresh()->iban)->toBe('ES0000000000000000000001'); // IBAN populated from new session + expect($accountB->refresh()->external_account_id)->toBe('new-uid-b'); + expect($accountB->refresh()->iban)->toBe('ES0000000000000000000002'); +}); + +test('callback stores iban when creating accounts for the first time', function () { + Queue::fake(); + + $user = User::factory()->onboarded()->create(); + Feature::for($user)->activate('open-banking'); + + $connection = BankingConnection::factory()->pending()->create([ + 'user_id' => $user->id, + 'aspsp_name' => 'Test Bank', + 'aspsp_country' => 'ES', + ]); + + $mockProvider = Mockery::mock(BankingProviderInterface::class); + $mockProvider->shouldReceive('createSession') + ->with('test-code') + ->once() + ->andReturn([ + 'session_id' => 'session-new', + 'accounts' => [ + [ + 'uid' => 'ext-account-1', + 'currency' => 'EUR', + 'name' => 'My Account', + 'account_id' => ['iban' => 'ES9999999999999999999999'], + ], + ], + 'access' => ['valid_until' => now()->addDays(90)->toIso8601String()], + ]); + + $this->app->instance(BankingProviderInterface::class, $mockProvider); + + $this->actingAs($user)->get('/open-banking/callback?code=test-code'); + + $this->assertDatabaseHas('accounts', [ + 'user_id' => $user->id, + 'external_account_id' => 'ext-account-1', + 'iban' => 'ES9999999999999999999999', + ]); +});