diff --git a/CHANGELOG.md b/CHANGELOG.md index 37ace8a79ae..c586c206082 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ Enterprise Fixes: - [data-manager] Fixed editing an event whose key contains `&` creating undeletable duplicate rows in the events table Security Fixes: +- [star-rating] The `/o?method=star` ratings read now requires star-rating read access to the application it is asked about. It previously performed no authorization, so the platform and application-version combinations that had received ratings could be read for any application by a caller with no account, token or session - [hooks] Internal event hooks are now scoped to the apps the hook belongs to: app creation is a global-admin-only event, and remote-config, cohort, alert and hook-chaining events are only delivered when the event's app is one the hook is scoped to - [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 diff --git a/plugins/star-rating/api/api.js b/plugins/star-rating/api/api.js index c3626c52fd4..000fe5848c8 100644 --- a/plugins/star-rating/api/api.js +++ b/plugins/star-rating/api/api.js @@ -1577,79 +1577,85 @@ function uploadFile(myfile, id, callback) { plugins.register('/o', function(ob) { var params = ob.params; if (params.qstring.method === 'star') { - if (params.qstring.period) { - //check if period comes from datapicker - if (params.qstring.period.indexOf(",") !== -1) { - try { - params.qstring.period = JSON.parse(params.qstring.period); - } - catch (SyntaxError) { - common.returnMessage(params, 400, 'Bad request parameter: period'); - return true; - } - } - else { - switch (params.qstring.period) { - case "prevMonth": - case "month": - case "day": - case "yesterday": - case "hour": - break; - default: - if (!/([0-9]+)days/.test(params.qstring.period)) { + //this read is app scoped: require the caller to hold star-rating + //read access on app_id, the same check the sibling reads in this + //file apply. Authorize before validating parameters so that an + //unauthorized caller cannot probe the endpoint. + validateRead(params, FEATURE_NAME, function() { + if (params.qstring.period) { + //check if period comes from datapicker + if (params.qstring.period.indexOf(",") !== -1) { + try { + params.qstring.period = JSON.parse(params.qstring.period); + } + catch (SyntaxError) { common.returnMessage(params, 400, 'Bad request parameter: period'); return true; } - break; + } + else { + switch (params.qstring.period) { + case "prevMonth": + case "month": + case "day": + case "yesterday": + case "hour": + break; + default: + if (!/([0-9]+)days/.test(params.qstring.period)) { + common.returnMessage(params, 400, 'Bad request parameter: period'); + return true; + } + break; + } } } - } - else { - common.returnMessage(params, 400, 'Missing request parameter: period'); - return true; - } - countlyCommon.setPeriod(params.qstring.period, true); - var periodObj = countlyCommon.periodObj; - var collectionName = crypto.createHash('sha1').update('[CLY]_star_rating' + params.qstring.app_id).digest('hex'); - var id_prefix = params.qstring.app_id + "_" + collectionName + "_"; - var documents = []; - for (var i = 0; i < periodObj.reqZeroDbDateIds.length; i++) { - documents.push(id_prefix + "no-segment_" + periodObj.reqZeroDbDateIds[i]); - for (var m = 0; m < common.base64.length; m++) { - documents.push(id_prefix + "no-segment_" + periodObj.reqZeroDbDateIds[i] + "_" + common.base64[m]); + else { + common.returnMessage(params, 400, 'Missing request parameter: period'); + return true; } - } - common.db.collection("events_data").find({ - '_id': { - $in: documents + countlyCommon.setPeriod(params.qstring.period, true); + var periodObj = countlyCommon.periodObj; + var collectionName = crypto.createHash('sha1').update('[CLY]_star_rating' + params.qstring.app_id).digest('hex'); + var id_prefix = params.qstring.app_id + "_" + collectionName + "_"; + var documents = []; + for (var i = 0; i < periodObj.reqZeroDbDateIds.length; i++) { + documents.push(id_prefix + "no-segment_" + periodObj.reqZeroDbDateIds[i]); + for (var m = 0; m < common.base64.length; m++) { + documents.push(id_prefix + "no-segment_" + periodObj.reqZeroDbDateIds[i] + "_" + common.base64[m]); + } } - }).toArray(function(err, docs) { - if (!err) { - var result = {}; - docs.forEach(function(doc) { - if (!doc.meta) { - doc.meta = {}; - } - if (!doc.meta.platform_version_rate) { - doc.meta.platform_version_rate = []; - } - if (doc.meta_v2 && doc.meta_v2.platform_version_rate) { - common.arrayAddUniq(doc.meta.platform_version_rate, Object.keys(doc.meta_v2.platform_version_rate)); - } - doc.meta.platform_version_rate.forEach(function(item) { - var data = item.split('**'); - if (result[data[0]] === undefined) { - result[data[0]] = []; + common.db.collection("events_data").find({ + '_id': { + $in: documents + } + }).toArray(function(err, docs) { + if (!err) { + var result = {}; + docs.forEach(function(doc) { + if (!doc.meta) { + doc.meta = {}; + } + if (!doc.meta.platform_version_rate) { + doc.meta.platform_version_rate = []; } - if (result[data[0]].indexOf(data[1]) === -1) { - result[data[0]].push(data[1]); + if (doc.meta_v2 && doc.meta_v2.platform_version_rate) { + common.arrayAddUniq(doc.meta.platform_version_rate, Object.keys(doc.meta_v2.platform_version_rate)); } + doc.meta.platform_version_rate.forEach(function(item) { + var data = item.split('**'); + if (result[data[0]] === undefined) { + result[data[0]] = []; + } + if (result[data[0]].indexOf(data[1]) === -1) { + result[data[0]].push(data[1]); + } + }); }); - }); - common.returnOutput(params, result); - return true; - } + common.returnOutput(params, result); + return true; + } + }); }); return true; }