From ed46fbf3cb3ed3595f66403830122e1ae02ef9a6 Mon Sep 17 00:00:00 2001 From: ivaosthu Date: Sat, 20 Jan 2018 14:09:42 +0100 Subject: [PATCH 1/7] Bump unleash-frontend to 3.0.0-alpha.6 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 254dbb54ea..eba9b47585 100644 --- a/package.json +++ b/package.json @@ -83,7 +83,7 @@ "prometheus-gc-stats": "^0.5.0", "response-time": "^2.3.2", "serve-favicon": "^2.3.0", - "unleash-frontend": "^3.0.0-alpha.5", + "unleash-frontend": "^3.0.0-alpha.6", "yallist": "^3.0.2", "yargs": "^10.0.3" }, From f08136e50808e649ddc4720a698fe37d06965669 Mon Sep 17 00:00:00 2001 From: ivaosthu Date: Sat, 20 Jan 2018 14:11:27 +0100 Subject: [PATCH 2/7] update yarn.lock --- yarn.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/yarn.lock b/yarn.lock index 414c504368..481f7024a6 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5775,9 +5775,9 @@ unique-temp-dir@^1.0.0: os-tmpdir "^1.0.1" uid2 "0.0.3" -unleash-frontend@^3.0.0-alpha.5: - version "3.0.0-alpha.5" - resolved "https://registry.yarnpkg.com/unleash-frontend/-/unleash-frontend-3.0.0-alpha.5.tgz#ef4c2bb9e24ba07465b1737098b92b6a036df282" +unleash-frontend@^3.0.0-alpha.6: + version "3.0.0-alpha.6" + resolved "https://registry.yarnpkg.com/unleash-frontend/-/unleash-frontend-3.0.0-alpha.6.tgz#439a5554a1169082b7ae19544002cad19409a27d" dependencies: debug "^3.1.0" immutable "^3.8.1" From 56ca8bde7aabffdc95fed5a78c4d08cb5e63eeb1 Mon Sep 17 00:00:00 2001 From: ivaosthu Date: Sat, 20 Jan 2018 13:28:04 +0100 Subject: [PATCH 3/7] Bugfix: more informative name validation errors Will know tell the user if the toggle name is already in use by an active feature toggle or an archived toggle. Also brings lates unleash-frontend fix, which prevents an invalid form from submitting. closes: #290, #291 --- lib/db/feature-toggle-store.js | 14 ++++-- lib/routes/admin-api/feature.js | 23 +++++----- lib/routes/admin-api/feature.test.js | 50 ++++++++++++++++++++++ test/fixtures/fake-feature-toggle-store.js | 12 +++++- 4 files changed, 82 insertions(+), 17 deletions(-) diff --git a/lib/db/feature-toggle-store.js b/lib/db/feature-toggle-store.js index c09fc470c0..4604039e14 100644 --- a/lib/db/feature-toggle-store.js +++ b/lib/db/feature-toggle-store.js @@ -51,12 +51,20 @@ class FeatureToggleStore { .then(this.rowToFeature); } - hasFeatureName(name) { + hasFeature(name) { return this.db - .first('name') + .first('name', 'archived') .from(TABLE) .where({ name }) - .then(n => !!n); + .then(row => { + if (!row) { + throw new NotFoundError('No feature toggle found'); + } + return { + name: row.name, + archived: row.archived === 1, + }; + }); } getArchivedFeatures() { diff --git a/lib/routes/admin-api/feature.js b/lib/routes/admin-api/feature.js index f3e09becb3..c20d7edc14 100644 --- a/lib/routes/admin-api/feature.js +++ b/lib/routes/admin-api/feature.js @@ -23,12 +23,7 @@ const handleErrors = (req, res, error) => { case NameExistsError: return res .status(403) - .json([ - { - msg: - 'A feature with this name already exists. Try re-activating it from the archive.', - }, - ]) + .json([{ msg: error.message }]) .end(); case ValidationError: return res @@ -97,13 +92,15 @@ module.exports.router = function(config) { function validateUniqueName(req) { return new Promise((resolve, reject) => { - featureToggleStore.hasFeatureName(req.body.name).then(hasName => { - if (hasName) { - reject(new NameExistsError('Feature name already exist')); - } else { - resolve(req); - } - }); + featureToggleStore + .hasFeature(req.body.name) + .then(definition => { + const msg = definition.archived + ? 'An archived toggle with that name already exist' + : 'A toggle with that name already exist'; + reject(new NameExistsError(msg)); + }) + .catch(() => resolve(req)); }); } diff --git a/lib/routes/admin-api/feature.test.js b/lib/routes/admin-api/feature.test.js index c540f3ef48..2a49bbdb45 100644 --- a/lib/routes/admin-api/feature.test.js +++ b/lib/routes/admin-api/feature.test.js @@ -81,6 +81,56 @@ test('should require at least one strategy when creating a feature toggle', t => .expect(400); }); +test('should be allowed to use new toggle name', t => { + t.plan(0); + const { request, base } = getSetup(); + + return request + .post(`${base}/api/admin/features/validate`) + .send({ name: 'new.name' }) + .set('Content-Type', 'application/json') + .expect(201); +}); + +test('should not be allowed to reuse active toggle name', t => { + t.plan(1); + const { request, featureToggleStore, base } = getSetup(); + featureToggleStore.addFeature({ + name: 'ts', + strategies: [{ name: 'default' }], + }); + + return request + .post(`${base}/api/admin/features/validate`) + .send({ name: 'ts' }) + .set('Content-Type', 'application/json') + .expect(403) + .expect(res => { + t.true(res.body[0].msg === 'A toggle with that name already exist'); + }); +}); + +test('should not be allowed to reuse archived toggle name', t => { + t.plan(1); + const { request, featureToggleStore, base } = getSetup(); + featureToggleStore.addArchivedFeature({ + name: 'ts.archived', + strategies: [{ name: 'default' }], + }); + + return request + .post(`${base}/api/admin/features/validate`) + .send({ name: 'ts.archived' }) + .set('Content-Type', 'application/json') + .expect(403) + .expect(res => { + t.true( + res.body[0].msg === + 'An archived toggle with that name already exist' + ); + }); +}); + test('should require at least one strategy when updating a feature toggle', t => { t.plan(0); const { request, featureToggleStore, base } = getSetup(); diff --git a/test/fixtures/fake-feature-toggle-store.js b/test/fixtures/fake-feature-toggle-store.js index 54aba77f1e..fdd4946f15 100644 --- a/test/fixtures/fake-feature-toggle-store.js +++ b/test/fixtures/fake-feature-toggle-store.js @@ -12,8 +12,18 @@ module.exports = () => { return Promise.reject(); } }, + hasFeature: name => { + const toggle = _features.find(f => f.name === name); + const archived = _archive.find(f => f.name === name); + if (toggle) { + return Promise.resolve({ name, archived: false }); + } else if (archived) { + return Promise.resolve({ name, archived: true }); + } else { + return Promise.reject(); + } + }, getFeatures: () => Promise.resolve(_features), - hasFeatureName: () => Promise.resolve(false), addFeature: feature => _features.push(feature), getArchivedFeatures: () => Promise.resolve(_archive), addArchivedFeature: feature => _archive.push(feature), From 2bdc05ae15947edc64782dd0eeb987207f605576 Mon Sep 17 00:00:00 2001 From: Paul Nelson Date: Sat, 20 Jan 2018 13:35:54 -0600 Subject: [PATCH 4/7] Add example and documentation around triggering login modal #234 --- docs/securing-unleash.md | 10 ++++++++++ examples/basic-auth-hook.js | 5 ++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/docs/securing-unleash.md b/docs/securing-unleash.md index 67a4214313..43a1dc3652 100644 --- a/docs/securing-unleash.md +++ b/docs/securing-unleash.md @@ -22,6 +22,16 @@ unleash.start({ }); ``` +Additionally, you can trigger the admin interfact to prompt the user to sign in by configuring your middleware to return a `401` status on +protected routes. The response body must contain a `message` and a `path` used to redirect the user to the proper login route. + +```json +{ + "message": "You must be logged in to use Unlseash", + "path": "/custom/login" +} +``` + Examples on custom authentication hooks: - [google-auth-hook.js](https://github.com/Unleash/unleash/blob/master/examples/google-auth-hook.js) - [basic-auth-hook.js](https://github.com/Unleash/unleash/blob/master/examples/basic-auth-hook.js) diff --git a/examples/basic-auth-hook.js b/examples/basic-auth-hook.js index 7d09b27278..ea0c88a765 100644 --- a/examples/basic-auth-hook.js +++ b/examples/basic-auth-hook.js @@ -16,7 +16,10 @@ function basicAuthentication(app) { return res .status('401') .set({ 'WWW-Authenticate': 'Basic realm="example"' }) - .end('access denied'); + .send({ + message: 'You must be authenticated to use Unleash', + path: '/custom/login', + }); } }); From b9c432d61758436c476871995e01d2267f44a08b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivar=20Conradi=20=C3=98sthus?= Date: Tue, 23 Jan 2018 08:55:10 +0100 Subject: [PATCH 5/7] Fix typo --- docs/securing-unleash.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/securing-unleash.md b/docs/securing-unleash.md index 43a1dc3652..d7d2b6c735 100644 --- a/docs/securing-unleash.md +++ b/docs/securing-unleash.md @@ -27,7 +27,7 @@ protected routes. The response body must contain a `message` and a `path` used t ```json { - "message": "You must be logged in to use Unlseash", + "message": "You must be logged in to use Unleash", "path": "/custom/login" } ``` From 20e690bea2924543918008ee28cbee117c852130 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivar=20Conradi=20=C3=98sthus?= Date: Tue, 23 Jan 2018 08:56:04 +0100 Subject: [PATCH 6/7] Update basic-auth-hook.js --- examples/basic-auth-hook.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/examples/basic-auth-hook.js b/examples/basic-auth-hook.js index ea0c88a765..70ec49a3fe 100644 --- a/examples/basic-auth-hook.js +++ b/examples/basic-auth-hook.js @@ -16,10 +16,7 @@ function basicAuthentication(app) { return res .status('401') .set({ 'WWW-Authenticate': 'Basic realm="example"' }) - .send({ - message: 'You must be authenticated to use Unleash', - path: '/custom/login', - }); + .send('access denied'); } }); From 25ec01fe6f9c919291fed6e4e33240217b87313e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivar=20Conradi=20=C3=98sthus?= Date: Tue, 23 Jan 2018 08:56:22 +0100 Subject: [PATCH 7/7] Update basic-auth-hook.js --- examples/basic-auth-hook.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/basic-auth-hook.js b/examples/basic-auth-hook.js index 70ec49a3fe..7d09b27278 100644 --- a/examples/basic-auth-hook.js +++ b/examples/basic-auth-hook.js @@ -16,7 +16,7 @@ function basicAuthentication(app) { return res .status('401') .set({ 'WWW-Authenticate': 'Basic realm="example"' }) - .send('access denied'); + .end('access denied'); } });