SIMSBIOHUB-287: User Access Request (#1103)

Fixed issues with users requesting access to the system --------- Co-authored-by: Nick Phura <[email protected]>
bcgov · Oct 5, 2023 · 3acdff4 · 3acdff4
1 parent 23374a8
commit 3acdff4
Show file tree

Hide file tree

Showing 10 changed files with 45 additions and 51 deletions.
diff --git 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
@@ -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,18 +166,17 @@ 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');
       }
 
       await connection.open();
 
       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
@@ -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<IAdministrativeActivityStanding>)}
    * @memberof AdministrativeActivityRepository
    */
-  async getAdministrativeActivityStanding(userIdentifier: string): Promise<IAdministrativeActivityStanding> {
+  async getAdministrativeActivityStanding(userGUID: string): Promise<IAdministrativeActivityStanding> {
     const sqlStatement = SQL`
       WITH
         administrative_activity_with_status
@@ -191,15 +191,15 @@ 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
           administrative_activity_status_type aast
         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
@@ -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
@@ -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<IAdministrativeActivityStanding>)}
    * @memberof AdministrativeActivityService
    */
-  async getAdministrativeActivityStanding(userIdentifier: string): Promise<IAdministrativeActivityStanding> {
-    return this.administrativeActivityRepository.getAdministrativeActivityStanding(userIdentifier);
+  async getAdministrativeActivityStanding(userGUID: string): Promise<IAdministrativeActivityStanding> {
+    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
@@ -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 protected]',
-        'test user',
-        [2]
-      );
+      expect(mockUseApi.admin.approveAccessRequest).toHaveBeenCalledWith(1, {
+        displayName: 'test user',
+        email: '[email protected]',
+        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
@@ -97,15 +97,14 @@ const AccessRequestList: React.FC<IAccessRequestListProps> = (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
@@ -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<void> => {
     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
@@ -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
@@ -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;
 }