Skip to content

Adds count class level permission #3814

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 15, 2017
Merged
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
32 changes: 32 additions & 0 deletions spec/CloudCode.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1470,4 +1470,36 @@ describe('afterFind hooks', () => {
});
});
});

it('should set count to true on beforeFind hooks if query is count', (done) => {
const hook = {
method: function(req) {
expect(req.count).toBe(true);
return Promise.resolve();
}
};
spyOn(hook, 'method').and.callThrough();
Parse.Cloud.beforeFind('Stuff', hook.method);
new Parse.Query('Stuff').count().then((count) => {
expect(count).toBe(0);
expect(hook.method).toHaveBeenCalled();
done();
});
});

it('should set count to false on beforeFind hooks if query is not count', (done) => {
const hook = {
method: function(req) {
expect(req.count).toBe(false);
return Promise.resolve();
}
};
spyOn(hook, 'method').and.callThrough();
Parse.Cloud.beforeFind('Stuff', hook.method);
new Parse.Query('Stuff').find().then((res) => {
expect(res.length).toBe(0);
expect(hook.method).toHaveBeenCalled();
done();
});
});
});
32 changes: 32 additions & 0 deletions spec/Schema.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,38 @@ describe('SchemaController', () => {
});
});

it('class-level permissions test count', (done) => {
var obj;
return config.database.loadSchema()
// Create a valid class
.then(schema => schema.validateObject('Stuff', {foo: 'bar'}))
.then(schema => {
var count = {};
return schema.setPermissions('Stuff', {
'create': {'*': true},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no quotes around object members

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was to match the style of the rest of the file, we'll update those at a later time (i.e. test linting)

'find': {'*': true},
'count': count
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm assuming that count = {} is roughly equivelent to count = { '*': false } ?

Copy link
Contributor Author

@flovilmart flovilmart May 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah but *: false is not a valid CLP, {} means lockdown, no one can execute a count

})
}).then(() => {
obj = new Parse.Object('Stuff');
obj.set('foo', 'bar');
return obj.save();
}).then((o) => {
obj = o;
var query = new Parse.Query('Stuff');
return query.find();
}).then((results) => {
expect(results.length).toBe(1);
var query = new Parse.Query('Stuff');
return query.count();
}).then(() => {
fail('Class permissions should have rejected this query.');
}, (err) => {
expect(err.message).toEqual('Permission denied for action count on class Stuff.');
done();
});
});

it('can add classes without needing an object', done => {
config.database.loadSchema()
.then(schema => schema.addClassIfNotExists('NewClass', {
Expand Down
3 changes: 3 additions & 0 deletions src/Controllers/DatabaseController.js
Original file line number Diff line number Diff line change
Expand Up @@ -744,6 +744,9 @@ DatabaseController.prototype.find = function(className, query, {
const isMaster = acl === undefined;
const aclGroup = acl || [];
op = op || (typeof query.objectId == 'string' && Object.keys(query).length === 1 ? 'get' : 'find');
// Count operation if counting
op = (count === true ? 'count' : op);

let classExists = true;
return this.loadSchema()
.then(schemaController => {
Expand Down
4 changes: 2 additions & 2 deletions src/Controllers/SchemaController.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ function verifyPermissionKey(key) {
}
}

const CLPValidKeys = Object.freeze(['find', 'get', 'create', 'update', 'delete', 'addField', 'readUserFields', 'writeUserFields']);
const CLPValidKeys = Object.freeze(['find', 'count', 'get', 'create', 'update', 'delete', 'addField', 'readUserFields', 'writeUserFields']);
function validateCLP(perms, fields) {
if (!perms) {
return;
Expand Down Expand Up @@ -820,7 +820,7 @@ export default class SchemaController {

// No matching CLP, let's check the Pointer permissions
// And handle those later
const permissionField = ['get', 'find'].indexOf(operation) > -1 ? 'readUserFields' : 'writeUserFields';
const permissionField = ['get', 'find', 'count'].indexOf(operation) > -1 ? 'readUserFields' : 'writeUserFields';

// Reject create when write lockdown
if (permissionField == 'writeUserFields' && operation == 'create') {
Expand Down
9 changes: 6 additions & 3 deletions src/triggers.js
Original file line number Diff line number Diff line change
Expand Up @@ -155,11 +155,12 @@ export function getRequestObject(triggerType, auth, parseObject, originalParseOb
return request;
}

export function getRequestQueryObject(triggerType, auth, query, config) {
export function getRequestQueryObject(triggerType, auth, query, count, config) {
var request = {
triggerName: triggerType,
query: query,
query,
master: false,
count,
log: config.loggerController
};

Expand Down Expand Up @@ -298,6 +299,7 @@ export function maybeRunQueryTrigger(triggerType, className, restWhere, restOpti
if (restWhere) {
parseQuery._where = restWhere;
}
let count = false;
if (restOptions) {
if (restOptions.include && restOptions.include.length > 0) {
parseQuery._include = restOptions.include.split(',');
Expand All @@ -308,8 +310,9 @@ export function maybeRunQueryTrigger(triggerType, className, restWhere, restOpti
if (restOptions.limit) {
parseQuery._limit = restOptions.limit;
}
count = !!restOptions.count;
}
const requestObject = getRequestQueryObject(triggerType, auth, parseQuery, config);
const requestObject = getRequestQueryObject(triggerType, auth, parseQuery, count, config);
return Promise.resolve().then(() => {
return trigger(requestObject);
}).then((result) => {
Expand Down