fix(ui): add state parameter in the pkce flow (#17235)

* Add state to pkce flow Signed-off-by: Jungho Son <js3692@users.noreply.github.com> * Call unset Signed-off-by: Jungho Son <js3692@users.noreply.github.com> --------- Signed-off-by: Jungho Son <js3692@users.noreply.github.com>
argoproj · Sep 22, 2024 · 555854c · 555854c
1 parent 0573ed7
commit 555854c
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 7 deletions.
diff --git a/ui/src/app/login/components/pkce-verify.tsx b/ui/src/app/login/components/pkce-verify.tsx
@@ -1,7 +1,7 @@
 import React, {useEffect, useState} from 'react';
 import {RouteComponentProps} from 'react-router';
 import {services} from '../../shared/services';
-import {PKCECodeVerifier, PKCELoginError, getPKCERedirectURI, pkceCallback} from './utils';
+import {PKCECodeVerifier, PKCEState, PKCELoginError, getPKCERedirectURI, pkceCallback} from './utils';
 
 import './pkce-verify.scss';
 
@@ -18,6 +18,7 @@ export const PKCEVerification = (props: RouteComponentProps<any>) => {
             .finally(() => {
                 setLoading(false);
                 PKCECodeVerifier.unset();
+                PKCEState.unset();
             });
     }, [props.location]);
 

diff --git a/ui/src/app/login/components/utils.ts b/ui/src/app/login/components/utils.ts
@@ -4,8 +4,8 @@ import {
     authorizationCodeGrantRequest,
     calculatePKCECodeChallenge,
     discoveryRequest,
-    expectNoState,
     generateRandomCodeVerifier,
+    generateRandomState,
     isOAuth2Error,
     parseWwwAuthenticateChallenges,
     processAuthorizationCodeOpenIDResponse,
@@ -22,6 +22,12 @@ export const PKCECodeVerifier = {
     unset: () => sessionStorage.removeItem(window.btoa('code_verifier'))
 };
 
+export const PKCEState = {
+    get: () => sessionStorage.getItem(window.btoa('pkce_session_id')),
+    set: (sessionId: string) => sessionStorage.setItem(window.btoa('pkce_session_id'), sessionId),
+    unset: () => sessionStorage.removeItem(window.btoa('pkce_session_id'))
+};
+
 export const getPKCERedirectURI = () => {
     const currentOrigin = new URL(window.location.origin);
 
@@ -74,6 +80,8 @@ export const pkceLogin = async (oidcConfig: AuthSettings['oidcConfig'], redirect
         throw new PKCELoginError('No Authorization Server endpoint found');
     }
 
+    const state = generateRandomState();
+
     const codeVerifier = generateRandomCodeVerifier();
 
     const codeChallange = await calculatePKCECodeChallenge(codeVerifier);
@@ -86,8 +94,10 @@ export const pkceLogin = async (oidcConfig: AuthSettings['oidcConfig'], redirect
     authorizationServerConsentScreen.searchParams.set('redirect_uri', redirectURI);
     authorizationServerConsentScreen.searchParams.set('response_type', 'code');
     authorizationServerConsentScreen.searchParams.set('scope', oidcConfig.scopes.join(' '));
+    authorizationServerConsentScreen.searchParams.set('state', state);
 
     PKCECodeVerifier.set(codeVerifier);
+    PKCEState.set(state);
 
     window.location.replace(authorizationServerConsentScreen.toString());
 };
@@ -110,18 +120,16 @@ export const pkceCallback = async (queryParams: string, oidcConfig: AuthSettings
         throw new PKCELoginError('No code in query parameters');
     }
 
-    if (callbackQueryParams.get('state') === '') {
-        callbackQueryParams.delete('state');
-    }
-
     const {authorizationServer} = await validateAndGetOIDCForPKCE(oidcConfig);
 
     const client: Client = {
         client_id: oidcConfig.clientID,
         token_endpoint_auth_method: 'none'
     };
 
-    const params = validateAuthResponse(authorizationServer, client, callbackQueryParams, expectNoState);
+    const expectedState = PKCEState.get();
+
+    const params = validateAuthResponse(authorizationServer, client, callbackQueryParams, expectedState);
 
     if (isOAuth2Error(params)) {
         throw new PKCELoginError('Error validating auth response');