Skip to content

Commit

Permalink
Merge 12f0ec3 into ab380ac
Browse files Browse the repository at this point in the history
  • Loading branch information
@eduardoboucas
eduardoboucas committed Aug 15, 2018
2 parents ab380ac + 12f0ec3 commit dec0f4c
Show file tree
Hide file tree
Showing 3 changed files with 192 additions and 39 deletions.
34 changes: 28 additions & 6 deletions dadi/lib/controller/clients.js
View file
Expand Up @@ -233,12 +233,24 @@ Clients.prototype.handleError = function (res, next) {
errors: err.data.map(role => `Invalid role: ${role}`)
})

case 'INVALID_SECRET':
return help.sendBackJSON(400, res, next)(null, {
success: false,
errors: ['The current secret supplied is not valid']
})

case 'MISSING_FIELDS':
return help.sendBackJSON(400, res, next)(null, {
success: false,
errors: err.data.map(field => `Missing field: ${field}`)
})

case 'MISSING_SECRET':
return help.sendBackJSON(400, res, next)(null, {
success: false,
errors: ['The current secret must be supplied via a `currentSecret` property']
})

case 'PROTECTED_DATA_FIELDS':
return help.sendBackJSON(400, res, next)(null, {
success: false,
Expand Down Expand Up @@ -383,18 +395,28 @@ Clients.prototype.postRole = function (req, res, next) {
}

Clients.prototype.put = function (req, res, next) {
let clientIsAdmin = acl.client.isAdmin(req.dadiApiClient)
let update = req.body

// A client can only be updated by themselves or by an admin.
if (
!acl.client.isAdmin(req.dadiApiClient) &&
req.params.clientId !== req.dadiApiClient.clientId
) {
if (!clientIsAdmin && req.params.clientId !== req.dadiApiClient.clientId) {
return this.handleError(res, next)(
acl.createError(req.dadiApiClient)
)
}

return this.validateDataObject(req.body.data, req.dadiApiClient).then(() => {
return model.update(req.params.clientId, req.body)
return this.validateDataObject(update.data, req.dadiApiClient).then(() => {
if (
!clientIsAdmin &&
typeof update.secret === 'string' &&
typeof update.currentSecret !== 'string'
) {
return Promise.reject(
new Error('MISSING_SECRET')
)
}

return model.update(req.params.clientId, update)
}).then(({results}) => {
if (results.length === 0) {
return Promise.reject(
Expand Down
75 changes: 43 additions & 32 deletions dadi/lib/model/acl/client.js
View file
Expand Up @@ -501,50 +501,61 @@ Client.prototype.setWriteCallback = function (callback) {
* @return {Promise<Object>}
*/
Client.prototype.update = function (clientId, update) {
let findQuery = {
clientId
}
let isUpdatingSecret = typeof update.currentSecret === 'string'

if (isUpdatingSecret) {
findQuery.secret = update.currentSecret

delete update.currentSecret
}

return this.validate(update, {
blockedFields: ['clientId'],
partial: true
}).then(() => {
let dataMerge = Promise.resolve(null)

if (update.data) {
dataMerge = this.model.find({
options: {
return this.model.find({
options: {
fields: {
data: 1
},
query: {
clientId
}
}).then(({results}) => {
if (results.length > 0) {
return results[0].data || {}
}
},
query: findQuery
})
}).then(({results}) => {
if (results.length === 0) {
if (isUpdatingSecret) {
return Promise.reject(
new Error('INVALID_SECRET')
)
}

return {}
})
return Promise.reject(
new Error('CLIENT_NOT_FOUND')
)
}

return dataMerge.then(data => {
if (data) {
let mergedData = Object.assign({}, data, update.data)
if (update.data) {
let mergedData = Object.assign({}, results[0].data, update.data)

Object.keys(mergedData).forEach(key => {
if (mergedData[key] === null) {
delete mergedData[key]
}
})
Object.keys(mergedData).forEach(key => {
if (mergedData[key] === null) {
delete mergedData[key]
}
})

update.data = mergedData
}
update.data = mergedData
}

return this.model.update({
query: {
clientId
},
rawOutput: true,
update,
validate: false
})
return this.model.update({
query: {
clientId
},
rawOutput: true,
update,
validate: false
})
})
}
Expand Down
122 changes: 121 additions & 1 deletion test/acceptance/acl/clients-api/put.js
View file
Expand Up @@ -425,7 +425,124 @@ module.exports = () => {
})
})
})
})
})

it('should return 400 when a non-admin client tries to update their secret without supplying the current secret', done => {
let testClient = {
clientId: 'apiClient',
secret: 'someSecret'
}

help.createACLClient(testClient).then(() => {
client
.post(config.get('auth.tokenUrl'))
.set('content-type', 'application/json')
.send({
clientId: testClient.clientId,
secret: testClient.secret
})
.expect(200)
.expect('content-type', 'application/json')
.end((err, res) => {
if (err) return done(err)

res.body.accessToken.should.be.String

let bearerToken = res.body.accessToken

client
.put(`/api/clients/${testClient.clientId}`)
.send({
secret: 'aNewSecret'
})
.set('content-type', 'application/json')
.set('Authorization', `Bearer ${bearerToken}`)
.expect('content-type', 'application/json')
.end((err, res) => {
res.statusCode.should.eql(400)

res.body.success.should.eql(false)
res.body.errors[0].should.eql('The current secret must be supplied via a `currentSecret` property')

client
.post(config.get('auth.tokenUrl'))
.set('content-type', 'application/json')
.send({
clientId: testClient.clientId,
secret: 'someSecret'
})
.expect(200)
.expect('content-type', 'application/json')
.end((err, res) => {
if (err) return done(err)

res.body.accessToken.should.be.String

done()
})
})
})
})
})

it('should return 400 when a non-admin client tries to update their secret and the current secret supplied is incorrect', done => {
let testClient = {
clientId: 'apiClient',
secret: 'someSecret'
}

help.createACLClient(testClient).then(() => {
client
.post(config.get('auth.tokenUrl'))
.set('content-type', 'application/json')
.send({
clientId: testClient.clientId,
secret: testClient.secret
})
.expect(200)
.expect('content-type', 'application/json')
.end((err, res) => {
if (err) return done(err)

res.body.accessToken.should.be.String

let bearerToken = res.body.accessToken

client
.put(`/api/clients/${testClient.clientId}`)
.send({
currentSecret: 'this does not look right',
secret: 'aNewSecret'
})
.set('content-type', 'application/json')
.set('Authorization', `Bearer ${bearerToken}`)
.expect('content-type', 'application/json')
.end((err, res) => {
res.statusCode.should.eql(400)

res.body.success.should.eql(false)
res.body.errors[0].should.eql('The current secret supplied is not valid')

client
.post(config.get('auth.tokenUrl'))
.set('content-type', 'application/json')
.send({
clientId: testClient.clientId,
secret: 'someSecret'
})
.expect(200)
.expect('content-type', 'application/json')
.end((err, res) => {
if (err) return done(err)

res.body.accessToken.should.be.String

done()
})
})
})
})
})
})

describe('success states', () => {
Expand Down Expand Up @@ -456,6 +573,7 @@ module.exports = () => {
client
.put(`/api/clients/${testClient.clientId}`)
.send({
currentSecret: 'someSecret',
secret: 'aNewSecret'
})
.set('content-type', 'application/json')
Expand Down Expand Up @@ -517,6 +635,7 @@ module.exports = () => {
client
.put('/api/client')
.send({
currentSecret: 'someSecret',
secret: 'aNewSecret'
})
.set('content-type', 'application/json')
Expand Down Expand Up @@ -1067,6 +1186,7 @@ module.exports = () => {
client
.put('/api/client')
.send({
currentSecret: 'someSecret',
secret: 'something'
})
.set('content-type', 'application/json')
Expand Down

0 comments on commit dec0f4c

Please sign in to comment.