Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
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

## Version 25.03.50
Fixes:
- [star-rating] Fix custom widget logo resolving to the wrong path (mis-detected as the global app logo) after editing a widget
Expand Down
46 changes: 34 additions & 12 deletions plugins/compliance-hub/api/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -177,7 +196,12 @@ const FEATURE_NAME = 'compliance_hub';
}
else if (total > 0) {
params.qstring.query = params.qstring.query || params.qstring.filter || {};
params.qstring.project = params.qstring.project || params.qstring.projection || {};
//consent_history holds only consent state and the device
//context it changed in, so full documents stay the response
//shape here. The projection is still fixed server side
//rather than taken from the request, so this endpoint
//cannot be steered with project/projection either.
params.qstring.project = {};

var parsedSearch = common.parseUserQuery(params.qstring.query);
if (parsedSearch.error) {
Expand Down Expand Up @@ -209,16 +233,6 @@ const FEATURE_NAME = 'compliance_hub';
params.qstring.query.ts = countlyCommon.getTimestampRangeQuery(params, false);
}

params.qstring.project = params.qstring.project || {};
if (typeof params.qstring.project === "string" && params.qstring.project.length) {
try {
params.qstring.project = JSON.parse(params.qstring.project);
}
catch (ex) {
params.qstring.project = {};
}
}

params.qstring.sort = params.qstring.sort || {};
if (typeof params.qstring.sort === "string" && params.qstring.sort.length) {
try {
Expand Down Expand Up @@ -294,7 +308,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')};
Expand Down
71 changes: 71 additions & 0 deletions plugins/compliance-hub/tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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() {
Expand Down
Loading