Skip to content

Commit

Permalink
Merge pull request #439 from dadi/patch/acl-fixes
Browse files Browse the repository at this point in the history 
Handle 403 on referenced documents
  • Loading branch information
@eduardoboucas
eduardoboucas committed Jun 25, 2018
2 parents e29a479 + a5290dc commit 2ea5d80
Show file tree
Hide file tree
Showing 6 changed files with 259 additions and 10 deletions.
11 changes: 11 additions & 0 deletions dadi/lib/fields/reference.js
View file
Expand Up @@ -185,6 +185,17 @@ module.exports.beforeOutput = function ({
})
})
)
}).catch(err => {
// If the `find` has failed due to insufficient permissions,
// we swallow the error because we don't want the main request
// to fail completely due to a 403 in one of the referenced
// collections. If we do nothing here, the document ID will
// be left untouched, which is what we want.
if (err.message === 'FORBIDDEN') {
return
}

return Promise.reject(err)
})
})
).then(() => {
Expand Down
2 changes: 1 addition & 1 deletion dadi/lib/model/acl/access.js
View file
Expand Up @@ -323,7 +323,7 @@ Access.prototype.resolveOwnTypes = function (matrix, clientId) {
splitTypes.own.forEach(baseType => {
let accessType = `${baseType}Own`

if (!matrix[accessType]) {
if (!matrix[accessType] || (matrix[baseType] === true)) {
return
}

Expand Down
5 changes: 3 additions & 2 deletions package.json
View file
Expand Up @@ -7,7 +7,8 @@
"docs": "jsdoc -c ./docs/conf.json -R README.md -r dadi/lib -d docs",
"commitmsg": "commitlint -e",
"test:prepare": "rm -rf cache && rm -rf config/config.test.json && rm -rf test/acceptance/temp-workspace && cp -R test/acceptance/workspace test/acceptance/temp-workspace",
"test": "npm run test:prepare && standard --fix 'dadi/**/*.js' | snazzy && env NODE_ENV=test ./node_modules/.bin/istanbul cover --report cobertura --report text --report html --report lcov ./node_modules/mocha/bin/_mocha",
"test": "npm run test:prepare && standard --fix 'dadi/**/*.js' | snazzy && env NODE_ENV=test ./node_modules/.bin/istanbul cover --report cobertura --report text --report html --report lcov ./node_modules/mocha/bin/_mocha && npm run test:cleanup",
"test:cleanup": "rm -rf test/acceptance/temp-workspace",
"posttest": "./scripts/coverage.js",
"start": "node start.js --node_env=development"
},
Expand All @@ -18,7 +19,7 @@
},
"dependencies": {
"@dadi/boot": "^1.1.3",
"@dadi/cache": "^2.0.2",
"@dadi/cache": "^3.0.0",
"@dadi/et": "^2.0.0",
"@dadi/format-error": "^1.7.0",
"@dadi/logger": "^1.3.0",
Expand Down
229 changes: 223 additions & 6 deletions test/acceptance/acl/collections-api.js
View file
Expand Up @@ -43,12 +43,16 @@ describe('Collections API', () => {
.set('content-type', 'application/json')
.send(creatingClient)
.end((err, res) => {
help.dropDatabase('testdb', 'test-schema', () => {
help.createDocWithParams(res.body.accessToken, { 'field1': '7', 'title': 'test doc' }, (err, doc1) => {
help.createDocWithParams(res.body.accessToken, { 'field1': '11', 'title': 'very long title' }, (err, doc2) => {
docs = [doc1._id, doc2._id]

help.removeACLData(done)
help.dropDatabase('library', 'person', () => {
help.dropDatabase('library', 'book', () => {
help.dropDatabase('testdb', 'test-schema', () => {
help.createDocWithParams(res.body.accessToken, { 'field1': '7', 'title': 'test doc' }, (err, doc1) => {
help.createDocWithParams(res.body.accessToken, { 'field1': '11', 'title': 'very long title' }, (err, doc2) => {
docs = [doc1._id, doc2._id]

help.removeACLData(done)
})
})
})
})
})
Expand Down Expand Up @@ -228,6 +232,141 @@ describe('Collections API', () => {
})
})

it('should return 200 and compose a Reference field if the client read permissions on both the parent and referenced collections', function (done) {
let testClient = {
clientId: 'apiClient',
secret: 'someSecret',
resources: {
'collection:library_book': PERMISSIONS.READ,
'collection:library_person': PERMISSIONS.READ
}
}

let authorId

help.getBearerTokenWithPermissions({
accessType: 'admin'
}).then(adminToken => {
return help.createDocument({
version: 'v1',
database: 'library',
collection: 'person',
document: {
name: 'James Lambie'
},
token: adminToken
}).then(response => {
authorId = response.results[0]._id

return help.createDocument({
version: 'v1',
database: 'library',
collection: 'book',
document: {
title: 'A Kiwi\'s guide to DADI API',
author: authorId
},
token: adminToken
})
})
}).then(response => {
return help.createACLClient(testClient).then(() => {
client
.post(config.get('auth.tokenUrl'))
.set('content-type', 'application/json')
.send(testClient)
.expect(200)
.end((err, res) => {
if (err) return done(err)

let bearerToken = res.body.accessToken

client
.get(`/v1/library/book/${response.results[0]._id}?compose=true`)
.set('content-type', 'application/json')
.set('Authorization', `Bearer ${bearerToken}`)
.end((err, res) => {
if (err) return done(err)

res.statusCode.should.eql(200)
res.body.results.length.should.eql(1)
res.body.results[0].author.name.should.eql('James Lambie')

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

it('should return 200 if the client has read permission on the given collection, but not compose a Reference field if they do not have read permissions on the referenced collection', function (done) {
let testClient = {
clientId: 'apiClient',
secret: 'someSecret',
resources: {
'collection:library_book': PERMISSIONS.READ,
'collection:library_person': PERMISSIONS.NO_READ
}
}

let authorId

help.getBearerTokenWithPermissions({
accessType: 'admin'
}).then(adminToken => {
return help.createDocument({
version: 'v1',
database: 'library',
collection: 'person',
document: {
name: 'James Lambie'
},
token: adminToken
}).then(response => {
authorId = response.results[0]._id

return help.createDocument({
version: 'v1',
database: 'library',
collection: 'book',
document: {
title: 'A Kiwi\'s guide to DADI API',
author: authorId
},
token: adminToken
})
})
}).then(response => {
return help.createACLClient(testClient).then(() => {
client
.post(config.get('auth.tokenUrl'))
.set('content-type', 'application/json')
.send(testClient)
.expect(200)
.end((err, res) => {
if (err) return done(err)

let bearerToken = res.body.accessToken

client
.get(`/v1/library/book/${response.results[0]._id}?compose=true`)
.set('content-type', 'application/json')
.set('Authorization', `Bearer ${bearerToken}`)
.end((err, res) => {
if (err) return done(err)

res.statusCode.should.eql(200)
res.body.results.length.should.eql(1)
res.body.results[0].author.should.eql(authorId)
should.not.exist(res.body.results[0]._composed)

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

it('should return 403 with an empty filter permission', function (done) {
let testClient = {
clientId: 'apiClient',
Expand Down Expand Up @@ -1105,6 +1244,84 @@ describe('Collections API', () => {
})
})

it('should return 200 with update permissions after they have been updated', function (done) {
let testClient = {
clientId: 'apiClient',
secret: 'someSecret',
resources: {
'collection:testdb_test-schema': {
create: true,
deleteOwn: true,
readOwn: true,
updateOwn: true
}
}
}

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

let userToken = res.body.accessToken

client
.put(`/vtest/testdb/test-schema/${document._id}`)
.send({
field1: 'something new'
})
.set('content-type', 'application/json')
.set('Authorization', `Bearer ${userToken}`)
.end((err, res) => {
if (err) return done(err)

res.statusCode.should.eql(404)

client
.put(`/api/clients/${testClient.clientId}/resources/collection:testdb_test-schema`)
.send({
read: true,
update: true
})
.set('content-type', 'application/json')
.set('Authorization', `Bearer ${adminToken}`)
.end((err, res) => {
if (err) return done(err)

setTimeout(() => {
client
.put(`/vtest/testdb/test-schema/${document._id}`)
.send({
field1: 'something new'
})
.set('content-type', 'application/json')
.set('Authorization', `Bearer ${userToken}`)
.end((err, res) => {
if (err) return done(err)

res.statusCode.should.eql(200)
res.body.results.length.should.eql(1)
res.body.results[0].field1.should.eql('something new')

done()
})
}, 1000)
})
})
})
})
})
})
})

it('should return 200 and not update any documents when the query differs from the filter permission', function (done) {
let testClient = {
clientId: 'apiClient',
Expand Down
20 changes: 20 additions & 0 deletions test/acceptance/help.js
View file
Expand Up @@ -36,6 +36,26 @@ module.exports.createDocWithParams = function (token, doc, done) {
})
}

module.exports.createDocument = function ({
version,
database,
collection,
document,
token
}) {
return new Promise((resolve, reject) => {
request(`http://${config.get('server.host')}:${config.get('server.port')}`)
.post(`/${version}/${database}/${collection}`)
.set('Authorization', `Bearer ${token}`)
.send(document)
.end((err, res) => {
if (err) return reject(err)

resolve(res.body)
})
})
}

// create a document with random string via the api
module.exports.createDocWithSpecificVersion = function (token, apiVersion, doc, done) {
request('http://' + config.get('server.host') + ':' + config.get('server.port'))
Expand Down
2 changes: 1 addition & 1 deletion test/acceptance/workspace/collections/v1/library/collection.person.json
View file
Expand Up @@ -37,7 +37,7 @@
},
"settings": {
"cache": false,
"authenticate": false,
"authenticate": true,
"count": 40,
"sort": "name",
"sortOrder": 1,
Expand Down

0 comments on commit 2ea5d80

Please sign in to comment.