fix open banking reconnect callback (#408)
This commit is contained in:
Víctor Falcón
GitHub
parent
d2e00f14e5
commit
c01f2b60e6
|
|
@ -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<int, array<string, mixed>>} $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.
|
||||
*
|
||||
|
|
|
|||
|
|
@ -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 () {
|
||||
|
|
|
|||
Loading…
Reference in New Issue