diff --git a/spec/ParseRole.spec.js b/spec/ParseRole.spec.js index f48fbf7fae..e1f93350aa 100644 --- a/spec/ParseRole.spec.js +++ b/spec/ParseRole.spec.js @@ -2,6 +2,7 @@ // 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. +var RestQuery = require("../src/RestQuery"); var Auth = require("../src/Auth").Auth; var Config = require("../src/Config"); @@ -60,21 +61,87 @@ describe('Parse Role testing', () => { }); - it("should recursively load roles", (done) => { + 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 (sibling) { + role.relation('roles').add(sibling); + } + return role.save({}, { useMasterKey: true }); + }; - var rolesNames = ["FooRole", "BarRole", "BazRole"]; + it("should not recursively load the same role multiple times", (done) => { + var rootRole = "RootRole"; + var roleNames = ["FooRole", "BarRole", "BazRole"]; + var allRoles = [rootRole].concat(roleNames); - 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 (sibling) { - role.relation('roles').add(sibling); - } - return role.save({}, { useMasterKey: true }); - } + var roleObjs = {}; + var createAllRoles = function(user) { + var promises = allRoles.map(function(roleName) { + return createRole(roleName, null, user) + .then(function(roleObj) { + roleObjs[roleName] = roleObj; + return roleObj; + }); + }); + return Promise.all(promises); + }; + + var restExecute = spyOn(RestQuery.prototype, "execute").and.callThrough(); + + var user, + auth, + getAllRolesSpy; + createTestUser().then( (newUser) => { + user = newUser; + return createAllRoles(user); + }).then ( (roles) => { + var rootRoleObj = roleObjs[rootRole]; + roles.forEach(function(role, i) { + // Add all roles to the RootRole + if (role.id !== rootRoleObj.id) { + role.relation("roles").add(rootRoleObj); + } + // Add all "roleNames" roles to the previous role + if (i > 0) { + role.relation("roles").add(roles[i - 1]); + } + }); + + return Parse.Object.saveAll(roles, { useMasterKey: true }); + }).then( () => { + auth = new Auth({config: new Config("test"), isMaster: true, user: user}); + getAllRolesSpy = spyOn(auth, "_getAllRoleNamesForId").and.callThrough(); + + return auth._loadRoles(); + }).then ( (roles) => { + expect(roles.length).toEqual(4); + + allRoles.forEach(function(name) { + expect(roles.indexOf("role:"+name)).not.toBe(-1); + }); + + // 1 Query for the initial setup + // 4 Queries for all the specific roles + // 1 Query for the final $in + expect(restExecute.calls.count()).toEqual(6); + + // 4 One query for each of the roles + // 3 Queries for the "RootRole" + expect(getAllRolesSpy.calls.count()).toEqual(7); + done() + }).catch( () => { + fail("should succeed"); + done(); + }); + + }); + + it("should recursively load roles", (done) => { + var rolesNames = ["FooRole", "BarRole", "BazRole"]; var roleIds = {}; createTestUser().then( (user) => { // Put the user on the 1st role @@ -97,7 +164,7 @@ describe('Parse Role testing', () => { expect(roles.length).toEqual(3); rolesNames.forEach( (name) => { expect(roles.indexOf('role:'+name)).not.toBe(-1); - }) + }); done(); }, function(err){ fail("should succeed") @@ -122,7 +189,7 @@ 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()); @@ -134,7 +201,7 @@ describe('Parse Role testing', () => { moderator.getRoles().add([admin, superModerator]); superContentManager.getRoles().add(superModerator); return Parse.Object.saveAll([admin, moderator, contentManager, superModerator, superContentManager], {useMasterKey: true}); - }).then(() => { + }).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 @@ -147,7 +214,7 @@ describe('Parse Role testing', () => { }); }) }); - + return Parse.Promise.when(promises); }).then((results) => { results.forEach((result) => { @@ -174,7 +241,7 @@ describe('Parse Role testing', () => { console.error(err); done(); }) - + }); it('can create role and query empty users', (done)=> { diff --git a/src/Auth.js b/src/Auth.js index cd5da9ff40..389598921f 100644 --- a/src/Auth.js +++ b/src/Auth.js @@ -63,7 +63,7 @@ var getAuthForSessionToken = function({ config, sessionToken, installationId } = return nobody(config); } - var now = new Date(), + var now = new Date(), expiresAt = new Date(results[0].expiresAt.iso); if(expiresAt < now) { throw new Parse.Error(Parse.Error.INVALID_SESSION_TOKEN, @@ -117,8 +117,9 @@ Auth.prototype._loadRoles = function() { var roleIDs = results.map(r => r.objectId); var promises = [Promise.resolve(roleIDs)]; + var queriedRoles = {}; for (var role of roleIDs) { - promises.push(this._getAllRoleNamesForId(role)); + promises.push(this._getAllRoleNamesForId(role, queriedRoles)); } return Promise.all(promises).then((results) => { var allIDs = []; @@ -146,8 +147,12 @@ Auth.prototype._loadRoles = function() { }; // Given a role object id, get any other roles it is part of -Auth.prototype._getAllRoleNamesForId = function(roleID) { - +Auth.prototype._getAllRoleNamesForId = function(roleID, queriedRoles = {}) { + // Don't need to requery this role as it is already being queried for. + if (queriedRoles[roleID] != null) { + return Promise.resolve([]); + } + queriedRoles[roleID] = true; // 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 @@ -167,13 +172,13 @@ Auth.prototype._getAllRoleNamesForId = function(roleID) { return Promise.resolve([]); } 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); + return this._getAllRoleNamesForId(roleId, queriedRoles); }); parentRolesPromises.push(Promise.resolve(roleIDs)); return Promise.all(parentRolesPromises);