From f1e31ca50ca8a7bb58291f0b463245108287e312 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Tue, 17 Jan 2017 10:44:25 +0100 Subject: [PATCH] Add app setting logoutSessionsOnSensitiveChanges Disable invalidation of access tokens by default to restore backwards compatibility with older 2.x versions. Add a new application-wide flag logoutSessionsOnSensitiveChanges that can be used to explicitly turn on/off the token invalidation. When the flag is not set, a verbose warning is printed to nudge the user to make a decision how they want to handle token invalidation. --- common/models/user.js | 29 +++++++++++++++++++ test/access-token.test.js | 2 ++ test/app.test.js | 3 ++ .../access-control/server/config.json | 3 +- .../server/config.json | 1 + .../simple-integration-app/server/config.json | 3 +- .../user-integration-app/server/config.json | 3 +- test/model.test.js | 1 + test/replication.rest.test.js | 2 ++ test/rest.middleware.test.js | 1 + test/role.test.js | 2 ++ test/support.js | 1 + test/user.test.js | 12 ++++++++ 13 files changed, 60 insertions(+), 3 deletions(-) diff --git a/common/models/user.js b/common/models/user.js index 27fafa7c..6288734b 100644 --- a/common/models/user.js +++ b/common/models/user.js @@ -807,6 +807,31 @@ module.exports = function(User) { UserModel.validatesUniquenessOf('username', {message: 'User already exists'}); } + UserModel.once('attached', function() { + if (UserModel.app.get('logoutSessionsOnSensitiveChanges') !== undefined) + return; + + g.warn([ + '', + 'The user model %j is attached to an application that does not specify', + 'whether other sessions should be invalidated when a password or', + 'an email has changed. Session invalidation is important for security', + 'reasons as it allows users to recover from various account breach', + 'situations.', + '', + 'We recommend turning this feature on by setting', + '"{{logoutSessionsOnSensitiveChanges}}" to {{true}} in', + '{{server/config.json}} (unless you have implemented your own solution', + 'for token invalidation).', + '', + 'We also recommend enabling "{{injectOptionsFromRemoteContext}}" in', + '%s\'s settings (typically via common/models/*.json file).', + 'This setting is required for the invalidation algorithm to keep ', + 'the current session valid.', + '' + ].join('\n'), UserModel.modelName, UserModel.modelName); + }); + return UserModel; }; @@ -832,6 +857,8 @@ module.exports = function(User) { // Delete old sessions once email is updated User.observe('before save', function beforeEmailUpdate(ctx, next) { + if (!ctx.Model.app.get('logoutSessionsOnSensitiveChanges')) return next(); + var emailChanged; if (ctx.isNewInstance) return next(); if (!ctx.where && !ctx.instance) return next(); @@ -872,6 +899,8 @@ module.exports = function(User) { }); User.observe('after save', function afterEmailUpdate(ctx, next) { + if (!ctx.Model.app.get('logoutSessionsOnSensitiveChanges')) return next(); + if (!ctx.instance && !ctx.data) return next(); if (!ctx.hookState.originalUserData) return next(); diff --git a/test/access-token.test.js b/test/access-token.test.js index e86feca9..9bf2d0b8 100644 --- a/test/access-token.test.js +++ b/test/access-token.test.js @@ -590,6 +590,7 @@ function createTestApp(testToken, settings, done) { }, settings.token); var app = loopback(); + app.set('logoutSessionsOnSensitiveChanges', true); app.use(cookieParser('secret')); app.use(loopback.token(tokenSettings)); @@ -652,6 +653,7 @@ function createTestApp(testToken, settings, done) { function givenLocalTokenModel() { var app = loopback({ localRegistry: true, loadBuiltinModels: true }); + app.set('logoutSessionsOnSensitiveChanges', true); app.dataSource('db', { connector: 'memory' }); var User = app.registry.getModel('User'); diff --git a/test/app.test.js b/test/app.test.js index 66bac4e7..7df1e793 100644 --- a/test/app.test.js +++ b/test/app.test.js @@ -718,6 +718,7 @@ describe('app', function() { beforeEach(function() { app = loopback(); + app.set('logoutSessionsOnSensitiveChanges', true); app.dataSource('db', { connector: 'memory' }); @@ -922,6 +923,7 @@ describe('app', function() { var AUTH_MODELS = ['User', 'ACL', 'AccessToken', 'Role', 'RoleMapping']; var app = loopback({ localRegistry: true, loadBuiltinModels: true }); require('../lib/builtin-models')(app.registry); + app.set('logoutSessionsOnSensitiveChanges', true); var db = app.dataSource('db', { connector: 'memory' }); app.enableAuth({ dataSource: 'db' }); @@ -937,6 +939,7 @@ describe('app', function() { it('detects already configured subclass of a required model', function() { var app = loopback({ localRegistry: true, loadBuiltinModels: true }); + app.set('logoutSessionsOnSensitiveChanges', true); var db = app.dataSource('db', { connector: 'memory' }); var Customer = app.registry.createModel('Customer', {}, { base: 'User' }); app.model(Customer, { dataSource: 'db' }); diff --git a/test/fixtures/access-control/server/config.json b/test/fixtures/access-control/server/config.json index 67364ab4..087d25be 100644 --- a/test/fixtures/access-control/server/config.json +++ b/test/fixtures/access-control/server/config.json @@ -1,5 +1,6 @@ { "port": 3000, "host": "0.0.0.0", + "logoutSessionsOnSensitiveChanges": true, "legacyExplorer": false -} \ No newline at end of file +} diff --git a/test/fixtures/shared-methods/model-config-defined-false/server/config.json b/test/fixtures/shared-methods/model-config-defined-false/server/config.json index 61ed16d6..69e75d3a 100644 --- a/test/fixtures/shared-methods/model-config-defined-false/server/config.json +++ b/test/fixtures/shared-methods/model-config-defined-false/server/config.json @@ -23,6 +23,7 @@ "disableStackTrace": false } }, + "logoutSessionsOnSensitiveChanges": true, "legacyExplorer": false } diff --git a/test/fixtures/simple-integration-app/server/config.json b/test/fixtures/simple-integration-app/server/config.json index c7cce73e..2dfdfd1f 100644 --- a/test/fixtures/simple-integration-app/server/config.json +++ b/test/fixtures/simple-integration-app/server/config.json @@ -11,5 +11,6 @@ "limit": "8kb" } }, + "logoutSessionsOnSensitiveChanges": true, "legacyExplorer": false -} \ No newline at end of file +} diff --git a/test/fixtures/user-integration-app/server/config.json b/test/fixtures/user-integration-app/server/config.json index c7cce73e..2dfdfd1f 100644 --- a/test/fixtures/user-integration-app/server/config.json +++ b/test/fixtures/user-integration-app/server/config.json @@ -11,5 +11,6 @@ "limit": "8kb" } }, + "logoutSessionsOnSensitiveChanges": true, "legacyExplorer": false -} \ No newline at end of file +} diff --git a/test/model.test.js b/test/model.test.js index 2ed1f0f2..727d9fa9 100644 --- a/test/model.test.js +++ b/test/model.test.js @@ -856,6 +856,7 @@ describe.onServer('Remote Methods', function() { function setupAppAndRequest() { app = loopback({localRegistry: true, loadBuiltinModels: true}); + app.set('logoutSessionsOnSensitiveChanges', true); app.dataSource('db', {connector: 'memory'}); diff --git a/test/replication.rest.test.js b/test/replication.rest.test.js index 9c436b2f..a6b5026a 100644 --- a/test/replication.rest.test.js +++ b/test/replication.rest.test.js @@ -462,6 +462,7 @@ describe('Replication over REST', function() { function setupServer(done) { serverApp = loopback(); + serverApp.set('logoutSessionsOnSensitiveChanges', true); serverApp.enableAuth(); serverApp.dataSource('db', { connector: 'memory' }); @@ -514,6 +515,7 @@ describe('Replication over REST', function() { function setupClient() { clientApp = loopback(); + clientApp.set('logoutSessionsOnSensitiveChanges', true); clientApp.dataSource('db', { connector: 'memory' }); clientApp.dataSource('remote', { connector: 'remote', diff --git a/test/rest.middleware.test.js b/test/rest.middleware.test.js index 58fd91ab..56307d5c 100644 --- a/test/rest.middleware.test.js +++ b/test/rest.middleware.test.js @@ -13,6 +13,7 @@ describe('loopback.rest', function() { // override the global app object provided by test/support.js // and create a local one that does not share state with other tests app = loopback({ localRegistry: true, loadBuiltinModels: true }); + app.set('logoutSessionsOnSensitiveChanges', true); var db = app.dataSource('db', { connector: 'memory' }); MyModel = app.registry.createModel('MyModel'); MyModel.attachTo(db); diff --git a/test/role.test.js b/test/role.test.js index a7bbf8b7..aeb9f5cd 100644 --- a/test/role.test.js +++ b/test/role.test.js @@ -23,6 +23,7 @@ describe('role model', function() { // Use local app registry to ensure models are isolated to avoid // pollutions from other tests app = loopback({ localRegistry: true, loadBuiltinModels: true }); + app.set('logoutSessionsOnSensitiveChanges', true); app.dataSource('db', { connector: 'memory' }); ACL = app.registry.getModel('ACL'); @@ -735,6 +736,7 @@ describe('role model', function() { describe('isOwner', function() { it('supports app-local model registry', function(done) { var app = loopback({ localRegistry: true, loadBuiltinModels: true }); + app.set('logoutSessionsOnSensitiveChanges', true); app.dataSource('db', { connector: 'memory' }); // attach all auth-related models to 'db' datasource app.enableAuth({ dataSource: 'db' }); diff --git a/test/support.js b/test/support.js index 915f38dc..327f0fc0 100644 --- a/test/support.js +++ b/test/support.js @@ -23,6 +23,7 @@ loopback.User.settings.saltWorkFactor = 4; beforeEach(function() { this.app = app = loopback(); + app.set('logoutSessionsOnSensitiveChanges', true); // setup default data sources loopback.setDefaultDataSourceForType('db', { diff --git a/test/user.test.js b/test/user.test.js index 18967744..ec88d671 100644 --- a/test/user.test.js +++ b/test/user.test.js @@ -31,6 +31,7 @@ describe('User', function() { // override the global app object provided by test/support.js // and create a local one that does not share state with other tests app = loopback({ localRegistry: true, loadBuiltinModels: true }); + app.set('logoutSessionsOnSensitiveChanges', true); app.dataSource('db', { connector: 'memory' }); // setup Email model, it's needed by User tests @@ -2326,6 +2327,17 @@ describe('User', function() { }); }); + it('preserves all sessions when logoutSessionsOnSensitiveChanges is disabled', + function(done) { + app.set('logoutSessionsOnSensitiveChanges', false); + user.updateAttributes( + {email: updatedEmailCredentials.email}, + function(err, userInstance) { + if (err) return done(err); + assertPreservedTokens(done); + }); + }); + function assertPreservedTokens(done) { AccessToken.find({where: {userId: user.id}}, function(err, tokens) { if (err) return done(err);