diff --git a/packages/server/src/models/SessionModel.ts b/packages/server/src/models/SessionModel.ts index 706cf0af3..5d1bb9a6e 100644 --- a/packages/server/src/models/SessionModel.ts +++ b/packages/server/src/models/SessionModel.ts @@ -34,8 +34,10 @@ export default class SessionModel extends BaseModel { await this.delete(sessionId); } - public async deleteByUserId(userId: Uuid) { - await this.db(this.tableName).where('user_id', '=', userId).delete(); + public async deleteByUserId(userId: Uuid, exceptSessionId: Uuid = '') { + const query = this.db(this.tableName).where('user_id', '=', userId); + if (exceptSessionId) query.where('id', '!=', exceptSessionId); + await query.delete(); } } diff --git a/packages/server/src/models/UserModel.ts b/packages/server/src/models/UserModel.ts index 73efb1320..579f201a1 100644 --- a/packages/server/src/models/UserModel.ts +++ b/packages/server/src/models/UserModel.ts @@ -379,8 +379,12 @@ export default class UserModel extends BaseModel { public async resetPassword(token: string, fields: CheckRepeatPasswordInput) { checkRepeatPassword(fields, true); const user = await this.models().token().userFromToken(token); - await this.models().user().save({ id: user.id, password: fields.password }); - await this.models().token().deleteByValue(user.id, token); + + await this.withTransaction(async () => { + await this.models().user().save({ id: user.id, password: fields.password }); + await this.models().session().deleteByUserId(user.id); + await this.models().token().deleteByValue(user.id, token); + }, 'UserModel::resetPassword'); } public async handleBetaUserEmails() { diff --git a/packages/server/src/routes/index/password.test.ts b/packages/server/src/routes/index/password.test.ts index 70cb355f0..e105b7af2 100644 --- a/packages/server/src/routes/index/password.test.ts +++ b/packages/server/src/routes/index/password.test.ts @@ -18,7 +18,15 @@ describe('index/password', function() { }); test('should queue an email to reset password', async function() { - const { user } = await createUserAndSession(1); + const { user, password } = await createUserAndSession(1); + + // Create a few sessions, to verify that they are all deleted when the + // password is changed. + await models().session().authenticate(user.email, password); + await models().session().authenticate(user.email, password); + await models().session().authenticate(user.email, password); + expect(await models().session().count()).toBe(4); + await models().email().deleteAll(); await execRequest('', 'POST', 'password/forgot', { email: user.email }); const emails = await models().email().all(); @@ -34,6 +42,9 @@ describe('index/password', function() { const loggedInUser = await models().user().login(user.email, newPassword); expect(loggedInUser.id).toBe(user.id); + + // Check that all sessions have been deleted + expect(await models().session().count()).toBe(0); }); test('should not queue an email for non-existing emails', async function() { diff --git a/packages/server/src/routes/index/users.test.ts b/packages/server/src/routes/index/users.test.ts index c17c82054..c2e171964 100644 --- a/packages/server/src/routes/index/users.test.ts +++ b/packages/server/src/routes/index/users.test.ts @@ -345,6 +345,27 @@ describe('index/users', function() { await expectThrow(async () => execRequest('', 'GET', path, null, { query: { token } })); }); + test('should delete sessions when changing password', async function() { + const { user, session, password } = await createUserAndSession(1); + + await models().session().authenticate(user.email, password); + await models().session().authenticate(user.email, password); + await models().session().authenticate(user.email, password); + + expect(await models().session().count()).toBe(4); + + await patchUser(session.id, { + id: user.id, + email: 'changed@example.com', + password: 'hunter11hunter22hunter33', + password2: 'hunter11hunter22hunter33', + }, '/users/me'); + + const sessions = await models().session().all(); + expect(sessions.length).toBe(1); + expect(sessions[0].id).toBe(session.id); + }); + test('should apply ACL', async function() { const { user: admin, session: adminSession } = await createUserAndSession(1, true); const { user: user1, session: session1 } = await createUserAndSession(2, false); diff --git a/packages/server/src/routes/index/users.ts b/packages/server/src/routes/index/users.ts index b822dea64..8d1501ef2 100644 --- a/packages/server/src/routes/index/users.ts +++ b/packages/server/src/routes/index/users.ts @@ -2,7 +2,7 @@ import { SubPath, redirect } from '../../utils/routeUtils'; import Router from '../../utils/Router'; import { RouteType } from '../../utils/types'; import { AppContext, HttpMethod } from '../../utils/types'; -import { bodyFields, formParse } from '../../utils/requestUtils'; +import { bodyFields, contextSessionId, formParse } from '../../utils/requestUtils'; import { ErrorForbidden, ErrorUnprocessableEntity } from '../../utils/errors'; import { User, UserFlag, UserFlagType, Uuid } from '../../services/database/types'; import config from '../../config'; @@ -64,7 +64,6 @@ function makeUser(isNew: boolean, fields: any): User { if ('can_share_folder' in fields) user.can_share_folder = boolOrDefaultToValue(fields, 'can_share_folder'); if ('can_upload' in fields) user.can_upload = intOrDefaultToValue(fields, 'can_upload'); if ('account_type' in fields) user.account_type = Number(fields.account_type); - if ('email' in fields) user.email = fields.email; const password = checkRepeatPassword(fields, false); if (password) user.password = password; @@ -337,15 +336,12 @@ router.post('users', async (path: SubPath, ctx: AppContext) => { } await models.user().save(userToSave, { isNew: false }); - } - // } else if (fields.user_cancel_subscription_button) { - // await cancelSubscriptionByUserId(models, userId); - // const sessionId = contextSessionId(ctx, false); - // if (sessionId) { - // await models.session().logout(sessionId); - // return redirect(ctx, config().baseUrl); - // } + // When changing the password, we also clear all session IDs for + // that user, except the current one (otherwise they would be + // logged out). + if (userToSave.password) await models.session().deleteByUserId(userToSave.id, contextSessionId(ctx)); + } } else if (fields.stop_impersonate_button) { await stopImpersonating(ctx); return redirect(ctx, config().baseUrl);