From 29f60f8140846f9d920f3e0bc163c83ed1c2bd3b Mon Sep 17 00:00:00 2001 From: Yannick Bungers Date: Thu, 6 Jan 2022 21:59:46 +0100 Subject: [PATCH 1/2] Change error types in checkLocalPassword and updateLocalPassword to InvalidCredentialsError and NoLocalIdentityError Signed-off-by: Yannick Bungers --- src/api/private/auth/auth.controller.ts | 15 ++++++++++----- src/errors/errors.ts | 8 ++++++++ src/identity/identity.service.ts | 18 +++++++++++------- src/identity/local/local.strategy.ts | 12 +++++++++--- 4 files changed, 38 insertions(+), 15 deletions(-) diff --git a/src/api/private/auth/auth.controller.ts b/src/api/private/auth/auth.controller.ts index d95f6cbe9..fa33c5045 100644 --- a/src/api/private/auth/auth.controller.ts +++ b/src/api/private/auth/auth.controller.ts @@ -17,7 +17,11 @@ import { } from '@nestjs/common'; import { Session } from 'express-session'; -import { AlreadyInDBError, NotInDBError } from '../../../errors/errors'; +import { + AlreadyInDBError, + InvalidCredentialsError, + NoLocalIdentityError, +} from '../../../errors/errors'; import { IdentityService } from '../../../identity/identity.service'; import { LocalAuthGuard } from '../../../identity/local/local.strategy'; import { LoginDto } from '../../../identity/local/login.dto'; @@ -80,10 +84,11 @@ export class AuthController { ); return; } catch (e) { - if (e instanceof NotInDBError) { - throw new UnauthorizedException( - 'Verifying your identity with the current password did not work.', - ); + if (e instanceof InvalidCredentialsError) { + throw new UnauthorizedException('Password is not correct'); + } + if (e instanceof NoLocalIdentityError) { + throw new BadRequestException('User has no local identity.'); } throw e; } diff --git a/src/errors/errors.ts b/src/errors/errors.ts index a42b6e31d..9ee14b990 100644 --- a/src/errors/errors.ts +++ b/src/errors/errors.ts @@ -43,3 +43,11 @@ export class MediaBackendError extends Error { export class PrimaryAliasDeletionForbiddenError extends Error { name = 'PrimaryAliasDeletionForbiddenError'; } + +export class InvalidCredentialsError extends Error { + name = 'InvalidCredentialsError'; +} + +export class NoLocalIdentityError extends Error { + name = 'NoLocalIdentityError'; +} diff --git a/src/identity/identity.service.ts b/src/identity/identity.service.ts index fa624c986..044eb86bf 100644 --- a/src/identity/identity.service.ts +++ b/src/identity/identity.service.ts @@ -8,7 +8,10 @@ import { InjectRepository } from '@nestjs/typeorm'; import { Repository } from 'typeorm'; import authConfiguration, { AuthConfig } from '../config/auth.config'; -import { NotInDBError } from '../errors/errors'; +import { + InvalidCredentialsError, + NoLocalIdentityError, +} from '../errors/errors'; import { ConsoleLoggerService } from '../logger/console-logger.service'; import { User } from '../users/user.entity'; import { checkPassword, hashPassword } from '../utils/password'; @@ -46,7 +49,7 @@ export class IdentityService { * Update the internal password of the specified the user * @param {User} user - the user, which identity should be updated * @param {string} newPassword - the new password - * @throws {NotInDBError} the specified user has no internal identity + * @throws {NoLocalIdentityError} the specified user has no internal identity * @return {Identity} the changed identity */ async updateLocalPassword( @@ -60,7 +63,7 @@ export class IdentityService { `The user with the username ${user.username} does not have a internal identity.`, 'updateLocalPassword', ); - throw new NotInDBError('This user has no internal identity.'); + throw new NoLocalIdentityError('This user has no internal identity.'); } internalIdentity.passwordHash = await hashPassword(newPassword); return await this.identityRepository.save(internalIdentity); @@ -68,10 +71,11 @@ export class IdentityService { /** * @async - * Login the user with their username and password + * Checks if the user and password combination matches * @param {User} user - the user to use * @param {string} password - the password to use - * @throws {NotInDBError} the specified user can't be logged in + * @throws {InvalidCredentialsError} the password and user do not match + * @throws {NoLocalIdentityError} the specified user has no internal identity */ async checkLocalPassword(user: User, password: string): Promise { const internalIdentity: Identity | undefined = @@ -81,14 +85,14 @@ export class IdentityService { `The user with the username ${user.username} does not have a internal identity.`, 'checkLocalPassword', ); - throw new NotInDBError(); + throw new NoLocalIdentityError(); } if (!(await checkPassword(password, internalIdentity.passwordHash ?? ''))) { this.logger.debug( `Password check for ${user.username} did not succeed.`, 'checkLocalPassword', ); - throw new NotInDBError(); + throw new InvalidCredentialsError(); } } } diff --git a/src/identity/local/local.strategy.ts b/src/identity/local/local.strategy.ts index 3952c9f90..5e2a6239d 100644 --- a/src/identity/local/local.strategy.ts +++ b/src/identity/local/local.strategy.ts @@ -7,7 +7,10 @@ import { Injectable, UnauthorizedException } from '@nestjs/common'; import { AuthGuard, PassportStrategy } from '@nestjs/passport'; import { Strategy } from 'passport-local'; -import { NotInDBError } from '../../errors/errors'; +import { + InvalidCredentialsError, + NoLocalIdentityError, +} from '../../errors/errors'; import { UserRelationEnum } from '../../users/user-relation.enum'; import { User } from '../../users/user.entity'; import { UsersService } from '../../users/users.service'; @@ -33,9 +36,12 @@ export class LocalStrategy extends PassportStrategy(Strategy, 'local') { await this.identityService.checkLocalPassword(user, password); return user; } catch (e) { - if (e instanceof NotInDBError) { + if ( + e instanceof InvalidCredentialsError || + e instanceof NoLocalIdentityError + ) { throw new UnauthorizedException( - 'This username and password combination did not work.', + 'This username and password combination is not valid.', ); } throw e; From f3899f3afdd6f60de0d5f40acec8f950fcac49ef Mon Sep 17 00:00:00 2001 From: Yannick Bungers Date: Thu, 6 Jan 2022 22:01:39 +0100 Subject: [PATCH 2/2] Update error types for checkLocalPassword and updateLocalPassword to InvalidCredentialsError and NoLocalIdentityError in tests Signed-off-by: Yannick Bungers --- src/identity/identity.service.spec.ts | 29 ++++++++++++++++++--------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/src/identity/identity.service.spec.ts b/src/identity/identity.service.spec.ts index 91b3d3d96..356670d36 100644 --- a/src/identity/identity.service.spec.ts +++ b/src/identity/identity.service.spec.ts @@ -10,7 +10,10 @@ import { Repository } from 'typeorm'; import appConfigMock from '../config/mock/app.config.mock'; import authConfigMock from '../config/mock/auth.config.mock'; -import { NotInDBError } from '../errors/errors'; +import { + InvalidCredentialsError, + NoLocalIdentityError, +} from '../errors/errors'; import { LoggerModule } from '../logger/logger.module'; import { User } from '../users/user.entity'; import { checkPassword, hashPassword } from '../utils/password'; @@ -88,7 +91,7 @@ describe('IdentityService', () => { it('fails, when user has no local identity', async () => { user.identities = Promise.resolve([]); await expect(service.updateLocalPassword(user, password)).rejects.toThrow( - NotInDBError, + NoLocalIdentityError, ); }); }); @@ -107,17 +110,23 @@ describe('IdentityService', () => { ); }); describe('fails', () => { + it('when the password is wrong', async () => { + const identity = Identity.create( + user, + ProviderType.LOCAL, + false, + ) as Identity; + identity.passwordHash = await hashPassword(password); + user.identities = Promise.resolve([identity]); + await expect( + service.checkLocalPassword(user, 'wrong_password'), + ).rejects.toThrow(InvalidCredentialsError); + }); it('when user has no local identity', async () => { user.identities = Promise.resolve([]); await expect( - service.updateLocalPassword(user, password), - ).rejects.toThrow(NotInDBError); - }); - it('when the password is wrong', async () => { - user.identities = Promise.resolve([]); - await expect( - service.updateLocalPassword(user, 'wrong_password'), - ).rejects.toThrow(NotInDBError); + service.checkLocalPassword(user, password), + ).rejects.toThrow(NoLocalIdentityError); }); }); });