Skip to content

Validate User model config at app boot #3265

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

Closed
ebarault wants to merge 2 commits into from
Closed

Validate User model config at app boot #3265

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8441475
first commit for general discussion
ebarault Mar 9, 2017
a657e8a
starting unit tests
ebarault Mar 23, 2017
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
221 changes: 177 additions & 44 deletions lib/application.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ var classify = require('underscore.string/classify');
var camelize = require('underscore.string/camelize');
var path = require('path');
var util = require('util');
var Promise = require('bluebird');

/**
* The `App` object represents a Loopback application.
Expand Down Expand Up @@ -398,85 +399,217 @@ app.enableAuth = function(options) {
}
};

this._verifyAuthModelRelations();

this.isAuthEnabled = true;
return this._verifyAuthModelRelations(app._warnOnBadAuthModelRelations)
Copy link
Member

@bajtos bajtos Mar 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In existing applications that are not waiting for enableAuth to resolve, when the verification code throws an error, the rejected promise will end up as unhandled promise rejection. This typically produces only a console warning, it does not crash the application as an uncaught error would.

(node:90355) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 3): Error

I am proposing to introduce an options flag to control the sync/async behaviour. For example:

// old way - throws error
app.enableAuth({dataSource: 'db'});

// new way - returns rejected promise
app.enableAuth({dataSource: 'db', async: true})
  .then(...)
  .catch(...);

// alternatively, replace the flag with a new method name
app.enableAuthAsync({dataSource: 'db'});

(The "Async" prefix is already used by Bluebird's promisify API)

.then(() => {
this.isAuthEnabled = true;
});
Copy link
Contributor Author

@ebarault ebarault Mar 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as _verifyAuthModelRelations is now async and returns a promise, this change is required
also i made enableAuth() return a promise to, as it's de facto async too now, it's not a breaking change to me, the method can still be called in an app while not caring about the returned promise if most if not all use cases.

This is useful in the tests so we can wait for enableAuth to return before checking the displayed warnings

Copy link
Member

@bajtos bajtos Mar 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am quite concerned about this change. We use app.isAuthEnabled in other places (e.g.

loopback/server/middleware/rest.js

Lines 51 to 54 in c3ab932

if (app.isAuthEnabled) {
var AccessToken = registry.getModelByType('AccessToken');
handlers.push(loopback.token({model: AccessToken, app: app}));
}
), but possibly in any 3rd party LoopBack code to detect whether the app has authentication enabled. While our REST middleware makes this check few ticks later (at which time this promise chain has already finished), I think it's not safe to assume that everybody else is doing the same.

Having said that, I agree that making enableAuth async & promise returning is a good direction.

What is your reasoning for enabling isAuthEnabled only after auth-model relations have been verified? What would happen if we keep only _verifyAuthModelRelations async and set app.isAuthEnabled right on the first tick, as we are doing now?

BTW in the current proposal, the app will end up in an inconsistent state when _verifyAuthModelRelations -models and the remoting authorization hook are all set up, but the flag is saying otherwise.

Copy link
Contributor Author

@ebarault ebarault Mar 23, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, i agree this is the main point to discuss.

In loopback i think this is not a problem since the features/modules relying on this flag comes few ticks later, but i agree we should care about 3rd party apps.
I dropped this implementation quickly to make this visible and get your feedback.

In fact i noted when implementing the tests that i could not easily test directly the _verifyAuthModelRelations method alone since it require the auth models and relations are all set up. Which is usually a benefit of using enableAuth(), so i needed to call _verifyAuthModelRelations through enableAuth, and then way for the latter's returned promise to resolve to properly get the warning messages. So at first i just modified enableAuth to return a promise. Then i considered the meaning of the flag and decided to set it after the health check has resolved.

The rationale:
Leaving aside backward compat for a sec: a valid auth setup health check function would not result in setting an isAuthEnabled flag true if the auth model was incorrect is some ways. We could project for v4 that the app deliberately stops in situations that now trigger only warnings, until the user fixes the issues or activate some failover flags to bypass the situation. Thoughts?

👉 For now: Setting app.isAuthEnabling to true before the call _verifyAuthModelRelations would probably not have a big impact actually and could keep a variant where it still set it sync and let the method return async after the checks have completed:

  • either the relations are not well set for single user model and the flag will be set and the app will warn, this is the current behavior, also this does not mean that the flag should actually be set since the app is not configured properly
  • or the multiple users config if not good (including relations), the flag will have been set but the app will crash, so there's not risk in setting the flag sync

So this could be

  // ...
  // still sync up to this point
  this.isAuthEnabled = true;
  return this._verifyAuthModelRelations(app._warnOnBadAuthModelRelations); // async
}

As the enableAuth() method currently returns nothing, there's no risk that it will conflict in existing 3rd party apps.

Of course we can additionaly already implement the target implement with a flag and set the flag only latter and decide to halt the app if warnings occur with a feature flag (wrapping your other proposal about an option to return sync or async)

Copy link
Member

@bajtos bajtos Mar 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dropped this implementation quickly to make this visible and get your feedback.

👍

Leaving aside backward compat for a sec: a valid auth setup health check function would not result in setting an isAuthEnabled flag true if the auth model was incorrect is some ways. We could project for v4 that the app deliberately stops in situations that now trigger only warnings, until the user fixes the issues or activate some failover flags to bypass the situation. Thoughts?

I agree that if the backwards compatibility was not an issue (e.g. in v4), then enableAuth should fail when the configuration is invalid.

// still sync up to this point
this.isAuthEnabled = true;
return this._verifyAuthModelRelations(app._warnOnBadAuthModelRelations); // async

👍 let's take this approach now.

};

app._verifyAuthModelRelations = function() {
app._verifyAuthModelRelations = function(warnFn) {
let self = this;

// Allow unit-tests (but also LoopBack users) to disable the warnings
if (this.get('_verifyAuthModelRelations') === false) return;
const warnOnBadSetup = this.get('_warnOnBadAuthModelsSetup') !== false;
// Prevent the app from being killed although the configuration is inconsistent
const abortOnBadUserSetup = this.get('_abortOnBadUserSetup') !== false;

Copy link
Contributor Author

@ebarault ebarault Mar 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reworded the flags to avoid double negations

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);
}
const models = this.models();

if (Model === User || Model.prototype instanceof User) {
scheduleVerification(Model, verifyUserRelations);
}
});
return verifyUserModelsSetup().then(verifyRelationsSetup);

function verifyUserModelsSetup() {
let userModels = models.filter(model => {
return (model === User || model.prototype instanceof User) ||
(model === AccessToken || model.prototype instanceof AccessToken);
});

// we use Promise.reflect() here to let all the tests go without throwing,
// this way we can get all the logs for all models we are checking
return Promise.map(userModels,
model => scheduleVerification(model, hasMultipleUserModelsConfig).reflect()
)
.then(inspections => {
// proceed to next checkups if no error
let hasErrors = inspections.filter(inspection => inspection.isRejected());
if (!hasErrors.length) {
return;
}

// else log and eventually kill the app
warnFn('BAD_USER_MODELS_SETUP');

// detailed logs
inspections.forEach(inspection => {
console.log(inspection);
let modelSetup = (inspection.value() || inspection.reason()).modelSetup;
console.warn(
'Model %j of type %j is set to use the %j user model config',
modelSetup.model,
modelSetup.instanceOf,
modelSetup.hasMultipleUserModelsConfig ? 'Multiple' : 'Single'
);
});

delete this.hasMultipleUserModelsConfig;

if (abortOnBadUserSetup) {
const msg = 'Application setup is inconsistent, please see the log ' +
'for more information';
const error = new Error(msg);
error.code = 'BAD_USER_MODELS_SETUP';
throw error;
}
});
}

function verifyRelationsSetup() {
models.forEach(Model => {
if (Model === AccessToken || Model.prototype instanceof AccessToken) {
scheduleVerification(Model, verifyAccessTokenRelations);
}

if (Model === User || Model.prototype instanceof User) {
scheduleVerification(Model, verifyUserRelations);
}
});
}

// instant or scheduled Model verifications, as a promise
function scheduleVerification(Model, verifyFn) {
if (Model.dataSource) {
verifyFn(Model);
} else {
Model.on('attached', () => verifyFn(Model));
return new Promise((resolve, reject) => {
if (Model.dataSource) {
try {
resolve(verifyFn(Model));
} catch (e) {
Copy link
Member

@bajtos bajtos Mar 21, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the Promise constructor is required to catch errors thrown by the factory function, therefore it shouldn't be needed to catch them ourselves.

Copy link
Contributor Author

@ebarault ebarault Mar 23, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, i checked. If i just do resolve(veryFn(Model))
then the verifyFn throws an error, the following error will be caught when trying to reflect the inspection result in scheduleVerification(model, hasMultipleUserModelsConfig).reflect();:

TypeError: cannot get fulfillment value of a non-fulfilled promise

Copy link
Member

@bajtos bajtos Mar 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's weird. This is what Bluebird's docs say about the error:

https://github.com/petkaantonov/bluebird/blob/master/docs/docs/error-explanations.md#error-cannot-get-fulfillment-value-of-a-non-fulfilled-promise

You can get this error when you're trying to call .value or .error when inspecting a promise where the promise has not been fulfilled or rejected yet.

For example:

var p = Promise.delay(1000);
p.inspect().value();

Consider using .isPending() .isFulfilled() and .isRejected() in order to inspect the promise for status.

This looks like a possible bug in bluebird to me.

Copy link
Contributor Author

@ebarault ebarault Mar 24, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nevermind... thanks for digging, the error was on my side...

reject(e);
}
} else {
Model.on('attached', () => {
scheduleVerification(Model, verifyFn);
});
}
});
}
Copy link
Contributor Author

@ebarault ebarault Mar 9, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i needed to change @bajtos first implem to add async support as we now expect on this function to resolve when done, so we can use it to first check user models config and then check how the relations are set up


function hasMultipleUserModelsConfig(model) {
let hasMultipleUserModelsConfig, isInstanceOfUser;

// check is the model is set to use mutiple users config (with polymorphic relations)
if (model === User || model.prototype instanceof User) {
isInstanceOfUser = true;
const hasManyTokens = model.relations && model.relations.accessTokens;
// TODO: handle when hasManyTokens is undefined?
hasMultipleUserModelsConfig = !!(hasManyTokens.polymorphic);
} else if (model === AccessToken || model.prototype instanceof AccessToken) {
const belongsToUser = model.relations && model.relations.user;
// the test on belongsToUser is required as we allow AccessToken model not
// to define the relation with custom user model for backward compatibility
// see https://github.com/strongloop/loopback/pull/3227
hasMultipleUserModelsConfig = !!(belongsToUser && belongsToUser.polymorphic);
}

let modelSetup = {
model: model.modelName,
instanceOf: isInstanceOfUser ? 'User' : 'AccessToken',
hasMultipleUserModelsConfig,
};

// check if model config is consistent with already parsed models' config
if (self.hasMultipleUserModelsConfig === undefined) {
self.hasMultipleUserModelsConfig = hasMultipleUserModelsConfig;
} else if (self.hasMultipleUserModelsConfig !== hasMultipleUserModelsConfig) {
let error = new Error('User models setup is inconsistent');
error.modelSetup = modelSetup;
throw error;
}

// if no error, return the modelSetup for further logging purposes
return {modelSetup};
}

function verifyAccessTokenRelations(Model) {
if (!warnOnBadSetup) return;
const belongsToUser = Model.relations && Model.relations.user;
if (belongsToUser) return;

const modelFrom = Model.modelName;
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);
const modelTo = (relationsConfig.user || {}).model;
if (modelTo) {
warnFn('CUSTOM_USER_MODEL_NOT_AVAILABLE', {modelFrom, modelTo});
Copy link
Contributor Author

@ebarault ebarault Mar 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i suggest changing the var names to modelFrom and modelTo, rather than modelName and userName, as it makes the tests more readable.
if you'd like to keep the original names, i will map the original names so the tests are readable

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);
warnFn('MISSING_RELATION_TO_USER', {modelFrom});
}

function verifyUserRelations(Model) {
if (!warnOnBadSetup) return;

const hasManyTokens = Model.relations && Model.relations.accessTokens;
if (hasManyTokens) return;

const modelFrom = Model.modelName;
const relationsConfig = Model.settings.relations || {};
const accessTokenName = (relationsConfig.accessTokens || {}).model;
if (accessTokenName) {
console.warn(
const modelTo = (relationsConfig.accessTokens || {}).model;
if (modelTo) {
warnFn('CUSTOM_ACCESS_TOKEN_MODEL_NOT_AVAILABLE', {modelFrom, modelTo});
return;
}
Copy link
Contributor Author

@ebarault ebarault Mar 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dito


warnFn('MISSING_RELATION_TO_ACCESS_TOKEN', {modelFrom});
}
};

// function warnOnBadAuthModelRelations(code, args) {
app._warnOnBadAuthModelRelations = function(code, args) {
Copy link
Contributor Author

@ebarault ebarault Mar 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

built-in warning function ("private")
i actually need it to be attached on the app object so i can swap it in the tests when calling enableAuth()

Copy link
Member

@bajtos bajtos Mar 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you considered passing the custom implementation via enableAuth options instead?

Copy link
Member

@bajtos bajtos Mar 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a big deal though.

switch (code) {
case 'BAD_USER_MODELS_SETUP':
g.warn('Application setup is inconsistent: some models are set to use ' +
'the Multiple user models configuration while some other models are set to ' +
'use the Single user model configuration. The model config is listed below.');
break;

case 'CUSTOM_USER_MODEL_NOT_AVAILABLE':
g.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',
args.modelFrom, args.modelTo, args.modelTo);
break;

case 'MISSING_RELATION_TO_USER':
g.warn(
'The model %j does not have "belongsTo User-like model" relation ' +
'configured.\n' +
'Learn more at http://ibm.biz/setup-loopback-auth',
args.modelFrom);
break;

case 'CUSTOM_ACCESS_TOKEN_MODEL_NOT_AVAILABLE':
g.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;
}
args.modelFrom, args.modelTo, args.modelTo);
break;

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);
case 'MISSING_RELATION_TO_ACCESS_TOKEN':
g.warn(
'The model %j does not have "hasMany AccessToken-like models" relation ' +
'configured.\n' +
'Learn more at http://ibm.biz/setup-loopback-auth',
args.modelFrom);
}
};

Expand Down
89 changes: 77 additions & 12 deletions test/app.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ var expect = require('./helpers/expect');
var it = require('./util/it');
var request = require('supertest');

var Promise = require('bluebird');

describe('app', function() {
var app;
beforeEach(function() {
Expand Down Expand Up @@ -859,10 +861,11 @@ describe('app', function() {
});

describe.onServer('enableAuth', function() {
it('should set app.isAuthEnabled to true', function() {
it('sets app.isAuthEnabled to true', function() {
expect(app.isAuthEnabled).to.not.equal(true);
app.enableAuth();
expect(app.isAuthEnabled).to.equal(true);
return app.enableAuth().then(() => {
expect(app.isAuthEnabled).to.equal(true);
Copy link
Member

@bajtos bajtos Mar 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This goes back to my first comment https://github.com/strongloop/loopback/pull/3265/files#r107652656.

When we have to change a test in a significant way, as we are doing here, then I think it's a very strong sign that we are breaking backwards compatibility.

});
});

it('auto-configures required models to provided dataSource', function() {
Expand All @@ -871,14 +874,14 @@ describe('app', function() {
require('../lib/builtin-models')(app.registry);
var db = app.dataSource('db', {connector: 'memory'});

app.enableAuth({dataSource: 'db'});

expect(Object.keys(app.models)).to.include.members(AUTH_MODELS);
return app.enableAuth({dataSource: 'db'}).then(() => {
Copy link
Member

@bajtos bajtos Mar 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change shouldn't be needed, because we are attaching the models synchronously in the current implementation. Unless I am missing something?

expect(Object.keys(app.models)).to.include.members(AUTH_MODELS);

AUTH_MODELS.forEach(function(m) {
var Model = app.models[m];
expect(Model.dataSource, m + '.dataSource').to.equal(db);
expect(Model.shared, m + '.shared').to.equal(m === 'User');
AUTH_MODELS.forEach(function(m) {
var Model = app.models[m];
expect(Model.dataSource, m + '.dataSource').to.equal(db);
expect(Model.shared, m + '.shared').to.equal(m === 'User');
});
});
});

Expand All @@ -892,9 +895,71 @@ describe('app', function() {
const AccessToken = app.registry.getModel('AccessToken');
AccessToken.settings.relations.user.model = 'Customer';

app.enableAuth({dataSource: 'db'});
return app.enableAuth({dataSource: 'db'}).then(() => {
Copy link
Member

@bajtos bajtos Mar 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto, I think this change is not needed.

expect(Object.keys(app.models)).to.not.include('User');
});
});

describe('auth models config health check', function() {
var app, warnings;
beforeEach(function() {
app = loopback({localRegistry: true, loadBuiltinModels: true});
app.dataSource('db', {connector: 'memory'});

expect(Object.keys(app.models)).to.not.include('User');
warnings = [];
function warnFn(code, args) {
warnings.push(Object.assign(args || {}, {code}));
}
app._warnOnBadAuthModelRelations = warnFn;
});

it('sets app.hasMultipleUserModelsConfig to false if the app ' +
'has a single User model correctly configured', function() {
return app.enableAuth({dataSource: 'db'}).then(() => {
expect(app.hasMultipleUserModelsConfig).to.be.false();
});
});

it('sets app.hasMultipleUserModelsConfig to true if the app ' +
'has multiple User models correctly configured', function() {
var Merchant = createModel(app, 'Merchant', {base: 'User'});
var Customer = createModel(app, 'Customer', {base: 'User'});
var Token = createModel(app, 'Token', {base: 'AccessToken'});

// Update AccessToken and Users to bind them through polymorphic relations
Token.belongsTo('user', {idName: 'id', polymorphic: {idType: 'string',
foreignKey: 'userId', discriminator: 'principalType'}});
Merchant.hasMany('accessTokens', {model: Token, polymorphic: {foreignKey: 'userId',
discriminator: 'principalType'}});
Customer.hasMany('accessTokens', {model: Token, polymorphic: {foreignKey: 'userId',
discriminator: 'principalType'}});
return app.enableAuth({dataSource: 'db'}).then(() => {
expect(app.hasMultipleUserModelsConfig).to.be.true();
});
});

it('warns if a custom user model is referenced in the ' +
'AccessToken "user" relation, but is not available', function() {
// Set AccessToken's belongsTo relation "user" to use a custom user model
// This model is deliberately not available
const AccessToken = app.registry.getModel('AccessToken');
AccessToken.settings.relations.user.model = 'Customer';

return app.enableAuth({dataSource: 'db'}).then(() => {
expect(warnings).to.eql([{
code: 'CUSTOM_USER_MODEL_NOT_AVAILABLE',
modelFrom: 'AccessToken',
modelTo: 'Customer',
}]);
});
});

// helpers
function createModel(app, name, options) {
var model = app.registry.createModel(Object.assign({name: name}, options));
app.model(model, {dataSource: 'db'});
return model;
}
});
});

Expand Down
2 changes: 1 addition & 1 deletion test/user.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2442,7 +2442,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('_warnOnBadAuthModelsSetup', false);
Copy link
Member

@bajtos bajtos Mar 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you mind reverting this rename? I don't really care about the name, it's just to avoid code churn.

Copy link
Contributor Author

@ebarault ebarault Mar 23, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it would mean that it's the exact same name as the app method
app. _verifyAuthModelRelations()
is it problematic?

the rationale for this change is: the _verifyAuthModelRelations method, for a same name, has now a larger scope than the equally named flag

Copy link
Member

@bajtos bajtos Mar 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it would mean that it's the exact same name as the app method

That shouldn't matter, because the flag is stored in a config hash-map.

the rationale for this change is: the _verifyAuthModelRelations method, for a same name, has now a larger scope than the equally named flag

I see your point, let's make the proposed change then.

app.set('remoting', {errorHandler: {debug: true, log: false}});
app.dataSource('db', {connector: 'memory'});
const User = app.registry.createModel({
Expand Down