verdnatura-mirror
/
loopback
mirror of https://github.com/strongloop/loopback.git
0
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix OWNER role to handle multiple relations 

Fix the code resolving OWNER role to correctly handle the situation
where the target model has multiple "belongsTo" relations to the User
model.

Introduce a new model setting "ownerRelations" that enables the new
behavior. When "ownerRelations" is set to true, then all "belongsTo"
relations are considered as granting ownership. Alternatively,
"ownerRelations" can be set to an array of the relations which
are granting ownership.

For example, a document can "belongTo" an author and a reviewer,
but only the author is an owner, the reviewer is not. In this case,
"ownerRelations" should be set to "['author']".
This commit is contained in:
pierreclr 2017-01-26 13:07:20 +01:00 committed by Miroslav Bajtoš
parent ef7175a4d5
commit e17132d061
No known key found for this signature in database
GPG Key ID: 6F2304BA9361C7E3
4 changed files with 555 additions and 81 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

114
common/models/role.js
View File

@ -222,6 +222,8 @@ module.exports = function(Role) {
* @promise
*/
Role.isOwner = function isOwner(modelClass, modelId, userId, principalType, options, callback) {
var _this = this;
if (!callback && typeof options === 'function') {
callback = options;
options = {};
@ -238,7 +240,7 @@ module.exports = function(Role) {
debug('isOwner(): %s %s userId: %s principalType: %s',
modelClass && modelClass.modelName, modelId, userId, principalType);
// Return false if userId is missing
// Resolve isOwner false if userId is missing
if (!userId) {
process.nextTick(function() {
callback(null, false);
@ -246,6 +248,23 @@ module.exports = function(Role) {
return callback.promise;
}
// At this stage, principalType is valid in one of 2 following condition:
// 1. the app has a single user model and principalType is 'USER'
// 2. the app has multiple user models and principalType is not 'USER'
// multiple user models
var isMultipleUsers = _isMultipleUsers();
var isPrincipalTypeValid =
(!isMultipleUsers && principalType === Principal.USER) ||
(isMultipleUsers && principalType !== Principal.USER);
// Resolve isOwner false if principalType is invalid
if (!isPrincipalTypeValid) {
process.nextTick(function() {
callback(null, false);
});
return callback.promise;
}
// Is the modelClass User or a subclass of User?
if (isUserClass(modelClass)) {
var userModelName = modelClass.modelName;
@ -265,10 +284,25 @@ module.exports = function(Role) {
}
debug('Model found: %j', inst);
// Historically, for principalType USER, we were resolving isOwner()
var ownerRelations = modelClass.settings.ownerRelations;
if (!ownerRelations) {
return legacyOwnershipCheck(inst);
} else {
return checkOwnership(inst);
}
});
return callback.promise;
// NOTE Historically, for principalType USER, we were resolving isOwner()
// as true if the model has "userId" or "owner" property matching
// id of the current user (principalId), even though there was no
// belongsTo relation set up.
// Additionaly, the original implementation did not support the
// possibility for a model to have multiple related users: when
// testing belongsTo relations, the first related user failing the
// ownership check induced the whole isOwner() to resolve as false.
// This behaviour will be pruned at next LoopBack major release.
function legacyOwnershipCheck(inst) {
var ownerId = inst.userId || inst.owner;
if (principalType === Principal.USER && ownerId && 'function' !== typeof ownerId) {
return callback(null, matches(ownerId, userId));
@ -282,9 +316,16 @@ module.exports = function(Role) {
if (!belongsToUser) {
continue;
}
// checking related user
var userModelName = rel.modelTo.modelName;
if (principalType === Principal.USER || principalType === userModelName) {
var relatedUser = rel.modelTo;
var userModelName = relatedUser.modelName;
var isMultipleUsers = _isMultipleUsers(relatedUser);
// a relation can be considered for isOwner resolution if:
// 1. the app has a single user model and principalType is 'USER'
// 2. the app has multiple user models and principalType is the related user model name
if ((!isMultipleUsers && principalType === Principal.USER) ||
(isMultipleUsers && principalType === userModelName)) {
debug('Checking relation %s to %s: %j', r, userModelName, rel);
inst[r](processRelatedUser);
return;
@ -295,15 +336,72 @@ module.exports = function(Role) {
callback(null, false);
function processRelatedUser(err, user) {
if (!err && user) {
if (err || !user) return callback(err, false);
debug('User found: %j', user.id);
callback(null, matches(user.id, userId));
} else {
callback(err, false);
}
}
function checkOwnership(inst) {
var ownerRelations = inst.constructor.settings.ownerRelations;
// collecting related users
var relWithUsers = [];
for (var r in modelClass.relations) {
var rel = modelClass.relations[r];
// relation should be belongsTo and target a User based class
if (rel.type !== 'belongsTo' || !isUserClass(rel.modelTo)) {
continue;
}
// checking related user
var relatedUser = rel.modelTo;
var userModelName = relatedUser.modelName;
var isMultipleUsers = _isMultipleUsers(relatedUser);
// a relation can be considered for isOwner resolution if:
// 1. the app has a single user model and principalType is 'USER'
// 2. the app has multiple user models and principalType is the related user model name
// In addition, if an array of relations if provided with the ownerRelations option,
// then the given relation name is further checked against this array
if ((!isMultipleUsers && principalType === Principal.USER) ||
(isMultipleUsers && principalType === userModelName)) {
debug('Checking relation %s to %s: %j', r, userModelName, rel);
if (ownerRelations === true) {
relWithUsers.push(r);
} else if (Array.isArray(ownerRelations) && ownerRelations.indexOf(r) !== -1) {
relWithUsers.push(r);
}
}
}
if (relWithUsers.length === 0) {
debug('No matching belongsTo relation found for model %j and user: %j principalType: %j',
modelId, userId, principalType);
return callback(null, false);
}
// check related users: someSeries is used to avoid spamming the db
async.someSeries(relWithUsers, processRelation, callback);
function processRelation(r, cb) {
inst[r](function processRelatedUser(err, user) {
if (err || !user) return cb(err, false);
debug('User found: %j (through %j)', user.id, r);
cb(null, matches(user.id, userId));
});
return callback.promise;
}
}
// A helper function to check if the app user config is multiple users or
// single user. It can be used with or without a reference user model.
// In case no user model is provided, we use the registry to get any of the
// user model by type. The relation with AccessToken is used to check
// if polymorphism is used, and thus if multiple users.
function _isMultipleUsers(userModel) {
var oneOfUserModels = userModel || _this.registry.getModelByType('User');
var accessTokensRel = oneOfUserModels.relations.accessTokens;
return !!(accessTokensRel && accessTokensRel.polymorphic);
}
};
Role.registerResolver(Role.AUTHENTICATED, function(role, context, callback) {

10
lib/application.js
View File

@ -458,6 +458,16 @@ app._verifyAuthModelRelations = function() {
function verifyUserRelations(Model) {
const hasManyTokens = Model.relations && Model.relations.accessTokens;
// display a temp warning message for users using multiple users config
if (hasManyTokens.polymorphic) {
console.warn(
'The app configuration follows the multiple user models setup ' +
'as described in http://ibm.biz/setup-loopback-auth',
'The built-in role resolver $owner is not currently compatible ' +
'with this configuration and should not be used in production.');
}
if (hasManyTokens) return;
const relationsConfig = Model.settings.relations || {};

183
test/multiple-user-principal-types.test.js
View File

@ -8,6 +8,7 @@ var expect = require('./helpers/expect');
var request = require('supertest');
var loopback = require('../');
var ctx = require('../lib/access-context');
var extend = require('util')._extend;
var AccessContext = ctx.AccessContext;
var Principal = ctx.Principal;
var Promise = require('bluebird');
@ -22,6 +23,7 @@ describe('Multiple users with custom principalType', function() {
beforeEach(function setupAppAndModels() {
// create a local app object that does not share state with other tests
app = loopback({localRegistry: true, loadBuiltinModels: true});
app.set('_verifyAuthModelRelations', false);
app.dataSource('db', {connector: 'memory'});
var userModelOptions = {
@ -350,22 +352,23 @@ describe('Multiple users with custom principalType', function() {
});
});
describe('built-in role resolver', function() {
it('supports AUTHENTICATED', function() {
describe('built-in role resolvers', function() {
it('supports $authenticated', function() {
return Role.isInRole(Role.AUTHENTICATED, userOneBaseContext)
.then(function(isInRole) {
expect(isInRole).to.be.true();
});
});
it('supports UNAUTHENTICATED', function() {
it('supports $unauthenticated', function() {
return Role.isInRole(Role.UNAUTHENTICATED, userOneBaseContext)
.then(function(isInRole) {
expect(isInRole).to.be.false();
});
});
it('supports OWNER', function() {
describe('$owner', function() {
it('supports legacy behavior with relations', function() {
var Album = app.registry.createModel('Album', {
name: String,
userId: Number,
@ -395,7 +398,36 @@ describe('Multiple users with custom principalType', function() {
});
});
it('expects OWNER to resolve false if owner has incorrect principalType', function() {
// With multiple users config, we cannot resolve a user based just on
// his id, as many users from different models could have the same id.
it('legacy behavior resolves false without belongsTo relation', function() {
var Album = app.registry.createModel('Album', {
name: String,
userId: Number,
owner: Number,
});
app.model(Album, {dataSource: 'db'});
return Album.create({
name: 'album',
userId: userFromOneModel.id,
owner: userFromOneModel.id,
})
.then(function(album) {
var authContext = {
principalType: OneUser.modelName,
principalId: userFromOneModel.id,
model: Album,
id: album.id,
};
return Role.isInRole(Role.OWNER, authContext);
})
.then(function(isOwner) {
expect(isOwner).to.be.false();
});
});
it('legacy behavior resolves false if owner has incorrect principalType', function() {
var Album = app.registry.createModel('Album', {
name: String,
userId: Number,
@ -412,18 +444,149 @@ describe('Multiple users with custom principalType', function() {
return Album.create({name: 'album', userId: userFromOneModel.id})
.then(function(album) {
var invalidContext = {
principalType: AnotherUser.modelName,
var invalidPrincipalTypes = [
'invalidContextName',
'USER',
AnotherUser.modelName,
];
var invalidContexts = invalidPrincipalTypes.map(principalType => {
return {
principalType,
principalId: userFromOneModel.id,
model: Album,
id: album.id,
};
return Role.isInRole(Role.OWNER, invalidContext);
});
return Promise.map(invalidContexts, context => {
return Role.isInRole(Role.OWNER, context)
.then(isOwner => {
return {
principalType: context.principalType,
isOwner,
};
});
});
})
.then(function(isOwner) {
expect(isOwner).to.be.false();
.then(result => {
expect(result).to.eql([
{principalType: 'invalidContextName', isOwner: false},
{principalType: 'USER', isOwner: false},
{principalType: AnotherUser.modelName, isOwner: false},
]);
});
});
it.skip('resolves the owner using the corrent belongsTo relation',
function() {
// passing {ownerRelations: true} will enable the new $owner role resolver
// with any belongsTo relation allowing to resolve truthy
var Message = createModelWithOptions(
'ModelWithAllRelations',
{ownerRelations: true}
);
var messages = [
{content: 'firstMessage', customerId: userFromOneModel.id},
{
content: 'secondMessage',
customerId: userFromOneModel.id,
shopKeeperId: userFromAnotherModel.id,
},
// this is the incriminated message where two foreignKeys have the
// same id but points towards two different user models. Although
// customers should come from userFromOneModel and shopKeeperIds should
// come from userFromAnotherModel. The inverted situation still resolves
// isOwner true for both the customer and the shopKeeper
{
content: 'thirdMessage',
customerId: userFromAnotherModel.id,
shopKeeperId: userFromOneModel.id,
},
{content: 'fourthMessage', customerId: userFromAnotherModel.id},
{content: 'fifthMessage'},
];
return Promise.map(messages, msg => {
return Message.create(msg);
})
.then(messages => {
return Promise.all([
isOwnerForMessage(userFromOneModel, messages[0]),
isOwnerForMessage(userFromAnotherModel, messages[0]),
isOwnerForMessage(userFromOneModel, messages[1]),
isOwnerForMessage(userFromAnotherModel, messages[1]),
isOwnerForMessage(userFromOneModel, messages[2]),
isOwnerForMessage(userFromAnotherModel, messages[2]),
isOwnerForMessage(userFromAnotherModel, messages[3]),
isOwnerForMessage(userFromOneModel, messages[4]),
]);
})
.then(result => {
expect(result).to.eql([
{userFrom: 'OneUser', msg: 'firstMessage', isOwner: true},
{userFrom: 'AnotherUser', msg: 'firstMessage', isOwner: false},
{userFrom: 'OneUser', msg: 'secondMessage', isOwner: true},
{userFrom: 'AnotherUser', msg: 'secondMessage', isOwner: true},
// these 2 tests fail because we cannot resolve ownership with
// multiple owners on a single model instance with a classic
// belongsTo relation, we need to use belongsTo with polymorphic
// discriminator to distinguish between the 2 models
{userFrom: 'OneUser', msg: 'thirdMessage', isOwner: false},
{userFrom: 'AnotherUser', msg: 'thirdMessage', isOwner: false},
{userFrom: 'AnotherUser', msg: 'fourthMessage', isOwner: false},
{userFrom: 'OneUser', msg: 'fifthMessage', isOwner: false},
]);
});
});
});
// helpers
function isOwnerForMessage(user, msg) {
var accessContext = {
principalType: user.constructor.modelName,
principalId: user.id,
model: msg.constructor,
id: msg.id,
};
return Role.isInRole(Role.OWNER, accessContext)
.then(isOwner => {
return {
userFrom: user.constructor.modelName,
msg: msg.content,
isOwner,
};
});
}
function createModelWithOptions(name, options) {
var baseOptions = {
relations: {
sender: {
type: 'belongsTo',
model: 'OneUser',
foreignKey: 'customerId',
},
receiver: {
type: 'belongsTo',
model: 'AnotherUser',
foreignKey: 'shopKeeperId',
},
},
};
options = extend(baseOptions, options);
var Model = app.registry.createModel(
name,
{content: String, senderType: String},
options
);
app.model(Model, {dataSource: 'db'});
return Model;
}
});
describe('isMappedToRole()', function() {

221
test/role.test.js
View File

@ -8,6 +8,7 @@ var assert = require('assert');
var sinon = require('sinon');
var loopback = require('../index');
var async = require('async');
var extend = require('util')._extend;
var expect = require('./helpers/expect');
var Promise = require('bluebird');
@ -363,7 +364,8 @@ describe('role model', function() {
});
});
it('should support owner role resolver', function(done) {
// this test should be split to address one resolver at a time
it('supports built-in role resolvers', function(done) {
Role.registerResolver('returnPromise', function(role, context) {
return new Promise(function(resolve) {
process.nextTick(function() {
@ -489,14 +491,22 @@ describe('role model', function() {
});
});
it('resolves OWNER via "userId" property with no relation', function() {
describe('$owner role resolver', function() {
var sender, receiver;
var users = [
{username: 'sender', email: 'sender@example.com', password: 'pass'},
{username: 'receiver', email: 'receiver@example.com', password: 'pass'},
];
describe('ownerRelations not set (legacy behaviour)', () => {
it('resolves the owner via property "userId"', function() {
var user;
var Album = app.registry.createModel('Album', {
name: String,
userId: Number,
});
app.model(Album, {dataSource: 'db'});
var user;
return User.create({email: 'test@example.com', password: 'pass'})
.then(u => {
user = u;
@ -504,21 +514,23 @@ describe('role model', function() {
})
.then(album => {
return Role.isInRole(Role.OWNER, {
principalType: ACL.USER, principalId: user.id,
model: Album, id: album.id,
principalType: ACL.USER,
principalId: user.id,
model: Album,
id: album.id,
});
})
.then(isInRole => expect(isInRole).to.be.true());
});
it('resolves OWNER via "owner" property with no relation', function() {
it('resolves the owner via property "owner"', function() {
var user;
var Album = app.registry.createModel('Album', {
name: String,
owner: Number,
});
app.model(Album, {dataSource: 'db'});
var user;
return User.create({email: 'test@example.com', password: 'pass'})
.then(u => {
user = u;
@ -526,13 +538,204 @@ describe('role model', function() {
})
.then(album => {
return Role.isInRole(Role.OWNER, {
principalType: ACL.USER, principalId: user.id,
model: Album, id: album.id,
principalType: ACL.USER,
principalId: user.id,
model: Album,
id: album.id,
});
})
.then(isInRole => expect(isInRole).to.be.true());
});
it('resolves the owner via a belongsTo relation', function() {
// passing no options will result calling
// the legacy $owner role resolver behavior
var Message = givenModelWithSenderReceiverRelations('ModelWithNoOptions');
return givenUsers()
.then(() => {
var messages = [
{content: 'firstMessage', senderId: sender.id},
{content: 'secondMessage', receiverId: receiver.id},
{content: 'thirdMessage'},
];
return Promise.map(messages, msg => {
return Message.create(msg);
});
})
.then(messages => {
return Promise.all([
isOwnerForMessage(sender, messages[0]),
isOwnerForMessage(receiver, messages[1]),
isOwnerForMessage(receiver, messages[2]),
]);
})
.then(result => {
expect(result).to.eql([
{user: 'sender', msg: 'firstMessage', isOwner: true},
{user: 'receiver', msg: 'secondMessage', isOwner: false},
{user: 'receiver', msg: 'thirdMessage', isOwner: false},
]);
});
});
});
it('resolves as false without belongsTo relation', function() {
var user;
var Album = app.registry.createModel(
'Album',
{
name: String,
userId: Number,
owner: Number,
},
// passing {ownerRelations: true} will enable the new $owner role resolver
// and hence resolve false when no belongsTo relation is defined
{ownerRelations: true}
);
app.model(Album, {dataSource: 'db'});
return User.create({email: 'test@example.com', password: 'pass'})
.then(u => {
user = u;
return Album.create({name: 'Album 1', userId: user.id, owner: user.id});
})
.then(album => {
return Role.isInRole(Role.OWNER, {
principalType: ACL.USER,
principalId: user.id,
model: Album,
id: album.id,
});
})
.then(isInRole => expect(isInRole).to.be.false());
});
it('resolves the owner using the corrent belongsTo relation', function() {
// passing {ownerRelations: true} will enable the new $owner role resolver
// with any belongsTo relation allowing to resolve truthy
var Message = givenModelWithSenderReceiverRelations(
'ModelWithAllRelations',
{ownerRelations: true}
);
return givenUsers()
.then(() => {
var messages = [
{content: 'firstMessage', senderId: sender.id},
{content: 'secondMessage', receiverId: receiver.id},
{content: 'thirdMessage'},
];
return Promise.map(messages, msg => {
return Message.create(msg);
});
})
.then(messages => {
return Promise.all([
isOwnerForMessage(sender, messages[0]),
isOwnerForMessage(receiver, messages[1]),
isOwnerForMessage(receiver, messages[2]),
]);
})
.then(result => {
expect(result).to.eql([
{user: 'sender', msg: 'firstMessage', isOwner: true},
{user: 'receiver', msg: 'secondMessage', isOwner: true},
{user: 'receiver', msg: 'thirdMessage', isOwner: false},
]);
});
});
it('allows fine-grained control of which relations grant ownership',
function() {
// passing {ownerRelations: true} will enable the new $owner role resolver
// with a specified list of belongsTo relations allowing to resolve truthy
var Message = givenModelWithSenderReceiverRelations(
'ModelWithCoercedRelations',
{ownerRelations: ['receiver']}
);
return givenUsers()
.then(() => {
var messages = [
{content: 'firstMessage', senderId: sender.id},
{content: 'secondMessage', receiverId: receiver.id},
{content: 'thirdMessage'},
];
return Promise.map(messages, msg => {
return Message.create(msg);
});
})
.then(messages => {
return Promise.all([
isOwnerForMessage(sender, messages[0]),
isOwnerForMessage(receiver, messages[1]),
isOwnerForMessage(receiver, messages[2]),
]);
})
.then(result => {
expect(result).to.eql([
{user: 'sender', msg: 'firstMessage', isOwner: false},
{user: 'receiver', msg: 'secondMessage', isOwner: true},
{user: 'receiver', msg: 'thirdMessage', isOwner: false},
]);
});
});
// helpers
function givenUsers() {
return Promise.map(users, user => {
return User.create(user);
})
.then(users => {
sender = users[0];
receiver = users[1];
});
}
function isOwnerForMessage(user, msg) {
var accessContext = {
principalType: ACL.USER,
principalId: user.id,
model: msg.constructor,
id: msg.id,
};
return Role.isInRole(Role.OWNER, accessContext)
.then(isOwner => {
return {
user: user.username,
msg: msg.content,
isOwner,
};
});
}
function givenModelWithSenderReceiverRelations(name, options) {
var baseOptions = {
relations: {
sender: {
type: 'belongsTo',
model: 'User',
foreignKey: 'senderId',
},
receiver: {
type: 'belongsTo',
model: 'User',
foreignKey: 'receiverId',
},
},
};
options = extend(baseOptions, options);
var Model = app.registry.createModel(
name,
{content: String},
options
);
app.model(Model, {dataSource: 'db'});
return Model;
}
});
it('passes accessToken to modelClass.findById when resolving OWNER', () => {
const Album = app.registry.createModel('Album', {name: String});
app.model(Album, {dataSource: 'db'});