Skip to content

Commit 7138dfb

Browse files
authored
feat: implement view and edit permission checks for pages and resources (#3031)
* feat(pages-and-resources): add read-only access for course_auditor role - Add VIEW_ADVANCED_SETTINGS and PAGE_AND_RESOURCES permissions - Add getPagesAndResourcesPermissions helper - Calculate isEditable and isReadOnly from user permissions - Propagate isEditable via PagesAndResourcesContext - Add disabled={!isEditable} to all forms, toggles, and buttons - Update AppCard and AppListNextButton with isEditable logic - Change default isEditable to false (fail closed) - Add unified permission gate showing PermissionDeniedAlert * feat(pages-and-resources): add permission tests for grading and pages/resources access * feat(pages-and-resources): implement advanced settings permissions and refactor related components * feat(pages-and-resources): update permissions terminology and enhance related components * feat(pages-and-resources): refactor isEditable context usage and improve accessibility attributes * test(pages-and-resources): remove unnecessary blank lines in OpenedXConfigForm and PageCard tests * refactor: change readOnly to isEditable to keep consistency * feat(pages-and-resources): remove isEditable prop from forms and related components * feat(pages-and-resources): remove isEditable prop and clean up related components * fix(lti-config-form): remove unnecessary blank line in consumerSecret input * feat: implement user permissions in menu items * feat(tests): update waffle flags for authorization in settings menu tests * refactor: remove advanced settings read only related things * feat: simplify permissions handling by removing advanced settings management * feat: replace useUserPermissions with useCourseUserPermissions for improved permission handling * refactor: address feedback
1 parent 95c7554 commit 7138dfb

19 files changed

Lines changed: 622 additions & 103 deletions

src/CourseAuthoringPage.test.tsx

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,14 +108,20 @@ describe('Course authoring page', () => {
108108

109109
axiosMock.onGet(
110110
`${courseAppsApiUrl}/${courseId}`,
111-
).reply(403);
111+
).reply(403, { response: { status: 403 } });
112112
await executeThunk(fetchCourseApps(courseId), store.dispatch);
113113
};
114114
test('renders PermissionDeniedAlert when courseAppsApiStatus is DENIED', async () => {
115115
mockPathname = '/editor/';
116116
await mockStoreDenied();
117117

118-
const wrapper = renderComponent(<CourseAuthoringPage />);
118+
// Test PagesAndResources (which has the PermissionDeniedAlert logic),
119+
// not CourseAuthoringPage which is just the layout wrapper
120+
const wrapper = renderComponent(
121+
<CourseAuthoringPage>
122+
<PagesAndResources />
123+
</CourseAuthoringPage>,
124+
);
119125
expect(await wrapper.findByTestId('permissionDeniedAlert')).toBeInTheDocument();
120126
});
121127
});

src/CourseAuthoringPage.tsx

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
11
import React, { useEffect } from 'react';
2-
import { useDispatch, useSelector } from 'react-redux';
2+
import { useDispatch } from 'react-redux';
33

44
import {
55
useLocation,
66
} from 'react-router-dom';
77
import { StudioFooterSlot } from '@edx/frontend-component-footer';
88
import Header from './header';
99
import NotFoundAlert from './generic/NotFoundAlert';
10-
import PermissionDeniedAlert from './generic/PermissionDeniedAlert';
1110
import { fetchOnlyStudioHomeData } from './studio-home/data/thunks';
12-
import { getCourseAppsApiStatus } from './pages-and-resources/data/selectors';
1311
import { RequestStatus } from './data/constants';
1412
import Loading from './generic/Loading';
1513
import { useCourseAuthoringContext } from './CourseAuthoringContext';
@@ -30,16 +28,12 @@ const CourseAuthoringPage = ({ children }: Props) => {
3028
const courseOrg = courseDetails?.org;
3129
const courseTitle = courseDetails?.name;
3230
const inProgress = courseDetailStatus === RequestStatus.IN_PROGRESS || courseDetailStatus === RequestStatus.PENDING;
33-
const courseAppsApiStatus = useSelector(getCourseAppsApiStatus);
3431
const { pathname } = useLocation();
3532
const isEditor = pathname.includes('/editor');
3633

3734
if (courseDetailStatus === RequestStatus.NOT_FOUND && !isEditor) {
3835
return <NotFoundAlert />;
3936
}
40-
if (courseAppsApiStatus === RequestStatus.DENIED) {
41-
return <PermissionDeniedAlert />;
42-
}
4337
return (
4438
<div>
4539
{

src/authz/constants.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,4 +24,7 @@ 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+
28+
VIEW_PAGES_AND_RESOURCES: 'courses.view_pages_and_resources',
29+
MANAGE_PAGES_AND_RESOURCES: 'courses.manage_pages_and_resources',
2730
};
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
import {
2+
getGradingPermissions,
3+
getPagesAndResourcesPermissions,
4+
getAdvancedSettingsPermissions,
5+
} from './permissionHelpers';
6+
import { COURSE_PERMISSIONS } from './constants';
7+
8+
describe('permissionHelpers', () => {
9+
const courseId = 'course-v1:org+course+run';
10+
11+
describe('getGradingPermissions', () => {
12+
it('returns VIEW and EDIT permissions with the correct actions and scope', () => {
13+
const result = getGradingPermissions(courseId);
14+
15+
expect(result.canViewGradingSettings.action).toBe(COURSE_PERMISSIONS.VIEW_GRADING_SETTINGS);
16+
expect(result.canViewGradingSettings.scope).toBe(courseId);
17+
expect(result.canEditGradingSettings.action).toBe(COURSE_PERMISSIONS.EDIT_GRADING_SETTINGS);
18+
expect(result.canEditGradingSettings.scope).toBe(courseId);
19+
});
20+
});
21+
22+
describe('getPagesAndResourcesPermissions', () => {
23+
it('returns VIEW and MANAGE permissions with the correct actions and scope', () => {
24+
const result = getPagesAndResourcesPermissions(courseId);
25+
26+
expect(result.canViewPagesAndResources.action).toBe(COURSE_PERMISSIONS.VIEW_PAGES_AND_RESOURCES);
27+
expect(result.canViewPagesAndResources.scope).toBe(courseId);
28+
expect(result.canManagePagesAndResources.action).toBe(COURSE_PERMISSIONS.MANAGE_PAGES_AND_RESOURCES);
29+
expect(result.canManagePagesAndResources.scope).toBe(courseId);
30+
});
31+
});
32+
33+
describe('getAdvancedSettingsPermissions', () => {
34+
it('returns MANAGE permission with the correct action and scope', () => {
35+
const result = getAdvancedSettingsPermissions(courseId);
36+
37+
expect(result.canManageAdvancedSettings.action).toBe(COURSE_PERMISSIONS.MANAGE_ADVANCED_SETTINGS);
38+
expect(result.canManageAdvancedSettings.scope).toBe(courseId);
39+
});
40+
41+
it('uses the provided courseId as scope', () => {
42+
const otherId = 'course-v1:another+test+run';
43+
const result = getAdvancedSettingsPermissions(otherId);
44+
45+
expect(result.canManageAdvancedSettings.scope).toBe(otherId);
46+
});
47+
});
48+
});

src/authz/permissionHelpers.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,3 +25,21 @@ export const getGradingPermissions = (courseId: string) => ({
2525
scope: courseId,
2626
},
2727
});
28+
29+
export const getPagesAndResourcesPermissions = (courseId: string) => ({
30+
canViewPagesAndResources: {
31+
action: COURSE_PERMISSIONS.VIEW_PAGES_AND_RESOURCES,
32+
scope: courseId,
33+
},
34+
canManagePagesAndResources: {
35+
action: COURSE_PERMISSIONS.MANAGE_PAGES_AND_RESOURCES,
36+
scope: courseId,
37+
},
38+
});
39+
40+
export const getAdvancedSettingsPermissions = (courseId: string) => ({
41+
canManageAdvancedSettings: {
42+
action: COURSE_PERMISSIONS.MANAGE_ADVANCED_SETTINGS,
43+
scope: courseId,
44+
},
45+
});

src/header/hooks.test.tsx

Lines changed: 95 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { getConfig, setConfig } from '@edx/frontend-platform';
33
import { renderHook, waitFor } from '@testing-library/react';
44
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
55
import { ReactNode } from 'react';
6-
import { useUserPermissions } from '@src/authz/data/apiHooks';
6+
import { useCourseUserPermissions } from '@src/authz/hooks';
77
import { mockWaffleFlags } from '@src/data/apiHooks.mock';
88
import messages from './messages';
99
import {
@@ -34,8 +34,9 @@ jest.mock('react-redux', () => ({
3434
useSelector: jest.fn(),
3535
}));
3636

37-
jest.mock('@src/authz/data/apiHooks', () => ({
38-
useUserPermissions: jest.fn(),
37+
jest.mock('@src/authz/hooks', () => ({
38+
...jest.requireActual('@src/authz/hooks'),
39+
useCourseUserPermissions: jest.fn(),
3940
}));
4041

4142
const createWrapper = () => {
@@ -56,6 +57,15 @@ const createWrapper = () => {
5657

5758
describe('header utils', () => {
5859
describe('getContentMenuItems', () => {
60+
beforeEach(() => {
61+
jest.mocked(useCourseUserPermissions).mockReturnValue({
62+
isLoading: false,
63+
isAuthzEnabled: false,
64+
canViewPagesAndResources: true,
65+
canManagePagesAndResources: true,
66+
} as ReturnType<typeof useCourseUserPermissions>);
67+
});
68+
5969
it('when video upload page enabled should include Video Uploads option', () => {
6070
jest.mocked(useSelector).mockReturnValue({
6171
librariesV2Enabled: false,
@@ -88,17 +98,49 @@ describe('header utils', () => {
8898
renderHook(() => useContentMenuItems('course-123'), { wrapper: createWrapper() }).result.current;
8999
expect(actualItems[1]).toEqual({ href: '/course/course-123/libraries', title: 'Library Updates' });
90100
});
101+
it('when authz enabled and user has canViewPagesAndResources should include pages and resources option', async () => {
102+
mockWaffleFlags({ enableAuthzCourseAuthoring: true });
103+
jest.mocked(useSelector).mockReturnValue({ librariesV2Enabled: false });
104+
jest.mocked(useCourseUserPermissions).mockReturnValue({
105+
isLoading: false,
106+
isAuthzEnabled: true,
107+
canViewPagesAndResources: true,
108+
canManagePagesAndResources: true,
109+
} as ReturnType<typeof useCourseUserPermissions>);
110+
const { result } = renderHook(() => useContentMenuItems('course-123'), { wrapper: createWrapper() });
111+
await waitFor(() => {
112+
expect(result.current.map((item) => item.title)).toContain('Pages & Resources');
113+
});
114+
});
115+
it('when authz enabled and user lacks canViewPagesAndResources should not include pages and resources option', async () => {
116+
mockWaffleFlags({ enableAuthzCourseAuthoring: true });
117+
jest.mocked(useSelector).mockReturnValue({ librariesV2Enabled: false });
118+
jest.mocked(useCourseUserPermissions).mockReturnValue({
119+
isLoading: false,
120+
isAuthzEnabled: true,
121+
canViewPagesAndResources: false,
122+
canManagePagesAndResources: false,
123+
} as ReturnType<typeof useCourseUserPermissions>);
124+
const { result } = renderHook(() => useContentMenuItems('course-123'), { wrapper: createWrapper() });
125+
await waitFor(() => {
126+
expect(result.current.map((item) => item.title)).not.toContain('Pages & Resources');
127+
});
128+
});
91129
});
92130

93131
describe('getSettingsMenuitems', () => {
94132
beforeEach(() => {
133+
mockWaffleFlags({ enableAuthzCourseAuthoring: false, useNewCertificatesPage: false });
95134
jest.mocked(useSelector).mockReturnValue({
96135
canAccessAdvancedSettings: true,
97136
});
98-
jest.mocked(useUserPermissions).mockReturnValue({
137+
jest.mocked(useCourseUserPermissions).mockReturnValue({
99138
isLoading: false,
100-
data: { canManageAdvancedSettings: true },
101-
} as any);
139+
isAuthzEnabled: false,
140+
canManageAdvancedSettings: true,
141+
canViewGradingSettings: true,
142+
canViewScheduleAndDetails: true,
143+
} as ReturnType<typeof useCourseUserPermissions>);
102144
});
103145

104146
it('when certificate page enabled should include certificates option', () => {
@@ -132,27 +174,29 @@ describe('header utils', () => {
132174
});
133175

134176
it('when the authz.enable_course_authoring flag is enabled and user has access to advanced settings should include advanced settings option', async () => {
135-
// Mock feature flag
136177
mockWaffleFlags({ enableAuthzCourseAuthoring: true });
137-
// Mock the useUserPermissions hook to return true for the authz.enable_course_authoring permission
138-
jest.mocked(useUserPermissions).mockReturnValue({
178+
jest.mocked(useCourseUserPermissions).mockReturnValue({
139179
isLoading: false,
140-
data: { canManageAdvancedSettings: true },
141-
} as any);
180+
isAuthzEnabled: true,
181+
canManageAdvancedSettings: true,
182+
canViewGradingSettings: true,
183+
canViewScheduleAndDetails: true,
184+
} as ReturnType<typeof useCourseUserPermissions>);
142185
const { result } = renderHook(() => useSettingMenuItems('course-123'), { wrapper: createWrapper() });
143186
await waitFor(() => {
144187
const actualItemsTitle = result.current.map((item) => item.title);
145188
expect(actualItemsTitle).toContain('Advanced Settings');
146189
});
147190
});
148191
it('when authz.enable_course_authoring flag is enabled and user has no access to advanced settings should not include advanced settings option', async () => {
149-
// Mock feature flag
150192
mockWaffleFlags({ enableAuthzCourseAuthoring: true });
151-
// Mock the useUserPermissions hook to return true for the authz.enable_course_authoring permission
152-
jest.mocked(useUserPermissions).mockReturnValue({
193+
jest.mocked(useCourseUserPermissions).mockReturnValue({
153194
isLoading: false,
154-
data: { canManageAdvancedSettings: false },
155-
} as any);
195+
isAuthzEnabled: true,
196+
canManageAdvancedSettings: false,
197+
canViewGradingSettings: true,
198+
canViewScheduleAndDetails: true,
199+
} as ReturnType<typeof useCourseUserPermissions>);
156200
const { result } = renderHook(() => useSettingMenuItems('course-123'), { wrapper: createWrapper() });
157201
await waitFor(() => {
158202
const actualItemsTitle = result.current.map((item) => item.title);
@@ -161,13 +205,11 @@ describe('header utils', () => {
161205
});
162206

163207
it('when authz.enable_course_authoring flag is enabled and the permission request is still loading, should not include advanced settings option', async () => {
164-
// Mock feature flag
165208
mockWaffleFlags({ enableAuthzCourseAuthoring: true });
166-
// Mock the useUserPermissions hook to return true for the authz.enable_course_authoring permission
167-
jest.mocked(useUserPermissions).mockReturnValue({
209+
jest.mocked(useCourseUserPermissions).mockReturnValue({
168210
isLoading: true,
169-
data: undefined,
170-
} as any);
211+
isAuthzEnabled: true,
212+
} as ReturnType<typeof useCourseUserPermissions>);
171213
const { result } = renderHook(() => useSettingMenuItems('course-123'), { wrapper: createWrapper() });
172214
await waitFor(() => {
173215
const actualItemsTitle = result.current.map((item) => item.title);
@@ -177,10 +219,13 @@ describe('header utils', () => {
177219

178220
it('when authz flag is enabled and user has canViewScheduleAndDetails should include schedule and details option', async () => {
179221
mockWaffleFlags({ enableAuthzCourseAuthoring: true });
180-
jest.mocked(useUserPermissions).mockReturnValue({
222+
jest.mocked(useCourseUserPermissions).mockReturnValue({
181223
isLoading: false,
182-
data: { canViewScheduleAndDetails: true },
183-
} as any);
224+
isAuthzEnabled: true,
225+
canViewScheduleAndDetails: true,
226+
canManageAdvancedSettings: true,
227+
canViewGradingSettings: true,
228+
} as ReturnType<typeof useCourseUserPermissions>);
184229
const { result } = renderHook(() => useSettingMenuItems('course-123'), { wrapper: createWrapper() });
185230
await waitFor(() => {
186231
const actualItemsTitle = result.current.map((item) => item.title);
@@ -190,10 +235,13 @@ describe('header utils', () => {
190235

191236
it('when authz flag is enabled and user lacks canViewScheduleAndDetails should not include schedule and details option', async () => {
192237
mockWaffleFlags({ enableAuthzCourseAuthoring: true });
193-
jest.mocked(useUserPermissions).mockReturnValue({
238+
jest.mocked(useCourseUserPermissions).mockReturnValue({
194239
isLoading: false,
195-
data: { canViewScheduleAndDetails: false },
196-
} as any);
240+
isAuthzEnabled: true,
241+
canViewScheduleAndDetails: false,
242+
canManageAdvancedSettings: true,
243+
canViewGradingSettings: true,
244+
} as ReturnType<typeof useCourseUserPermissions>);
197245
const { result } = renderHook(() => useSettingMenuItems('course-123'), { wrapper: createWrapper() });
198246
await waitFor(() => {
199247
const actualItemsTitle = result.current.map((item) => item.title);
@@ -203,10 +251,13 @@ describe('header utils', () => {
203251

204252
it('when authz flag is enabled and user has canViewGradingSettings should include grading option', async () => {
205253
mockWaffleFlags({ enableAuthzCourseAuthoring: true });
206-
jest.mocked(useUserPermissions).mockReturnValue({
254+
jest.mocked(useCourseUserPermissions).mockReturnValue({
207255
isLoading: false,
208-
data: { canViewGradingSettings: true },
209-
} as any);
256+
isAuthzEnabled: true,
257+
canViewGradingSettings: true,
258+
canManageAdvancedSettings: true,
259+
canViewScheduleAndDetails: true,
260+
} as ReturnType<typeof useCourseUserPermissions>);
210261
const { result } = renderHook(() => useSettingMenuItems('course-123'), { wrapper: createWrapper() });
211262
await waitFor(() => {
212263
const actualItemsTitle = result.current.map((item) => item.title);
@@ -216,10 +267,13 @@ describe('header utils', () => {
216267

217268
it('when authz flag is enabled and user lacks canViewGradingSettings should not include grading option', async () => {
218269
mockWaffleFlags({ enableAuthzCourseAuthoring: true });
219-
jest.mocked(useUserPermissions).mockReturnValue({
270+
jest.mocked(useCourseUserPermissions).mockReturnValue({
220271
isLoading: false,
221-
data: { canViewGradingSettings: false },
222-
} as any);
272+
isAuthzEnabled: true,
273+
canViewGradingSettings: false,
274+
canManageAdvancedSettings: true,
275+
canViewScheduleAndDetails: true,
276+
} as ReturnType<typeof useCourseUserPermissions>);
223277
const { result } = renderHook(() => useSettingMenuItems('course-123'), { wrapper: createWrapper() });
224278
await waitFor(() => {
225279
const actualItemsTitle = result.current.map((item) => item.title);
@@ -228,6 +282,14 @@ describe('header utils', () => {
228282
});
229283

230284
it('should include roles and permissions option', () => {
285+
mockWaffleFlags({ enableAuthzCourseAuthoring: true, useNewCertificatesPage: false });
286+
jest.mocked(useCourseUserPermissions).mockReturnValue({
287+
isLoading: false,
288+
isAuthzEnabled: true,
289+
canManageAdvancedSettings: true,
290+
canViewGradingSettings: true,
291+
canViewScheduleAndDetails: true,
292+
} as ReturnType<typeof useCourseUserPermissions>);
231293
setConfig({
232294
...getConfig(),
233295
ADMIN_CONSOLE_URL: 'http://admin-console.example.com',

0 commit comments

Comments
 (0)