fix: user name in ad synchronization (#1621)

Author: joaofrparreira

backend/src/libs/test-utils/mocks/factories/azure-user-factory.ts

@@ -1,25 +1,27 @@
 import { faker } from '@faker-js/faker';
 import { buildTestFactory } from './generic-factory.mock';
-import { AzureUserDTO } from 'src/modules/azure/dto/azure-user.dto';
+import { AzureUserSyncDTO } from 'src/modules/azure/dto/azure-user.dto';
 
-const mockUserData = (): AzureUserDTO => {
+const mockUserData = (): AzureUserSyncDTO => {
 	//xGeeks AD style, the '.' is mandatory for some tests
-	const firstName = faker.person.firstName();
-	const lastName = faker.person.lastName();
-	const mail = firstName[0].toLowerCase() + '.' + lastName.toLowerCase() + '@xgeeks.com';
+	const firstName = `${faker.person.firstName()} ${faker.person.middleName()}`;
+	const lastName = `${faker.person.middleName()} ${faker.person.lastName()}`;
+	const mail = `${firstName[0].toLowerCase()}.${lastName.split(' ').at(-1).toLowerCase()}@xgeeks.com`;
 
 	return {
 		id: faker.string.uuid(),
-		displayName: firstName + ' ' + lastName,
+		displayName: firstName.split(' ')[0] + ' ' + lastName.split(' ').at(-1),
 		mail: mail,
 		userPrincipalName: mail,
 		createdDateTime: faker.date.past({ years: 5 }),
 		accountEnabled: faker.datatype.boolean(),
 		deletedDateTime: faker.datatype.boolean() ? faker.date.recent({ days: 1 }) : null,
-		employeeLeaveDateTime: faker.datatype.boolean() ? faker.date.recent({ days: 1 }) : null
+		employeeLeaveDateTime: faker.datatype.boolean() ? faker.date.recent({ days: 1 }) : null,
+		givenName: firstName,
+		surName: lastName
 	};
 };
 
-export const AzureUserFactory = buildTestFactory<AzureUserDTO>(() => {
+export const AzureUserFactory = buildTestFactory<AzureUserSyncDTO>(() => {
 	return mockUserData();
 });

backend/src/modules/azure/dto/azure-user.dto.ts

@@ -8,3 +8,8 @@ export type AzureUserDTO = {
 	deletedDateTime: Date | null;
 	employeeLeaveDateTime: Date | null;
 };
+
+export type AzureUserSyncDTO = AzureUserDTO & {
+	givenName: string;
+	surName: string;
+};

backend/src/modules/azure/interfaces/services/auth.azure.service.interface.ts

@@ -1,7 +1,7 @@
-import { AzureUserDTO } from '../../dto/azure-user.dto';
+import { AzureUserDTO, AzureUserSyncDTO } from '../../dto/azure-user.dto';
 
 export interface AuthAzureServiceInterface {
 	getUserFromAzure(email: string): Promise<AzureUserDTO | undefined>;
 	fetchUserPhoto(userId: string): Promise<any>;
-	getADUsers(): Promise<Array<AzureUserDTO>>;
+	getADUsers(): Promise<Array<AzureUserSyncDTO>>;
 }

backend/src/modules/azure/schedules/synchronize-ad-users.cron.azure.use-case.spec.ts

@@ -116,13 +116,22 @@ describe('SynchronizeAdUsersCronUseCase', () => {
 			employeeLeaveDateTime: null,
 			deletedDateTime: null
 		});
+		const userWithInvalidName = AzureUserFactory.create({
+			employeeLeaveDateTime: null,
+			deletedDateTime: null
+		});
+
+		//Invalidate last name - represents a user only with first name
+		userWithInvalidName.surName = '';
+		userWithInvalidName.displayName = userWithInvalidName.displayName.split(' ')[0];
+
 		const azureUserDto: CreateUserAzureDto = {
 			email: userNotInApp.mail,
 			firstName: userNotInApp.displayName.split(' ')[0],
-			lastName: userNotInApp.displayName.split(' ')[-1],
+			lastName: userNotInApp.displayName.split(' ').at(-1),
 			providerAccountCreatedAt: userNotInApp.createdDateTime
 		};
-		const finalADUsers = [userNotInApp, ...usersAD];
+		const finalADUsers = [userNotInApp, userWithInvalidName, ...usersAD];
 		const userNotInAD = UserFactory.create({
 			isDeleted: false,
 			email: '[email protected]'
@@ -149,7 +158,7 @@ describe('SynchronizeAdUsersCronUseCase', () => {
 		await synchronizeADUsers.execute();
 		expect(getTeamByNameUseCase.execute).toHaveBeenCalledWith('xgeeks');
 		expect(deleteUserMock.execute).toHaveBeenCalledWith(userNotInAD._id);
-		expect(createUserServiceMock.createMany).toHaveBeenCalledWith([azureUserDto]);
+		expect(createUserServiceMock.createMany).toHaveBeenCalledWith([azureUserDto]); //userWithInvalidName isn't called here because of the name rules
 		expect(addAndRemoveTeamUserUseCase.execute).toHaveBeenCalledWith(updateUsersTeam);
 	});
 });

backend/src/modules/azure/schedules/synchronize-ad-users.cron.azure.use-case.ts

@@ -10,7 +10,7 @@ import {
 } from 'src/modules/users/constants';
 import { UseCase } from 'src/libs/interfaces/use-case.interface';
 import User from 'src/modules/users/entities/user.schema';
-import { AzureUserDTO } from '../dto/azure-user.dto';
+import { AzureUserDTO, AzureUserSyncDTO } from '../dto/azure-user.dto';
 import { CreateUserServiceInterface } from 'src/modules/users/interfaces/services/create.user.service.interface';
 import { GET_TEAM_BY_NAME_USE_CASE } from 'src/modules/teams/constants';
 import Team from 'src/modules/teams/entities/team.schema';
@@ -61,27 +61,26 @@ export class SynchronizeADUsersCronUseCase implements SynchronizeADUsersCronUseC
 				throw new Error('Azure AD users list is empty.');
 			}
 
-			const usersApp = await this.getAllUsersIncludeDeletedUseCase.execute();
-
-			if (!usersApp.length) {
-				throw new Error('Split app users list is empty.');
-			}
-
-			let usersAppFiltered = usersApp;
+			let usersApp = await this.getAllUsersIncludeDeletedUseCase.execute();
 
 			if (userEmailDomain) {
 				const emailDomain = '@' + userEmailDomain;
-				usersAppFiltered = usersAppFiltered.filter(
-					(user) => user.email && user.email.endsWith(emailDomain)
-				);
+				usersApp = usersApp.filter((user) => user.email && user.email.endsWith(emailDomain));
 			}
 
 			const today = new Date();
+
 			//Filter out users that don't have a '.' in the beggining of the email
 			let usersADFiltered = usersADAll.filter((u) =>
 				/[a-z]+\.[a-zA-Z0-9]+@/.test(u.userPrincipalName)
 			);
 
+			//Filter out users that don't have at least first and last name
+			usersADFiltered = usersADFiltered.filter(
+				(u: AzureUserSyncDTO) =>
+					!(u.displayName.split(' ').length < 2 && (!u.givenName || !u.surName))
+			);
+
 			//Filter out users that have a deletedDateTime bigger than 'today'
 			usersADFiltered = usersADFiltered.filter((u) =>
 				'deletedDateTime' in u ? u.deletedDateTime === null || u.deletedDateTime >= today : true
@@ -94,8 +93,8 @@ export class SynchronizeADUsersCronUseCase implements SynchronizeADUsersCronUseC
 					: true
 			);
 
-			await this.removeUsersFromApp(usersADFiltered, usersAppFiltered);
-			await this.addUsersToApp(usersADFiltered, usersAppFiltered, team);
+			await this.removeUsersFromApp(usersADFiltered, usersApp);
+			await this.addUsersToApp(usersADFiltered, usersApp, team);
 
 			this.logger.log('Synchronization of users between App and AD runned successfully.');
 		} catch (err) {
@@ -125,7 +124,7 @@ export class SynchronizeADUsersCronUseCase implements SynchronizeADUsersCronUseC
 		}
 	}
 	private async addUsersToApp(
-		usersADFiltered: Array<AzureUserDTO>,
+		usersADFiltered: Array<AzureUserSyncDTO>,
 		usersApp: Array<User>,
 		team: Team
 	) {
@@ -137,16 +136,20 @@ export class SynchronizeADUsersCronUseCase implements SynchronizeADUsersCronUseC
 		);
 
 		try {
-			const usersToCreate: Array<CreateUserAzureDto> = notIntersectedUsers.map((user) => {
-				const splittedName = user.displayName.split(' ');
-
-				return {
-					email: user.mail ?? user.userPrincipalName,
-					firstName: splittedName[0],
-					lastName: splittedName[-1],
-					providerAccountCreatedAt: user.createdDateTime
-				};
-			});
+			const usersToCreate: Array<CreateUserAzureDto> = notIntersectedUsers
+				.map((user) => {
+					const splittedName = user.displayName.split(' ');
+					const firstName = user.givenName?.split(' ')[0] ?? splittedName[0];
+					const lastName = user.surName?.split(' ').at(-1) ?? splittedName.at(-1);
+
+					return {
+						email: user.mail ?? user.userPrincipalName,
+						firstName,
+						lastName,
+						providerAccountCreatedAt: user.createdDateTime
+					};
+				})
+				.filter((u) => u.firstName && u.lastName);
 
 			const createdUsers = await this.createUserService.createMany(usersToCreate);
 

backend/src/modules/azure/services/auth.azure.service.ts

@@ -4,7 +4,7 @@ import { ConfidentialClientApplication } from '@azure/msal-node';
 import { Client, PageCollection } from '@microsoft/microsoft-graph-client';
 import { ConfigService } from '@nestjs/config';
 import { AZURE_AUTHORITY, AZURE_CLIENT_ID, AZURE_CLIENT_SECRET } from 'src/libs/constants/azure';
-import { AzureUserDTO } from '../dto/azure-user.dto';
+import { AzureUserDTO, AzureUserSyncDTO } from '../dto/azure-user.dto';
 
 export type AzureDecodedUser = {
 	unique_name: string;
@@ -71,7 +71,7 @@ export default class AuthAzureService implements AuthAzureServiceInterface {
 		return this.graphClient.api(`/users/${userId}/photos/240x240/$value`).get();
 	}
 
-	async getADUsers(): Promise<Array<AzureUserDTO>> {
+	async getADUsers(): Promise<Array<AzureUserSyncDTO>> {
 		let response: PageCollection = await this.graphClient
 			.api('/users')
 			.header('ConsistencyLevel', 'eventual')
@@ -85,7 +85,9 @@ export default class AuthAzureService implements AuthAzureServiceInterface {
 				'createdDateTime',
 				'accountEnabled',
 				'deletedDateTime',
-				'employeeLeaveDateTime'
+				'employeeLeaveDateTime',
+				'givenName',
+				'surname'
 			])
 			.get();
 

backend/src/modules/users/utils/sortings.ts

@@ -3,7 +3,7 @@ import User from '../entities/user.schema';
 
 export const sortAlphabetically = (a: User, b: User) => {
 	if (a.firstName.toLowerCase() === b.firstName.toLowerCase()) {
-		return a.lastName.toLowerCase() < b.lastName.toLowerCase() ? -1 : 1;
+		return a.lastName?.toLowerCase() < b.lastName?.toLowerCase() ? -1 : 1;
 	}
 
 	return a.firstName.toLowerCase() < b.firstName.toLowerCase() ? -1 : 1;