Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ui] When your token expires and you sign in again, redirect to your original route. #24374

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading