From 4ee086dcd0da8c7a3d805b6e8d610ed3fc9c0081 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Fri, 9 Dec 2016 15:36:54 +0100 Subject: [PATCH] Invalidate AccessTokens on password change Invalidate all existing sessions (delete all access tokens) after user's password was changed. --- common/models/user.js | 47 +++- test/user.test.js | 602 +++++++++++++++++++++--------------------- 2 files changed, 343 insertions(+), 306 deletions(-) diff --git a/common/models/user.js b/common/models/user.js index b598c872..c89c34e9 100644 --- a/common/models/user.js +++ b/common/models/user.js @@ -645,6 +645,19 @@ module.exports = function(User) { err.statusCode = 422; throw err; }; + + User._invalidateAccessTokensOfUsers = function(userIds, cb) { + if (!Array.isArray(userIds) || !userIds.length) + return process.nextTick(cb); + + var accessTokenRelation = this.relations.accessTokens; + if (!accessTokenRelation) + return process.nextTick(cb); + + var AccessToken = accessTokenRelation.modelTo; + AccessToken.deleteAll({userId: {inq: userIds}}, cb); + }; + /*! * Setup an extended user model. */ @@ -809,8 +822,20 @@ module.exports = function(User) { var emailChanged; if (ctx.isNewInstance) return next(); if (!ctx.where && !ctx.instance) return next(); - var where = ctx.where || { id: ctx.instance.id }; - ctx.Model.find({ where: where }, function(err, userInstances) { + + 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 = ctx.where || {id: ctx.instance.id}; + ctx.Model.find({where: where}, function(err, userInstances) { if (err) return next(err); ctx.hookState.originalUserData = userInstances.map(function(u) { return { id: u.id, email: u.email }; @@ -828,24 +853,26 @@ module.exports = function(User) { ctx.data.emailVerified = false; } } + next(); }); }); User.observe('after save', function afterEmailUpdate(ctx, next) { - if (!ctx.Model.relations.accessTokens) return next(); - var AccessToken = ctx.Model.relations.accessTokens.modelTo; if (!ctx.instance && !ctx.data) return next(); - var newEmail = (ctx.instance || ctx.data).email; - if (!newEmail) return next(); if (!ctx.hookState.originalUserData) return next(); - var idsToExpire = ctx.hookState.originalUserData.filter(function(u) { - return u.email !== newEmail; + + var newEmail = (ctx.instance || ctx.data).email; + var isPasswordChange = ctx.hookState.isPasswordChange; + + if (!newEmail && !isPasswordChange) return next(); + + var userIdsToExpire = ctx.hookState.originalUserData.filter(function(u) { + return (newEmail && u.email !== newEmail) || isPasswordChange; }).map(function(u) { return u.id; }); - if (!idsToExpire.length) return next(); - AccessToken.deleteAll({ userId: { inq: idsToExpire }}, next); + ctx.Model._invalidateAccessTokensOfUsers(userIdsToExpire, next); }); }; diff --git a/test/user.test.js b/test/user.test.js index cd764ff5..c9d0f7df 100644 --- a/test/user.test.js +++ b/test/user.test.js @@ -1951,295 +1951,245 @@ describe('User', function() { }); }); - describe('Email Update', function() { - describe('User changing email property', function() { - var user, originalUserToken1, originalUserToken2, newUserCreated; - var currentEmailCredentials = { email: 'original@example.com', password: 'bar' }; - var updatedEmailCredentials = { email: 'updated@example.com', password: 'bar' }; - var newUserCred = { email: 'newuser@example.com', password: 'newpass' }; + describe('AccessToken (session) invalidation', function() { + var user, originalUserToken1, originalUserToken2, newUserCreated; + var currentEmailCredentials = {email: 'original@example.com', password: 'bar'}; + var updatedEmailCredentials = {email: 'updated@example.com', password: 'bar'}; + var newUserCred = {email: 'newuser@example.com', password: 'newpass'}; - beforeEach('create user then login', function createAndLogin(done) { - async.series([ - function createUserWithOriginalEmail(next) { - User.create(currentEmailCredentials, function(err, userCreated) { - if (err) return next(err); - user = userCreated; - next(); - }); - }, - function firstLoginWithOriginalEmail(next) { - User.login(currentEmailCredentials, function(err, accessToken1) { - if (err) return next(err); - assert(accessToken1.userId); - originalUserToken1 = accessToken1.id; - next(); - }); - }, - function secondLoginWithOriginalEmail(next) { - User.login(currentEmailCredentials, function(err, accessToken2) { - if (err) return next(err); - assert(accessToken2.userId); - originalUserToken2 = accessToken2.id; - next(); - }); - }, - ], done); - }); - - it('invalidates sessions when email is changed using `updateAttributes`', function(done) { - user.updateAttributes( - { email: updatedEmailCredentials.email }, - function(err, userInstance) { - if (err) return done(err); - assertNoAccessTokens(done); + beforeEach('create user then login', function createAndLogin(done) { + async.series([ + function createUserWithOriginalEmail(next) { + User.create(currentEmailCredentials, function(err, userCreated) { + if (err) return next(err); + user = userCreated; + next(); }); - }); - - it('invalidates sessions when email is changed using `replaceAttributes`', function(done) { - user.replaceAttributes(updatedEmailCredentials, function(err, userInstance) { - if (err) return done(err); - assertNoAccessTokens(done); - }); - }); - - it('invalidates sessions when email is changed using `updateOrCreate`', function(done) { - User.updateOrCreate({ - id: user.id, - email: updatedEmailCredentials.email, - password: updatedEmailCredentials.password, - }, function(err, userInstance) { - if (err) return done(err); - assertNoAccessTokens(done); - }); - }); - - it('invalidates sessions when the email is changed using `replaceById`', function(done) { - User.replaceById(user.id, updatedEmailCredentials, function(err, userInstance) { - if (err) return done(err); - assertNoAccessTokens(done); - }); - }); - - it('invalidates sessions when the email is changed using `replaceOrCreate`', function(done) { - User.replaceOrCreate({ - id: user.id, - email: updatedEmailCredentials.email, - password: updatedEmailCredentials.password, - }, function(err, userInstance) { - if (err) return done(err); - assertNoAccessTokens(done); - }); - }); - - it('keeps sessions AS IS if firstName is added using `updateAttributes`', function(done) { - user.updateAttributes({ 'firstName': 'Janny' }, function(err, userInstance) { - if (err) return done(err); - assertUntouchedTokens(done); - }); - }); - - it('keeps sessions AS IS if firstName is added using `replaceAttributes`', function(done) { - user.replaceAttributes({ - email: currentEmailCredentials.email, - password: currentEmailCredentials.password, - firstName: 'Candy', - }, function(err, userInstance) { - if (err) return done(err); - assertUntouchedTokens(done); - }); - }); - - it('keeps sessions AS IS if firstName is added using `updateOrCreate`', function(done) { - User.updateOrCreate({ - id: user.id, - firstName: 'Loay', - email: currentEmailCredentials.email, - password: currentEmailCredentials.password, - }, function(err, userInstance) { - if (err) return done(err); - assertUntouchedTokens(done); - }); - }); - - it('keeps sessions AS IS if firstName is added using `replaceById`', function(done) { - User.replaceById( - user.id, - { - firstName: 'Miroslav', - email: currentEmailCredentials.email, - password: currentEmailCredentials.password, - }, function(err, userInstance) { - if (err) return done(err); - assertUntouchedTokens(done); + }, + function firstLoginWithOriginalEmail(next) { + User.login(currentEmailCredentials, function(err, accessToken1) { + if (err) return next(err); + assert(accessToken1.userId); + originalUserToken1 = accessToken1.id; + next(); }); - }); - - it('keeps sessions AS IS if a new user is created using `create`', function(done) { - async.series([ - function(next) { - User.create(newUserCred, function(err, newUserInstance) { - if (err) return done(err); - newUserCreated = newUserInstance; - next(); - }); - }, - function(next) { - User.login(newUserCred, function(err, newAccessToken) { - if (err) return done(err); - assert(newAccessToken.id); - assertPreservedToken(next); - }); - }, - ], done); - }); - - it('keeps sessions AS IS if a new user is created using `updateOrCreate`', function(done) { - async.series([ - function(next) { - User.create(newUserCred, function(err, newUserInstance2) { - if (err) return done(err); - newUserCreated = newUserInstance2; - next(); - }); - }, - function(next) { - User.login(newUserCred, function(err, newAccessToken2) { - if (err) return done(err); - assert(newAccessToken2.id); - assertPreservedToken(next); - }); - }, - ], done); - }); - - it('keeps sessions AS IS if non-email property is changed using updateAll', function(done) { - var userPartial; - async.series([ - function createPartialUser(next) { - User.create( - { email: 'partial@example.com', password: 'pass1', age: 25 }, - function(err, partialInstance) { - if (err) return next(err); - userPartial = partialInstance; - next(); - }); - }, - function loginPartiallUser(next) { - User.login({ email: 'partial@example.com', password: 'pass1' }, function(err, ats) { - if (err) return next(err); - next(); - }); - }, - function updatePartialUser(next) { - User.updateAll( - { id: userPartial.id }, - { age: userPartial.age + 1 }, - function(err, info) { - if (err) return next(err); - next(); - }); - }, - function verifyTokensOfPartialUser(next) { - AccessToken.find({ where: { userId: userPartial.id }}, function(err, tokens1) { - if (err) return next(err); - expect(tokens1.length).to.equal(1); - next(); - }); - }, - ], done); - }); - - function assertPreservedToken(done) { - AccessToken.find({ where: { userId: user.id }}, function(err, tokens) { - if (err) return done(err); - expect(tokens.length).to.equal(2); - expect([tokens[0].id, tokens[1].id]).to.have.members([originalUserToken1, - originalUserToken2]); - done(); - }); - } - - function assertNoAccessTokens(done) { - AccessToken.find({ where: { userId: user.id }}, function(err, tokens) { - if (err) return done(err); - expect(tokens.length).to.equal(0); - done(); - }); - } - - function assertUntouchedTokens(done) { - AccessToken.find({ where: { userId: user.id }}, function(err, tokens) { - if (err) return done(err); - expect(tokens.length).to.equal(2); - done(); - }); - } + }, + function secondLoginWithOriginalEmail(next) { + User.login(currentEmailCredentials, function(err, accessToken2) { + if (err) return next(err); + assert(accessToken2.userId); + originalUserToken2 = accessToken2.id; + next(); + }); + }, + ], done); }); - describe('User not changing email property', function() { + it('invalidates sessions when email is changed using `updateAttributes`', function(done) { + user.updateAttributes( + {email: updatedEmailCredentials.email}, + function(err, userInstance) { + if (err) return done(err); + assertNoAccessTokens(done); + }); + }); + + it('invalidates sessions after `replaceAttributes`', function(done) { + // The way how the invalidation is implemented now, all sessions + // are invalidated on a full replace + user.replaceAttributes(currentEmailCredentials, function(err, userInstance) { + if (err) return done(err); + assertNoAccessTokens(done); + }); + }); + + it('invalidates sessions when email is changed using `updateOrCreate`', function(done) { + User.updateOrCreate({ + id: user.id, + email: updatedEmailCredentials.email, + }, function(err, userInstance) { + if (err) return done(err); + assertNoAccessTokens(done); + }); + }); + + it('invalidates sessions after `replaceById`', function(done) { + // The way how the invalidation is implemented now, all sessions + // are invalidated on a full replace + User.replaceById(user.id, currentEmailCredentials, function(err, userInstance) { + if (err) return done(err); + assertNoAccessTokens(done); + }); + }); + + it('invalidates sessions after `replaceOrCreate`', function(done) { + // The way how the invalidation is implemented now, all sessions + // are invalidated on a full replace + User.replaceOrCreate({ + id: user.id, + email: currentEmailCredentials.email, + password: currentEmailCredentials.password, + }, function(err, userInstance) { + if (err) return done(err); + assertNoAccessTokens(done); + }); + }); + + it('keeps sessions AS IS if firstName is added using `updateAttributes`', function(done) { + user.updateAttributes({'firstName': 'Janny'}, function(err, userInstance) { + if (err) return done(err); + assertUntouchedTokens(done); + }); + }); + + it('keeps sessions AS IS if firstName is added using `updateOrCreate`', function(done) { + User.updateOrCreate({ + id: user.id, + firstName: 'Loay', + email: currentEmailCredentials.email, + }, function(err, userInstance) { + if (err) return done(err); + assertUntouchedTokens(done); + }); + }); + + it('keeps sessions AS IS if a new user is created using `create`', function(done) { + async.series([ + function(next) { + User.create(newUserCred, function(err, newUserInstance) { + if (err) return done(err); + newUserCreated = newUserInstance; + next(); + }); + }, + function(next) { + User.login(newUserCred, function(err, newAccessToken) { + if (err) return done(err); + assert(newAccessToken.id); + assertPreservedTokens(next); + }); + }, + ], done); + }); + + it('keeps sessions AS IS if a new user is created using `updateOrCreate`', function(done) { + async.series([ + function(next) { + User.create(newUserCred, function(err, newUserInstance2) { + if (err) return done(err); + newUserCreated = newUserInstance2; + next(); + }); + }, + function(next) { + User.login(newUserCred, function(err, newAccessToken2) { + if (err) return done(err); + assert(newAccessToken2.id); + assertPreservedTokens(next); + }); + }, + ], done); + }); + + it('keeps sessions AS IS if non-email property is changed using updateAll', function(done) { + var userPartial; + async.series([ + function createPartialUser(next) { + User.create( + {email: 'partial@example.com', password: 'pass1', age: 25}, + function(err, partialInstance) { + if (err) return next(err); + userPartial = partialInstance; + next(); + }); + }, + function loginPartiallUser(next) { + User.login({email: 'partial@example.com', password: 'pass1'}, function(err, ats) { + if (err) return next(err); + next(); + }); + }, + function updatePartialUser(next) { + User.updateAll( + {id: userPartial.id}, + {age: userPartial.age + 1}, + function(err, info) { + if (err) return next(err); + next(); + }); + }, + function verifyTokensOfPartialUser(next) { + AccessToken.find({where: {userId: userPartial.id}}, function(err, tokens1) { + if (err) return next(err); + expect(tokens1.length).to.equal(1); + next(); + }); + }, + ], done); + }); + + it('preserves other users\' sessions if their email is untouched', function(done) { var user1, user2, user3; - it('preserves other users\' sessions if their email is untouched', function(done) { - async.series([ - function(next) { - User.create({ email: 'user1@example.com', password: 'u1pass' }, function(err, u1) { + async.series([ + function(next) { + User.create({email: 'user1@example.com', password: 'u1pass'}, function(err, u1) { + if (err) return done(err); + User.create({email: 'user2@example.com', password: 'u2pass'}, function(err, u2) { if (err) return done(err); - User.create({ email: 'user2@example.com', password: 'u2pass' }, function(err, u2) { + User.create({email: 'user3@example.com', password: 'u3pass'}, function(err, u3) { if (err) return done(err); - User.create({ email: 'user3@example.com', password: 'u3pass' }, function(err, u3) { - if (err) return done(err); - user1 = u1; - user2 = u2; - user3 = u3; - next(); - }); + user1 = u1; + user2 = u2; + user3 = u3; + next(); }); }); - }, - function(next) { - User.login( - { email: 'user1@example.com', password: 'u1pass' }, - function(err, accessToken1) { - if (err) return next(err); - User.login( - { email: 'user2@example.com', password: 'u2pass' }, - function(err, accessToken2) { - if (err) return next(err); - User.login({ email: 'user3@example.com', password: 'u3pass' }, - function(err, accessToken3) { - if (err) return next(err); - next(); - }); - }); - }); - }, - function(next) { - user2.updateAttribute('email', 'user2Update@b.com', function(err, userInstance) { + }); + }, + function(next) { + User.login( + {email: 'user1@example.com', password: 'u1pass'}, + function(err, accessToken1) { if (err) return next(err); - assert.equal(userInstance.email, 'user2Update@b.com'); - next(); - }); - }, - function(next) { - AccessToken.find({ where: { userId: user1.id }}, function(err, tokens1) { - if (err) return next(err); - AccessToken.find({ where: { userId: user2.id }}, function(err, tokens2) { - if (err) return next(err); - AccessToken.find({ where: { userId: user3.id }}, function(err, tokens3) { + User.login( + {email: 'user2@example.com', password: 'u2pass'}, + function(err, accessToken2) { if (err) return next(err); - - expect(tokens1.length).to.equal(1); - expect(tokens2.length).to.equal(0); - expect(tokens3.length).to.equal(1); - next(); + User.login({email: 'user3@example.com', password: 'u3pass'}, + function(err, accessToken3) { + if (err) return next(err); + next(); + }); }); + }); + }, + function(next) { + user2.updateAttribute('email', 'user2Update@b.com', function(err, userInstance) { + if (err) return next(err); + assert.equal(userInstance.email, 'user2Update@b.com'); + next(); + }); + }, + function(next) { + AccessToken.find({where: {userId: user1.id}}, function(err, tokens1) { + if (err) return next(err); + AccessToken.find({where: {userId: user2.id}}, function(err, tokens2) { + if (err) return next(err); + AccessToken.find({where: {userId: user3.id}}, function(err, tokens3) { + if (err) return next(err); + + expect(tokens1.length).to.equal(1); + expect(tokens2.length).to.equal(0); + expect(tokens3.length).to.equal(1); + next(); }); }); - }, - ], done); - }); + }); + }, + ], done); }); - it('invalidates sessions after using updateAll', function(done) { + it('invalidates correct sessions after changing email using updateAll', function(done) { var userSpecial, userNormal; async.series([ function createSpecialUser(next) { @@ -2251,27 +2201,12 @@ describe('User', function() { next(); }); }, - function createNormaluser(next) { - User.create( - { email: 'normal@example.com', password: 'pass2' }, - function(err, normalInstance) { - if (err) return next(err); - userNormal = normalInstance; - next(); - }); - }, function loginSpecialUser(next) { User.login({ email: 'special@example.com', password: 'pass1' }, function(err, ats) { if (err) return next(err); next(); }); }, - function loginNormalUser(next) { - User.login({ email: 'normal@example.com', password: 'pass2' }, function(err, atn) { - if (err) return next(err); - next(); - }); - }, function updateSpecialUser(next) { User.updateAll( { name: 'Special' }, @@ -2281,21 +2216,96 @@ describe('User', function() { }); }, function verifyTokensOfSpecialUser(next) { - AccessToken.find({ where: { userId: userSpecial.id }}, function(err, tokens1) { + AccessToken.find({where: {userId: userSpecial.id}}, function(err, tokens1) { if (err) return done(err); - expect(tokens1.length).to.equal(0); - next(); - }); - }, - function verifyTokensOfNormalUser(next) { - AccessToken.find({ userId: userNormal.userId }, function(err, tokens2) { - if (err) return done(err); - expect(tokens2.length).to.equal(1); + expect(tokens1.length, 'tokens - special user tokens').to.equal(0); next(); }); }, + assertPreservedTokens, ], done); }); + + it('invalidates session when password is reset', function(done) { + user.updateAttribute('password', 'newPass', function(err, user2) { + if (err) return done(err); + assertNoAccessTokens(done); + }); + }); + + it('preserves other user sessions if their password is untouched', function(done) { + var user1, user2, user1Token; + async.series([ + function(next) { + User.create({email: 'user1@example.com', password: 'u1pass'}, function(err, u1) { + if (err) return done(err); + User.create({email: 'user2@example.com', password: 'u2pass'}, function(err, u2) { + if (err) return done(err); + user1 = u1; + user2 = u2; + next(); + }); + }); + }, + function(next) { + User.login({email: 'user1@example.com', password: 'u1pass'}, function(err, at1) { + User.login({email: 'user2@example.com', password: 'u2pass'}, function(err, at2) { + assert(at1.userId); + assert(at2.userId); + user1Token = at1.id; + next(); + }); + }); + }, + function(next) { + user2.updateAttribute('password', 'newPass', function(err, user2Instance) { + if (err) return next(err); + assert(user2Instance); + next(); + }); + }, + function(next) { + AccessToken.find({where: {userId: user1.id}}, function(err, tokens1) { + if (err) return next(err); + AccessToken.find({where: {userId: user2.id}}, function(err, tokens2) { + if (err) return next(err); + expect(tokens1.length).to.equal(1); + expect(tokens2.length).to.equal(0); + assert.equal(tokens1[0].id, user1Token); + next(); + }); + }); + }, + ], function(err) { + done(); + }); + }); + + function assertPreservedTokens(done) { + AccessToken.find({where: {userId: user.id}}, function(err, tokens) { + if (err) return done(err); + expect(tokens.length).to.equal(2); + expect([tokens[0].id, tokens[1].id]).to.have.members([originalUserToken1, + originalUserToken2]); + done(); + }); + } + + function assertNoAccessTokens(done) { + AccessToken.find({where: {userId: user.id}}, function(err, tokens) { + if (err) return done(err); + expect(tokens.length).to.equal(0); + done(); + }); + } + + function assertUntouchedTokens(done) { + AccessToken.find({where: {userId: user.id}}, function(err, tokens) { + if (err) return done(err); + expect(tokens.length).to.equal(2); + done(); + }); + } }); describe('Verification after updating email', function() {