From 3acdff4a3d0b4d25baabdaafb6afc05694a1b001 Mon Sep 17 00:00:00 2001 From: Al Rosenthal Date: Thu, 5 Oct 2023 16:56:19 -0700 Subject: [PATCH] SIMSBIOHUB-287: User Access Request (#1103) Fixed issues with users requesting access to the system --------- Co-authored-by: Nick Phura --- api/src/paths/administrative-activity.test.ts | 2 +- api/src/paths/administrative-activity.ts | 11 +++++----- .../administrative-activity-repository.ts | 14 ++++++------- .../administrative-activity-service.test.ts | 2 +- .../administrative-activity-service.ts | 8 +++---- .../admin/users/AccessRequestList.test.tsx | 17 +++++++-------- .../admin/users/AccessRequestList.tsx | 17 +++++++-------- app/src/hooks/api/useAdminApi.ts | 21 ++++++++----------- app/src/hooks/useKeycloakWrapper.tsx | 2 +- app/src/interfaces/useAdminApi.interface.ts | 2 +- 10 files changed, 45 insertions(+), 51 deletions(-) diff --git a/api/src/paths/administrative-activity.test.ts b/api/src/paths/administrative-activity.test.ts index cdb381b727..97b5e212ba 100644 --- a/api/src/paths/administrative-activity.test.ts +++ b/api/src/paths/administrative-activity.test.ts @@ -109,7 +109,7 @@ describe('getAdministrativeActivityStanding', () => { sinon.stub(db, 'getAPIUserDBConnection').returns(dbConnectionObj); const mockResponse: IAdministrativeActivityStanding = { - has_pending_acccess_request: true, + has_pending_access_request: true, has_one_or_more_project_roles: true }; diff --git a/api/src/paths/administrative-activity.ts b/api/src/paths/administrative-activity.ts index a3f7ff1d0a..2db83ff03a 100644 --- a/api/src/paths/administrative-activity.ts +++ b/api/src/paths/administrative-activity.ts @@ -87,9 +87,9 @@ GET.apiDoc = { 'application/json': { schema: { type: 'object', - required: ['has_pending_acccess_request', 'has_one_or_more_project_roles'], + required: ['has_pending_access_request', 'has_one_or_more_project_roles'], properties: { - has_pending_acccess_request: { + has_pending_access_request: { type: 'boolean' }, has_one_or_more_project_roles: { @@ -166,10 +166,9 @@ export function getAdministrativeActivityStanding(): RequestHandler { const connection = getAPIUserDBConnection(); try { - // TODO Update to use user guid instead of identifier - const userIdentifier = getUserGuid(req['keycloak_token']); + const userGUID = getUserGuid(req['keycloak_token']); - if (!userIdentifier) { + if (!userGUID) { throw new HTTP400('Failed to identify user'); } @@ -177,7 +176,7 @@ export function getAdministrativeActivityStanding(): RequestHandler { const administrativeActivityService = new AdministrativeActivityService(connection); - const response = await administrativeActivityService.getAdministrativeActivityStanding(userIdentifier); + const response = await administrativeActivityService.getAdministrativeActivityStanding(userGUID); await connection.commit(); diff --git a/api/src/repositories/administrative-activity-repository.ts b/api/src/repositories/administrative-activity-repository.ts index 5c4e47cd13..0a0dd1dd4c 100644 --- a/api/src/repositories/administrative-activity-repository.ts +++ b/api/src/repositories/administrative-activity-repository.ts @@ -9,7 +9,7 @@ import { jsonSchema } from '../zod-schema/json'; import { BaseRepository } from './base-repository'; export const IAdministrativeActivityStanding = z.object({ - has_pending_acccess_request: z.boolean(), + has_pending_access_request: z.boolean(), has_one_or_more_project_roles: z.boolean() }); @@ -176,13 +176,13 @@ export class AdministrativeActivityRepository extends BaseRepository { } /** - * SQL query to count pending records in the administrative_activity table. + * SQL query to count pending records in the administrative_activity table for a given user GUID * - * @param {string} userIdentifier + * @param {string} userGUID * @return {*} {(Promise)} * @memberof AdministrativeActivityRepository */ - async getAdministrativeActivityStanding(userIdentifier: string): Promise { + async getAdministrativeActivityStanding(userGUID: string): Promise { const sqlStatement = SQL` WITH administrative_activity_with_status @@ -191,7 +191,7 @@ export class AdministrativeActivityRepository extends BaseRepository { CASE WHEN COUNT(*) > 0 THEN TRUE ELSE FALSE - END AS has_pending_acccess_request + END AS has_pending_access_request FROM administrative_activity aa LEFT OUTER JOIN @@ -199,7 +199,7 @@ export class AdministrativeActivityRepository extends BaseRepository { ON aa.administrative_activity_status_type_id = aast.administrative_activity_status_type_id WHERE - (aa.data -> 'username')::text = '"' || ${userIdentifier} || '"' + (aa.data -> 'userGuid')::text = '"' || ${userGUID} || '"' AND aast.name = 'Pending' ), @@ -217,7 +217,7 @@ export class AdministrativeActivityRepository extends BaseRepository { ON pp.system_user_id = su.system_user_id WHERE - su.user_identifier = ${userIdentifier} + su.user_guid = ${userGUID} ) SELECT * FROM diff --git a/api/src/services/administrative-activity-service.test.ts b/api/src/services/administrative-activity-service.test.ts index 1a7ead543e..0f570b87cf 100644 --- a/api/src/services/administrative-activity-service.test.ts +++ b/api/src/services/administrative-activity-service.test.ts @@ -98,7 +98,7 @@ describe('AdministrativeActivityService', () => { const dbConnection = getMockDBConnection(); const mockRepoResponse = { - has_pending_acccess_request: false, + has_pending_access_request: false, has_one_or_more_project_roles: false }; const repoStub = sinon diff --git a/api/src/services/administrative-activity-service.ts b/api/src/services/administrative-activity-service.ts index d0f27e8e7c..e1306e090c 100644 --- a/api/src/services/administrative-activity-service.ts +++ b/api/src/services/administrative-activity-service.ts @@ -88,14 +88,14 @@ export class AdministrativeActivityService extends DBService { } /** - * Fetch an existing administrative activity record for a user, based on their user identifier. + * Fetch an existing administrative activity record for a user, based on their user GUID. * - * @param {string} userIdentifier + * @param {string} userGUID * @return {*} {(Promise)} * @memberof AdministrativeActivityService */ - async getAdministrativeActivityStanding(userIdentifier: string): Promise { - return this.administrativeActivityRepository.getAdministrativeActivityStanding(userIdentifier); + async getAdministrativeActivityStanding(userGUID: string): Promise { + return this.administrativeActivityRepository.getAdministrativeActivityStanding(userGUID); } /** diff --git a/app/src/features/admin/users/AccessRequestList.test.tsx b/app/src/features/admin/users/AccessRequestList.test.tsx index 2a64cad211..67b93462b7 100644 --- a/app/src/features/admin/users/AccessRequestList.test.tsx +++ b/app/src/features/admin/users/AccessRequestList.test.tsx @@ -223,15 +223,14 @@ describe('AccessRequestList', () => { await waitFor(() => { expect(refresh).toHaveBeenCalledTimes(1); expect(mockUseApi.admin.approveAccessRequest).toHaveBeenCalledTimes(1); - expect(mockUseApi.admin.approveAccessRequest).toHaveBeenCalledWith( - 1, - 'aaaa', - 'testusername', - SYSTEM_IDENTITY_SOURCE.IDIR, - 'email@email.com', - 'test user', - [2] - ); + expect(mockUseApi.admin.approveAccessRequest).toHaveBeenCalledWith(1, { + displayName: 'test user', + email: 'email@email.com', + identitySource: 'IDIR', + roleIds: [2], + userGuid: 'aaaa', + userIdentifier: 'testusername' + }); }); }); diff --git a/app/src/features/admin/users/AccessRequestList.tsx b/app/src/features/admin/users/AccessRequestList.tsx index a2822cb51c..2e148b9cd9 100644 --- a/app/src/features/admin/users/AccessRequestList.tsx +++ b/app/src/features/admin/users/AccessRequestList.tsx @@ -97,15 +97,14 @@ const AccessRequestList: React.FC = (props) => { setActiveReviewDialog({ open: false, request: null }); try { - await biohubApi.admin.approveAccessRequest( - updatedRequest.id, - updatedRequest.data.userGuid, - updatedRequest.data.username, - updatedRequest.data.identitySource, - updatedRequest.data.email, - updatedRequest.data.displayName, - (values.system_role && [values.system_role]) || [] - ); + await biohubApi.admin.approveAccessRequest(updatedRequest.id, { + userGuid: updatedRequest.data.userGuid, + userIdentifier: updatedRequest.data.username, + identitySource: updatedRequest.data.identitySource, + email: updatedRequest.data.email, + displayName: updatedRequest.data.displayName, + roleIds: (values.system_role && [values.system_role]) || [] + }); try { await biohubApi.admin.sendGCNotification( diff --git a/app/src/hooks/api/useAdminApi.ts b/app/src/hooks/api/useAdminApi.ts index f1269a3856..5355c0deb0 100644 --- a/app/src/hooks/api/useAdminApi.ts +++ b/app/src/hooks/api/useAdminApi.ts @@ -58,20 +58,17 @@ const useAdminApi = (axios: AxiosInstance) => { const approveAccessRequest = async ( administrativeActivityId: number, - userGuid: string | null, - userIdentifier: string, - identitySource: string, - displayName: string, - email: string, - roleIds: number[] = [] + userData: { + userGuid: string | null; + userIdentifier: string; + identitySource: string; + email: string; + displayName: string; + roleIds: number[]; + } ): Promise => { const { data } = await axios.put(`/api/administrative-activity/system-access/${administrativeActivityId}/approve`, { - userGuid, - userIdentifier, - identitySource, - displayName, - email, - roleIds: roleIds + ...userData }); return data; diff --git a/app/src/hooks/useKeycloakWrapper.tsx b/app/src/hooks/useKeycloakWrapper.tsx index 0042055c8f..e3a9d10d71 100644 --- a/app/src/hooks/useKeycloakWrapper.tsx +++ b/app/src/hooks/useKeycloakWrapper.tsx @@ -319,7 +319,7 @@ function useKeycloakWrapper(): IKeycloakWrapper { systemRoles: getSystemRoles(), hasSystemRole, isSystemUser, - hasAccessRequest: !!administrativeActivityStandingDataLoader.data?.has_pending_acccess_request, + hasAccessRequest: !!administrativeActivityStandingDataLoader.data?.has_pending_access_request, hasOneOrMoreProjectRoles: !!administrativeActivityStandingDataLoader.data?.has_one_or_more_project_roles, getUserIdentifier, getUserGuid, diff --git a/app/src/interfaces/useAdminApi.interface.ts b/app/src/interfaces/useAdminApi.interface.ts index 6af45ce947..dfda377fa7 100644 --- a/app/src/interfaces/useAdminApi.interface.ts +++ b/app/src/interfaces/useAdminApi.interface.ts @@ -34,7 +34,7 @@ export interface IGetAccessRequestsListResponse { } export interface IGetAdministrativeActivityStanding { - has_pending_acccess_request: boolean; + has_pending_access_request: boolean; has_one_or_more_project_roles: boolean; }