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

fix(pkce): 18045 PKCE respect base href and Reauth to PKCE on token expire #20110

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
79 changes: 55 additions & 24 deletions ui/src/app/app.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import {DataLoader, NavigationManager, Notifications, NotificationsManager, PageContext, Popup, PopupManager, PopupProps} from 'argo-ui';
import {DataLoader, NavigationManager, NotificationType, Notifications, NotificationsManager, PageContext, Popup, PopupManager, PopupProps} from 'argo-ui';
import {createBrowserHistory} from 'history';
import * as PropTypes from 'prop-types';
import * as React from 'react';
import {Helmet} from 'react-helmet';
import {Redirect, Route, RouteComponentProps, Router, Switch} from 'react-router';
import {Subscription} from 'rxjs';
import applications from './applications';
import help from './help';
import login from './login';
Expand All @@ -19,6 +20,7 @@ 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();
Expand Down Expand Up @@ -86,28 +88,6 @@ async function isExpiredSSO() {
return false;
}

requests.onError.subscribe(async err => {
if (err.status === 401) {
if (history.location.pathname.startsWith('/login')) {
return;
}

const isSSO = await isExpiredSSO();
// location might change after async method call, so we need to check again.
if (history.location.pathname.startsWith('/login')) {
return;
}
// Query for basehref and remove trailing /.
// If basehref is the default `/` it will become an empty string.
const basehref = document.querySelector('head > base').getAttribute('href').replace(/\/$/, '');
if (isSSO) {
window.location.href = `${basehref}/auth/login?return_url=${encodeURIComponent(location.href)}`;
} else {
history.push(`/login?return_url=${encodeURIComponent(location.href)}`);
}
}
});

export class App extends React.Component<
{},
{popupProps: PopupProps; showVersionPanel: boolean; error: Error; navItems: NavItem[]; routes: Routes; extensionsLoaded: boolean; authSettings: AuthSettings}
Expand All @@ -126,6 +106,8 @@ export class App extends React.Component<
private navigationManager: NavigationManager;
private navItems: NavItem[];
private routes: Routes;
private popupPropsSubscription: Subscription;
private unauthorizedSubscription: Subscription;

constructor(props: {}) {
super(props);
Expand All @@ -135,11 +117,16 @@ export class App extends React.Component<
this.navigationManager = new NavigationManager(history);
this.navItems = navItems;
this.routes = routes;
this.popupPropsSubscription = null;
this.unauthorizedSubscription = null;
services.extensions.addEventListener('systemLevel', this.onAddSystemLevelExtension.bind(this));
}

public async componentDidMount() {
this.popupManager.popupProps.subscribe(popupProps => this.setState({popupProps}));
this.popupPropsSubscription = this.popupManager.popupProps.subscribe(popupProps => this.setState({popupProps}));
this.subscribeUnauthorized().then(subscription => {
this.unauthorizedSubscription = subscription;
});
const authSettings = await services.authService.settings();
const {trackingID, anonymizeUsers} = authSettings.googleAnalytics || {trackingID: '', anonymizeUsers: true};
const {loggedIn, username} = await services.users.get();
Expand Down Expand Up @@ -167,6 +154,15 @@ export class App extends React.Component<
this.setState({...this.state, navItems: this.navItems, routes: this.routes, extensionsLoaded: false, authSettings});
}

public componentWillUnmount() {
if (this.popupPropsSubscription) {
this.popupPropsSubscription.unsubscribe();
}
if (this.unauthorizedSubscription) {
this.unauthorizedSubscription.unsubscribe();
}
}

public render() {
if (this.state.error != null) {
const stack = this.state.error.stack;
Expand Down Expand Up @@ -242,6 +238,41 @@ export class App extends React.Component<
return {history, apis: {popup: this.popupManager, notifications: this.notificationsManager, navigation: this.navigationManager}};
}

private async subscribeUnauthorized() {
return requests.onError.subscribe(async err => {
if (err.status === 401) {
if (history.location.pathname.startsWith('/login')) {
return;
}

const isSSO = await isExpiredSSO();
// location might change after async method call, so we need to check again.
if (history.location.pathname.startsWith('/login')) {
return;
}
// Query for basehref and remove trailing /.
// 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)}`;
}
} else {
history.push(`/login?return_url=${encodeURIComponent(location.href)}`);
}
}
});
}

private onAddSystemLevelExtension(extension: SystemLevelExtension) {
const extendedNavItems = this.navItems;
const extendedRoutes = this.routes;
Expand Down
3 changes: 2 additions & 1 deletion ui/src/app/login/components/pkce-verify.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ 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';

Expand Down Expand Up @@ -32,7 +33,7 @@ export const PKCEVerification = (props: RouteComponentProps<any>) => {
<div>
<h3>Error occurred: </h3>
<p>{error?.message || JSON.stringify(error)}</p>
<a href='/login'>Try to Login again</a>
<a href={requests.toAbsURL('/login')}>Try to Login again</a>
</div>
</div>
);
Expand Down
11 changes: 8 additions & 3 deletions ui/src/app/login/components/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
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));

Expand All @@ -31,7 +32,7 @@ export const PKCEState = {
export const getPKCERedirectURI = () => {
const currentOrigin = new URL(window.location.origin);

currentOrigin.pathname = '/pkce/verify';
currentOrigin.pathname = requests.toAbsURL('/pkce/verify');

return currentOrigin;
};
Expand Down Expand Up @@ -153,7 +154,11 @@ export const pkceCallback = async (queryParams: string, oidcConfig: AuthSettings
throw new PKCELoginError('No token in response');
}

document.cookie = `argocd.token=${result.id_token}; path=/`;
// 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, '')}`;
austin5219 marked this conversation as resolved.
Show resolved Hide resolved

window.location.replace('/applications');
window.location.replace(requests.toAbsURL('/applications'));
};