mirror of
https://github.com/immich-app/immich.git
synced 2025-11-01 11:07:11 -04:00
feat: logout sessions on password change (#23188)
* log out ohter sessions on password change * translations * update and add tests * rename event to UserLogoutOtherSessions * fix typo * requested changes * fix tests * fix medium:test * use ValidateBoolean * fix format * dont delete current session id * Update server/src/dtos/auth.dto.ts Co-authored-by: Daniel Dietzler <36593685+danieldietzler@users.noreply.github.com> * rename event and invalidateOtherSessions * chore: cleanup --------- Co-authored-by: Daniel Dietzler <36593685+danieldietzler@users.noreply.github.com> Co-authored-by: Jason Rasmussen <jason@rasm.me>
This commit is contained in:
parent
6bb1a9e083
commit
382481735a
@ -669,6 +669,8 @@
|
||||
"change_password_description": "This is either the first time you are signing into the system or a request has been made to change your password. Please enter the new password below.",
|
||||
"change_password_form_confirm_password": "Confirm Password",
|
||||
"change_password_form_description": "Hi {name},\n\nThis is either the first time you are signing into the system or a request has been made to change your password. Please enter the new password below.",
|
||||
"change_password_form_log_out": "Log out all other devices",
|
||||
"change_password_form_log_out_description": "It is recommended to log out of all other devices",
|
||||
"change_password_form_new_password": "New Password",
|
||||
"change_password_form_password_mismatch": "Passwords do not match",
|
||||
"change_password_form_reenter_new_password": "Re-enter New Password",
|
||||
|
||||
@ -13,30 +13,36 @@ part of openapi.api;
|
||||
class ChangePasswordDto {
|
||||
/// Returns a new [ChangePasswordDto] instance.
|
||||
ChangePasswordDto({
|
||||
this.invalidateSessions = false,
|
||||
required this.newPassword,
|
||||
required this.password,
|
||||
});
|
||||
|
||||
bool invalidateSessions;
|
||||
|
||||
String newPassword;
|
||||
|
||||
String password;
|
||||
|
||||
@override
|
||||
bool operator ==(Object other) => identical(this, other) || other is ChangePasswordDto &&
|
||||
other.invalidateSessions == invalidateSessions &&
|
||||
other.newPassword == newPassword &&
|
||||
other.password == password;
|
||||
|
||||
@override
|
||||
int get hashCode =>
|
||||
// ignore: unnecessary_parenthesis
|
||||
(invalidateSessions.hashCode) +
|
||||
(newPassword.hashCode) +
|
||||
(password.hashCode);
|
||||
|
||||
@override
|
||||
String toString() => 'ChangePasswordDto[newPassword=$newPassword, password=$password]';
|
||||
String toString() => 'ChangePasswordDto[invalidateSessions=$invalidateSessions, newPassword=$newPassword, password=$password]';
|
||||
|
||||
Map<String, dynamic> toJson() {
|
||||
final json = <String, dynamic>{};
|
||||
json[r'invalidateSessions'] = this.invalidateSessions;
|
||||
json[r'newPassword'] = this.newPassword;
|
||||
json[r'password'] = this.password;
|
||||
return json;
|
||||
@ -51,6 +57,7 @@ class ChangePasswordDto {
|
||||
final json = value.cast<String, dynamic>();
|
||||
|
||||
return ChangePasswordDto(
|
||||
invalidateSessions: mapValueOfType<bool>(json, r'invalidateSessions') ?? false,
|
||||
newPassword: mapValueOfType<String>(json, r'newPassword')!,
|
||||
password: mapValueOfType<String>(json, r'password')!,
|
||||
);
|
||||
|
||||
@ -11477,6 +11477,10 @@
|
||||
},
|
||||
"ChangePasswordDto": {
|
||||
"properties": {
|
||||
"invalidateSessions": {
|
||||
"default": false,
|
||||
"type": "boolean"
|
||||
},
|
||||
"newPassword": {
|
||||
"example": "password",
|
||||
"minLength": 8,
|
||||
|
||||
@ -561,6 +561,7 @@ export type SignUpDto = {
|
||||
password: string;
|
||||
};
|
||||
export type ChangePasswordDto = {
|
||||
invalidateSessions?: boolean;
|
||||
newPassword: string;
|
||||
password: string;
|
||||
};
|
||||
|
||||
@ -183,7 +183,7 @@ describe(AuthController.name, () => {
|
||||
it('should be an authenticated route', async () => {
|
||||
await request(ctx.getHttpServer())
|
||||
.post('/auth/change-password')
|
||||
.send({ password: 'password', newPassword: 'Password1234' });
|
||||
.send({ password: 'password', newPassword: 'Password1234', invalidateSessions: false });
|
||||
expect(ctx.authenticate).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@ -4,7 +4,7 @@ import { IsEmail, IsNotEmpty, IsString, MinLength } from 'class-validator';
|
||||
import { AuthApiKey, AuthSession, AuthSharedLink, AuthUser, UserAdmin } from 'src/database';
|
||||
import { ImmichCookie, UserMetadataKey } from 'src/enum';
|
||||
import { UserMetadataItem } from 'src/types';
|
||||
import { Optional, PinCode, toEmail } from 'src/validation';
|
||||
import { Optional, PinCode, toEmail, ValidateBoolean } from 'src/validation';
|
||||
|
||||
export type CookieResponse = {
|
||||
isSecure: boolean;
|
||||
@ -83,6 +83,9 @@ export class ChangePasswordDto {
|
||||
@MinLength(8)
|
||||
@ApiProperty({ example: 'password' })
|
||||
newPassword!: string;
|
||||
|
||||
@ValidateBoolean({ optional: true, default: false })
|
||||
invalidateSessions?: boolean;
|
||||
}
|
||||
|
||||
export class PinCodeSetupDto {
|
||||
|
||||
@ -74,6 +74,12 @@ delete from "session"
|
||||
where
|
||||
"id" = $1::uuid
|
||||
|
||||
-- SessionRepository.invalidate
|
||||
delete from "session"
|
||||
where
|
||||
"userId" = $1
|
||||
and "id" != $2
|
||||
|
||||
-- SessionRepository.lockAll
|
||||
update "session"
|
||||
set
|
||||
|
||||
@ -88,6 +88,8 @@ type EventMap = {
|
||||
UserDelete: [UserEvent];
|
||||
UserRestore: [UserEvent];
|
||||
|
||||
AuthChangePassword: [{ userId: string; currentSessionId?: string; invalidateSessions?: boolean }];
|
||||
|
||||
// websocket events
|
||||
WebsocketConnect: [{ userId: string }];
|
||||
};
|
||||
|
||||
@ -101,6 +101,15 @@ export class SessionRepository {
|
||||
await this.db.deleteFrom('session').where('id', '=', asUuid(id)).execute();
|
||||
}
|
||||
|
||||
@GenerateSql({ params: [{ userId: DummyValue.UUID, excludeId: DummyValue.UUID }] })
|
||||
async invalidate({ userId, excludeId }: { userId: string; excludeId?: string }) {
|
||||
await this.db
|
||||
.deleteFrom('session')
|
||||
.where('userId', '=', userId)
|
||||
.$if(!!excludeId, (qb) => qb.where('id', '!=', excludeId!))
|
||||
.execute();
|
||||
}
|
||||
|
||||
@GenerateSql({ params: [DummyValue.UUID] })
|
||||
async lockAll(userId: string) {
|
||||
await this.db.updateTable('session').set({ pinExpiresAt: null }).where('userId', '=', userId).execute();
|
||||
|
||||
@ -124,6 +124,11 @@ describe(AuthService.name, () => {
|
||||
|
||||
expect(mocks.user.getForChangePassword).toHaveBeenCalledWith(user.id);
|
||||
expect(mocks.crypto.compareBcrypt).toHaveBeenCalledWith('old-password', 'hash-password');
|
||||
expect(mocks.event.emit).toHaveBeenCalledWith('AuthChangePassword', {
|
||||
userId: user.id,
|
||||
currentSessionId: auth.session?.id,
|
||||
shouldLogoutSessions: undefined,
|
||||
});
|
||||
});
|
||||
|
||||
it('should throw when password does not match existing password', async () => {
|
||||
@ -147,6 +152,25 @@ describe(AuthService.name, () => {
|
||||
|
||||
await expect(sut.changePassword(auth, dto)).rejects.toBeInstanceOf(BadRequestException);
|
||||
});
|
||||
|
||||
it('should change the password and logout other sessions', async () => {
|
||||
const user = factory.userAdmin();
|
||||
const auth = factory.auth({ user });
|
||||
const dto = { password: 'old-password', newPassword: 'new-password', invalidateSessions: true };
|
||||
|
||||
mocks.user.getForChangePassword.mockResolvedValue({ id: user.id, password: 'hash-password' });
|
||||
mocks.user.update.mockResolvedValue(user);
|
||||
|
||||
await sut.changePassword(auth, dto);
|
||||
|
||||
expect(mocks.user.getForChangePassword).toHaveBeenCalledWith(user.id);
|
||||
expect(mocks.crypto.compareBcrypt).toHaveBeenCalledWith('old-password', 'hash-password');
|
||||
expect(mocks.event.emit).toHaveBeenCalledWith('AuthChangePassword', {
|
||||
userId: user.id,
|
||||
invalidateSessions: true,
|
||||
currentSessionId: auth.session?.id,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('logout', () => {
|
||||
|
||||
@ -104,6 +104,12 @@ export class AuthService extends BaseService {
|
||||
|
||||
const updatedUser = await this.userRepository.update(user.id, { password: hashedPassword });
|
||||
|
||||
await this.eventRepository.emit('AuthChangePassword', {
|
||||
userId: user.id,
|
||||
currentSessionId: auth.session?.id,
|
||||
invalidateSessions: dto.invalidateSessions,
|
||||
});
|
||||
|
||||
return mapUserAdmin(updatedUser);
|
||||
}
|
||||
|
||||
|
||||
@ -43,17 +43,13 @@ describe('SessionService', () => {
|
||||
describe('logoutDevices', () => {
|
||||
it('should logout all devices', async () => {
|
||||
const currentSession = factory.session();
|
||||
const otherSession = factory.session();
|
||||
const auth = factory.auth({ session: currentSession });
|
||||
|
||||
mocks.session.getByUserId.mockResolvedValue([currentSession, otherSession]);
|
||||
mocks.session.delete.mockResolvedValue();
|
||||
mocks.session.invalidate.mockResolvedValue();
|
||||
|
||||
await sut.deleteAll(auth);
|
||||
|
||||
expect(mocks.session.getByUserId).toHaveBeenCalledWith(auth.user.id);
|
||||
expect(mocks.session.delete).toHaveBeenCalledWith(otherSession.id);
|
||||
expect(mocks.session.delete).not.toHaveBeenCalledWith(currentSession.id);
|
||||
expect(mocks.session.invalidate).toHaveBeenCalledWith({ userId: auth.user.id, excludeId: currentSession.id });
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@ -1,6 +1,6 @@
|
||||
import { BadRequestException, Injectable } from '@nestjs/common';
|
||||
import { DateTime } from 'luxon';
|
||||
import { OnJob } from 'src/decorators';
|
||||
import { OnEvent, OnJob } from 'src/decorators';
|
||||
import { AuthDto } from 'src/dtos/auth.dto';
|
||||
import {
|
||||
SessionCreateDto,
|
||||
@ -10,6 +10,7 @@ import {
|
||||
mapSession,
|
||||
} from 'src/dtos/session.dto';
|
||||
import { JobName, JobStatus, Permission, QueueName } from 'src/enum';
|
||||
import { ArgOf } from 'src/repositories/event.repository';
|
||||
import { BaseService } from 'src/services/base.service';
|
||||
|
||||
@Injectable()
|
||||
@ -69,18 +70,19 @@ export class SessionService extends BaseService {
|
||||
await this.sessionRepository.delete(id);
|
||||
}
|
||||
|
||||
async deleteAll(auth: AuthDto): Promise<void> {
|
||||
const userId = auth.user.id;
|
||||
const currentSessionId = auth.session?.id;
|
||||
await this.sessionRepository.invalidate({ userId, excludeId: currentSessionId });
|
||||
}
|
||||
|
||||
async lock(auth: AuthDto, id: string): Promise<void> {
|
||||
await this.requireAccess({ auth, permission: Permission.SessionLock, ids: [id] });
|
||||
await this.sessionRepository.update(id, { pinExpiresAt: null });
|
||||
}
|
||||
|
||||
async deleteAll(auth: AuthDto): Promise<void> {
|
||||
const sessions = await this.sessionRepository.getByUserId(auth.user.id);
|
||||
for (const session of sessions) {
|
||||
if (session.id === auth.session?.id) {
|
||||
continue;
|
||||
}
|
||||
await this.sessionRepository.delete(session.id);
|
||||
}
|
||||
@OnEvent({ name: 'AuthChangePassword' })
|
||||
async onAuthChangePassword({ userId, currentSessionId }: ArgOf<'AuthChangePassword'>): Promise<void> {
|
||||
await this.sessionRepository.invalidate({ userId, excludeId: currentSessionId });
|
||||
}
|
||||
}
|
||||
|
||||
@ -131,6 +131,7 @@ describe(AuthService.name, () => {
|
||||
describe('changePassword', () => {
|
||||
it('should change the password and login with it', async () => {
|
||||
const { sut, ctx } = setup();
|
||||
ctx.getMock(EventRepository).emit.mockResolvedValue();
|
||||
const dto = { password: 'password', newPassword: 'new-password' };
|
||||
const passwordHashed = await hash(dto.password, 10);
|
||||
const { user } = await ctx.newUser({ password: passwordHashed });
|
||||
|
||||
@ -4,6 +4,7 @@
|
||||
NotificationType,
|
||||
} from '$lib/components/shared-components/notification/notification';
|
||||
import SettingInputField from '$lib/components/shared-components/settings/setting-input-field.svelte';
|
||||
import SettingSwitch from '$lib/components/shared-components/settings/setting-switch.svelte';
|
||||
import { SettingInputFieldType } from '$lib/constants';
|
||||
import { changePassword } from '@immich/sdk';
|
||||
import { Button } from '@immich/ui';
|
||||
@ -14,10 +15,11 @@
|
||||
let password = $state('');
|
||||
let newPassword = $state('');
|
||||
let confirmPassword = $state('');
|
||||
let invalidateSessions = $state(false);
|
||||
|
||||
const handleChangePassword = async () => {
|
||||
try {
|
||||
await changePassword({ changePasswordDto: { password, newPassword } });
|
||||
await changePassword({ changePasswordDto: { password, newPassword, invalidateSessions } });
|
||||
|
||||
notificationController.show({
|
||||
message: $t('updated_password'),
|
||||
@ -69,6 +71,12 @@
|
||||
passwordAutocomplete="new-password"
|
||||
/>
|
||||
|
||||
<SettingSwitch
|
||||
title={$t('log_out_all_devices')}
|
||||
subtitle={$t('change_password_form_log_out_description')}
|
||||
bind:checked={invalidateSessions}
|
||||
/>
|
||||
|
||||
<div class="flex justify-end">
|
||||
<Button
|
||||
shape="round"
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user