diff --git a/CHANGELOG.md b/CHANGELOG.md index eea97813bf7..11cee770713 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,8 +2,8 @@ Fixes: - [events] Fix sum chart tooltip displaying the raw floating-point value instead of a rounded number - Security Fixes: +- [compliance-hub] The consents table now returns a fixed set of fields; a projection supplied on the request is no longer used to widen the response beyond the consent columns - [dashboards] Widgets are no longer copied when the copying user has no access to the apps they reference, and widget app ids are validated on widget create and update Enterprise Features: diff --git a/plugins/compliance-hub/api/api.js b/plugins/compliance-hub/api/api.js index d983c89c006..3259c6e45b9 100644 --- a/plugins/compliance-hub/api/api.js +++ b/plugins/compliance-hub/api/api.js @@ -10,6 +10,25 @@ var plugin = {}, const FEATURE_NAME = 'compliance_hub'; +/** + * Fields the Compliance Hub users table renders, and the only app_users fields + * /o/app_users/consents may return. These match the columns bound in + * frontend/public/templates/user.html: did, d, av, consent, lac, plus uid and + * appUserExport for the per-row consent-history and export actions. + * + * compliance_hub read is a consent-scoped permission, so this endpoint must not + * be able to return the rest of an app_users document. + */ +const CONSENT_TABLE_PROJECTION = Object.freeze({ + "did": 1, + "d": 1, + "av": 1, + "consent": 1, + "lac": 1, + "uid": 1, + "appUserExport": 1 +}); + (function() { plugins.register("/permissions/features", function(ob) { ob.features.push(FEATURE_NAME); @@ -294,7 +313,15 @@ const FEATURE_NAME = 'compliance_hub'; return; } params.qstring.query = parsedC.query; - params.qstring.project = params.qstring.project || params.qstring.projection || {"did": 1, "d": 1, "av": 1, "consent": 1, "lac": 1, "uid": 1, "appUserExport": 1}; + //The consent table is a fixed set of columns, so the + //projection is fixed server side and any project/projection + //supplied by the caller is ignored. Previously the default + //applied only when the caller omitted both parameters, so + //project={} - an empty projection, which Mongo reads as "all + //fields" - returned whole app_users documents (name, email, + //phone, custom properties, payment totals, location) to a + //member holding nothing but compliance_hub read. + params.qstring.project = Object.assign({}, CONSENT_TABLE_PROJECTION); if (params.qstring.sSearch && params.qstring.sSearch !== "") { params.qstring.query.did = {"$regex": new RegExp(".*" + params.qstring.sSearch + ".*", 'i')}; diff --git a/plugins/compliance-hub/tests.js b/plugins/compliance-hub/tests.js index af58c85f6ee..59b81accb4b 100644 --- a/plugins/compliance-hub/tests.js +++ b/plugins/compliance-hub/tests.js @@ -83,6 +83,14 @@ describe('Testing Compliance Hub', function() { }); }); describe('Check app_users consents', function() { + // The consent table is a fixed set of columns. The projection used to be + // taken from the request when supplied, so project={} - which Mongo reads + // as "all fields" - returned whole app_users documents (name, email, + // phone, custom properties, payment totals, location) to any caller with + // compliance_hub read. The projection is now fixed server side. + // _id is included because Mongo returns it unless explicitly excluded. + var ALLOWED_FIELDS = ["_id", "did", "d", "av", "consent", "lac", "uid", "appUserExport"]; + it('should list app users with consent data', function(done) { request .get('/o/app_users/consents?api_key=' + API_KEY_ADMIN + '&app_id=' + APP_ID) @@ -96,6 +104,69 @@ describe('Testing Compliance Hub', function() { setTimeout(done, 100); }); }); + + it('should ignore an empty projection supplied by the caller', function(done) { + request + .get('/o/app_users/consents?api_key=' + API_KEY_ADMIN + '&app_id=' + APP_ID + '&project=' + encodeURIComponent('{}')) + .expect(200) + .end(function(err, res) { + if (err) { + return done(err); + } + var ob = JSON.parse(res.text); + ob.should.have.property('aaData'); + ob.aaData.should.be.an.Array; + ob.aaData.forEach(function(row) { + Object.keys(row).forEach(function(field) { + ALLOWED_FIELDS.should.containEql(field); + }); + }); + setTimeout(done, 100); + }); + }); + + it('should ignore an explicit projection asking for extra fields', function(done) { + var project = JSON.stringify({uid: 1, did: 1, name: 1, email: 1, username: 1, custom: 1, consent: 1}); + request + .get('/o/app_users/consents?api_key=' + API_KEY_ADMIN + '&app_id=' + APP_ID + '&project=' + encodeURIComponent(project)) + .expect(200) + .end(function(err, res) { + if (err) { + return done(err); + } + var ob = JSON.parse(res.text); + ob.should.have.property('aaData'); + ob.aaData.forEach(function(row) { + row.should.not.have.property('name'); + row.should.not.have.property('email'); + row.should.not.have.property('username'); + row.should.not.have.property('custom'); + Object.keys(row).forEach(function(field) { + ALLOWED_FIELDS.should.containEql(field); + }); + }); + setTimeout(done, 100); + }); + }); + + it('should ignore the projection alias parameter as well', function(done) { + request + .get('/o/app_users/consents?api_key=' + API_KEY_ADMIN + '&app_id=' + APP_ID + '&projection=' + encodeURIComponent('{}')) + .expect(200) + .end(function(err, res) { + if (err) { + return done(err); + } + var ob = JSON.parse(res.text); + ob.should.have.property('aaData'); + ob.aaData.forEach(function(row) { + Object.keys(row).forEach(function(field) { + ALLOWED_FIELDS.should.containEql(field); + }); + }); + setTimeout(done, 100); + }); + }); }); describe('Reset App', function() {