Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
17 changes: 15 additions & 2 deletions plugins/star-rating/api/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -1330,6 +1330,19 @@ function uploadFile(myfile, id, callback) {
* @apiDescription: Get feedback widgets with or without filters
* @apiParam: 'app_key', app_key of related application provided by sdk request
*/
//These two lookups serve the sdk, so they answer without a session and without an
//app_id to scope by, and they have to keep doing that or widgets stop rendering.
//What they must not do is hand out the fields the app-scoped /feedback/widgets
//deliberately withholds: targeting, which is the audience segmentation query, and
//cohortID, which that handler fetches only to test membership and then deletes with
//the comment "no need to return more data than needed".
//
//Excluded rather than allow-listed on purpose. These endpoints render every widget
//type, so an allow-list drawn from the rating-only projection above would drop the
//fields surveys and nps need, and the caller is an sdk in the field that cannot be
//redeployed. Naming the internal fields cannot break rendering.
const WIDGET_INTERNAL_FIELDS = {targeting: 0, cohortID: 0};

plugins.register('/o/feedback/multiple-widgets-by-id', function(ob) {
var params = ob.params;
var collectionName = 'feedback_widgets';
Expand All @@ -1348,7 +1361,7 @@ function uploadFile(myfile, id, callback) {
_id: {
$in: widgetIdsArray
}
}).toArray(function(err, docs) {
}, {projection: WIDGET_INTERNAL_FIELDS}).toArray(function(err, docs) {
if (!err) {
if (docs.length) {
common.returnOutput(params, docs);
Expand Down Expand Up @@ -1510,7 +1523,7 @@ function uploadFile(myfile, id, callback) {

common.db.collection(collectionName).findOne({
"_id": widgetId
}, function(err, doc) {
}, {projection: WIDGET_INTERNAL_FIELDS}, function(err, doc) {
if (err) {
common.returnMessage(params, 500, err.message);
}
Expand Down
39 changes: 39 additions & 0 deletions test/unit-tests/plugins.star-rating.widget-by-id-fields.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
require("should");
var fs = require("fs");

// /o/feedback/multiple-widgets-by-id and /o/feedback/widget answer the sdk without a
// session and without an app_id to scope by, so they cannot be gated without breaking
// widget rendering. What they can do is stop returning the fields the app-scoped
// /feedback/widgets deliberately withholds: targeting, the audience segmentation
// query, and cohortID, which that handler deletes with "no need to return more data
// than needed".
//
// Asserted against the source because both handlers are registered on a plugin bus at
// load time and their bodies are not reachable as functions from a unit test.

describe("feedback widget by-id lookups", function() {
var src = fs.readFileSync(__dirname + "/../../plugins/star-rating/api/api.js", "utf8");

it("declares the internal fields as excluded", function() {
src.should.match(/const WIDGET_INTERNAL_FIELDS = \{targeting: 0, cohortID: 0\}/);
});

it("applies the exclusion to the batch lookup", function() {
var batch = src.slice(src.indexOf("'/o/feedback/multiple-widgets-by-id'"));
batch = batch.slice(0, batch.indexOf("plugins.register", 10));
batch.should.match(/\$in: widgetIdsArray[\s\S]{0,80}projection: WIDGET_INTERNAL_FIELDS/);
});

it("applies the exclusion to the single lookup", function() {
var single = src.slice(src.indexOf("'/o/feedback/widget'"));
single = single.slice(0, single.indexOf("plugins.register", 10));
single.should.match(/"_id": widgetId[\s\S]{0,60}projection: WIDGET_INTERNAL_FIELDS/);
});

it("excludes rather than allow-lists, so widget types beyond rating still render", function() {
// an inclusion projection here would drop the fields surveys and nps need, and
// the caller is an sdk in the field that cannot be redeployed
var decl = src.match(/const WIDGET_INTERNAL_FIELDS = \{[^}]*\}/)[0];
decl.should.not.match(/: 1/);
});
});
Loading