From e4d9adfee133299ea24cc726c3c595e6601d66aa Mon Sep 17 00:00:00 2001 From: MBergCap <111343628+MBergCap@users.noreply.github.com> Date: Fri, 19 Apr 2024 09:58:44 +0200 Subject: [PATCH] N21-1858 prevent migration to same login system (#4934) * add loggables --- ...igration-system.loggable-exception.spec.ts | 37 +++++++++++++++++ ...gin-migration-system.loggable-exception.ts | 34 ++++++++++++++++ .../user-login-migration/loggable/index.ts | 2 + ...ystem-not-found.loggable-exception.spec.ts | 25 ++++++++++++ ...ule-system-not-found.loggable-exception.ts | 24 +++++++++++ .../user-login-migration.service.spec.ts | 40 +++++++++++++++++-- .../service/user-login-migration.service.ts | 20 +++++----- 7 files changed, 169 insertions(+), 13 deletions(-) create mode 100644 apps/server/src/modules/user-login-migration/loggable/identical-user-login-migration-system.loggable-exception.spec.ts create mode 100644 apps/server/src/modules/user-login-migration/loggable/identical-user-login-migration-system.loggable-exception.ts create mode 100644 apps/server/src/modules/user-login-migration/loggable/moin-schule-system-not-found.loggable-exception.spec.ts create mode 100644 apps/server/src/modules/user-login-migration/loggable/moin-schule-system-not-found.loggable-exception.ts diff --git a/apps/server/src/modules/user-login-migration/loggable/identical-user-login-migration-system.loggable-exception.spec.ts b/apps/server/src/modules/user-login-migration/loggable/identical-user-login-migration-system.loggable-exception.spec.ts new file mode 100644 index 00000000000..d81f84b7ccc --- /dev/null +++ b/apps/server/src/modules/user-login-migration/loggable/identical-user-login-migration-system.loggable-exception.spec.ts @@ -0,0 +1,37 @@ +import { IdenticalUserLoginMigrationSystemLoggableException } from '@modules/user-login-migration/loggable/identical-user-login-migration-system.loggable-exception'; +import { ObjectId } from '@mikro-orm/mongodb'; + +describe(IdenticalUserLoginMigrationSystemLoggableException.name, () => { + describe('getLogMessage', () => { + const setup = () => { + const schoolId = new ObjectId().toHexString(); + const targetSystemId = new ObjectId().toHexString(); + + const exception: IdenticalUserLoginMigrationSystemLoggableException = + new IdenticalUserLoginMigrationSystemLoggableException(schoolId, targetSystemId); + + return { + schoolId, + targetSystemId, + exception, + }; + }; + + it('should return log message', () => { + const { exception, schoolId, targetSystemId } = setup(); + + const result = exception.getLogMessage(); + + expect(result).toEqual({ + type: 'IDENTICAL_USER_LOGIN_MIGRATION_SYSTEM', + message: + 'The migration cannot be started, because the target system and current schools login system are the same.', + stack: exception.stack, + data: { + schoolId, + targetSystemId, + }, + }); + }); + }); +}); diff --git a/apps/server/src/modules/user-login-migration/loggable/identical-user-login-migration-system.loggable-exception.ts b/apps/server/src/modules/user-login-migration/loggable/identical-user-login-migration-system.loggable-exception.ts new file mode 100644 index 00000000000..2e1a7391f06 --- /dev/null +++ b/apps/server/src/modules/user-login-migration/loggable/identical-user-login-migration-system.loggable-exception.ts @@ -0,0 +1,34 @@ +import { ErrorLogMessage, Loggable, LogMessage, ValidationErrorLogMessage } from '@src/core/logger'; +import { BusinessError } from '@shared/common'; +import { EntityId } from '@shared/domain/types'; +import { HttpStatus } from '@nestjs/common'; + +export class IdenticalUserLoginMigrationSystemLoggableException extends BusinessError implements Loggable { + constructor(private readonly schoolId: string | undefined, private readonly targetSystemId: EntityId | undefined) { + super( + { + type: 'IDENTICAL_USER_LOGIN_MIGRATION_SYSTEM', + title: 'Identical user login migration system', + defaultMessage: + 'The migration cannot be started, because the target system and current schools login system are the same.', + }, + HttpStatus.INTERNAL_SERVER_ERROR, + { + schoolId, + targetSystemId, + } + ); + } + + getLogMessage(): LogMessage | ErrorLogMessage | ValidationErrorLogMessage { + return { + type: this.type, + message: this.message, + stack: this.stack, + data: { + schoolId: this.schoolId, + targetSystemId: this.targetSystemId, + }, + }; + } +} diff --git a/apps/server/src/modules/user-login-migration/loggable/index.ts b/apps/server/src/modules/user-login-migration/loggable/index.ts index 2c0fd3a8650..b20d3584ed5 100644 --- a/apps/server/src/modules/user-login-migration/loggable/index.ts +++ b/apps/server/src/modules/user-login-migration/loggable/index.ts @@ -10,4 +10,6 @@ export * from './external-school-number-missing.loggable-exception'; export * from './user-migration-database-operation-failed.loggable-exception'; export * from './school-migration-database-operation-failed.loggable-exception'; export * from './invalid-user-login-migration.loggable-exception'; +export * from './identical-user-login-migration-system.loggable-exception'; +export * from './moin-schule-system-not-found.loggable-exception'; export * from './debug'; diff --git a/apps/server/src/modules/user-login-migration/loggable/moin-schule-system-not-found.loggable-exception.spec.ts b/apps/server/src/modules/user-login-migration/loggable/moin-schule-system-not-found.loggable-exception.spec.ts new file mode 100644 index 00000000000..fa9e4e00956 --- /dev/null +++ b/apps/server/src/modules/user-login-migration/loggable/moin-schule-system-not-found.loggable-exception.spec.ts @@ -0,0 +1,25 @@ +import { MoinSchuleSystemNotFoundLoggableException } from '@modules/user-login-migration/loggable/moin-schule-system-not-found.loggable-exception'; + +describe(MoinSchuleSystemNotFoundLoggableException.name, () => { + describe('getLogMessage', () => { + const setup = () => { + const exception: MoinSchuleSystemNotFoundLoggableException = new MoinSchuleSystemNotFoundLoggableException(); + + return { + exception, + }; + }; + + it('should return log message', () => { + const { exception } = setup(); + + const result = exception.getLogMessage(); + + expect(result).toEqual({ + type: 'MOIN_SCHULE_SYSTEM_NOT_FOUND', + message: 'Cannot find moin.schule system', + stack: exception.stack, + }); + }); + }); +}); diff --git a/apps/server/src/modules/user-login-migration/loggable/moin-schule-system-not-found.loggable-exception.ts b/apps/server/src/modules/user-login-migration/loggable/moin-schule-system-not-found.loggable-exception.ts new file mode 100644 index 00000000000..93c0dfce2cf --- /dev/null +++ b/apps/server/src/modules/user-login-migration/loggable/moin-schule-system-not-found.loggable-exception.ts @@ -0,0 +1,24 @@ +import { ErrorLogMessage, Loggable, LogMessage, ValidationErrorLogMessage } from '@src/core/logger'; +import { BusinessError } from '@shared/common'; +import { HttpStatus } from '@nestjs/common'; + +export class MoinSchuleSystemNotFoundLoggableException extends BusinessError implements Loggable { + constructor() { + super( + { + type: 'MOIN_SCHULE_SYSTEM_NOT_FOUND', + title: 'moin.schule system not found', + defaultMessage: 'Cannot find moin.schule system', + }, + HttpStatus.INTERNAL_SERVER_ERROR + ); + } + + getLogMessage(): LogMessage | ErrorLogMessage | ValidationErrorLogMessage { + return { + type: this.type, + message: this.message, + stack: this.stack, + }; + } +} diff --git a/apps/server/src/modules/user-login-migration/service/user-login-migration.service.spec.ts b/apps/server/src/modules/user-login-migration/service/user-login-migration.service.spec.ts index ff97173baa2..ee4a7cff300 100644 --- a/apps/server/src/modules/user-login-migration/service/user-login-migration.service.spec.ts +++ b/apps/server/src/modules/user-login-migration/service/user-login-migration.service.spec.ts @@ -5,13 +5,14 @@ import { LegacySchoolService } from '@modules/legacy-school'; import { LegacySystemService } from '@modules/system'; import { SystemDto } from '@modules/system/service'; import { UserService } from '@modules/user'; -import { InternalServerErrorException } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; import { LegacySchoolDo, UserDO, UserLoginMigrationDO } from '@shared/domain/domainobject'; import { EntityId, SchoolFeature } from '@shared/domain/types'; import { UserLoginMigrationRepo } from '@shared/repo'; import { legacySchoolDoFactory, userDoFactory, userLoginMigrationDOFactory } from '@shared/testing'; import { + IdenticalUserLoginMigrationSystemLoggableException, + MoinSchuleSystemNotFoundLoggableException, UserLoginMigrationAlreadyClosedLoggableException, UserLoginMigrationGracePeriodExpiredLoggableException, } from '../loggable'; @@ -339,12 +340,45 @@ describe(UserLoginMigrationService.name, () => { }; }; - it('should throw an InternalServerErrorException', async () => { + it('should throw a MoinSchuleSystemNotFoundLoggableException', async () => { const { schoolId } = setup(); const func = async () => service.startMigration(schoolId); - await expect(func).rejects.toThrow(new InternalServerErrorException('Cannot find SANIS system')); + await expect(func).rejects.toThrow(new MoinSchuleSystemNotFoundLoggableException()); + }); + }); + + describe('when creating a new migration but the SANIS system and schools login system are the same', () => { + const setup = () => { + const targetSystemId: EntityId = new ObjectId().toHexString(); + const system: SystemDto = new SystemDto({ + id: targetSystemId, + type: 'oauth2', + alias: 'SANIS', + }); + + const schoolId: EntityId = new ObjectId().toHexString(); + const school: LegacySchoolDo = legacySchoolDoFactory.buildWithId({ systems: [targetSystemId] }, schoolId); + + schoolService.getSchoolById.mockResolvedValue(school); + systemService.findByType.mockResolvedValue([system]); + userLoginMigrationRepo.findBySchoolId.mockResolvedValue(null); + + return { + schoolId, + targetSystemId, + }; + }; + + it('should throw an IdenticalUserLoginMigrationSystemLoggableException', async () => { + const { schoolId, targetSystemId } = setup(); + + const func = async () => service.startMigration(schoolId); + + await expect(func).rejects.toThrow( + new IdenticalUserLoginMigrationSystemLoggableException(schoolId, targetSystemId) + ); }); }); }); diff --git a/apps/server/src/modules/user-login-migration/service/user-login-migration.service.ts b/apps/server/src/modules/user-login-migration/service/user-login-migration.service.ts index e9ade8188af..44876628e7a 100644 --- a/apps/server/src/modules/user-login-migration/service/user-login-migration.service.ts +++ b/apps/server/src/modules/user-login-migration/service/user-login-migration.service.ts @@ -2,13 +2,15 @@ import { Configuration } from '@hpi-schul-cloud/commons/lib'; import { LegacySchoolService } from '@modules/legacy-school'; import { LegacySystemService, SystemDto } from '@modules/system'; import { UserService } from '@modules/user'; -import { Injectable, InternalServerErrorException } from '@nestjs/common'; +import { Injectable } from '@nestjs/common'; import { LegacySchoolDo, UserDO, UserLoginMigrationDO } from '@shared/domain/domainobject'; import { EntityId, SchoolFeature, SystemTypeEnum } from '@shared/domain/types'; import { UserLoginMigrationRepo } from '@shared/repo'; import { UserLoginMigrationAlreadyClosedLoggableException, UserLoginMigrationGracePeriodExpiredLoggableException, + IdenticalUserLoginMigrationSystemLoggableException, + MoinSchuleSystemNotFoundLoggableException, } from '../loggable'; @Injectable() @@ -110,20 +112,18 @@ export class UserLoginMigrationService { private async createNewMigration(school: LegacySchoolDo): Promise { const oauthSystems: SystemDto[] = await this.systemService.findByType(SystemTypeEnum.OAUTH); - const sanisSystem: SystemDto | undefined = oauthSystems.find((system: SystemDto) => system.alias === 'SANIS'); + const moinSchuleSystem: SystemDto | undefined = oauthSystems.find((system: SystemDto) => system.alias === 'SANIS'); - if (!sanisSystem) { - throw new InternalServerErrorException('Cannot find SANIS system'); + if (!moinSchuleSystem) { + throw new MoinSchuleSystemNotFoundLoggableException(); + } else if (school.systems?.includes(moinSchuleSystem.id as string)) { + throw new IdenticalUserLoginMigrationSystemLoggableException(school.id, moinSchuleSystem.id); } - const systemIds: EntityId[] = - school.systems?.filter((systemId: EntityId) => systemId !== (sanisSystem.id as string)) || []; - const sourceSystemId = systemIds[0]; - const userLoginMigrationDO: UserLoginMigrationDO = new UserLoginMigrationDO({ schoolId: school.id as string, - targetSystemId: sanisSystem.id as string, - sourceSystemId, + targetSystemId: moinSchuleSystem.id as string, + sourceSystemId: school.systems?.[0], startedAt: new Date(), });