UI: Removes success notification on faking a success response for `self` (#4906)

In order to continue supporting the legacy ACL system, we replace
the 500 error from a non-existent `self` endpoint with a response of a
`null` `AccessorID` - which makes sense (a null AccessorID means old
API)

We then redirect the user to the old ACL pages which then gives a 403
if their token was wrong which then redirects them back to the login page.

Due to the multiple redirects and not wanting to test the validity of the token
before redirecting (thus calling the same API endpoint twice), it is not
straightforwards to turn the 'faked' response from the `self` endpoint
into an error (flash messages are 'lost' through multiple redirects).

In order to make this a slightly better experience, you can now return a
`false` during execution of an action requiring success/failure
feedback, this essentially skips the notification, so if the action is
'successful' but you don't want to show the notification, you can. This
resolves showing a successful notification when the `self` endpoint
response is faked. The last part of the puzzle is to make sure that the
global 403 catching error in the application Route also produces an
erroneous notification.

Please note this can only happen with a ui client using the new ACL
system when communicating with a cluster using the old ACL system, and
only when you enter the wrong token.

Lastly, further acceptance tests have been added around this

This commit also adds functionality to avoid any possible double 
notification messages, to avoid UI overlapping
This commit is contained in:
John Cowen 2018-11-07 15:57:41 +00:00 committed by GitHub
parent f5bb65931b
commit 751f8552b2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 158 additions and 26 deletions

View File

@ -3,11 +3,13 @@ import { inject as service } from '@ember/service';
import { hash } from 'rsvp'; import { hash } from 'rsvp';
import { get } from '@ember/object'; import { get } from '@ember/object';
import { next } from '@ember/runloop'; import { next } from '@ember/runloop';
import { Promise } from 'rsvp';
import WithBlockingActions from 'consul-ui/mixins/with-blocking-actions';
const $html = document.documentElement; const $html = document.documentElement;
const removeLoading = function() { const removeLoading = function() {
return $html.classList.remove('ember-loading'); return $html.classList.remove('ember-loading');
}; };
export default Route.extend({ export default Route.extend(WithBlockingActions, {
init: function() { init: function() {
this._super(...arguments); this._super(...arguments);
}, },
@ -61,11 +63,13 @@ export default Route.extend({
// if a token has not been sent at all, it just gives you a 200 with an empty dataset // if a token has not been sent at all, it just gives you a 200 with an empty dataset
const model = this.modelFor('dc'); const model = this.modelFor('dc');
if (error.status === '403') { if (error.status === '403') {
return get(this, 'feedback').execute(() => {
return get(this, 'settings') return get(this, 'settings')
.delete('token') .delete('token')
.then(() => { .then(() => {
return this.transitionTo('dc.acls.tokens', model.dc.Name); return Promise.reject(this.transitionTo('dc.acls.tokens', model.dc.Name));
}); });
}, 'authorize');
} }
if (error.status === '') { if (error.status === '') {
error.message = 'Error'; error.message = 'Error';

View File

@ -13,7 +13,7 @@ export default Route.extend(WithBlockingActions, {
return get(this, 'repo') return get(this, 'repo')
.self(secret, dc) .self(secret, dc)
.then(item => { .then(item => {
get(this, 'settings') return get(this, 'settings')
.persist({ .persist({
token: { token: {
AccessorID: get(item, 'AccessorID'), AccessorID: get(item, 'AccessorID'),
@ -25,7 +25,11 @@ export default Route.extend(WithBlockingActions, {
// take the user to the legacy acls // take the user to the legacy acls
// otherwise just refresh the page // otherwise just refresh the page
if (get(item, 'token.AccessorID') === null) { if (get(item, 'token.AccessorID') === null) {
return this.transitionTo('dc.acls'); // returning false for a feedback action means even though
// its successful, please skip this notification and don't display it
return this.transitionTo('dc.acls').then(function() {
return false;
});
} else { } else {
this.refresh(); this.refresh();
} }

View File

@ -25,6 +25,10 @@ export default Service.extend({
handle() handle()
//TODO: pass this through to getAction.. //TODO: pass this through to getAction..
.then(item => { .then(item => {
// returning exactly `false` for a feedback action means even though
// its successful, please skip this notification and don't display it
if (item !== false) {
notify.clearMessages();
// TODO right now the majority of `item` is a Transition // TODO right now the majority of `item` is a Transition
// but you can resolve an object // but you can resolve an object
notify.add({ notify.add({
@ -34,8 +38,10 @@ export default Service.extend({
action: getAction(), action: getAction(),
item: item, item: item,
}); });
}
}) })
.catch(e => { .catch(e => {
notify.clearMessages();
get(this, 'logger').execute(e); get(this, 'logger').execute(e);
if (e.name === 'TransitionAborted') { if (e.name === 'TransitionAborted') {
notify.add({ notify.add({

View File

@ -28,6 +28,8 @@ export default function(isValidServerError, P = Promise) {
if (isValidServerError(e)) { if (isValidServerError(e)) {
enable(true); enable(true);
authorize(false); authorize(false);
} else {
return P.reject(e);
} }
break; break;
case '403': case '403':

View File

@ -0,0 +1,52 @@
@setupApplicationTest
Feature: dc / acls / tokens / index: ACL Login Errors
Scenario: I get any 500 error that is not the specific legacy token cluster one
Given 1 datacenter model with the value "dc-1"
Given the url "/v1/acl/tokens" responds with a 500 status
When I visit the tokens page for yaml
---
dc: dc-1
---
Then the url should be /dc-1/acls/tokens
Then I see the text "500 (The backend responded with an error)" in "[data-test-error]"
Scenario: I get a 500 error from acl/tokens that is the specific legacy one
Given 1 datacenter model with the value "dc-1"
And the url "/v1/acl/tokens" responds with from yaml
---
status: 500
body: "rpc error making call: rpc: can't find method ACL.TokenRead"
---
When I visit the tokens page for yaml
---
dc: dc-1
---
Then the url should be /dc-1/acls/tokens
Then ".app-view" has the "unauthorized" class
Scenario: I get a 500 error from acl/token/self that is the specific legacy one
Given 1 datacenter model with the value "dc-1"
Given the url "/v1/acl/tokens" responds with from yaml
---
status: 500
body: "rpc error making call: rpc: can't find method ACL.TokenRead"
---
And the url "/v1/acl/token/self" responds with from yaml
---
status: 500
body: "rpc error making call: rpc: can't find method ACL.TokenRead"
---
And the url "/v1/acl/list" responds with a 403 status
When I visit the tokens page for yaml
---
dc: dc-1
---
Then the url should be /dc-1/acls/tokens
Then ".app-view" has the "unauthorized" class
Then I fill in with yaml
---
secret: something
---
And I submit
Then ".app-view" has the "unauthorized" class
And "[data-notification]" has the "error" class

View File

@ -0,0 +1,10 @@
import steps from '../../../steps';
// step definitions that are shared between features should be moved to the
// tests/acceptance/steps/steps.js file
export default function(assert) {
return steps(assert).then('I should find a file', function() {
assert.ok(true, this.step);
});
}

View File

@ -84,6 +84,9 @@ export default function(assert) {
.given(['the url "$url" responds with a $status status'], function(url, status) { .given(['the url "$url" responds with a $status status'], function(url, status) {
return api.server.respondWithStatus(url.split('?')[0], parseInt(status)); return api.server.respondWithStatus(url.split('?')[0], parseInt(status));
}) })
.given(['the url "$url" responds with from yaml\n$yaml'], function(url, data) {
api.server.respondWith(url.split('?')[0], data);
})
// interactions // interactions
.when('I visit the $name page', function(name) { .when('I visit the $name page', function(name) {
currentPage = pages[name]; currentPage = pages[name];

View File

@ -2,7 +2,13 @@ import { moduleFor, test } from 'ember-qunit';
moduleFor('route:application', 'Unit | Route | application', { moduleFor('route:application', 'Unit | Route | application', {
// Specify the other units that are required for this test. // Specify the other units that are required for this test.
needs: ['service:repository/dc', 'service:settings'], needs: [
'service:repository/dc',
'service:settings',
'service:feedback',
'service:flashMessages',
'service:logger',
],
}); });
test('it exists', function(assert) { test('it exists', function(assert) {

View File

@ -610,8 +610,8 @@
resolved "https://registry.yarnpkg.com/@hashicorp/consul-api-double/-/consul-api-double-2.0.1.tgz#eaf2e3f230fbdd876c90b931fd4bb4d94aac10e2" resolved "https://registry.yarnpkg.com/@hashicorp/consul-api-double/-/consul-api-double-2.0.1.tgz#eaf2e3f230fbdd876c90b931fd4bb4d94aac10e2"
"@hashicorp/ember-cli-api-double@^1.3.0": "@hashicorp/ember-cli-api-double@^1.3.0":
version "1.6.1" version "1.7.0"
resolved "https://registry.yarnpkg.com/@hashicorp/ember-cli-api-double/-/ember-cli-api-double-1.6.1.tgz#c45efe045da971dcb0915f182cdc4a75929a7ade" resolved "https://registry.yarnpkg.com/@hashicorp/ember-cli-api-double/-/ember-cli-api-double-1.7.0.tgz#4fdab6152157dd82b999de030c593c87e0cdb8b7"
dependencies: dependencies:
"@hashicorp/api-double" "^1.3.0" "@hashicorp/api-double" "^1.3.0"
array-range "^1.0.1" array-range "^1.0.1"
@ -2028,7 +2028,7 @@ body-parser@1.18.2:
raw-body "2.3.2" raw-body "2.3.2"
type-is "~1.6.15" type-is "~1.6.15"
body-parser@^1.18.3: body-parser@1.18.3, body-parser@^1.18.3:
version "1.18.3" version "1.18.3"
resolved "https://registry.yarnpkg.com/body-parser/-/body-parser-1.18.3.tgz#5b292198ffdd553b3a0f20ded0592b956955c8b4" resolved "https://registry.yarnpkg.com/body-parser/-/body-parser-1.18.3.tgz#5b292198ffdd553b3a0f20ded0592b956955c8b4"
dependencies: dependencies:
@ -5299,7 +5299,7 @@ expand-tilde@^2.0.0, expand-tilde@^2.0.2:
dependencies: dependencies:
homedir-polyfill "^1.0.1" homedir-polyfill "^1.0.1"
express@^4.10.7, express@^4.12.3, express@^4.16.2: express@^4.10.7, express@^4.12.3:
version "4.16.3" version "4.16.3"
resolved "https://registry.yarnpkg.com/express/-/express-4.16.3.tgz#6af8a502350db3246ecc4becf6b5a34d22f7ed53" resolved "https://registry.yarnpkg.com/express/-/express-4.16.3.tgz#6af8a502350db3246ecc4becf6b5a34d22f7ed53"
dependencies: dependencies:
@ -5334,6 +5334,41 @@ express@^4.10.7, express@^4.12.3, express@^4.16.2:
utils-merge "1.0.1" utils-merge "1.0.1"
vary "~1.1.2" vary "~1.1.2"
express@^4.16.2:
version "4.16.4"
resolved "https://registry.yarnpkg.com/express/-/express-4.16.4.tgz#fddef61926109e24c515ea97fd2f1bdbf62df12e"
dependencies:
accepts "~1.3.5"
array-flatten "1.1.1"
body-parser "1.18.3"
content-disposition "0.5.2"
content-type "~1.0.4"
cookie "0.3.1"
cookie-signature "1.0.6"
debug "2.6.9"
depd "~1.1.2"
encodeurl "~1.0.2"
escape-html "~1.0.3"
etag "~1.8.1"
finalhandler "1.1.1"
fresh "0.5.2"
merge-descriptors "1.0.1"
methods "~1.1.2"
on-finished "~2.3.0"
parseurl "~1.3.2"
path-to-regexp "0.1.7"
proxy-addr "~2.0.4"
qs "6.5.2"
range-parser "~1.2.0"
safe-buffer "5.1.2"
send "0.16.2"
serve-static "1.13.2"
setprototypeof "1.1.0"
statuses "~1.4.0"
type-is "~1.6.16"
utils-merge "1.0.1"
vary "~1.1.2"
extend-shallow@^2.0.1: extend-shallow@^2.0.1:
version "2.0.1" version "2.0.1"
resolved "https://registry.yarnpkg.com/extend-shallow/-/extend-shallow-2.0.1.tgz#51af7d614ad9a9f610ea1bafbb989d6b1c56890f" resolved "https://registry.yarnpkg.com/extend-shallow/-/extend-shallow-2.0.1.tgz#51af7d614ad9a9f610ea1bafbb989d6b1c56890f"
@ -7833,7 +7868,7 @@ mdurl@^1.0.1:
media-typer@0.3.0: media-typer@0.3.0:
version "0.3.0" version "0.3.0"
resolved "https://registry.yarnpkg.com/media-typer/-/media-typer-0.3.0.tgz#8710d7af0aa626f8fffa1ce00168545263255748" resolved "http://registry.npmjs.org/media-typer/-/media-typer-0.3.0.tgz#8710d7af0aa626f8fffa1ce00168545263255748"
mem@^1.1.0: mem@^1.1.0:
version "1.1.0" version "1.1.0"
@ -7956,7 +7991,11 @@ mime-db@~1.36.0:
version "1.36.0" version "1.36.0"
resolved "https://registry.yarnpkg.com/mime-db/-/mime-db-1.36.0.tgz#5020478db3c7fe93aad7bbcc4dcf869c43363397" resolved "https://registry.yarnpkg.com/mime-db/-/mime-db-1.36.0.tgz#5020478db3c7fe93aad7bbcc4dcf869c43363397"
mime-types@^2.1.12, mime-types@~2.1.17, mime-types@~2.1.18, mime-types@~2.1.19, mime-types@~2.1.7: mime-db@~1.37.0:
version "1.37.0"
resolved "https://registry.yarnpkg.com/mime-db/-/mime-db-1.37.0.tgz#0b6a0ce6fdbe9576e25f1f2d2fde8830dc0ad0d8"
mime-types@^2.1.12, mime-types@~2.1.17, mime-types@~2.1.19, mime-types@~2.1.7:
version "2.1.20" version "2.1.20"
resolved "https://registry.yarnpkg.com/mime-types/-/mime-types-2.1.20.tgz#930cb719d571e903738520f8470911548ca2cc19" resolved "https://registry.yarnpkg.com/mime-types/-/mime-types-2.1.20.tgz#930cb719d571e903738520f8470911548ca2cc19"
dependencies: dependencies:
@ -7968,6 +8007,12 @@ mime-types@^2.1.18:
dependencies: dependencies:
mime-db "~1.33.0" mime-db "~1.33.0"
mime-types@~2.1.18:
version "2.1.21"
resolved "https://registry.yarnpkg.com/mime-types/-/mime-types-2.1.21.tgz#28995aa1ecb770742fe6ae7e58f9181c744b3f96"
dependencies:
mime-db "~1.37.0"
mime@1.4.1: mime@1.4.1:
version "1.4.1" version "1.4.1"
resolved "https://registry.yarnpkg.com/mime/-/mime-1.4.1.tgz#121f9ebc49e3766f311a76e1fa1c8003c4b03aa6" resolved "https://registry.yarnpkg.com/mime/-/mime-1.4.1.tgz#121f9ebc49e3766f311a76e1fa1c8003c4b03aa6"
@ -8979,7 +9024,7 @@ promised-io@*:
version "0.3.5" version "0.3.5"
resolved "https://registry.yarnpkg.com/promised-io/-/promised-io-0.3.5.tgz#4ad217bb3658bcaae9946b17a8668ecd851e1356" resolved "https://registry.yarnpkg.com/promised-io/-/promised-io-0.3.5.tgz#4ad217bb3658bcaae9946b17a8668ecd851e1356"
proxy-addr@~2.0.3: proxy-addr@~2.0.3, proxy-addr@~2.0.4:
version "2.0.4" version "2.0.4"
resolved "https://registry.yarnpkg.com/proxy-addr/-/proxy-addr-2.0.4.tgz#ecfc733bf22ff8c6f407fa275327b9ab67e48b93" resolved "https://registry.yarnpkg.com/proxy-addr/-/proxy-addr-2.0.4.tgz#ecfc733bf22ff8c6f407fa275327b9ab67e48b93"
dependencies: dependencies:
@ -9707,7 +9752,7 @@ safe-buffer@5.1.1:
version "5.1.1" version "5.1.1"
resolved "https://registry.yarnpkg.com/safe-buffer/-/safe-buffer-5.1.1.tgz#893312af69b2123def71f57889001671eeb2c853" resolved "https://registry.yarnpkg.com/safe-buffer/-/safe-buffer-5.1.1.tgz#893312af69b2123def71f57889001671eeb2c853"
safe-buffer@^5.0.1, safe-buffer@^5.1.0, safe-buffer@^5.1.1, safe-buffer@^5.1.2, safe-buffer@~5.1.0, safe-buffer@~5.1.1: safe-buffer@5.1.2, safe-buffer@^5.0.1, safe-buffer@^5.1.0, safe-buffer@^5.1.1, safe-buffer@^5.1.2, safe-buffer@~5.1.0, safe-buffer@~5.1.1:
version "5.1.2" version "5.1.2"
resolved "https://registry.yarnpkg.com/safe-buffer/-/safe-buffer-5.1.2.tgz#991ec69d296e0313747d59bdfd2b745c35f8828d" resolved "https://registry.yarnpkg.com/safe-buffer/-/safe-buffer-5.1.2.tgz#991ec69d296e0313747d59bdfd2b745c35f8828d"