Skip to content

Commit

Permalink
Fixes for Class Level and Pointer Permissions (#1989)
Browse files Browse the repository at this point in the history 
* Fixes for Pointer Permissions

- Fix bug that would leave public CLP when setting a new set of permissions
- Sets empty permissions if missing to match parse.com API
- Updates tests to reflect changes

* Adds regression test for #1991

* Fit -> It
  • Loading branch information
@flovilmart @drew-gross
flovilmart authored and drew-gross committed Jun 6, 2016
1 parent ac705a8 commit e7e2369
Show file tree
Hide file tree
Showing 6 changed files with 96 additions and 41 deletions.
18 changes: 9 additions & 9 deletions spec/PointerPermissions.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ describe('Pointer Permissions', () => {
it('tests CLP / Pointer Perms / ACL write (PP Locked)', (done) => {
/*
tests:
CLP: update open ({"*": true})
CLP: update closed ({})
PointerPerm: "owner"
ACL: logged in user has access
Expand All @@ -300,7 +300,7 @@ describe('Pointer Permissions', () => {
password: 'password'
});
let obj = new Parse.Object('AnObject');
Parse.Object.saveAll([user, user2]).then(() => {
Parse.Object.saveAll([user, user2]).then(() => {
let ACL = new Parse.ACL();
ACL.setReadAccess(user, true);
ACL.setWriteAccess(user, true);
Expand All @@ -310,7 +310,7 @@ describe('Pointer Permissions', () => {
}).then(() => {
return config.database.loadSchema().then((schema) => {
// Lock the update, and let only owner write
return schema.updateClass('AnObject', {}, {update: {"*": true}, writeUserFields: ['owner']});
return schema.updateClass('AnObject', {}, {update: {}, writeUserFields: ['owner']});
});
}).then(() => {
return Parse.User.logIn('user1', 'password');
Expand All @@ -329,7 +329,7 @@ describe('Pointer Permissions', () => {
it('tests CLP / Pointer Perms / ACL write (ACL Locked)', (done) => {
/*
tests:
CLP: update open ({"*": true})
CLP: update closed ({})
PointerPerm: "owner"
ACL: logged in user has access
*/
Expand All @@ -355,7 +355,7 @@ describe('Pointer Permissions', () => {
}).then(() => {
return config.database.loadSchema().then((schema) => {
// Lock the update, and let only owner write
return schema.updateClass('AnObject', {}, {update: {"*": true}, writeUserFields: ['owner']});
return schema.updateClass('AnObject', {}, {update: {}, writeUserFields: ['owner']});
});
}).then(() => {
return Parse.User.logIn('user2', 'password');
Expand All @@ -374,7 +374,7 @@ describe('Pointer Permissions', () => {
it('tests CLP / Pointer Perms / ACL write (ACL/PP OK)', (done) => {
/*
tests:
CLP: update open ({"*": true})
CLP: update closed ({})
PointerPerm: "owner"
ACL: logged in user has access
*/
Expand All @@ -400,7 +400,7 @@ describe('Pointer Permissions', () => {
}).then(() => {
return config.database.loadSchema().then((schema) => {
// Lock the update, and let only owner write
return schema.updateClass('AnObject', {}, {update: {"*": true}, writeUserFields: ['owner']});
return schema.updateClass('AnObject', {}, {update: {}, writeUserFields: ['owner']});
});
}).then(() => {
return Parse.User.logIn('user2', 'password');
Expand All @@ -419,7 +419,7 @@ describe('Pointer Permissions', () => {
it('tests CLP / Pointer Perms / ACL read (PP locked)', (done) => {
/*
tests:
CLP: find/get open ({"*": true})
CLP: find/get open ({})
PointerPerm: "owner" : read
ACL: logged in user has access
Expand Down Expand Up @@ -447,7 +447,7 @@ describe('Pointer Permissions', () => {
}).then(() => {
return config.database.loadSchema().then((schema) => {
// Lock the update, and let only owner write
return schema.updateClass('AnObject', {}, {find: {"*": true}, get: {"*": true}, readUserFields: ['owner']});
return schema.updateClass('AnObject', {}, {find: {}, get: {}, readUserFields: ['owner']});
});
}).then(() => {
return Parse.User.logIn('user1', 'password');
Expand Down
2 changes: 2 additions & 0 deletions spec/Schema.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ describe('SchemaController', () => {
var get = {};
get[user.id] = true;
return schema.setPermissions('Stuff', {
'create': {'*': true},
'find': find,
'get': get
});
Expand All @@ -152,6 +153,7 @@ describe('SchemaController', () => {
done();
}, (e) => {
fail('Class permissions should have allowed this get query');
done();
});
});
});
Expand Down
68 changes: 50 additions & 18 deletions spec/schemas.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -963,18 +963,10 @@ describe('schemas', () => {
create: {
'role:admin': true
},
get: {
'*': true
},
update: {
'*': true
},
addField: {
'*': true
},
delete: {
'*': true
}
get: {},
update: {},
delete: {},
addField: {}
});
done();
});
Expand Down Expand Up @@ -1018,6 +1010,9 @@ describe('schemas', () => {
json: true,
body: {
classLevelPermissions: {
create: {
'*': true
},
find: {
'*': true
},
Expand All @@ -1040,14 +1035,14 @@ describe('schemas', () => {
})
});

it('should not be able to add a field', done => {
it('should be able to add a field', done => {
request.post({
url: 'http://localhost:8378/1/schemas/AClass',
headers: masterKeyHeaders,
json: true,
body: {
classLevelPermissions: {
find: {
create: {
'*': true
},
addField: {
Expand Down Expand Up @@ -1243,7 +1238,7 @@ describe('schemas', () => {
}).then(() => {
return Parse.User.logIn('user', 'user').then(() => {
let obj = new Parse.Object('AClass');
return obj.save();
return obj.save(null, {useMasterKey: true});
})
}).then(() => {
let query = new Parse.Query('AClass');
Expand Down Expand Up @@ -1292,7 +1287,7 @@ describe('schemas', () => {
}).then(() => {
return Parse.User.logIn('user', 'user').then(() => {
let obj = new Parse.Object('AClass');
return obj.save();
return obj.save(null, {useMasterKey: true});
})
}).then(() => {
let query = new Parse.Query('AClass');
Expand Down Expand Up @@ -1357,7 +1352,7 @@ describe('schemas', () => {
}).then(() => {
return Parse.User.logIn('user', 'user').then(() => {
let obj = new Parse.Object('AClass');
return obj.save();
return obj.save(null, {useMasterKey: true});
})
}).then(() => {
let query = new Parse.Query('AClass');
Expand Down Expand Up @@ -1415,7 +1410,7 @@ describe('schemas', () => {
}).then(() => {
return Parse.User.logIn('user', 'user').then(() => {
let obj = new Parse.Object('AClass');
return obj.save();
return obj.save(null, {useMasterKey: true});
})
}).then(() => {
let query = new Parse.Query('AClass');
Expand Down Expand Up @@ -1544,6 +1539,7 @@ describe('schemas', () => {

it('can login when addFields is false (issue #1355)', (done) => {
setPermissionsOnClass('_User', {
'create': {'*': true},
'addField': {}
}).then(() => {
return Parse.User.signUp('foo', 'bar');
Expand Down Expand Up @@ -1573,4 +1569,40 @@ describe('schemas', () => {
});
});
});

it("regression test for #1991", done => {
let user = new Parse.User();
user.setUsername('user');
user.setPassword('user');
let role = new Parse.Role('admin', new Parse.ACL());
let obj = new Parse.Object('AnObject');
Parse.Object.saveAll([user, role]).then(() => {
role.relation('users').add(user);
return role.save(null, {useMasterKey: true});
}).then(() => {
return setPermissionsOnClass('AnObject', {
'get': {"*": true},
'find': {"*": true},
'create': {'*': true},
'update': {'role:admin': true},
'delete': {'role:admin': true}
})
}).then(() => {
return obj.save();
}).then(() => {
return Parse.User.logIn('user', 'user')
}).then(() => {
return obj.destroy();
}).then((result) => {
let query = new Parse.Query('AnObject');
return query.find();
}).then((results) => {
expect(results.length).toBe(0);
done();
}).catch((err) => {
fail('should not fail');
console.error(err);
done();
});
});
});
15 changes: 12 additions & 3 deletions src/Adapters/Storage/Mongo/MongoSchemaCollection.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,15 @@ function mongoSchemaFieldsToParseSchemaFields(schema) {
return response;
}

const emptyCLPS = Object.freeze({
find: {},
get: {},
create: {},
update: {},
delete: {},
addField: {},
});

const defaultCLPS = Object.freeze({
find: {'*': true},
get: {'*': true},
Expand All @@ -53,14 +62,14 @@ const defaultCLPS = Object.freeze({
});

function mongoSchemaToParseSchema(mongoSchema) {
let clpsFromMongoObject = {};
let clps = defaultCLPS;
if (mongoSchema._metadata && mongoSchema._metadata.class_permissions) {
clpsFromMongoObject = mongoSchema._metadata.class_permissions;
clps = {...emptyCLPS, ...mongoSchema._metadata.class_permissions};
}
return {
className: mongoSchema._id,
fields: mongoSchemaFieldsToParseSchemaFields(mongoSchema),
classLevelPermissions: {...defaultCLPS, ...clpsFromMongoObject},
classLevelPermissions: clps,
};
}

Expand Down
5 changes: 5 additions & 0 deletions src/Controllers/DatabaseController.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -704,6 +704,11 @@ DatabaseController.prototype.deleteSchema = function(className) {
}

DatabaseController.prototype.addPointerPermissions = function(schema, className, operation, query, aclGroup = []) {
// Check if class has public permission for operation
// If the BaseCLP pass, let go through
if (schema.testBaseCLP(className, aclGroup, operation)) {
return query;
}
let perms = schema.perms[className];
let field = ['get', 'find'].indexOf(operation) > -1 ? 'readUserFields' : 'writeUserFields';
let userACL = aclGroup.filter((acl) => {
Expand Down
29 changes: 18 additions & 11 deletions src/Controllers/SchemaController.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -632,30 +632,36 @@ class SchemaController {
}
return Promise.resolve(this);
}

// Validates an operation passes class-level-permissions set in the schema
validatePermission(className, aclGroup, operation) {
// Validates the base CLP for an operation
testBaseCLP(className, aclGroup, operation) {
if (!this.perms[className] || !this.perms[className][operation]) {
return Promise.resolve();
return true;
}
let classPerms = this.perms[className];
let perms = classPerms[operation];
// Handle the public scenario quickly
if (perms['*']) {
return Promise.resolve();
return true;
}
// Check permissions against the aclGroup provided (array of userId/roles)
let found = false;
for (let i = 0; i < aclGroup.length && !found; i++) {
if (perms[aclGroup[i]]) {
found = true;
}
if (aclGroup.some(acl => { return perms[acl] === true })) {
return true;
}
return false;
}

if (found) {
// Validates an operation passes class-level-permissions set in the schema
validatePermission(className, aclGroup, operation) {
if (this.testBaseCLP(className, aclGroup, operation)) {
return Promise.resolve();
}

if (!this.perms[className] || !this.perms[className][operation]) {
return true;
}
let classPerms = this.perms[className];
let perms = classPerms[operation];
// No matching CLP, let's check the Pointer permissions
// And handle those later
let permissionField = ['get', 'find'].indexOf(operation) > -1 ? 'readUserFields' : 'writeUserFields';
Expand All @@ -666,6 +672,7 @@ class SchemaController {
'Permission denied for this action.');
}

// Process the readUserFields later
if (Array.isArray(classPerms[permissionField]) && classPerms[permissionField].length > 0) {
return Promise.resolve();
}
Expand Down

0 comments on commit e7e2369

Please sign in to comment.