From 8c5416ecafda88306bc84d53fb20cf218c72335f Mon Sep 17 00:00:00 2001 From: John Cowen Date: Wed, 20 Jun 2018 10:22:38 +0100 Subject: [PATCH 1/2] Ensure a blank token is sent if the localStorage kv doesn't exist --- ui-v2/app/services/settings.js | 6 +++- ui-v2/app/templates/settings.hbs | 2 +- .../acceptance/steps/token-header-steps.js | 10 ++++++ ui-v2/tests/acceptance/token-header.feature | 36 +++++++++++++++++++ ui-v2/tests/helpers/yadda-annotations.js | 1 + ui-v2/tests/pages.js | 2 ++ ui-v2/tests/pages/settings.js | 6 ++++ ui-v2/tests/steps.js | 31 ++++++++++++++++ 8 files changed, 92 insertions(+), 2 deletions(-) create mode 100644 ui-v2/tests/acceptance/steps/token-header-steps.js create mode 100644 ui-v2/tests/acceptance/token-header.feature create mode 100644 ui-v2/tests/pages/settings.js diff --git a/ui-v2/app/services/settings.js b/ui-v2/app/services/settings.js index bc7c6456d5..cc3d8021e4 100644 --- a/ui-v2/app/services/settings.js +++ b/ui-v2/app/services/settings.js @@ -7,8 +7,12 @@ export default Service.extend({ storage: window.localStorage, findHeaders: function() { // TODO: if possible this should be a promise + const token = get(this, 'storage').getItem('token'); + // TODO: The old UI always sent ?token= + // replicate the old functionality here + // but remove this to be cleaner if its not necessary return { - 'X-Consul-Token': get(this, 'storage').getItem('token'), + 'X-Consul-Token': token === null ? '' : token, }; }, findAll: function(key) { diff --git a/ui-v2/app/templates/settings.hbs b/ui-v2/app/templates/settings.hbs index 8b84f3e86c..8406d57a0b 100644 --- a/ui-v2/app/templates/settings.hbs +++ b/ui-v2/app/templates/settings.hbs @@ -13,7 +13,7 @@
diff --git a/ui-v2/tests/acceptance/steps/token-header-steps.js b/ui-v2/tests/acceptance/steps/token-header-steps.js new file mode 100644 index 0000000000..c5f07c8043 --- /dev/null +++ b/ui-v2/tests/acceptance/steps/token-header-steps.js @@ -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); + }); +} diff --git a/ui-v2/tests/acceptance/token-header.feature b/ui-v2/tests/acceptance/token-header.feature new file mode 100644 index 0000000000..a1c35b80ad --- /dev/null +++ b/ui-v2/tests/acceptance/token-header.feature @@ -0,0 +1,36 @@ +@setupApplicationTest +Feature: token headers + In order to authenticate with tokens + As a user + I need to be able to specify a ACL token AND/OR leave it blank to authenticate with the API + Scenario: Arriving at the index page having not set a token previously + Given 1 datacenter model with the value "datacenter" + When I visit the index page + Then the url should be /datacenter/services + And a GET request is made to "/v1/catalog/datacenters" from yaml + --- + headers: + X-Consul-Token: '' + --- + Scenario: Set a token and then navigate to the index page + Given 1 datacenter model with the value "datacenter" + When I visit the settings page + Then the url should be /settings + Then I type with yaml + --- + token: [Token] + --- + And I submit + When I visit the index page + Then the url should be /datacenter/services + And a GET request is made to "/v1/catalog/datacenters" from yaml + --- + headers: + X-Consul-Token: [Token] + --- + Where: + --------- + | Token | + | token | + | '' | + --------- diff --git a/ui-v2/tests/helpers/yadda-annotations.js b/ui-v2/tests/helpers/yadda-annotations.js index bb7b427c16..7c972601e4 100644 --- a/ui-v2/tests/helpers/yadda-annotations.js +++ b/ui-v2/tests/helpers/yadda-annotations.js @@ -64,6 +64,7 @@ function setupScenario(featureAnnotations, scenarioAnnotations) { } return function(model) { model.afterEach(function() { + window.localStorage.clear(); api.server.reset(); }); }; diff --git a/ui-v2/tests/pages.js b/ui-v2/tests/pages.js index 1c96d8b948..e89056fa13 100644 --- a/ui-v2/tests/pages.js +++ b/ui-v2/tests/pages.js @@ -1,5 +1,6 @@ import index from 'consul-ui/tests/pages/index'; import dcs from 'consul-ui/tests/pages/dc'; +import settings from 'consul-ui/tests/pages/settings'; import services from 'consul-ui/tests/pages/dc/services/index'; import service from 'consul-ui/tests/pages/dc/services/show'; import nodes from 'consul-ui/tests/pages/dc/nodes/index'; @@ -12,6 +13,7 @@ import acl from 'consul-ui/tests/pages/dc/acls/edit'; export default { index, dcs, + settings, services, service, nodes, diff --git a/ui-v2/tests/pages/settings.js b/ui-v2/tests/pages/settings.js new file mode 100644 index 0000000000..0418b03853 --- /dev/null +++ b/ui-v2/tests/pages/settings.js @@ -0,0 +1,6 @@ +import { create, visitable, clickable } from 'ember-cli-page-object'; + +export default create({ + visit: visitable('/settings'), + submit: clickable('[type=submit]'), +}); diff --git a/ui-v2/tests/steps.js b/ui-v2/tests/steps.js index a75d11499f..4f8b3db00a 100644 --- a/ui-v2/tests/steps.js +++ b/ui-v2/tests/steps.js @@ -150,6 +150,37 @@ export default function(assert) { ); }); }) + // TODO: This one can replace the above one, it covers more use cases + // also DRY it out a bit + .then('a $method request is made to "$url" from yaml\n$yaml', function(method, url, yaml) { + const request = api.server.history[api.server.history.length - 2]; + assert.equal( + request.method, + method, + `Expected the request method to be ${method}, was ${request.method}` + ); + assert.equal(request.url, url, `Expected the request url to be ${url}, was ${request.url}`); + let data = yaml.body || {}; + const body = JSON.parse(request.requestBody); + Object.keys(data).forEach(function(key, i, arr) { + assert.equal( + body[key], + data[key], + `Expected the payload to contain ${key} to equal ${body[key]}, ${key} was ${data[key]}` + ); + }); + data = yaml.headers || {}; + const headers = request.requestHeaders; + Object.keys(data).forEach(function(key, i, arr) { + assert.equal( + headers[key], + data[key], + `Expected the payload to contain ${key} to equal ${headers[key]}, ${key} was ${ + data[key] + }` + ); + }); + }) .then('a $method request is made to "$url" with the body "$body"', function( method, url, From 0f6214d0ce4953b89d056e3f66fe86cc854d4f21 Mon Sep 17 00:00:00 2001 From: John Cowen Date: Wed, 20 Jun 2018 14:38:54 +0100 Subject: [PATCH 2/2] Make sure token is set to blank if nothing is typed in settings --- ui-v2/app/services/settings.js | 6 +++++- ui-v2/tests/acceptance/settings/update.feature | 15 +++++++++++++++ .../acceptance/steps/settings/update-steps.js | 10 ++++++++++ ui-v2/tests/acceptance/token-header.feature | 2 +- ui-v2/tests/steps.js | 9 +++++++++ 5 files changed, 40 insertions(+), 2 deletions(-) create mode 100644 ui-v2/tests/acceptance/settings/update.feature create mode 100644 ui-v2/tests/acceptance/steps/settings/update-steps.js diff --git a/ui-v2/app/services/settings.js b/ui-v2/app/services/settings.js index cc3d8021e4..2e51de89a9 100644 --- a/ui-v2/app/services/settings.js +++ b/ui-v2/app/services/settings.js @@ -16,14 +16,18 @@ export default Service.extend({ }; }, findAll: function(key) { - return Promise.resolve({ token: get(this, 'storage').getItem('token') }); + const token = get(this, 'storage').getItem('token'); + return Promise.resolve({ token: token === null ? '' : token }); }, findBySlug: function(slug) { + // TODO: Force localStorage to always be strings... + // const value = get(this, 'storage').getItem(slug); return Promise.resolve(get(this, 'storage').getItem(slug)); }, persist: function(obj) { const storage = get(this, 'storage'); Object.keys(obj).forEach((item, i) => { + // TODO: ...everywhere storage.setItem(item, obj[item]); }); return Promise.resolve(obj); diff --git a/ui-v2/tests/acceptance/settings/update.feature b/ui-v2/tests/acceptance/settings/update.feature new file mode 100644 index 0000000000..98ac584d0b --- /dev/null +++ b/ui-v2/tests/acceptance/settings/update.feature @@ -0,0 +1,15 @@ +@setupApplicationTest +Feature: settings / update: Update Settings + In order to authenticate with an ACL token + As a user + I need to be able to add my token via the UI + Scenario: I click Save without actually typing anything + Given 1 datacenter model with the value "datacenter" + When I visit the settings page + Then the url should be /settings + And I submit + Then I have settings like yaml + --- + token: '' + --- + diff --git a/ui-v2/tests/acceptance/steps/settings/update-steps.js b/ui-v2/tests/acceptance/steps/settings/update-steps.js new file mode 100644 index 0000000000..960cdf533d --- /dev/null +++ b/ui-v2/tests/acceptance/steps/settings/update-steps.js @@ -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); + }); +} diff --git a/ui-v2/tests/acceptance/token-header.feature b/ui-v2/tests/acceptance/token-header.feature index a1c35b80ad..750a3fe745 100644 --- a/ui-v2/tests/acceptance/token-header.feature +++ b/ui-v2/tests/acceptance/token-header.feature @@ -12,7 +12,7 @@ Feature: token headers headers: X-Consul-Token: '' --- - Scenario: Set a token and then navigate to the index page + Scenario: Set the token to [Token] and then navigate to the index page Given 1 datacenter model with the value "datacenter" When I visit the settings page Then the url should be /settings diff --git a/ui-v2/tests/steps.js b/ui-v2/tests/steps.js index 4f8b3db00a..459d8fd951 100644 --- a/ui-v2/tests/steps.js +++ b/ui-v2/tests/steps.js @@ -242,6 +242,15 @@ export default function(assert) { `Expected ${num} ${model}s with ${property} set to "${value}", saw ${len}` ); }) + .then('I have settings like yaml\n$yaml', function(data) { + // TODO: Inject this + const settings = window.localStorage; + Object.keys(data).forEach(function(prop) { + const actual = settings.getItem(prop); + const expected = data[prop]; + assert.strictEqual(actual, expected, `Expected settings to be ${expected} was ${actual}`); + }); + }) .then('I see $property on the $component like yaml\n$yaml', function( property, component,