N21-2057 Team invitations with duplicate emails

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

@@ -1,5 +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';
 export { SchoolExternalToolCreatedLoggable } from './school-external-tool-created.loggable';

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

@@ -22,7 +22,6 @@ 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({
@@ -51,7 +50,6 @@ describe(SchulconnexUserProvisioningService.name, () => {
 		userService = module.get(UserService);
 		roleService = module.get(RoleService);
 		accountService = module.get(AccountService);
-		logger = module.get(Logger);
 	});
 
 	afterAll(async () => {
@@ -140,27 +138,6 @@ describe(SchulconnexUserProvisioningService.name, () => {
 				});
 			});
 
-			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);
-
-				expect(userService.save).toHaveBeenCalledWith(new UserDO({ ...savedUser, id: undefined }));
-			});
-
 			it('should return the saved user', async () => {
 				const { externalUser, schoolId, savedUser, systemId } = setupUser();
 
@@ -198,35 +175,9 @@ 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,12 +1,10 @@
 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';
 
@@ -15,8 +13,7 @@ export class SchulconnexUserProvisioningService {
 	constructor(
 		private readonly userService: UserService,
 		private readonly roleService: RoleService,
-		private readonly accountService: AccountService,
-		private readonly logger: Logger
+		private readonly accountService: AccountService
 	) {}
 
 	public async provisionExternalUser(
@@ -26,14 +23,12 @@ export class SchulconnexUserProvisioningService {
 	): Promise<UserDO> {
 		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);
 
 		let createNewAccount = false;
 		let user: UserDO;
 		if (foundUser) {
-			user = this.updateUser(externalUser, foundUser, isEmailUnique, roleRefs, schoolId);
+			user = this.updateUser(externalUser, foundUser, roleRefs, schoolId);
 		} else {
 			if (!schoolId) {
 				throw new UnprocessableEntityException(
@@ -42,7 +37,7 @@ export class SchulconnexUserProvisioningService {
 			}
 
 			createNewAccount = true;
-			user = this.createUser(externalUser, isEmailUnique, schoolId, roleRefs);
+			user = this.createUser(externalUser, schoolId, roleRefs);
 		}
 
 		const savedUser: UserDO = await this.userService.save(user);
@@ -59,20 +54,6 @@ 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);
@@ -89,32 +70,26 @@ export class SchulconnexUserProvisioningService {
 	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.email = externalUser.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 {
+	private createUser(externalUser: ExternalUserDto, schoolId: string, roleRefs?: RoleReference[]): UserDO {
 		const user: UserDO = new UserDO({
 			externalId: externalUser.externalId,
 			firstName: externalUser.firstName ?? '',
 			lastName: externalUser.lastName ?? '',
-			email: isEmailUnique ? externalUser.email ?? '' : '',
+			email: externalUser.email ?? '',
 			roles: roleRefs ?? [],
 			schoolId,
 			birthday: externalUser.birthday,

apps/server/src/modules/user/service/user.service.spec.ts

@@ -946,119 +946,4 @@ describe('UserService', () => {
 			});
 		});
 	});
-
-	describe('isEmailUniqueForExternal', () => {
-		describe('when email does not exist', () => {
-			const setup = () => {
-				const email = 'email';
-
-				userDORepo.findByEmail.mockResolvedValue([]);
-
-				return {
-					email,
-				};
-			};
-
-			it('should return true', async () => {
-				const { email } = setup();
-
-				const result: boolean = await service.isEmailUniqueForExternal(email, undefined);
-
-				expect(result).toBe(true);
-			});
-		});
-
-		describe('when an existing user is found', () => {
-			describe('when existing user is the external user', () => {
-				const setup = () => {
-					const email = 'email';
-					const externalId = 'externalId';
-					const existingUser: UserDO = userDoFactory.build({ email, externalId });
-
-					userDORepo.findByEmail.mockResolvedValue([existingUser]);
-
-					return {
-						email,
-						externalId,
-					};
-				};
-
-				it('should return true', async () => {
-					const { email, externalId } = setup();
-
-					const result: boolean = await service.isEmailUniqueForExternal(email, externalId);
-
-					expect(result).toBe(true);
-				});
-			});
-
-			describe('when existing user is not the external user', () => {
-				const setup = () => {
-					const email = 'email';
-					const externalId = 'externalId';
-					const otherUserWithSameEmail: UserDO = userDoFactory.build({ email });
-
-					userDORepo.findByEmail.mockResolvedValue([otherUserWithSameEmail]);
-
-					return {
-						email,
-						externalId,
-					};
-				};
-
-				it('should return false', async () => {
-					const { email, externalId } = setup();
-
-					const result: boolean = await service.isEmailUniqueForExternal(email, externalId);
-
-					expect(result).toBe(false);
-				});
-			});
-
-			describe('when existing user is not the external user and external user is not already provisioned.', () => {
-				const setup = () => {
-					const email = 'email';
-					const otherUserWithSameEmail: UserDO = userDoFactory.build({ email });
-
-					userDORepo.findByEmail.mockResolvedValue([otherUserWithSameEmail]);
-
-					return {
-						email,
-					};
-				};
-
-				it('should return false', async () => {
-					const { email } = setup();
-
-					const result: boolean = await service.isEmailUniqueForExternal(email, undefined);
-
-					expect(result).toBe(false);
-				});
-			});
-		});
-
-		describe('when multiple users are found', () => {
-			const setup = () => {
-				const email = 'email';
-				const externalId = 'externalId';
-				const existingUser: UserDO = userDoFactory.build({ email, externalId });
-				const otherUserWithSameEmail: UserDO = userDoFactory.build({ email });
-
-				userDORepo.findByEmail.mockResolvedValue([existingUser, otherUserWithSameEmail]);
-
-				return {
-					email,
-					externalId,
-				};
-			};
-
-			it('should return false', async () => {
-				const { email, externalId } = setup();
-
-				const result: boolean = await service.isEmailUniqueForExternal(email, externalId);
-
-				expect(result).toBe(false);
-			});
-		});
-	});
 });

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

@@ -283,18 +283,4 @@ export class UserService implements DeletionService, IEventHandler<UserDeletedEv
 
 		return DomainDeletionReportBuilder.build(DomainName.CALENDAR, extractedOperationReport);
 	}
-
-	public async isEmailUniqueForExternal(email: string, externalId?: string): Promise<boolean> {
-		const foundUsers: UserDO[] = await this.findByEmail(email);
-		if (!externalId && foundUsers.length) {
-			return false;
-		}
-
-		const unmatchedUsers: UserDO[] = foundUsers.filter((user: UserDO) => user.externalId !== externalId);
-		if (unmatchedUsers.length) {
-			return false;
-		}
-
-		return true;
-	}
 }

apps/server/src/shared/domain/entity/user.entity.ts

@@ -51,7 +51,6 @@ interface UserInfo {
 export class User extends BaseEntityWithTimestamps implements EntityWithSchool {
 	@Property()
 	@Index()
-	// @Unique()
 	email: string;
 
 	@Property()

src/services/user/hooks/publicTeachers.js

@@ -26,7 +26,7 @@ const mapRoleFilterQuery = (hook) => {
 
 const filterForPublicTeacher = (hook) => {
 	// Limit accessible fields
-	hook.params.query.$select = ['_id', 'firstName', 'lastName'];
+	hook.params.query.$select = ['_id', 'firstName', 'lastName', 'schoolId'];
 
 	// Limit accessible user (only teacher which are discoverable)
 	hook.params.query.roles = ['teacher'];