From c01f2b60e6b0aa28a06c2365ff116439dc376e6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Falc=C3=B3n?= Date: Wed, 20 May 2026 14:03:02 +0100 Subject: [PATCH] fix open banking reconnect callback (#408) --- .../OpenBanking/AuthorizationController.php | 68 +++++++++++++++++-- .../AuthorizationControllerTest.php | 63 +++++++++++++++++ 2 files changed, 127 insertions(+), 4 deletions(-) diff --git a/app/Http/Controllers/OpenBanking/AuthorizationController.php b/app/Http/Controllers/OpenBanking/AuthorizationController.php index 27e3dbc4..7c7ae73b 100644 --- a/app/Http/Controllers/OpenBanking/AuthorizationController.php +++ b/app/Http/Controllers/OpenBanking/AuthorizationController.php @@ -10,6 +10,7 @@ use App\Http\Controllers\OpenBanking\Concerns\HandlesSubscriptionGate; use App\Http\Requests\OpenBanking\StartAuthorizationRequest; use App\Jobs\SyncBankingConnectionJob; use App\Models\BankingConnection; +use App\Models\User; use Illuminate\Http\JsonResponse; use Illuminate\Http\RedirectResponse; use Illuminate\Http\Request; @@ -175,10 +176,7 @@ class AuthorizationController extends Controller ->with('error', 'Failed to connect to your bank. Please try again.'); } - $connection = $user->bankingConnections() - ->where('status', BankingConnectionStatus::Pending) - ->latest() - ->first(); + $connection = $this->findPendingConnectionForSession($user, $sessionData); if (! $connection) { return redirect()->route($errorRedirectRoute, $errorRedirectParams) @@ -221,6 +219,68 @@ class AuthorizationController extends Controller return redirect()->route('open-banking.map-accounts', $connection); } + /** + * Find the pending connection that belongs to the callback session. + * + * Multiple reconnection flows may be pending at the same time. Never pick an + * arbitrary latest connection, because that can attach one bank's session and + * transactions to another bank's existing account. + * + * @param array{aspsp?: array{name?: string, country?: string}, accounts?: array>} $sessionData + */ + private function findPendingConnectionForSession(User $user, array $sessionData): ?BankingConnection + { + $pendingConnections = $user->bankingConnections() + ->where('status', BankingConnectionStatus::Pending) + ->get(); + + if ($pendingConnections->isEmpty()) { + return null; + } + + $aspspName = $sessionData['aspsp']['name'] ?? null; + $aspspCountry = $sessionData['aspsp']['country'] ?? null; + + if (is_string($aspspName) && is_string($aspspCountry)) { + $matchedByInstitution = $pendingConnections + ->first(fn (BankingConnection $connection): bool => $connection->aspsp_name === $aspspName + && $connection->aspsp_country === $aspspCountry); + + if ($matchedByInstitution) { + return $matchedByInstitution; + } + } + + $ibans = collect($sessionData['accounts'] ?? []) + ->map(fn (array $account): ?string => $account['account_id']['iban'] ?? null) + ->filter() + ->values(); + + if ($ibans->isNotEmpty()) { + $matchedByIban = $pendingConnections + ->first(fn (BankingConnection $connection): bool => $connection->accounts() + ->whereIn('iban', $ibans) + ->exists()); + + if ($matchedByIban) { + return $matchedByIban; + } + } + + if ($pendingConnections->count() === 1) { + return $pendingConnections->first(); + } + + Log::warning('Unable to disambiguate pending EnableBanking callback', [ + 'user_id' => $user->id, + 'pending_connection_ids' => $pendingConnections->pluck('id')->all(), + 'aspsp_name' => is_string($aspspName) ? $aspspName : null, + 'aspsp_country' => is_string($aspspCountry) ? $aspspCountry : null, + ]); + + return null; + } + /** * Refresh external_account_id and iban on existing accounts after a reconnect. * diff --git a/tests/Feature/OpenBanking/AuthorizationControllerTest.php b/tests/Feature/OpenBanking/AuthorizationControllerTest.php index bac3dc6f..abb0abfa 100644 --- a/tests/Feature/OpenBanking/AuthorizationControllerTest.php +++ b/tests/Feature/OpenBanking/AuthorizationControllerTest.php @@ -461,6 +461,69 @@ test('callback with existing accounts skips mapping on reconnect', function () { Queue::assertPushed(SyncBankingConnectionJob::class); }); +test('callback matches the pending reconnect by institution when multiple reconnects are open', function () { + Queue::fake(); + + $user = User::factory()->onboarded()->create(); + $bbvaConnection = BankingConnection::factory()->pending()->create([ + 'user_id' => $user->id, + 'aspsp_name' => 'BBVA', + 'aspsp_country' => 'ES', + 'created_at' => now()->subHours(2), + ]); + $ingConnection = BankingConnection::factory()->pending()->create([ + 'user_id' => $user->id, + 'aspsp_name' => 'ING', + 'aspsp_country' => 'ES', + 'created_at' => now()->subHour(), + ]); + + $bbvaAccount = Account::factory()->create([ + 'user_id' => $user->id, + 'banking_connection_id' => $bbvaConnection->id, + 'external_account_id' => 'old-bbva-uid', + 'iban' => 'ES0000000000000000008058', + ]); + $ingAccount = Account::factory()->create([ + 'user_id' => $user->id, + 'banking_connection_id' => $ingConnection->id, + 'external_account_id' => 'old-ing-uid', + 'iban' => 'ES0000000000000000001111', + ]); + + $mockProvider = Mockery::mock(BankingProviderInterface::class); + $mockProvider->shouldReceive('createSession') + ->once() + ->andReturn([ + 'session_id' => 'new-bbva-session', + 'accounts' => [ + [ + 'uid' => 'new-bbva-uid', + 'currency' => 'EUR', + 'name' => 'BBVA Account', + 'account_id' => ['iban' => 'ES0000000000000000008058'], + ], + ], + 'aspsp' => ['name' => 'BBVA', 'country' => 'ES'], + 'access' => ['valid_until' => now()->addDays(90)->toIso8601String()], + ]); + + $this->app->instance(BankingProviderInterface::class, $mockProvider); + + $response = $this->actingAs($user)->get('/open-banking/callback?code=test-code'); + + $response->assertRedirect(route('settings.connections.index')); + + expect($bbvaConnection->refresh()->status)->toBe(BankingConnectionStatus::Active); + expect($bbvaConnection->session_id)->toBe('new-bbva-session'); + expect($bbvaAccount->refresh()->external_account_id)->toBe('new-bbva-uid'); + expect($ingConnection->refresh()->status)->toBe(BankingConnectionStatus::Pending); + expect($ingConnection->session_id)->toBeNull(); + expect($ingAccount->refresh()->external_account_id)->toBe('old-ing-uid'); + + Queue::assertPushed(SyncBankingConnectionJob::class, fn (SyncBankingConnectionJob $job): bool => $job->bankingConnection->id === $bbvaConnection->id); +}); + // refreshAccountIds tests test('reconnect callback updates external_account_id when enable banking issues new account uids', function () {