From b76d2cd09cc0f45cb83c939fd361485ea5ea05f3 Mon Sep 17 00:00:00 2001 From: Chelsea Shaw <82459713+hashishaw@users.noreply.github.com> Date: Wed, 20 Oct 2021 10:38:29 -0400 Subject: [PATCH] UI/OIDC provider fix (#12871) * Add cluster name to oidc-provider path * Move oidc-provider route up on router * Return base url for changelog if no version * OIDC Provider check on targetRouteName instead of transitionToTargetRoute * restore dynamic provider segment on route * Fix redirect after auth issue * handle permission denied --- ui/app/mixins/cluster-route.js | 6 ++-- ui/app/router.js | 7 ++-- .../vault/cluster/identity/oidc-provider.js | 32 ++++++++++++------- .../core/addon/helpers/changelog-url-for.js | 2 +- 4 files changed, 27 insertions(+), 20 deletions(-) diff --git a/ui/app/mixins/cluster-route.js b/ui/app/mixins/cluster-route.js index 66127b89b..74f58668d 100644 --- a/ui/app/mixins/cluster-route.js +++ b/ui/app/mixins/cluster-route.js @@ -21,9 +21,6 @@ export default Mixin.create({ transitionToTargetRoute(transition = {}) { const targetRoute = this.targetRouteName(transition); - if (OIDC_PROVIDER === this.router.currentRouteName || OIDC_PROVIDER === transition?.to?.name) { - return RSVP.resolve(); - } if ( targetRoute && targetRoute !== this.routeName && @@ -83,6 +80,9 @@ export default Mixin.create({ return DR_REPLICATION_SECONDARY; } + if ((transition && transition.targetName === OIDC_PROVIDER) || this.routeName === OIDC_PROVIDER) { + return OIDC_PROVIDER; + } if (!isAuthed) { if ((transition && transition.targetName === OIDC_CALLBACK) || this.routeName === OIDC_CALLBACK) { return OIDC_CALLBACK; diff --git a/ui/app/router.js b/ui/app/router.js index 95a6e83e5..20c3e00d0 100644 --- a/ui/app/router.js +++ b/ui/app/router.js @@ -9,6 +9,9 @@ export default class Router extends EmberRouter { Router.map(function() { this.route('vault', { path: '/' }, function() { this.route('cluster', { path: '/:cluster_name' }, function() { + this.route('identity', function() { + this.route('oidc-provider', { path: '/oidc/provider/:provider_name/authorize' }); + }); this.route('oidc-callback', { path: '/auth/*auth_path/oidc/callback' }); this.route('auth'); this.route('init'); @@ -139,10 +142,6 @@ Router.map(function() { } this.route('not-found', { path: '/*path' }); - - this.route('identity', function() { - this.route('oidc-provider', { path: '/oidc/provider/:oidc_name/authorize' }); - }); }); this.route('not-found', { path: '/*path' }); }); diff --git a/ui/app/routes/vault/cluster/identity/oidc-provider.js b/ui/app/routes/vault/cluster/identity/oidc-provider.js index 7f5a4c66a..bdc9d41fd 100644 --- a/ui/app/routes/vault/cluster/identity/oidc-provider.js +++ b/ui/app/routes/vault/cluster/identity/oidc-provider.js @@ -13,6 +13,7 @@ export default class VaultClusterIdentityOidcProviderRoute extends Route { } _redirect(url, params) { + if (!url) return; let redir = this._buildUrl(url, params); this.win.location.replace(redir); } @@ -27,23 +28,27 @@ export default class VaultClusterIdentityOidcProviderRoute extends Route { error: 'login_required', }); } else if (!currentToken || 'login' === qp.prompt?.toLowerCase()) { + let shouldLogout = !!currentToken; if ('login' === qp.prompt?.toLowerCase()) { - this.auth.deleteCurrentToken(); + // need to remove before redirect to avoid infinite loop qp.prompt = null; } - let { cluster_name } = this.paramsFor('vault.cluster'); - let url = this.router.urlFor(transition.to.name, transition.to.params, { queryParams: qp }); - return this.transitionTo(AUTH, cluster_name, { queryParams: { redirect_to: url } }); + return this._redirectToAuth(transition.to.params?.provider_name, qp, shouldLogout); } } - _redirectToAuth(oidcName, queryParams, logout = false) { + _redirectToAuth(provider_name, queryParams, logout = false) { let { cluster_name } = this.paramsFor('vault.cluster'); - let currentRoute = this.router.urlFor(PROVIDER, oidcName, { queryParams }); + let url = this.router.urlFor(PROVIDER, cluster_name, provider_name, { queryParams }); + // This is terrible, I'm sorry + // Need to do this because transitionTo (as used in auth-form) expects url without + // rootURL /ui/ at the beginning, but urlFor builds it in. We can't use currentRoute + // because it hasn't transitioned yet + url = url.replace(/^(\/?ui)/, ''); if (logout) { this.auth.deleteCurrentToken(); } - return this.transitionTo(AUTH, cluster_name, { queryParams: { redirect_to: currentRoute } }); + return this.transitionTo(AUTH, cluster_name, { queryParams: { redirect_to: url } }); } _buildUrl(urlString, params) { @@ -72,11 +77,14 @@ export default class VaultClusterIdentityOidcProviderRoute extends Route { } async model(params) { - let { oidc_name, ...qp } = params; + let { provider_name, ...qp } = params; let decodedRedirect = decodeURI(qp.redirect_uri); - let url = this._buildUrl(`${this.win.origin}/v1/identity/oidc/provider/${oidc_name}/authorize`, qp); + let endpoint = this._buildUrl( + `${this.win.origin}/v1/identity/oidc/provider/${provider_name}/authorize`, + qp + ); try { - const response = await this.auth.ajax(url, 'GET', {}); + const response = await this.auth.ajax(endpoint, 'GET', {}); if ('consent' === qp.prompt?.toLowerCase()) { return { consent: { @@ -90,8 +98,8 @@ export default class VaultClusterIdentityOidcProviderRoute extends Route { } catch (errorRes) { let resp = await errorRes.json(); let code = resp.error; - if (code === 'max_age_violation') { - this._redirectToAuth(oidc_name, qp, true); + if (code === 'max_age_violation' || resp?.errors?.includes('permission denied')) { + this._redirectToAuth(provider_name, qp, true); } else if (code === 'invalid_redirect_uri') { return { error: { diff --git a/ui/lib/core/addon/helpers/changelog-url-for.js b/ui/lib/core/addon/helpers/changelog-url-for.js index 37178e7ba..e9db7f66a 100644 --- a/ui/lib/core/addon/helpers/changelog-url-for.js +++ b/ui/lib/core/addon/helpers/changelog-url-for.js @@ -15,7 +15,7 @@ etc. export function changelogUrlFor([version]) { let url = 'https://www.github.com/hashicorp/vault/blob/main/CHANGELOG.md#'; - + if (!version) return url; try { // strip the '+prem' from enterprise versions and remove periods let versionNumber = version