N21-1447 user-login-migration refactorings

apps/server/src/modules/authentication/index.ts

@@ -1,3 +1,4 @@
 export { ICurrentUser } from './interface';
 export { JWT, CurrentUser, Authenticate } from './decorator';
 export { AuthenticationModule } from './authentication.module';
+export { AuthenticationService } from './services';

apps/server/src/modules/authentication/services/index.ts

@@ -0,0 +1,2 @@
+export * from './ldap.service';
+export * from './authentication.service';

apps/server/src/modules/authentication/strategy/oauth2.strategy.spec.ts

@@ -1,8 +1,7 @@
 import { createMock, DeepMocked } from '@golevelup/ts-jest';
 import { AccountService } from '@modules/account/services/account.service';
 import { AccountDto } from '@modules/account/services/dto';
-import { OAuthTokenDto } from '@modules/oauth';
-import { OAuthService } from '@modules/oauth/service/oauth.service';
+import { OAuthTokenDto, OAuthService } from '@modules/oauth';
 import { UnauthorizedException } from '@nestjs/common';
 import { Test, TestingModule } from '@nestjs/testing';
 import { EntityId, RoleName } from '@shared/domain';

apps/server/src/modules/authentication/strategy/oauth2.strategy.ts

@@ -1,7 +1,6 @@
 import { AccountService } from '@modules/account/services/account.service';
 import { AccountDto } from '@modules/account/services/dto';
-import { OAuthTokenDto } from '@modules/oauth';
-import { OAuthService } from '@modules/oauth/service/oauth.service';
+import { OAuthTokenDto, OAuthService } from '@modules/oauth';
 import { Injectable, UnauthorizedException } from '@nestjs/common';
 import { PassportStrategy } from '@nestjs/passport';
 import { UserDO } from '@shared/domain/domainobject/user.do';

apps/server/src/modules/oauth/index.ts

@@ -1,2 +1,3 @@
 export * from './oauth.module';
 export * from './interface';
+export * from './service';

apps/server/src/modules/oauth/service/index.ts

@@ -0,0 +1,3 @@
+export * from './hydra.service';
+export * from './oauth.service';
+export * from './oauth-adapter.service';

apps/server/src/modules/oauth/service/oauth.service.ts

@@ -1,7 +1,6 @@
 import { DefaultEncryptionService, IEncryptionService } from '@infra/encryption';
 import { LegacySchoolService } from '@modules/legacy-school';
-import { ProvisioningService } from '@modules/provisioning';
-import { OauthDataDto } from '@modules/provisioning/dto';
+import { ProvisioningService, OauthDataDto } from '@modules/provisioning';
 import { SystemService } from '@modules/system';
 import { SystemDto } from '@modules/system/service';
 import { UserService } from '@modules/user';

apps/server/src/modules/oauth/uc/hydra-oauth.uc.spec.ts

@@ -7,8 +7,7 @@ import { OauthConfig } from '@shared/domain';
 import { axiosResponseFactory } from '@shared/testing';
 import { LegacyLogger } from '@src/core/logger';
 import { HydraRedirectDto } from '@modules/oauth/service/dto/hydra.redirect.dto';
-import { HydraSsoService } from '@modules/oauth/service/hydra.service';
-import { OAuthService } from '@modules/oauth/service/oauth.service';
+import { OAuthService, HydraSsoService } from '@modules/oauth';
 import { AxiosResponse } from 'axios';
 import { HydraOauthUc } from '.';
 import { AuthorizationParams } from '../controller/dto';

apps/server/src/modules/provisioning/index.ts

@@ -1,4 +1,4 @@
 export * from './provisioning.module';
-export * from './dto/provisioning.dto';
+export * from './dto';
 export * from './service/provisioning.service';
 export * from './strategy';

apps/server/src/modules/user-login-migration/loggable/user-login-migration-already-closed.loggable-exception.ts

@@ -3,7 +3,7 @@ import { EntityId } from '@shared/domain';
 import { ErrorLogMessage, Loggable, LogMessage, ValidationErrorLogMessage } from '@src/core/logger';
 
 export class UserLoginMigrationAlreadyClosedLoggableException extends UnprocessableEntityException implements Loggable {
-	constructor(private readonly userLoginMigrationId: EntityId, private readonly closedAt: Date) {
+	constructor(private readonly closedAt: Date, private readonly userLoginMigrationId?: EntityId) {
 		super();
 	}
 

apps/server/src/modules/user-login-migration/loggable/user-login-migration-mandatory.loggable.spec.ts

@@ -0,0 +1,33 @@
+import { ObjectId } from 'bson';
+import { UserLoginMigrationMandatoryLoggable } from './user-login-migration-mandatory.loggable';
+
+describe(UserLoginMigrationMandatoryLoggable.name, () => {
+	describe('getLogMessage', () => {
+		const setup = () => {
+			const userId = new ObjectId().toHexString();
+			const userLoginMigrationId = new ObjectId().toHexString();
+			const exception = new UserLoginMigrationMandatoryLoggable(userId, userLoginMigrationId, true);
+
+			return {
+				exception,
+				userId,
+				userLoginMigrationId,
+			};
+		};
+
+		it('should return the correct log message', () => {
+			const { exception, userId, userLoginMigrationId } = setup();
+
+			const message = exception.getLogMessage();
+
+			expect(message).toEqual({
+				message: 'The school administrator changed the requirement status of the user login migration for his school.',
+				data: {
+					userId,
+					userLoginMigrationId,
+					mandatory: true,
+				},
+			});
+		});
+	});
+});

apps/server/src/modules/user-login-migration/loggable/user-login-migration-mandatory.loggable.ts

@@ -4,7 +4,7 @@ import { ErrorLogMessage, Loggable, LogMessage, ValidationErrorLogMessage } from
 export class UserLoginMigrationMandatoryLoggable implements Loggable {
 	constructor(
 		private readonly userId: EntityId,
-		private readonly userLoginMigrationId: EntityId,
+		private readonly userLoginMigrationId: EntityId | undefined,
 		private readonly mandatory: boolean
 	) {}
 

apps/server/src/modules/user-login-migration/loggable/user-login-migration-start.loggable.spec.ts

@@ -0,0 +1,32 @@
+import { ObjectId } from 'bson';
+import { UserLoginMigrationStartLoggable } from './user-login-migration-start.loggable';
+
+describe(UserLoginMigrationStartLoggable.name, () => {
+	describe('getLogMessage', () => {
+		const setup = () => {
+			const userId = new ObjectId().toHexString();
+			const userLoginMigrationId = new ObjectId().toHexString();
+			const exception = new UserLoginMigrationStartLoggable(userId, userLoginMigrationId);
+
+			return {
+				exception,
+				userId,
+				userLoginMigrationId,
+			};
+		};
+
+		it('should return the correct log message', () => {
+			const { exception, userId, userLoginMigrationId } = setup();
+
+			const message = exception.getLogMessage();
+
+			expect(message).toEqual({
+				message: 'The school administrator started the migration for his school.',
+				data: {
+					userId,
+					userLoginMigrationId,
+				},
+			});
+		});
+	});
+});

apps/server/src/modules/user-login-migration/loggable/user-login-migration-start.loggable.ts

@@ -2,7 +2,7 @@ import { EntityId } from '@shared/domain';
 import { ErrorLogMessage, Loggable, LogMessage, ValidationErrorLogMessage } from '@src/core/logger';
 
 export class UserLoginMigrationStartLoggable implements Loggable {
-	constructor(private readonly userId: EntityId, private readonly userLoginMigrationId: EntityId) {}
+	constructor(private readonly userId: EntityId, private readonly userLoginMigrationId: EntityId | undefined) {}
 
 	getLogMessage(): LogMessage | ErrorLogMessage | ValidationErrorLogMessage {
 		return {

apps/server/src/modules/user-login-migration/service/dto/index.ts

@@ -1,3 +1,2 @@
 export * from './migration.dto';
 export * from './page-content.dto';
-export * from './school-migration-flags';

apps/server/src/modules/user-login-migration/service/migration-check.service.spec.ts

@@ -87,6 +87,7 @@ describe('MigrationCheckService', () => {
 					targetSystemId: 'targetSystemId',
 					startedAt: new Date('2023-03-03'),
 				});
+
 				schoolService.getSchoolBySchoolNumber.mockResolvedValue(school);
 				userService.findByExternalId.mockResolvedValue(null);
 				userLoginMigrationRepo.findBySchoolId.mockResolvedValue(userLoginMigration);

apps/server/src/modules/user-login-migration/service/migration-check.service.ts

@@ -1,8 +1,8 @@
+import { LegacySchoolService } from '@modules/legacy-school';
+import { UserService } from '@modules/user';
 import { Injectable } from '@nestjs/common';
 import { EntityId, LegacySchoolDo, UserDO, UserLoginMigrationDO } from '@shared/domain';
 import { UserLoginMigrationRepo } from '@shared/repo';
-import { LegacySchoolService } from '@modules/legacy-school';
-import { UserService } from '@modules/user';
 
 @Injectable()
 export class MigrationCheckService {
@@ -12,22 +12,43 @@ export class MigrationCheckService {
 		private readonly userLoginMigrationRepo: UserLoginMigrationRepo
 	) {}
 
-	async shouldUserMigrate(externalUserId: string, systemId: EntityId, officialSchoolNumber: string): Promise<boolean> {
+	public async shouldUserMigrate(
+		externalUserId: string,
+		systemId: EntityId,
+		officialSchoolNumber: string
+	): Promise<boolean> {
 		const school: LegacySchoolDo | null = await this.schoolService.getSchoolBySchoolNumber(officialSchoolNumber);
 
-		if (school && school.id) {
-			const userLoginMigration: UserLoginMigrationDO | null = await this.userLoginMigrationRepo.findBySchoolId(
-				school.id
-			);
+		if (!school?.id) {
+			return false;
+		}
+
+		const userLoginMigration: UserLoginMigrationDO | null = await this.userLoginMigrationRepo.findBySchoolId(school.id);
+
+		if (!userLoginMigration || !this.isMigrationActive(userLoginMigration)) {
+			return false;
+		}
 
-			const user: UserDO | null = await this.userService.findByExternalId(externalUserId, systemId);
+		const user: UserDO | null = await this.userService.findByExternalId(externalUserId, systemId);
 
-			if (user?.lastLoginSystemChange && userLoginMigration && !userLoginMigration.closedAt) {
-				const hasMigrated: boolean = user.lastLoginSystemChange > userLoginMigration.startedAt;
-				return !hasMigrated;
-			}
-			return !!userLoginMigration && !userLoginMigration.closedAt;
+		if (this.isUserMigrated(user, userLoginMigration)) {
+			return false;
 		}
-		return false;
+
+		return true;
+	}
+
+	private isMigrationDataValid(user: UserDO | null, userLoginMigration: UserLoginMigrationDO): boolean {
+		return !!user?.lastLoginSystemChange && !userLoginMigration.closedAt;
+	}
+
+	private isUserMigrated(user: UserDO | null, userLoginMigration: UserLoginMigrationDO): boolean {
+		return (
+			!!user && user.lastLoginSystemChange !== undefined && user.lastLoginSystemChange > userLoginMigration.startedAt
+		);
+	}
+
+	private isMigrationActive(userLoginMigration: UserLoginMigrationDO): boolean {
+		return !userLoginMigration.closedAt;
 	}
 }

apps/server/src/modules/user-login-migration/service/school-migration.service.spec.ts

@@ -303,8 +303,8 @@ describe(SchoolMigrationService.name, () => {
 				const closedAt: Date = new Date('2023-05-01');
 
 				const userLoginMigration: UserLoginMigrationDO = userLoginMigrationDOFactory.buildWithId({
-					schoolId: 'schoolId',
-					targetSystemId: 'targetSystemId',
+					schoolId: new ObjectId().toHexString(),
+					targetSystemId: new ObjectId().toHexString(),
 					startedAt: new Date('2023-05-01'),
 					closedAt,
 					finishedAt: new Date('2023-05-01'),
@@ -379,7 +379,7 @@ describe(SchoolMigrationService.name, () => {
 				};
 			};
 
-			it('should call userLoginMigrationRepo.findBySchoolId', async () => {
+			it('should find user login migration by school id', async () => {
 				setup();
 
 				await service.hasSchoolMigratedUser('schoolId');
@@ -399,7 +399,7 @@ describe(SchoolMigrationService.name, () => {
 				};
 			};
 
-			it('should call userService.findUsers', async () => {
+			it('should call user service to find users', async () => {
 				const { userLoginMigration } = setup();
 
 				await service.hasSchoolMigratedUser('schoolId');

apps/server/src/modules/user-login-migration/service/school-migration.service.ts

@@ -20,7 +20,11 @@ export class SchoolMigrationService {
 		private readonly userLoginMigrationRepo: UserLoginMigrationRepo
 	) {}
 
-	async migrateSchool(existingSchool: LegacySchoolDo, externalId: string, targetSystemId: string): Promise<void> {
+	public async migrateSchool(
+		existingSchool: LegacySchoolDo,
+		externalId: string,
+		targetSystemId: string
+	): Promise<void> {
 		const schoolDOCopy: LegacySchoolDo = new LegacySchoolDo({ ...existingSchool });
 
 		try {
@@ -45,7 +49,7 @@ export class SchoolMigrationService {
 		await this.schoolService.save(school);
 	}
 
-	private async tryRollbackMigration(originalSchoolDO: LegacySchoolDo) {
+	private async tryRollbackMigration(originalSchoolDO: LegacySchoolDo): Promise<void> {
 		try {
 			await this.schoolService.save(originalSchoolDO);
 		} catch (error: unknown) {
@@ -55,7 +59,7 @@ export class SchoolMigrationService {
 		}
 	}
 
-	async getSchoolForMigration(
+	public async getSchoolForMigration(
 		userId: string,
 		externalId: string,
 		officialSchoolNumber: string
@@ -89,7 +93,7 @@ export class SchoolMigrationService {
 		return isExternalIdEquivalent;
 	}
 
-	async markUnmigratedUsersAsOutdated(userLoginMigration: UserLoginMigrationDO): Promise<void> {
+	public async markUnmigratedUsersAsOutdated(userLoginMigration: UserLoginMigrationDO): Promise<void> {
 		const startTime: number = performance.now();
 
 		const notMigratedUsers: Page<UserDO> = await this.userService.findUsers({
@@ -112,7 +116,7 @@ export class SchoolMigrationService {
 		);
 	}
 
-	async unmarkOutdatedUsers(userLoginMigration: UserLoginMigrationDO): Promise<void> {
+	public async unmarkOutdatedUsers(userLoginMigration: UserLoginMigrationDO): Promise<void> {
 		const startTime: number = performance.now();
 
 		const migratedUsers: Page<UserDO> = await this.userService.findUsers({
@@ -132,7 +136,7 @@ export class SchoolMigrationService {
 		);
 	}
 
-	async hasSchoolMigratedUser(schoolId: string): Promise<boolean> {
+	public async hasSchoolMigratedUser(schoolId: string): Promise<boolean> {
 		const userLoginMigration: UserLoginMigrationDO | null = await this.userLoginMigrationRepo.findBySchoolId(schoolId);
 
 		if (!userLoginMigration) {