[ui, epic] SSO and Auth improvements #15110
Conversation
Ember Asset Size actionAs of ace1b60 Files that got Bigger 🚨:
Files that stayed the same size 🤷:
|
philrenaud
changed the title
* Basic dropdown styles * Some cleanup * delog * Default nomad hover state styles * Component separation-of-concerns and acceptance tests for auth dropdown * lintfix
* 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
* 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
…e 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
Ember Test Audit comparison
|
* 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
* styling and moving columns around * autofocus and enter press handling * Styles refined * Split up manager and regular tests * Standardizing to a binary status state
* Serializer for unique-by-name * Use @classic because of class extension
|
|
||
| @alias('id') accessor; | ||
|
|
||
| get isExpired() { | ||
| return this.expirationTime && this.expirationTime < new Date(); |
There was a problem hiding this comment.
question: ember-data sets the date attribute to use ISO 8601 format whereas JavaScript's Date constructor gives us a Unix time. Should we have concern re: date formatting in this comparison?
There was a problem hiding this comment.
Hm, this is a good concern to raise but in manual and acceptance testing I have found that the comparison works as expected.
| e.detail === 'ACL token expired' || | ||
| e.detail === 'ACL token not found' |
There was a problem hiding this comment.
question: Does this mean we're hard coding the error message that we're expecting from the API here? And thus, if they added a period to the end of the message, we wouldn't be able to match here?
There was a problem hiding this comment.
That's right — this is a pretty fragile way of doing a match. Luckily this PR also includes visual diff and acceptance tests for this situation, so if this ever changes we should be getting plenty of alarm bells.
| return { | ||
| authMethods: this.store.findAll('auth-method'), | ||
| }; |
There was a problem hiding this comment.
nit: If this is the only property returned on the model couldn't we just write:
async model() {
return this.store.findAll('auth-method')
}
There was a problem hiding this comment.
For sure, but I kind of like the explicitness of asking for this.model.authMethods on a tokens page.
(If you asked me in a vacuum what this.model would iterate on a page called tokens, I would not assume it would be auth-methods, so IMO better to make it explicit)
| e.detail === 'ACL token not found' | ||
| ) | ||
| ) { | ||
| this.router.transitionTo('settings.tokens'); |
There was a problem hiding this comment.
question: Should a flash message accompany this transition?
There was a problem hiding this comment.
Token errors (not found, expired) are shown as part of the Tokens service — I've added screenshots and some explanations why I'm using errors in service/model instead of here in the application route in the PR notes: #15073
|
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. |
(UI Epic branch)