Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PIMS-1718: Keycloak not updating role #2434

Merged
merged 6 commits into from
Jun 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions express-api/src/services/keycloak/keycloakService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -241,17 +241,17 @@ const getKeycloakUser = async (guid: string) => {
* @returns {IKeycloakRole[]} A list of the user's roles.
* @throws If the user is not found.
*/
const getKeycloakUserRoles = async (username: string) => {
const getKeycloakUserRoles = async (username: string): Promise<IKeycloakRole[]> => {
const existingRolesResponse: IKeycloakRolesResponse | IKeycloakErrorResponse =
await getUserRoles(username);

if (!keycloakUserRolesSchema.safeParse(existingRolesResponse).success) {
const message = `keycloakService.getKeycloakUserRoles: ${
(existingRolesResponse as IKeycloakErrorResponse).message
}`;
const message = `keycloakService.getKeycloakUserRoles: ${(existingRolesResponse as IKeycloakErrorResponse).message}`;
logger.warn(message);
throw new Error(message);
}
return (existingRolesResponse as IKeycloakRolesResponse).data;
// Ensure the response always returns an array of roles
return (existingRolesResponse as IKeycloakRolesResponse).data || [];
};

/**
Expand Down Expand Up @@ -288,7 +288,10 @@ const updateKeycloakUserRoles = async (username: string, roles: string[]) => {
(e as IKeycloakErrorResponse).message
}`;
logger.warn(message);
throw new Error(message);
throw new ErrorWithCode(
`Failed to update user ${username}'s Keycloak roles. User's Keycloak account may not be active.`,
500,
);
}
};

Expand Down
4 changes: 4 additions & 0 deletions express-api/src/services/users/usersServices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ const addUser = async (user: User) => {
};

const updateUser = async (user: DeepPartial<User>) => {
const roleName = user.Role?.Name;
const resource = await AppDataSource.getRepository(User).findOne({ where: { Id: user.Id } });
if (!resource) {
throw new ErrorWithCode('Resource does not exist.', 404);
Expand All @@ -272,6 +273,9 @@ const updateUser = async (user: DeepPartial<User>) => {
...user,
DisplayName: `${user.LastName}, ${user.FirstName}`,
});
if (roleName) {
await KeycloakService.updateKeycloakUserRoles(resource.Username, [roleName]);
}
return retUser.generatedMaps[0];
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ describe('UNIT - KeycloakService', () => {
(getUserRoles as jest.Mock).mockResolvedValueOnce({ message: 'no user found' });

expect(KeycloakService.updateKeycloakUserRoles(mockUser.username, ['role1'])).rejects.toThrow(
`keycloakService.updateKeycloakUserRoles: `,
/^Failed to update user/,
);
expect(getUserRoles).toHaveBeenCalledTimes(1);
});
Expand Down
Loading