From 4f4faec3595bd40409657b1b15b413ff0390e15e Mon Sep 17 00:00:00 2001
From: lohit <lohit@usebruno.com>
Date: Fri, 27 Feb 2026 15:18:51 +0000
Subject: [PATCH] fix(`oauth2`): prevent false callback matches on `root path
 URLs` (#7315)

* fix(oauth2): prevent false callback matches on root path URLs and handle errors first

Move error check before callback URL matching in onWindowRedirect so
OAuth error responses are rejected immediately. Remove redundant error
param check from matchesCallbackUrl and require a code param or hash
fragment to match, preventing false positives on intermediate pages
when the callback URL is a root path like https://hostname/.

* fix(oauth2): clarify error handling comment

Remove "on the callback URL" from comment since error checking
now happens before callback URL matching.
---
 .../ipc/network/authorize-user-in-window.js   | 25 +++++++----
 .../tests/network/authorize-user.spec.js      | 41 +++++++++++++++++++
 2 files changed, 57 insertions(+), 9 deletions(-)

diff --git a/packages/bruno-electron/src/ipc/network/authorize-user-in-window.js b/packages/bruno-electron/src/ipc/network/authorize-user-in-window.js
index f713e828d..e8ecf994b 100644
--- a/packages/bruno-electron/src/ipc/network/authorize-user-in-window.js
+++ b/packages/bruno-electron/src/ipc/network/authorize-user-in-window.js
@@ -2,7 +2,13 @@ const { BrowserWindow } = require('electron');
 const { preferencesUtil } = require('../../store/preferences');
 
 const matchesCallbackUrl = (url, callbackUrl) => {
-  return url ? url.href.startsWith(callbackUrl.href) : false;
+  if (!url) return false;
+  // Match the callback URL and require an OAuth2 response indicator
+  // (code query params for authorization code flow, or hash fragment for implicit flow).
+  // This prevents false matches on intermediate pages (e.g. /auth/login) when the
+  // callback URL is a root path like https://hostname/.
+  return url.href.startsWith(callbackUrl.href)
+    && (url.searchParams.has('code') || url.hash.length > 1);
 };
 
 const authorizeUserInWindow = ({ authorizeUrl, callbackUrl, session, additionalHeaders = {}, grantType = 'authorization_code' }) => {
@@ -145,14 +151,8 @@ const authorizeUserInWindow = ({ authorizeUrl, callbackUrl, session, additionalH
         callbackUrlObj = null;
       }
 
-      // Check if redirect is to the callback URL and contains an authorization code
-      if (callbackUrlObj && matchesCallbackUrl(urlObj, callbackUrlObj)) {
-        finalUrl = url;
-        window.close();
-        return;
-      }
-
-      // Handle OAuth error responses
+      // Handle OAuth error responses first, so we reject with
+      // a descriptive error instead of resolving with a null authorization code
       if (urlObj.searchParams.has('error')) {
         const error = urlObj.searchParams.get('error');
         const errorDescription = urlObj.searchParams.get('error_description');
@@ -165,6 +165,13 @@ const authorizeUserInWindow = ({ authorizeUrl, callbackUrl, session, additionalH
         };
         reject(new Error(JSON.stringify(errorData)));
         window.close();
+        return;
+      }
+
+      if (callbackUrlObj && matchesCallbackUrl(urlObj, callbackUrlObj)) {
+        finalUrl = url;
+        window.close();
+        return;
       }
     }
 
diff --git a/packages/bruno-electron/tests/network/authorize-user.spec.js b/packages/bruno-electron/tests/network/authorize-user.spec.js
index 03c76cfb8..056d32e73 100644
--- a/packages/bruno-electron/tests/network/authorize-user.spec.js
+++ b/packages/bruno-electron/tests/network/authorize-user.spec.js
@@ -16,4 +16,45 @@ describe('matchesCallbackUrl', () => {
 
     expect(actual).toBe(expected);
   });
+
+  describe('root path callback URL', () => {
+    const rootPathCases = [
+      { url: 'https://hostname/auth/login', expected: false, desc: 'intermediate login page without code' },
+      { url: 'https://hostname/consent', expected: false, desc: 'intermediate consent page without code' },
+      { url: 'https://hostname/?code=abcd', expected: true, desc: 'root callback with authorization code' },
+      { url: 'https://hostname/?error=access_denied', expected: false, desc: 'root callback with error (handled separately by onWindowRedirect)' },
+      { url: 'https://hostname/#access_token=xyz', expected: true, desc: 'root callback with implicit flow hash' },
+      { url: 'https://hostname/', expected: false, desc: 'root path without any OAuth2 params' },
+      { url: 'https://other-host/?code=abcd', expected: false, desc: 'different host with code param' }
+    ];
+
+    it.each(rootPathCases)('$desc ($url) - should be $expected', ({ url, expected }) => {
+      let callBackUrl = 'https://hostname/';
+
+      let actual = matchesCallbackUrl(new URL(url), new URL(callBackUrl));
+
+      expect(actual).toBe(expected);
+    });
+  });
+
+  describe('implicit flow with hash fragments', () => {
+    const implicitCases = [
+      { url: 'https://callback.url/endpoint#access_token=xyz&token_type=bearer', expected: true, desc: 'callback with hash fragment' },
+      { url: 'https://callback.url/endpoint#', expected: false, desc: 'callback with empty hash' },
+      { url: 'https://callback.url/endpoint', expected: false, desc: 'callback without hash or code' }
+    ];
+
+    it.each(implicitCases)('$desc ($url) - should be $expected', ({ url, expected }) => {
+      let callBackUrl = 'https://callback.url/endpoint';
+
+      let actual = matchesCallbackUrl(new URL(url), new URL(callBackUrl));
+
+      expect(actual).toBe(expected);
+    });
+  });
+
+  it('should return false for null url', () => {
+    let callBackUrl = 'https://callback.url/endpoint';
+    expect(matchesCallbackUrl(null, new URL(callBackUrl))).toBe(false);
+  });
 });