Skip to content

Commit

Permalink
Merge 8070091 into 4b2aa17
Browse files Browse the repository at this point in the history
  • Loading branch information
javierbrea committed May 2, 2019
2 parents 4b2aa17 + 8070091 commit 99607fb
Show file tree
Hide file tree
Showing 16 changed files with 233 additions and 21 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
### Fixed
### Removed

## [1.0.0-beta.3] - 2019-05-01
### Added
- Add user adminPermissions property, intended for grant admin permissions to plugin users when login using apiKey
- Avoid any other user than admin updating adminPermissions property

## [1.0.0-beta.2] - 2019-03-02
### Added
- Add web ui
Expand Down
12 changes: 7 additions & 5 deletions lib/api/users.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,14 @@ const Operations = (service, commands) => {
return commands.user.getById(params.path.id)
.then(user => {
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)) &&
// Only operators and admin users can be modified, or plugins in case adminPermissions is the uniq property being modified
([roles.ADMIN, roles.OPERATOR].includes(user.role) || (user.role === roles.PLUGIN && Object.keys(body).length === 1 && !isUndefined(body.adminPermissions))) &&
// Role and adminPermissions can be only modified by administrators
(userData.role === roles.ADMIN || (params.path.id === userData._id && !body.role && !body.adminPermissions)) &&
// Role can be modified only to operator or admin roles
(!body.role || [roles.ADMIN, roles.OPERATOR].includes(body.role))
(!body.role || [roles.ADMIN, roles.OPERATOR].includes(body.role)) &&
// adminPermissions can be modified only to plugin roles
(!body.adminPermissions || user.role === roles.PLUGIN)
) {
return Promise.resolve()
}
Expand Down
8 changes: 8 additions & 0 deletions lib/api/users.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@
"updatedAt": {
"description": "Last update date timestamp",
"type": "string"
},
"adminPermissions": {
"description": "Grant admin permissions to this user",
"type": "boolean"
}
},
"additionalProperties": false,
Expand Down Expand Up @@ -98,6 +102,10 @@
"description": "Role assigned to the user",
"type": "string",
"enum": ["admin", "operator", "module", "plugin", "service-registerer"]
},
"adminPermissions": {
"description": "Grant admin permissions to this user",
"type": "boolean"
}
},
"additionalProperties": false,
Expand Down
5 changes: 3 additions & 2 deletions lib/commands/securityToken.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,12 @@ const Commands = (service, models, client) => {
return Promise.resolve(token)
}

const add = (userData, type) => {
const add = (userData, type, creatorUser) => {
const token = new models.SecurityToken({
token: randToken.generate(64),
_user: userData._id,
type
type,
createdBy: creatorUser && creatorUser._id.toString()
})
return token.save()
.catch(error => utils.transformValidationErrors(error, service))
Expand Down
2 changes: 1 addition & 1 deletion lib/commands/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const templates = require('../templates')
const utils = require('../utils')
const { INITIAL_ADMIN_USER, SERVICE_REGISTERER_USER, ANONYMOUS_USER } = require('../security/utils')

const PUBLIC_FIELDS = 'name email role updatedAt createdAt'
const PUBLIC_FIELDS = 'name email role updatedAt createdAt adminPermissions'

const Commands = (service, models, client) => {
const ensureUser = user => {
Expand Down
3 changes: 3 additions & 0 deletions lib/models/securityToken.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ const Model = service => {
securityUtils.API_KEY,
securityUtils.JWT
]
},
createdBy: {
type: String
}
},
{
Expand Down
3 changes: 3 additions & 0 deletions lib/models/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ const Model = service => {
type: String,
required: [true, templates.userRoleRequired()],
enum: _.map(roles, role => role)
},
adminPermissions: {
type: Boolean
}
},
{
Expand Down
4 changes: 2 additions & 2 deletions lib/security/apiKey.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ const Methods = (service, commands) => {
return Promise.reject(new service.errors.Forbidden())
}

const authenticateHandler = (params, body) => commands.user.getById(body.user)
.then(user => commands.securityToken.add(user, utils.API_KEY)
const authenticateHandler = (params, body, res, userData) => commands.user.getById(body.user)
.then(user => commands.securityToken.add(user, utils.API_KEY, userData)
.then(securityToken => Promise.resolve(securityToken.token))
)

Expand Down
7 changes: 6 additions & 1 deletion lib/security/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,12 @@ const cleanUserData = userData => ({
})

const GetUserBySecurityToken = commands => securityToken => commands.securityToken.getUser(securityToken)
.then(userData => Promise.resolve(cleanUserData(userData)))
.then(userData => {
if (userData.adminPermissions) {
userData.role = roles.ADMIN
}
return Promise.resolve(cleanUserData(userData))
})

const GetAnonymousUser = commands => () => {
if (anonymousUserPromise) {
Expand Down
8 changes: 4 additions & 4 deletions npm-shrinkwrap.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "domapic-controller",
"version": "1.0.0-beta.2",
"version": "1.0.0-beta.3",
"description": "Controller for Domapic systems",
"main": "server.js",
"keywords": [
Expand All @@ -24,7 +24,7 @@
},
"dependencies": {
"domapic-base": "1.0.0-beta.20",
"domapic-controller-ui": "1.0.0-beta.2",
"domapic-controller-ui": "1.0.0-beta.3",
"express-mongo-sanitize": "1.3.2",
"inquirer": "6.2.1",
"inquirer-autocomplete-prompt": "1.0.1",
Expand Down
2 changes: 1 addition & 1 deletion sonar-project.properties
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
sonar.organization=domapic
sonar.projectKey=domapic-controller
sonar.projectVersion=1.0.0-beta.2
sonar.projectVersion=1.0.0-beta.3

sonar.sources=.
sonar.exclusions=node_modules/**
Expand Down
68 changes: 67 additions & 1 deletion test/functional/specs/users-api.specs.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ test.describe('users api', function () {
let authenticator = utils.Authenticator()
let adminUserId
let pluginId
let pluginUserId
let entityId
let operatorUserId

Expand Down Expand Up @@ -72,6 +73,16 @@ test.describe('users api', function () {
})
}

const getApiKey = user => {
return utils.request('/auth/apikey', {
method: 'POST',
body: {
user
},
...authenticator.credentials()
})
}

const getAbilities = function (filters) {
return utils.request('/abilities', {
method: 'GET',
Expand Down Expand Up @@ -471,7 +482,6 @@ test.describe('users api', function () {

test.describe('update user', () => {
let moduleUserId
let pluginUserId

test.before(() => {
return utils.doLogin(authenticator)
Expand Down Expand Up @@ -536,6 +546,51 @@ test.describe('users api', function () {
])
})
})

test.it('should be able to update adminPermissions of plugin users', () => {
return getUsers().then(usersResponse => {
return Promise.resolve(usersResponse.body.find(userData => userData.name === 'foo-plugin')._id)
}).then(pluginId => {
return updateUser(pluginId, {
adminPermissions: true
}).then(response => {
return test.expect(response.statusCode).to.equal(204)
})
})
})
})

test.describe('when user has role "plugin" with adminPermissions checked and is logged using api key', () => {
let pluginUserId
let pluginApiKey

test.before(() => {
return utils.ensureUserAndDoLogin(authenticator, pluginUser).then(() => {
return getUserMe().then(response => {
pluginUserId = response.body._id
return getApiKey(pluginUserId).then(response => {
pluginApiKey = response.body.apiKey
authenticator.loginApiKey(response.body.name, pluginApiKey)
return Promise.resolve()
})
})
})
})

test.after(() => {
return updateUser(pluginUserId, {
adminPermissions: false
})
})

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.describe('when user is operator', () => {
Expand Down Expand Up @@ -630,6 +685,17 @@ test.describe('users api', function () {
return utils.ensureUserAndDoLogin(authenticator, pluginUser)
})

test.it('should not be able to update self data', () => {
return updateUser(pluginUserId, {
adminPermissions: true
}).then(response => {
return Promise.all([
test.expect(response.body.message).to.contain('Not authorized'),
test.expect(response.statusCode).to.equal(403)
])
})
})

test.describe('add user', () => {
test.it('should return 201 when adding a new user with role "operator"', () => {
return utils.createUser(authenticator, newOperatorUser).then(response => {
Expand Down
76 changes: 75 additions & 1 deletion test/unit/lib/api/users.specs.js
Original file line number Diff line number Diff line change
Expand Up @@ -450,9 +450,50 @@ test.describe('users api', () => {
})
})

test.it('should resolve if target user has plugin role and only adminPermissions are being updated', () => {
commandsMocks.stubs.user.getById.resolves({
role: 'plugin'
})

return operations.updateUser.auth(fooUser, fooParams, {
adminPermissions: true
}).then(() => {
return test.expect(true).to.be.true()
})
})

test.it('should reject if target user has plugin role and any other key plus adminPermissions is being updated', () => {
commandsMocks.stubs.user.getById.resolves({
role: 'plugin'
})

return operations.updateUser.auth(fooUser, fooParams, {
adminPermissions: true,
name: 'foo'
}).then(() => {
return test.assert.fail()
}, err => {
return test.expect(err).to.be.an.instanceof(Error)
})
})

test.it('should reject if target user has plugin role and any other key than adminPermissions is being updated', () => {
commandsMocks.stubs.user.getById.resolves({
role: 'plugin'
})

return operations.updateUser.auth(fooUser, fooParams, {
name: 'foo'
}).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'
role: 'plugin'
})

return operations.updateUser.auth({
Expand All @@ -467,6 +508,39 @@ test.describe('users api', () => {
})
})

test.it('should reject if user is admin and wants to update adminPermissions of a non plugin user', () => {
commandsMocks.stubs.user.getById.resolves({
role: 'operator'
})

return operations.updateUser.auth({
fooUser
}, fooParams, {
adminPermissions: true
}).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 adminPermissions', () => {
commandsMocks.stubs.user.getById.resolves({
role: 'operator'
})

return operations.updateUser.auth({
...fooUser,
role: 'plugin'
}, fooParams, {
adminPermissions: true
}).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'
Expand Down
Loading

0 comments on commit 99607fb

Please sign in to comment.