squash!: Improve permission validation hook with a clearer and safer API

rodmgwgu · rodmgwgu · commit a435e1da6bd3 · 2025-12-01T12:05:45.000-06:00
diff --git a/src/authz/data/api.ts b/src/authz/data/api.ts
@@ -1,10 +1,41 @@
 import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth';
-import { PermissionValidationRequest, PermissionValidationResponse } from '@src/authz/types';
+import {
+  PermissionValidationAnswer,
+  PermissionValidationQuery,
+  PermissionValidationRequestItem,
+  PermissionValidationResponseItem,
+} from '@src/authz/types';
 import { getApiUrl } from './utils';
 
 export const validateUserPermissions = async (
-  validations: PermissionValidationRequest[],
-): Promise<PermissionValidationResponse[]> => {
-  const { data } = await getAuthenticatedHttpClient().post(getApiUrl('/api/authz/v1/permissions/validate/me'), validations);
-  return data;
+  query: PermissionValidationQuery,
+): Promise<PermissionValidationAnswer> => {
+  // Convert the validations query object into an array for the API request
+  const request: PermissionValidationRequestItem[] = Object.values(query);
+
+  const { data }: { data: PermissionValidationResponseItem[] } = await getAuthenticatedHttpClient().post(
+    getApiUrl('/api/authz/v1/permissions/validate/me'),
+    request,
+  );
+
+  // Convert the API response back into the expected answer format
+  const result: PermissionValidationAnswer = {};
+  data.forEach((item: { action: string; scope?: string; allowed: boolean }) => {
+    const key = Object.keys(query).find(
+      (k) => query[k].action === item.action
+        && query[k].scope === item.scope,
+    );
+    if (key) {
+      result[key] = item.allowed;
+    }
+  });
+
+  // Fill any missing keys with false
+  Object.keys(query).forEach((key) => {
+    if (!(key in result)) {
+      result[key] = false;
+    }
+  });
+
+  return result;
 };
diff --git a/src/authz/data/apiHooks.test.tsx b/src/authz/data/apiHooks.test.tsx
@@ -2,7 +2,7 @@ import { act, ReactNode } from 'react';
 import { renderHook, waitFor } from '@testing-library/react';
 import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
 import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth';
-import { useValidateUserPermissions } from './apiHooks';
+import { useUserPermissions } from './apiHooks';
 
 jest.mock('@edx/frontend-platform/auth', () => ({
   getAuthenticatedHttpClient: jest.fn(),
@@ -18,64 +18,134 @@ const createWrapper = () => {
   });
 
   const wrapper = ({ children }: { children: ReactNode }) => (
-    <QueryClientProvider client={queryClient}>
-      {children}
-    </QueryClientProvider>
+    <QueryClientProvider client={queryClient}>{children}</QueryClientProvider>
   );
 
   return wrapper;
 };
 
-const permissions = [
-  {
+const singlePermission = {
+  canRead: {
     action: 'act:read',
-    object: 'lib:test-lib',
-    scope: 'org:OpenedX',
+    scope: 'lib:test-lib',
   },
+};
+
+const mockValidSinglePermission = [
+  { action: 'act:read', scope: 'lib:test-lib', allowed: true },
+];
+
+const mockInvalidSinglePermission = [
+  { action: 'act:read', scope: 'lib:test-lib', allowed: false },
+];
+
+const mockEmptyPermissions = [
+  // No permissions returned
 ];
 
-const mockValidPermissions = [
-  { action: 'act:read', object: 'lib:test-lib', allowed: true },
+const multiplePermissions = {
+  canRead: {
+    action: 'act:read',
+    scope: 'lib:test-lib',
+  },
+  canWrite: {
+    action: 'act:write',
+    scope: 'lib:test-lib',
+  },
+};
+
+const mockValidMultiplePermissions = [
+  { action: 'act:read', scope: 'lib:test-lib', allowed: true },
+  { action: 'act:write', scope: 'lib:test-lib', allowed: true },
 ];
 
-const mockInvalidPermissions = [
-  { action: 'act:read', object: 'lib:test-lib', allowed: false },
+const mockInvalidMultiplePermissions = [
+  { action: 'act:read', scope: 'lib:test-lib', allowed: false },
+  { action: 'act:write', scope: 'lib:test-lib', allowed: false },
 ];
 
-describe('useValidateUserPermissions', () => {
+describe('useUserPermissions', () => {
   beforeEach(() => {
     jest.clearAllMocks();
   });
 
-  it('returns allowed true when permissions are valid', async () => {
+  it('returns allowed true when permission is valid', async () => {
     getAuthenticatedHttpClient.mockReturnValue({
-      post: jest.fn().mockResolvedValueOnce({ data: mockValidPermissions }),
+      post: jest.fn().mockResolvedValueOnce({ data: mockValidSinglePermission }),
     });
 
-    const { result } = renderHook(() => useValidateUserPermissions(permissions), {
+    const { result } = renderHook(() => useUserPermissions(singlePermission), {
       wrapper: createWrapper(),
     });
 
     await waitFor(() => expect(result.current).toBeDefined());
     await waitFor(() => expect(result.current.data).toBeDefined());
 
     expect(getAuthenticatedHttpClient).toHaveBeenCalled();
-    expect(result.current.data![0].allowed).toBe(true);
+    expect(result.current.data!.canRead).toBe(true);
+  });
+
+  it('returns allowed false when permission is invalid', async () => {
+    getAuthenticatedHttpClient.mockReturnValue({
+      post: jest.fn().mockResolvedValue({ data: mockInvalidSinglePermission }),
+    });
+
+    const { result } = renderHook(() => useUserPermissions(singlePermission), {
+      wrapper: createWrapper(),
+    });
+    await waitFor(() => expect(result.current).toBeDefined());
+    await waitFor(() => expect(result.current.data).toBeDefined());
+
+    expect(getAuthenticatedHttpClient).toHaveBeenCalled();
+    expect(result.current.data!.canRead).toBe(false);
+  });
+
+  it('returns allowed true when multiple permissions are valid', async () => {
+    getAuthenticatedHttpClient.mockReturnValue({
+      post: jest.fn().mockResolvedValueOnce({ data: mockValidMultiplePermissions }),
+    });
+
+    const { result } = renderHook(() => useUserPermissions(multiplePermissions), {
+      wrapper: createWrapper(),
+    });
+
+    await waitFor(() => expect(result.current).toBeDefined());
+    await waitFor(() => expect(result.current.data).toBeDefined());
+
+    expect(getAuthenticatedHttpClient).toHaveBeenCalled();
+    expect(result.current.data!.canRead).toBe(true);
+    expect(result.current.data!.canWrite).toBe(true);
+  });
+
+  it('returns allowed false when multiple permissions are invalid', async () => {
+    getAuthenticatedHttpClient.mockReturnValue({
+      post: jest.fn().mockResolvedValue({ data: mockInvalidMultiplePermissions }),
+    });
+
+    const { result } = renderHook(() => useUserPermissions(multiplePermissions), {
+      wrapper: createWrapper(),
+    });
+    await waitFor(() => expect(result.current).toBeDefined());
+    await waitFor(() => expect(result.current.data).toBeDefined());
+
+    expect(getAuthenticatedHttpClient).toHaveBeenCalled();
+    expect(result.current.data!.canRead).toBe(false);
+    expect(result.current.data!.canWrite).toBe(false);
   });
 
-  it('returns allowed false when permissions are invalid', async () => {
+  it('returns allowed false when the permission is not included in the server response', async () => {
     getAuthenticatedHttpClient.mockReturnValue({
-      post: jest.fn().mockResolvedValue({ data: mockInvalidPermissions }),
+      post: jest.fn().mockResolvedValue({ data: mockEmptyPermissions }),
     });
 
-    const { result } = renderHook(() => useValidateUserPermissions(permissions), {
+    const { result } = renderHook(() => useUserPermissions(singlePermission), {
       wrapper: createWrapper(),
     });
     await waitFor(() => expect(result.current).toBeDefined());
     await waitFor(() => expect(result.current.data).toBeDefined());
 
     expect(getAuthenticatedHttpClient).toHaveBeenCalled();
-    expect(result.current.data![0].allowed).toBe(false);
+    expect(result.current.data!.canRead).toBe(false);
   });
 
   it('handles error when the API call fails', async () => {
@@ -87,7 +157,7 @@ describe('useValidateUserPermissions', () => {
 
     try {
       act(() => {
-        renderHook(() => useValidateUserPermissions(permissions), {
+        renderHook(() => useUserPermissions(singlePermission), {
           wrapper: createWrapper(),
         });
       });
diff --git a/src/authz/data/apiHooks.ts b/src/authz/data/apiHooks.ts
@@ -1,10 +1,10 @@
 import { useQuery } from '@tanstack/react-query';
-import { PermissionValidationRequest, PermissionValidationResponse } from '@src/authz/types';
+import { PermissionValidationAnswer, PermissionValidationQuery } from '@src/authz/types';
 import { validateUserPermissions } from './api';
 
 const adminConsoleQueryKeys = {
   all: ['authz'],
-  permissions: (permissions: PermissionValidationRequest[]) => [...adminConsoleQueryKeys.all, 'validatePermissions', permissions] as const,
+  permissions: (permissions: PermissionValidationQuery) => [...adminConsoleQueryKeys.all, 'validatePermissions', permissions] as const,
 };
 
 /**
@@ -13,19 +13,23 @@ const adminConsoleQueryKeys = {
  * - Determine whether the current user can access certain object.
  * - Provide role-based rendering logic for UI components.
  *
- * @param permissions - The array of objects and actions to validate.
+ * @param permissions - A key/value map of objects and actions to validate.
+ * The key is an arbitrary string to identify the permission check,
+ * and the value is an object containing the action and optional scope.
  *
  * @example
- * const { data } = useValidateUserPermissions([{
-           "action": "act:read",
-           "scope": "org:OpenedX"
-       }]);
- * if (data[0].allowed) { ... }
+ * const { isLoading, data } = useUserPermissions({
+ *     "canRead": {
+ *         "action": "act:read",
+ *         "scope": "org:OpenedX"
+ *      }
+ *    });
+ * if (data.canRead) { ... }
  *
  */
-export const useValidateUserPermissions = (
-  permissions: PermissionValidationRequest[],
-) => useQuery<PermissionValidationResponse[], Error>({
+export const useUserPermissions = (
+  permissions: PermissionValidationQuery,
+) => useQuery<PermissionValidationAnswer, Error>({
   queryKey: adminConsoleQueryKeys.permissions(permissions),
   queryFn: () => validateUserPermissions(permissions),
   retry: false,
diff --git a/src/authz/types.ts b/src/authz/types.ts
@@ -1,8 +1,16 @@
-export interface PermissionValidationRequest {
+export interface PermissionValidationRequestItem {
   action: string;
   scope?: string;
 }
 
-export interface PermissionValidationResponse extends PermissionValidationRequest {
+export interface PermissionValidationResponseItem extends PermissionValidationRequestItem {
   allowed: boolean;
 }
+
+export interface PermissionValidationQuery {
+  [permissionKey: string]: PermissionValidationRequestItem;
+}
+
+export interface PermissionValidationAnswer {
+  [permissionKey: string]: boolean;
+}
diff --git a/src/library-authoring/common/context/LibraryContext.tsx b/src/library-authoring/common/context/LibraryContext.tsx
@@ -7,7 +7,7 @@ import {
   useState,
 } from 'react';
 import { useParams } from 'react-router-dom';
-import { useValidateUserPermissions } from '@src/authz/data/apiHooks';
+import { useUserPermissions } from '@src/authz/data/apiHooks';
 import { CONTENT_LIBRARY_PERMISSIONS } from '@src/authz/constants';
 import { ContainerType } from '../../../generic/key-utils';
 
@@ -16,10 +16,6 @@ import type { ContentLibrary, BlockTypeMetadata } from '../../data/api';
 import { useContentLibrary } from '../../data/apiHooks';
 import { useComponentPickerContext } from './ComponentPickerContext';
 
-const LIBRARY_PERMISSIONS = [
-  CONTENT_LIBRARY_PERMISSIONS.PUBLISH_LIBRARY_CONTENT,
-];
-
 export interface ComponentEditorInfo {
   usageKey: string;
   blockType?:string
@@ -114,10 +110,13 @@ export const LibraryProvider = ({
     componentPickerMode,
   } = useComponentPickerContext();
 
-  const permissions = LIBRARY_PERMISSIONS.map(action => ({ action, scope: libraryId }));
-
-  const { isLoading: isLoadingUserPermissions, data: userPermissions } = useValidateUserPermissions(permissions);
-  const canPublish = userPermissions ? userPermissions[0]?.allowed : false;
+  const { isLoading: isLoadingUserPermissions, data: userPermissions } = useUserPermissions({
+    canPublish: {
+      action: CONTENT_LIBRARY_PERMISSIONS.PUBLISH_LIBRARY_CONTENT,
+      scope: libraryId,
+    },
+  });
+  const canPublish = userPermissions?.canPublish || false;
   const readOnly = !!componentPickerMode || !libraryData?.canEditLibrary;
 
   // Parse the initial collectionId and/or container ID(s) from the current URL params
