mirror of
https://github.com/argoproj/argo-cd.git
synced 2026-02-20 01:28:45 +01:00
chore: move OIDC PKCE support from UI to backend (#21729)
Signed-off-by: Yann Soubeyrand <8511577+yann-soubeyrand@users.noreply.github.com> Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com> Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Co-authored-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
This commit is contained in:
Yann Soubeyrand
Binary file not shown.
|
Before Width: | Height: | Size: 101 KiB After Width: | Height: | Size: 157 KiB |
@@ -19,3 +19,21 @@ However, since Argo CD now sends unmodified kubeVersions to Helm, the generated
|
||||
The `--staticassets` directory in the API server (`/app/shared` by default) is now protected against out-of-bounds
|
||||
symlinks. This is to help protect against symlink attacks. If you have any symlinks in your `--staticassets` directory
|
||||
to a location outside the directory, they will return a 500 error starting with 3.1.
|
||||
|
||||
## OpenID Connect authorization code flow with PKCE is now handled by the server instead of the UI
|
||||
|
||||
Previously, when PKCE was enabled, the authorization code flow (the process which happens when you log in to Argo CD using OpenID Connect) was handled by the UI, whereas this flow was handled by the server if PKCE was not enabled. The server now always handles this flow, PKCE being enabled or not.
|
||||
|
||||
### Detection
|
||||
|
||||
To check whether PKCE is used or not, run the following command:
|
||||
|
||||
```shell
|
||||
kubectl get cm argocd-cm -o=jsonpath="{.data.oidc\.config}" | grep enablePKCEAuthentication
|
||||
```
|
||||
|
||||
If it returns `"enablePKCEAuthentication": true`, then PKCE is used.
|
||||
|
||||
### Remediation
|
||||
|
||||
On your identity provider, ensure that the OIDC client used for Argo CD has the `/auth/callback` endpoint of your Argo CD URL (e.g. https://argocd.example.com/auth/callback) in the redirect URIs.
|
||||
|
||||
@@ -347,10 +347,9 @@ data:
|
||||
# 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
|
||||
# PKCE is an OIDC extension to prevent authorization code interception attacks.
|
||||
# Make sure the identity provider supports it and that it is activated for Argo CD OIDC client.
|
||||
# Default is false.
|
||||
enablePKCEAuthentication: true
|
||||
```
|
||||
|
||||
|
||||
@@ -107,7 +107,6 @@ Also you can set __Home URL__ to _/applications_ path and __Valid Post logout re
|
||||
The Valid Redirect URIs should be set to:
|
||||
- http://localhost:8085/auth/callback (needed for argo-cd cli, depends on value from [--sso-port](../../user-guide/commands/argocd_login.md))
|
||||
- https://{hostname}/auth/callback
|
||||
- https://{hostname}/pkce/verify
|
||||
|
||||
|
||||
|
||||
|
||||
@@ -622,7 +622,7 @@ func getTestServer(t *testing.T, anonymousEnabled bool, withFakeSSO bool, useDex
|
||||
})
|
||||
oidcServer := ts
|
||||
if !useDexForSSO {
|
||||
oidcServer = testutil.GetOIDCTestServer(t)
|
||||
oidcServer = testutil.GetOIDCTestServer(t, nil)
|
||||
}
|
||||
if withFakeSSO {
|
||||
cm.Data["url"] = ts.URL
|
||||
|
||||
@@ -31,7 +31,6 @@
|
||||
"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",
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import {DataLoader, NavigationManager, NotificationType, Notifications, NotificationsManager, PageContext, Popup, PopupManager, PopupProps} from 'argo-ui';
|
||||
import {DataLoader, NavigationManager, Notifications, NotificationsManager, PageContext, Popup, PopupManager, PopupProps} from 'argo-ui';
|
||||
import {createBrowserHistory} from 'history';
|
||||
import * as PropTypes from 'prop-types';
|
||||
import * as React from 'react';
|
||||
@@ -19,8 +19,6 @@ 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';
|
||||
import {getPKCERedirectURI, pkceLogin} from './login/components/utils';
|
||||
import {SystemLevelExtension} from './shared/services/extensions-service';
|
||||
|
||||
services.viewPreferences.init();
|
||||
@@ -36,8 +34,7 @@ const routes: Routes = {
|
||||
'/applications': {component: applications.component},
|
||||
'/settings': {component: settings.component},
|
||||
'/user-info': {component: userInfo.component},
|
||||
'/help': {component: help.component},
|
||||
'/pkce/verify': {component: PKCEVerification, noLayout: true}
|
||||
'/help': {component: help.component}
|
||||
};
|
||||
|
||||
interface NavItem {
|
||||
@@ -250,18 +247,7 @@ export class App extends React.Component<{}, {popupProps: PopupProps; showVersio
|
||||
// If basehref is the default `/` it will become an empty string.
|
||||
const basehref = document.querySelector('head > base').getAttribute('href').replace(/\/$/, '');
|
||||
if (isSSO) {
|
||||
const authSettings = await services.authService.settings();
|
||||
|
||||
if (authSettings?.oidcConfig?.enablePKCEAuthentication) {
|
||||
pkceLogin(authSettings.oidcConfig, getPKCERedirectURI().toString()).catch(err => {
|
||||
this.getChildContext().apis.notifications.show({
|
||||
type: NotificationType.Error,
|
||||
content: err?.message || JSON.stringify(err)
|
||||
});
|
||||
});
|
||||
} else {
|
||||
window.location.href = `${basehref}/auth/login?return_url=${encodeURIComponent(location.href)}`;
|
||||
}
|
||||
window.location.href = `${basehref}/auth/login?return_url=${encodeURIComponent(location.href)}`;
|
||||
} else {
|
||||
history.push(`/login?return_url=${encodeURIComponent(location.href)}`);
|
||||
}
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import {FormField, NotificationType} from 'argo-ui';
|
||||
import {FormField} from 'argo-ui';
|
||||
import * as PropTypes from 'prop-types';
|
||||
import * as React from 'react';
|
||||
import {Form, Text} from 'react-form';
|
||||
@@ -7,7 +7,6 @@ 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');
|
||||
|
||||
@@ -62,18 +61,7 @@ export class Login extends React.Component<RouteComponentProps<{}>, State> {
|
||||
</div>
|
||||
{ssoConfigured && (
|
||||
<div className='login__box_saml width-control'>
|
||||
<a
|
||||
{...(authSettings?.oidcConfig?.enablePKCEAuthentication
|
||||
? {
|
||||
onClick: () =>
|
||||
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)}`})}>
|
||||
<a 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>) || (
|
||||
|
||||
@@ -1,8 +0,0 @@
|
||||
.pkce-verify {
|
||||
&__container {
|
||||
display: flex;
|
||||
align-items: center;
|
||||
justify-content: center;
|
||||
height: 100vh;
|
||||
}
|
||||
}
|
||||
@@ -1,47 +0,0 @@
|
||||
import React, {useEffect, useState} from 'react';
|
||||
import {RouteComponentProps} from 'react-router';
|
||||
import {services} from '../../shared/services';
|
||||
import {PKCECodeVerifier, PKCEState, PKCELoginError, getPKCERedirectURI, pkceCallback} from './utils';
|
||||
import requests from '../../shared/services/requests';
|
||||
|
||||
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();
|
||||
PKCEState.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={requests.toAbsURL('/login')}>Try to Login again</a>
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
return (
|
||||
<div className='pkce-verify__container'>
|
||||
success. if you are not being redirected automatically please <a href='/applications'>click here</a>
|
||||
</div>
|
||||
);
|
||||
};
|
||||
@@ -1,177 +0,0 @@
|
||||
import {
|
||||
AuthorizationServer,
|
||||
Client,
|
||||
authorizationCodeGrantRequest,
|
||||
calculatePKCECodeChallenge,
|
||||
discoveryRequest,
|
||||
generateRandomCodeVerifier,
|
||||
generateRandomState,
|
||||
isOAuth2Error,
|
||||
parseWwwAuthenticateChallenges,
|
||||
processAuthorizationCodeOpenIDResponse,
|
||||
processDiscoveryResponse,
|
||||
validateAuthResponse
|
||||
} from 'oauth4webapi';
|
||||
import {AuthSettings} from '../../shared/models';
|
||||
import requests from '../../shared/services/requests';
|
||||
|
||||
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 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);
|
||||
|
||||
currentOrigin.pathname = requests.toAbsURL('/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);
|
||||
|
||||
// This sets the return path for the user after the pkce auth flow.
|
||||
// This is ignored if the return path would be the login page as it would just loop.
|
||||
if (!location.pathname.startsWith(requests.toAbsURL('/login'))) {
|
||||
sessionStorage.setItem('return_url', location.pathname + location.search);
|
||||
}
|
||||
|
||||
if (!authorizationServer.authorization_endpoint) {
|
||||
throw new PKCELoginError('No Authorization Server endpoint found');
|
||||
}
|
||||
|
||||
const state = generateRandomState();
|
||||
|
||||
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(' '));
|
||||
authorizationServerConsentScreen.searchParams.set('state', state);
|
||||
|
||||
PKCECodeVerifier.set(codeVerifier);
|
||||
PKCEState.set(state);
|
||||
|
||||
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');
|
||||
}
|
||||
|
||||
const {authorizationServer} = await validateAndGetOIDCForPKCE(oidcConfig);
|
||||
|
||||
const client: Client = {
|
||||
client_id: oidcConfig.clientID,
|
||||
token_endpoint_auth_method: 'none'
|
||||
};
|
||||
|
||||
const expectedState = PKCEState.get();
|
||||
|
||||
const params = validateAuthResponse(authorizationServer, client, callbackQueryParams, expectedState);
|
||||
|
||||
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');
|
||||
}
|
||||
|
||||
// This regex removes any leading or trailing '/' characters and the result is appended to a '/'.
|
||||
// This is because when base href if not just '/' toAbsURL() will append a trailing '/'.
|
||||
// Just removing a trailing '/' from the string would break when base href is not specified, defaulted to '/'.
|
||||
// This pattern is used to handle both cases.
|
||||
document.cookie = `argocd.token=${result.id_token}; path=/${requests.toAbsURL('').replace(/^\/|\/$/g, '')}`;
|
||||
|
||||
const returnURL = sessionStorage.getItem('return_url');
|
||||
|
||||
if (returnURL) {
|
||||
sessionStorage.removeItem('return_url');
|
||||
window.location.replace(returnURL);
|
||||
} else {
|
||||
window.location.replace(requests.toAbsURL('/applications'));
|
||||
}
|
||||
};
|
||||
@@ -551,10 +551,6 @@ export interface AuthSettings {
|
||||
};
|
||||
oidcConfig: {
|
||||
name: string;
|
||||
issuer: string;
|
||||
clientID: string;
|
||||
scopes: string[];
|
||||
enablePKCEAuthentication: boolean;
|
||||
};
|
||||
help: {
|
||||
chatUrl: string;
|
||||
|
||||
@@ -6897,11 +6897,6 @@ 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"
|
||||
|
||||
@@ -87,7 +87,7 @@ func GenerateDexConfigYAML(argocdSettings *settings.ArgoCDSettings, disableTLS b
|
||||
"id": "argo-cd-pkce",
|
||||
"name": "Argo CD PKCE",
|
||||
"redirectURIs": []string{
|
||||
"http://localhost:4000/pkce/verify",
|
||||
"http://localhost:4000/auth/callback",
|
||||
},
|
||||
"public": true,
|
||||
}
|
||||
|
||||
@@ -62,6 +62,8 @@ type ClientApp struct {
|
||||
clientID string
|
||||
// OAuth2 client secret of this application
|
||||
clientSecret string
|
||||
// Use Proof Key for Code Exchange (PKCE)
|
||||
usePKCE bool
|
||||
// Use Azure Workload Identity for clientID auth instead of clientSecret
|
||||
useAzureWorkloadIdentity bool
|
||||
// Callback URL for OAuth2 responses (e.g. https://argocd.example.com/auth/callback)
|
||||
@@ -117,6 +119,7 @@ func NewClientApp(settings *settings.ArgoCDSettings, dexServerAddr string, dexTL
|
||||
a := ClientApp{
|
||||
clientID: settings.OAuth2ClientID(),
|
||||
clientSecret: settings.OAuth2ClientSecret(),
|
||||
usePKCE: settings.OAuth2UsePKCE(),
|
||||
useAzureWorkloadIdentity: settings.UseAzureWorkloadIdentity(),
|
||||
redirectURI: redirectURL,
|
||||
issuerURL: settings.IssuerURL(),
|
||||
@@ -183,7 +186,7 @@ func (a *ClientApp) oauth2Config(request *http.Request, scopes []string) (*oauth
|
||||
}
|
||||
|
||||
// generateAppState creates an app state nonce
|
||||
func (a *ClientApp) generateAppState(returnURL string, w http.ResponseWriter) (string, error) {
|
||||
func (a *ClientApp) generateAppState(returnURL string, pkceVerifier string, w http.ResponseWriter) (string, error) {
|
||||
// According to the spec (https://www.rfc-editor.org/rfc/rfc6749#section-10.10), this must be guessable with
|
||||
// probability <= 2^(-128). The following call generates one of 52^24 random strings, ~= 2^136 possibilities.
|
||||
randStr, err := rand.String(24)
|
||||
@@ -193,7 +196,7 @@ func (a *ClientApp) generateAppState(returnURL string, w http.ResponseWriter) (s
|
||||
if returnURL == "" {
|
||||
returnURL = a.baseHRef
|
||||
}
|
||||
cookieValue := fmt.Sprintf("%s:%s", randStr, returnURL)
|
||||
cookieValue := fmt.Sprintf("%s\n%s\n%s", randStr, returnURL, pkceVerifier)
|
||||
if encrypted, err := crypto.Encrypt([]byte(cookieValue), a.encryptionKey); err != nil {
|
||||
return "", err
|
||||
} else {
|
||||
@@ -211,23 +214,24 @@ func (a *ClientApp) generateAppState(returnURL string, w http.ResponseWriter) (s
|
||||
return randStr, nil
|
||||
}
|
||||
|
||||
func (a *ClientApp) verifyAppState(r *http.Request, w http.ResponseWriter, state string) (string, error) {
|
||||
func (a *ClientApp) verifyAppState(r *http.Request, w http.ResponseWriter, state string) (string, string, error) {
|
||||
c, err := r.Cookie(common.StateCookieName)
|
||||
if err != nil {
|
||||
return "", err
|
||||
return "", "", err
|
||||
}
|
||||
val, err := hex.DecodeString(c.Value)
|
||||
if err != nil {
|
||||
return "", err
|
||||
return "", "", err
|
||||
}
|
||||
val, err = crypto.Decrypt(val, a.encryptionKey)
|
||||
if err != nil {
|
||||
return "", err
|
||||
return "", "", err
|
||||
}
|
||||
cookieVal := string(val)
|
||||
redirectURL := a.baseHRef
|
||||
parts := strings.SplitN(cookieVal, ":", 2)
|
||||
if len(parts) == 2 && parts[1] != "" {
|
||||
pkceVerifier := ""
|
||||
parts := strings.SplitN(cookieVal, "\n", 3)
|
||||
if len(parts) > 1 && parts[1] != "" {
|
||||
if !isValidRedirectURL(parts[1],
|
||||
append([]string{a.settings.URL, a.baseHRef}, a.settings.AdditionalURLs...)) {
|
||||
sanitizedURL := parts[1]
|
||||
@@ -235,12 +239,15 @@ func (a *ClientApp) verifyAppState(r *http.Request, w http.ResponseWriter, state
|
||||
sanitizedURL = sanitizedURL[:100]
|
||||
}
|
||||
log.Warnf("Failed to verify app state - got invalid redirectURL %q", sanitizedURL)
|
||||
return "", fmt.Errorf("failed to verify app state: %w", ErrInvalidRedirectURL)
|
||||
return "", "", fmt.Errorf("failed to verify app state: %w", ErrInvalidRedirectURL)
|
||||
}
|
||||
redirectURL = parts[1]
|
||||
}
|
||||
if len(parts) > 2 {
|
||||
pkceVerifier = parts[2]
|
||||
}
|
||||
if parts[0] != state {
|
||||
return "", fmt.Errorf("invalid state in '%s' cookie", common.AuthCookieName)
|
||||
return "", "", fmt.Errorf("invalid state in '%s' cookie", common.AuthCookieName)
|
||||
}
|
||||
// set empty cookie to clear it
|
||||
http.SetCookie(w, &http.Cookie{
|
||||
@@ -250,7 +257,7 @@ func (a *ClientApp) verifyAppState(r *http.Request, w http.ResponseWriter, state
|
||||
SameSite: http.SameSiteLaxMode,
|
||||
Secure: a.secureCookie,
|
||||
})
|
||||
return redirectURL, nil
|
||||
return redirectURL, pkceVerifier, nil
|
||||
}
|
||||
|
||||
// isValidRedirectURL checks whether the given redirectURL matches on of the
|
||||
@@ -309,6 +316,7 @@ func (a *ClientApp) HandleLogin(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
scopes := make([]string, 0)
|
||||
pkceVerifier := ""
|
||||
var opts []oauth2.AuthCodeOption
|
||||
if config := a.settings.OIDCConfig(); config != nil {
|
||||
scopes = GetScopesOrDefault(config.RequestedScopes)
|
||||
@@ -328,7 +336,11 @@ func (a *ClientApp) HandleLogin(w http.ResponseWriter, r *http.Request) {
|
||||
http.Error(w, "Invalid redirect URL: the protocol and host (including port) must match and the path must be within allowed URLs if provided", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
stateNonce, err := a.generateAppState(returnURL, w)
|
||||
if a.usePKCE {
|
||||
pkceVerifier = oauth2.GenerateVerifier()
|
||||
opts = append(opts, oauth2.S256ChallengeOption(pkceVerifier))
|
||||
}
|
||||
stateNonce, err := a.generateAppState(returnURL, pkceVerifier, w)
|
||||
if err != nil {
|
||||
log.Errorf("Failed to initiate login flow: %v", err)
|
||||
http.Error(w, "Failed to initiate login flow", http.StatusInternalServerError)
|
||||
@@ -412,7 +424,7 @@ func (a *ClientApp) HandleCallback(w http.ResponseWriter, r *http.Request) {
|
||||
a.handleImplicitFlow(r, w, state)
|
||||
return
|
||||
}
|
||||
returnURL, err := a.verifyAppState(r, w, state)
|
||||
returnURL, pkceVerifier, err := a.verifyAppState(r, w, state)
|
||||
if err != nil {
|
||||
http.Error(w, err.Error(), http.StatusBadRequest)
|
||||
return
|
||||
@@ -434,6 +446,10 @@ func (a *ClientApp) HandleCallback(w http.ResponseWriter, r *http.Request) {
|
||||
}
|
||||
}
|
||||
|
||||
if a.usePKCE {
|
||||
options = append(options, oauth2.VerifierOption(pkceVerifier))
|
||||
}
|
||||
|
||||
token, err := oauth2Config.Exchange(ctx, code, options...)
|
||||
if err != nil {
|
||||
http.Error(w, fmt.Sprintf("failed to get token: %v", err), http.StatusInternalServerError)
|
||||
@@ -544,7 +560,8 @@ func (a *ClientApp) handleImplicitFlow(r *http.Request, w http.ResponseWriter, s
|
||||
CookieName: common.AuthCookieName,
|
||||
}
|
||||
if state != "" {
|
||||
returnURL, err := a.verifyAppState(r, w, state)
|
||||
// Not using pkceVerifier, since PKCE is not supported in implicit flow.
|
||||
returnURL, _, err := a.verifyAppState(r, w, state)
|
||||
if err != nil {
|
||||
http.Error(w, err.Error(), http.StatusBadRequest)
|
||||
return
|
||||
|
||||
@@ -127,7 +127,7 @@ func TestHandleCallback(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestClientApp_HandleLogin(t *testing.T) {
|
||||
oidcTestServer := test.GetOIDCTestServer(t)
|
||||
oidcTestServer := test.GetOIDCTestServer(t, nil)
|
||||
t.Cleanup(oidcTestServer.Close)
|
||||
|
||||
dexTestServer := test.GetDexTestServer(t)
|
||||
@@ -408,7 +408,7 @@ func Test_Login_Flow(t *testing.T) {
|
||||
// Show that SSO login works when no redirect URL is provided, and we fall back to the configured base href for the
|
||||
// Argo CD instance.
|
||||
|
||||
oidcTestServer := test.GetOIDCTestServer(t)
|
||||
oidcTestServer := test.GetOIDCTestServer(t, nil)
|
||||
t.Cleanup(oidcTestServer.Close)
|
||||
|
||||
cdSettings := &settings.ArgoCDSettings{
|
||||
@@ -416,8 +416,8 @@ func Test_Login_Flow(t *testing.T) {
|
||||
OIDCConfigRAW: fmt.Sprintf(`
|
||||
name: Test
|
||||
issuer: %s
|
||||
clientID: xxx
|
||||
clientSecret: yyy
|
||||
clientID: test-client-id
|
||||
clientSecret: test-client-secret
|
||||
requestedScopes: ["oidc"]`, oidcTestServer.URL),
|
||||
OIDCTLSInsecureSkipVerify: true,
|
||||
}
|
||||
@@ -435,7 +435,7 @@ requestedScopes: ["oidc"]`, oidcTestServer.URL),
|
||||
redirectURL, err := w.Result().Location()
|
||||
require.NoError(t, err)
|
||||
|
||||
state := redirectURL.Query()["state"]
|
||||
state := redirectURL.Query().Get("state")
|
||||
|
||||
req = httptest.NewRequest(http.MethodGet, fmt.Sprintf("https://argocd.example.com/auth/callback?state=%s&code=abc", state), http.NoBody)
|
||||
for _, cookie := range w.Result().Cookies() {
|
||||
@@ -446,11 +446,65 @@ requestedScopes: ["oidc"]`, oidcTestServer.URL),
|
||||
|
||||
app.HandleCallback(w, req)
|
||||
|
||||
assert.Equal(t, 303, w.Code)
|
||||
assert.NotContains(t, w.Body.String(), ErrInvalidRedirectURL.Error())
|
||||
}
|
||||
|
||||
func Test_Login_Flow_With_PKCE(t *testing.T) {
|
||||
var codeChallenge string
|
||||
|
||||
oidcTestServer := test.GetOIDCTestServer(t, func(r *http.Request) {
|
||||
codeVerifier := r.FormValue("code_verifier")
|
||||
assert.NotEmpty(t, codeVerifier)
|
||||
assert.Equal(t, oauth2.S256ChallengeFromVerifier(codeVerifier), codeChallenge)
|
||||
})
|
||||
t.Cleanup(oidcTestServer.Close)
|
||||
|
||||
cdSettings := &settings.ArgoCDSettings{
|
||||
URL: "https://example.com/argocd",
|
||||
OIDCConfigRAW: fmt.Sprintf(`
|
||||
name: Test
|
||||
issuer: %s
|
||||
clientID: test-client-id
|
||||
clientSecret: test-client-secret
|
||||
requestedScopes: ["oidc"]
|
||||
enablePKCEAuthentication: true`, oidcTestServer.URL),
|
||||
OIDCTLSInsecureSkipVerify: true,
|
||||
}
|
||||
app, err := NewClientApp(cdSettings, "", nil, "/", cache.NewInMemoryCache(24*time.Hour))
|
||||
require.NoError(t, err)
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "https://example.com/argocd/auth/login", http.NoBody)
|
||||
|
||||
app.HandleLogin(w, req)
|
||||
|
||||
redirectURL, err := w.Result().Location()
|
||||
require.NoError(t, err)
|
||||
|
||||
codeChallenge = redirectURL.Query().Get("code_challenge")
|
||||
|
||||
assert.NotEmpty(t, codeChallenge)
|
||||
assert.Equal(t, "S256", redirectURL.Query().Get("code_challenge_method"))
|
||||
|
||||
state := redirectURL.Query().Get("state")
|
||||
|
||||
req = httptest.NewRequest(http.MethodGet, fmt.Sprintf("https://example.com/argocd/auth/callback?state=%s&code=abc", state), http.NoBody)
|
||||
for _, cookie := range w.Result().Cookies() {
|
||||
req.AddCookie(cookie)
|
||||
}
|
||||
|
||||
w = httptest.NewRecorder()
|
||||
|
||||
app.HandleCallback(w, req)
|
||||
|
||||
assert.Equal(t, 303, w.Code)
|
||||
assert.NotContains(t, w.Body.String(), ErrInvalidRedirectURL.Error())
|
||||
}
|
||||
|
||||
func TestClientApp_HandleCallback(t *testing.T) {
|
||||
oidcTestServer := test.GetOIDCTestServer(t)
|
||||
oidcTestServer := test.GetOIDCTestServer(t, nil)
|
||||
t.Cleanup(oidcTestServer.Close)
|
||||
|
||||
dexTestServer := test.GetDexTestServer(t)
|
||||
@@ -758,7 +812,8 @@ func TestGenerateAppState(t *testing.T) {
|
||||
app, err := NewClientApp(&settings.ArgoCDSettings{ServerSignature: signature, URL: expectedReturnURL}, "", nil, "", cache.NewInMemoryCache(24*time.Hour))
|
||||
require.NoError(t, err)
|
||||
generateResponse := httptest.NewRecorder()
|
||||
state, err := app.generateAppState(expectedReturnURL, generateResponse)
|
||||
expectedPKCEVerifier := oauth2.GenerateVerifier()
|
||||
state, err := app.generateAppState(expectedReturnURL, expectedPKCEVerifier, generateResponse)
|
||||
require.NoError(t, err)
|
||||
|
||||
t.Run("VerifyAppState_Successful", func(t *testing.T) {
|
||||
@@ -767,9 +822,10 @@ func TestGenerateAppState(t *testing.T) {
|
||||
req.AddCookie(cookie)
|
||||
}
|
||||
|
||||
returnURL, err := app.verifyAppState(req, httptest.NewRecorder(), state)
|
||||
returnURL, pkceVerifier, err := app.verifyAppState(req, httptest.NewRecorder(), state)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, expectedReturnURL, returnURL)
|
||||
assert.Equal(t, expectedPKCEVerifier, pkceVerifier)
|
||||
})
|
||||
|
||||
t.Run("VerifyAppState_Failed", func(t *testing.T) {
|
||||
@@ -778,7 +834,7 @@ func TestGenerateAppState(t *testing.T) {
|
||||
req.AddCookie(cookie)
|
||||
}
|
||||
|
||||
_, err := app.verifyAppState(req, httptest.NewRecorder(), "wrong state")
|
||||
_, _, err := app.verifyAppState(req, httptest.NewRecorder(), "wrong state")
|
||||
require.Error(t, err)
|
||||
})
|
||||
}
|
||||
@@ -803,7 +859,7 @@ func TestGenerateAppState_XSS(t *testing.T) {
|
||||
|
||||
expectedReturnURL := "javascript: alert('hi')"
|
||||
generateResponse := httptest.NewRecorder()
|
||||
state, err := app.generateAppState(expectedReturnURL, generateResponse)
|
||||
state, err := app.generateAppState(expectedReturnURL, "", generateResponse)
|
||||
require.NoError(t, err)
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "/", http.NoBody)
|
||||
@@ -811,7 +867,7 @@ func TestGenerateAppState_XSS(t *testing.T) {
|
||||
req.AddCookie(cookie)
|
||||
}
|
||||
|
||||
returnURL, err := app.verifyAppState(req, httptest.NewRecorder(), state)
|
||||
returnURL, _, err := app.verifyAppState(req, httptest.NewRecorder(), state)
|
||||
require.ErrorIs(t, err, ErrInvalidRedirectURL)
|
||||
assert.Empty(t, returnURL)
|
||||
})
|
||||
@@ -819,7 +875,7 @@ func TestGenerateAppState_XSS(t *testing.T) {
|
||||
t.Run("valid return URL succeeds", func(t *testing.T) {
|
||||
expectedReturnURL := "https://argocd.example.com/some/path"
|
||||
generateResponse := httptest.NewRecorder()
|
||||
state, err := app.generateAppState(expectedReturnURL, generateResponse)
|
||||
state, err := app.generateAppState(expectedReturnURL, "", generateResponse)
|
||||
require.NoError(t, err)
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "/", http.NoBody)
|
||||
@@ -827,7 +883,7 @@ func TestGenerateAppState_XSS(t *testing.T) {
|
||||
req.AddCookie(cookie)
|
||||
}
|
||||
|
||||
returnURL, err := app.verifyAppState(req, httptest.NewRecorder(), state)
|
||||
returnURL, _, err := app.verifyAppState(req, httptest.NewRecorder(), state)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, expectedReturnURL, returnURL)
|
||||
})
|
||||
@@ -847,7 +903,7 @@ func TestGenerateAppState_NoReturnURL(t *testing.T) {
|
||||
require.NoError(t, err)
|
||||
|
||||
req.AddCookie(&http.Cookie{Name: common.StateCookieName, Value: hex.EncodeToString(encrypted)})
|
||||
returnURL, err := app.verifyAppState(req, httptest.NewRecorder(), "123")
|
||||
returnURL, _, err := app.verifyAppState(req, httptest.NewRecorder(), "123")
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, "/argo-cd", returnURL)
|
||||
}
|
||||
|
||||
@@ -593,7 +593,7 @@ func getKubeClientWithConfig(config map[string]string, secretConfig map[string][
|
||||
}
|
||||
|
||||
func TestSessionManager_VerifyToken(t *testing.T) {
|
||||
oidcTestServer := utiltest.GetOIDCTestServer(t)
|
||||
oidcTestServer := utiltest.GetOIDCTestServer(t, nil)
|
||||
t.Cleanup(oidcTestServer.Close)
|
||||
|
||||
dexTestServer := utiltest.GetDexTestServer(t)
|
||||
|
||||
@@ -1893,6 +1893,13 @@ func (a *ArgoCDSettings) OAuth2ClientSecret() string {
|
||||
return ""
|
||||
}
|
||||
|
||||
func (a *ArgoCDSettings) OAuth2UsePKCE() bool {
|
||||
if oidcConfig := a.OIDCConfig(); oidcConfig != nil {
|
||||
return oidcConfig.EnablePKCEAuthentication
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
func (a *ArgoCDSettings) UseAzureWorkloadIdentity() bool {
|
||||
if oidcConfig := a.OIDCConfig(); oidcConfig != nil && oidcConfig.Azure != nil {
|
||||
return oidcConfig.Azure.UseWorkloadIdentity
|
||||
|
||||
@@ -215,13 +215,13 @@ func oidcMockHandler(t *testing.T, url string, tokenRequestPreHandler func(r *ht
|
||||
}
|
||||
}
|
||||
|
||||
func GetOIDCTestServer(t *testing.T) *httptest.Server {
|
||||
func GetOIDCTestServer(t *testing.T, tokenRequestPreHandler func(r *http.Request)) *httptest.Server {
|
||||
t.Helper()
|
||||
ts := httptest.NewTLSServer(http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {
|
||||
// Start with a placeholder. We need the server URL before setting up the real handler.
|
||||
}))
|
||||
ts.Config.Handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
oidcMockHandler(t, ts.URL, nil)(w, r)
|
||||
oidcMockHandler(t, ts.URL, tokenRequestPreHandler)(w, r)
|
||||
})
|
||||
return ts
|
||||
}
|
||||
@@ -258,14 +258,19 @@ func mockTokenEndpointResponse(issuer string) (TokenResponse, error) {
|
||||
|
||||
// Helper function to generate a JWT token
|
||||
func generateJWTToken(issuer string) (string, error) {
|
||||
token := jwt.NewWithClaims(jwt.SigningMethodHS256, jwt.MapClaims{
|
||||
token := jwt.NewWithClaims(jwt.SigningMethodRS512, jwt.MapClaims{
|
||||
"sub": "1234567890",
|
||||
"aud": "test-client-id",
|
||||
"name": "John Doe",
|
||||
"iat": time.Now().Unix(),
|
||||
"iss": issuer,
|
||||
"exp": time.Now().Add(time.Hour).Unix(), // Set the expiration time
|
||||
})
|
||||
tokenString, err := token.SignedString([]byte("secret"))
|
||||
key, err := jwt.ParseRSAPrivateKeyFromPEM(PrivateKey)
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("failed to parse RSA private key: %w", err)
|
||||
}
|
||||
tokenString, err := token.SignedString(key)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user