Fixed / added unit tests for keycloak role sync changes

express-api/src/controllers/admin/users/usersController.ts

@@ -166,8 +166,12 @@ export const getAllRoles = async (req: Request, res: Response) => {
             "bearerAuth": []
       }]
    */
-  const roles = await userServices.getKeycloakRoles();
-  return res.status(200).send(roles);
+  try {
+    const roles = await userServices.getKeycloakRoles();
+    return res.status(200).send(roles);
+  } catch (e) {
+    return res.status(500).send('Something went wrong accessing the keycloak service.');
+  }
 };
 
 /**

express-api/src/services/keycloak/keycloakService.ts

@@ -80,23 +80,23 @@ const syncKeycloakRoles = async () => {
       };
       await rolesServices.updateRole(overwriteRole);
     }
+  }
+  //This deletion section is somewhat clunky. Could consider delete cascade on the schema to avoid some of this.
+  const internalRolesForDeletion = await AppDataSource.getRepository(Role).findBy({
+    Name: Not(In(roles.map((a) => a.name))),
+  });
 
-    //This deletion section is somewhat clunky. Could consider delete cascade on the schema to avoid some of this.
-    const internalRolesForDeletion = await AppDataSource.getRepository(Role).findBy({
-      Name: Not(In(roles.map((a) => a.name))),
+  if (internalRolesForDeletion.length) {
+    const roleIdsForDeletion = internalRolesForDeletion.map((role) => role.Id);
+    await AppDataSource.getRepository(User)
+      .createQueryBuilder()
+      .update(User)
+      .set({ RoleId: null })
+      .where('RoleId IN (:...ids)', { ids: roleIdsForDeletion })
+      .execute();
+    await AppDataSource.getRepository(Role).delete({
+      Id: In(roleIdsForDeletion),
     });
-    if (internalRolesForDeletion.length) {
-      const roleIdsForDeletion = internalRolesForDeletion.map((role) => role.Id);
-      await AppDataSource.getRepository(User)
-        .createQueryBuilder()
-        .update(User)
-        .set({ RoleId: null })
-        .where('RoleId IN (:...ids)', { ids: roleIdsForDeletion })
-        .execute();
-      await AppDataSource.getRepository(Role).delete({
-        Id: In(roleIdsForDeletion),
-      });
-    }
   }
   return roles;
 };

express-api/tests/unit/controllers/admin/roles/rolesController.test.ts

@@ -27,6 +27,12 @@ jest.mock('@/services/admin/rolesServices', () => ({
   removeRole: (role: RolesEntity) => _deleteRole(role),
 }));
 
+const _syncKeycloakRoles = jest.fn().mockImplementation(() => {});
+
+jest.mock('@/services/keycloak/keycloakService', () => ({
+  syncKeycloakRoles: () => _syncKeycloakRoles(),
+}));
+
 describe('UNIT - Roles Admin', () => {
   beforeEach(() => {
     const { mockReq, mockRes } = getRequestHandlerMocks();
@@ -43,7 +49,7 @@ describe('UNIT - Roles Admin', () => {
     });
 
     it('should return status 200 and a filtered roles', async () => {
-      mockRequest.body.filter = { name: 'big name' };
+      mockRequest.query = { name: 'big name' };
       const role = produceRole();
       role.Name = 'big name';
       await getRoles(mockRequest, mockResponse);

express-api/tests/unit/controllers/admin/users/usersController.test.ts

@@ -243,23 +243,18 @@ describe('UNIT - Users Admin', () => {
   });
 
   describe('Controller getAllRoles', () => {
-    const user = produceUser();
-    beforeEach(() => {
-      mockRequest.params = {
-        username: user.Username,
-      };
-    });
     it('should return status 200 and a list of roles assigned to a user', async () => {
       await getAllRoles(mockRequest, mockResponse);
       expect(mockResponse.statusValue).toBe(200);
       expect(mockResponse.sendValue.at(0)).toBe('admin');
     });
 
-    it('should return status 400 when no username is provided', async () => {
-      mockRequest.params = {};
+    it('should return status 400 when internal service throws', async () => {
+      _getRoles.mockImplementationOnce(() => {
+        throw Error();
+      });
       await getAllRoles(mockRequest, mockResponse);
-      expect(mockResponse.statusValue).toBe(400);
-      expect(mockResponse.sendValue).toBe('Username was empty.');
+      expect(mockResponse.statusValue).toBe(500);
     });
   });
 

express-api/tests/unit/controllers/users/usersController.test.ts

@@ -34,6 +34,7 @@ const _getUser = jest
   .fn()
   .mockImplementation((guid: string) => ({ ...produceUser(), KeycloakUserId: guid }));
 const _normalizeKeycloakUser = jest.fn().mockImplementation(() => {});
+
 jest.mock('@/services/users/usersServices', () => ({
   activateUser: () => _activateUser(),
   getAccessRequest: () => _getAccessRequest(),
@@ -48,6 +49,16 @@ jest.mock('@/services/users/usersServices', () => ({
   normalizeKeycloakUser: () => _normalizeKeycloakUser(),
 }));
 
+const _syncKeycloakRoles = jest.fn();
+const _syncKeycloakUser = jest
+  .fn()
+  .mockImplementation((username: string) => ({ ...produceUser(), Username: username }));
+
+jest.mock('@/services/keycloak/keycloakService.ts', () => ({
+  syncKeycloakRoles: () => _syncKeycloakRoles(),
+  syncKeycloakUser: () => _syncKeycloakUser(),
+}));
+
 describe('UNIT - Testing controllers for users routes.', () => {
   let mockRequest: Request & MockReq, mockResponse: Response & MockRes;
 

express-api/tests/unit/services/admin/usersServices.test.ts

@@ -31,6 +31,7 @@ jest.mock('@/services/keycloak/keycloakService', () => ({
   getKeycloakUserRoles: (): IKeycloakRole[] => [{ name: 'abc' }],
   updateKeycloakUserRoles: (username: string, roles: string[]): IKeycloakRole[] =>
     roles.map((a) => ({ name: a })),
+  syncKeycloakUser: (username: string) => ({ ...produceUser(), Username: username }),
 }));
 
 describe('UNIT - admin user services', () => {

express-api/tests/unit/services/keycloak/keycloakService.test.ts

@@ -12,6 +12,11 @@ import {
   unassignUserRole,
 } from '@bcgov/citz-imb-kc-css-api';
 import { faker } from '@faker-js/faker';
+import { produceRole, produceUser } from 'tests/testUtils/factories';
+import { AppDataSource } from '@/appDataSource';
+import { Role } from '@/typeorm/Entities/Role';
+import { User } from '@/typeorm/Entities/User';
+import { DeepPartial } from 'typeorm';
 
 jest.mock('@bcgov/citz-imb-kc-css-api');
 
@@ -24,17 +29,87 @@ const mockUser: IKeycloakUser = {
     display_name: [faker.person.fullName()],
   },
 };
+// eslint-disable-next-line @typescript-eslint/no-unused-vars
+const _updateUser = jest.fn().mockImplementation((_user: DeepPartial<User>) => ({ raw: {} }));
+
+jest.mock('@/services/admin/usersServices', () => ({
+  getUsers: () => [produceUser()],
+  getUserById: () => produceUser(),
+  updateUser: (user: DeepPartial<User>) => _updateUser(user),
+}));
+
+const _getRoles = jest.fn();
+const _addRole = jest.fn();
+const _updateRole = jest.fn();
+const _getRoleByName = jest.fn().mockImplementation(async () => produceRole());
+
+jest.mock('@/services/admin/rolesServices', () => ({
+  getRoles: () => _getRoles(),
+  addRole: () => _addRole(),
+  updateRole: () => _updateRole(),
+  getRoleByName: () => _getRoleByName(),
+}));
+
+const _getKeycloakRoles = jest.spyOn(KeycloakService, 'getKeycloakRoles');
+const _getKeycloakUserRoles = jest.spyOn(KeycloakService, 'getKeycloakUserRoles');
+
+const _repoFindBy = jest.spyOn(AppDataSource.getRepository(Role), 'findBy');
+const _repoDelete = jest
+  .spyOn(AppDataSource.getRepository(Role), 'delete')
+  .mockImplementation(async () => ({ raw: {} }));
+// eslint-disable-next-line @typescript-eslint/no-explicit-any
+const userCreateQueryBuilder: any = {
+  update: () => userCreateQueryBuilder,
+  set: () => userCreateQueryBuilder,
+  where: () => userCreateQueryBuilder,
+  execute: () => {},
+};
+jest
+  .spyOn(AppDataSource.getRepository(User), 'createQueryBuilder')
+  .mockImplementation(() => userCreateQueryBuilder);
 
 describe('UNIT - KeycloakService', () => {
   beforeEach(() => {
-    jest.resetAllMocks();
+    jest.clearAllMocks();
   });
 
   describe('syncKeycloakRoles', () => {
-    // TODO: finish tests when function is implemented
-    xit('should add roles to PIMS if they do not already exist', () => {});
-    xit('should remove roles from PIMS if they do not exist in Keycloak', () => {});
-    xit('should update roles in PIMS if they do not match what is in Keycloak', () => {});
+    beforeEach(() => jest.clearAllMocks());
+
+    it('should add roles to PIMS if they do not already exist', async () => {
+      _getKeycloakRoles.mockImplementationOnce(async () => [{ name: 'Administrator' }]);
+      _getRoles.mockImplementationOnce(async () => []);
+      _addRole.mockImplementationOnce(async (role) => role);
+      _repoFindBy.mockImplementationOnce(async () => []);
+      await KeycloakService.syncKeycloakRoles();
+
+      expect(_addRole).toHaveBeenCalledTimes(1);
+      expect(_repoDelete).toHaveBeenCalledTimes(0);
+    });
+    it('should remove roles from PIMS if they do not exist in Keycloak', async () => {
+      _getKeycloakRoles.mockImplementationOnce(async () => []);
+      _getRoles.mockImplementationOnce(async () => []);
+      _repoFindBy.mockImplementationOnce(async () => {
+        return [{ ...produceRole(), Name: 'OldRole' }];
+      });
+      await KeycloakService.syncKeycloakRoles();
+
+      expect(_addRole).toHaveBeenCalledTimes(0);
+      expect(_repoDelete).toHaveBeenCalledTimes(1);
+    });
+    it('should update roles in PIMS if they do not match what is in Keycloak', async () => {
+      _getKeycloakRoles.mockImplementationOnce(async () => [{ name: 'Administrator' }]);
+      _getRoles.mockImplementationOnce(async () =>
+        Promise.resolve([{ ...produceRole(), Name: 'Administrator' }]),
+      );
+      _updateRole.mockImplementationOnce(async (role) => role);
+      _repoFindBy.mockImplementationOnce(async () => []);
+
+      await KeycloakService.syncKeycloakRoles();
+
+      expect(_updateRole).toHaveBeenCalledTimes(1);
+      expect(_repoDelete).toHaveBeenCalledTimes(0);
+    });
   });
 
   describe('getKeycloakRoles', () => {
@@ -114,8 +189,16 @@ describe('UNIT - KeycloakService', () => {
 
   describe('syncKeycloakUser', () => {
     // TODO: finish tests when function is implemented
-    xit('should add update user roles in PIMS', () => {});
-    xit('should add user and roles in PIMS if they do not exist', () => {});
+    it('should add update user roles in PIMS', async () => {
+      _getKeycloakUserRoles.mockImplementationOnce(async () => [{ name: 'Test' }]);
+      await KeycloakService.syncKeycloakUser('test');
+      expect(_updateUser.mock.calls[0][0].RoleId).toBeTruthy();
+    });
+    it('should null role if no role is present', async () => {
+      _getKeycloakUserRoles.mockImplementationOnce(async () => []);
+      await KeycloakService.syncKeycloakUser('test');
+      expect(_updateUser.mock.calls[0][0].RoleId).toBeNull();
+    });
   });
 
   describe('getKeycloakUsers', () => {