From cc1686339a756562f0bf71266c5f3b2c8e4a80fb Mon Sep 17 00:00:00 2001 From: Javier Brea Date: Mon, 7 Jan 2019 09:11:23 +0100 Subject: [PATCH 01/15] Add update user api resource --- lib/api/users.js | 30 +++++- lib/api/users.json | 77 ++++++++++++-- lib/commands/user.js | 5 + test/unit/lib/Commands.specs.js | 1 + test/unit/lib/api/users.specs.js | 149 +++++++++++++++++++++++++++ test/unit/lib/commands/User.mocks.js | 1 + test/unit/lib/commands/user.specs.js | 46 +++++++++ test/unit/lib/models/User.mocks.js | 1 + 8 files changed, 299 insertions(+), 11 deletions(-) diff --git a/lib/api/users.js b/lib/api/users.js index 241cddd..97a5aa9 100644 --- a/lib/api/users.js +++ b/lib/api/users.js @@ -6,6 +6,8 @@ const definition = require('./users.json') const { roles } = require('../security/utils') const events = require('../events') +const LOCATION = 'location' +const LOCATION_ROOT = '/api/users/' const EVENT_ENTITY = 'user' const Operations = (service, commands) => { @@ -31,22 +33,44 @@ const Operations = (service, commands) => { handler: (params, body, res) => commands.user.add(body) .then(user => { res.status(201) - res.header('location', `/api/users/${user._id}`) + res.header(LOCATION, `${LOCATION_ROOT}${user._id}`) events.plugin(EVENT_ENTITY, events.OPERATIONS.CREATE, user) return Promise.resolve() }) }, getUser: { + auth: (userData, params) => commands.user.getById(params.path.id) + .then(user => { + if (rolesBasedAuth(userData.role, user.role) || params.path.id === userData._id) { + return Promise.resolve() + } + return Promise.reject(new service.errors.Forbidden()) + }), + handler: (params, body, res) => commands.user.getById(params.path.id) + }, + updateUser: { auth: (userData, params, body) => { return commands.user.getById(params.path.id) .then(user => { - if (rolesBasedAuth(userData.role, user.role) || params.path.id === userData._id) { + if ( + // Only operators and admin users can be modified + [roles.ADMIN, roles.OPERATOR].includes(user.role) && + // Role can be only modified by administrators + (userData.role === roles.ADMIN || (params.path.id === userData._id && !body.role)) && + // Role can be modified only to operator or admin roles + (!body.role || [roles.ADMIN, roles.OPERATOR].includes(body.role)) + ) { return Promise.resolve() } return Promise.reject(new service.errors.Forbidden()) }) }, - handler: (params, body, res) => commands.user.getById(params.path.id) + handler: (params, body, res) => commands.user.update(params.path.id, body).then(user => { + res.status(204) + res.header(LOCATION, `${LOCATION_ROOT}${user._id}`) + events.plugin(EVENT_ENTITY, events.OPERATIONS.UPDATE, user) + return Promise.resolve() + }) }, getUserMe: { handler: (params, body, res, userData) => commands.user.getById(userData._id) diff --git a/lib/api/users.json b/lib/api/users.json index 10e48b5..33dc33f 100644 --- a/lib/api/users.json +++ b/lib/api/users.json @@ -47,8 +47,14 @@ "updatedAt": "2018-07-28T17:13:09.730Z" } }, + "Users": { + "type": "array", + "items": { + "$ref": "#/components/schemas/User" + } + }, "NewUser": { - "description": "User data", + "description": "New user data", "type": "object", "properties": { "name": { @@ -80,16 +86,30 @@ "password": "foopass" } }, - "Users": { - "type": "array", - "items": { - "$ref": "#/components/schemas/User" + "UpdateUser": { + "description": "Update user data", + "type": "object", + "properties": { + "password": { + "description": "User password", + "type": "string" + }, + "role": { + "description": "Role assigned to the user", + "type": "string", + "enum": ["admin", "operator", "module", "plugin", "service-registerer"] + } + }, + "additionalProperties": false, + "example": { + "role": "operator", + "password": "foopass" } } }, "responses": { - "NewUser": { - "description": "New user added", + "User": { + "description": "User added or modified", "headers": { "location": { "$ref": "#/components/headers/ContentLocation" @@ -160,7 +180,7 @@ }, "responses": { "201": { - "$ref": "#/components/responses/NewUser" + "$ref": "#/components/responses/User" } }, "security": [{ @@ -232,6 +252,47 @@ }, { "apiKey": [] }] + }, + "patch": { + "parameters": [ + { + "in": "path", + "name": "id", + "schema": { + "type": "string" + }, + "required": true, + "description": "User id" + } + ], + "tags": ["user"], + "summary": "Update user", + "description": "Update user", + "operationId": "updateUser", + "requestBody": { + "description": "Data of the user to be modified", + "required": true, + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/UpdateUser" + } + } + } + }, + "responses": { + "204": { + "$ref": "#/components/responses/User" + }, + "404": { + "$ref": "#/components/responses/NotFoundError" + } + }, + "security": [{ + "jwt": [] + }, { + "apiKey": [] + }] } } } diff --git a/lib/commands/user.js b/lib/commands/user.js index 083dcd0..1d8e73f 100644 --- a/lib/commands/user.js +++ b/lib/commands/user.js @@ -32,6 +32,10 @@ const Commands = (service, models, client) => { .catch(() => Promise.resolve(null)) .then(ensureUser) + const update = (_id, data) => models.User.findOneAndUpdate({ + _id + }, data, { runValidators: true, new: true }).catch(error => utils.transformValidationErrors(error, service)).then(ensureUser) + const remove = (filter = {}) => models.User.findOneAndRemove(filter).then(ensureUser) .then(user => service.tracer.debug(templates.userRemoved(user))) @@ -47,6 +51,7 @@ const Commands = (service, models, client) => { getFiltered, get, getById, + update, remove, init } diff --git a/test/unit/lib/Commands.specs.js b/test/unit/lib/Commands.specs.js index 91b9c8a..291ab85 100644 --- a/test/unit/lib/Commands.specs.js +++ b/test/unit/lib/Commands.specs.js @@ -32,6 +32,7 @@ test.describe('Commands', () => { 'getFiltered', 'get', 'getById', + 'update', 'init', 'remove' ) diff --git a/test/unit/lib/api/users.specs.js b/test/unit/lib/api/users.specs.js index f46e8a3..190c523 100644 --- a/test/unit/lib/api/users.specs.js +++ b/test/unit/lib/api/users.specs.js @@ -426,6 +426,155 @@ test.describe('users api', () => { }) }) }) + + test.describe('updateUser auth', () => { + const fooUser = { + _id: 'foo-user-id', + role: 'admin' + } + const fooParams = { + path: { + id: 'foo-user-id' + } + } + + test.it('should reject if target user has not operator or admin role', () => { + commandsMocks.stubs.user.getById.resolves({ + role: 'module' + }) + + return operations.updateUser.auth(fooUser, fooParams, {}).then(() => { + return test.assert.fail() + }, err => { + return test.expect(err).to.be.an.instanceof(Error) + }) + }) + + test.it('should reject if user is not admin and wants to update role', () => { + commandsMocks.stubs.user.getById.resolves({ + role: 'operator' + }) + + return operations.updateUser.auth({ + ...fooUser, + role: 'operator' + }, fooParams, { + role: 'admin' + }).then(() => { + return test.assert.fail() + }, err => { + return test.expect(err).to.be.an.instanceof(Error) + }) + }) + + test.it('should reject if user is admin and wants to update to a role different to admin or operator', () => { + commandsMocks.stubs.user.getById.resolves({ + role: 'operator' + }) + + return operations.updateUser.auth(fooUser, fooParams, { + role: 'module' + }).then(() => { + return test.assert.fail() + }, err => { + return test.expect(err).to.be.an.instanceof(Error) + }) + }) + + test.it('should resolve if user is admin and wants to update to a role admin or operator', () => { + commandsMocks.stubs.user.getById.resolves({ + role: 'operator' + }) + + return operations.updateUser.auth(fooUser, fooParams, { + role: 'admin' + }).then(() => { + return test.expect(true).to.be.true() + }) + }) + + test.it('should resolve if logged user is editing himself and wants to update data different to role', () => { + commandsMocks.stubs.user.getById.resolves({ + _id: 'foo-user-id', + role: 'operator' + }) + + return operations.updateUser.auth({ + ...fooUser, + role: 'operator' + }, fooParams, { + password: 'foo' + }).then(() => { + return test.expect(true).to.be.true() + }) + }) + }) + + test.describe('updateUser handler', () => { + const fooId = 'foo-id' + const fooUser = { + _id: fooId, + name: 'foo-name' + } + const fooBody = { + password: 'foo' + } + const fooParams = { + path: { + id: fooId + } + } + let sandbox + let response + + test.beforeEach(() => { + sandbox = test.sinon.createSandbox() + response = { + status: sandbox.stub(), + header: sandbox.stub() + } + commandsMocks.stubs.user.update.resolves(fooUser) + }) + + test.afterEach(() => { + sandbox.restore() + }) + + test.it('should call to update user, passing the received body and id', () => { + return operations.updateUser.handler(fooParams, fooBody, response) + .then((result) => { + return test.expect(commandsMocks.stubs.user.update).to.have.been.calledWith(fooId, fooBody) + }) + }) + + test.it('should add a 204 header to response', () => { + return operations.updateUser.handler(fooParams, fooBody, response) + .then(() => { + return test.expect(response.status).to.have.been.calledWith(204) + }) + }) + + test.it('should set the response header with the user id', () => { + return operations.updateUser.handler(fooParams, fooBody, response) + .then(() => { + return test.expect(response.header).to.have.been.calledWith('location', '/api/users/foo-id') + }) + }) + + test.it('should emit a plugin event', () => { + return operations.updateUser.handler(fooParams, fooBody, response) + .then(() => { + return test.expect(eventsMocks.stubs.plugin).to.have.been.calledWith('user', 'updated', fooUser) + }) + }) + + test.it('should resolve the promise with no value', () => { + return operations.updateUser.handler(fooParams, fooBody, response) + .then((result) => { + return test.expect(result).to.be.undefined() + }) + }) + }) }) test.describe('openapi', () => { diff --git a/test/unit/lib/commands/User.mocks.js b/test/unit/lib/commands/User.mocks.js index c2989c9..8673988 100644 --- a/test/unit/lib/commands/User.mocks.js +++ b/test/unit/lib/commands/User.mocks.js @@ -12,6 +12,7 @@ const Mock = function () { getFiltered: sandbox.stub().usingPromise().resolves(), get: sandbox.stub().usingPromise().resolves(), getById: sandbox.stub().usingPromise().resolves(), + update: sandbox.stub().usingPromise().resolves(), remove: sandbox.stub().usingPromise().resolves(), init: sandbox.stub().usingPromise().resolves() } diff --git a/test/unit/lib/commands/user.specs.js b/test/unit/lib/commands/user.specs.js index 2f64b52..8b9aa57 100644 --- a/test/unit/lib/commands/user.specs.js +++ b/test/unit/lib/commands/user.specs.js @@ -176,6 +176,52 @@ test.describe('user commands', () => { }) }) + test.describe('update method', () => { + const fooId = 'foo-id' + const fooData = {role: 'foo-role'} + + test.it('should call to user model findOneAndUpdate method, and return the result', () => { + const fooResult = 'foo' + modelsMocks.stubs.User.findOneAndUpdate.resolves(fooResult) + return commands.update(fooId, fooData) + .then((result) => { + return Promise.all([ + test.expect(result).to.equal(fooResult), + test.expect(modelsMocks.stubs.User.findOneAndUpdate).to.have.been.calledWith({ + _id: fooId + }, fooData) + ]) + }) + }) + + test.it('should call to transform the received error if updating user fails', () => { + let updateError = new Error('update error') + modelsMocks.stubs.User.findOneAndUpdate.rejects(updateError) + utilMocks.stubs.transformValidationErrors.rejects(updateError) + return commands.update(fooId, fooData) + .then(() => { + return test.assert.fail() + }, (err) => { + return Promise.all([ + test.expect(err).to.equal(updateError), + test.expect(utilMocks.stubs.transformValidationErrors).to.have.been.calledWith(updateError) + ]) + }) + }) + + test.it('should return a not found error if no user is found', () => { + const fooError = new Error('foo error') + modelsMocks.stubs.User.findOneAndUpdate.resolves(null) + baseMocks.stubs.service.errors.NotFound.returns(fooError) + return commands.update(fooId, fooData) + .then(() => { + return test.assert.fail() + }, err => { + return test.expect(err).to.equal(fooError) + }) + }) + }) + test.describe('remove method', () => { const fooFilter = {name: 'foo-name'} diff --git a/test/unit/lib/models/User.mocks.js b/test/unit/lib/models/User.mocks.js index 3b0bbd2..9118787 100644 --- a/test/unit/lib/models/User.mocks.js +++ b/test/unit/lib/models/User.mocks.js @@ -15,6 +15,7 @@ const Mock = function () { UserStub.find = sandbox.stub().usingPromise().resolves() UserStub.findById = sandbox.stub().usingPromise().resolves() UserStub.findOneAndRemove = sandbox.stub().usingPromise().resolves() + UserStub.findOneAndUpdate = sandbox.stub().usingPromise().resolves() const stubs = { user: userStub, From e7a37981235fb09a8747eaec9ea456d83e3df8fe Mon Sep 17 00:00:00 2001 From: Javier Brea Date: Mon, 7 Jan 2019 09:41:52 +0100 Subject: [PATCH 02/15] Add update user functional tests --- test/functional/specs/users-api.specs.js | 157 +++++++++++++++++++++++ 1 file changed, 157 insertions(+) diff --git a/test/functional/specs/users-api.specs.js b/test/functional/specs/users-api.specs.js index c7a8251..d789a04 100644 --- a/test/functional/specs/users-api.specs.js +++ b/test/functional/specs/users-api.specs.js @@ -24,6 +24,14 @@ test.describe('users api', function () { }) } + const updateUser = function (userId, body) { + return utils.request(`/users/${userId}`, { + method: 'PATCH', + body, + ...authenticator.credentials() + }) + } + const getUserMe = function () { return utils.request(`/users/me`, { method: 'GET', @@ -392,6 +400,155 @@ test.describe('users api', function () { testRole(pluginUser, serviceUser) testRole(serviceRegistererUser, newUser) + test.describe('update user', () => { + let operatorUserId + let moduleUserId + let pluginUserId + + test.before(() => { + return utils.doLogin(authenticator) + .then(() => { + return getUsers().then(usersResponse => { + operatorUserId = usersResponse.body.find(userData => userData.role === 'operator')._id + moduleUserId = usersResponse.body.find(userData => userData.role === 'module')._id + pluginUserId = usersResponse.body.find(userData => userData.role === 'plugin')._id + return Promise.resolve() + }) + }) + }) + + test.describe('when user is admin', () => { + test.it('should return a bad data error when trying to update email', () => { + return updateUser(adminUserId, { + role: 'admin', + email: 'foo-other-email@foo.com' + }).then(response => { + return Promise.all([ + test.expect(response.body.message).to.contain('"email" exists in instance when not allowed'), + test.expect(response.statusCode).to.equal(422) + ]) + }) + }) + + test.it('should be able to update self data, including role', () => { + return updateUser(adminUserId, { + role: 'admin' + }).then(response => { + return test.expect(response.statusCode).to.equal(204) + }) + }) + + test.it('should be able to update data of operator users, including role', () => { + return updateUser(operatorUserId, { + password: 'foo', + role: 'operator' + }).then(response => { + return test.expect(response.statusCode).to.equal(204) + }) + }) + + test.it('should not be able to update data of module users', () => { + return updateUser(moduleUserId, { + password: 'foo' + }).then(response => { + return Promise.all([ + test.expect(response.body.message).to.contain('Not authorized'), + test.expect(response.statusCode).to.equal(403) + ]) + }) + }) + + test.it('should not be able to update data of plugin users', () => { + return updateUser(pluginUserId, { + password: 'foo' + }).then(response => { + return Promise.all([ + test.expect(response.body.message).to.contain('Not authorized'), + test.expect(response.statusCode).to.equal(403) + ]) + }) + }) + }) + + test.describe('when user is operator', () => { + test.before(() => { + return utils.ensureUserAndDoLogin(authenticator, operatorUser) + }) + + test.it('should be able to update self data', () => { + return updateUser(operatorUserId, { + password: 'foo' + }).then(response => { + return test.expect(response.statusCode).to.equal(204) + }) + }) + + test.it('should not be able to update self role', () => { + return updateUser(operatorUserId, { + password: 'foo', + role: 'admin' + }).then(response => { + return Promise.all([ + test.expect(response.body.message).to.contain('Not authorized'), + test.expect(response.statusCode).to.equal(403) + ]) + }) + }) + + test.it('should not be able to update data of module users', () => { + return updateUser(moduleUserId, { + password: 'foo', + role: 'admin' + }).then(response => { + return Promise.all([ + test.expect(response.body.message).to.contain('Not authorized'), + test.expect(response.statusCode).to.equal(403) + ]) + }) + }) + + test.it('should not be able to update data of plugin users', () => { + return updateUser(pluginUserId, { + password: 'foo', + role: 'admin' + }).then(response => { + return Promise.all([ + test.expect(response.body.message).to.contain('Not authorized'), + test.expect(response.statusCode).to.equal(403) + ]) + }) + }) + }) + + test.describe('when user is module', () => { + test.before(() => { + return utils.ensureUserAndDoLogin(authenticator, operatorUser) + }) + + test.it('should not be able to update self data', () => { + return updateUser(moduleUserId, { + password: 'foo' + }).then(response => { + return Promise.all([ + test.expect(response.body.message).to.contain('Not authorized'), + test.expect(response.statusCode).to.equal(403) + ]) + }) + }) + + test.it('should not be able to update other users data', () => { + return updateUser(pluginUserId, { + password: 'foo' + }).then(response => { + return Promise.all([ + test.expect(response.body.message).to.contain('Not authorized'), + test.expect(response.statusCode).to.equal(403) + ]) + }) + }) + }) + }) + test.describe('when user has role "plugin"', () => { let operatorUserId const newOperatorUser = { From ec33eee499dfc331bcdf6ea26de700ae46c9dbcc Mon Sep 17 00:00:00 2001 From: Javier Brea Date: Mon, 7 Jan 2019 11:06:43 +0100 Subject: [PATCH 03/15] Add user and service delete methods --- lib/api/services.js | 12 +++++++- lib/api/services.json | 30 +++++++++++++++++++ lib/api/users.js | 12 +++++++- lib/api/users.json | 30 +++++++++++++++++++ lib/commands/ability.js | 5 +++- lib/commands/composed.js | 25 +++++++++++++++- lib/commands/securityToken.js | 5 +++- lib/commands/service.js | 7 ++++- lib/commands/servicePluginConfig.js | 5 +++- lib/templates.js | 3 +- test/unit/lib/Commands.specs.js | 16 ++++++---- test/unit/lib/commands/Ability.mocks.js | 3 +- test/unit/lib/commands/Composed.mocks.js | 4 ++- test/unit/lib/commands/SecurityToken.mocks.js | 3 +- test/unit/lib/commands/Service.mocks.js | 3 +- .../lib/commands/ServicePluginConfig.mocks.js | 3 +- 16 files changed, 148 insertions(+), 18 deletions(-) diff --git a/lib/api/services.js b/lib/api/services.js index 6f19192..901bbc3 100644 --- a/lib/api/services.js +++ b/lib/api/services.js @@ -3,7 +3,7 @@ const { omitBy, isUndefined } = require('lodash') const definition = require('./services.json') -const { roles } = require('../security/utils') +const { roles, onlyAdmin } = require('../security/utils') const events = require('../events') const EVENT_ENTITY = 'service' @@ -44,6 +44,16 @@ const Operations = (service, commands) => ({ events.plugin(EVENT_ENTITY, events.OPERATIONS.CREATE, serviceData) return Promise.resolve() })) + }, + deleteService: { + auth: onlyAdmin, + handler: (params, body, res) => commands.composed.removeService(params.path.id).then(() => { + res.status(204) + events.plugin(EVENT_ENTITY, events.OPERATIONS.DELETE, { + _id: params.path.id + }) + return Promise.resolve() + }) } }) diff --git a/lib/api/services.json b/lib/api/services.json index a25ff87..3b47030 100644 --- a/lib/api/services.json +++ b/lib/api/services.json @@ -320,6 +320,36 @@ }, { "apiKey": [] }] + }, + "delete": { + "parameters": [ + { + "in": "path", + "name": "id", + "schema": { + "type": "string" + }, + "required": true, + "description": "Service id" + } + ], + "tags": ["service"], + "summary": "Delete service", + "description": "Delete service", + "operationId": "deleteService", + "responses": { + "204": { + "$ref": "#/components/responses/Deleted" + }, + "404": { + "$ref": "#/components/responses/NotFoundError" + } + }, + "security": [{ + "jwt": [] + }, { + "apiKey": [] + }] } } } diff --git a/lib/api/users.js b/lib/api/users.js index 97a5aa9..d97eb62 100644 --- a/lib/api/users.js +++ b/lib/api/users.js @@ -3,7 +3,7 @@ const { omitBy, isUndefined } = require('lodash') const definition = require('./users.json') -const { roles } = require('../security/utils') +const { roles, onlyAdmin } = require('../security/utils') const events = require('../events') const LOCATION = 'location' @@ -72,6 +72,16 @@ const Operations = (service, commands) => { return Promise.resolve() }) }, + deleteUser: { + auth: onlyAdmin, + handler: (params, body, res) => commands.composed.removeUser(params.path.id).then(() => { + res.status(204) + events.plugin(EVENT_ENTITY, events.OPERATIONS.DELETE, { + _id: params.path.id + }) + return Promise.resolve() + }) + }, getUserMe: { handler: (params, body, res, userData) => commands.user.getById(userData._id) } diff --git a/lib/api/users.json b/lib/api/users.json index 33dc33f..7449a82 100644 --- a/lib/api/users.json +++ b/lib/api/users.json @@ -293,6 +293,36 @@ }, { "apiKey": [] }] + }, + "delete": { + "parameters": [ + { + "in": "path", + "name": "id", + "schema": { + "type": "string" + }, + "required": true, + "description": "User id" + } + ], + "tags": ["user"], + "summary": "Delete user", + "description": "Delete user", + "operationId": "deleteUser", + "responses": { + "204": { + "$ref": "#/components/responses/Deleted" + }, + "404": { + "$ref": "#/components/responses/NotFoundError" + } + }, + "security": [{ + "jwt": [] + }, { + "apiKey": [] + }] } } } diff --git a/lib/commands/ability.js b/lib/commands/ability.js index 65c6103..77b2a6a 100644 --- a/lib/commands/ability.js +++ b/lib/commands/ability.js @@ -103,6 +103,8 @@ const Commands = (service, models, client) => { return Promise.resolve(ability) }) + const findAndRemove = (filter = {}) => models.Ability.deleteMany(filter) + return { add, getFiltered, @@ -112,7 +114,8 @@ const Commands = (service, models, client) => { remove, validateAction, validateEvent, - validateState + validateState, + findAndRemove } } diff --git a/lib/commands/composed.js b/lib/commands/composed.js index f875c5e..346c235 100644 --- a/lib/commands/composed.js +++ b/lib/commands/composed.js @@ -65,13 +65,36 @@ const Commands = (service, models, client, commands) => { return Promise.resolve(userData) } + const removeService = _id => { + const serviceQuery = { + _service: _id + } + return commands.service.remove(_id) + .then(() => commands.servicePluginConfig.findAndRemove(serviceQuery)) + .then(() => commands.ability.findAndRemove(serviceQuery)) + } + + const removeUser = _id => { + const userQuery = { + _user: _id + } + return commands.user.remove({ + _id + }) + .then(() => commands.securityToken.findAndRemove(userQuery)) + .then(() => commands.service.getFiltered(userQuery)) + .then(services => Promise.all(services.map(service => removeService(service._id)))) + } + return { initUsers, dispatchAbilityAction, triggerAbilityEvent, getAbilityState, getServiceOwner, - getAbilityOwner + getAbilityOwner, + removeService, + removeUser } } diff --git a/lib/commands/securityToken.js b/lib/commands/securityToken.js index 5fc9e32..840a9ab 100644 --- a/lib/commands/securityToken.js +++ b/lib/commands/securityToken.js @@ -40,12 +40,15 @@ const Commands = (service, models, client) => { token })))) + const findAndRemove = (filter = {}) => models.SecurityToken.deleteMany(filter) + return { add, remove, get, getFiltered, - getUser + getUser, + findAndRemove } } diff --git a/lib/commands/service.js b/lib/commands/service.js index 56aec3c..2de810d 100644 --- a/lib/commands/service.js +++ b/lib/commands/service.js @@ -37,12 +37,17 @@ const Commands = (service, models, client) => { _id }, data, { runValidators: true, context: 'query', new: true }).catch(error => utils.transformValidationErrors(error, service)).then(ensureService) + const remove = _id => models.Service.findOneAndRemove({ + _id + }).then(ensureService).then(serviceData => service.tracer.debug(templates.serviceRemoved(serviceData))) + return { add, getFiltered, get, getById, - update + update, + remove } } diff --git a/lib/commands/servicePluginConfig.js b/lib/commands/servicePluginConfig.js index dc43b20..0c8c431 100644 --- a/lib/commands/servicePluginConfig.js +++ b/lib/commands/servicePluginConfig.js @@ -42,12 +42,15 @@ const Commands = (service, models, client) => { _id }, data, { runValidators: true, context: 'query', new: true }).catch(error => utils.transformValidationErrors(error, service)).then(ensureServicePluginConfig) + const findAndRemove = (filter = {}) => models.ServicePluginConfig.deleteMany(filter) + return { add, getFiltered, get, getById, - update + update, + findAndRemove } } diff --git a/lib/templates.js b/lib/templates.js index ec102c8..79b48ce 100644 --- a/lib/templates.js +++ b/lib/templates.js @@ -16,7 +16,7 @@ const templates = { notValidUrl: '"{{value}}" is not a valid url', userAdded: 'New user with name "{{name}}" was added. Assigned id: "{{_id}}"', - userRemoved: 'User with name "{{name}}" was removed', + userRemoved: 'User with name "{{name}}" and id "{{_id}}" was removed', userNameAlreadyExists: 'User name already exists', userNotFound: 'User not found', emailAlreadyExists: 'Email already exists', @@ -43,6 +43,7 @@ const templates = { serviceUrlRequired: 'Service url required', serviceTypeRequired: 'Service type required', serviceUrlAlreadyExists: 'Service url already exists', + serviceRemoved: 'Service with name "{{name}}" and id "{{_id}}" was removed', serviceNotFound: 'Service not found', serviceAdded: 'New service with name "{{name}}" was added. Assigned id: "{{_id}}"', userHasNotRelatedService: 'User has not a related service', diff --git a/test/unit/lib/Commands.specs.js b/test/unit/lib/Commands.specs.js index 291ab85..32b6b61 100644 --- a/test/unit/lib/Commands.specs.js +++ b/test/unit/lib/Commands.specs.js @@ -44,7 +44,8 @@ test.describe('Commands', () => { 'getUser', 'getFiltered', 'remove', - 'get' + 'get', + 'findAndRemove' ) }) @@ -55,7 +56,9 @@ test.describe('Commands', () => { 'getAbilityState', 'triggerAbilityEvent', 'getServiceOwner', - 'getAbilityOwner' + 'getAbilityOwner', + 'removeService', + 'removeUser' ) }) @@ -65,7 +68,8 @@ test.describe('Commands', () => { 'getFiltered', 'get', 'getById', - 'update' + 'update', + 'remove' ) }) @@ -75,7 +79,8 @@ test.describe('Commands', () => { 'getFiltered', 'get', 'getById', - 'update' + 'update', + 'findAndRemove' ) }) @@ -89,7 +94,8 @@ test.describe('Commands', () => { 'remove', 'validateAction', 'validateState', - 'validateEvent' + 'validateEvent', + 'findAndRemove' ) }) diff --git a/test/unit/lib/commands/Ability.mocks.js b/test/unit/lib/commands/Ability.mocks.js index 227b8f2..e1b9d48 100644 --- a/test/unit/lib/commands/Ability.mocks.js +++ b/test/unit/lib/commands/Ability.mocks.js @@ -15,7 +15,8 @@ const Mock = function () { remove: sandbox.stub().usingPromise().resolves(), validateAction: sandbox.stub().usingPromise().resolves(), validateState: sandbox.stub().usingPromise().resolves(), - validateEvent: sandbox.stub().usingPromise().resolves() + validateEvent: sandbox.stub().usingPromise().resolves(), + findAndRemove: sandbox.stub().usingPromise().resolves() } const stubs = { diff --git a/test/unit/lib/commands/Composed.mocks.js b/test/unit/lib/commands/Composed.mocks.js index 0418152..2283606 100644 --- a/test/unit/lib/commands/Composed.mocks.js +++ b/test/unit/lib/commands/Composed.mocks.js @@ -12,7 +12,9 @@ const Mock = function () { getAbilityState: sandbox.stub().usingPromise().resolves(), triggerAbilityEvent: sandbox.stub().usingPromise().resolves(), getServiceOwner: sandbox.stub().usingPromise().resolves(), - getAbilityOwner: sandbox.stub().usingPromise().resolves() + getAbilityOwner: sandbox.stub().usingPromise().resolves(), + removeService: sandbox.stub().usingPromise().resolves(), + removeUser: sandbox.stub().usingPromise().resolves() } const stubs = { diff --git a/test/unit/lib/commands/SecurityToken.mocks.js b/test/unit/lib/commands/SecurityToken.mocks.js index 07daf71..59a146c 100644 --- a/test/unit/lib/commands/SecurityToken.mocks.js +++ b/test/unit/lib/commands/SecurityToken.mocks.js @@ -11,7 +11,8 @@ const Mock = function () { getUser: sandbox.stub().usingPromise().resolves(), getFiltered: sandbox.stub().usingPromise().resolves(), remove: sandbox.stub().usingPromise().resolves(), - get: sandbox.stub().usingPromise().resolves() + get: sandbox.stub().usingPromise().resolves(), + findAndRemove: sandbox.stub().usingPromise().resolves() } const stubs = { diff --git a/test/unit/lib/commands/Service.mocks.js b/test/unit/lib/commands/Service.mocks.js index e42be7b..86d8913 100644 --- a/test/unit/lib/commands/Service.mocks.js +++ b/test/unit/lib/commands/Service.mocks.js @@ -11,7 +11,8 @@ const Mock = function () { getFiltered: sandbox.stub().usingPromise().resolves(), get: sandbox.stub().usingPromise().resolves(), getById: sandbox.stub().usingPromise().resolves(), - update: sandbox.stub().usingPromise().resolves() + update: sandbox.stub().usingPromise().resolves(), + remove: sandbox.stub().usingPromise().resolves() } const stubs = { diff --git a/test/unit/lib/commands/ServicePluginConfig.mocks.js b/test/unit/lib/commands/ServicePluginConfig.mocks.js index 3abcb7b..cfe853e 100644 --- a/test/unit/lib/commands/ServicePluginConfig.mocks.js +++ b/test/unit/lib/commands/ServicePluginConfig.mocks.js @@ -11,7 +11,8 @@ const Mock = function () { getFiltered: sandbox.stub().usingPromise().resolves(), get: sandbox.stub().usingPromise().resolves(), getById: sandbox.stub().usingPromise().resolves(), - update: sandbox.stub().usingPromise().resolves() + update: sandbox.stub().usingPromise().resolves(), + findAndRemove: sandbox.stub().usingPromise().resolves() } const stubs = { From ef52de5487f9cd670d11ea399b1fbbc3078870b1 Mon Sep 17 00:00:00 2001 From: Javier Brea Date: Mon, 7 Jan 2019 12:10:58 +0100 Subject: [PATCH 04/15] Add unit tests --- lib/commands/ability.js | 2 +- lib/commands/securityToken.js | 2 +- lib/commands/servicePluginConfig.js | 2 +- test/unit/lib/api/services.specs.js | 64 ++++++++++++++++ test/unit/lib/api/users.specs.js | 53 +++++++++++++ test/unit/lib/commands/ability.specs.js | 15 ++++ test/unit/lib/commands/composed.specs.js | 75 ++++++++++++++++++- test/unit/lib/commands/securityToken.specs.js | 14 ++++ test/unit/lib/commands/service.specs.js | 27 +++++++ .../lib/commands/servicePluginConfig.specs.js | 15 ++++ test/unit/lib/models/Ability.mocks.js | 1 + test/unit/lib/models/SecurityToken.mocks.js | 1 + test/unit/lib/models/Service.mocks.js | 1 + .../lib/models/ServicePluginConfig.mocks.js | 1 + 14 files changed, 269 insertions(+), 4 deletions(-) diff --git a/lib/commands/ability.js b/lib/commands/ability.js index 77b2a6a..a9ec3d3 100644 --- a/lib/commands/ability.js +++ b/lib/commands/ability.js @@ -103,7 +103,7 @@ const Commands = (service, models, client) => { return Promise.resolve(ability) }) - const findAndRemove = (filter = {}) => models.Ability.deleteMany(filter) + const findAndRemove = filter => models.Ability.deleteMany(filter) return { add, diff --git a/lib/commands/securityToken.js b/lib/commands/securityToken.js index 840a9ab..d6fbb78 100644 --- a/lib/commands/securityToken.js +++ b/lib/commands/securityToken.js @@ -40,7 +40,7 @@ const Commands = (service, models, client) => { token })))) - const findAndRemove = (filter = {}) => models.SecurityToken.deleteMany(filter) + const findAndRemove = filter => models.SecurityToken.deleteMany(filter) return { add, diff --git a/lib/commands/servicePluginConfig.js b/lib/commands/servicePluginConfig.js index 0c8c431..5153efe 100644 --- a/lib/commands/servicePluginConfig.js +++ b/lib/commands/servicePluginConfig.js @@ -42,7 +42,7 @@ const Commands = (service, models, client) => { _id }, data, { runValidators: true, context: 'query', new: true }).catch(error => utils.transformValidationErrors(error, service)).then(ensureServicePluginConfig) - const findAndRemove = (filter = {}) => models.ServicePluginConfig.deleteMany(filter) + const findAndRemove = filter => models.ServicePluginConfig.deleteMany(filter) return { add, diff --git a/test/unit/lib/api/services.specs.js b/test/unit/lib/api/services.specs.js index f6366d0..221ae8b 100644 --- a/test/unit/lib/api/services.specs.js +++ b/test/unit/lib/api/services.specs.js @@ -301,6 +301,70 @@ test.describe('services api', () => { }) }) }) + + test.describe('deleteService handler', () => { + const fooId = 'foo-service-id' + let sandbox + let response + + test.beforeEach(() => { + sandbox = test.sinon.createSandbox() + response = { + status: sandbox.stub(), + header: sandbox.stub() + } + }) + + test.afterEach(() => { + sandbox.restore() + }) + + test.it('should call to delete service, passing the received id', () => { + return operations.deleteService.handler({ + path: { + id: fooId + } + }, null, response) + .then((result) => { + return test.expect(commandsMocks.stubs.composed.removeService).to.have.been.calledWith(fooId) + }) + }) + + test.it('should add a 204 header to response', () => { + return operations.deleteService.handler({ + path: { + id: fooId + } + }, null, response) + .then(() => { + return test.expect(response.status).to.have.been.calledWith(204) + }) + }) + + test.it('should emit a plugin event', () => { + return operations.deleteService.handler({ + path: { + id: fooId + } + }, null, response) + .then(() => { + return test.expect(eventsMocks.stubs.plugin).to.have.been.calledWith('service', 'deleted', { + _id: fooId + }) + }) + }) + + test.it('should resolve the promise with no value', () => { + return operations.deleteService.handler({ + path: { + id: fooId + } + }, null, response) + .then((result) => { + return test.expect(result).to.be.undefined() + }) + }) + }) }) test.describe('openapi', () => { diff --git a/test/unit/lib/api/users.specs.js b/test/unit/lib/api/users.specs.js index 190c523..17afaf5 100644 --- a/test/unit/lib/api/users.specs.js +++ b/test/unit/lib/api/users.specs.js @@ -575,6 +575,59 @@ test.describe('users api', () => { }) }) }) + + test.describe('deleteUser handler', () => { + const fooId = 'foo-id' + const fooParams = { + path: { + id: fooId + } + } + let sandbox + let response + + test.beforeEach(() => { + sandbox = test.sinon.createSandbox() + response = { + status: sandbox.stub(), + header: sandbox.stub() + } + }) + + test.afterEach(() => { + sandbox.restore() + }) + + test.it('should call to delete user, passing the received id', () => { + return operations.deleteUser.handler(fooParams, null, response) + .then((result) => { + return test.expect(commandsMocks.stubs.composed.removeUser).to.have.been.calledWith(fooId) + }) + }) + + test.it('should add a 204 header to response', () => { + return operations.deleteUser.handler(fooParams, null, response) + .then(() => { + return test.expect(response.status).to.have.been.calledWith(204) + }) + }) + + test.it('should emit a plugin event', () => { + return operations.deleteUser.handler(fooParams, null, response) + .then(() => { + return test.expect(eventsMocks.stubs.plugin).to.have.been.calledWith('user', 'deleted', { + _id: fooId + }) + }) + }) + + test.it('should resolve the promise with no value', () => { + return operations.deleteUser.handler(fooParams, null, response) + .then((result) => { + return test.expect(result).to.be.undefined() + }) + }) + }) }) test.describe('openapi', () => { diff --git a/test/unit/lib/commands/ability.specs.js b/test/unit/lib/commands/ability.specs.js index f221df2..34f5768 100644 --- a/test/unit/lib/commands/ability.specs.js +++ b/test/unit/lib/commands/ability.specs.js @@ -598,5 +598,20 @@ test.describe('ability commands', () => { }) }) }) + + test.describe('findAndRemove method', () => { + const fooFilter = { + _id: 'foo-id' + } + + test.it('should call to ability model deleteMany method', () => { + const fooResult = 'foo' + modelsMocks.stubs.Ability.deleteMany.resolves(fooResult) + return commands.findAndRemove(fooFilter) + .then((result) => { + return test.expect(modelsMocks.stubs.Ability.deleteMany).to.have.been.calledWith(fooFilter) + }) + }) + }) }) }) diff --git a/test/unit/lib/commands/composed.specs.js b/test/unit/lib/commands/composed.specs.js index 19a08b6..b305df4 100644 --- a/test/unit/lib/commands/composed.specs.js +++ b/test/unit/lib/commands/composed.specs.js @@ -17,6 +17,7 @@ test.describe('composed commands', () => { let abilityCommandsMocks let serviceCommandsMocks let logCommandsMocks + let servicePluginConfigCommandMocks test.beforeEach(() => { baseMocks = new mocks.Base() @@ -27,6 +28,7 @@ test.describe('composed commands', () => { securityTokenCommandsMocks = new mocks.commands.SecurityToken() abilityCommandsMocks = new mocks.commands.Ability() serviceCommandsMocks = new mocks.commands.Service() + servicePluginConfigCommandMocks = new mocks.commands.ServicePluginConfig() logCommandsMocks = new mocks.commands.Log() commands = composed.Commands(baseMocks.stubs.service, modelsMocks.stubs, clientMocks.stubs, { @@ -34,7 +36,8 @@ test.describe('composed commands', () => { securityToken: securityTokenCommandsMocks.stubs.commands, ability: abilityCommandsMocks.stubs.commands, service: serviceCommandsMocks.stubs.commands, - log: logCommandsMocks.stubs.commands + log: logCommandsMocks.stubs.commands, + servicePluginConfig: servicePluginConfigCommandMocks.stubs.commands }) }) @@ -48,6 +51,7 @@ test.describe('composed commands', () => { abilityCommandsMocks.restore() serviceCommandsMocks.restore() logCommandsMocks.restore() + servicePluginConfigCommandMocks.restore() }) test.describe('initUsers method', () => { @@ -342,5 +346,74 @@ test.describe('composed commands', () => { }) }) }) + + test.describe('removeService method', () => { + test.it('should call to remove service, servicePluginConfigs and abilities', () => { + const fooId = 'foo-id' + return commands.removeService(fooId).then(() => { + return Promise.all([ + test.expect(serviceCommandsMocks.stubs.commands.remove).to.have.been.calledWith(fooId), + test.expect(servicePluginConfigCommandMocks.stubs.commands.findAndRemove).to.have.been.calledWith({ + _service: fooId + }), + test.expect(abilityCommandsMocks.stubs.commands.findAndRemove).to.have.been.calledWith({ + _service: fooId + }) + ]) + }) + }) + }) + + test.describe('removeUser method', () => { + const fooId = 'foo-id' + const fooUserServices = [ + { + _id: 'foo-service-id-1' + }, + { + _id: 'foo-service-id-2' + } + ] + test.beforeEach(() => { + serviceCommandsMocks.stubs.commands.getFiltered.resolves(fooUserServices) + }) + + test.it('should call to remove user and user securityTokens', () => { + return commands.removeUser(fooId).then(() => { + return Promise.all([ + test.expect(userCommandsMocks.stubs.commands.remove).to.have.been.calledWith({ + _id: fooId + }), + test.expect(securityTokenCommandsMocks.stubs.commands.findAndRemove).to.have.been.calledWith({ + _user: fooId + }) + ]) + }) + }) + + test.it('should call to remove all services that belongs to user', () => { + return commands.removeUser(fooId).then(() => { + return Promise.all([ + test.expect(serviceCommandsMocks.stubs.commands.getFiltered).to.have.been.calledWith({ + _user: fooId + }), + test.expect(serviceCommandsMocks.stubs.commands.remove).to.have.been.calledWith(fooUserServices[0]._id), + test.expect(servicePluginConfigCommandMocks.stubs.commands.findAndRemove).to.have.been.calledWith({ + _service: fooUserServices[0]._id + }), + test.expect(abilityCommandsMocks.stubs.commands.findAndRemove).to.have.been.calledWith({ + _service: fooUserServices[0]._id + }), + test.expect(serviceCommandsMocks.stubs.commands.remove).to.have.been.calledWith(fooUserServices[1]._id), + test.expect(servicePluginConfigCommandMocks.stubs.commands.findAndRemove).to.have.been.calledWith({ + _service: fooUserServices[1]._id + }), + test.expect(abilityCommandsMocks.stubs.commands.findAndRemove).to.have.been.calledWith({ + _service: fooUserServices[1]._id + }) + ]) + }) + }) + }) }) }) diff --git a/test/unit/lib/commands/securityToken.specs.js b/test/unit/lib/commands/securityToken.specs.js index 2834911..e65497d 100644 --- a/test/unit/lib/commands/securityToken.specs.js +++ b/test/unit/lib/commands/securityToken.specs.js @@ -213,5 +213,19 @@ test.describe('securityToken commands', () => { }) }) }) + + test.describe('findAndRemove method', () => { + const fooFilter = { + _id: 'foo-id' + } + + test.it('should call to securityToken model deleteMany method', () => { + modelsMocks.stubs.SecurityToken.deleteMany.resolves({}) + return commands.findAndRemove(fooFilter) + .then((result) => { + return test.expect(modelsMocks.stubs.SecurityToken.deleteMany).to.have.been.calledWith(fooFilter) + }) + }) + }) }) }) diff --git a/test/unit/lib/commands/service.specs.js b/test/unit/lib/commands/service.specs.js index dd74c3d..6f5e133 100644 --- a/test/unit/lib/commands/service.specs.js +++ b/test/unit/lib/commands/service.specs.js @@ -245,5 +245,32 @@ test.describe('service commands', () => { }) }) }) + + test.describe('remove method', () => { + const fooId = 'foo-id' + + test.it('should call to service model findOneAndRemove method', () => { + const fooResult = 'foo' + modelsMocks.stubs.Service.findOneAndRemove.resolves(fooResult) + return commands.remove(fooId) + .then((result) => { + return test.expect(modelsMocks.stubs.Service.findOneAndRemove).to.have.been.calledWith({ + _id: fooId + }) + }) + }) + + test.it('should return a not found error if no service is found', () => { + const fooError = new Error('foo error') + modelsMocks.stubs.Service.findOneAndRemove.resolves(null) + baseMocks.stubs.service.errors.NotFound.returns(fooError) + return commands.remove(fooId) + .then(() => { + return test.assert.fail() + }, err => { + return test.expect(err).to.equal(fooError) + }) + }) + }) }) }) diff --git a/test/unit/lib/commands/servicePluginConfig.specs.js b/test/unit/lib/commands/servicePluginConfig.specs.js index 07f26ed..e39f52c 100644 --- a/test/unit/lib/commands/servicePluginConfig.specs.js +++ b/test/unit/lib/commands/servicePluginConfig.specs.js @@ -264,5 +264,20 @@ test.describe('service commands', () => { }) }) }) + + test.describe('findAndRemove method', () => { + const fooFilter = { + _id: 'foo-id' + } + + test.it('should call to servicePluginConfig model deleteMany method', () => { + const fooResult = 'foo' + modelsMocks.stubs.ServicePluginConfig.deleteMany.resolves(fooResult) + return commands.findAndRemove(fooFilter) + .then((result) => { + return test.expect(modelsMocks.stubs.ServicePluginConfig.deleteMany).to.have.been.calledWith(fooFilter) + }) + }) + }) }) }) diff --git a/test/unit/lib/models/Ability.mocks.js b/test/unit/lib/models/Ability.mocks.js index f242f68..de02729 100644 --- a/test/unit/lib/models/Ability.mocks.js +++ b/test/unit/lib/models/Ability.mocks.js @@ -16,6 +16,7 @@ const Mock = function () { AbilityStub.findById = sandbox.stub().usingPromise().resolves() AbilityStub.findOneAndUpdate = sandbox.stub().usingPromise().resolves() AbilityStub.findOneAndRemove = sandbox.stub().usingPromise().resolves() + AbilityStub.deleteMany = sandbox.stub().usingPromise().resolves() const stubs = { ability: abilityStub, diff --git a/test/unit/lib/models/SecurityToken.mocks.js b/test/unit/lib/models/SecurityToken.mocks.js index 3504a1e..3d96185 100644 --- a/test/unit/lib/models/SecurityToken.mocks.js +++ b/test/unit/lib/models/SecurityToken.mocks.js @@ -14,6 +14,7 @@ const Mock = function () { SecurityTokenStub.findOne = sandbox.stub().usingPromise().resolves() SecurityTokenStub.find = sandbox.stub().usingPromise().resolves() SecurityTokenStub.deleteOne = sandbox.stub().usingPromise().resolves() + SecurityTokenStub.deleteMany = sandbox.stub().usingPromise().resolves() const stubs = { securityToken: securityTokenStub, diff --git a/test/unit/lib/models/Service.mocks.js b/test/unit/lib/models/Service.mocks.js index 6830f45..127ac3c 100644 --- a/test/unit/lib/models/Service.mocks.js +++ b/test/unit/lib/models/Service.mocks.js @@ -15,6 +15,7 @@ const Mock = function () { ServiceStub.findOne = sandbox.stub().usingPromise().resolves() ServiceStub.findById = sandbox.stub().usingPromise().resolves() ServiceStub.findOneAndUpdate = sandbox.stub().usingPromise().resolves() + ServiceStub.findOneAndRemove = sandbox.stub().usingPromise().resolves() const stubs = { service: serviceStub, diff --git a/test/unit/lib/models/ServicePluginConfig.mocks.js b/test/unit/lib/models/ServicePluginConfig.mocks.js index 59dba84..100f1ae 100644 --- a/test/unit/lib/models/ServicePluginConfig.mocks.js +++ b/test/unit/lib/models/ServicePluginConfig.mocks.js @@ -15,6 +15,7 @@ const Mock = function () { ModelStub.findOne = sandbox.stub().usingPromise().resolves() ModelStub.findById = sandbox.stub().usingPromise().resolves() ModelStub.findOneAndUpdate = sandbox.stub().usingPromise().resolves() + ModelStub.deleteMany = sandbox.stub().usingPromise().resolves() const stubs = { servicePluginConfig: modelStub, From 98e37af23afe41eb14daa2f9d963fc418e6cd921 Mon Sep 17 00:00:00 2001 From: Javier Brea Date: Mon, 7 Jan 2019 13:02:21 +0100 Subject: [PATCH 05/15] Add delete user functional tests --- test/functional/specs/users-api.specs.js | 176 +++++++++++++++++++++++ 1 file changed, 176 insertions(+) diff --git a/test/functional/specs/users-api.specs.js b/test/functional/specs/users-api.specs.js index d789a04..c609768 100644 --- a/test/functional/specs/users-api.specs.js +++ b/test/functional/specs/users-api.specs.js @@ -4,6 +4,7 @@ const test = require('narval') const utils = require('./utils') test.describe('users api', function () { + this.timeout(10000) let authenticator = utils.Authenticator() let adminUserId let pluginId @@ -32,6 +33,13 @@ test.describe('users api', function () { }) } + const deleteUser = function (userId, body) { + return utils.request(`/users/${userId}`, { + method: 'DELETE', + ...authenticator.credentials() + }) + } + const getUserMe = function () { return utils.request(`/users/me`, { method: 'GET', @@ -39,6 +47,66 @@ test.describe('users api', function () { }) } + const getServices = function (filters) { + return utils.request('/services', { + method: 'GET', + query: filters, + ...authenticator.credentials() + }) + } + + const addService = function (serviceData) { + return utils.request('/services', { + method: 'POST', + body: serviceData, + ...authenticator.credentials() + }) + } + + const addAbility = function (abilityData) { + return utils.request('/abilities', { + method: 'POST', + body: abilityData, + ...authenticator.credentials() + }) + } + + const getAbilities = function (filters) { + return utils.request('/abilities', { + method: 'GET', + query: filters, + ...authenticator.credentials() + }) + } + + const getAccessToken = userData => { + return utils.getAccessToken(authenticator, userData) + } + + const getAuthTokens = filters => { + return utils.request('/auth/tokens', { + method: 'GET', + query: filters, + ...authenticator.credentials() + }) + } + + const addServicePluginConfig = servicePluginConfigData => { + return utils.request('/service-plugin-configs', { + method: 'POST', + body: servicePluginConfigData, + ...authenticator.credentials() + }) + } + + const getServicePluginConfigs = query => { + return utils.request(`/service-plugin-configs`, { + method: 'GET', + query, + ...authenticator.credentials() + }) + } + const newUser = { name: 'foo-service', role: 'operator', @@ -668,4 +736,112 @@ test.describe('users api', function () { testUser(pluginUser) testUser(serviceRegistererUser) }) + + test.describe('delete user', () => { + let serviceId + let userId + + test.before(() => { + return utils.doLogin(authenticator) + .then(() => { + return getUsers() + .then(response => { + userId = response.body.find(user => user.name === serviceUser.name)._id + return Promise.resolve() + }) + }) + .then(() => { + return utils.ensureUserAndDoLogin(authenticator, serviceUser) + }) + .then(() => { + return getAccessToken(serviceUser) + }) + .then(() => { + return addService({ + processId: 'foo-service-id', + description: 'foo-description', + package: 'foo-package', + version: '1.0.0', + apiKey: 'dasasfdfsdf423efwsfds', + url: 'https://192.168.1.1', + type: 'module' + }).then(response => { + serviceId = response.headers.location.split('/').pop() + return Promise.resolve() + }) + }) + .then(() => { + return addAbility({ + name: 'foo-ability-name' + }) + }) + .then(() => { + return addServicePluginConfig({ + _service: serviceId, + pluginPackageName: 'foo-plugin', + config: { + foo: 'foo-data' + } + }) + }) + }) + + test.describe('when user is not admin', () => { + test.it('should return a forbidden error', () => { + return deleteUser(userId).then(response => { + return test.expect(response.statusCode).to.equal(403) + }) + }) + }) + + test.describe('when user is admin', () => { + test.before(() => { + return utils.doLogin(authenticator) + }) + + test.it('should delete user and all related services, abilities, securityTokens and servicePluginConfigs', () => { + return Promise.all([ + getServices(), + getAbilities(), + getAuthTokens(), + getServicePluginConfigs() + ]).then(previousResults => { + return deleteUser(userId).then(response => { + return Promise.all([ + getServices(), + getAbilities(), + getAuthTokens(), + getServicePluginConfigs(), + getUsers() + ]).then(afterResults => { + const previousServices = previousResults[0].body.filter(service => service._user === userId) + const previousAbilities = previousResults[1].body.filter(ability => ability._service === serviceId) + const previousTokens = previousResults[2].body.filter(token => token._user === userId) + const previousServicesConfigs = previousResults[3].body.filter(serviceConfig => serviceConfig._service === serviceId) + + const afterServices = afterResults[0].body.filter(service => service._user === userId) + const afterAbilities = afterResults[1].body.filter(ability => ability._service === serviceId) + const afterTokens = afterResults[2].body.filter(token => token._user === userId) + const afterServicesConfigs = afterResults[3].body.filter(serviceConfig => serviceConfig._service === serviceId) + + const afterUser = afterResults[4].body.find(user => user.name === serviceUser.name) + + return Promise.all([ + test.expect(response.statusCode).to.equal(204), + test.expect(previousServices.length).to.be.above(0), + test.expect(previousAbilities.length).to.be.above(0), + test.expect(previousTokens.length).to.be.above(0), + test.expect(previousServicesConfigs.length).to.be.above(0), + test.expect(afterServices.length).to.equal(0), + test.expect(afterAbilities.length).to.equal(0), + test.expect(afterTokens.length).to.equal(0), + test.expect(afterServicesConfigs.length).to.equal(0), + test.expect(afterUser).to.be.undefined() + ]) + }) + }) + }) + }) + }) + }) }) From 0c7b8b26f91edb690b3059f4a7f67a1f38240431 Mon Sep 17 00:00:00 2001 From: Javier Brea Date: Mon, 7 Jan 2019 13:05:20 +0100 Subject: [PATCH 06/15] Add functional test for operator user deletion --- test/functional/specs/users-api.specs.js | 86 +++++++++++++----------- 1 file changed, 48 insertions(+), 38 deletions(-) diff --git a/test/functional/specs/users-api.specs.js b/test/functional/specs/users-api.specs.js index c609768..effd54f 100644 --- a/test/functional/specs/users-api.specs.js +++ b/test/functional/specs/users-api.specs.js @@ -9,6 +9,7 @@ test.describe('users api', function () { let adminUserId let pluginId let entityId + let operatorUserId const getUsers = function (query) { return utils.request('/users', { @@ -469,7 +470,6 @@ test.describe('users api', function () { testRole(serviceRegistererUser, newUser) test.describe('update user', () => { - let operatorUserId let moduleUserId let pluginUserId @@ -799,45 +799,55 @@ test.describe('users api', function () { return utils.doLogin(authenticator) }) - test.it('should delete user and all related services, abilities, securityTokens and servicePluginConfigs', () => { - return Promise.all([ - getServices(), - getAbilities(), - getAuthTokens(), - getServicePluginConfigs() - ]).then(previousResults => { - return deleteUser(userId).then(response => { - return Promise.all([ - getServices(), - getAbilities(), - getAuthTokens(), - getServicePluginConfigs(), - getUsers() - ]).then(afterResults => { - const previousServices = previousResults[0].body.filter(service => service._user === userId) - const previousAbilities = previousResults[1].body.filter(ability => ability._service === serviceId) - const previousTokens = previousResults[2].body.filter(token => token._user === userId) - const previousServicesConfigs = previousResults[3].body.filter(serviceConfig => serviceConfig._service === serviceId) - - const afterServices = afterResults[0].body.filter(service => service._user === userId) - const afterAbilities = afterResults[1].body.filter(ability => ability._service === serviceId) - const afterTokens = afterResults[2].body.filter(token => token._user === userId) - const afterServicesConfigs = afterResults[3].body.filter(serviceConfig => serviceConfig._service === serviceId) - - const afterUser = afterResults[4].body.find(user => user.name === serviceUser.name) + test.describe('when deleting an operator', () => { + test.it('should delete user', () => { + return deleteUser(operatorUserId).then(response => { + return test.expect(response.statusCode).to.equal(204) + }) + }) + }) + test.describe('when is deleting a service', () => { + test.it('should delete user and all related services, abilities, securityTokens and servicePluginConfigs', () => { + return Promise.all([ + getServices(), + getAbilities(), + getAuthTokens(), + getServicePluginConfigs() + ]).then(previousResults => { + return deleteUser(userId).then(response => { return Promise.all([ - test.expect(response.statusCode).to.equal(204), - test.expect(previousServices.length).to.be.above(0), - test.expect(previousAbilities.length).to.be.above(0), - test.expect(previousTokens.length).to.be.above(0), - test.expect(previousServicesConfigs.length).to.be.above(0), - test.expect(afterServices.length).to.equal(0), - test.expect(afterAbilities.length).to.equal(0), - test.expect(afterTokens.length).to.equal(0), - test.expect(afterServicesConfigs.length).to.equal(0), - test.expect(afterUser).to.be.undefined() - ]) + getServices(), + getAbilities(), + getAuthTokens(), + getServicePluginConfigs(), + getUsers() + ]).then(afterResults => { + const previousServices = previousResults[0].body.filter(service => service._user === userId) + const previousAbilities = previousResults[1].body.filter(ability => ability._service === serviceId) + const previousTokens = previousResults[2].body.filter(token => token._user === userId) + const previousServicesConfigs = previousResults[3].body.filter(serviceConfig => serviceConfig._service === serviceId) + + const afterServices = afterResults[0].body.filter(service => service._user === userId) + const afterAbilities = afterResults[1].body.filter(ability => ability._service === serviceId) + const afterTokens = afterResults[2].body.filter(token => token._user === userId) + const afterServicesConfigs = afterResults[3].body.filter(serviceConfig => serviceConfig._service === serviceId) + + const afterUser = afterResults[4].body.find(user => user.name === serviceUser.name) + + return Promise.all([ + test.expect(response.statusCode).to.equal(204), + test.expect(previousServices.length).to.be.above(0), + test.expect(previousAbilities.length).to.be.above(0), + test.expect(previousTokens.length).to.be.above(0), + test.expect(previousServicesConfigs.length).to.be.above(0), + test.expect(afterServices.length).to.equal(0), + test.expect(afterAbilities.length).to.equal(0), + test.expect(afterTokens.length).to.equal(0), + test.expect(afterServicesConfigs.length).to.equal(0), + test.expect(afterUser).to.be.undefined() + ]) + }) }) }) }) From 1c74636d3d503cc3e39c76f7bc66c605a33396c7 Mon Sep 17 00:00:00 2001 From: Javier Brea Date: Mon, 7 Jan 2019 18:53:51 +0100 Subject: [PATCH 07/15] Add service remove functional tests --- test/functional/specs/services-api.specs.js | 120 ++++++++++++++++++++ 1 file changed, 120 insertions(+) diff --git a/test/functional/specs/services-api.specs.js b/test/functional/specs/services-api.specs.js index 68402d3..42a4866 100644 --- a/test/functional/specs/services-api.specs.js +++ b/test/functional/specs/services-api.specs.js @@ -4,6 +4,7 @@ const test = require('narval') const utils = require('./utils') test.describe('services api', function () { + this.timeout(10000) let authenticator = utils.Authenticator() let pluginId let entityId @@ -39,6 +40,57 @@ test.describe('services api', function () { }) } + const addAbility = function (abilityData) { + return utils.request('/abilities', { + method: 'POST', + body: abilityData, + ...authenticator.credentials() + }) + } + + const getAbilities = function (filters) { + return utils.request('/abilities', { + method: 'GET', + query: filters, + ...authenticator.credentials() + }) + } + + const getAccessToken = userData => { + return utils.getAccessToken(authenticator, userData) + } + + const getAuthTokens = filters => { + return utils.request('/auth/tokens', { + method: 'GET', + query: filters, + ...authenticator.credentials() + }) + } + + const addServicePluginConfig = servicePluginConfigData => { + return utils.request('/service-plugin-configs', { + method: 'POST', + body: servicePluginConfigData, + ...authenticator.credentials() + }) + } + + const getServicePluginConfigs = query => { + return utils.request(`/service-plugin-configs`, { + method: 'GET', + query, + ...authenticator.credentials() + }) + } + + const deleteService = function (serviceId, body) { + return utils.request(`/services/${serviceId}`, { + method: 'DELETE', + ...authenticator.credentials() + }) + } + const adminUser = { name: 'foo-admin-2', role: 'admin', @@ -515,4 +567,72 @@ test.describe('services api', function () { testRole(adminUser) testRole(operatorUser) testRole(serviceRegistererUser) + + test.describe('delete service', () => { + test.before(() => { + return utils.ensureUserAndDoLogin(authenticator, moduleUser) + .then(() => { + return addAbility({ + name: 'foo-ability-name' + }) + }) + .then(() => { + return addServicePluginConfig({ + _service: entityId, + pluginPackageName: 'foo-plugin', + config: { + foo: 'foo-data' + } + }) + }) + }) + + test.describe('when user is not admin', () => { + test.it('should return a forbidden error', () => { + return deleteService(entityId).then(response => { + return test.expect(response.statusCode).to.equal(403) + }) + }) + }) + + test.describe('when user is admin', () => { + test.before(() => { + return utils.doLogin(authenticator) + }) + + test.it('should delete service and all related abilities and servicePluginConfigs', () => { + return Promise.all([ + getServices(), + getAbilities(), + getServicePluginConfigs() + ]).then(previousResults => { + return deleteService(entityId).then(response => { + return Promise.all([ + getServices(), + getAbilities(), + getServicePluginConfigs() + ]).then(afterResults => { + const previousServices = previousResults[0].body.filter(service => service._id === entityId) + const previousAbilities = previousResults[1].body.filter(ability => ability._service === entityId) + const previousServicesConfigs = previousResults[2].body.filter(serviceConfig => serviceConfig._service === entityId) + + const afterServices = afterResults[0].body.filter(service => service._id === entityId) + const afterAbilities = afterResults[1].body.filter(ability => ability._service === entityId) + const afterServicesConfigs = afterResults[2].body.filter(serviceConfig => serviceConfig._service === entityId) + + return Promise.all([ + test.expect(response.statusCode).to.equal(204), + test.expect(previousServices.length).to.be.above(0), + test.expect(previousAbilities.length).to.be.above(0), + test.expect(previousServicesConfigs.length).to.be.above(0), + test.expect(afterServices.length).to.equal(0), + test.expect(afterAbilities.length).to.equal(0), + test.expect(afterServicesConfigs.length).to.equal(0) + ]) + }) + }) + }) + }) + }) + }) }) From 97800a552c33958f50a3bbb336787a4ff655e33c Mon Sep 17 00:00:00 2001 From: Javier Brea Date: Mon, 7 Jan 2019 18:59:40 +0100 Subject: [PATCH 08/15] Fix unit tests --- test/functional/specs/services-api.specs.js | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/test/functional/specs/services-api.specs.js b/test/functional/specs/services-api.specs.js index 42a4866..a69ae00 100644 --- a/test/functional/specs/services-api.specs.js +++ b/test/functional/specs/services-api.specs.js @@ -56,18 +56,6 @@ test.describe('services api', function () { }) } - const getAccessToken = userData => { - return utils.getAccessToken(authenticator, userData) - } - - const getAuthTokens = filters => { - return utils.request('/auth/tokens', { - method: 'GET', - query: filters, - ...authenticator.credentials() - }) - } - const addServicePluginConfig = servicePluginConfigData => { return utils.request('/service-plugin-configs', { method: 'POST', From b0bcc4f3681f00fca6f89dd6ed891d4ace5ba692 Mon Sep 17 00:00:00 2001 From: Javier Brea Date: Mon, 7 Jan 2019 19:28:21 +0100 Subject: [PATCH 09/15] Add api resource for deleting service plugin configurations --- lib/api/servicePluginConfigs.js | 107 ++++++++++-------- lib/api/servicePluginConfigs.json | 30 +++++ lib/commands/servicePluginConfig.js | 4 + lib/templates.js | 3 +- test/unit/lib/Api.specs.js | 1 + test/unit/lib/Commands.specs.js | 1 + .../lib/api/ServicePluginConfigs.mocks.js | 3 + .../lib/commands/ServicePluginConfig.mocks.js | 1 + 8 files changed, 104 insertions(+), 46 deletions(-) diff --git a/lib/api/servicePluginConfigs.js b/lib/api/servicePluginConfigs.js index 0607ecb..a49c4d5 100644 --- a/lib/api/servicePluginConfigs.js +++ b/lib/api/servicePluginConfigs.js @@ -6,63 +6,80 @@ const definition = require('./servicePluginConfigs.json') const { roles } = require('../security/utils') const events = require('../events') +const LOCATION = 'location' +const LOCATION_ROOT = '/api/service-plugin-configs/' const EVENT_ENTITY = 'servicePluginConfig' -const Operations = (service, commands) => ({ - getServicePluginConfigs: { - handler: (params, body, res) => { - const filter = omitBy({ - _service: params.query.service, - pluginPackageName: params.query['plugin-package-name'] - }, isUndefined) - return commands.servicePluginConfig.getFiltered(filter) - } - }, - getServicePluginConfig: { - handler: (params, body, res) => commands.servicePluginConfig.getById(params.path.id) - }, - updateServicePluginConfig: { - auth: (userData, params, body) => { - if (userData.role === roles.PLUGIN || userData.role === roles.ADMIN) { - return Promise.resolve() - } - return commands.servicePluginConfig.getById(params.path.id).then(servicePluginConfigData => { - return commands.service.getById(servicePluginConfigData._service).then(serviceData => { - if (serviceData._user === userData._id) { - return Promise.resolve() - } - return Promise.reject(new service.errors.Forbidden()) - }) - }) - }, - handler: (params, body, res) => commands.servicePluginConfig.update(params.path.id, body).then(servicePluginConfigData => { - res.status(204) - res.header('location', `/api/service-plugin-configs/${servicePluginConfigData._id}`) - events.plugin(EVENT_ENTITY, events.OPERATIONS.UPDATE, servicePluginConfigData) +const Operations = (service, commands) => { + const adminPluginOrOwner = (userData, params, body) => { + if (userData.role === roles.PLUGIN || userData.role === roles.ADMIN) { return Promise.resolve() - }) - }, - addServicePluginConfig: { - auth: (userData, params, body) => { - if (userData.role === roles.PLUGIN || userData.role === roles.ADMIN) { - return Promise.resolve() - } - return commands.service.getById(body._service).then(serviceData => { + } + return commands.servicePluginConfig.getById(params.path.id).then(servicePluginConfigData => { + return commands.service.getById(servicePluginConfigData._service).then(serviceData => { if (serviceData._user === userData._id) { return Promise.resolve() } return Promise.reject(new service.errors.Forbidden()) }) + }) + } + + return { + getServicePluginConfigs: { + handler: (params, body, res) => { + const filter = omitBy({ + _service: params.query.service, + pluginPackageName: params.query['plugin-package-name'] + }, isUndefined) + return commands.servicePluginConfig.getFiltered(filter) + } + }, + getServicePluginConfig: { + handler: (params, body, res) => commands.servicePluginConfig.getById(params.path.id) }, - handler: (params, body, res, userData) => commands.servicePluginConfig.add(body) - .then(servicePluginConfigData => { - res.status(201) - res.header('location', `/api/service-plugin-configs/${servicePluginConfigData._id}`) - events.plugin(EVENT_ENTITY, events.OPERATIONS.CREATE, servicePluginConfigData) + updateServicePluginConfig: { + auth: adminPluginOrOwner, + handler: (params, body, res) => commands.servicePluginConfig.update(params.path.id, body).then(servicePluginConfigData => { + res.status(204) + res.header(LOCATION, `${LOCATION_ROOT}${servicePluginConfigData._id}`) + events.plugin(EVENT_ENTITY, events.OPERATIONS.UPDATE, servicePluginConfigData) return Promise.resolve() }) + }, + addServicePluginConfig: { + auth: (userData, params, body) => { + if (userData.role === roles.PLUGIN || userData.role === roles.ADMIN) { + return Promise.resolve() + } + return commands.service.getById(body._service).then(serviceData => { + if (serviceData._user === userData._id) { + return Promise.resolve() + } + return Promise.reject(new service.errors.Forbidden()) + }) + }, + handler: (params, body, res, userData) => commands.servicePluginConfig.add(body) + .then(servicePluginConfigData => { + res.status(201) + res.header(LOCATION, `${LOCATION_ROOT}${servicePluginConfigData._id}`) + events.plugin(EVENT_ENTITY, events.OPERATIONS.CREATE, servicePluginConfigData) + return Promise.resolve() + }) + }, + deleteServicePluginConfig: { + auth: adminPluginOrOwner, + handler: (params, body, res, userData) => commands.servicePluginConfig.remove(params.path.id) + .then(servicePluginConfigData => { + res.status(204) + events.plugin(EVENT_ENTITY, events.OPERATIONS.DELETE, { + _id: params.path.id + }) + return Promise.resolve() + }) + } } -}) +} const openapi = () => [definition] diff --git a/lib/api/servicePluginConfigs.json b/lib/api/servicePluginConfigs.json index e33d8d3..0388904 100644 --- a/lib/api/servicePluginConfigs.json +++ b/lib/api/servicePluginConfigs.json @@ -284,6 +284,36 @@ }, { "apiKey": [] }] + }, + "delete": { + "parameters": [ + { + "in": "path", + "name": "id", + "schema": { + "type": "string" + }, + "required": true, + "description": "Service plugin configuration id" + } + ], + "tags": ["service"], + "summary": "Delete service plugin configuration", + "description": "Delete service plugin configuration", + "operationId": "deleteServicePluginConfiguration", + "responses": { + "204": { + "$ref": "#/components/responses/Deleted" + }, + "404": { + "$ref": "#/components/responses/NotFoundError" + } + }, + "security": [{ + "jwt": [] + }, { + "apiKey": [] + }] } } } diff --git a/lib/commands/servicePluginConfig.js b/lib/commands/servicePluginConfig.js index 5153efe..880e6f2 100644 --- a/lib/commands/servicePluginConfig.js +++ b/lib/commands/servicePluginConfig.js @@ -42,6 +42,9 @@ const Commands = (service, models, client) => { _id }, data, { runValidators: true, context: 'query', new: true }).catch(error => utils.transformValidationErrors(error, service)).then(ensureServicePluginConfig) + const remove = (_id) => models.ServicePluginConfig.findOneAndRemove({ _id }).then(ensureServicePluginConfig) + .then(servicePluginConfig => service.tracer.debug(templates.servicePluginConfigRemoved(servicePluginConfig))) + const findAndRemove = filter => models.ServicePluginConfig.deleteMany(filter) return { @@ -50,6 +53,7 @@ const Commands = (service, models, client) => { get, getById, update, + remove, findAndRemove } } diff --git a/lib/templates.js b/lib/templates.js index 79b48ce..20b67a9 100644 --- a/lib/templates.js +++ b/lib/templates.js @@ -76,7 +76,8 @@ const templates = { pluginPackageNameRequired: 'Plugin package name is required', servicePluginConfigAlreadyExists: 'Configuration for provided plugin already exists for same service', servicePluginConfigNotFound: 'Service plugin configuration not found', - servicePluginConfigAdded: 'New service plugin configuration was added for service "{{_service}}" and plugin "{{pluginPackageName}}"' + servicePluginConfigAdded: 'New service plugin configuration was added for service "{{_service}}" and plugin "{{pluginPackageName}}"', + servicePluginConfigRemoved: 'ServicePluginConfig with id "{{_id}}" of service "{{_service}}" was removed' } module.exports = domapic.utils.templates.compile(templates) diff --git a/test/unit/lib/Api.specs.js b/test/unit/lib/Api.specs.js index 8a23923..c80b730 100644 --- a/test/unit/lib/Api.specs.js +++ b/test/unit/lib/Api.specs.js @@ -54,6 +54,7 @@ test.describe('Api', () => { 'getServicePluginConfig', 'addServicePluginConfig', 'updateServicePluginConfig', + 'deleteServicePluginConfig', 'getAbilities', 'getAbility', 'updateAbility', diff --git a/test/unit/lib/Commands.specs.js b/test/unit/lib/Commands.specs.js index 32b6b61..6264f43 100644 --- a/test/unit/lib/Commands.specs.js +++ b/test/unit/lib/Commands.specs.js @@ -80,6 +80,7 @@ test.describe('Commands', () => { 'get', 'getById', 'update', + 'remove', 'findAndRemove' ) }) diff --git a/test/unit/lib/api/ServicePluginConfigs.mocks.js b/test/unit/lib/api/ServicePluginConfigs.mocks.js index 4c5de74..87ab1e5 100644 --- a/test/unit/lib/api/ServicePluginConfigs.mocks.js +++ b/test/unit/lib/api/ServicePluginConfigs.mocks.js @@ -18,6 +18,9 @@ const Mock = function () { }, updateServicePluginConfig: { handler: sandbox.stub().usingPromise().resolves() + }, + deleteServicePluginConfig: { + handler: sandbox.stub().usingPromise().resolves() } } diff --git a/test/unit/lib/commands/ServicePluginConfig.mocks.js b/test/unit/lib/commands/ServicePluginConfig.mocks.js index cfe853e..bb2102d 100644 --- a/test/unit/lib/commands/ServicePluginConfig.mocks.js +++ b/test/unit/lib/commands/ServicePluginConfig.mocks.js @@ -12,6 +12,7 @@ const Mock = function () { get: sandbox.stub().usingPromise().resolves(), getById: sandbox.stub().usingPromise().resolves(), update: sandbox.stub().usingPromise().resolves(), + remove: sandbox.stub().usingPromise().resolves(), findAndRemove: sandbox.stub().usingPromise().resolves() } From dfae35360adfcfdb09b7d54b0ec38ff6a8b15364 Mon Sep 17 00:00:00 2001 From: Javier Brea Date: Mon, 7 Jan 2019 19:35:08 +0100 Subject: [PATCH 10/15] Add remove command unit tests --- lib/commands/servicePluginConfig.js | 2 +- .../lib/commands/servicePluginConfig.specs.js | 27 +++++++++++++++++++ .../lib/models/ServicePluginConfig.mocks.js | 1 + 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/lib/commands/servicePluginConfig.js b/lib/commands/servicePluginConfig.js index 880e6f2..4ae0f7d 100644 --- a/lib/commands/servicePluginConfig.js +++ b/lib/commands/servicePluginConfig.js @@ -42,7 +42,7 @@ const Commands = (service, models, client) => { _id }, data, { runValidators: true, context: 'query', new: true }).catch(error => utils.transformValidationErrors(error, service)).then(ensureServicePluginConfig) - const remove = (_id) => models.ServicePluginConfig.findOneAndRemove({ _id }).then(ensureServicePluginConfig) + const remove = _id => models.ServicePluginConfig.findOneAndRemove({ _id }).then(ensureServicePluginConfig) .then(servicePluginConfig => service.tracer.debug(templates.servicePluginConfigRemoved(servicePluginConfig))) const findAndRemove = filter => models.ServicePluginConfig.deleteMany(filter) diff --git a/test/unit/lib/commands/servicePluginConfig.specs.js b/test/unit/lib/commands/servicePluginConfig.specs.js index e39f52c..939f9f5 100644 --- a/test/unit/lib/commands/servicePluginConfig.specs.js +++ b/test/unit/lib/commands/servicePluginConfig.specs.js @@ -265,6 +265,33 @@ test.describe('service commands', () => { }) }) + test.describe('remove method', () => { + const fooId = 'foo-id' + + test.it('should call to servicePluginConfig model findOneAndRemove method', () => { + const fooResult = 'foo' + modelsMocks.stubs.ServicePluginConfig.findOneAndRemove.resolves(fooResult) + return commands.remove(fooId) + .then((result) => { + return test.expect(modelsMocks.stubs.ServicePluginConfig.findOneAndRemove).to.have.been.calledWith({ + _id: fooId + }) + }) + }) + + test.it('should return a not found error if no servicePluginConfig is found', () => { + const fooError = new Error('foo error') + modelsMocks.stubs.ServicePluginConfig.findOneAndRemove.resolves(null) + baseMocks.stubs.service.errors.NotFound.returns(fooError) + return commands.remove(fooId) + .then(() => { + return test.assert.fail() + }, err => { + return test.expect(err).to.equal(fooError) + }) + }) + }) + test.describe('findAndRemove method', () => { const fooFilter = { _id: 'foo-id' diff --git a/test/unit/lib/models/ServicePluginConfig.mocks.js b/test/unit/lib/models/ServicePluginConfig.mocks.js index 100f1ae..247b40c 100644 --- a/test/unit/lib/models/ServicePluginConfig.mocks.js +++ b/test/unit/lib/models/ServicePluginConfig.mocks.js @@ -15,6 +15,7 @@ const Mock = function () { ModelStub.findOne = sandbox.stub().usingPromise().resolves() ModelStub.findById = sandbox.stub().usingPromise().resolves() ModelStub.findOneAndUpdate = sandbox.stub().usingPromise().resolves() + ModelStub.findOneAndRemove = sandbox.stub().usingPromise().resolves() ModelStub.deleteMany = sandbox.stub().usingPromise().resolves() const stubs = { From c2929acfdaabedfa24fa850e4d3b150b8dbe961e Mon Sep 17 00:00:00 2001 From: Javier Brea Date: Mon, 7 Jan 2019 19:43:34 +0100 Subject: [PATCH 11/15] Add delete servicePluginConfig handler and auth unit tests --- .../lib/api/servicePluginConfigs.specs.js | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/test/unit/lib/api/servicePluginConfigs.specs.js b/test/unit/lib/api/servicePluginConfigs.specs.js index ba75511..edbd73c 100644 --- a/test/unit/lib/api/servicePluginConfigs.specs.js +++ b/test/unit/lib/api/servicePluginConfigs.specs.js @@ -140,6 +140,7 @@ test.describe('servicePluginConfigs api', () => { testWriteAuth('addServicePluginConfig') testWriteAuth('updateServicePluginConfig') + testWriteAuth('deleteServicePluginConfig') test.describe('addServicePluginConfig auth', () => { test.it('should call to check the service owner with the provided service in body', () => { @@ -331,6 +332,60 @@ test.describe('servicePluginConfigs api', () => { }) }) }) + + test.describe('deleteServicePluginConfig handler', () => { + const fooId = 'foo-service-plugin-config-id' + const fooParams = { + path: { + id: fooId + } + } + let sandbox + let response + + test.beforeEach(() => { + sandbox = test.sinon.createSandbox() + response = { + status: sandbox.stub(), + header: sandbox.stub() + } + commandsMocks.stubs.servicePluginConfig.remove.resolves() + }) + + test.afterEach(() => { + sandbox.restore() + }) + + test.it('should call to update servicePluginConfig, passing the received name and body', () => { + return operations.deleteServicePluginConfig.handler(fooParams, null, response) + .then((result) => { + return test.expect(commandsMocks.stubs.servicePluginConfig.remove).to.have.been.calledWith(fooId) + }) + }) + + test.it('should add a 204 header to response', () => { + return operations.deleteServicePluginConfig.handler(fooParams, null, response) + .then(() => { + return test.expect(response.status).to.have.been.calledWith(204) + }) + }) + + test.it('should emit a plugin event', () => { + return operations.deleteServicePluginConfig.handler(fooParams, null, response) + .then(() => { + return test.expect(eventsMocks.stubs.plugin).to.have.been.calledWith('servicePluginConfig', 'deleted', { + _id: fooId + }) + }) + }) + + test.it('should resolve the promise with no value', () => { + return operations.deleteServicePluginConfig.handler(fooParams, null, response) + .then((result) => { + return test.expect(result).to.be.undefined() + }) + }) + }) }) test.describe('openapi', () => { From 3c38f5f999f1be8dc6d5450ba263cf015f5d8221 Mon Sep 17 00:00:00 2001 From: Javier Brea Date: Tue, 8 Jan 2019 19:39:56 +0100 Subject: [PATCH 12/15] Add delete service plugin configurations functional tests --- lib/api/servicePluginConfigs.json | 2 +- .../specs/service-plugin-configs-api.specs.js | 49 +++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/lib/api/servicePluginConfigs.json b/lib/api/servicePluginConfigs.json index 0388904..f5f127d 100644 --- a/lib/api/servicePluginConfigs.json +++ b/lib/api/servicePluginConfigs.json @@ -300,7 +300,7 @@ "tags": ["service"], "summary": "Delete service plugin configuration", "description": "Delete service plugin configuration", - "operationId": "deleteServicePluginConfiguration", + "operationId": "deleteServicePluginConfig", "responses": { "204": { "$ref": "#/components/responses/Deleted" diff --git a/test/functional/specs/service-plugin-configs-api.specs.js b/test/functional/specs/service-plugin-configs-api.specs.js index 013e49d..2e79544 100644 --- a/test/functional/specs/service-plugin-configs-api.specs.js +++ b/test/functional/specs/service-plugin-configs-api.specs.js @@ -8,6 +8,8 @@ test.describe('service-plugin-configs api', function () { let authenticator = utils.Authenticator() let serviceId let servicePluginConfigId + let servicePluginConfigId2 + let servicePluginConfigId3 const addServicePluginConfig = servicePluginConfigData => { return utils.request('/service-plugin-configs', { @@ -25,6 +27,13 @@ test.describe('service-plugin-configs api', function () { }) } + const deleteServicePluginConfig = id => { + return utils.request(`/service-plugin-configs/${id}`, { + method: 'DELETE', + ...authenticator.credentials() + }) + } + const getServicePluginConfigs = query => { return utils.request(`/service-plugin-configs`, { method: 'GET', @@ -151,6 +160,7 @@ test.describe('service-plugin-configs api', function () { foo: 'foo-data' } }).then(response => { + servicePluginConfigId2 = response.headers.location.split('/').pop() return test.expect(response.statusCode).to.equal(201) }) }) @@ -213,6 +223,7 @@ test.describe('service-plugin-configs api', function () { ] } }).then(response => { + servicePluginConfigId3 = response.headers.location.split('/').pop() return test.expect(response.statusCode).to.equal(201) }) }) @@ -289,5 +300,43 @@ test.describe('service-plugin-configs api', function () { }) }) }) + + test.describe('delete', () => { + test.it('should allow to delete if provided config belongs to logged user', () => { + return utils.ensureUserAndDoLogin(authenticator, moduleUser) + .then(() => { + return deleteServicePluginConfig(servicePluginConfigId).then(response => { + return test.expect(response.statusCode).to.equal(204) + }) + }) + }) + + test.it('should allow to delete if logged user is plugin', () => { + return utils.ensureUserAndDoLogin(authenticator, pluginUser) + .then(() => { + return deleteServicePluginConfig(servicePluginConfigId2).then(response => { + return test.expect(response.statusCode).to.equal(204) + }) + }) + }) + + test.it('should allow to delete if logged user is admin', () => { + return utils.ensureUserAndDoLogin(authenticator, adminUser) + .then(() => { + return deleteServicePluginConfig(servicePluginConfigId3).then(response => { + return test.expect(response.statusCode).to.equal(204) + }) + }) + }) + + test.it('should have really deleted configurations from database', () => { + return getServicePluginConfigs().then(response => { + return Promise.all([ + test.expect(response.statusCode).to.equal(200), + test.expect(response.body.length).to.equal(0) + ]) + }) + }) + }) }) }) From 03f9457507172fde1cd98d53499dfba13bdf7f9b Mon Sep 17 00:00:00 2001 From: Javier Brea Date: Tue, 8 Jan 2019 19:52:42 +0100 Subject: [PATCH 13/15] Upgrade version --- CHANGELOG.md | 6 ++++++ npm-shrinkwrap.json | 2 +- package.json | 2 +- sonar-project.properties | 2 +- 4 files changed, 9 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 73b65a5..f086b8f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,12 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Fixed ### Removed +## [1.0.0-beta.1] - 2019-01-08 +### Added +- Add DELETE method to servicePluginConfigs api +- Add DELETE method to services api +- Add PATCH and DELETE methods to users api + ## [1.0.0-alpha.14] - 2019-01-06 ### Added - Add anonymous default user, which will be used as logged user for requests with authentication disabled. When this user is logged in, services and abilities will be added to user with same name as service, not to logged user. In this way, the services connection process will work when authentication is disabled, and services registered will still be connected if authentication is enabled again. diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index 34d5b07..89c2023 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -1,6 +1,6 @@ { "name": "domapic-controller", - "version": "1.0.0-alpha.14", + "version": "1.0.0-beta.1", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index 82341f0..036f42a 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "domapic-controller", - "version": "1.0.0-alpha.14", + "version": "1.0.0-beta.1", "description": "Controller for Domapic systems", "main": "server.js", "keywords": [ diff --git a/sonar-project.properties b/sonar-project.properties index e516740..1a22d4c 100644 --- a/sonar-project.properties +++ b/sonar-project.properties @@ -1,6 +1,6 @@ sonar.organization=domapic sonar.projectKey=domapic-controller -sonar.projectVersion=1.0.0-alpha.14 +sonar.projectVersion=1.0.0-beta.1 sonar.sources=. sonar.exclusions=node_modules/** From 790625c4639b6399f2d3185e44d4b67ff209e0eb Mon Sep 17 00:00:00 2001 From: Javier Brea Date: Tue, 8 Jan 2019 19:53:45 +0100 Subject: [PATCH 14/15] Upgrade mongoose --- npm-shrinkwrap.json | 13 ++++++------- package.json | 2 +- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index 89c2023..03f8a8e 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -4236,9 +4236,9 @@ } }, "memory-pager": { - "version": "1.3.1", - "resolved": "https://registry.npmjs.org/memory-pager/-/memory-pager-1.3.1.tgz", - "integrity": "sha512-pUf/sGkym2WqFZYTVmdASnSbNfpGc9rwxEHOePx4lT/fD+NHGL1U16Uy4o6PMiVcDv4mp6MI/vaF0c/Kd1QEUQ==", + "version": "1.4.0", + "resolved": "https://registry.npmjs.org/memory-pager/-/memory-pager-1.4.0.tgz", + "integrity": "sha512-ycuyV5gKpZln7HB/A11wCpAxEY9VQ2EhYU1F56pUAxvmj6OyOHtB9tkLLjAyFsPdghSP2S3Ujk3aYJCusgiMZg==", "optional": true }, "merge-descriptors": { @@ -4513,14 +4513,13 @@ } }, "mongoose": { - "version": "5.4.0", - "resolved": "https://registry.npmjs.org/mongoose/-/mongoose-5.4.0.tgz", - "integrity": "sha512-pFKa6askJ6xwZT6mWuYBwa2R9ryd1+JrXUhKuAUxEGrUMTi8ADcJC/RgBg4fZ1lL6VPVVChsc9wpVn4X6gcWlg==", + "version": "5.4.2", + "resolved": "https://registry.npmjs.org/mongoose/-/mongoose-5.4.2.tgz", + "integrity": "sha512-bZFYe9oikiLZ2npfSbIGNYpJ0MJVQaSQhU09VFxvu6r7nVGixbt031ewZldKbWwHsG/JPJLX2HhUa8are84ryQ==", "requires": { "async": "2.6.1", "bson": "~1.1.0", "kareem": "2.3.0", - "lodash.get": "4.4.2", "mongodb": "3.1.10", "mongodb-core": "3.1.9", "mongoose-legacy-pluralize": "1.0.2", diff --git a/package.json b/package.json index 036f42a..31bcbb6 100644 --- a/package.json +++ b/package.json @@ -30,7 +30,7 @@ "isuri": "2.0.3", "jsonschema": "1.2.4", "md5": "2.2.1", - "mongoose": "5.4.0", + "mongoose": "5.4.2", "rand-token": "0.4.0" }, "devDependencies": { From 88a86c07b7b0945de5180f5d3331291957a5327a Mon Sep 17 00:00:00 2001 From: Javier Brea Date: Tue, 8 Jan 2019 19:54:55 +0100 Subject: [PATCH 15/15] Add change to changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f086b8f..adada84 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Add DELETE method to services api - Add PATCH and DELETE methods to users api +### Changed +- Upgrade mongoose version + ## [1.0.0-alpha.14] - 2019-01-06 ### Added - Add anonymous default user, which will be used as logged user for requests with authentication disabled. When this user is logged in, services and abilities will be added to user with same name as service, not to logged user. In this way, the services connection process will work when authentication is disabled, and services registered will still be connected if authentication is enabled again.