From 498b29b3cf789d05fb2884a1ba6548159f59c9d9 Mon Sep 17 00:00:00 2001 From: Phil Renaud Date: Thu, 7 Nov 2024 10:43:43 -0500 Subject: [PATCH] [ui] When your token expires and you sign in again, redirect to your original route. (#24374) * Upon sign-in post-expiry/403, redirect to original route * Tests for token expiry re-routing * Had made one of the new test tokens a management token, which conflicted with another test but was not necessary --- .changelog/24374.txt | 3 + ui/app/components/forbidden-message.js | 1 + ui/app/controllers/application.js | 1 + ui/app/controllers/settings/tokens.js | 24 ++++ ui/app/routes/application.js | 1 + ui/app/services/token.js | 2 + ui/app/templates/application.hbs | 7 +- .../components/forbidden-message.hbs | 4 +- ui/tests/acceptance/token-test.js | 111 ++++++++++++++++++ 9 files changed, 149 insertions(+), 5 deletions(-) create mode 100644 .changelog/24374.txt diff --git a/.changelog/24374.txt b/.changelog/24374.txt new file mode 100644 index 00000000000..6e4c5089db1 --- /dev/null +++ b/.changelog/24374.txt @@ -0,0 +1,3 @@ +```release-note:improvement +ui: When your token expires, upon signing back in, redirect to your original route +``` diff --git a/ui/app/components/forbidden-message.js b/ui/app/components/forbidden-message.js index a864fa94522..ef0c5d59d5f 100644 --- a/ui/app/components/forbidden-message.js +++ b/ui/app/components/forbidden-message.js @@ -11,6 +11,7 @@ import { inject as service } from '@ember/service'; export default class ForbiddenMessage extends Component { @service token; @service store; + @service router; get authMethods() { return this.store.findAll('auth-method'); diff --git a/ui/app/controllers/application.js b/ui/app/controllers/application.js index abb6d7a9968..69ae95a7237 100644 --- a/ui/app/controllers/application.js +++ b/ui/app/controllers/application.js @@ -22,6 +22,7 @@ export default class ApplicationController extends Controller { @service system; @service token; @service notifications; + @service router; /** * @type {KeyboardService} diff --git a/ui/app/controllers/settings/tokens.js b/ui/app/controllers/settings/tokens.js index 074086001e5..957ebfb62d4 100644 --- a/ui/app/controllers/settings/tokens.js +++ b/ui/app/controllers/settings/tokens.js @@ -24,6 +24,7 @@ export default class Tokens extends Controller { @service store; @service router; @service system; + @service notifications; queryParams = ['code', 'state', 'jwtAuthMethod']; @tracked secret = this.token.secret; @@ -145,6 +146,7 @@ export default class Tokens extends Controller { this.token.get('fetchSelfTokenAndPolicies').perform().catch(); this.signInStatus = 'success'; + this.optionallyRedirectPathAfterSignIn(); }, () => { this.token.set('secret', undefined); @@ -174,6 +176,7 @@ export default class Tokens extends Controller { this.signInStatus = 'success'; this.token.set('tokenNotFound', false); + this.optionallyRedirectPathAfterSignIn(); }, () => { this.token.set('secret', undefined); @@ -183,6 +186,26 @@ export default class Tokens extends Controller { } } + /** + * If the user was redirected to the login page because their token expired, + * redirect them back to the page they were on. + */ + optionallyRedirectPathAfterSignIn() { + if (this.token.postExpiryPath) { + this.router.transitionTo(this.token.postExpiryPath); + this.token.postExpiryPath = null; + + // Because they won't be on the page to see "Successfully signed in", use a toast. + this.notifications.add({ + title: 'Successfully signed in', + message: + 'You were redirected back to the page you were on before you were signed out.', + color: 'success', + timeout: 10000, + }); + } + } + // Generate a 20-char nonce, using window.crypto to // create a sufficiently-large output then trimming generateNonce() { @@ -270,6 +293,7 @@ export default class Tokens extends Controller { this.signInStatus = 'success'; this.token.set('tokenNotFound', false); + this.optionallyRedirectPathAfterSignIn(); } else { this.state = 'failure'; this.code = null; diff --git a/ui/app/routes/application.js b/ui/app/routes/application.js index b2cc7effaa1..d45418bc12c 100644 --- a/ui/app/routes/application.js +++ b/ui/app/routes/application.js @@ -153,6 +153,7 @@ export default class ApplicationRoute extends Route { e.detail === 'ACL token not found' ) ) { + this.token.postExpiryPath = this.router.currentURL; this.router.transitionTo('settings.tokens'); } else { this.controllerFor('application').set('error', error); diff --git a/ui/app/services/token.js b/ui/app/services/token.js index 102fcf61bcb..9cd9066f7da 100644 --- a/ui/app/services/token.js +++ b/ui/app/services/token.js @@ -196,6 +196,8 @@ export default class TokenService extends Service { title: 'Your access has expired', message: `Your token will need to be re-authenticated`, }); + const currentPath = this.router.currentURL; + this.postExpiryPath = currentPath; } this.monitorTokenTime.cancelAll(); // Stop updating time left after expiration } diff --git a/ui/app/templates/application.hbs b/ui/app/templates/application.hbs index bae05ce7933..ec153893aa7 100644 --- a/ui/app/templates/application.hbs +++ b/ui/app/templates/application.hbs @@ -20,7 +20,7 @@ queue (action close) (action (optional flash.customCloseAction)) - + }} as |T|> {{#if flash.title}} @@ -80,12 +80,12 @@

Not Authorized

{{#if this.token.secret}}

Your - ACL token + ACL token does not provide the required permissions. Contact your administrator if this is an error.

{{else}}

Provide an - ACL token + ACL token with requisite permissions to view this.

{{/if}} {{else}} @@ -108,6 +108,7 @@ @route="settings.tokens" data-test-error-signin-link class="button is-white" + {{on "click" (action (mut this.token.postExpiryPath) this.router.currentURL)}} >Go to Sign In diff --git a/ui/app/templates/components/forbidden-message.hbs b/ui/app/templates/components/forbidden-message.hbs index 42c29977ce4..502d2235dce 100644 --- a/ui/app/templates/components/forbidden-message.hbs +++ b/ui/app/templates/components/forbidden-message.hbs @@ -13,12 +13,12 @@ {{else}} required {{/if}} - permission for this resource.
Contact your administrator if this is an error. + permission for this resource.
Contact your administrator if this is an error. {{else}} {{#if this.authMethods}} Sign in with {{#each this.authMethods as |authMethod|}} - {{authMethod.name}}, + {{authMethod.name}}, {{/each}} or {{/if}} diff --git a/ui/tests/acceptance/token-test.js b/ui/tests/acceptance/token-test.js index c29dd44b200..c4953a72472 100644 --- a/ui/tests/acceptance/token-test.js +++ b/ui/tests/acceptance/token-test.js @@ -36,6 +36,8 @@ let job; let node; let managementToken; let clientToken; +let recentlyExpiredToken; +let soonExpiringToken; module('Acceptance | tokens', function (hooks) { setupApplicationTest(hooks); @@ -53,6 +55,12 @@ module('Acceptance | tokens', function (hooks) { job = server.create('job'); managementToken = server.create('token'); clientToken = server.create('token'); + recentlyExpiredToken = server.create('token', { + expirationTime: moment().add(-5, 'm').toDate(), + }); + soonExpiringToken = server.create('token', { + expirationTime: moment().add(1, 's').toDate(), + }); }); test('it passes an accessibility audit', async function (assert) { @@ -757,6 +765,109 @@ module('Acceptance | tokens', function (hooks) { window.localStorage.nomadTokenSecret = null; }); + // Note: this differs from the 500-throwing errors above. + // In Nomad 1.5, errors for expired tokens moved from 500 "ACL token expired" to 403 "Permission Denied" + // In practice, the UI handles this differently: 403s can be either ACL-policy-denial or token-expired-denial related. + // As such, instead of an automatic redirect to the tokens page, like we did for a 500, we prompt the user with in-app + // error messages but otherwise keep them on their route, with actions to re-authenticate. + test('When a token expires with permission denial, the user is prompted to redirect to the token page (jobs page)', async function (assert) { + assert.expect(4); + window.localStorage.clear(); + + window.localStorage.nomadTokenSecret = recentlyExpiredToken.secretId; // simulate refreshing the page with an expired token + server.pretender.get('/v1/jobs/statuses', function () { + return [403, {}, 'Permission Denied']; + }); + + await visit('/jobs'); + + assert + .dom('[data-test-error]') + .exists('Error message is shown on the Jobs page'); + await click('[data-test-permission-link]'); + assert.equal( + currentURL(), + '/settings/tokens', + 'Redirected to the tokens page' + ); + + server.pretender.get('/v1/jobs/statuses', function () { + return [200, {}, null]; + }); + await Tokens.visit(); + + await Tokens.secret(recentlyExpiredToken.secretId).submit(); + assert.equal(currentURL(), '/jobs'); + + assert.dom('.flash-message.alert-success').exists(); + }); + + // Evaluations page (and others) fall back to application.hbs handling of error messages + test('When a token expires with permission denial, the user is prompted to redirect to the token page (evaluations page)', async function (assert) { + window.localStorage.clear(); + window.localStorage.nomadTokenSecret = recentlyExpiredToken.secretId; // simulate refreshing the page with an expired token + server.pretender.get('/v1/evaluations', function () { + return [403, {}, 'Permission Denied']; + }); + + await visit('/evaluations'); + + assert + .dom('[data-test-error]') + .exists('Error message is shown on the Evaluations page'); + await click('[data-test-error-acl-link]'); + assert.equal( + currentURL(), + '/settings/tokens', + 'Redirected to the tokens page' + ); + + server.pretender.get('/v1/evaluations', function () { + return [200, {}, JSON.stringify([])]; + }); + + await Tokens.secret(managementToken.secretId).submit(); + + assert.equal(currentURL(), '/evaluations'); + + assert.dom('.flash-message.alert-success').exists(); + }); + + module('Token Expiry and redirect', function (hooks) { + hooks.beforeEach(function () { + window.localStorage.nomadTokenSecret = soonExpiringToken.secretId; + }); + + test('When a token expires while the user is on a page, the notification saves redirect route', async function (assert) { + // window.localStorage.nomadTokenSecret = soonExpiringToken.secretId; + await Jobs.visit(); + assert.equal(currentURL(), '/jobs'); + + assert + .dom('.flash-message.alert-warning button') + .exists('A global alert exists and has a clickable button'); + + await click('.flash-message.alert-warning button'); + + assert.equal( + currentURL(), + '/settings/tokens', + 'Redirected to tokens page on notification action' + ); + + assert + .dom('[data-test-token-expired]') + .exists('Notification is rendered'); + + await Tokens.secret(managementToken.secretId).submit(); + assert.equal( + currentURL(), + '/jobs', + 'Redirected to initial route on manager sign in' + ); + }); + }); + function getHeader({ requestHeaders }, name) { // Headers are case-insensitive, but object property look up is not return (