Skip to content
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.

Sign up for GitHub

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

Auth._loadRoles should not query the same role twice. #1366

Merged
merged 1 commit into from
Apr 5, 2016
Merged

Auth._loadRoles should not query the same role twice. #1366

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
103 changes: 85 additions & 18 deletions spec/ParseRole.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down Expand Up @@ -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
Expand All @@ -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")
Expand All @@ -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());
Expand All @@ -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
Expand All @@ -147,7 +214,7 @@ describe('Parse Role testing', () => {
});
})
});

return Parse.Promise.when(promises);
}).then((results) => {
results.forEach((result) => {
Expand All @@ -174,7 +241,7 @@ describe('Parse Role testing', () => {
console.error(err);
done();
})

});

it('can create role and query empty users', (done)=> {
Expand Down
17 changes: 11 additions & 6 deletions src/Auth.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 = [];
Expand Down Expand Up @@ -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
Expand All @@ -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);
Expand Down