From 05db4337cf2663e5b5997915bb39a64058c1b5bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Mon, 30 Jan 2017 11:30:05 +0100 Subject: [PATCH 1/2] Preserve sessions on User.save() making no changes --- common/models/user.js | 35 ++++++++++++----------------------- test/user.test.js | 7 +++++++ 2 files changed, 19 insertions(+), 23 deletions(-) diff --git a/common/models/user.js b/common/models/user.js index be45933d..03056188 100644 --- a/common/models/user.js +++ b/common/models/user.js @@ -866,30 +866,16 @@ module.exports = function(User) { next(); }); - // Delete old sessions once email is updated - User.observe('before save', function beforeEmailUpdate(ctx, next) { + User.observe('before save', function prepareForTokenInvalidation(ctx, next) { if (!ctx.Model.app.get('logoutSessionsOnSensitiveChanges')) return next(); - var emailChanged; if (ctx.isNewInstance) return next(); if (!ctx.where && !ctx.instance) return next(); + var pkName = ctx.Model.definition.idName() || 'id'; - var isPartialUpdateChangingPassword = ctx.data && 'password' in ctx.data; - - // Full replace of User instance => assume password change. - // HashPassword returns a different value for each invocation, - // therefore we cannot tell whether ctx.instance.password is the same - // or not. - var isFullReplaceChangingPassword = !!ctx.instance; - - ctx.hookState.isPasswordChange = isPartialUpdateChangingPassword || - isFullReplaceChangingPassword; - - var where; - if (ctx.where) { - where = ctx.where; - } else { + var where = ctx.where; + if (!where) { where = {}; where[pkName] = ctx.instance[pkName]; } @@ -899,9 +885,11 @@ module.exports = function(User) { ctx.hookState.originalUserData = userInstances.map(function(u) { var user = {}; user[pkName] = u[pkName]; - user['email'] = u['email']; + user.email = u.email; + user.password = u.password; return user; }); + var emailChanged; if (ctx.instance) { emailChanged = ctx.instance.email !== ctx.hookState.originalUserData[0].email; if (emailChanged && ctx.Model.settings.emailVerificationRequired) { @@ -920,7 +908,7 @@ module.exports = function(User) { }); }); - User.observe('after save', function afterEmailUpdate(ctx, next) { + User.observe('after save', function invalidateOtherTokens(ctx, next) { if (!ctx.Model.app.get('logoutSessionsOnSensitiveChanges')) return next(); if (!ctx.instance && !ctx.data) return next(); @@ -928,12 +916,13 @@ module.exports = function(User) { var pkName = ctx.Model.definition.idName() || 'id'; var newEmail = (ctx.instance || ctx.data).email; - var isPasswordChange = ctx.hookState.isPasswordChange; + var newPassword = (ctx.instance || ctx.data).password; - if (!newEmail && !isPasswordChange) return next(); + if (!newEmail && !newPassword) return next(); var userIdsToExpire = ctx.hookState.originalUserData.filter(function(u) { - return (newEmail && u.email !== newEmail) || isPasswordChange; + return (newEmail && u.email !== newEmail) || + (newPassword && u.password !== newPassword); }).map(function(u) { return u[pkName]; }); diff --git a/test/user.test.js b/test/user.test.js index 2b7dbffc..d468d4b8 100644 --- a/test/user.test.js +++ b/test/user.test.js @@ -2097,6 +2097,13 @@ describe('User', function() { }); }); + it('keeps sessions AS IS when calling save() with no changes', function(done) { + user.save(function(err) { + if (err) return done(err); + assertPreservedTokens(done); + }); + }); + it('keeps sessions AS IS if firstName is added using `updateOrCreate`', function(done) { User.updateOrCreate({ pk: user.pk, From 0cc2b5b8db280154466e3bce6850a5dfde963d94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Tue, 31 Jan 2017 15:53:41 +0100 Subject: [PATCH 2/2] Fix detection of logoutSessionsOnSensitiveChanges Modify the code detecting whether logoutSessionsOnSensitiveChanges is enabled to correctly handle the case when the model is not attached to any application, as is the case with loopback-component-passport tests. --- common/models/user.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/common/models/user.js b/common/models/user.js index 03056188..4929fb84 100644 --- a/common/models/user.js +++ b/common/models/user.js @@ -867,7 +867,9 @@ module.exports = function(User) { }); User.observe('before save', function prepareForTokenInvalidation(ctx, next) { - if (!ctx.Model.app.get('logoutSessionsOnSensitiveChanges')) return next(); + var invalidationEnabled = ctx.Model.app && + ctx.Model.app.get('logoutSessionsOnSensitiveChanges'); + if (!invalidationEnabled) return next(); if (ctx.isNewInstance) return next(); if (!ctx.where && !ctx.instance) return next(); @@ -909,7 +911,9 @@ module.exports = function(User) { }); User.observe('after save', function invalidateOtherTokens(ctx, next) { - if (!ctx.Model.app.get('logoutSessionsOnSensitiveChanges')) return next(); + var invalidationEnabled = ctx.Model.app && + ctx.Model.app.get('logoutSessionsOnSensitiveChanges'); + if (!invalidationEnabled) return next(); if (!ctx.instance && !ctx.data) return next(); if (!ctx.hookState.originalUserData) return next();