Skip to content

Commit

Permalink
Server: Delete all sessions when a password is changed or reset
Browse files Browse the repository at this point in the history
  • Loading branch information
laurent22 committed Oct 25, 2021
1 parent 82227b2 commit b497177
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 15 deletions.
6 changes: 4 additions & 2 deletions packages/server/src/models/SessionModel.ts
Expand Up @@ -34,8 +34,10 @@ export default class SessionModel extends BaseModel<Session> {
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();
}

}
8 changes: 6 additions & 2 deletions packages/server/src/models/UserModel.ts
Expand Up @@ -379,8 +379,12 @@ export default class UserModel extends BaseModel<User> {
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() {
Expand Down
13 changes: 12 additions & 1 deletion packages/server/src/routes/index/password.test.ts
Expand Up @@ -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();
Expand 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() {
Expand Down
21 changes: 21 additions & 0 deletions packages/server/src/routes/index/users.test.ts
Expand Up @@ -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);
Expand Down
16 changes: 6 additions & 10 deletions packages/server/src/routes/index/users.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit b497177

Please sign in to comment.