Skip to content

Commit

Permalink
Updates users-info endpoint to accept multiple roles (#5929)
Browse files Browse the repository at this point in the history
Updates users-info endpoint to accept multiple roles as a JSON stringified string to support how roles are used in production.

#5362
  • Loading branch information
dianabarsan authored and kennsippell committed Sep 22, 2019
1 parent f76ba1f commit 5621346
Show file tree
Hide file tree
Showing 4 changed files with 155 additions and 12 deletions.
2 changes: 1 addition & 1 deletion api/README.md
Expand Up @@ -999,7 +999,7 @@ When the requester has an online role, the following query parameters are accept
| Variable | Description | Required |
| -------- | ------------------------------------------ | -------- |
| facility_id | String identifier representing the uuid of the user's facility | true |
| role | String identifier representing the user role - must be configured as an offline role | true |
| role | String identifier representing the user role - must be configured as an offline role. Accepts valid UTF-8 JSON array for multiple of roles. | true |
| contact_id | String identifier representing the uuid of the user's associated contact | false |

##### Example
Expand Down
30 changes: 28 additions & 2 deletions api/src/controllers/users.js
Expand Up @@ -43,6 +43,31 @@ const basicAuthValid = (credentials, username) => {

const isChangingPassword = req => Object.keys(req.body).includes('password');

const getRoles = req => {
const params = req.query;
let roles;
try {
roles = JSON.parse(params.role);
} catch (err) {
// if json.parse fails, consider we just got one string role as param
return [params.role];
}

if (typeof roles === 'string') {
roles = [roles];
}

if (!Array.isArray(roles)) {
throw { code: 400, reason: 'Role parameter must be either a string or a JSON encoded array' };
}

if (roles.some(role => typeof role !== 'string')) {
throw { code: 400, reason: 'All roles should be strings' };
}

return roles;
};

const getInfoUserCtx = req => {
if (!auth.isOnlineOnly(req.userCtx)) {
return req.userCtx;
Expand All @@ -56,12 +81,13 @@ const getInfoUserCtx = req => {
throw { code: 400, reason: 'Missing required query params: role and/or facility_id' };
}

if (!auth.isOffline([ params.role ])) {
const roles = getRoles(req);
if (!auth.isOffline(roles)) {
throw { code: 400, reason: 'Provided role is not offline' };
}

return {
roles: [ params.role ],
roles: roles,
facility_id: params.facility_id,
contact_id: params.contact_id
};
Expand Down
86 changes: 80 additions & 6 deletions api/tests/mocha/controllers/users.spec.js
Expand Up @@ -25,10 +25,13 @@ describe('Users Controller', () => {
userCtx = { name: 'user', roles: ['admin'] };
req = { query: {}, userCtx };
res = { json: sinon.stub() };
auth.isOnlineOnly.returns(true);
});

describe('online users', () => {
beforeEach(() => {
auth.isOnlineOnly.returns(true);
});

it('should respond with error when user does not have required permissions', () => {
serverUtils.error.resolves();
auth.hasAllPermissions.returns(false);
Expand Down Expand Up @@ -97,7 +100,7 @@ describe('Users Controller', () => {
auth.isOffline.returns(true);
auth.hasAllPermissions.returns(true);
const authContext = {
userCtx: { roles: [req.query.role], facility_id: req.query.facility_id },
userCtx: { roles: ['some_role'], facility_id: req.query.facility_id },
contactsByDepthKeys: [['some_facility_id']],
subjectIds: ['some_facility_id', 'a', 'b', 'c']
};
Expand All @@ -109,7 +112,7 @@ describe('Users Controller', () => {
chai.expect(serverUtils.error.callCount).to.equal(0);
chai.expect(authorization.getAuthorizationContext.callCount).to.equal(1);
chai.expect(authorization.getAuthorizationContext.args[0]).to.deep.equal([{
roles: [req.query.role],
roles: ['some_role'],
facility_id: req.query.facility_id,
contact_id: undefined
}]);
Expand All @@ -129,7 +132,7 @@ describe('Users Controller', () => {
auth.isOffline.returns(true);
auth.hasAllPermissions.returns(true);
const authContext = {
userCtx: { roles: [req.query.role], facility_id: req.query.facility_id, contact_id: 'some_contact_id' },
userCtx: { roles: ['some_role'], facility_id: req.query.facility_id, contact_id: 'some_contact_id' },
contactsByDepthKeys: [['some_facility_id']],
subjectIds: ['some_facility_id', 'a', 'b', 'c']
};
Expand All @@ -141,7 +144,7 @@ describe('Users Controller', () => {
chai.expect(serverUtils.error.callCount).to.equal(0);
chai.expect(authorization.getAuthorizationContext.callCount).to.equal(1);
chai.expect(authorization.getAuthorizationContext.args[0]).to.deep.equal([{
roles: [req.query.role],
roles: ['some_role'],
facility_id: req.query.facility_id,
contact_id: req.query.contact_id
}]);
Expand All @@ -160,7 +163,7 @@ describe('Users Controller', () => {
auth.isOffline.returns(true);
auth.hasAllPermissions.returns(true);
const authContext = {
userCtx: { roles: [req.query.role], facility_id: req.query.facility_id },
userCtx: { roles: ['some_role'], facility_id: req.query.facility_id },
contactsByDepthKeys: [['some_facility_id']],
subjectIds: ['some_facility_id', 'a', 'b', 'c']
};
Expand All @@ -173,6 +176,77 @@ describe('Users Controller', () => {
chai.expect(res.json.args[0]).to.deep.equal([{ total_docs: 10500, warn: true, limit: 10000 }]);
});
});

it('should parse role json and use all provided roles', () => {
req.query = {
role: JSON.stringify(['role1', 'role2']),
facility_id: 'some_facility_id'
};
auth.isOffline.returns(true);
auth.hasAllPermissions.returns(true);
const authContext = {
userCtx: { roles: ['role1', 'role2'], facility_id: req.query.facility_id },
contactsByDepthKeys: [['some_facility_id']],
subjectIds: ['some_facility_id', 'a', 'b', 'c']
};
const docIds = Array.from(Array(1000), (x, idx) => idx + 1);
authorization.getAuthorizationContext.resolves(authContext);
authorization.getAllowedDocIds.resolves(docIds);

return controller.info(req, res).then(() => {
chai.expect(authorization.getAuthorizationContext.args[0]).to.deep.equal([{
roles: ['role1', 'role2'],
facility_id: 'some_facility_id',
contact_id: undefined,
}]);
chai.expect(res.json.callCount).to.equal(1);
chai.expect(res.json.args[0]).to.deep.equal([{ total_docs: 1000, warn: false, limit: 10000 }]);
});
});

describe('roles scenarios', () => {
const expected = { total_docs: 3, warn: false, limit: 10000 };
const scenarios = [
{ role: 'aaaa', name: 'string single role' },
{ role: JSON.stringify('aaaa'), name: 'json single role' },
{ role: JSON.stringify(['1', '2', '3']), name: 'array of string roles works'},
{ role: JSON.stringify(['1', '2']).slice(0, -2), name: 'malformed json = param treated as string'},
{ role: JSON.stringify({ not: 'an array' }), fail: true, name: 'JSON object' },
{ role: JSON.stringify(['1', { not: 'a string' }]), fail: true, name: 'JSON array with objects' },
{ role: JSON.stringify(['1', 32, '2']), fail: true, name: 'JSON array with numbers' },
{ role: JSON.stringify(['1', undefined, '2']), fail: true, name: 'JSON array with undefined' },
{ role: JSON.stringify(['1', null, '2']), fail: true, name: 'JSON array with undefined' },
{ role: JSON.stringify(['1', true, '2']), fail: true, name: 'JSON array with boolean' },
];

beforeEach(() => {
authorization.getAuthorizationContext.callsFake(userCtx => Promise.resolve({ userCtx }));
authorization.getAllowedDocIds.resolves(['1', '2', '3']);
auth.isOffline.returns(true);
auth.hasAllPermissions.returns(true);
serverUtils.error.resolves();
});

scenarios.map(scenario => {
it(scenario.name, () => {
req.query = {
role: scenario.role,
facility_id: 'some_facility_id'
};
return controller.info(req, res).then(() => {
if (scenario.fail) {
chai.expect(serverUtils.error.callCount).to.equal(1);
chai.expect(serverUtils.error.args[0][0].code).to.deep.equal(400);
chai.expect(res.json.callCount).to.equal(0);
} else {
chai.expect(serverUtils.error.callCount).to.equal(0);
chai.expect(res.json.callCount).to.equal(1);
chai.expect(res.json.args[0]).to.deep.equal([expected]);
}
});
});
});
});
});

describe('offline users', () => {
Expand Down
49 changes: 46 additions & 3 deletions tests/e2e/api/controllers/users.js
Expand Up @@ -254,7 +254,7 @@ describe('Users API', () => {
_id: 'fixture:user:offline',
name: 'OfflineUser'
},
roles: ['district_admin']
roles: ['district_admin', 'this', 'user', 'will', 'be', 'offline']
},
{
username: 'online',
Expand All @@ -277,6 +277,7 @@ describe('Users API', () => {
let onlineRequestOptions;
const nbrOfflineDocs = 30;
let expectedNbrDocs = nbrOfflineDocs + 4; // _design/medic-client + org.couchdb.user:offline + fixture:offline + OfflineUser
let docsForAll;

beforeAll(done => {
utils
Expand All @@ -296,7 +297,10 @@ describe('Users API', () => {
return utils.saveDocs(docs);
})
.then(() => utils.requestOnTestDb('/_design/medic/_view/docs_by_replication_key?key="_all"'))
.then(resp => expectedNbrDocs += resp.rows.length)
.then(resp => {
docsForAll = resp.rows.length + 2; // _design/medic-client + org.couchdb.user:doc
expectedNbrDocs += resp.rows.length;
})
.then(done);
});

Expand Down Expand Up @@ -334,6 +338,17 @@ describe('Users API', () => {
});
});

it('should return correct number of allowed docs when requested by online user for an array of roles', () => {
const params = {
role: JSON.stringify(['random', 'district_admin', 'random']),
facility_id: 'fixture:offline'
};
onlineRequestOptions.path += '?' + querystring.stringify(params);
return utils.request(onlineRequestOptions).then(resp => {
expect(resp).toEqual({ total_docs: expectedNbrDocs, warn: false, limit: 10000 });
});
});

it('should ignore parameters for requests from offline users', () => {
const params = {
role: 'district_admin',
Expand All @@ -350,7 +365,7 @@ describe('Users API', () => {
role: 'national_admin',
facility_id: 'fixture:offline'
};
offlineRequestOptions.path += '?' + querystring.stringify(params);
onlineRequestOptions.path += '?' + querystring.stringify(params);
onlineRequestOptions.headers = { 'Content-Type': 'application/json' };
return utils
.request(onlineRequestOptions)
Expand All @@ -359,5 +374,33 @@ describe('Users API', () => {
expect(err.statusCode).toEqual(400);
});
});

it('should throw error for array roles of online user', () => {
const params = {
role: JSON.stringify(['random', 'national_admin', 'random']),
facility_id: 'fixture:offline'
};
onlineRequestOptions.path += '?' + querystring.stringify(params);
return utils
.request(onlineRequestOptions)
.then(resp => expect(resp).toEqual('should have thrown'))
.catch(err => {
expect(err.statusCode).toEqual(400);
});
});

it('should return correct response for non-existent facility', () => {
const params = {
role: JSON.stringify(['district_admin']),
facility_id: 'IdonTExist'
};
onlineRequestOptions.path += '?' + querystring.stringify(params);
onlineRequestOptions.headers = { 'Content-Type': 'application/json' };
return utils
.request(onlineRequestOptions)
.then(resp => {
expect(resp).toEqual({ total_docs: docsForAll, warn: false, limit: 10000 });
});
});
});
}) ;

0 comments on commit 5621346

Please sign in to comment.