Skip to content

Commit

Permalink
[ui] When your token expires and you sign in again, redirect to your …
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
philrenaud authored Nov 7, 2024
1 parent 4ef4beb commit 498b29b
Show file tree
Hide file tree
Showing 9 changed files with 149 additions and 5 deletions.
3 changes: 3 additions & 0 deletions .changelog/24374.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
ui: When your token expires, upon signing back in, redirect to your original route
```
1 change: 1 addition & 0 deletions ui/app/components/forbidden-message.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
1 change: 1 addition & 0 deletions ui/app/controllers/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export default class ApplicationController extends Controller {
@service system;
@service token;
@service notifications;
@service router;

/**
* @type {KeyboardService}
Expand Down
24 changes: 24 additions & 0 deletions ui/app/controllers/settings/tokens.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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() {
Expand Down Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions ui/app/routes/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 2 additions & 0 deletions ui/app/services/token.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
7 changes: 4 additions & 3 deletions ui/app/templates/application.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
queue
(action close)
(action (optional flash.customCloseAction))

}}
as |T|>
{{#if flash.title}}
Expand Down Expand Up @@ -80,12 +80,12 @@
<h1 data-test-error-title class="title is-spaced">Not Authorized</h1>
{{#if this.token.secret}}
<p data-test-error-message class="subtitle">Your
<LinkTo @route="settings.tokens" data-test-error-acl-link>ACL token</LinkTo>
<LinkTo @route="settings.tokens" data-test-error-acl-link {{on "click" (action (mut this.token.postExpiryPath) this.router.currentURL)}}>ACL token</LinkTo>
does not provide the required permissions. Contact your
administrator if this is an error.</p>
{{else}}
<p data-test-error-message class="subtitle">Provide an
<LinkTo @route="settings.tokens" data-test-error-acl-link>ACL token</LinkTo>
<LinkTo @route="settings.tokens" data-test-error-acl-link {{on "click" (action (mut this.token.postExpiryPath) this.router.currentURL)}}>ACL token</LinkTo>
with requisite permissions to view this.</p>
{{/if}}
{{else}}
Expand All @@ -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</LinkTo>
</div>
</div>
Expand Down
4 changes: 2 additions & 2 deletions ui/app/templates/components/forbidden-message.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@
{{else}}
required
{{/if}}
<LinkTo @route="settings.tokens">permission</LinkTo> for this resource.<br /> Contact your administrator if this is an error.
<LinkTo data-test-permission-link @route="settings.tokens" {{on "click" (action (mut this.token.postExpiryPath) this.router.currentURL)}}>permission</LinkTo> for this resource.<br /> Contact your administrator if this is an error.
{{else}}
{{#if this.authMethods}}
Sign in with
{{#each this.authMethods as |authMethod|}}
<LinkTo @route="settings.tokens">{{authMethod.name}}</LinkTo>,
<LinkTo @route="settings.tokens">{{authMethod.name}}</LinkTo>,
{{/each}}
or
{{/if}}
Expand Down
111 changes: 111 additions & 0 deletions ui/tests/acceptance/token-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ let job;
let node;
let managementToken;
let clientToken;
let recentlyExpiredToken;
let soonExpiringToken;

module('Acceptance | tokens', function (hooks) {
setupApplicationTest(hooks);
Expand All @@ -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) {
Expand Down Expand Up @@ -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 (
Expand Down

0 comments on commit 498b29b

Please sign in to comment.