diff --git a/CHANGELOG.md b/CHANGELOG.md index 16097a26b6..4b19133650 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## [Unreleased] - Add sdkVersion in client registration +- disable edit of built-in strategies ## 3.0.0-alpha.1 - upgrade unleash-frontend to 3.0.0-alpha.1 diff --git a/lib/db/strategy-store.js b/lib/db/strategy-store.js index bee8e41fd3..6eaa15193b 100644 --- a/lib/db/strategy-store.js +++ b/lib/db/strategy-store.js @@ -7,7 +7,7 @@ const { } = require('../event-type'); const logger = require('../logger'); const NotFoundError = require('../error/notfound-error'); -const STRATEGY_COLUMNS = ['name', 'description', 'parameters']; +const STRATEGY_COLUMNS = ['name', 'description', 'parameters', 'built_in']; const TABLE = 'strategies'; class StrategyStore { @@ -46,9 +46,9 @@ class StrategyStore { if (!row) { throw new NotFoundError('No strategy found'); } - return { name: row.name, + editable: row.built_in !== 1, description: row.description, parameters: row.parameters, }; diff --git a/lib/routes/admin-api/strategy-schema.js b/lib/routes/admin-api/strategy-schema.js index ef910fe7c7..2b73711caf 100644 --- a/lib/routes/admin-api/strategy-schema.js +++ b/lib/routes/admin-api/strategy-schema.js @@ -4,6 +4,7 @@ const joi = require('joi'); const strategySchema = joi.object().keys({ name: joi.string().regex(/^[a-zA-Z0-9\\.\\-]{3,100}$/).required(), + editable: joi.boolean().default(true), description: joi.string(), parameters: joi.array().required().items( joi.object().keys({ diff --git a/lib/routes/admin-api/strategy.js b/lib/routes/admin-api/strategy.js index 9f19125032..7375946004 100644 --- a/lib/routes/admin-api/strategy.js +++ b/lib/routes/admin-api/strategy.js @@ -33,6 +33,28 @@ const handleError = (req, res, error) => { } }; +function validateEditable(strategyName) { + return strategy => { + if (strategy.editable === false) { + throw new Error( + `Cannot edit strategy ${strategyName}, editable is false` + ); + } + return strategy; + }; +} + +function validateInput(data) { + return new Promise((resolve, reject) => { + joi.validate(data, strategySchema, (err, cleaned) => { + if (err) { + return reject(err); + } + return resolve(cleaned); + }); + }); +} + exports.router = function(config) { const { strategyStore, eventStore } = config.stores; const router = Router(); @@ -57,6 +79,7 @@ exports.router = function(config) { strategyStore .getStrategy(strategyName) + .then(validateEditable(strategyName)) .then(() => eventStore.store({ type: eventType.STRATEGY_DELETED, @@ -70,6 +93,17 @@ exports.router = function(config) { .catch(error => handleError(req, res, error)); }); + function validateStrategyName(data) { + return new Promise((resolve, reject) => { + strategyStore + .getStrategy(data.name) + .then(() => + reject(new NameExistsError('Feature name already exist')) + ) + .catch(() => resolve(data)); + }); + } + router.post('/', (req, res) => { const data = req.body; validateInput(data) @@ -93,6 +127,7 @@ exports.router = function(config) { strategyStore .getStrategy(strategyName) + .then(validateEditable(strategyName)) .then(() => validateInput(updatedStrategy)) .then(() => eventStore.store({ @@ -105,27 +140,5 @@ exports.router = function(config) { .catch(error => handleError(req, res, error)); }); - function validateStrategyName(data) { - return new Promise((resolve, reject) => { - strategyStore - .getStrategy(data.name) - .then(() => - reject(new NameExistsError('Feature name already exist')) - ) - .catch(() => resolve(data)); - }); - } - - function validateInput(data) { - return new Promise((resolve, reject) => { - joi.validate(data, strategySchema, (err, cleaned) => { - if (err) { - return reject(err); - } - return resolve(cleaned); - }); - }); - } - return router; }; diff --git a/lib/routes/admin-api/strategy.test.js b/lib/routes/admin-api/strategy.test.js index 49375a33bb..f761c2f0ae 100644 --- a/lib/routes/admin-api/strategy.test.js +++ b/lib/routes/admin-api/strategy.test.js @@ -29,7 +29,7 @@ test.beforeEach(() => { logger.setLevel('FATAL'); }); -test('should add version numbers for /stategies', t => { +test('add version numbers for /stategies', t => { t.plan(1); const { request, base } = getSetup(); @@ -42,7 +42,7 @@ test('should add version numbers for /stategies', t => { }); }); -test('should require a name when creating a new stratey', t => { +test('require a name when creating a new stratey', t => { t.plan(1); const { request, base } = getSetup(); @@ -55,7 +55,7 @@ test('should require a name when creating a new stratey', t => { }); }); -test('should require parameters array when creating a new stratey', t => { +test('require parameters array when creating a new stratey', t => { t.plan(1); const { request, base } = getSetup(); @@ -68,7 +68,7 @@ test('should require parameters array when creating a new stratey', t => { }); }); -test('should create a new stratey with empty parameters', t => { +test('create a new stratey with empty parameters', t => { t.plan(0); const { request, base } = getSetup(); @@ -78,7 +78,7 @@ test('should create a new stratey with empty parameters', t => { .expect(201); }); -test('should not be possible to override name', t => { +test('not be possible to override name', t => { t.plan(0); const { request, base, strategyStore } = getSetup(); strategyStore.addStrategy({ name: 'Testing', parameters: [] }); @@ -89,7 +89,7 @@ test('should not be possible to override name', t => { .expect(403); }); -test('should update strategy', t => { +test('update strategy', t => { t.plan(0); const name = 'AnotherStrat'; const { request, base, strategyStore } = getSetup(); @@ -101,7 +101,7 @@ test('should update strategy', t => { .expect(200); }); -test('should not update uknown strategy', t => { +test('not update uknown strategy', t => { t.plan(0); const name = 'UnknownStrat'; const { request, base } = getSetup(); @@ -112,7 +112,7 @@ test('should not update uknown strategy', t => { .expect(404); }); -test('should validate format when updating strategy', t => { +test('validate format when updating strategy', t => { t.plan(0); const name = 'AnotherStrat'; const { request, base, strategyStore } = getSetup(); @@ -123,3 +123,46 @@ test('should validate format when updating strategy', t => { .send({}) .expect(400); }); + +test('editable=false will stop delete request', t => { + t.plan(0); + const name = 'default'; + const { request, base } = getSetup(); + + return request.delete(`${base}/api/admin/strategies/${name}`).expect(500); +}); + +test('editable=false will stop edit request', t => { + t.plan(0); + const name = 'default'; + const { request, base } = getSetup(); + + return request + .put(`${base}/api/admin/strategies/${name}`) + .send({ name, parameters: [] }) + .expect(500); +}); + +test('editable=true will allow delete request', t => { + t.plan(0); + const name = 'deleteStrat'; + const { request, base, strategyStore } = getSetup(); + strategyStore.addStrategy({ name, parameters: [] }); + + return request + .delete(`${base}/api/admin/strategies/${name}`) + .send({}) + .expect(200); +}); + +test('editable=true will allow edit request', t => { + t.plan(0); + const name = 'editStrat'; + const { request, base, strategyStore } = getSetup(); + strategyStore.addStrategy({ name, parameters: [] }); + + return request + .put(`${base}/api/admin/strategies/${name}`) + .send({ name, parameters: [] }) + .expect(200); +}); diff --git a/test/fixtures/fake-strategies-store.js b/test/fixtures/fake-strategies-store.js index 82bbc1f86d..5f0d2be847 100644 --- a/test/fixtures/fake-strategies-store.js +++ b/test/fixtures/fake-strategies-store.js @@ -3,7 +3,7 @@ const NotFoundError = require('../../lib/error/notfound-error'); module.exports = () => { - const _strategies = [{ name: 'default', parameters: {} }]; + const _strategies = [{ name: 'default', editable: false, parameters: {} }]; return { getStrategies: () => Promise.resolve(_strategies),