diff --git a/src/api/private/auth/auth.controller.ts b/src/api/private/auth/auth.controller.ts index b2f0b3bc7..d95f6cbe9 100644 --- a/src/api/private/auth/auth.controller.ts +++ b/src/api/private/auth/auth.controller.ts @@ -9,10 +9,10 @@ import { ConflictException, Controller, Delete, - NotFoundException, Post, Put, Req, + UnauthorizedException, UseGuards, } from '@nestjs/common'; import { Session } from 'express-session'; @@ -70,6 +70,10 @@ export class AuthController { @Body() changePasswordDto: UpdatePasswordDto, ): Promise { try { + await this.identityService.checkLocalPassword( + user, + changePasswordDto.currentPassword, + ); await this.identityService.updateLocalPassword( user, changePasswordDto.newPassword, @@ -77,7 +81,9 @@ export class AuthController { return; } catch (e) { if (e instanceof NotInDBError) { - throw new NotFoundException(e.message); + throw new UnauthorizedException( + 'Verifying your identity with the current password did not work.', + ); } throw e; } diff --git a/src/identity/identity.service.spec.ts b/src/identity/identity.service.spec.ts index c93b4b797..91b3d3d96 100644 --- a/src/identity/identity.service.spec.ts +++ b/src/identity/identity.service.spec.ts @@ -102,9 +102,9 @@ describe('IdentityService', () => { ) as Identity; identity.passwordHash = await hashPassword(password); user.identities = Promise.resolve([identity]); - await expect( - service.loginWithLocalIdentity(user, password), - ).resolves.toEqual(undefined); + await expect(service.checkLocalPassword(user, password)).resolves.toEqual( + undefined, + ); }); describe('fails', () => { it('when user has no local identity', async () => { diff --git a/src/identity/identity.service.ts b/src/identity/identity.service.ts index 22e573b27..fa624c986 100644 --- a/src/identity/identity.service.ts +++ b/src/identity/identity.service.ts @@ -73,20 +73,20 @@ export class IdentityService { * @param {string} password - the password to use * @throws {NotInDBError} the specified user can't be logged in */ - async loginWithLocalIdentity(user: User, password: string): Promise { + async checkLocalPassword(user: User, password: string): Promise { const internalIdentity: Identity | undefined = await getFirstIdentityFromUser(user, ProviderType.LOCAL); if (internalIdentity === undefined) { this.logger.debug( `The user with the username ${user.username} does not have a internal identity.`, - 'loginWithLocalIdentity', + 'checkLocalPassword', ); throw new NotInDBError(); } if (!(await checkPassword(password, internalIdentity.passwordHash ?? ''))) { this.logger.debug( `Password check for ${user.username} did not succeed.`, - 'loginWithLocalIdentity', + 'checkLocalPassword', ); throw new NotInDBError(); } diff --git a/src/identity/local/local.strategy.ts b/src/identity/local/local.strategy.ts index 3f927577b..3952c9f90 100644 --- a/src/identity/local/local.strategy.ts +++ b/src/identity/local/local.strategy.ts @@ -30,7 +30,7 @@ export class LocalStrategy extends PassportStrategy(Strategy, 'local') { const user = await this.userService.getUserByUsername(username, [ UserRelationEnum.IDENTITIES, ]); - await this.identityService.loginWithLocalIdentity(user, password); + await this.identityService.checkLocalPassword(user, password); return user; } catch (e) { if (e instanceof NotInDBError) { diff --git a/src/identity/local/update-password.dto.ts b/src/identity/local/update-password.dto.ts index bfe473b32..abc379276 100644 --- a/src/identity/local/update-password.dto.ts +++ b/src/identity/local/update-password.dto.ts @@ -6,6 +6,8 @@ import { IsString } from 'class-validator'; export class UpdatePasswordDto { + @IsString() + currentPassword: string; @IsString() newPassword: string; } diff --git a/test/private-api/auth.e2e-spec.ts b/test/private-api/auth.e2e-spec.ts index c1976f219..8a14b281c 100644 --- a/test/private-api/auth.e2e-spec.ts +++ b/test/private-api/auth.e2e-spec.ts @@ -112,6 +112,7 @@ describe('Auth', () => { it('works', async () => { // Change password const changePasswordDto: UpdatePasswordDto = { + currentPassword: password, newPassword: newPassword, }; await request(testSetup.app.getHttpServer()) @@ -133,6 +134,7 @@ describe('Auth', () => { cookie = response.get('Set-Cookie')[0]; // Reset password const changePasswordBackDto: UpdatePasswordDto = { + currentPassword: newPassword, newPassword: password, }; await request(testSetup.app.getHttpServer()) @@ -146,6 +148,7 @@ describe('Auth', () => { testSetup.configService.get('authConfig').local.enableLogin = false; // Try to change password const changePasswordDto: UpdatePasswordDto = { + currentPassword: password, newPassword: newPassword, }; await request(testSetup.app.getHttpServer()) @@ -177,6 +180,29 @@ describe('Auth', () => { .send(JSON.stringify(loginOldPasswordDto)) .expect(201); }); + it('fails, when old password is wrong', async () => { + // Try to change password + const changePasswordDto: UpdatePasswordDto = { + currentPassword: 'wrong', + newPassword: newPassword, + }; + await request(testSetup.app.getHttpServer()) + .put('/api/private/auth/local') + .set('Content-Type', 'application/json') + .set('Cookie', cookie) + .send(JSON.stringify(changePasswordDto)) + .expect(401); + // old password still does work for login + const loginOldPasswordDto: LoginDto = { + password: password, + username: username, + }; + await request(testSetup.app.getHttpServer()) + .post('/api/private/auth/local/login') + .set('Content-Type', 'application/json') + .send(JSON.stringify(loginOldPasswordDto)) + .expect(201); + }); }); describe('POST /auth/local/login', () => { diff --git a/test/private-api/register-and-login.e2e-spec.ts b/test/private-api/register-and-login.e2e-spec.ts index e84520fef..82e75c675 100644 --- a/test/private-api/register-and-login.e2e-spec.ts +++ b/test/private-api/register-and-login.e2e-spec.ts @@ -114,6 +114,7 @@ describe('Register and Login', () => { .set('Content-Type', 'application/json') .send( JSON.stringify({ + currentPassword: PASSWORD, newPassword: 'newPassword', }), )