Skip to content

Commit

Permalink
feat: PKCE authentication flow for web logins #9890 (#15889)
Browse files Browse the repository at this point in the history
feat: PKCE authentication flow for web logins #9890 (#15889)

Signed-off-by: Mayursinh Sarvaiya <marvinduff97@gmail.com>
  • Loading branch information
Marvin9 authored Oct 30, 2023
1 parent 8241869 commit d747eb3
Show file tree
Hide file tree
Showing 16 changed files with 413 additions and 117 deletions.
3 changes: 3 additions & 0 deletions assets/swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -4462,6 +4462,9 @@
"clientID": {
"type": "string"
},
"enablePKCEAuthentication": {
"type": "boolean"
},
"idTokenClaims": {
"type": "object",
"additionalProperties": {
Expand Down
6 changes: 6 additions & 0 deletions docs/operator-manual/user-management/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,12 @@ data:
# for the 'localhost' (CLI) client to Dex. This field is optional. If omitted, the CLI will
# use the same clientID as the Argo CD server
cliClientID: vvvvwwwwxxxxyyyyzzzz
# PKCE authentication flow processes authorization flow from browser only - default false
# uses the clientID
# make sure the Identity Provider (IdP) is public and doesn't need clientSecret
# make sure the Identity Provider (IdP) has this redirect URI registered: https://argocd.example.com/pkce/verify
enablePKCEAuthentication: true
```

!!! note
Expand Down
212 changes: 127 additions & 85 deletions pkg/apiclient/settings/settings.pb.go

Large diffs are not rendered by default.

11 changes: 6 additions & 5 deletions server/settings/settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,11 +131,12 @@ func (s *Server) Get(ctx context.Context, q *settingspkg.SettingsQuery) (*settin
}
if oidcConfig := argoCDSettings.OIDCConfig(); oidcConfig != nil {
set.OIDCConfig = &settingspkg.OIDCConfig{
Name: oidcConfig.Name,
Issuer: oidcConfig.Issuer,
ClientID: oidcConfig.ClientID,
CLIClientID: oidcConfig.CLIClientID,
Scopes: oidcConfig.RequestedScopes,
Name: oidcConfig.Name,
Issuer: oidcConfig.Issuer,
ClientID: oidcConfig.ClientID,
CLIClientID: oidcConfig.CLIClientID,
Scopes: oidcConfig.RequestedScopes,
EnablePKCEAuthentication: oidcConfig.EnablePKCEAuthentication,
}
if len(argoCDSettings.OIDCConfig().RequestedIDTokenClaims) > 0 {
set.OIDCConfig.IDTokenClaims = argoCDSettings.OIDCConfig().RequestedIDTokenClaims
Expand Down
1 change: 1 addition & 0 deletions server/settings/settings.proto
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ message OIDCConfig {
string cliClientID = 4 [(gogoproto.customname) = "CLIClientID"];
repeated string scopes = 5;
map<string, git.luolix.top.argoproj.argo_cd.server.settings.oidc.Claim> idTokenClaims = 6 [(gogoproto.customname) = "IDTokenClaims"];
bool enablePKCEAuthentication = 7;
}

// SettingsService
Expand Down
1 change: 1 addition & 0 deletions ui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
"minimatch": "^3.1.2",
"moment": "^2.29.4",
"monaco-editor": "^0.33.0",
"oauth4webapi": "^2.3.0",
"path": "^0.12.7",
"prop-types": "^15.8.1",
"react": "^16.9.3",
Expand Down
4 changes: 3 additions & 1 deletion ui/src/app/app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {hashCode} from './shared/utils';
import {Banner} from './ui-banner/ui-banner';
import userInfo from './user-info';
import {AuthSettings} from './shared/models';
import {PKCEVerification} from './login/components/pkce-verify';

services.viewPreferences.init();
const bases = document.getElementsByTagName('base');
Expand All @@ -32,7 +33,8 @@ const routes: Routes = {
'/applications': {component: applications.component},
'/settings': {component: settings.component},
'/user-info': {component: userInfo.component},
'/help': {component: help.component}
'/help': {component: help.component},
'/pkce/verify': {component: PKCEVerification, noLayout: true}
};

interface NavItem {
Expand Down
17 changes: 15 additions & 2 deletions ui/src/app/login/components/login.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {FormField} from 'argo-ui';
import {FormField, NotificationType} from 'argo-ui';
import * as PropTypes from 'prop-types';
import * as React from 'react';
import {Form, Text} from 'react-form';
Expand All @@ -7,6 +7,7 @@ import {RouteComponentProps} from 'react-router';
import {AppContext} from '../../shared/context';
import {AuthSettings} from '../../shared/models';
import {services} from '../../shared/services';
import {getPKCERedirectURI, pkceLogin} from './utils';

require('./login.scss');

Expand Down Expand Up @@ -61,7 +62,19 @@ export class Login extends React.Component<RouteComponentProps<{}>, State> {
</div>
{ssoConfigured && (
<div className='login__box_saml width-control'>
<a href={`auth/login?return_url=${encodeURIComponent(this.state.returnUrl)}`}>
<a
{...(authSettings?.oidcConfig?.enablePKCEAuthentication
? {
onClick: async () => {
pkceLogin(authSettings.oidcConfig, getPKCERedirectURI().toString()).catch(err => {
this.appContext.apis.notifications.show({
type: NotificationType.Error,
content: err?.message || JSON.stringify(err)
});
});
}
}
: {href: `auth/login?return_url=${encodeURIComponent(this.state.returnUrl)}`})}>
<button className='argo-button argo-button--base argo-button--full-width argo-button--xlg'>
{(authSettings.oidcConfig && <span>Log in via {authSettings.oidcConfig.name}</span>) ||
(authSettings.dexConfig.connectors.length === 1 && <span>Log in via {authSettings.dexConfig.connectors[0].name}</span>) || (
Expand Down
8 changes: 8 additions & 0 deletions ui/src/app/login/components/pkce-verify.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
.pkce-verify {
&__container {
display: flex;
align-items: center;
justify-content: center;
height: 100vh;
}
}
45 changes: 45 additions & 0 deletions ui/src/app/login/components/pkce-verify.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import React, {useEffect, useState} from 'react';
import {RouteComponentProps} from 'react-router';
import {services} from '../../shared/services';
import {PKCECodeVerifier, PKCELoginError, getPKCERedirectURI, pkceCallback} from './utils';

import './pkce-verify.scss';

export const PKCEVerification = (props: RouteComponentProps<any>) => {
const [loading, setLoading] = useState(true);
const [error, setError] = useState<PKCELoginError | Error>();

useEffect(() => {
setLoading(true);
services.authService
.settings()
.then(authSettings => pkceCallback(props.location.search, authSettings.oidcConfig, getPKCERedirectURI().toString()))
.catch(err => setError(err))
.finally(() => {
setLoading(false);
PKCECodeVerifier.unset();
});
}, [props.location]);

if (loading) {
return <div className='pkce-verify__container'>Processing...</div>;
}

if (error) {
return (
<div className='pkce-verify__container'>
<div>
<h3>Error occurred: </h3>
<p>{error?.message || JSON.stringify(error)}</p>
<a href='/login'>Try to Login again</a>
</div>
</div>
);
}

return (
<div className='pkce-verify__container'>
success. if you are not being redirected automatically please &nbsp;<a href='/applications'>click here</a>
</div>
);
};
155 changes: 155 additions & 0 deletions ui/src/app/login/components/utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
import {
AuthorizationServer,
Client,
authorizationCodeGrantRequest,
calculatePKCECodeChallenge,
discoveryRequest,
expectNoState,
generateRandomCodeVerifier,
isOAuth2Error,
parseWwwAuthenticateChallenges,
processAuthorizationCodeOpenIDResponse,
processDiscoveryResponse,
validateAuthResponse
} from 'oauth4webapi';
import {AuthSettings} from '../../shared/models';

export const discoverAuthServer = (issuerURL: URL): Promise<AuthorizationServer> => discoveryRequest(issuerURL).then(res => processDiscoveryResponse(issuerURL, res));

export const PKCECodeVerifier = {
get: () => sessionStorage.getItem(window.btoa('code_verifier')),
set: (codeVerifier: string) => sessionStorage.setItem(window.btoa('code_verifier'), codeVerifier),
unset: () => sessionStorage.removeItem(window.btoa('code_verifier'))
};

export const getPKCERedirectURI = () => {
const currentOrigin = new URL(window.location.origin);

currentOrigin.pathname = '/pkce/verify';

return currentOrigin;
};

export class PKCELoginError extends Error {
constructor(message: string) {
super(message);
this.name = 'PKCELoginError';
}
}

const validateAndGetOIDCForPKCE = async (oidcConfig: AuthSettings['oidcConfig']) => {
if (!oidcConfig) {
throw new PKCELoginError('No OIDC Config found');
}

let issuerURL: URL;
try {
issuerURL = new URL(oidcConfig.issuer);
} catch (e) {
throw new PKCELoginError(`Invalid oidc issuer ${oidcConfig.issuer}`);
}

if (!oidcConfig.clientID) {
throw new PKCELoginError('No OIDC Client Id found');
}

let authorizationServer: AuthorizationServer;
try {
authorizationServer = await discoverAuthServer(issuerURL);
} catch (e) {
throw new PKCELoginError(e);
}

return {
issuerURL,
authorizationServer,
clientID: oidcConfig.clientID
};
};

export const pkceLogin = async (oidcConfig: AuthSettings['oidcConfig'], redirectURI: string) => {
const {authorizationServer} = await validateAndGetOIDCForPKCE(oidcConfig);

if (!authorizationServer.authorization_endpoint) {
throw new PKCELoginError('No Authorization Server endpoint found');
}

if (!authorizationServer?.code_challenge_methods_supported?.includes('S256')) {
throw new PKCELoginError('Authorization Server does not support S256 code challenge method');
}

const codeVerifier = generateRandomCodeVerifier();

const codeChallange = await calculatePKCECodeChallenge(codeVerifier);

const authorizationServerConsentScreen = new URL(authorizationServer.authorization_endpoint);

authorizationServerConsentScreen.searchParams.set('client_id', oidcConfig.clientID);
authorizationServerConsentScreen.searchParams.set('code_challenge', codeChallange);
authorizationServerConsentScreen.searchParams.set('code_challenge_method', 'S256');
authorizationServerConsentScreen.searchParams.set('redirect_uri', redirectURI);
authorizationServerConsentScreen.searchParams.set('response_type', 'code');
authorizationServerConsentScreen.searchParams.set('scope', oidcConfig.scopes.join(' '));

PKCECodeVerifier.set(codeVerifier);

window.location.replace(authorizationServerConsentScreen.toString());
};

export const pkceCallback = async (queryParams: string, oidcConfig: AuthSettings['oidcConfig'], redirectURI: string) => {
const codeVerifier = PKCECodeVerifier.get();

if (!codeVerifier) {
throw new PKCELoginError('No code verifier found in session');
}

let callbackQueryParams = new URLSearchParams();
try {
callbackQueryParams = new URLSearchParams(queryParams);
} catch (e) {
throw new PKCELoginError('Invalid query parameters');
}

if (!callbackQueryParams.get('code')) {
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);

if (isOAuth2Error(params)) {
throw new PKCELoginError('Error validating auth response');
}

const response = await authorizationCodeGrantRequest(authorizationServer, client, params, redirectURI, codeVerifier);

const authChallengeExtract = parseWwwAuthenticateChallenges(response);

if (authChallengeExtract?.length > 0) {
throw new PKCELoginError('Error parsing authentication challenge');
}

const result = await processAuthorizationCodeOpenIDResponse(authorizationServer, client, response);

if (isOAuth2Error(result)) {
throw new PKCELoginError(`Error getting token ${result.error_description}`);
}

if (!result.id_token) {
throw new PKCELoginError('No token in response');
}

document.cookie = `argocd.token=${result.id_token}; path=/`;

window.location.replace('/applications');
};
4 changes: 4 additions & 0 deletions ui/src/app/shared/models.ts
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,10 @@ export interface AuthSettings {
};
oidcConfig: {
name: string;
issuer: string;
clientID: string;
scopes: string[];
enablePKCEAuthentication: boolean;
};
help: {
chatUrl: string;
Expand Down
5 changes: 5 additions & 0 deletions ui/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -6682,6 +6682,11 @@ oas-validator@^5.0.8:
should "^13.2.1"
yaml "^1.10.0"

oauth4webapi@^2.3.0:
version "2.3.0"
resolved "https://registry.yarnpkg.com/oauth4webapi/-/oauth4webapi-2.3.0.tgz#d01aeb83b60dbe3ff9ef1c6ec4a39e29c7be7ff6"
integrity sha512-JGkb5doGrwzVDuHwgrR4nHJayzN4h59VCed6EW8Tql6iHDfZIabCJvg6wtbn5q6pyB2hZruI3b77Nudvq7NmvA==

object-assign@^4.0.1, object-assign@^4.1.1:
version "4.1.1"
resolved "https://registry.yarnpkg.com/object-assign/-/object-assign-4.1.1.tgz#2109adc7965887cfc05cbbd442cac8bfbb360863"
Expand Down
12 changes: 10 additions & 2 deletions util/dex/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,14 @@ func GenerateDexConfigYAML(argocdSettings *settings.ArgoCDSettings, disableTls b
redirectURL,
},
}
argoCDPKCEStaticClient := map[string]interface{}{
"id": "argo-cd-pkce",
"name": "Argo CD PKCE",
"redirectURIs": []string{
"http://localhost:4000/pkce/verify",
},
"public": true,
}
argoCDCLIStaticClient := map[string]interface{}{
"id": common.ArgoCDCLIClientAppID,
"name": common.ArgoCDCLIClientAppName,
Expand All @@ -75,9 +83,9 @@ func GenerateDexConfigYAML(argocdSettings *settings.ArgoCDSettings, disableTls b

staticClients, ok := dexCfg["staticClients"].([]interface{})
if ok {
dexCfg["staticClients"] = append([]interface{}{argoCDStaticClient, argoCDCLIStaticClient}, staticClients...)
dexCfg["staticClients"] = append([]interface{}{argoCDStaticClient, argoCDCLIStaticClient, argoCDPKCEStaticClient}, staticClients...)
} else {
dexCfg["staticClients"] = []interface{}{argoCDStaticClient, argoCDCLIStaticClient}
dexCfg["staticClients"] = []interface{}{argoCDStaticClient, argoCDCLIStaticClient, argoCDPKCEStaticClient}
}

dexRedirectURL, err := argocdSettings.DexRedirectURL()
Expand Down
8 changes: 4 additions & 4 deletions util/dex/dex_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -293,9 +293,9 @@ func Test_GenerateDexConfig(t *testing.T) {
}
clients, ok := dexCfg["staticClients"].([]interface{})
assert.True(t, ok)
assert.Equal(t, 3, len(clients))
assert.Equal(t, 4, len(clients))

customClient := clients[2].(map[string]interface{})
customClient := clients[3].(map[string]interface{})
assert.Equal(t, "argo-workflow", customClient["id"].(string))
assert.Equal(t, 1, len(customClient["redirectURIs"].([]interface{})))
})
Expand All @@ -315,9 +315,9 @@ func Test_GenerateDexConfig(t *testing.T) {
}
clients, ok := dexCfg["staticClients"].([]interface{})
assert.True(t, ok)
assert.Equal(t, 3, len(clients))
assert.Equal(t, 4, len(clients))

customClient := clients[2].(map[string]interface{})
customClient := clients[3].(map[string]interface{})
assert.Equal(t, "barfoo", customClient["secret"])
})
t.Run("Override dex oauth2 configuration", func(t *testing.T) {
Expand Down
Loading

0 comments on commit d747eb3

Please sign in to comment.