fix(banking): update external_account_id on reconnect and store IBAN (#220)

## Problem

Enable Banking issues **new account UIDs** (`external_account_id`) with
every new session. On reconnect, `AuthorizationController::callback()`
was correctly updating `session_id` on the `banking_connections` table
but **never refreshing `external_account_id` on child accounts**.

When the background `SyncBankingConnectionJob` ran after a reconnect, it
called `GET /accounts/{old-uid}/transactions` using stale UIDs from the
expired session, causing Enable Banking to return `401 EXPIRED_SESSION`
— surfaced to users as _"Authentication failed. Your credentials may
have expired or been revoked."_

## Changes

- **`AuthorizationController`** — added `refreshAccountIds()` private
method, called in the reconnect branch after `session_id` is updated and
before the sync job is dispatched. Matches existing accounts by **IBAN
first**, falls back to **positional order** (`created_at ASC`) for
legacy accounts without a stored IBAN.
- **`AuthorizationController`** — `createAccountsFromSession()` and
`createAccountsFromPending()` now persist the `iban` field on account
creation so future reconnects can use IBAN matching.
- **`Account` model** — `iban` added to `$fillable`.
- **Migration** — adds nullable `iban` column to `accounts` table.

## Tests

4 new feature tests in `AuthorizationControllerTest`:

- `reconnect callback updates external_account_id when enable banking
issues new account uids`
- `reconnect callback matches accounts by iban before falling back to
position`
- `reconnect callback uses positional fallback for accounts without
stored iban`
- `callback stores iban when creating accounts for the first time`

## Deploy notes

Run the migration after deploying:

```
php artisan migrate --force
```
This commit is contained in:
Víctor Falcón 2026-03-12 10:28:23 +00:00 committed by GitHub
parent b1cf133b5a
commit 4408f719b4
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 297 additions and 5 deletions

View File

@ -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<int, array<string, mixed>> $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,
]);
}
}

View File

@ -28,6 +28,7 @@ class Account extends Model
'encrypted',
'banking_connection_id',
'external_account_id',
'iban',
'linked_at',
];

View File

@ -0,0 +1,28 @@
<?php
use Illuminate\Database\Migrations\Migration;
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Support\Facades\Schema;
return new class extends Migration
{
/**
* Run the migrations.
*/
public function up(): void
{
Schema::table('accounts', function (Blueprint $table) {
$table->string('iban')->nullable()->after('external_account_id');
});
}
/**
* Reverse the migrations.
*/
public function down(): void
{
Schema::table('accounts', function (Blueprint $table) {
$table->dropColumn('iban');
});
}
};

View File

@ -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',
]);
});