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); + }); });