Merge pull request #3231 from strongloop/warn-on-misconfigured-accessToken-user

Warn on misconfigured access token user

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'));

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;
+}

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 Token, ACL;
-var ds = loopback.createDataSource({connector: loopback.Memory});
-Token.attachTo(ds);
-var ACL = loopback.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,22 @@ describe('AccessToken', function() {
 describe('app.enableAuth()', function() {
   var app;
   beforeEach(function setupAuthWithModels() {
-    app = loopback();
+    app = loopback({localRegistry: true, loadBuiltinModels: true});
-    app.enableAuth({dataSource: ds});
+    app.dataSource('db', {connector: 'memory'});
+
+    Token = app.registry.createModel({
+      name: 'MyToken',
+      base: 'AccessToken',
+    });
+    app.model(Token, {dataSource: 'db'});
+
+    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);
 
@@ -517,7 +535,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 +545,6 @@ describe('app.enableAuth()', function() {
       http: {verb: 'GET', path: '/token'},
     });
 
-    var app = loopback();
     app.model(TestModel, {dataSource: null});
 
     app.enableAuth();
@@ -552,8 +569,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 +618,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 +651,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 +673,8 @@ function createTestApp(testToken, settings, done) {
     modelOptions[key] = modelSettings[key];
   });
 
-  var TestModel = loopback.PersistedModel.extend('test', {}, modelOptions);
+  var TestModel = app.registry.createModel('test', {}, modelOptions);
-
+  app.model(TestModel, {dataSource: 'db'});
-  TestModel.attachTo(loopback.memory());
-  app.model(TestModel);
 
   return app;
 }

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');

test/fixtures/access-control/common/models/access-token.json

@@ -15,5 +15,12 @@
       "principalId": "$everyone",
       "property": "create"
     }
-  ]
+  ],
-}
+  "relations": {
+    "user": {
+      "type": "belongsTo",
+      "model": "user",
+      "foreignKey": "userId"
+    }
+  }
+}

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'});
   }
 

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({