Merge pull request #1954 from hedgedoc/invalid-credentials-error

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;
     }

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';
+}

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);
       });
     });
   });

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<void> {
     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();
     }
   }
 }

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;