Skip to content

Commit fc51f17

Browse files
feat: authz permission validation for Course Updates (#2938)
* feat: adding permission validations from authz for course updates for view and manage * refactor: replace useUserPermissionsWithAuthzCourse with useCourseUserPermissions Remove the deprecated useUserPermissionsWithAuthzCourse hook and migrate all consumers to use useCourseUserPermissions, which provides a flatter API by spreading permission booleans directly into the return object.
1 parent 7138dfb commit fc51f17

14 files changed

Lines changed: 587 additions & 50 deletions

src/authz/constants.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ export const COURSE_PERMISSIONS = {
2424
VIEW_SCHEDULE_AND_DETAILS: 'courses.view_schedule_and_details',
2525
EDIT_SCHEDULE: 'courses.edit_schedule',
2626
EDIT_DETAILS: 'courses.edit_details',
27+
VIEW_COURSE_UPDATES: 'courses.view_course_updates',
28+
MANAGE_COURSE_UPDATES: 'courses.manage_course_updates',
2729

2830
VIEW_PAGES_AND_RESOURCES: 'courses.view_pages_and_resources',
2931
MANAGE_PAGES_AND_RESOURCES: 'courses.manage_pages_and_resources',

src/authz/hooks.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,8 @@ describe('useCourseUserPermissions', () => {
6060

6161
expect(result.current.isLoading).toBe(true);
6262
expect(result.current.isAuthzEnabled).toBe(true);
63-
expect(result.current.canView).toBeUndefined();
64-
expect(result.current.canEdit).toBeUndefined();
63+
expect(result.current.canView).toBe(false);
64+
expect(result.current.canEdit).toBe(false);
6565
});
6666

6767
it('falls back to false for permissions absent from server response when authz is enabled', () => {

src/authz/hooks.test.tsx

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
import React from 'react';
2+
import { renderHook, waitFor } from '@testing-library/react';
3+
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
4+
5+
import * as authzApi from '@src/authz/data/api';
6+
import { PermissionValidationQuery } from '@src/authz/types';
7+
import { mockWaffleFlags } from '@src/data/apiHooks.mock';
8+
import { useCourseUserPermissions } from './hooks';
9+
10+
jest.mock('@src/data/api');
11+
jest.mock('@src/authz/data/api');
12+
13+
const mockedAuthzApi = jest.mocked(authzApi);
14+
15+
describe('useCourseUserPermissions', () => {
16+
let queryClient: QueryClient;
17+
18+
const createWrapper = () =>
19+
function TestWrapper({ children }: { children: React.ReactNode; }) {
20+
return (
21+
<QueryClientProvider client={queryClient}>
22+
{children}
23+
</QueryClientProvider>
24+
);
25+
};
26+
27+
const mockPermissions: PermissionValidationQuery = {
28+
canViewFiles: {
29+
action: 'course.view_files',
30+
scope: 'course-v1:Test+101+2023',
31+
},
32+
canManageFiles: {
33+
action: 'course.manage_files',
34+
scope: 'course-v1:Test+101+2023',
35+
},
36+
};
37+
38+
beforeEach(() => {
39+
queryClient = new QueryClient({
40+
defaultOptions: {
41+
queries: { retry: false },
42+
},
43+
});
44+
jest.clearAllMocks();
45+
});
46+
47+
it('returns all permissions as true when authz is disabled', async () => {
48+
mockWaffleFlags({
49+
enableAuthzCourseAuthoring: false,
50+
});
51+
52+
const { result } = renderHook(
53+
() => useCourseUserPermissions('course-v1:Test+101+2023', mockPermissions),
54+
{ wrapper: createWrapper() },
55+
);
56+
57+
await waitFor(() => {
58+
expect(result.current.isLoading).toBe(false);
59+
});
60+
61+
expect(result.current.isAuthzEnabled).toBe(false);
62+
expect(result.current.canViewFiles).toBe(true);
63+
expect(result.current.canManageFiles).toBe(true);
64+
});
65+
66+
it('returns loading state when authz is enabled and permissions are loading', async () => {
67+
mockWaffleFlags({
68+
enableAuthzCourseAuthoring: true,
69+
});
70+
71+
mockedAuthzApi.validateUserPermissions.mockImplementation(
72+
() => new Promise(() => {}),
73+
);
74+
75+
const { result } = renderHook(
76+
() => useCourseUserPermissions('course-v1:Test+101+2023', mockPermissions),
77+
{ wrapper: createWrapper() },
78+
);
79+
80+
await waitFor(() => {
81+
expect(result.current.isAuthzEnabled).toBe(true);
82+
});
83+
84+
expect(result.current.isLoading).toBe(true);
85+
expect(result.current.canViewFiles).toBe(false);
86+
expect(result.current.canManageFiles).toBe(false);
87+
});
88+
89+
it('returns actual permission values when authz is enabled and permissions loaded', async () => {
90+
mockWaffleFlags({
91+
enableAuthzCourseAuthoring: true,
92+
});
93+
94+
mockedAuthzApi.validateUserPermissions.mockResolvedValue({
95+
canViewFiles: true,
96+
canManageFiles: false,
97+
});
98+
99+
const { result } = renderHook(
100+
() => useCourseUserPermissions('course-v1:Test+101+2023', mockPermissions),
101+
{ wrapper: createWrapper() },
102+
);
103+
104+
await waitFor(() => {
105+
expect(result.current.isLoading).toBe(false);
106+
});
107+
108+
expect(result.current.isAuthzEnabled).toBe(true);
109+
expect(result.current.canViewFiles).toBe(true);
110+
expect(result.current.canManageFiles).toBe(false);
111+
});
112+
113+
it('falls back to false for undefined permissions when authz is enabled', async () => {
114+
mockWaffleFlags({
115+
enableAuthzCourseAuthoring: true,
116+
});
117+
118+
mockedAuthzApi.validateUserPermissions.mockResolvedValue({
119+
canViewFiles: true,
120+
});
121+
122+
const { result } = renderHook(
123+
() => useCourseUserPermissions('course-v1:Test+101+2023', mockPermissions),
124+
{ wrapper: createWrapper() },
125+
);
126+
127+
await waitFor(() => {
128+
expect(result.current.isLoading).toBe(false);
129+
});
130+
131+
expect(result.current.isAuthzEnabled).toBe(true);
132+
expect(result.current.canViewFiles).toBe(true);
133+
expect(result.current.canManageFiles).toBe(false);
134+
});
135+
});

src/authz/hooks.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,29 +42,35 @@ export const useCourseUserPermissions = <Query extends PermissionValidationQuery
4242
permissions: Query,
4343
): UseCourseUserPermissionsReturn<Query> => {
4444
const waffleFlags = useWaffleFlags(courseId);
45+
const isWaffleFlagsLoading: boolean = waffleFlags?.isLoading ?? true;
4546
const isAuthzEnabled: boolean = waffleFlags?.enableAuthzCourseAuthoring ?? false;
4647

4748
const {
4849
isLoading: isLoadingUserPermissions,
4950
data: userPermissions,
5051
} = useUserPermissions(permissions, isAuthzEnabled);
5152

53+
const isLoading = isWaffleFlagsLoading || (isAuthzEnabled && isLoadingUserPermissions);
54+
5255
const resolvePermission = (key: string): boolean => {
5356
if (!isAuthzEnabled) {
5457
return true;
5558
}
5659
return userPermissions?.[key] ?? false;
5760
};
5861

59-
const permissionResults: Record<string, boolean> = isLoadingUserPermissions
60-
? {}
62+
const permissionResults: Record<string, boolean> = isLoading
63+
? Object.keys(permissions).reduce<Record<string, boolean>>((acc, key) => {
64+
acc[key] = false;
65+
return acc;
66+
}, {})
6167
: Object.keys(permissions).reduce<Record<string, boolean>>((acc, key) => {
6268
acc[key] = resolvePermission(key);
6369
return acc;
6470
}, {});
6571

6672
return {
67-
isLoading: isAuthzEnabled ? isLoadingUserPermissions : false,
73+
isLoading,
6874
isAuthzEnabled,
6975
...permissionResults as PermissionValidationAnswer<Query>,
7076
};

src/authz/permissionHelpers.test.ts

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,38 @@ import {
22
getGradingPermissions,
33
getPagesAndResourcesPermissions,
44
getAdvancedSettingsPermissions,
5+
getCourseUpdatesPermissions,
56
} from './permissionHelpers';
67
import { COURSE_PERMISSIONS } from './constants';
78

9+
const courseId = 'course-v1:org+course+run';
10+
811
describe('permissionHelpers', () => {
9-
const courseId = 'course-v1:org+course+run';
12+
describe('getCourseUpdatesPermissions', () => {
13+
it('should return correct permission structure for course updates operations', () => {
14+
const result = getCourseUpdatesPermissions(courseId);
15+
16+
expect(result).toEqual({
17+
canViewCourseUpdates: {
18+
action: COURSE_PERMISSIONS.VIEW_COURSE_UPDATES,
19+
scope: courseId,
20+
},
21+
canManageCourseUpdates: {
22+
action: COURSE_PERMISSIONS.MANAGE_COURSE_UPDATES,
23+
scope: courseId,
24+
},
25+
});
26+
});
27+
28+
it('should use the provided courseId as scope for all permissions', () => {
29+
const customCourseId = 'course-v1:TestOrg+TestCourse+2024';
30+
const result = getCourseUpdatesPermissions(customCourseId);
31+
32+
Object.values(result).forEach(permission => {
33+
expect(permission.scope).toBe(customCourseId);
34+
});
35+
});
36+
});
1037

1138
describe('getGradingPermissions', () => {
1239
it('returns VIEW and EDIT permissions with the correct actions and scope', () => {

src/authz/permissionHelpers.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,17 @@ export const getGradingPermissions = (courseId: string) => ({
2626
},
2727
});
2828

29+
export const getCourseUpdatesPermissions = (courseId: string) => ({
30+
canViewCourseUpdates: {
31+
action: COURSE_PERMISSIONS.VIEW_COURSE_UPDATES,
32+
scope: courseId,
33+
},
34+
canManageCourseUpdates: {
35+
action: COURSE_PERMISSIONS.MANAGE_COURSE_UPDATES,
36+
scope: courseId,
37+
},
38+
});
39+
2940
export const getPagesAndResourcesPermissions = (courseId: string) => ({
3041
canViewPagesAndResources: {
3142
action: COURSE_PERMISSIONS.VIEW_PAGES_AND_RESOURCES,

0 commit comments

Comments
 (0)