-
Notifications
You must be signed in to change notification settings - Fork 2k
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, sso] warn user about pending token expiry #15091
Conversation
ui/app/services/token.js
Outdated
@task(function* () { | ||
while (this.selfToken) { | ||
const diff = new Date(this.selfToken.expirationTime) - new Date(); | ||
// Let the user know at the 10 minute mark, | ||
// or any time they refresh with under 10 minutes left | ||
if (diff < 1000 * 60 * MINUTES_LEFT_AT_WARNING) { | ||
const existingNotification = this.flashMessages.queue?.find( | ||
(m) => m.title === EXPIRY_NOTIFICATION_TITLE | ||
); | ||
// For the sake of updating the "time left" message, we keep running the task down to the moment of expiration | ||
if (diff > 0) { | ||
if (existingNotification) { | ||
existingNotification.set( | ||
'message', | ||
`Your token access expires ${moment( | ||
this.selfToken.expirationTime | ||
).fromNow()}` | ||
); | ||
} else { | ||
if (!this.expirationNotificationDismissed) { | ||
this.flashMessages.add({ | ||
title: EXPIRY_NOTIFICATION_TITLE, | ||
message: `Your token access expires ${moment( | ||
this.selfToken.expirationTime | ||
).fromNow()}`, | ||
type: 'error', | ||
destroyOnClick: false, | ||
sticky: true, | ||
customCloseAction: () => { | ||
this.set('expirationNotificationDismissed', true); | ||
}, | ||
customAction: { | ||
label: 'Re-authenticate', | ||
action: () => { | ||
this.router.transitionTo('settings.tokens'); | ||
}, | ||
}, | ||
}); | ||
} | ||
} | ||
} else { | ||
if (existingNotification) { | ||
existingNotification.setProperties({ | ||
title: 'Your access has expired', | ||
message: `Your token will need to be re-authenticated`, | ||
}); | ||
} | ||
this.monitorTokenTime.cancelAll(); // Stop updating time left after expiration | ||
} | ||
} | ||
yield timeout(1000); | ||
} | ||
}) | ||
monitorTokenTime; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is using a syntax common to Nomad's front-end, inherited from Ember Concurrency. There were a few attempts to handle this using native JS async/await but, despite the large block size, this ended up feeling more streamlined.
@@ -1,3 +1,5 @@ | |||
$bonusRightPadding: 20px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
variablizing the px-specific space we hold open for our close button
ui/app/templates/application.hbs
Outdated
<span class="close-button" role="button" {{on "click" (queue (action close) (action flash.customCloseAction))}}>×</span> | ||
{{#if flash.title}} | ||
<h3>{{flash.title}}</h3> | ||
{{/if}} | ||
{{#if flash.message}} | ||
<p>{{flash.message}}</p> | ||
{{/if}} | ||
{{#if flash.customAction}} | ||
<button type="button" class="button custom-action-button" {{on "click" (action flash.customAction.action)}}>{{flash.customAction.label}}</button> | ||
{{/if}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a couple spots in our notifications for custom actions; both optional. One on close and another that appears as a button in the notification itself.
<div class="columns"> | ||
<div class="column"> | ||
<h3 class="title is-4">Token Failed to Authenticate</h3> | ||
<p>The token secret you have provided does not match an existing token, or has expired.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review this file with "Hide Whitespace" enabled for the sake of sanity; this block was just moved into an ...unless
of the token.isExpired block from before.
Ember Asset Size actionAs of c000e5e Files that got Bigger 🚨:
Files that stayed the same size 🤷:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
would be worth adding a changelog entry for this.
154c35d
to
c000e5e
Compare
* Handle error states generally * Dont direct, just redirect * no longer need explicit error on controller * Linting on _blank * Custom notification actions and shift the template to within an else block * Lintfix * Make the closeAction optional * changelog * Add a mirage token that will always expire in 11 minutes * Test for token expiry with ember concurrency waiters * concurrency handling for earlier test, and button redirect test
* Top nav auth dropdown (#15055) * Basic dropdown styles * Some cleanup * delog * Default nomad hover state styles * Component separation-of-concerns and acceptance tests for auth dropdown * lintfix * [ui, sso] Handle token expiry 500s (#15073) * Handle error states generally * Dont direct, just redirect * no longer need explicit error on controller * Redirect on token-doesnt-exist * Forgot to import our time lib * Linting on _blank * Redirect tests * changelog * [ui, sso] warn user about pending token expiry (#15091) * Handle error states generally * Dont direct, just redirect * no longer need explicit error on controller * Linting on _blank * Custom notification actions and shift the template to within an else block * Lintfix * Make the closeAction optional * changelog * Add a mirage token that will always expire in 11 minutes * Test for token expiry with ember concurrency waiters * concurrency handling for earlier test, and button redirect test * [ui] if ACLs are disabled, remove the Sign In link from the top of the UI (#15114) * Remove top nav link if ACLs disabled * Change to an enabled-by-default model since you get no agent config when ACLs are disabled but you lack a token * PR feedback addressed; down with double negative conditionals * lintfix * ember getter instead of ?.prop * [SSO] Auth Methods and Mock OIDC Flow (#15155) * Big ol first pass at a redirect sign in flow * dont recursively add queryparams on redirect * Passing state and code qps * In which I go off the deep end and embed a faux provider page in the nomad ui * Buggy but self-contained flow * Flow auto-delay added and a little more polish to resetting token * secret passing turned to accessor passing * Handle SSO Failure * General cleanup and test fix * Lintfix * SSO flow acceptance tests * Percy snapshots added * Explicitly note the OIDC test route is mirage only * Handling failure case for complete-auth * Leentfeex * Tokens page styles (#15273) * styling and moving columns around * autofocus and enter press handling * Styles refined * Split up manager and regular tests * Standardizing to a binary status state * Serialize auth-methods response to use "name" as primary key (#15380) * Serializer for unique-by-name * Use @classic because of class extension
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
Throw a global notification when an active token is within 10 minutes of expiring, and prompt the user to re-authenticate.
These notifications pair with the tokens page changes from #15073 to resolve #15061; these notifications will be seen anyplace in the UI.
TODO