Commit 732df3cd authored by Tamlyn Rhodes's avatar Tamlyn Rhodes

Merge remote-tracking branch 'origin/master' into fragments

# Conflicts: # src/routes/api_collections.js # src/routes/util.js
parents 1301e9ea 0a676241
......@@ -10,8 +10,14 @@ const express = require('express')
const api = express.Router()
const sse = require('pubsweet-sse')
const { objectId, buildChangeData, fieldSelector, getTeams,
applyPermissionFilter } = require('./util')
const {
createFilterFromQuery,
objectId,
buildChangeData,
fieldSelector,
getTeams,
applyPermissionFilter
} = require('./util')
const passport = require('passport')
const authBearer = passport.authenticate('bearer', { session: false })
......@@ -27,11 +33,12 @@ api.get('/collections', authBearerAndPublic, async (req, res, next) => {
filterable: collections
})
const collectionsWithSelectedFields = await Promise.all(filteredCollections.map(async collection => {
const collectionsWithSelectedFields = (await Promise.all(filteredCollections.map(async collection => {
collection.owners = await User.ownersWithUsername(collection)
const properties = await applyPermissionFilter({ req, target: collection })
return fieldSelector(req)(properties)
}))
})))
.filter(createFilterFromQuery(req.query))
res.status(STATUS.OK).json(collectionsWithSelectedFields)
} catch (err) {
......@@ -121,12 +128,13 @@ api.get('/collections/:collectionId/teams', authBearerAndPublic, async (req, res
await applyPermissionFilter({ req, target: collection })
try {
const teams = await getTeams({
const teams = (await getTeams({
req,
Team,
id: collection.id,
type: 'collection'
})
}))
.filter(createFilterFromQuery(req.query))
res.status(STATUS.OK).json(teams)
} catch (err) {
......
......@@ -5,7 +5,7 @@ const passport = require('passport')
const authsome = require('../helpers/authsome')
const Team = require('../models/Team')
const { authorizationError } = require('./util')
const { createFilterFromQuery, authorizationError } = require('./util')
const authBearer = passport.authenticate('bearer', { session: false })
const api = express.Router({mergeParams: true})
......@@ -22,7 +22,8 @@ api.get('/teams', authBearer, async (req, res, next) => {
throw authorizationError(req.user, req.method, req)
}
const teams = await Team.all()
const teams = (await Team.all())
.filter(createFilterFromQuery(req.query))
res.status(STATUS.OK).json(teams)
} catch (err) {
......
......@@ -7,7 +7,7 @@ const express = require('express')
const User = require('../models/User')
const authsome = require('../helpers/authsome')
const { authorizationError } = require('./util')
const { createFilterFromQuery, authorizationError } = require('./util')
const Team = require('../models/Team')
const AuthorizationError = require('../errors/AuthorizationError')
......@@ -63,7 +63,9 @@ api.get('/users', authBearer, async (req, res, next) => {
throw authorizationError(req.user, req.method, req.path)
}
const users = await User.all()
const users = (await User.all())
.filter(createFilterFromQuery(req.query))
return res.status(STATUS.OK).json({users: users})
} catch (err) {
next(err)
......
const pick = require('lodash/pick')
const _ = require('lodash')
const AuthorizationError = require('../errors/AuthorizationError')
const NotFoundError = require('../errors/NotFoundError')
const authsome = require('../helpers/authsome')
......@@ -27,10 +27,19 @@ Util.buildChangeData = (input, output) => {
return data
}
Util.createFilterFromQuery = query => {
const filterPaths = _.difference(_.keys(query), ['fields'])
return (item) => {
return filterPaths.every(filterPath => {
return _.has(item, filterPath) && _.get(item, filterPath) === query[filterPath]
})
}
}
Util.fieldSelector = req => {
const fields = req.query.fields ? req.query.fields.split(/\s*,\s*/) : null
return item => fields ? pick(item, fields.concat('id')) : item
return item => fields ? _.pick(item, fields.concat('id', 'rev')) : item
}
Util.getTeams = async (opts) => {
......
......@@ -66,6 +66,48 @@ describe('Collections API', () => {
expect(collection.title).toEqual(fixtures.collection.title)
})
it('should allow an admin user to filter collections with query params', async () => {
const adminToken = await authenticateAdmin()
await api.collections.create(fixtures.collection, adminToken)
.expect(STATUS.CREATED)
await api.collections.create(fixtures.collection2, adminToken)
.expect(STATUS.CREATED)
const query1 = {'owners.0.username': 'admin'}
const collections1 = await api.collections.list(adminToken, query1)
.expect(STATUS.OK)
.then(res => res.body)
expect(collections1).toHaveLength(2)
const query2 = {'title': fixtures.collection.title}
const collections2 = await api.collections.list(adminToken, query2)
.expect(STATUS.OK)
.then(res => res.body)
expect(collections2).toHaveLength(1)
})
it('should return empty array if filtering on non-existent property', async () => {
const adminToken = await authenticateAdmin()
await api.collections.create(fixtures.collection, adminToken)
.expect(STATUS.CREATED)
await api.collections.create(fixtures.collection2, adminToken)
.expect(STATUS.CREATED)
const query = {'nonexistent': 'x'}
const collections = await api.collections.list(adminToken, query)
.expect(STATUS.OK)
.then(res => res.body)
expect(collections).toHaveLength(0)
})
it('collection.owners should be augmented with usernames (both singular and plural endpoints)', async () => {
const adminToken = await authenticateAdmin()
......@@ -561,6 +603,21 @@ describe('Collections API', () => {
expect(Object.keys(collection)).not.toContain('filtered')
})
it('should return empty array if trying to filter with query params on properties for which user lacks authorization', async () => {
const token = await authenticateUser()
await api.collections.create(
Object.assign({}, fixtures.collection, { filtered: 'example' }), token)
.expect(STATUS.CREATED)
.then(res => res.body)
const collections = await api.collections.list(token, { filtered: 'example' })
.expect(STATUS.OK)
.then(res => res.body)
expect(collections).toHaveLength(0)
})
it('should filter collection properties based on authorization on update', async () => {
const token = await authenticateUser()
......
......@@ -127,7 +127,6 @@ describe('Teams API - per collection or fragment', () => {
)
).then(
res => {
teamId = res.body.id
expect(res.body.name).toEqual(team.name)
}
).then(
......
......@@ -4,6 +4,12 @@ const collection = {
'published': true
}
const collection2 = {
'type': 'collection',
'title': 'Second collection',
'published': true
}
const updatedCollection = {
'title': 'Update Blogger posts'
}
......@@ -81,6 +87,7 @@ const readerTeam = {
module.exports = {
collection: collection,
collection2,
updatedCollection: updatedCollection,
fragment: fragment,
updatedFragment: updatedFragment,
......
......@@ -198,16 +198,14 @@ const users = {
}
const collections = {
list: (token, options) => {
let url = COLLECTIONS_ROOT
list: (token, query = {}) => {
const url = COLLECTIONS_ROOT
if (options && options.fields) {
url += '?' + querystring.stringify({
fields: options.fields.join(',')
})
if (query.fields) {
query.fields = query.fields.join(',')
}
const req = request(api).get(url)
const req = request(api).get(url).query(query)
return authorizedRequest(req, token)
},
create: (collection, token) => {
......
const createDb = require('../src/db')
const STATUS = require('http-status-codes')
const range = require('lodash/range')
const Model = require('../src/models/Model')
const User = require('../src/models/User')
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment