From 58187175c340580b3520a0fe064f24b834512588 Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Thu, 5 Mar 2020 12:22:32 +0200 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20Deleted=20all=20but=20active=20sess?= =?UTF-8?q?ions=20on=20password=20change=20(#11639)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit closes #10323 * Fixed usage of hasMany for user->session * Refactored changePassword to async function * Deleted all user sessions when password changed * Tested for session retained after password changed * Added the session to the frame * Skipped the current session when changing password --- core/server/api/canary/users.js | 1 + core/server/api/shared/http.js | 1 + core/server/api/v2/users.js | 1 + core/server/models/user.js | 47 +++++++++++++----------- core/test/acceptance/admin/users_spec.js | 9 ++++- 5 files changed, 36 insertions(+), 23 deletions(-) diff --git a/core/server/api/canary/users.js b/core/server/api/canary/users.js index 0979695a4a..362f86dd76 100644 --- a/core/server/api/canary/users.js +++ b/core/server/api/canary/users.js @@ -160,6 +160,7 @@ module.exports = { } }, query(frame) { + frame.options.skipSessionID = frame.original.session.id; return models.User.changePassword(frame.data.password[0], frame.options); } }, diff --git a/core/server/api/shared/http.js b/core/server/api/shared/http.js index c0b69834b1..4a81c9c215 100644 --- a/core/server/api/shared/http.js +++ b/core/server/api/shared/http.js @@ -40,6 +40,7 @@ const http = (apiImpl) => { query: req.query, params: req.params, user: req.user, + session: req.session, context: { api_key: apiKey, user: user, diff --git a/core/server/api/v2/users.js b/core/server/api/v2/users.js index 955df58754..56847b8b17 100644 --- a/core/server/api/v2/users.js +++ b/core/server/api/v2/users.js @@ -155,6 +155,7 @@ module.exports = { } }, query(frame) { + frame.options.skipSessionID = frame.original.session.id; return models.User.changePassword(frame.data.password[0], frame.options); } }, diff --git a/core/server/models/user.js b/core/server/models/user.js index b345278332..465023677d 100644 --- a/core/server/models/user.js +++ b/core/server/models/user.js @@ -235,7 +235,7 @@ User = ghostBookshelf.Model.extend({ }, sessions: function sessions() { - return this.hasMany('Sessions'); + return this.hasMany('Session'); }, roles: function roles() { @@ -864,31 +864,36 @@ User = ghostBookshelf.Model.extend({ * @param {Object} object * @param {Object} unfilteredOptions */ - changePassword: function changePassword(object, unfilteredOptions) { - var options = this.filterOptions(unfilteredOptions, 'changePassword'), - self = this, - newPassword = object.newPassword, - userId = object.user_id, - oldPassword = object.oldPassword, - isLoggedInUser = userId === options.context.user, - user; + changePassword: async function changePassword(object, unfilteredOptions) { + const options = this.filterOptions(unfilteredOptions, 'changePassword'); + const newPassword = object.newPassword; + const userId = object.user_id; + const oldPassword = object.oldPassword; + const isLoggedInUser = userId === options.context.user; + const skipSessionID = unfilteredOptions.skipSessionID; options.require = true; + options.withRelated = ['sessions']; - return self.forge({id: userId}).fetch(options) - .then(function then(_user) { - user = _user; + const user = await this.forge({id: userId}).fetch(options); - if (isLoggedInUser) { - return self.isPasswordCorrect({ - plainPassword: oldPassword, - hashedPassword: user.get('password') - }); - } - }) - .then(function then() { - return user.save({password: newPassword}); + if (isLoggedInUser) { + await this.isPasswordCorrect({ + plainPassword: oldPassword, + hashedPassword: user.get('password') }); + } + + const updatedUser = await user.save({password: newPassword}); + + const sessions = user.related('sessions'); + for (const session of sessions) { + if (session.get('session_id') !== skipSessionID) { + await session.destroy(options); + } + } + + return updatedUser; }, transferOwnership: function transferOwnership(object, unfilteredOptions) { diff --git a/core/test/acceptance/admin/users_spec.js b/core/test/acceptance/admin/users_spec.js index 8dd4c142e4..699ebdcf7d 100644 --- a/core/test/acceptance/admin/users_spec.js +++ b/core/test/acceptance/admin/users_spec.js @@ -310,8 +310,8 @@ describe('User API', function () { }); }); - it('Can change password', function () { - return request + it('Can change password and retain the session', async function () { + await request .put(localUtils.API.getApiQuery('users/password')) .set('Origin', config.get('url')) .send({ @@ -329,5 +329,10 @@ describe('User API', function () { should.exist(res.body.password); should.exist(res.body.password[0].message); }); + + await request + .get(localUtils.API.getApiQuery('session/')) + .set('Origin', config.get('url')) + .expect(200); }); });