From abf8246382ee8a321bdeb03c98050b484ab19069 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Fri, 24 Feb 2017 14:17:12 +0100 Subject: [PATCH 1/4] Fix test/access-token.test to use local registry --- test/access-token.test.js | 50 +++++++++++++++++++++++---------------- 1 file changed, 30 insertions(+), 20 deletions(-) diff --git a/test/access-token.test.js b/test/access-token.test.js index d8584a17..a8f20752 100644 --- a/test/access-token.test.js +++ b/test/access-token.test.js @@ -13,17 +13,23 @@ var loopback = require('../'); var extend = require('util')._extend; var session = require('express-session'); var request = require('supertest'); -var app = loopback(); -var Token = loopback.AccessToken.extend('MyToken'); -var ds = loopback.createDataSource({connector: loopback.Memory}); -Token.attachTo(ds); -var ACL = loopback.ACL; +var Token, ACL; describe('loopback.token(options)', function() { var app; beforeEach(function(done) { - app = loopback(); + app = loopback({localRegistry: true, loadBuiltinModels: true}); + app.dataSource('db', {connector: 'memory'}); + + Token = app.registry.createModel({ + name: 'MyToken', + base: 'AccessToken', + }); + app.model(Token, {dataSource: 'db'}); + + ACL = app.registry.getModel('ACL'); + createTestingToken.call(this, done); }); @@ -284,7 +290,6 @@ describe('loopback.token(options)', function() { ' when enableDoubkecheck is true', function(done) { var token = this.token; - var app = loopback(); app.use(function(req, res, next) { req.accessToken = null; next(); @@ -319,7 +324,6 @@ describe('loopback.token(options)', function() { function(done) { var token = this.token; var tokenStub = {id: 'stub id'}; - var app = loopback(); app.use(function(req, res, next) { req.accessToken = tokenStub; @@ -439,8 +443,18 @@ describe('AccessToken', function() { describe('app.enableAuth()', function() { var app; beforeEach(function setupAuthWithModels() { - app = loopback(); - app.enableAuth({dataSource: ds}); + app = loopback({localRegistry: true, loadBuiltinModels: true}); + app.dataSource('db', {connector: 'memory'}); + + Token = app.registry.createModel({ + name: 'MyToken', + base: 'AccessToken', + }); + app.model(Token, {dataSource: 'db'}); + + ACL = app.registry.getModel('ACL'); + + app.enableAuth({dataSource: 'db'}); }); beforeEach(createTestingToken); @@ -517,7 +531,7 @@ describe('app.enableAuth()', function() { }); it('stores token in the context', function(done) { - var TestModel = loopback.createModel('TestModel', {base: 'Model'}); + var TestModel = app.registry.createModel('TestModel', {base: 'Model'}); TestModel.getToken = function(cb) { var ctx = LoopBackContext.getCurrentContext(); cb(null, ctx && ctx.get('accessToken') || null); @@ -527,7 +541,6 @@ describe('app.enableAuth()', function() { http: {verb: 'GET', path: '/token'}, }); - var app = loopback(); app.model(TestModel, {dataSource: null}); app.enableAuth(); @@ -552,8 +565,6 @@ describe('app.enableAuth()', function() { // See https://github.com/strongloop/loopback-context/issues/6 it('checks whether context is active', function(done) { - var app = loopback(); - app.enableAuth(); app.use(contextMiddleware()); app.use(session({ @@ -603,7 +614,8 @@ function createTestApp(testToken, settings, done) { currentUserLiteral: 'me', }, settings.token); - var app = loopback(); + var app = loopback({localRegistry: true, loadBuiltinModels: true}); + app.dataSource('db', {connector: 'memory'}); app.use(cookieParser('secret')); app.use(loopback.token(tokenSettings)); @@ -635,7 +647,7 @@ function createTestApp(testToken, settings, done) { res.status(200).send(result); }); app.use(loopback.rest()); - app.enableAuth(); + app.enableAuth({dataSource: 'db'}); Object.keys(appSettings).forEach(function(key) { app.set(key, appSettings[key]); @@ -657,10 +669,8 @@ function createTestApp(testToken, settings, done) { modelOptions[key] = modelSettings[key]; }); - var TestModel = loopback.PersistedModel.extend('test', {}, modelOptions); - - TestModel.attachTo(loopback.memory()); - app.model(TestModel); + var TestModel = app.registry.createModel('test', {}, modelOptions); + app.model(TestModel, {dataSource: 'db'}); return app; } From c45954cdaadba3f6a36949d13bba020dd324d4e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Fri, 24 Feb 2017 14:58:40 +0100 Subject: [PATCH 2/4] Use local registry in test/replication.rest.test --- test/replication.rest.test.js | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/test/replication.rest.test.js b/test/replication.rest.test.js index 55f87758..70723924 100644 --- a/test/replication.rest.test.js +++ b/test/replication.rest.test.js @@ -466,15 +466,13 @@ describe('Replication over REST', function() { }; function setupServer(done) { - serverApp = loopback(); + serverApp = loopback({localRegistry: true, loadBuiltinModels: true}); serverApp.set('remoting', {errorHandler: {debug: true, log: false}}); - serverApp.enableAuth(); - serverApp.dataSource('db', {connector: 'memory'}); // Setup a custom access-token model that is not shared // with the client app - var ServerToken = loopback.createModel('ServerToken', {}, { + var ServerToken = serverApp.registry.createModel('ServerToken', {}, { base: 'AccessToken', relations: { user: { @@ -485,18 +483,17 @@ describe('Replication over REST', function() { }, }); serverApp.model(ServerToken, {dataSource: 'db', public: false}); - serverApp.model(loopback.ACL, {dataSource: 'db', public: false}); - serverApp.model(loopback.Role, {dataSource: 'db', public: false}); - serverApp.model(loopback.RoleMapping, {dataSource: 'db', public: false}); - ServerUser = loopback.createModel('ServerUser', USER_PROPS, USER_OPTS); + ServerUser = serverApp.registry.createModel('ServerUser', USER_PROPS, USER_OPTS); serverApp.model(ServerUser, { dataSource: 'db', public: true, relations: {accessTokens: {model: 'ServerToken'}}, }); - ServerCar = loopback.createModel('ServerCar', CAR_PROPS, CAR_OPTS); + serverApp.enableAuth({dataSource: 'db'}); + + ServerCar = serverApp.registry.createModel('ServerCar', CAR_PROPS, CAR_OPTS); serverApp.model(ServerCar, {dataSource: 'db', public: true}); serverApp.use(function(req, res, next) { @@ -518,7 +515,7 @@ describe('Replication over REST', function() { } function setupClient() { - clientApp = loopback(); + clientApp = loopback({localRegistry: true, loadBuiltinModels: true}); clientApp.dataSource('db', {connector: 'memory'}); clientApp.dataSource('remote', { connector: 'remote', @@ -529,23 +526,26 @@ describe('Replication over REST', function() { // model. This causes the in-process replication to work differently // than client-server replication. // As a workaround, we manually setup unique Checkpoint for ClientModel. - var ClientCheckpoint = loopback.Checkpoint.extend('ClientCheckpoint'); + var ClientCheckpoint = clientApp.registry.createModel({ + name: 'ClientCheckpoint', + base: 'Checkpoint', + }); ClientCheckpoint.attachTo(clientApp.dataSources.db); - LocalUser = loopback.createModel('LocalUser', USER_PROPS, USER_OPTS); + LocalUser = clientApp.registry.createModel('LocalUser', USER_PROPS, USER_OPTS); if (LocalUser.Change) LocalUser.Change.Checkpoint = ClientCheckpoint; clientApp.model(LocalUser, {dataSource: 'db'}); - LocalCar = loopback.createModel('LocalCar', CAR_PROPS, CAR_OPTS); + LocalCar = clientApp.registry.createModel('LocalCar', CAR_PROPS, CAR_OPTS); LocalCar.Change.Checkpoint = ClientCheckpoint; clientApp.model(LocalCar, {dataSource: 'db'}); var remoteOpts = createRemoteModelOpts(USER_OPTS); - RemoteUser = loopback.createModel('RemoteUser', USER_PROPS, remoteOpts); + RemoteUser = clientApp.registry.createModel('RemoteUser', USER_PROPS, remoteOpts); clientApp.model(RemoteUser, {dataSource: 'remote'}); remoteOpts = createRemoteModelOpts(CAR_OPTS); - RemoteCar = loopback.createModel('RemoteCar', CAR_PROPS, remoteOpts); + RemoteCar = clientApp.registry.createModel('RemoteCar', CAR_PROPS, remoteOpts); clientApp.model(RemoteCar, {dataSource: 'remote'}); } From f0c9700e1d89de04768071377410f660cb62469a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Fri, 24 Feb 2017 15:11:06 +0100 Subject: [PATCH 3/4] Deep-clone model settings in lib/builtin-models Fix the code loading builtin models to always clone the JSON object used as model settings/definition. This is needed to allow applications to modify model settings while not affecting settings of new models created in the local registry of another app. --- lib/builtin-models.js | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/lib/builtin-models.js b/lib/builtin-models.js index f9342a84..0679f670 100644 --- a/lib/builtin-models.js +++ b/lib/builtin-models.js @@ -4,6 +4,9 @@ // License text available at https://opensource.org/licenses/MIT 'use strict'; + +const assert = require('assert'); + module.exports = function(registry) { // NOTE(bajtos) we must use static require() due to browserify limitations @@ -52,8 +55,33 @@ module.exports = function(registry) { require('../common/models/checkpoint.js')); function createModel(definitionJson, customizeFn) { + // Clone the JSON definition to allow applications + // to modify model settings while not affecting + // settings of new models created in the local registry + // of another app. + // This is needed because require() always returns the same + // object instance it loaded during the first call. + definitionJson = cloneDeepJson(definitionJson); + var Model = registry.createModel(definitionJson); customizeFn(Model); return Model; } }; + +// Because we are cloning objects created by JSON.parse, +// the cloning algorithm can stay much simpler than a general-purpose +// "cloneDeep" e.g. from lodash. +function cloneDeepJson(obj) { + const result = Array.isArray(obj) ? [] : {}; + assert.equal(Object.getPrototypeOf(result), Object.getPrototypeOf(obj)); + for (const key in obj) { + const value = obj[key]; + if (typeof value === 'object') { + result[key] = cloneDeepJson(value); + } else { + result[key] = value; + } + } + return result; +} From 79f441b9c4e614dd39e35ca22008fd17dfc403e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Fri, 24 Feb 2017 14:07:41 +0100 Subject: [PATCH 4/4] Verify User and AccessToken relations at startup Modify `app.enableAuth()` to verify that (custom) User and AccessToken models have correctly configured their hasMany/belongsTo relations and print a warning otherwise. --- lib/application.js | 79 +++++++++++++++++++ test/access-token.test.js | 4 + test/app.test.js | 4 + .../common/models/access-token.json | 11 ++- test/user.test.js | 1 + 5 files changed, 97 insertions(+), 2 deletions(-) diff --git a/lib/application.js b/lib/application.js index 92bf7200..62db7858 100644 --- a/lib/application.js +++ b/lib/application.js @@ -398,9 +398,88 @@ app.enableAuth = function(options) { } }; + this._verifyAuthModelRelations(); + this.isAuthEnabled = true; }; +app._verifyAuthModelRelations = function() { + // Allow unit-tests (but also LoopBack users) to disable the warnings + if (this.get('_verifyAuthModelRelations') === false) return; + + const AccessToken = this.registry.findModel('AccessToken'); + const User = this.registry.findModel('User'); + this.models().forEach(Model => { + if (Model === AccessToken || Model.prototype instanceof AccessToken) { + scheduleVerification(Model, verifyAccessTokenRelations); + } + + if (Model === User || Model.prototype instanceof User) { + scheduleVerification(Model, verifyUserRelations); + } + }); + + function scheduleVerification(Model, verifyFn) { + if (Model.dataSource) { + verifyFn(Model); + } else { + Model.on('attached', () => verifyFn(Model)); + } + } + + function verifyAccessTokenRelations(Model) { + const belongsToUser = Model.relations && Model.relations.user; + if (belongsToUser) return; + + const relationsConfig = Model.settings.relations || {}; + const userName = (relationsConfig.user || {}).model; + if (userName) { + console.warn( + 'The model %j configures "belongsTo User-like models" relation ' + + 'with target model %j. However, the model %j is not attached to ' + + 'the application and therefore cannot be used by this relation. ' + + 'This typically happens when the application has a custom ' + + 'custom User subclass, but does not fix AccessToken relations ' + + 'to use this new model.\n' + + 'Learn more at http://ibm.biz/setup-loopback-auth', + Model.modelName, userName, userName); + return; + } + + console.warn( + 'The model %j does not have "belongsTo User-like model" relation ' + + 'configured.\n' + + 'Learn more at http://ibm.biz/setup-loopback-auth', + Model.modelName); + } + + function verifyUserRelations(Model) { + const hasManyTokens = Model.relations && Model.relations.accessTokens; + if (hasManyTokens) return; + + const relationsConfig = Model.settings.relations || {}; + const accessTokenName = (relationsConfig.accessTokens || {}).model; + if (accessTokenName) { + console.warn( + 'The model %j configures "hasMany AccessToken-like models" relation ' + + 'with target model %j. However, the model %j is not attached to ' + + 'the application and therefore cannot be used by this relation. ' + + 'This typically happens when the application has a custom ' + + 'AccessToken subclass, but does not fix User relations to use this ' + + 'new model.\n' + + 'Learn more at http://ibm.biz/setup-loopback-auth', + Model.modelName, accessTokenName, accessTokenName); + return; + } + + console.warn( + 'The model %j does not have "hasMany AccessToken-like models" relation ' + + 'configured.\n' + + 'Learn more at http://ibm.biz/setup-loopback-auth', + Model.modelName); + } +}; + app.boot = function(options) { throw new Error( g.f('{{`app.boot`}} was removed, use the new module {{loopback-boot}} instead')); diff --git a/test/access-token.test.js b/test/access-token.test.js index a8f20752..dace75b5 100644 --- a/test/access-token.test.js +++ b/test/access-token.test.js @@ -454,6 +454,10 @@ describe('app.enableAuth()', function() { ACL = app.registry.getModel('ACL'); + // Fix User's "hasMany accessTokens" relation to use our new MyToken model + const User = app.registry.getModel('User'); + User.settings.relations.accessTokens.model = 'MyToken'; + app.enableAuth({dataSource: 'db'}); }); beforeEach(createTestingToken); diff --git a/test/app.test.js b/test/app.test.js index b160ce35..ab56eff3 100644 --- a/test/app.test.js +++ b/test/app.test.js @@ -888,6 +888,10 @@ describe('app', function() { var Customer = app.registry.createModel('Customer', {}, {base: 'User'}); app.model(Customer, {dataSource: 'db'}); + // Fix AccessToken's "belongsTo user" relation to use our new Customer model + const AccessToken = app.registry.getModel('AccessToken'); + AccessToken.settings.relations.user.model = 'Customer'; + app.enableAuth({dataSource: 'db'}); expect(Object.keys(app.models)).to.not.include('User'); diff --git a/test/fixtures/access-control/common/models/access-token.json b/test/fixtures/access-control/common/models/access-token.json index 6b42d503..6061de02 100644 --- a/test/fixtures/access-control/common/models/access-token.json +++ b/test/fixtures/access-control/common/models/access-token.json @@ -15,5 +15,12 @@ "principalId": "$everyone", "property": "create" } - ] -} \ No newline at end of file + ], + "relations": { + "user": { + "type": "belongsTo", + "model": "user", + "foreignKey": "userId" + } + } +} diff --git a/test/user.test.js b/test/user.test.js index f9b7d5d7..a87c0f82 100644 --- a/test/user.test.js +++ b/test/user.test.js @@ -2374,6 +2374,7 @@ describe('User', function() { it('handles subclassed user with no accessToken relation', () => { // setup a new LoopBack app, we don't want to use shared models app = loopback({localRegistry: true, loadBuiltinModels: true}); + app.set('_verifyAuthModelRelations', false); app.set('remoting', {errorHandler: {debug: true, log: false}}); app.dataSource('db', {connector: 'memory'}); const User = app.registry.createModel({