From baacd248392286fe463bbbadfb8e1bc7d9a64dbf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Vi=CC=81ctor=20Falco=CC=81n?= <victoor89@gmail.com>
Date: Wed, 12 Aug 2026 11:20:41 +0200
Subject: [PATCH] refactor(open-banking): extract the callback authorization
 error branch

The error-param cleanup and the two isOnboarded ternaries picking the failure
destination were the bulk of callback()'s complexity. Pull them into
handleAuthorizationError() and failureRedirect(), which also collapses the four
repeated (route, params, 'error', message) argument lists into one call each.

No behaviour change.
---
 .../OpenBanking/AuthorizationController.php   | 88 ++++++++++++-------
 1 file changed, 54 insertions(+), 34 deletions(-)

diff --git a/app/Http/Controllers/OpenBanking/AuthorizationController.php b/app/Http/Controllers/OpenBanking/AuthorizationController.php
index bcaa4719..a4499b4c 100644
--- a/app/Http/Controllers/OpenBanking/AuthorizationController.php
+++ b/app/Http/Controllers/OpenBanking/AuthorizationController.php
@@ -165,44 +165,14 @@ class AuthorizationController extends Controller
                 ->with('error', __('Please log back in to finish connecting your bank account.'));
         }
 
-        $errorRedirectRoute = $user->isOnboarded() ? 'settings.connections.index' : 'onboarding';
-        $errorRedirectParams = $user->isOnboarded() ? [] : ['step' => 'create-account'];
-
         if ($request->has('error')) {
-            $errorDescription = $request->query('error_description');
-            $errorMessage = is_string($errorDescription) && $errorDescription !== ''
-                ? $errorDescription
-                : 'Authorization was denied or cancelled.';
-
-            Log::warning('EnableBanking authorization error', [
-                'error' => $request->query('error'),
-                'description' => $errorDescription,
-            ]);
-
-            $pendingConnection = $connection ?? $user->bankingConnections()
-                ->where('status', BankingConnectionStatus::Pending)
-                ->latest()
-                ->first();
-
-            if ($pendingConnection) {
-                if ($pendingConnection->accounts()->exists()) {
-                    $pendingConnection->update([
-                        'status' => BankingConnectionStatus::Error,
-                        'error_message' => $errorMessage,
-                        'state_token' => null,
-                    ]);
-                } else {
-                    $pendingConnection->delete();
-                }
-            }
-
-            return $this->finishRedirect($errorRedirectRoute, $errorRedirectParams, 'error', $errorMessage);
+            return $this->handleAuthorizationError($request, $user, $connection);
         }
 
         $code = $request->query('code');
 
         if (! $code) {
-            return $this->finishRedirect($errorRedirectRoute, $errorRedirectParams, 'error', 'No authorization code received.');
+            return $this->failureRedirect($user, 'No authorization code received.');
         }
 
         try {
@@ -214,13 +184,13 @@ class AuthorizationController extends Controller
                 $connection->update(['state_token' => null]);
             }
 
-            return $this->finishRedirect($errorRedirectRoute, $errorRedirectParams, 'error', 'Failed to connect to your bank. Please try again.');
+            return $this->failureRedirect($user, 'Failed to connect to your bank. Please try again.');
         }
 
         $connection ??= $this->findPendingConnectionForSession($user, $sessionData);
 
         if (! $connection) {
-            return $this->finishRedirect($errorRedirectRoute, $errorRedirectParams, 'error', 'No pending connection found.');
+            return $this->failureRedirect($user, 'No pending connection found.');
         }
 
         $isReconnect = $connection->accounts()->exists();
@@ -264,6 +234,56 @@ class AuthorizationController extends Controller
         return $this->finishRedirect('open-banking.map-accounts', ['connection' => $connection]);
     }
 
+    /**
+     * Clean up after a bank that denied or cancelled the authorization.
+     *
+     * A pending connection that already has accounts is a reconnect, so it is kept
+     * and marked as failing. A brand new one has nothing worth keeping.
+     */
+    private function handleAuthorizationError(Request $request, User $user, ?BankingConnection $connection): RedirectResponse|Response
+    {
+        $errorDescription = $request->query('error_description');
+        $errorMessage = is_string($errorDescription) && $errorDescription !== ''
+            ? $errorDescription
+            : 'Authorization was denied or cancelled.';
+
+        Log::warning('EnableBanking authorization error', [
+            'error' => $request->query('error'),
+            'description' => $errorDescription,
+        ]);
+
+        $pendingConnection = $connection ?? $user->bankingConnections()
+            ->where('status', BankingConnectionStatus::Pending)
+            ->latest()
+            ->first();
+
+        if ($pendingConnection) {
+            if ($pendingConnection->accounts()->exists()) {
+                $pendingConnection->update([
+                    'status' => BankingConnectionStatus::Error,
+                    'error_message' => $errorMessage,
+                    'state_token' => null,
+                ]);
+            } else {
+                $pendingConnection->delete();
+            }
+        }
+
+        return $this->failureRedirect($user, $errorMessage);
+    }
+
+    /**
+     * Abandon the callback with a message, sending the user wherever they came from.
+     *
+     * A user still onboarding has no connections screen to land on yet.
+     */
+    private function failureRedirect(User $user, string $message): RedirectResponse|Response
+    {
+        return $user->isOnboarded()
+            ? $this->finishRedirect('settings.connections.index', [], 'error', $message)
+            : $this->finishRedirect('onboarding', ['step' => 'create-account'], 'error', $message);
+    }
+
     /**
      * Resolve the connection a callback belongs to from the state token EnableBanking
      * echoes back. This works without a logged-in session.