From 845f51abb54afcb88be36395021a370158942e86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Falc=C3=B3n?= Date: Sat, 4 Jul 2026 20:24:54 +0200 Subject: [PATCH] refactor(open-banking): extract shared connect-controller flow into a base class (#639) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Why The five API-key OpenBanking "connect" controllers — Binance, Bitpanda, Coinbase, Indexa Capital and Interactive Brokers — each carried a ~60-line `store()` that was near-identical: subscription gate → credential validation → `Bank::firstOrCreate` → create the `BankingConnection` (Pending) → update it to `AwaitingMapping` with pending accounts → auto-map during onboarding or redirect to mapping. Any change to that flow, or a bug in it, had to be made five times. ## What Extract the shared flow into a small class hierarchy, so each controller declares only what actually varies per provider. - **`OpenBankingConnectController`** (abstract) owns the whole flow in a `connect()` template method, with per-provider extension points: `provider()`, `providerName()`, `bankLogo()`, `aspspCountry()`, `fetchProviderData()`, `credentialErrorMessage()`, `buildPendingAccounts()`, and an optional `emptyProviderDataMessage()` guard. - **`CryptoPortfolioConnectController`** (abstract, extends the above) implements the two hooks the crypto providers share: `aspspCountry()` (from the request) and the single "Crypto Portfolio" `buildPendingAccounts()` (uid derived from the provider enum value, so the generated uids are unchanged). - The five controllers shrink to their genuine differences (client, names, logo, error copy, and — for IB — the Flex error mapping and empty-statement guard). Net **−110 lines** across the five `store()` methods, and the connection lifecycle now lives in one place. ## Behavior No functional change. The credential-failure warning log is now a single structured message with a `provider` key instead of five free-text variants (the only observable difference; HTTP responses are byte-for-byte identical). Verified by the full suite: **1869 passing, 0 failing** (`--exclude-testsuite=Browser`); `pint --test`, `format` and `lint` clean. ## Review-driven follow-up commits Two review agents (architecture/duplication/coverage, and behavior/regressions) ran against the first commit. Applied, each as its own commit: - Finished the crypto dedup via the intermediate base (the reviewers flagged the still-duplicated crypto hooks). - Dropped the over-engineered per-provider `validationFailureLogMessage()` hook in favor of a structured base log. - Added the missing test for the Interactive Brokers empty-statement guard (the only conditional hook, previously unexercised): asserts 422 + NAV-section message, no connection row, and no warning logged. ## Deliberately out of scope - **Wise** is left as-is: it builds pending accounts mid-flow (needs the client), creates the connection in one step, and has its own empty-portfolio guard — it does not fit this template. - **Pre-existing** (not introduced here, noted by review): `Bank::firstOrCreate` returns an existing bank without refreshing its logo, so `aspsp_logo` can copy a stale/null value. Worth a separate fix. --- .../OpenBanking/BinanceController.php | 91 ++++-------- .../OpenBanking/BitpandaController.php | 91 ++++-------- .../OpenBanking/CoinbaseController.php | 91 ++++-------- .../CryptoPortfolioConnectController.php | 28 ++++ .../OpenBanking/IndexaCapitalController.php | 96 +++++-------- .../InteractiveBrokersController.php | 109 ++++++--------- .../OpenBankingConnectController.php | 131 ++++++++++++++++++ .../InteractiveBrokersControllerTest.php | 22 +++ 8 files changed, 328 insertions(+), 331 deletions(-) create mode 100644 app/Http/Controllers/OpenBanking/CryptoPortfolioConnectController.php create mode 100644 app/Http/Controllers/OpenBanking/OpenBankingConnectController.php diff --git a/app/Http/Controllers/OpenBanking/BinanceController.php b/app/Http/Controllers/OpenBanking/BinanceController.php index 40426412..81aed0f7 100644 --- a/app/Http/Controllers/OpenBanking/BinanceController.php +++ b/app/Http/Controllers/OpenBanking/BinanceController.php @@ -2,88 +2,47 @@ namespace App\Http\Controllers\OpenBanking; -use App\Enums\BankingConnectionStatus; use App\Enums\BankingProvider; -use App\Http\Controllers\Controller; -use App\Http\Controllers\OpenBanking\Concerns\CreatesAccountsFromPending; -use App\Http\Controllers\OpenBanking\Concerns\HandlesSubscriptionGate; use App\Http\Requests\OpenBanking\ConnectBinanceRequest; -use App\Jobs\SyncBankingConnectionJob; -use App\Models\Bank; use App\Services\AccountUserCurrencyService; use App\Services\Banking\BinanceClient; use Illuminate\Http\JsonResponse; -use Illuminate\Support\Facades\Log; -class BinanceController extends Controller +class BinanceController extends CryptoPortfolioConnectController { - use CreatesAccountsFromPending; - use HandlesSubscriptionGate; - /** * Validate Binance API credentials and create a connection. */ public function store(ConnectBinanceRequest $request, AccountUserCurrencyService $accountUserCurrencyService): JsonResponse { - $validated = $request->validated(); - $user = auth()->user(); + return $this->connect($request->validated(), $accountUserCurrencyService); + } - if ($this->shouldBlockOpenBankingAccess($user)) { - return $this->subscribeJsonResponse(); - } + protected function provider(): BankingProvider + { + return BankingProvider::Binance; + } + protected function providerName(): string + { + return 'Binance'; + } + + protected function bankLogo(): ?string + { + return 'https://whisper.money/storage/banks/logos/t1h5rqi19dJTPl6ZadziPjNwm0lrcdTFBRzB3iCy.png'; + } + + protected function fetchProviderData(array $validated): mixed + { $client = new BinanceClient($validated['api_key'], $validated['api_secret']); + $client->getAccount(); - try { - $client->getAccount(); - } catch (\Throwable $e) { - Log::warning('Binance credential validation failed', ['error' => $e->getMessage()]); + return null; + } - return response()->json([ - 'message' => 'Invalid API credentials or failed to connect to Binance.', - ], 422); - } - - $bank = Bank::firstOrCreate( - ['name' => 'Binance', 'user_id' => null], - ['name' => 'Binance', 'logo' => 'https://whisper.money/storage/banks/logos/t1h5rqi19dJTPl6ZadziPjNwm0lrcdTFBRzB3iCy.png'], - ); - - $connection = $user->bankingConnections()->create([ - 'provider' => BankingProvider::Binance, - ...BankingProvider::Binance->credentialColumns($validated), - 'aspsp_name' => 'Binance', - 'aspsp_country' => $validated['country'], - 'aspsp_logo' => $bank->logo, - 'status' => BankingConnectionStatus::Pending, - ]); - - $pendingAccounts = [ - [ - 'uid' => 'binance-portfolio', - 'currency' => $user->currency_code, - 'name' => 'Crypto Portfolio', - ], - ]; - - $connection->update([ - 'status' => BankingConnectionStatus::AwaitingMapping, - 'pending_accounts_data' => $pendingAccounts, - ]); - - if (! $user->isOnboarded()) { - $this->createAccountsFromPending($user, $connection, $accountUserCurrencyService); - SyncBankingConnectionJob::dispatch($connection); - - return response()->json([ - 'redirect_url' => route('onboarding', ['step' => 'create-account']), - 'connection_id' => $connection->id, - ]); - } - - return response()->json([ - 'redirect_url' => route('open-banking.map-accounts', $connection), - 'connection_id' => $connection->id, - ]); + protected function credentialErrorMessage(\Throwable $e): string + { + return 'Invalid API credentials or failed to connect to Binance.'; } } diff --git a/app/Http/Controllers/OpenBanking/BitpandaController.php b/app/Http/Controllers/OpenBanking/BitpandaController.php index c0365550..bc3779ff 100644 --- a/app/Http/Controllers/OpenBanking/BitpandaController.php +++ b/app/Http/Controllers/OpenBanking/BitpandaController.php @@ -2,88 +2,47 @@ namespace App\Http\Controllers\OpenBanking; -use App\Enums\BankingConnectionStatus; use App\Enums\BankingProvider; -use App\Http\Controllers\Controller; -use App\Http\Controllers\OpenBanking\Concerns\CreatesAccountsFromPending; -use App\Http\Controllers\OpenBanking\Concerns\HandlesSubscriptionGate; use App\Http\Requests\OpenBanking\ConnectBitpandaRequest; -use App\Jobs\SyncBankingConnectionJob; -use App\Models\Bank; use App\Services\AccountUserCurrencyService; use App\Services\Banking\BitpandaClient; use Illuminate\Http\JsonResponse; -use Illuminate\Support\Facades\Log; -class BitpandaController extends Controller +class BitpandaController extends CryptoPortfolioConnectController { - use CreatesAccountsFromPending; - use HandlesSubscriptionGate; - /** * Validate Bitpanda API key and create a connection. */ public function store(ConnectBitpandaRequest $request, AccountUserCurrencyService $accountUserCurrencyService): JsonResponse { - $validated = $request->validated(); - $user = auth()->user(); + return $this->connect($request->validated(), $accountUserCurrencyService); + } - if ($this->shouldBlockOpenBankingAccess($user)) { - return $this->subscribeJsonResponse(); - } + protected function provider(): BankingProvider + { + return BankingProvider::Bitpanda; + } + protected function providerName(): string + { + return 'Bitpanda'; + } + + protected function bankLogo(): ?string + { + return 'https://whisper.money/storage/banks/logos/7Y6gl0gaFH1mStJMcUQ9VpgzX1kduyumm0dDhGlf.png'; + } + + protected function fetchProviderData(array $validated): mixed + { $client = new BitpandaClient($validated['api_key']); + $client->getCryptoWallets(); - try { - $client->getCryptoWallets(); - } catch (\Throwable $e) { - Log::warning('Bitpanda credential validation failed', ['error' => $e->getMessage()]); + return null; + } - return response()->json([ - 'message' => 'Invalid API key or failed to connect to Bitpanda.', - ], 422); - } - - $bank = Bank::firstOrCreate( - ['name' => 'Bitpanda', 'user_id' => null], - ['name' => 'Bitpanda', 'logo' => 'https://whisper.money/storage/banks/logos/7Y6gl0gaFH1mStJMcUQ9VpgzX1kduyumm0dDhGlf.png'], - ); - - $connection = $user->bankingConnections()->create([ - 'provider' => BankingProvider::Bitpanda, - ...BankingProvider::Bitpanda->credentialColumns($validated), - 'aspsp_name' => 'Bitpanda', - 'aspsp_country' => $validated['country'], - 'aspsp_logo' => $bank->logo, - 'status' => BankingConnectionStatus::Pending, - ]); - - $pendingAccounts = [ - [ - 'uid' => 'bitpanda-portfolio', - 'currency' => $user->currency_code, - 'name' => 'Crypto Portfolio', - ], - ]; - - $connection->update([ - 'status' => BankingConnectionStatus::AwaitingMapping, - 'pending_accounts_data' => $pendingAccounts, - ]); - - if (! $user->isOnboarded()) { - $this->createAccountsFromPending($user, $connection, $accountUserCurrencyService); - SyncBankingConnectionJob::dispatch($connection); - - return response()->json([ - 'redirect_url' => route('onboarding', ['step' => 'create-account']), - 'connection_id' => $connection->id, - ]); - } - - return response()->json([ - 'redirect_url' => route('open-banking.map-accounts', $connection), - 'connection_id' => $connection->id, - ]); + protected function credentialErrorMessage(\Throwable $e): string + { + return 'Invalid API key or failed to connect to Bitpanda.'; } } diff --git a/app/Http/Controllers/OpenBanking/CoinbaseController.php b/app/Http/Controllers/OpenBanking/CoinbaseController.php index 312728e5..0628278a 100644 --- a/app/Http/Controllers/OpenBanking/CoinbaseController.php +++ b/app/Http/Controllers/OpenBanking/CoinbaseController.php @@ -2,88 +2,47 @@ namespace App\Http\Controllers\OpenBanking; -use App\Enums\BankingConnectionStatus; use App\Enums\BankingProvider; -use App\Http\Controllers\Controller; -use App\Http\Controllers\OpenBanking\Concerns\CreatesAccountsFromPending; -use App\Http\Controllers\OpenBanking\Concerns\HandlesSubscriptionGate; use App\Http\Requests\OpenBanking\ConnectCoinbaseRequest; -use App\Jobs\SyncBankingConnectionJob; -use App\Models\Bank; use App\Services\AccountUserCurrencyService; use App\Services\Banking\CoinbaseClient; use Illuminate\Http\JsonResponse; -use Illuminate\Support\Facades\Log; -class CoinbaseController extends Controller +class CoinbaseController extends CryptoPortfolioConnectController { - use CreatesAccountsFromPending; - use HandlesSubscriptionGate; - /** * Validate Coinbase CDP API credentials and create a connection. */ public function store(ConnectCoinbaseRequest $request, AccountUserCurrencyService $accountUserCurrencyService): JsonResponse { - $validated = $request->validated(); - $user = auth()->user(); + return $this->connect($request->validated(), $accountUserCurrencyService); + } - if ($this->shouldBlockOpenBankingAccess($user)) { - return $this->subscribeJsonResponse(); - } + protected function provider(): BankingProvider + { + return BankingProvider::Coinbase; + } + protected function providerName(): string + { + return 'Coinbase'; + } + + protected function bankLogo(): ?string + { + return 'https://whisper.money/storage/banks/logos/coinbase.png'; + } + + protected function fetchProviderData(array $validated): mixed + { $client = new CoinbaseClient($validated['api_key_name'], $validated['private_key']); + $client->getAccounts(limit: 1); - try { - $client->getAccounts(limit: 1); - } catch (\Throwable $e) { - Log::warning('Coinbase credential validation failed', ['error' => $e->getMessage()]); + return null; + } - return response()->json([ - 'message' => 'Invalid API credentials or failed to connect to Coinbase.', - ], 422); - } - - $bank = Bank::firstOrCreate( - ['name' => 'Coinbase', 'user_id' => null], - ['name' => 'Coinbase', 'logo' => 'https://whisper.money/storage/banks/logos/coinbase.png'], - ); - - $connection = $user->bankingConnections()->create([ - 'provider' => BankingProvider::Coinbase, - ...BankingProvider::Coinbase->credentialColumns($validated), - 'aspsp_name' => 'Coinbase', - 'aspsp_country' => $validated['country'], - 'aspsp_logo' => $bank->logo, - 'status' => BankingConnectionStatus::Pending, - ]); - - $pendingAccounts = [ - [ - 'uid' => 'coinbase-portfolio', - 'currency' => $user->currency_code, - 'name' => 'Crypto Portfolio', - ], - ]; - - $connection->update([ - 'status' => BankingConnectionStatus::AwaitingMapping, - 'pending_accounts_data' => $pendingAccounts, - ]); - - if (! $user->isOnboarded()) { - $this->createAccountsFromPending($user, $connection, $accountUserCurrencyService); - SyncBankingConnectionJob::dispatch($connection); - - return response()->json([ - 'redirect_url' => route('onboarding', ['step' => 'create-account']), - 'connection_id' => $connection->id, - ]); - } - - return response()->json([ - 'redirect_url' => route('open-banking.map-accounts', $connection), - 'connection_id' => $connection->id, - ]); + protected function credentialErrorMessage(\Throwable $e): string + { + return 'Invalid API credentials or failed to connect to Coinbase.'; } } diff --git a/app/Http/Controllers/OpenBanking/CryptoPortfolioConnectController.php b/app/Http/Controllers/OpenBanking/CryptoPortfolioConnectController.php new file mode 100644 index 00000000..f45ad7f8 --- /dev/null +++ b/app/Http/Controllers/OpenBanking/CryptoPortfolioConnectController.php @@ -0,0 +1,28 @@ + $this->provider()->value.'-portfolio', + 'currency' => $user->currency_code, + 'name' => 'Crypto Portfolio', + ], + ]; + } +} diff --git a/app/Http/Controllers/OpenBanking/IndexaCapitalController.php b/app/Http/Controllers/OpenBanking/IndexaCapitalController.php index 7dfd0c28..66ca187d 100644 --- a/app/Http/Controllers/OpenBanking/IndexaCapitalController.php +++ b/app/Http/Controllers/OpenBanking/IndexaCapitalController.php @@ -2,95 +2,65 @@ namespace App\Http\Controllers\OpenBanking; -use App\Enums\BankingConnectionStatus; use App\Enums\BankingProvider; -use App\Http\Controllers\Controller; -use App\Http\Controllers\OpenBanking\Concerns\CreatesAccountsFromPending; -use App\Http\Controllers\OpenBanking\Concerns\HandlesSubscriptionGate; use App\Http\Requests\OpenBanking\ConnectIndexaCapitalRequest; -use App\Jobs\SyncBankingConnectionJob; -use App\Models\Bank; +use App\Models\User; use App\Services\AccountUserCurrencyService; use App\Services\Banking\IndexaCapitalClient; use Illuminate\Http\JsonResponse; -use Illuminate\Support\Facades\Log; -class IndexaCapitalController extends Controller +class IndexaCapitalController extends OpenBankingConnectController { - use CreatesAccountsFromPending; - use HandlesSubscriptionGate; - /** * Validate the Indexa Capital API token and create a connection. */ public function store(ConnectIndexaCapitalRequest $request, AccountUserCurrencyService $accountUserCurrencyService): JsonResponse { - $validated = $request->validated(); - $user = auth()->user(); + return $this->connect($request->validated(), $accountUserCurrencyService); + } - if ($this->shouldBlockOpenBankingAccess($user)) { - return $this->subscribeJsonResponse(); - } + protected function provider(): BankingProvider + { + return BankingProvider::IndexaCapital; + } + protected function providerName(): string + { + return 'Indexa Capital'; + } + + protected function bankLogo(): ?string + { + return '/images/banks/logos/indexa-capital.jpg'; + } + + protected function aspspCountry(array $validated): string + { + return 'ES'; + } + + protected function fetchProviderData(array $validated): mixed + { $client = new IndexaCapitalClient($validated['api_token']); - try { - $userData = $client->getUser(); - } catch (\Throwable $e) { - Log::warning('Indexa Capital token validation failed', ['error' => $e->getMessage()]); + return $client->getUser(); + } - return response()->json([ - 'message' => 'Invalid API token or failed to connect to Indexa Capital.', - ], 422); - } - - $bank = Bank::firstOrCreate( - ['name' => 'Indexa Capital', 'user_id' => null], - ['name' => 'Indexa Capital', 'logo' => '/images/banks/logos/indexa-capital.jpg'], - ); - - $connection = $user->bankingConnections()->create([ - 'provider' => BankingProvider::IndexaCapital, - ...BankingProvider::IndexaCapital->credentialColumns($validated), - 'aspsp_name' => 'Indexa Capital', - 'aspsp_country' => 'ES', - 'aspsp_logo' => $bank->logo, - 'status' => BankingConnectionStatus::Pending, - ]); - - $pendingAccounts = $this->buildPendingAccounts($userData); - - $connection->update([ - 'status' => BankingConnectionStatus::AwaitingMapping, - 'pending_accounts_data' => $pendingAccounts, - ]); - - if (! $user->isOnboarded()) { - $this->createAccountsFromPending($user, $connection, $accountUserCurrencyService); - SyncBankingConnectionJob::dispatch($connection); - - return response()->json([ - 'redirect_url' => route('onboarding', ['step' => 'create-account']), - 'connection_id' => $connection->id, - ]); - } - - return response()->json([ - 'redirect_url' => route('open-banking.map-accounts', $connection), - 'connection_id' => $connection->id, - ]); + protected function credentialErrorMessage(\Throwable $e): string + { + return 'Invalid API token or failed to connect to Indexa Capital.'; } /** * Build the pending accounts data in the same format as EnableBanking. * - * @return array + * @param array{accounts?: array} $providerData */ - private function buildPendingAccounts(array $userData): array + protected function buildPendingAccounts(mixed $providerData, User $user): array { $accounts = []; - foreach ($userData['accounts'] ?? [] as $account) { + foreach ($providerData['accounts'] ?? [] as $account) { $accountNumber = $account['account_number'] ?? null; if (! $accountNumber) { diff --git a/app/Http/Controllers/OpenBanking/InteractiveBrokersController.php b/app/Http/Controllers/OpenBanking/InteractiveBrokersController.php index 19469e47..15127b00 100644 --- a/app/Http/Controllers/OpenBanking/InteractiveBrokersController.php +++ b/app/Http/Controllers/OpenBanking/InteractiveBrokersController.php @@ -2,96 +2,57 @@ namespace App\Http\Controllers\OpenBanking; -use App\Enums\BankingConnectionStatus; use App\Enums\BankingProvider; use App\Exceptions\Banking\TransientBankingProviderException; -use App\Http\Controllers\Controller; -use App\Http\Controllers\OpenBanking\Concerns\CreatesAccountsFromPending; -use App\Http\Controllers\OpenBanking\Concerns\HandlesSubscriptionGate; use App\Http\Requests\OpenBanking\ConnectInteractiveBrokersRequest; -use App\Jobs\SyncBankingConnectionJob; -use App\Models\Bank; +use App\Models\User; use App\Services\AccountUserCurrencyService; use App\Services\Banking\InteractiveBrokersClient; use Illuminate\Http\Client\RequestException; use Illuminate\Http\JsonResponse; -use Illuminate\Support\Facades\Log; -class InteractiveBrokersController extends Controller +class InteractiveBrokersController extends OpenBankingConnectController { - use CreatesAccountsFromPending; - use HandlesSubscriptionGate; - /** * Validate the Flex credentials and create a connection. */ public function store(ConnectInteractiveBrokersRequest $request, AccountUserCurrencyService $accountUserCurrencyService): JsonResponse { - $validated = $request->validated(); - $user = auth()->user(); + return $this->connect($request->validated(), $accountUserCurrencyService); + } - if ($this->shouldBlockOpenBankingAccess($user)) { - return $this->subscribeJsonResponse(); - } + protected function provider(): BankingProvider + { + return BankingProvider::InteractiveBrokers; + } + protected function providerName(): string + { + return 'Interactive Brokers'; + } + + protected function bankLogo(): ?string + { + return '/images/banks/logos/interactive-brokers.png'; + } + + protected function aspspCountry(array $validated): string + { + return 'US'; + } + + protected function fetchProviderData(array $validated): mixed + { $client = new InteractiveBrokersClient($validated['token'], $validated['query_id']); - try { - $accounts = $client->fetchStatement(); - } catch (\Throwable $e) { - Log::warning('Interactive Brokers connection validation failed', ['error' => $e->getMessage()]); - - return response()->json([ - 'message' => $this->connectErrorMessage($e), - ], 422); - } - - if (empty($accounts)) { - return response()->json([ - 'message' => 'No accounts found in the Flex statement. Check that your Flex Query includes the NAV section.', - ], 422); - } - - $bank = Bank::firstOrCreate( - ['name' => 'Interactive Brokers', 'user_id' => null], - ['name' => 'Interactive Brokers', 'logo' => '/images/banks/logos/interactive-brokers.png'], - ); - - $connection = $user->bankingConnections()->create([ - 'provider' => BankingProvider::InteractiveBrokers, - ...BankingProvider::InteractiveBrokers->credentialColumns($validated), - 'aspsp_name' => 'Interactive Brokers', - 'aspsp_country' => 'US', - 'aspsp_logo' => $bank->logo, - 'status' => BankingConnectionStatus::Pending, - ]); - - $connection->update([ - 'status' => BankingConnectionStatus::AwaitingMapping, - 'pending_accounts_data' => $this->buildPendingAccounts($accounts), - ]); - - if (! $user->isOnboarded()) { - $this->createAccountsFromPending($user, $connection, $accountUserCurrencyService); - SyncBankingConnectionJob::dispatch($connection); - - return response()->json([ - 'redirect_url' => route('onboarding', ['step' => 'create-account']), - 'connection_id' => $connection->id, - ]); - } - - return response()->json([ - 'redirect_url' => route('open-banking.map-accounts', $connection), - 'connection_id' => $connection->id, - ]); + return $client->fetchStatement(); } /** * Turn a Flex failure into a message the user can act on: bad credentials, * a busy/rate-limited service, or a statement that is still generating. */ - private function connectErrorMessage(\Throwable $e): string + protected function credentialErrorMessage(\Throwable $e): string { if ($e instanceof RequestException && in_array($e->response->status(), [401, 403], true)) { return 'Invalid Flex token or query ID, or failed to connect to Interactive Brokers.'; @@ -108,17 +69,25 @@ class InteractiveBrokersController extends Controller return 'Invalid Flex token or query ID, or failed to connect to Interactive Brokers.'; } + protected function emptyProviderDataMessage(mixed $providerData): ?string + { + if (empty($providerData)) { + return 'No accounts found in the Flex statement. Check that your Flex Query includes the NAV section.'; + } + + return null; + } + /** * Build pending accounts from the parsed Flex statement. * - * @param array, investedAmount: float|null}> $accounts - * @return array + * @param array, investedAmount: float|null}> $providerData */ - private function buildPendingAccounts(array $accounts): array + protected function buildPendingAccounts(mixed $providerData, User $user): array { $pending = []; - foreach ($accounts as $account) { + foreach ($providerData as $account) { $pending[] = [ 'uid' => $account['account_id'], 'currency' => $account['currency'] !== '' ? $account['currency'] : 'USD', diff --git a/app/Http/Controllers/OpenBanking/OpenBankingConnectController.php b/app/Http/Controllers/OpenBanking/OpenBankingConnectController.php new file mode 100644 index 00000000..310c383b --- /dev/null +++ b/app/Http/Controllers/OpenBanking/OpenBankingConnectController.php @@ -0,0 +1,131 @@ + $validated + */ + abstract protected function aspspCountry(array $validated): string; + + /** + * Build the provider client, validate the credentials, and return whatever + * data is needed to build pending accounts (or null). Must throw on failure. + * + * @param array $validated + */ + abstract protected function fetchProviderData(array $validated): mixed; + + abstract protected function credentialErrorMessage(\Throwable $e): string; + + /** + * @return array + */ + abstract protected function buildPendingAccounts(mixed $providerData, User $user): array; + + /** + * Optional guard: return a 422 message when the validated provider data is + * unusable (e.g. an empty statement). Returning null keeps the flow going. + */ + protected function emptyProviderDataMessage(mixed $providerData): ?string + { + return null; + } + + /** + * Shared connect flow: gate on subscription, validate the credentials, create + * the pending connection, then auto-map (onboarding) or redirect to mapping. + * + * @param array $validated + */ + protected function connect(array $validated, AccountUserCurrencyService $accountUserCurrencyService): JsonResponse + { + $user = auth()->user(); + + if ($this->shouldBlockOpenBankingAccess($user)) { + return $this->subscribeJsonResponse(); + } + + try { + $providerData = $this->fetchProviderData($validated); + } catch (\Throwable $e) { + Log::warning('OpenBanking credential validation failed', [ + 'provider' => $this->provider()->value, + 'error' => $e->getMessage(), + ]); + + return response()->json([ + 'message' => $this->credentialErrorMessage($e), + ], 422); + } + + if (($message = $this->emptyProviderDataMessage($providerData)) !== null) { + return response()->json(['message' => $message], 422); + } + + $bank = Bank::firstOrCreate( + ['name' => $this->providerName(), 'user_id' => null], + ['name' => $this->providerName(), 'logo' => $this->bankLogo()], + ); + + $connection = $user->bankingConnections()->create([ + 'provider' => $this->provider(), + ...$this->provider()->credentialColumns($validated), + 'aspsp_name' => $this->providerName(), + 'aspsp_country' => $this->aspspCountry($validated), + 'aspsp_logo' => $bank->logo, + 'status' => BankingConnectionStatus::Pending, + ]); + + $connection->update([ + 'status' => BankingConnectionStatus::AwaitingMapping, + 'pending_accounts_data' => $this->buildPendingAccounts($providerData, $user), + ]); + + return $this->connectionResponse($user, $connection, $accountUserCurrencyService); + } + + private function connectionResponse(User $user, BankingConnection $connection, AccountUserCurrencyService $accountUserCurrencyService): JsonResponse + { + if (! $user->isOnboarded()) { + $this->createAccountsFromPending($user, $connection, $accountUserCurrencyService); + SyncBankingConnectionJob::dispatch($connection); + + return response()->json([ + 'redirect_url' => route('onboarding', ['step' => 'create-account']), + 'connection_id' => $connection->id, + ]); + } + + return response()->json([ + 'redirect_url' => route('open-banking.map-accounts', $connection), + 'connection_id' => $connection->id, + ]); + } +} diff --git a/tests/Feature/OpenBanking/InteractiveBrokersControllerTest.php b/tests/Feature/OpenBanking/InteractiveBrokersControllerTest.php index e780e7a0..eda548a8 100644 --- a/tests/Feature/OpenBanking/InteractiveBrokersControllerTest.php +++ b/tests/Feature/OpenBanking/InteractiveBrokersControllerTest.php @@ -6,6 +6,7 @@ use App\Models\Bank; use App\Models\BankingConnection; use App\Models\User; use Illuminate\Support\Facades\Http; +use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Queue; use Illuminate\Support\Sleep; @@ -164,3 +165,24 @@ test('reports a rate-limit message when IB throttles the request', function () { ->assertUnprocessable() ->assertJsonFragment(['message' => 'Interactive Brokers is rate limiting requests. Please wait a few minutes and try again.']); }); + +test('a valid but empty flex statement returns 422 without creating a connection or logging a warning', function () { + Log::spy(); + + $user = User::factory()->onboarded()->create(); + Http::fake([ + '*SendRequest*' => Http::response('Success999'), + '*GetStatement*' => Http::response(''), + ]); + + $this->actingAs($user)->postJson('/open-banking/interactive-brokers/connect', ibConnect()) + ->assertUnprocessable() + ->assertJsonFragment(['message' => 'No accounts found in the Flex statement. Check that your Flex Query includes the NAV section.']); + + $this->assertDatabaseMissing('banking_connections', [ + 'user_id' => $user->id, + 'provider' => 'interactivebrokers', + ]); + + Log::shouldNotHaveReceived('warning'); +});