diff options
author | James E. Blair <jim@acmegating.com> | 2022-09-27 10:42:08 -0700 |
---|---|---|
committer | James E. Blair <jim@acmegating.com> | 2022-10-25 20:19:40 -0700 |
commit | 25c948d2a0079892d29f5aeb5f4bd398128ea7ce (patch) | |
tree | 3e523621a0e4e1df183709e39e8fdcfe76dd29ee | |
parent | 8c47d9ce4e97ff624a93fb1caa6de1d9c0bdccba (diff) | |
download | zuul-25c948d2a0079892d29f5aeb5f4bd398128ea7ce.tar.gz |
Linger on auth_callback page until login is complete
Verifying the auth token and obtaining user metadata involves some
async HTTP requests in the background. If we remove the auth callback
information from the window location too soon, then the UserManager
will not complete the login process. This currently generally works
because this process tends to complete before the /info and /tenant/info
calls to Zuul. However, future changes to support a read-only
authentication requirement will need to alter this sequence.
This approach is more robust and easier to follow. Essentially the
sequence is:
* Return from IdP to /auth_callback
* If /auth_callback is in our location, short-circuit normal rendering
and render only the AuthCallbackPage
* When background processing is complete, onSignIn will be called and
we will set the user.redirect property in redux.
* AuthCallbackPage has an effect callback on user.redirect which will
cause it to perform the redirect away from auth_callback once
signin is complete.
This process leaves the AuthCallbackPage on the screen a little longer,
so this change updates it to use the newer EmptyPage and Spinner elements
that we have been using elsewhere, so if users see it, it appears more
intentional.
Change-Id: I206c020626c7fd73e58efc29dd50376203679721
-rw-r--r-- | web/src/App.jsx | 12 | ||||
-rw-r--r-- | web/src/ZuulAuthProvider.jsx | 3 | ||||
-rw-r--r-- | web/src/actions/user.js | 3 | ||||
-rw-r--r-- | web/src/pages/AuthCallback.jsx | 46 | ||||
-rw-r--r-- | web/src/reducers/user.js | 3 | ||||
-rw-r--r-- | web/src/routes.js | 7 |
6 files changed, 55 insertions, 19 deletions
diff --git a/web/src/App.jsx b/web/src/App.jsx index 5dd3b09d9..da7b2aa03 100644 --- a/web/src/App.jsx +++ b/web/src/App.jsx @@ -69,6 +69,8 @@ import { fetchConfigErrorsAction } from './actions/configErrors' import { routes } from './routes' import { setTenantAction } from './actions/tenant' import { configureAuthFromTenant, configureAuthFromInfo } from './actions/auth' +import { getHomepageUrl } from './api' +import AuthCallbackPage from './pages/AuthCallback' class App extends React.Component { static propTypes = { @@ -82,6 +84,7 @@ class App extends React.Component { dispatch: PropTypes.func, isKebabDropdownOpen: PropTypes.bool, user: PropTypes.object, + auth: PropTypes.object, } state = { @@ -117,6 +120,12 @@ class App extends React.Component { const { info, tenant } = this.props const allRoutes = [] + if ((window.location.origin + window.location.pathname) === + (getHomepageUrl() + 'auth_callback')) { + // Sit on the auth callback page until login and token + // validation is complete (it will internally redirect when complete) + return <AuthCallbackPage/> + } if (info.isFetching) { return <Fetching /> } @@ -482,6 +491,7 @@ export default withRouter(connect( info: state.info, tenant: state.tenant, timezone: state.timezone, - user: state.user + user: state.user, + auth: state.auth, }) )(App)) diff --git a/web/src/ZuulAuthProvider.jsx b/web/src/ZuulAuthProvider.jsx index 7f844e843..b5d538c90 100644 --- a/web/src/ZuulAuthProvider.jsx +++ b/web/src/ZuulAuthProvider.jsx @@ -67,7 +67,8 @@ class ZuulAuthProvider extends React.Component { onSignIn: async (user) => { // Update redux with the logged in state and send the // credentials to any other tabs. - this.props.dispatch(userLoggedIn(user)) + const redirect = localStorage.getItem('zuul_auth_redirect') + this.props.dispatch(userLoggedIn(user, redirect)) this.props.channel.postMessage({ type: 'signIn', auth_params: auth_params, diff --git a/web/src/actions/user.js b/web/src/actions/user.js index 9b3261c9f..d01d4c24c 100644 --- a/web/src/actions/user.js +++ b/web/src/actions/user.js @@ -36,11 +36,12 @@ export const fetchUserACLRequest = (tenant) => ({ tenant: tenant, }) -export const userLoggedIn = (user) => (dispatch) => { +export const userLoggedIn = (user, redirect) => (dispatch) => { dispatch({ type: USER_LOGGED_IN, user: user, token: getToken(user), + redirect: redirect, }) } diff --git a/web/src/pages/AuthCallback.jsx b/web/src/pages/AuthCallback.jsx index c31f1d222..c8f765be2 100644 --- a/web/src/pages/AuthCallback.jsx +++ b/web/src/pages/AuthCallback.jsx @@ -13,29 +13,55 @@ // under the License. import React, { useEffect } from 'react' +import PropTypes from 'prop-types' +import { connect } from 'react-redux' import { useHistory } from 'react-router-dom' - -import { Fetching } from '../containers/Fetching' +import { + EmptyState, + EmptyStateBody, + EmptyStateIcon, + Spinner, + Title, +} from '@patternfly/react-core' +import { + FingerprintIcon, +} from '@patternfly/react-icons' // Several pages use the location hash in a way that would be // difficult to disentangle from the OIDC callback parameters. This // dedicated callback page accepts the OIDC params and then internally // redirects to the page we saved before redirecting to the IDP. -function AuthCallbackPage() { - let history = useHistory() +function AuthCallbackPage(props) { + const history = useHistory() + const { user } = props useEffect(() => { - const redirect = localStorage.getItem('zuul_auth_redirect') - history.push(redirect) - }, [history]) + if (user.redirect) { + history.push(user.redirect) + } + }, [history, user]) return ( <> - <div>Login successful. You will be redirected shortly...</div> - <Fetching /> + <EmptyState> + <EmptyStateIcon icon={FingerprintIcon} /> + <Title headingLevel="h1">Login in progress</Title> + <EmptyStateBody> + <p> + You will be redirected shortly... + </p> + <Spinner size="xl" /> + </EmptyStateBody> + </EmptyState> </> ) } -export default AuthCallbackPage +AuthCallbackPage.propTypes = { + user: PropTypes.object, +} + +export default connect((state) => ({ + user: state.user, +}))(AuthCallbackPage) diff --git a/web/src/reducers/user.js b/web/src/reducers/user.js index 1a36fdae1..215cbfb1a 100644 --- a/web/src/reducers/user.js +++ b/web/src/reducers/user.js @@ -29,6 +29,7 @@ export default (state = { scope: [], isAdmin: false, tenant: null, + redirect: null, }, action) => { switch (action.type) { case USER_LOGGED_IN: { @@ -36,6 +37,7 @@ export default (state = { isFetching: false, data: action.user, token: action.token, + redirect: action.redirect, scope: [], isAdmin: false } @@ -45,6 +47,7 @@ export default (state = { isFetching: false, data: null, token: null, + redirect: null, scope: [], isAdmin: false } diff --git a/web/src/routes.js b/web/src/routes.js index a60216a97..886b1a645 100644 --- a/web/src/routes.js +++ b/web/src/routes.js @@ -34,7 +34,6 @@ import ConfigErrorsPage from './pages/ConfigErrors' import TenantsPage from './pages/Tenants' import StreamPage from './pages/Stream' import OpenApiPage from './pages/OpenApi' -import AuthCallbackPage from './pages/AuthCallback' // The Route object are created in the App component. // Object with a title are created in the menu. @@ -162,11 +161,7 @@ const routes = () => [ component: ComponentsPage, noTenantPrefix: true, }, - { - to: '/auth_callback', - component: AuthCallbackPage, - noTenantPrefix: true, - }, + // auth_callback is handled in App.jsx ] export { routes } |