N21-1888- check email in provisioning (#4955)

- check email in provisioning
- log warning

apps/server/src/modules/provisioning/loggable/email-already-exists.loggable.spec.ts

@@ -0,0 +1,32 @@
+import { EmailAlreadyExistsLoggable } from '@modules/provisioning/loggable/email-already-exists.loggable';
+
+describe('EmailAlreadyExistsLoggableException', () => {
+	describe('getLogMessage', () => {
+		const setup = () => {
+			const email = 'mock-email';
+			const externalId = '789';
+
+			const loggable = new EmailAlreadyExistsLoggable(email, externalId);
+
+			return {
+				loggable,
+				email,
+				externalId,
+			};
+		};
+
+		it('should return the correct log message', () => {
+			const { loggable, email, externalId } = setup();
+
+			const message = loggable.getLogMessage();
+
+			expect(message).toEqual({
+				message: 'The Email to be provisioned already exists.',
+				data: {
+					email,
+					externalId,
+				},
+			});
+		});
+	});
+});

apps/server/src/modules/provisioning/loggable/email-already-exists.loggable.ts

@@ -0,0 +1,15 @@
+import { ErrorLogMessage, Loggable, LogMessage, ValidationErrorLogMessage } from '@src/core/logger';
+
+export class EmailAlreadyExistsLoggable implements Loggable {
+	constructor(private readonly email: string, private readonly externalId?: string) {}
+
+	getLogMessage(): LogMessage | ErrorLogMessage | ValidationErrorLogMessage {
+		return {
+			message: 'The Email to be provisioned already exists.',
+			data: {
+				email: this.email,
+				externalId: this.externalId,
+			},
+		};
+	}
+}

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

@@ -1,3 +1,4 @@
 export * from './user-for-group-not-found.loggable';
 export * from './school-for-group-not-found.loggable';
 export * from './group-role-unknown.loggable';
+export { EmailAlreadyExistsLoggable } from './email-already-exists.loggable';

apps/server/src/modules/provisioning/strategy/oidc/service/schulconnex-user-provisioning.service.spec.ts

@@ -1,5 +1,5 @@
 import { createMock, DeepMocked } from '@golevelup/ts-jest';
-import { AccountService, AccountSave } from '@modules/account';
+import { AccountSave, AccountService } from '@modules/account';
 import { RoleService } from '@modules/role';
 import { RoleDto } from '@modules/role/service/dto/role.dto';
 import { UserService } from '@modules/user';
@@ -8,6 +8,7 @@ import { Test, TestingModule } from '@nestjs/testing';
 import { UserDO } from '@shared/domain/domainobject';
 import { RoleName } from '@shared/domain/interface';
 import { userDoFactory } from '@shared/testing';
+import { Logger } from '@src/core/logger';
 import CryptoJS from 'crypto-js';
 import { ExternalUserDto } from '../../../dto';
 import { SchulconnexUserProvisioningService } from './schulconnex-user-provisioning.service';
@@ -21,6 +22,7 @@ describe(SchulconnexUserProvisioningService.name, () => {
 	let userService: DeepMocked<UserService>;
 	let roleService: DeepMocked<RoleService>;
 	let accountService: DeepMocked<AccountService>;
+	let logger: DeepMocked<Logger>;
 
 	beforeAll(async () => {
 		module = await Test.createTestingModule({
@@ -38,13 +40,18 @@ describe(SchulconnexUserProvisioningService.name, () => {
 					provide: AccountService,
 					useValue: createMock<AccountService>(),
 				},
+				{
+					provide: Logger,
+					useValue: createMock<Logger>(),
+				},
 			],
 		}).compile();
 
 		service = module.get(SchulconnexUserProvisioningService);
 		userService = module.get(UserService);
 		roleService = module.get(RoleService);
 		accountService = module.get(AccountService);
+		logger = module.get(Logger);
 	});
 
 	afterAll(async () => {
@@ -90,6 +97,7 @@ describe(SchulconnexUserProvisioningService.name, () => {
 				roles: [RoleName.USER],
 				birthday,
 			});
+			const minimalViableExternalUser: ExternalUserDto = new ExternalUserDto({ externalId: 'externalUserId' });
 			const userRole: RoleDto = new RoleDto({
 				id: 'roleId',
 				name: RoleName.USER,
@@ -111,6 +119,7 @@ describe(SchulconnexUserProvisioningService.name, () => {
 				existingUser,
 				savedUser,
 				externalUser,
+				minimalViableExternalUser,
 				userRole,
 				schoolId,
 				systemId,
@@ -119,10 +128,33 @@ describe(SchulconnexUserProvisioningService.name, () => {
 		};
 
 		describe('when the user does not exist yet', () => {
+			describe('when the external user has no email or roles', () => {
+				it('should return the saved user', async () => {
+					const { minimalViableExternalUser, schoolId, savedUser, systemId } = setupUser();
+
+					userService.findByExternalId.mockResolvedValue(null);
+
+					const result: UserDO = await service.provisionExternalUser(minimalViableExternalUser, systemId, schoolId);
+
+					expect(result).toEqual(savedUser);
+				});
+			});
+
+			it('should call user service to check uniqueness of email', async () => {
+				const { externalUser, schoolId, systemId } = setupUser();
+
+				userService.findByExternalId.mockResolvedValue(null);
+
+				await service.provisionExternalUser(externalUser, systemId, schoolId);
+
+				expect(userService.isEmailUniqueForExternal).toHaveBeenCalledWith(externalUser.email, undefined);
+			});
+
 			it('should call the user service to save the user', async () => {
 				const { externalUser, schoolId, savedUser, systemId } = setupUser();
 
 				userService.findByExternalId.mockResolvedValue(null);
+				userService.isEmailUniqueForExternal.mockResolvedValue(true);
 
 				await service.provisionExternalUser(externalUser, systemId, schoolId);
 
@@ -166,9 +198,35 @@ describe(SchulconnexUserProvisioningService.name, () => {
 					await expect(promise).rejects.toThrow(UnprocessableEntityException);
 				});
 			});
+
+			describe('when the external user has an email, that already exists', () => {
+				it('should log EmailAlreadyExistsLoggable', async () => {
+					const { externalUser, systemId, schoolId } = setupUser();
+
+					userService.findByExternalId.mockResolvedValue(null);
+					userService.isEmailUniqueForExternal.mockResolvedValue(false);
+
+					await service.provisionExternalUser(externalUser, systemId, schoolId);
+
+					expect(logger.warning).toHaveBeenCalledWith({
+						email: externalUser.email,
+					});
+				});
+			});
 		});
 
 		describe('when the user already exists', () => {
+			it('should call user service to check uniqueness of email', async () => {
+				const { externalUser, schoolId, systemId, existingUser } = setupUser();
+
+				userService.findByExternalId.mockResolvedValue(existingUser);
+				userService.isEmailUniqueForExternal.mockResolvedValue(true);
+
+				await service.provisionExternalUser(externalUser, systemId, schoolId);
+
+				expect(userService.isEmailUniqueForExternal).toHaveBeenCalledWith(externalUser.email, existingUser.externalId);
+			});
+
 			it('should call the user service to save the user', async () => {
 				const { externalUser, schoolId, existingUser, systemId } = setupUser();
 

apps/server/src/modules/provisioning/strategy/oidc/service/schulconnex-user-provisioning.service.ts

@@ -1,9 +1,12 @@
-import { AccountService, AccountSave } from '@modules/account';
+import { AccountSave, AccountService } from '@modules/account';
+import { EmailAlreadyExistsLoggable } from '@modules/provisioning/loggable';
 import { RoleDto, RoleService } from '@modules/role';
 import { UserService } from '@modules/user';
 import { Injectable, UnprocessableEntityException } from '@nestjs/common';
 import { RoleReference, UserDO } from '@shared/domain/domainobject';
+import { RoleName } from '@shared/domain/interface';
 import { EntityId } from '@shared/domain/types';
+import { Logger } from '@src/core/logger';
 import CryptoJS from 'crypto-js';
 import { ExternalUserDto } from '../../../dto';
 
@@ -12,49 +15,34 @@ export class SchulconnexUserProvisioningService {
 	constructor(
 		private readonly userService: UserService,
 		private readonly roleService: RoleService,
-		private readonly accountService: AccountService
+		private readonly accountService: AccountService,
+		private readonly logger: Logger
 	) {}
 
 	public async provisionExternalUser(
 		externalUser: ExternalUserDto,
 		systemId: EntityId,
 		schoolId?: string
 	): Promise<UserDO> {
-		let roleRefs: RoleReference[] | undefined;
-		if (externalUser.roles) {
-			const roles: RoleDto[] = await this.roleService.findByNames(externalUser.roles);
-			roleRefs = roles.map((role: RoleDto): RoleReference => new RoleReference({ id: role.id || '', name: role.name }));
-		}
+		const foundUser: UserDO | null = await this.userService.findByExternalId(externalUser.externalId, systemId);
+
+		const isEmailUnique: boolean = await this.checkUniqueEmail(externalUser.email, foundUser?.externalId);
+
+		const roleRefs: RoleReference[] | undefined = await this.createRoleReferences(externalUser.roles);
 
-		const existingUser: UserDO | null = await this.userService.findByExternalId(externalUser.externalId, systemId);
-		let user: UserDO;
 		let createNewAccount = false;
-		if (existingUser) {
-			user = existingUser;
-			user.firstName = externalUser.firstName ?? existingUser.firstName;
-			user.lastName = externalUser.lastName ?? existingUser.lastName;
-			user.email = externalUser.email ?? existingUser.email;
-			user.roles = roleRefs ?? existingUser.roles;
-			user.schoolId = schoolId ?? existingUser.schoolId;
-			user.birthday = externalUser.birthday ?? existingUser.birthday;
+		let user: UserDO;
+		if (foundUser) {
+			user = this.updateUser(externalUser, foundUser, isEmailUnique, roleRefs, schoolId);
 		} else {
-			createNewAccount = true;
-
 			if (!schoolId) {
 				throw new UnprocessableEntityException(
 					`Unable to create new external user ${externalUser.externalId} without a school`
 				);
 			}
 
-			user = new UserDO({
-				externalId: externalUser.externalId,
-				firstName: externalUser.firstName ?? '',
-				lastName: externalUser.lastName ?? '',
-				email: externalUser.email ?? '',
-				roles: roleRefs ?? [],
-				schoolId,
-				birthday: externalUser.birthday,
-			});
+			createNewAccount = true;
+			user = this.createUser(externalUser, isEmailUnique, schoolId, roleRefs);
 		}
 
 		const savedUser: UserDO = await this.userService.save(user);
@@ -70,4 +58,68 @@ export class SchulconnexUserProvisioningService {
 
 		return savedUser;
 	}
+
+	private async checkUniqueEmail(email?: string, externalId?: string): Promise<boolean> {
+		if (email) {
+			const isEmailUnique: boolean = await this.userService.isEmailUniqueForExternal(email, externalId);
+
+			if (!isEmailUnique) {
+				this.logger.warning(new EmailAlreadyExistsLoggable(email, externalId));
+			}
+
+			return isEmailUnique;
+		}
+
+		return true;
+	}
+
+	private async createRoleReferences(roles?: RoleName[]): Promise<RoleReference[] | undefined> {
+		if (roles) {
+			const foundRoles: RoleDto[] = await this.roleService.findByNames(roles);
+			const roleRefs = foundRoles.map(
+				(role: RoleDto): RoleReference => new RoleReference({ id: role.id || '', name: role.name })
+			);
+
+			return roleRefs;
+		}
+
+		return undefined;
+	}
+
+	private updateUser(
+		externalUser: ExternalUserDto,
+		foundUser: UserDO,
+		isEmailUnique: boolean,
+		roleRefs?: RoleReference[],
+		schoolId?: string
+	): UserDO {
+		const user: UserDO = foundUser;
+		user.firstName = externalUser.firstName ?? foundUser.firstName;
+		user.lastName = externalUser.lastName ?? foundUser.lastName;
+		user.email = isEmailUnique ? externalUser.email ?? foundUser.email : foundUser.email;
+		user.roles = roleRefs ?? foundUser.roles;
+		user.schoolId = schoolId ?? foundUser.schoolId;
+		user.birthday = externalUser.birthday ?? foundUser.birthday;
+
+		return user;
+	}
+
+	private createUser(
+		externalUser: ExternalUserDto,
+		isEmailUnique: boolean,
+		schoolId: string,
+		roleRefs?: RoleReference[]
+	): UserDO {
+		const user: UserDO = new UserDO({
+			externalId: externalUser.externalId,
+			firstName: externalUser.firstName ?? '',
+			lastName: externalUser.lastName ?? '',
+			email: isEmailUnique ? externalUser.email ?? '' : '',
+			roles: roleRefs ?? [],
+			schoolId,
+			birthday: externalUser.birthday,
+		});
+
+		return user;
+	}
 }