Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: PKCE authentication flow for web logins #9890 #15889

Merged
merged 8 commits into from
Oct 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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{}{
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jessesuen forgot to tell that I have added static client as well for PKCE

"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
Loading