Skip to content

Commit

Permalink
Fix Role.isOwner() when multiple user models
Browse files Browse the repository at this point in the history
  • Loading branch information
@ebarault
ebarault committed Feb 16, 2017
1 parent 6453e8e commit a885e9e
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 40 deletions.
55 changes: 37 additions & 18 deletions common/models/role.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,8 @@ module.exports = function(Role) {
}
var modelClass = context.model;
var modelId = context.modelId;
var userId = context.getUserId();
Role.isOwner(modelClass, modelId, userId, callback);
var user = context.getUser();
Role.isOwner(modelClass, modelId, user.id, user.principalType, callback);
});

function isUserClass(modelClass) {
Expand Down Expand Up @@ -210,16 +210,24 @@ module.exports = function(Role) {
* @param {Function} modelClass The model class
* @param {*} modelId The model ID
* @param {*} userId The user ID
* @param {String} principalType The user principalType
* @callback {Function} [callback] The callback function
* @param {String|Error} err The error string or object
* @param {Boolean} isOwner True if the user is an owner.
* @promise
*/
Role.isOwner = function isOwner(modelClass, modelId, userId, callback) {
Role.isOwner = function isOwner(modelClass, modelId, userId, principalType, callback) {
if (!callback && typeof principalType === 'function') {
callback = principalType;
principalType = undefined;
}
principalType = principalType || Principal.USER;

assert(modelClass, 'Model class is required');
if (!callback) callback = utils.createPromiseCallback();

debug('isOwner(): %s %s userId: %s', modelClass && modelClass.modelName, modelId, userId);
debug('isOwner(): %s %s userId: %s principalType: %s',
modelClass && modelClass.modelName, modelId, userId, principalType);

// No userId is present
if (!userId) {
Expand All @@ -231,9 +239,13 @@ module.exports = function(Role) {

// Is the modelClass User or a subclass of User?
if (isUserClass(modelClass)) {
process.nextTick(function() {
callback(null, matches(modelId, userId));
});
var userModelName = modelClass.modelName;
// matching ids is enough if principalType is USER or matches given user model name
if (principalType === Principal.USER || principalType === userModelName) {
process.nextTick(function() {
callback(null, matches(modelId, userId));
});
}
return callback.promise;
}

Expand All @@ -244,31 +256,39 @@ module.exports = function(Role) {
return;
}
debug('Model found: %j', inst);

// loopback v2 implementation alows to resolve isOwner() if principalType is USER,
// instance ownerId (.userId or .owner) exists and is equal to user's id
var ownerId = inst.userId || inst.owner;
// Ensure ownerId exists and is not a function/relation
if (ownerId && 'function' !== typeof ownerId) {
if (callback) callback(null, matches(ownerId, userId));
if (principalType === Principal.USER && ownerId && 'function' !== typeof ownerId) {
callback(null, matches(ownerId, userId));
return;
} else {
// Try to follow belongsTo
for (var r in modelClass.relations) {
var rel = modelClass.relations[r];
if (rel.type === 'belongsTo' && isUserClass(rel.modelTo)) {
debug('Checking relation %s to %s: %j', r, rel.modelTo.modelName, rel);
// relation should be belongsTo and target a User based class
if (rel.type !== 'belongsTo' && !isUserClass(rel.modelTo)) {
continue;
}
// checking related user
var userModelName = rel.modelTo.modelName;
if (principalType === Principal.USER || principalType === userModelName) {
debug('Checking relation %s to %s: %j', r, userModelName, rel);
inst[r](processRelatedUser);
return;
}
}
debug('No matching belongsTo relation found for model %j and user: %j', modelId, userId);
if (callback) callback(null, false);
debug('No matching belongsTo relation found for model %j and user: %j principalType: %j',
modelId, userId, principalType);
callback(null, false);
}

function processRelatedUser(err, user) {
if (!err && user) {
debug('User found: %j', user.id);
if (callback) callback(null, matches(user.id, userId));
callback(null, matches(user.id, userId));
} else {
if (callback) callback(err, false);
callback(err, false);
}
}
});
Expand Down Expand Up @@ -371,7 +391,6 @@ module.exports = function(Role) {
var inRole = context.principals.some(function(p) {
var principalType = p.type || undefined;
var principalId = p.id || undefined;

// Check if it's the same role
return principalType === RoleMapping.ROLE && principalId === role;
});
Expand Down
14 changes: 11 additions & 3 deletions lib/access-context.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,14 @@ AccessContext.prototype.addPrincipal = function(principalType, principalId, prin
* @returns {*}
*/
AccessContext.prototype.getUserId = function() {
return this.getUser().id;
};

/**
* Get the user
* @returns {*}
*/
AccessContext.prototype.getUser = function() {
var BaseUser = this.registry.getModel('User');
for (var i = 0; i < this.principals.length; i++) {
var p = this.principals[i];
Expand All @@ -138,17 +146,17 @@ AccessContext.prototype.getUserId = function() {

// the principalType must either be 'USER'
if (p.type === Principal.USER) {
return p.id;
return {id: p.id, principalType: p.type};
}

// or permit to resolve a valid user model
var userModel = this.registry.findModel(p.type);
if (!userModel) continue;
if (userModel.prototype instanceof BaseUser) {
return p.id;
return {id: p.id, principalType: p.type};
}
}
return null;
return {};
};

/**
Expand Down
65 changes: 46 additions & 19 deletions test/multiple-user-principal-types.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,8 @@ describe('Multiple users with custom principalType', function() {
accessContext = new AccessContext({registry: OneUser.registry});
});

describe('getUserId()', function() {
it('returns userId although principals contain non USER principals',
describe('getUser()', function() {
it('returns user although principals contain non USER principals',
function() {
return Promise.try(function() {
addToAccessContext([
Expand All @@ -214,21 +214,27 @@ describe('Multiple users with custom principalType', function() {
{type: Principal.SCOPE},
{type: OneUser.modelName, id: userFromOneModel.id},
]);
var userId = accessContext.getUserId();
expect(userId).to.equal(userFromOneModel.id);
var user = accessContext.getUser();
expect(user).to.eql({
id: userFromOneModel.id,
principalType: OneUser.modelName,
});
});
});

it('returns userId although principals contain invalid principals',
it('returns user although principals contain invalid principals',
function() {
return Promise.try(function() {
addToAccessContext([
{type: 'AccessToken'},
{type: 'invalidModelName'},
{type: OneUser.modelName, id: userFromOneModel.id},
]);
var userId = accessContext.getUserId();
expect(userId).to.equal(userFromOneModel.id);
var user = accessContext.getUser();
expect(user).to.eql({
id: userFromOneModel.id,
principalType: OneUser.modelName,
});
});
});

Expand All @@ -238,8 +244,11 @@ describe('Multiple users with custom principalType', function() {
return ThirdUser.create(commonCredentials)
.then(function(userFromThirdModel) {
accessContext.addPrincipal(ThirdUser.modelName, userFromThirdModel.id);
var userId = accessContext.getUserId();
expect(userId).to.equal(userFromThirdModel.id);
var user = accessContext.getUser();
expect(user).to.eql({
id: userFromThirdModel.id,
principalType: ThirdUser.modelName,
});
});
});
});
Expand Down Expand Up @@ -357,6 +366,7 @@ describe('Multiple users with custom principalType', function() {
});

it('supports OWNER', function() {
var baseContext;
var Album = app.registry.createModel('Album', {
name: String,
userId: Number,
Expand All @@ -373,19 +383,36 @@ describe('Multiple users with custom principalType', function() {

return Album.create({name: 'album', userId: userFromOneModel.id})
.then(function(album) {
return Role.isInRole(
Role.OWNER,
{
principalType: OneUser.modelName,
principalId: userFromOneModel.id,
model: Album,
id: album.id,
});
baseContext = {
principalId: userFromOneModel.id,
model: Album,
id: album.id,
};
var validContext = buildContextWith({principalType: OneUser.modelName});
var invalidContext = buildContextWith({principalType: AnotherUser.modelName});

return isInRole([
{role: Role.OWNER, context: validContext},
{role: Role.OWNER, context: invalidContext},
]);
})
.then(function(isInRole) {
expect(isInRole).to.be.true();
.then(function(data) {
expect(data).to.eql([true, false]);
});

// helper
function buildContextWith(properties) {
return Object.assign(properties, baseContext);
}
});

// helper
function isInRole(requests) {
var isInRoles = requests.map(function(request) {
return Role.isInRole(request.role, request.context);
});
return Promise.all(isInRoles);
}
});

describe('isMappedToRole()', function() {
Expand Down

0 comments on commit a885e9e

Please sign in to comment.