fix(oauth2): prevent code injection in OAuth2 callback handling

packages/bruno-app/src/components/RequestPane/Auth/OAuth2/Oauth2ActionButtons/index.js

@@ -5,7 +5,10 @@ import { cloneDeep, find, get } from 'lodash';
 import { IconLoader2, IconX } from '@tabler/icons';
 import { interpolate } from '@usebruno/common';
 import { fetchOauth2Credentials, clearOauth2Cache, refreshOauth2Credentials, cancelOauth2AuthorizationRequest, isOauth2AuthorizationRequestInProgress } from 'providers/ReduxStore/slices/collections/actions';
+import { responseReceived } from 'providers/ReduxStore/slices/collections';
+import { updateResponsePaneTab } from 'providers/ReduxStore/slices/tabs';
 import { getAllVariables } from 'utils/collections/index';
+import { formatIpcError } from 'utils/common/error';
 import Button from 'ui/Button';
 
 const Oauth2ActionButtons = ({ item, request, collection, url: accessTokenUrl, credentialsId }) => {
@@ -41,6 +44,25 @@ const Oauth2ActionButtons = ({ item, request, collection, url: accessTokenUrl, c
   const credentialsData = find(collection?.oauth2Credentials, (creds) => creds?.url == interpolatedAccessTokenUrl && creds?.collectionUid == collectionUid && creds?.credentialsId == credentialsId);
   const creds = credentialsData?.credentials || {};
 
+  const showOauth2Error = (errorMessage) => {
+    dispatch(
+      responseReceived({
+        itemUid: item.uid,
+        collectionUid,
+        response: {
+          error: errorMessage,
+          status: null,
+          headers: {},
+          data: null,
+          dataBuffer: null,
+          size: 0,
+          duration: 0
+        }
+      })
+    );
+    dispatch(updateResponsePaneTab({ uid: item.uid, responsePaneTab: 'response' }));
+  };
+
   const handleFetchOauth2Credentials = async () => {
     let requestCopy = cloneDeep(request);
     requestCopy.oauth2 = requestCopy?.auth.oauth2;
@@ -59,6 +81,7 @@ const Oauth2ActionButtons = ({ item, request, collection, url: accessTokenUrl, c
         const errorMessage = result?.error || 'No access token received from authorization server';
         console.error(errorMessage);
         toast.error(errorMessage);
+        showOauth2Error(errorMessage);
         return;
       }
 
@@ -70,7 +93,9 @@ const Oauth2ActionButtons = ({ item, request, collection, url: accessTokenUrl, c
       if (error?.message && error.message.includes('cancelled by user')) {
         return;
       }
-      toast.error(error?.message || 'An error occurred while fetching token!');
+      const errorMessage = formatIpcError(error) || 'An error occurred while fetching token!';
+      toast.error(errorMessage);
+      showOauth2Error(errorMessage);
     } finally {
       toggleFetchingToken(false);
       toggleFetchingAuthorizationCode(false);
@@ -97,14 +122,17 @@ const Oauth2ActionButtons = ({ item, request, collection, url: accessTokenUrl, c
         const errorMessage = result?.error || 'No access token received from authorization server';
         console.error(errorMessage);
         toast.error(errorMessage);
+        showOauth2Error(errorMessage);
         return;
       }
 
       toast.success('Token refreshed successfully!');
     } catch (error) {
       console.error(error);
       toggleRefreshingToken(false);
-      toast.error(error?.message || 'An error occurred while refreshing token!');
+      const errorMessage = formatIpcError(error) || 'An error occurred while refreshing token!';
+      toast.error(errorMessage);
+      showOauth2Error(errorMessage);
     }
   };
 

packages/bruno-electron/src/ipc/network/authorize-user-in-system-browser.js

@@ -1,7 +1,7 @@
 const { shell } = require('electron');
 const { registerOauth2AuthorizationRequest, rejectOauth2AuthorizationRequest } = require('../../utils/oauth2-protocol-handler');
 
-const authorizeUserInSystemBrowser = ({ authorizeUrl, callbackUrl, grantType = 'authorization_code' }) => {
+const authorizeUserInSystemBrowser = ({ authorizeUrl, callbackUrl, grantType = 'authorization_code', expectedState = null }) => {
   return new Promise((resolve, reject) => {
     // Replace callback URL in authorization URL
     const authorizationUrlObj = new URL(authorizeUrl);
@@ -51,7 +51,7 @@ const authorizeUserInSystemBrowser = ({ authorizeUrl, callbackUrl, grantType = '
       reject(error);
     };
 
-    registerOauth2AuthorizationRequest(wrappedResolve, wrappedReject, debugInfo);
+    registerOauth2AuthorizationRequest(wrappedResolve, wrappedReject, debugInfo, expectedState);
 
     // Open system browser
     shell.openExternal(modifiedAuthorizeUrl).catch((error) => {

packages/bruno-electron/src/ipc/network/authorize-user-in-window.js

@@ -11,7 +11,7 @@ const matchesCallbackUrl = (url, callbackUrl) => {
     && (url.searchParams.has('code') || url.hash.length > 1);
 };
 
-const authorizeUserInWindow = ({ authorizeUrl, callbackUrl, session, additionalHeaders = {}, grantType = 'authorization_code' }) => {
+const authorizeUserInWindow = ({ authorizeUrl, callbackUrl, session, additionalHeaders = {}, grantType = 'authorization_code', expectedState = null }) => {
   return new Promise(async (resolve, reject) => {
     let finalUrl = null;
     let debugInfo = {
@@ -202,6 +202,22 @@ const authorizeUserInWindow = ({ authorizeUrl, callbackUrl, session, additionalH
 
       if (finalUrl) {
         try {
+          // Validate the state parameter to protect against CSRF / authorization
+          // code injection. The returned state must match the cryptographically
+          // random state issued when the flow was initiated.
+          if (expectedState) {
+            const finalUrlObj = new URL(finalUrl);
+            const returnedState
+              = finalUrlObj.searchParams.get('state')
+                || (finalUrlObj.hash ? new URLSearchParams(finalUrlObj.hash.substring(1)).get('state') : null);
+
+            if (returnedState !== expectedState) {
+              return reject(
+                new Error('OAuth2 state mismatch: the returned state does not match the issued state. Aborting to prevent authorization code injection.')
+              );
+            }
+          }
+
           // Handle different grant types differently
           if (grantType === 'implicit') {
             // For implicit flow, tokens are in the URL hash fragment

packages/bruno-electron/src/utils/oauth2-protocol-handler.js

@@ -1,6 +1,6 @@
 let oauth2AuthorizationRequest = null;
 
-const registerOauth2AuthorizationRequest = (resolve, reject, debugInfo = null) => {
+const registerOauth2AuthorizationRequest = (resolve, reject, debugInfo = null, expectedState = null) => {
   // Cancel any existing pending request
   if (oauth2AuthorizationRequest) {
     oauth2AuthorizationRequest.reject(new Error('Authorization cancelled: new request started'));
@@ -10,6 +10,7 @@ const registerOauth2AuthorizationRequest = (resolve, reject, debugInfo = null) =
     resolve,
     reject,
     debugInfo,
+    expectedState,
     timestamp: Date.now()
   };
 };
@@ -80,6 +81,23 @@ const handleOauth2ProtocolUrl = (url) => {
       return;
     }
 
+    // Validate the state parameter to protect against CSRF / authorization code
+    // injection. The returned state must match the cryptographically random state
+    // issued when the flow was initiated.
+    const expectedState = oauth2AuthorizationRequest?.expectedState;
+    if (expectedState) {
+      const returnedState
+        = urlObj.searchParams.get('state')
+          || (urlObj.hash ? new URLSearchParams(urlObj.hash.substring(1)).get('state') : null);
+
+      if (returnedState !== expectedState) {
+        rejectOauth2AuthorizationRequest(
+          new Error('OAuth2 state mismatch: the returned state does not match the issued state. Aborting to prevent authorization code injection.')
+        );
+        return;
+      }
+    }
+
     // Check if this is an implicit grant (tokens in hash fragment)
     if (urlObj.hash) {
       const hash = urlObj.hash.substring(1); // Remove the leading #

packages/bruno-electron/src/utils/oauth2.js

@@ -308,6 +308,10 @@ const getOAuth2AuthorizationCode = (request, codeChallenge, collectionUid) => {
     const { callbackUrl, clientId, authorizationUrl, scope, state, pkce, accessTokenUrl, additionalParameters } = oauth2;
     const useSystemBrowser = preferencesUtil.shouldUseSystemBrowser();
     const effectiveCallbackUrl = callbackUrl && callbackUrl.length ? callbackUrl : BRUNO_OAUTH2_CALLBACK_URL;
+    // Always append a cryptographically random nonce to the user-configured state
+    // (or generate a fully random one when none is set). The state is validated when
+    // the callback is received to prevent authorization code injection / CSRF.
+    const effectiveState = generateState({ userState: state });
 
     const authorizationUrlWithQueryParams = new URL(authorizationUrl);
     authorizationUrlWithQueryParams.searchParams.append('response_type', 'code');
@@ -324,8 +328,8 @@ const getOAuth2AuthorizationCode = (request, codeChallenge, collectionUid) => {
       authorizationUrlWithQueryParams.searchParams.append('code_challenge', codeChallenge);
       authorizationUrlWithQueryParams.searchParams.append('code_challenge_method', 'S256');
     }
-    if (state) {
-      authorizationUrlWithQueryParams.searchParams.append('state', state);
+    if (effectiveState) {
+      authorizationUrlWithQueryParams.searchParams.append('state', effectiveState);
     }
     if (additionalParameters?.authorization?.length) {
       additionalParameters.authorization.forEach((param) => {
@@ -344,6 +348,7 @@ const getOAuth2AuthorizationCode = (request, codeChallenge, collectionUid) => {
         authorizeUrl,
         callbackUrl: effectiveCallbackUrl,
         session: oauth2Store.getSessionIdOfCollection({ collectionUid, url: accessTokenUrl }),
+        expectedState: effectiveState,
         additionalHeaders: getAdditionalHeaders(additionalParameters?.authorization)
       });
       resolve({ authorizationCode, debugInfo });
@@ -707,6 +712,16 @@ const generateCodeVerifier = () => {
   return crypto.randomBytes(22).toString('hex');
 };
 
+// Generate a cryptographically random state value used to protect OAuth2
+// authorization flows against CSRF / authorization code injection.
+const generateState = ({ userState }) => {
+  let cryptographicallyRandomString = crypto.randomBytes(16).toString('hex');
+  if (userState && userState.length) {
+    return userState + cryptographicallyRandomString;
+  }
+  return cryptographicallyRandomString;
+};
+
 const generateCodeChallenge = (codeVerifier) => {
   const hash = crypto.createHash('sha256');
   hash.update(codeVerifier);
@@ -760,6 +775,9 @@ const getOAuth2TokenUsingImplicitGrant = async ({ request, collectionUid, forceF
   } = oauth2;
   const useSystemBrowser = preferencesUtil.shouldUseSystemBrowser();
   const effectiveCallbackUrl = callbackUrl && callbackUrl.length ? callbackUrl : BRUNO_OAUTH2_CALLBACK_URL;
+  // Use the user-configured state if present, otherwise generate a cryptographically
+  // random one. The state is validated when the callback is received to prevent CSRF.
+  const effectiveState = generateState({ userState: state });
 
   // Validate required fields
   if (!authorizationUrl) {
@@ -844,8 +862,8 @@ const getOAuth2TokenUsingImplicitGrant = async ({ request, collectionUid, forceF
   if (scope) {
     authorizationUrlWithQueryParams.searchParams.append('scope', scope);
   }
-  if (state) {
-    authorizationUrlWithQueryParams.searchParams.append('state', state);
+  if (effectiveState) {
+    authorizationUrlWithQueryParams.searchParams.append('state', effectiveState);
   }
   if (additionalParameters?.authorization?.length) {
     additionalParameters.authorization.forEach((param) => {
@@ -866,6 +884,7 @@ const getOAuth2TokenUsingImplicitGrant = async ({ request, collectionUid, forceF
       callbackUrl: effectiveCallbackUrl,
       session: oauth2Store.getSessionIdOfCollection({ collectionUid, url: authorizationUrl }),
       grantType: 'implicit',
+      expectedState: effectiveState,
       additionalHeaders: getAdditionalHeaders(additionalParameters?.authorization)
     });
 
@@ -954,5 +973,6 @@ module.exports = {
   refreshOauth2Token,
   generateCodeVerifier,
   generateCodeChallenge,
+  generateState,
   updateCollectionOauth2Credentials
 };

packages/bruno-electron/tests/utils/oauth2-protocol-handler.spec.js

@@ -0,0 +1,122 @@
+const {
+  registerOauth2AuthorizationRequest,
+  handleOauth2ProtocolUrl,
+  cancelOAuth2AuthorizationRequest,
+  isOauth2AuthorizationRequestInProgress
+} = require('../../src/utils/oauth2-protocol-handler');
+
+describe('handleOauth2ProtocolUrl - state validation', () => {
+  let resolve;
+  let reject;
+
+  beforeEach(() => {
+    resolve = jest.fn();
+    reject = jest.fn();
+  });
+
+  afterEach(() => {
+    // Clear any pending request between tests
+    if (isOauth2AuthorizationRequestInProgress()) {
+      cancelOAuth2AuthorizationRequest();
+    }
+    jest.clearAllMocks();
+  });
+
+  describe('authorization code flow (state in query params)', () => {
+    it('should resolve with the code when the returned state matches', () => {
+      registerOauth2AuthorizationRequest(resolve, reject, null, 'expected-state');
+
+      handleOauth2ProtocolUrl('bruno://oauth2/callback?code=auth-code-123&state=expected-state');
+
+      expect(resolve).toHaveBeenCalledWith('auth-code-123');
+      expect(reject).not.toHaveBeenCalled();
+    });
+
+    it('should reject when the returned state does not match', () => {
+      registerOauth2AuthorizationRequest(resolve, reject, null, 'expected-state');
+
+      handleOauth2ProtocolUrl('bruno://oauth2/callback?code=auth-code-123&state=attacker-state');
+
+      expect(resolve).not.toHaveBeenCalled();
+      expect(reject).toHaveBeenCalledWith(
+        expect.objectContaining({ message: expect.stringContaining('state mismatch') })
+      );
+    });
+
+    it('should reject when no state is returned but one was expected', () => {
+      registerOauth2AuthorizationRequest(resolve, reject, null, 'expected-state');
+
+      handleOauth2ProtocolUrl('bruno://oauth2/callback?code=auth-code-123');
+
+      expect(resolve).not.toHaveBeenCalled();
+      expect(reject).toHaveBeenCalledWith(
+        expect.objectContaining({ message: expect.stringContaining('state mismatch') })
+      );
+    });
+  });
+
+  describe('implicit flow (state in hash fragment)', () => {
+    it('should resolve with tokens when the returned state matches', () => {
+      registerOauth2AuthorizationRequest(resolve, reject, null, 'expected-state');
+
+      handleOauth2ProtocolUrl(
+        'bruno://oauth2/callback#access_token=token-abc&token_type=bearer&state=expected-state'
+      );
+
+      expect(resolve).toHaveBeenCalledWith(
+        expect.objectContaining({ access_token: 'token-abc' })
+      );
+      expect(reject).not.toHaveBeenCalled();
+    });
+
+    it('should reject when the hash state does not match', () => {
+      registerOauth2AuthorizationRequest(resolve, reject, null, 'expected-state');
+
+      handleOauth2ProtocolUrl(
+        'bruno://oauth2/callback#access_token=token-abc&token_type=bearer&state=attacker-state'
+      );
+
+      expect(resolve).not.toHaveBeenCalled();
+      expect(reject).toHaveBeenCalledWith(
+        expect.objectContaining({ message: expect.stringContaining('state mismatch') })
+      );
+    });
+
+    it('should reject when no hash state is returned but one was expected', () => {
+      registerOauth2AuthorizationRequest(resolve, reject, null, 'expected-state');
+
+      handleOauth2ProtocolUrl(
+        'bruno://oauth2/callback#access_token=token-abc&token_type=bearer'
+      );
+
+      expect(resolve).not.toHaveBeenCalled();
+      expect(reject).toHaveBeenCalledWith(
+        expect.objectContaining({ message: expect.stringContaining('state mismatch') })
+      );
+    });
+  });
+
+  describe('when no expected state was registered (backward compatibility)', () => {
+    it('should resolve without validating state', () => {
+      registerOauth2AuthorizationRequest(resolve, reject, null, null);
+
+      handleOauth2ProtocolUrl('bruno://oauth2/callback?code=auth-code-123');
+
+      expect(resolve).toHaveBeenCalledWith('auth-code-123');
+      expect(reject).not.toHaveBeenCalled();
+    });
+  });
+
+  describe('error responses are handled before state validation', () => {
+    it('should reject with the provider error even if state is absent', () => {
+      registerOauth2AuthorizationRequest(resolve, reject, null, 'expected-state');
+
+      handleOauth2ProtocolUrl('bruno://oauth2/callback?error=access_denied');
+
+      expect(resolve).not.toHaveBeenCalled();
+      expect(reject).toHaveBeenCalledWith(
+        expect.objectContaining({ message: expect.stringContaining('Authorization Failed') })
+      );
+    });
+  });
+});