Skip to content

Commit

Permalink
Add app setting logoutSessionsOnSensitiveChanges
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bajtos committed Jan 17, 2017
1 parent d35e1a1 commit dd657a9
Show file tree
Hide file tree
Showing 13 changed files with 60 additions and 3 deletions.
29 changes: 29 additions & 0 deletions common/models/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 was changed. Invalidating sessions is important for security',
'reasons, to allow users to recover from the situation where',
'an attacker got access to their account.',
'',
'We are recommending to turn this feature on by setting',
'"{{logoutSessionsOnSensitiveChanges}}" to {{true}} in',
'{{server/config.json}}, unless you have implemented your own solution',
'for token invalidation.',
'',
'You should also enable "{{injectOptionsFromRemoteContext}}" in %s\'s',
'settings (typically via common/models/*.json file). This is',
'needed to allow the invalidation algorithm to keep the current',
'session valid.',
''
].join('\n'), UserModel.modelName, UserModel.modelName);
});

return UserModel;
};

Expand All @@ -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();
Expand Down Expand Up @@ -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();

Expand Down
2 changes: 2 additions & 0 deletions test/access-token.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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');
Expand Down
3 changes: 3 additions & 0 deletions test/app.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -718,6 +718,7 @@ describe('app', function() {

beforeEach(function() {
app = loopback();
app.set('logoutSessionsOnSensitiveChanges', true);
app.dataSource('db', {
connector: 'memory'
});
Expand Down Expand Up @@ -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' });
Expand All @@ -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' });
Expand Down
3 changes: 2 additions & 1 deletion test/fixtures/access-control/server/config.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{
"port": 3000,
"host": "0.0.0.0",
"logoutSessionsOnSensitiveChanges": true,
"legacyExplorer": false
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
"disableStackTrace": false
}
},
"logoutSessionsOnSensitiveChanges": true,
"legacyExplorer": false
}

3 changes: 2 additions & 1 deletion test/fixtures/simple-integration-app/server/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,6 @@
"limit": "8kb"
}
},
"logoutSessionsOnSensitiveChanges": true,
"legacyExplorer": false
}
}
3 changes: 2 additions & 1 deletion test/fixtures/user-integration-app/server/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,6 @@
"limit": "8kb"
}
},
"logoutSessionsOnSensitiveChanges": true,
"legacyExplorer": false
}
}
1 change: 1 addition & 0 deletions test/model.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'});

Expand Down
2 changes: 2 additions & 0 deletions test/replication.rest.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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' });
Expand Down Expand Up @@ -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',
Expand Down
1 change: 1 addition & 0 deletions test/rest.middleware.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 2 additions & 0 deletions test/role.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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' });
Expand Down
1 change: 1 addition & 0 deletions test/support.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', {
Expand Down
12 changes: 12 additions & 0 deletions test/user.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -2313,6 +2314,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);
Expand Down

0 comments on commit dd657a9

Please sign in to comment.