From 020a8aa0c6fb767cc6a71db01fdaccd028583593 Mon Sep 17 00:00:00 2001 From: Matthew Irish Date: Thu, 1 Aug 2019 18:50:43 -0500 Subject: [PATCH] UI - support redirecting to an intended URL after authentication (#7088) * add redirect_to query param * alias auth controller state to vault controller where the query param is defined * capture the current url before redirecting a user to auth if they're being redirected * consume and reset the redirectTo query param when authenticating * make sure that the current url when logging out does not get set as the redirect_to query param * add unit tests for the mixin and make it so that redirects from the root don't end up in redirect_to * acceptance tests for redirect --- ui/app/components/auth-form.js | 12 +++++- ui/app/controllers/vault.js | 2 + ui/app/controllers/vault/cluster/auth.js | 2 +- ui/app/mixins/cluster-route.js | 25 ++++++++++--- ui/app/routes/vault/cluster/logout.js | 2 +- ui/tests/acceptance/redirect-to-test.js | 39 ++++++++++++++++++++ ui/tests/unit/mixins/cluster-route-test.js | 43 +++++++++++++++++++++- 7 files changed, 114 insertions(+), 11 deletions(-) create mode 100644 ui/tests/acceptance/redirect-to-test.js diff --git a/ui/app/components/auth-form.js b/ui/app/components/auth-form.js index 288463430213..65edd9b12757 100644 --- a/ui/app/components/auth-form.js +++ b/ui/app/components/auth-form.js @@ -192,12 +192,20 @@ export default Component.extend(DEFAULTS, { authenticate: task(function*(backendType, data) { let clusterId = this.cluster.id; - let targetRoute = this.redirectTo || 'vault.cluster'; try { let authResponse = yield this.auth.authenticate({ clusterId, backend: backendType, data }); let { isRoot, namespace } = authResponse; - let transition = this.router.transitionTo(targetRoute, { queryParams: { namespace } }); + let transition; + let { redirectTo } = this; + if (redirectTo) { + // reset the value on the controller because it's bound here + this.set('redirectTo', ''); + // here we don't need the namespace because it will be encoded in redirectTo + transition = this.router.transitionTo(redirectTo); + } else { + transition = this.router.transitionTo('vault.cluster', { queryParams: { namespace } }); + } // returning this w/then because if we keep it // in the task, it will get cancelled when the component in un-rendered yield transition.followRedirects().then(() => { diff --git a/ui/app/controllers/vault.js b/ui/app/controllers/vault.js index bd00f9815d61..562a3c39c222 100644 --- a/ui/app/controllers/vault.js +++ b/ui/app/controllers/vault.js @@ -7,9 +7,11 @@ export default Controller.extend({ queryParams: [ { wrappedToken: 'wrapped_token', + redirectTo: 'redirect_to', }, ], wrappedToken: '', + redirectTo: '', env: config.environment, auth: service(), store: service(), diff --git a/ui/app/controllers/vault/cluster/auth.js b/ui/app/controllers/vault/cluster/auth.js index 8036a9eee50c..8733761393e5 100644 --- a/ui/app/controllers/vault/cluster/auth.js +++ b/ui/app/controllers/vault/cluster/auth.js @@ -11,7 +11,7 @@ export default Controller.extend({ queryParams: [{ authMethod: 'with' }], wrappedToken: alias('vaultController.wrappedToken'), authMethod: '', - redirectTo: null, + redirectTo: alias('vaultController.redirectTo'), updateNamespace: task(function*(value) { // debounce diff --git a/ui/app/mixins/cluster-route.js b/ui/app/mixins/cluster-route.js index 2b7ff244ed23..7ed1860728fc 100644 --- a/ui/app/mixins/cluster-route.js +++ b/ui/app/mixins/cluster-route.js @@ -6,26 +6,41 @@ const INIT = 'vault.cluster.init'; const UNSEAL = 'vault.cluster.unseal'; const AUTH = 'vault.cluster.auth'; const CLUSTER = 'vault.cluster'; +const CLUSTER_INDEX = 'vault.cluster.index'; const OIDC_CALLBACK = 'vault.cluster.oidc-callback'; const DR_REPLICATION_SECONDARY = 'vault.cluster.replication-dr-promote'; -export { INIT, UNSEAL, AUTH, CLUSTER, DR_REPLICATION_SECONDARY }; +export { INIT, UNSEAL, AUTH, CLUSTER, CLUSTER_INDEX, DR_REPLICATION_SECONDARY }; export default Mixin.create({ auth: service(), store: service(), + router: service(), - transitionToTargetRoute(transition) { + transitionToTargetRoute(transition = {}) { const targetRoute = this.targetRouteName(transition); - if (targetRoute && targetRoute !== this.routeName) { + + if ( + targetRoute && + targetRoute !== this.routeName && + targetRoute !== transition.targetName && + targetRoute !== this.router.currentRouteName + ) { + if ( + // only want to redirect if we're going to authenticate + targetRoute === AUTH && + transition.targetName !== CLUSTER_INDEX + ) { + return this.transitionTo(targetRoute, { queryParams: { redirect_to: this.router.currentURL } }); + } return this.transitionTo(targetRoute); } return RSVP.resolve(); }, - beforeModel() { - return this.transitionToTargetRoute(); + beforeModel(transition) { + return this.transitionToTargetRoute(transition); }, clusterModel() { diff --git a/ui/app/routes/vault/cluster/logout.js b/ui/app/routes/vault/cluster/logout.js index d54b22ad9244..c2232706359b 100644 --- a/ui/app/routes/vault/cluster/logout.js +++ b/ui/app/routes/vault/cluster/logout.js @@ -22,7 +22,7 @@ export default Route.extend(ModelBoundaryRoute, { this.console.set('isOpen', false); this.console.clearLog(true); this.clearModelCache(); - this.replaceWith('vault.cluster'); + this.replaceWith('vault.cluster.auth', { queryParams: { redirect_to: '' } }); this.flashMessages.clearMessages(); this.permissions.reset(); }, diff --git a/ui/tests/acceptance/redirect-to-test.js b/ui/tests/acceptance/redirect-to-test.js new file mode 100644 index 000000000000..4cdec62df659 --- /dev/null +++ b/ui/tests/acceptance/redirect-to-test.js @@ -0,0 +1,39 @@ +import { currentURL, visit } from '@ember/test-helpers'; +import { module, test } from 'qunit'; +import { setupApplicationTest } from 'ember-qunit'; +import authPage from 'vault/tests/pages/auth'; + +module('Acceptance | redirect_to functionality', function(hooks) { + setupApplicationTest(hooks); + + test('redirect to a route after authentication', async function(assert) { + let url = '/vault/secrets/secret/create'; + await visit(url); + assert.equal( + currentURL(), + `/vault/auth?redirect_to=${encodeURIComponent(url)}&with=token`, + 'encodes url for the query param' + ); + // the login method on this page does another visit call that we don't want here + await authPage.tokenInput('root').submit(); + assert.equal(currentURL(), url, 'navigates to the redirect_to url after auth'); + }); + + test('redirect from root does not include redirect_to', async function(assert) { + let url = '/'; + await visit(url); + assert.equal(currentURL(), `/vault/auth?with=token`, 'there is no redirect_to query param'); + }); + + test('redirect to a route after authentication with a query param', async function(assert) { + let url = '/vault/secrets/secret/create?initialKey=hello'; + await visit(url); + assert.equal( + currentURL(), + `/vault/auth?redirect_to=${encodeURIComponent(url)}&with=token`, + 'encodes url for the query param' + ); + await authPage.tokenInput('root').submit(); + assert.equal(currentURL(), url, 'navigates to the redirect_to with the query param after auth'); + }); +}); diff --git a/ui/tests/unit/mixins/cluster-route-test.js b/ui/tests/unit/mixins/cluster-route-test.js index 9c7671bdd649..bbe58a141620 100644 --- a/ui/tests/unit/mixins/cluster-route-test.js +++ b/ui/tests/unit/mixins/cluster-route-test.js @@ -1,13 +1,21 @@ import { assign } from '@ember/polyfills'; import EmberObject from '@ember/object'; import ClusterRouteMixin from 'vault/mixins/cluster-route'; -import { INIT, UNSEAL, AUTH, CLUSTER, DR_REPLICATION_SECONDARY } from 'vault/mixins/cluster-route'; +import { + INIT, + UNSEAL, + AUTH, + CLUSTER, + CLUSTER_INDEX, + DR_REPLICATION_SECONDARY, +} from 'vault/mixins/cluster-route'; import { module, test } from 'qunit'; +import sinon from 'sinon'; module('Unit | Mixin | cluster route', function() { function createClusterRoute( clusterModel = {}, - methods = { hasKeyData: () => false, authToken: () => null } + methods = { router: {}, hasKeyData: () => false, authToken: () => null, transitionTo: () => {} } ) { let ClusterRouteObject = EmberObject.extend( ClusterRouteMixin, @@ -80,4 +88,35 @@ module('Unit | Mixin | cluster route', function() { 'forwards when not a DR secondary and navigating to DR_REPLICATION_SECONDARY' ); }); + + test('#transitionToTargetRoute', function(assert) { + let redirectRouteURL = '/vault/secrets/secret/create'; + let subject = createClusterRoute({ needsInit: false, sealed: false }); + subject.router.currentURL = redirectRouteURL; + let spy = sinon.spy(subject, 'transitionTo'); + subject.transitionToTargetRoute(); + assert.ok( + spy.calledWithExactly(AUTH, { queryParams: { redirect_to: redirectRouteURL } }), + 'calls transitionTo with the expected args' + ); + + spy.restore(); + }); + + test('#transitionToTargetRoute with auth as a target', function(assert) { + let subject = createClusterRoute({ needsInit: false, sealed: false }); + let spy = sinon.spy(subject, 'transitionTo'); + // in this case it's already transitioning to the AUTH route so we don't need to call transitionTo again + subject.transitionToTargetRoute({ targetName: AUTH }); + assert.ok(spy.notCalled, 'transitionTo is not called'); + spy.restore(); + }); + + test('#transitionToTargetRoute with auth target, coming from cluster route', function(assert) { + let subject = createClusterRoute({ needsInit: false, sealed: false }); + let spy = sinon.spy(subject, 'transitionTo'); + subject.transitionToTargetRoute({ targetName: CLUSTER_INDEX }); + assert.ok(spy.calledWithExactly(AUTH), 'calls transitionTo without redirect_to'); + spy.restore(); + }); });