diff --git a/apps/server/src/modules/authentication/controllers/api-test/login.api.spec.ts b/apps/server/src/modules/authentication/controllers/api-test/login.api.spec.ts index 29fa84dc438..7adbd663e87 100644 --- a/apps/server/src/modules/authentication/controllers/api-test/login.api.spec.ts +++ b/apps/server/src/modules/authentication/controllers/api-test/login.api.spec.ts @@ -218,8 +218,6 @@ describe('Login Controller (api)', () => { const decodedToken = jwt.decode(token); // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access expect(response.body.accessToken).toBeDefined(); - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - expect(response.body.accessToken).toBeDefined(); expect(decodedToken).toHaveProperty('userId'); expect(decodedToken).toHaveProperty('accountId'); expect(decodedToken).toHaveProperty('schoolId'); diff --git a/apps/server/src/modules/authentication/loggable/account-not-found.loggable-exception.spec.ts b/apps/server/src/modules/authentication/loggable/account-not-found.loggable-exception.spec.ts new file mode 100644 index 00000000000..660e60a3d97 --- /dev/null +++ b/apps/server/src/modules/authentication/loggable/account-not-found.loggable-exception.spec.ts @@ -0,0 +1,25 @@ +import { AccountNotFoundLoggableException } from './account-not-found.loggable-exception'; + +describe(AccountNotFoundLoggableException.name, () => { + describe('getLogMessage', () => { + const setup = () => { + const exception = new AccountNotFoundLoggableException(); + + return { + exception, + }; + }; + + it('should return the correct log message', () => { + const { exception } = setup(); + + const message = exception.getLogMessage(); + + expect(message).toEqual({ + type: 'UNAUTHORIZED_EXCEPTION', + stack: expect.any(String), + data: {}, + }); + }); + }); +}); diff --git a/apps/server/src/modules/authentication/loggable/account-not-found.loggable-exception.ts b/apps/server/src/modules/authentication/loggable/account-not-found.loggable-exception.ts new file mode 100644 index 00000000000..3d1a76b3c7a --- /dev/null +++ b/apps/server/src/modules/authentication/loggable/account-not-found.loggable-exception.ts @@ -0,0 +1,27 @@ +import { HttpStatus } from '@nestjs/common'; +import { BusinessError } from '@shared/common'; +import { Loggable } from '@src/core/logger/interfaces'; +import { ErrorLogMessage } from '@src/core/logger/types'; + +export class AccountNotFoundLoggableException extends BusinessError implements Loggable { + constructor() { + super( + { + type: 'UNAUTHORIZED_EXCEPTION', + title: 'Login has failed because account not found', + defaultMessage: 'Login has failed because account not found', + }, + HttpStatus.UNAUTHORIZED + ); + } + + getLogMessage(): ErrorLogMessage { + const message: ErrorLogMessage = { + type: this.type, + stack: this.stack, + data: {}, + }; + + return message; + } +} diff --git a/apps/server/src/modules/authentication/loggable/index.ts b/apps/server/src/modules/authentication/loggable/index.ts index 7e6fcda9db1..ed6249cc5bd 100644 --- a/apps/server/src/modules/authentication/loggable/index.ts +++ b/apps/server/src/modules/authentication/loggable/index.ts @@ -1 +1,4 @@ export * from './school-in-migration.loggable-exception'; +export * from './account-not-found.loggable-exception'; +export * from './user-could-not-be-authenticated.loggable.exception'; +export * from './user-authenticated.loggable'; diff --git a/apps/server/src/modules/authentication/loggable/user-authenticated.loggable.spec.ts b/apps/server/src/modules/authentication/loggable/user-authenticated.loggable.spec.ts new file mode 100644 index 00000000000..65b75abe810 --- /dev/null +++ b/apps/server/src/modules/authentication/loggable/user-authenticated.loggable.spec.ts @@ -0,0 +1,24 @@ +import { UserAuthenticatedLoggable } from './user-authenticated.loggable'; + +describe(UserAuthenticatedLoggable.name, () => { + describe('getLogMessage', () => { + const setup = () => { + const loggable = new UserAuthenticatedLoggable(); + + return { + loggable, + }; + }; + + it('should return the correct log message', () => { + const { loggable } = setup(); + + const message = loggable.getLogMessage(); + + expect(message).toEqual({ + message: 'SUCCESSFULLY_AUTHENTICATED', + data: {}, + }); + }); + }); +}); diff --git a/apps/server/src/modules/authentication/loggable/user-authenticated.loggable.ts b/apps/server/src/modules/authentication/loggable/user-authenticated.loggable.ts new file mode 100644 index 00000000000..58045c197b1 --- /dev/null +++ b/apps/server/src/modules/authentication/loggable/user-authenticated.loggable.ts @@ -0,0 +1,13 @@ +import { Loggable } from '@src/core/logger/interfaces'; +import { LogMessage } from '@src/core/logger/types'; + +export class UserAuthenticatedLoggable implements Loggable { + getLogMessage(): LogMessage { + const message: LogMessage = { + message: 'SUCCESSFULLY_AUTHENTICATED', + data: {}, + }; + + return message; + } +} diff --git a/apps/server/src/modules/authentication/loggable/user-could-not-be-authenticated.loggable.exception.spec.ts b/apps/server/src/modules/authentication/loggable/user-could-not-be-authenticated.loggable.exception.spec.ts new file mode 100644 index 00000000000..40d7504f0fd --- /dev/null +++ b/apps/server/src/modules/authentication/loggable/user-could-not-be-authenticated.loggable.exception.spec.ts @@ -0,0 +1,27 @@ +import { LdapUserCouldNotBeAuthenticatedLoggableException } from './user-could-not-be-authenticated.loggable.exception'; + +describe(LdapUserCouldNotBeAuthenticatedLoggableException.name, () => { + describe('getLogMessage', () => { + const setup = () => { + const err = new Error('test'); + const exception = new LdapUserCouldNotBeAuthenticatedLoggableException(err); + + return { + exception, + err, + }; + }; + + it('should return the correct log message', () => { + const { err, exception } = setup(); + + const message = exception.getLogMessage(); + + expect(message).toEqual({ + type: 'UNAUTHORIZED_EXCEPTION', + stack: expect.any(String), + data: { error: JSON.stringify(err) }, + }); + }); + }); +}); diff --git a/apps/server/src/modules/authentication/loggable/user-could-not-be-authenticated.loggable.exception.ts b/apps/server/src/modules/authentication/loggable/user-could-not-be-authenticated.loggable.exception.ts new file mode 100644 index 00000000000..91e8e8c6219 --- /dev/null +++ b/apps/server/src/modules/authentication/loggable/user-could-not-be-authenticated.loggable.exception.ts @@ -0,0 +1,29 @@ +import { HttpStatus } from '@nestjs/common'; +import { BusinessError } from '@shared/common'; +import { Loggable } from '@src/core/logger/interfaces'; +import { ErrorLogMessage } from '@src/core/logger/types'; + +export class LdapUserCouldNotBeAuthenticatedLoggableException extends BusinessError implements Loggable { + constructor(private readonly error: Error) { + super( + { + type: 'UNAUTHORIZED_EXCEPTION', + title: 'User could not be authenticated', + defaultMessage: 'LdapService connection failed because User could not be authenticated', + }, + HttpStatus.UNAUTHORIZED + ); + } + + getLogMessage(): ErrorLogMessage { + const message: ErrorLogMessage = { + type: this.type, + stack: this.stack, + data: { + error: JSON.stringify(this.error), + }, + }; + + return message; + } +} diff --git a/apps/server/src/modules/authentication/services/ldap.service.spec.ts b/apps/server/src/modules/authentication/services/ldap.service.spec.ts index 22f27573d3d..9fce46f1751 100644 --- a/apps/server/src/modules/authentication/services/ldap.service.spec.ts +++ b/apps/server/src/modules/authentication/services/ldap.service.spec.ts @@ -1,9 +1,9 @@ import { createMock } from '@golevelup/ts-jest'; -import { UnauthorizedException } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; import { SystemEntity } from '@shared/domain/entity'; import { systemEntityFactory } from '@shared/testing'; -import { LegacyLogger } from '@src/core/logger'; +import { Logger } from '@src/core/logger'; +import { LdapUserCouldNotBeAuthenticatedLoggableException } from '../loggable'; import { LdapService } from './ldap.service'; const mockClient = { @@ -47,8 +47,8 @@ describe('LdapService', () => { providers: [ LdapService, { - provide: LegacyLogger, - useValue: createMock(), + provide: Logger, + useValue: createMock(), }, ], }).compile(); @@ -76,19 +76,10 @@ describe('LdapService', () => { }); describe('when user is not authorized', () => { - it('should throw unauthorized error', async () => { + it('should throw UserCouldNotAuthenticateLoggableException', async () => { const system: SystemEntity = systemEntityFactory.withLdapConfig().buildWithId(); await expect(ldapService.checkLdapCredentials(system, 'mockUsername', 'mockPassword')).rejects.toThrow( - new UnauthorizedException('User could not authenticate') - ); - }); - }); - - describe('when connected flag is not set', () => { - it('should throw unauthorized error', async () => { - const system: SystemEntity = systemEntityFactory.withLdapConfig().buildWithId(); - await expect(ldapService.checkLdapCredentials(system, 'connectWithoutFlag', 'mockPassword')).rejects.toThrow( - new UnauthorizedException('User could not authenticate') + LdapUserCouldNotBeAuthenticatedLoggableException ); }); }); diff --git a/apps/server/src/modules/authentication/services/ldap.service.ts b/apps/server/src/modules/authentication/services/ldap.service.ts index 4a5609985fc..3436d4e76e0 100644 --- a/apps/server/src/modules/authentication/services/ldap.service.ts +++ b/apps/server/src/modules/authentication/services/ldap.service.ts @@ -1,13 +1,13 @@ import { Injectable, UnauthorizedException } from '@nestjs/common'; import { SystemEntity } from '@shared/domain/entity'; -import { ErrorUtils } from '@src/core/error/utils'; -import { LegacyLogger } from '@src/core/logger'; +import { Logger } from '@src/core/logger'; import { Client, createClient } from 'ldapjs'; import { LdapConnectionError } from '../errors/ldap-connection.error'; +import { UserAuthenticatedLoggable, LdapUserCouldNotBeAuthenticatedLoggableException } from '../loggable'; @Injectable() export class LdapService { - constructor(private readonly logger: LegacyLogger) { + constructor(private readonly logger: Logger) { this.logger.setContext(LdapService.name); } @@ -37,15 +37,9 @@ export class LdapService { client.on('connect', () => { client.bind(username, password, (err) => { if (err) { - this.logger.debug(err); - reject( - new UnauthorizedException( - 'User could not authenticate', - ErrorUtils.createHttpExceptionOptions(err, 'LdapService:connect') - ) - ); + reject(new LdapUserCouldNotBeAuthenticatedLoggableException(err)); } else { - this.logger.debug('[LDAP] Bind successful'); + this.logger.debug(new UserAuthenticatedLoggable()); resolve(client); } }); diff --git a/apps/server/src/modules/authentication/strategy/oauth2.strategy.spec.ts b/apps/server/src/modules/authentication/strategy/oauth2.strategy.spec.ts index 1263161ea90..efd16d2773c 100644 --- a/apps/server/src/modules/authentication/strategy/oauth2.strategy.spec.ts +++ b/apps/server/src/modules/authentication/strategy/oauth2.strategy.spec.ts @@ -1,7 +1,6 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; -import { AccountService, Account } from '@modules/account'; +import { Account, AccountService } from '@modules/account'; import { OAuthService, OAuthTokenDto } from '@modules/oauth'; -import { UnauthorizedException } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; import { UserDO } from '@shared/domain/domainobject/user.do'; import { RoleName } from '@shared/domain/interface'; @@ -12,8 +11,9 @@ import { ICurrentUser, OauthCurrentUser } from '../interface'; import { SchoolInMigrationLoggableException } from '../loggable'; -import { Oauth2Strategy } from './oauth2.strategy'; +import { AccountNotFoundLoggableException } from '../loggable/account-not-found.loggable-exception'; import { UserAccountDeactivatedLoggableException } from '../loggable/user-account-deactivated-exception'; +import { Oauth2Strategy } from './oauth2.strategy'; describe('Oauth2Strategy', () => { let module: TestingModule; @@ -133,7 +133,7 @@ describe('Oauth2Strategy', () => { accountService.findByUserId.mockResolvedValue(null); }; - it('should throw an UnauthorizedException', async () => { + it('should throw an AccountNotFoundLoggableException', async () => { setup(); const func = async () => @@ -141,7 +141,8 @@ describe('Oauth2Strategy', () => { body: { code: 'code', redirectUri: 'redirectUri', systemId: 'systemId' }, }); - await expect(func).rejects.toThrow(new UnauthorizedException('no account found')); + const loggableException = new AccountNotFoundLoggableException(); + await expect(func).rejects.toThrow(loggableException); }); }); diff --git a/apps/server/src/modules/authentication/strategy/oauth2.strategy.ts b/apps/server/src/modules/authentication/strategy/oauth2.strategy.ts index 1cc56d31173..697ffa9d1e3 100644 --- a/apps/server/src/modules/authentication/strategy/oauth2.strategy.ts +++ b/apps/server/src/modules/authentication/strategy/oauth2.strategy.ts @@ -1,12 +1,12 @@ import { AccountService, Account } from '@modules/account'; import { OAuthService, OAuthTokenDto } from '@modules/oauth'; -import { Injectable, UnauthorizedException } from '@nestjs/common'; +import { Injectable } from '@nestjs/common'; import { PassportStrategy } from '@nestjs/passport'; import { UserDO } from '@shared/domain/domainobject/user.do'; import { Strategy } from 'passport-custom'; import { Oauth2AuthorizationBodyParams } from '../controllers/dto'; import { ICurrentUser, OauthCurrentUser } from '../interface'; -import { SchoolInMigrationLoggableException } from '../loggable'; +import { AccountNotFoundLoggableException, SchoolInMigrationLoggableException } from '../loggable'; import { CurrentUserMapper } from '../mapper'; import { UserAccountDeactivatedLoggableException } from '../loggable/user-account-deactivated-exception'; @@ -29,7 +29,7 @@ export class Oauth2Strategy extends PassportStrategy(Strategy, 'oauth2') { const account: Account | null = await this.accountService.findByUserId(user.id); if (!account) { - throw new UnauthorizedException('no account found'); + throw new AccountNotFoundLoggableException(); } if (account.deactivatedAt !== undefined && account.deactivatedAt.getTime() <= Date.now()) {