From c9f8453171860b5dd0dd3d5073ae42cb1d09ccc4 Mon Sep 17 00:00:00 2001 From: Florent Vilmart Date: Fri, 4 Mar 2016 18:53:37 -0500 Subject: [PATCH 1/2] Fix reversed roles lookup --- spec/ParseRole.spec.js | 65 +++++++++++++++++++++++++++++++++++++----- src/Auth.js | 14 +++++---- 2 files changed, 67 insertions(+), 12 deletions(-) diff --git a/spec/ParseRole.spec.js b/spec/ParseRole.spec.js index 8b4f989f93..7f19cd7957 100644 --- a/spec/ParseRole.spec.js +++ b/spec/ParseRole.spec.js @@ -1,4 +1,4 @@ - +"use strict"; // Roles are not accessible without the master key, so they are not intended // for use by clients. We can manually test them using the master key. @@ -64,26 +64,30 @@ describe('Parse Role testing', () => { var rolesNames = ["FooRole", "BarRole", "BazRole"]; - var createRole = function(name, parent, user) { + var createRole = function(name, sibling, user) { var role = new Parse.Role(name, new Parse.ACL()); if (user) { var users = role.relation('users'); users.add(user); } - if (parent) { - role.relation('roles').add(parent); + if (sibling) { + role.relation('roles').add(sibling); } return role.save({}, { useMasterKey: true }); } var roleIds = {}; createTestUser().then( (user) => { - - return createRole(rolesNames[0], null, null).then( (aRole) => { + // Put the user on the 1st role + return createRole(rolesNames[0], null, user).then( (aRole) => { roleIds[aRole.get("name")] = aRole.id; + // set the 1st role as a sibling of the second + // user will should have 2 role now return createRole(rolesNames[1], aRole, null); }).then( (anotherRole) => { roleIds[anotherRole.get("name")] = anotherRole.id; - return createRole(rolesNames[2], anotherRole, user); + // set this role as a sibling of the last + // the user should now have 3 roles + return createRole(rolesNames[2], anotherRole, null); }).then( (lastRole) => { roleIds[lastRole.get("name")] = lastRole.id; var auth = new Auth({ config: new Config("test"), isMaster: true, user: user }); @@ -118,6 +122,53 @@ describe('Parse Role testing', () => { }); }); }); + + it("Should properly resolve roles", (done) => { + let admin = new Parse.Role("Admin", new Parse.ACL()); + let moderator = new Parse.Role("Moderator", new Parse.ACL()); + let contentCreator = new Parse.Role('ContentManager', new Parse.ACL()); + + Parse.Object.saveAll([admin, moderator, contentCreator], {useMasterKey: true}).then(() => { + contentCreator.getRoles().add(moderator); + moderator.getRoles().add(admin); + return Parse.Object.saveAll([admin, moderator, contentCreator], {useMasterKey: true}); + }).then(() => { + var auth = new Auth({ config: new Config("test"), isMaster: true }); + // For each role, fetch their sibling, what they inherit + // return with result and roleId for later comparison + let promises = [admin, moderator, contentCreator].map((role) => { + return auth._getAllRoleNamesForId(role.id).then((result) => { + return Parse.Promise.as({ + id: role.id, + roleIds: result + }); + }) + }); + + return Parse.Promise.when(promises); + }).then((results) => { + + results.forEach((result) => { + let id = result.id; + let roleIds = result.roleIds; + if (id == admin.id) { + expect(roleIds.length).toBe(2); + expect(roleIds.indexOf(moderator.id)).not.toBe(-1); + expect(roleIds.indexOf(contentCreator.id)).not.toBe(-1); + } else if (id == moderator.id) { + expect(roleIds.length).toBe(1); + expect(roleIds.indexOf(contentCreator.id)).toBe(0); + } else if (id == contentCreator.id) { + expect(roleIds.length).toBe(0); + } + }); + done(); + }).fail((err) => { + console.error(err); + done(); + }) + + }); }); diff --git a/src/Auth.js b/src/Auth.js index 0b285789db..9fae8b3556 100644 --- a/src/Auth.js +++ b/src/Auth.js @@ -139,18 +139,18 @@ Auth.prototype._loadRoles = function() { }; // Given a role object id, get any other roles it is part of -// TODO: Make recursive to support role nesting beyond 1 level deep Auth.prototype._getAllRoleNamesForId = function(roleID) { + + // As per documentation, a Role inherits AnotherRole + // if this Role is in the roles pointer of this AnotherRole + // Let's find all the roles where this role is in a roles relation var rolePointer = { __type: 'Pointer', className: '_Role', objectId: roleID }; var restWhere = { - '$relatedTo': { - key: 'roles', - object: rolePointer - } + 'roles': rolePointer }; var query = new RestQuery(this.config, master(this.config), '_Role', restWhere, {}); @@ -161,6 +161,10 @@ Auth.prototype._getAllRoleNamesForId = function(roleID) { } var roleIDs = results.map(r => r.objectId); + // we found a list of roles where the roleID + // is referenced in the roles relation, + // Get the roles where those found roles are also + // referenced the same way var parentRolesPromises = roleIDs.map( (roleId) => { return this._getAllRoleNamesForId(roleId); }); From 17bc79b372becbfab526aa719e44a7306dd8f32e Mon Sep 17 00:00:00 2001 From: Florent Vilmart Date: Fri, 4 Mar 2016 19:40:22 -0500 Subject: [PATCH 2/2] Improves tests, ensure unicity of roleIds --- spec/ParseRole.spec.js | 29 ++++++++++++++++++----------- src/Auth.js | 9 ++------- 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/spec/ParseRole.spec.js b/spec/ParseRole.spec.js index 7f19cd7957..636b833827 100644 --- a/spec/ParseRole.spec.js +++ b/spec/ParseRole.spec.js @@ -126,20 +126,23 @@ describe('Parse Role testing', () => { it("Should properly resolve roles", (done) => { let admin = new Parse.Role("Admin", new Parse.ACL()); let moderator = new Parse.Role("Moderator", new Parse.ACL()); - let contentCreator = new Parse.Role('ContentManager', new Parse.ACL()); - - Parse.Object.saveAll([admin, moderator, contentCreator], {useMasterKey: true}).then(() => { - contentCreator.getRoles().add(moderator); - moderator.getRoles().add(admin); - return Parse.Object.saveAll([admin, moderator, contentCreator], {useMasterKey: true}); + let superModerator = new Parse.Role("SuperModerator", new Parse.ACL()); + let contentManager = new Parse.Role('ContentManager', new Parse.ACL()); + let superContentManager = new Parse.Role('SuperContentManager', new Parse.ACL()); + Parse.Object.saveAll([admin, moderator, contentManager, superModerator, superContentManager], {useMasterKey: true}).then(() => { + contentManager.getRoles().add([moderator, superContentManager]); + moderator.getRoles().add([admin, superModerator]); + superContentManager.getRoles().add(superModerator); + return Parse.Object.saveAll([admin, moderator, contentManager, superModerator, superContentManager], {useMasterKey: true}); }).then(() => { var auth = new Auth({ config: new Config("test"), isMaster: true }); // For each role, fetch their sibling, what they inherit // return with result and roleId for later comparison - let promises = [admin, moderator, contentCreator].map((role) => { + let promises = [admin, moderator, contentManager, superModerator].map((role) => { return auth._getAllRoleNamesForId(role.id).then((result) => { return Parse.Promise.as({ id: role.id, + name: role.get('name'), roleIds: result }); }) @@ -147,19 +150,23 @@ describe('Parse Role testing', () => { return Parse.Promise.when(promises); }).then((results) => { - results.forEach((result) => { let id = result.id; let roleIds = result.roleIds; if (id == admin.id) { expect(roleIds.length).toBe(2); expect(roleIds.indexOf(moderator.id)).not.toBe(-1); - expect(roleIds.indexOf(contentCreator.id)).not.toBe(-1); + expect(roleIds.indexOf(contentManager.id)).not.toBe(-1); } else if (id == moderator.id) { expect(roleIds.length).toBe(1); - expect(roleIds.indexOf(contentCreator.id)).toBe(0); - } else if (id == contentCreator.id) { + expect(roleIds.indexOf(contentManager.id)).toBe(0); + } else if (id == contentManager.id) { expect(roleIds.length).toBe(0); + } else if (id == superModerator.id) { + expect(roleIds.length).toBe(3); + expect(roleIds.indexOf(moderator.id)).not.toBe(-1); + expect(roleIds.indexOf(contentManager.id)).not.toBe(-1); + expect(roleIds.indexOf(superContentManager.id)).not.toBe(-1); } }); done(); diff --git a/src/Auth.js b/src/Auth.js index 9fae8b3556..b45f93f3f7 100644 --- a/src/Auth.js +++ b/src/Auth.js @@ -173,14 +173,9 @@ Auth.prototype._getAllRoleNamesForId = function(roleID) { }).then(function(results){ // Flatten let roleIDs = results.reduce( (memo, result) => { - if (typeof result == "object") { - memo = memo.concat(result); - } else { - memo.push(result); - } - return memo; + return memo.concat(result); }, []); - return Promise.resolve(roleIDs); + return Promise.resolve([...new Set(roleIDs)]); }); };