Skip to content

Commit 3f531d5

Browse files
feat: permission validations from authz for files page (#2941)
* feat: adding permission validations from authz for files page for view, create, edit and delete * test: adding tests to increase code coverage * refactor(authz): remove duplicate hook and gate lock/unlock behind canEditFiles - Remove useUserPermissionsWithAuthzCourse in favor of useCourseUserPermissions which provides the same functionality with better generic typing - Migrate all consumers (CourseFilesTable, FilesPage, header hooks) to use useCourseUserPermissions with flat destructuring - Hide Lock/Unlock option in FileMenu, MoreInfoColumn, and FileInfoModalSidebar when canEditFiles is false (course_auditor should not see lock/unlock) - Add unit tests for lock/unlock visibility based on permissions - Fix clipboard mock in tests using Object.defineProperty - Update FilesPage.test.jsx mocks to match new flat return shape * test: remove deprecated useNewCertificatesPage and useNewVideoUploadsPage flag references These flags are being removed in #3066 and should be treated as always true. Remove test scenarios for the disabled state and clean up mock overrides that set them to false.
1 parent 1d83fef commit 3f531d5

21 files changed

Lines changed: 1038 additions & 246 deletions

src/authz/constants.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,4 +29,8 @@ export const COURSE_PERMISSIONS = {
2929

3030
VIEW_PAGES_AND_RESOURCES: 'courses.view_pages_and_resources',
3131
MANAGE_PAGES_AND_RESOURCES: 'courses.manage_pages_and_resources',
32+
VIEW_FILES: 'courses.view_files',
33+
CREATE_FILES: 'courses.create_files',
34+
DELETE_FILES: 'courses.delete_files',
35+
EDIT_FILES: 'courses.edit_files',
3236
};

src/authz/hooks.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ describe('useCourseUserPermissions', () => {
4949
expect(result.current.canEdit).toBe(false);
5050
});
5151

52-
it('returns isLoading=true and no permission keys while authz permissions are loading', () => {
52+
it('returns isLoading=true and permissions as false while authz permissions are loading', () => {
5353
mockWaffleFlags({ enableAuthzCourseAuthoring: true });
5454
jest.mocked(useUserPermissions).mockReturnValue({
5555
isLoading: true,

src/authz/hooks.test.tsx

Lines changed: 0 additions & 135 deletions
This file was deleted.

src/authz/permissionHelpers.test.ts

Lines changed: 100 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,15 @@ import {
33
getPagesAndResourcesPermissions,
44
getAdvancedSettingsPermissions,
55
getCourseUpdatesPermissions,
6+
getFilesPermissions,
67
} from './permissionHelpers';
78
import { COURSE_PERMISSIONS } from './constants';
89

910
const courseId = 'course-v1:org+course+run';
1011

1112
describe('permissionHelpers', () => {
13+
const mockCourseId = 'course-v1:edX+DemoX+Demo_Course';
14+
1215
describe('getCourseUpdatesPermissions', () => {
1316
it('should return correct permission structure for course updates operations', () => {
1417
const result = getCourseUpdatesPermissions(courseId);
@@ -25,51 +28,118 @@ describe('permissionHelpers', () => {
2528
});
2629
});
2730

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+
describe('getFilesPermissions', () => {
32+
it('should return correct permission structure for file operations', () => {
33+
const result = getFilesPermissions(mockCourseId);
3134

32-
Object.values(result).forEach(permission => {
33-
expect(permission.scope).toBe(customCourseId);
35+
expect(result).toEqual({
36+
canViewFiles: {
37+
action: COURSE_PERMISSIONS.VIEW_FILES,
38+
scope: mockCourseId,
39+
},
40+
canCreateFiles: {
41+
action: COURSE_PERMISSIONS.CREATE_FILES,
42+
scope: mockCourseId,
43+
},
44+
canDeleteFiles: {
45+
action: COURSE_PERMISSIONS.DELETE_FILES,
46+
scope: mockCourseId,
47+
},
48+
canEditFiles: {
49+
action: COURSE_PERMISSIONS.EDIT_FILES,
50+
scope: mockCourseId,
51+
},
52+
});
53+
});
54+
55+
it('should use the provided courseId as scope for all permissions', () => {
56+
const customCourseId = 'course-v1:TestOrg+TestCourse+2024';
57+
const result = getFilesPermissions(customCourseId);
58+
59+
Object.values(result).forEach(permission => {
60+
expect(permission.scope).toBe(customCourseId);
61+
});
3462
});
3563
});
36-
});
3764

38-
describe('getGradingPermissions', () => {
39-
it('returns VIEW and EDIT permissions with the correct actions and scope', () => {
40-
const result = getGradingPermissions(courseId);
65+
describe('getGradingPermissions', () => {
66+
it('returns VIEW and EDIT permissions with the correct actions and scope', () => {
67+
const result = getGradingPermissions(courseId);
4168

42-
expect(result.canViewGradingSettings.action).toBe(COURSE_PERMISSIONS.VIEW_GRADING_SETTINGS);
43-
expect(result.canViewGradingSettings.scope).toBe(courseId);
44-
expect(result.canEditGradingSettings.action).toBe(COURSE_PERMISSIONS.EDIT_GRADING_SETTINGS);
45-
expect(result.canEditGradingSettings.scope).toBe(courseId);
69+
expect(result.canViewGradingSettings.action).toBe(COURSE_PERMISSIONS.VIEW_GRADING_SETTINGS);
70+
expect(result.canViewGradingSettings.scope).toBe(courseId);
71+
expect(result.canEditGradingSettings.action).toBe(COURSE_PERMISSIONS.EDIT_GRADING_SETTINGS);
72+
expect(result.canEditGradingSettings.scope).toBe(courseId);
73+
});
74+
});
75+
76+
describe('getPagesAndResourcesPermissions', () => {
77+
it('returns VIEW and MANAGE permissions with the correct actions and scope', () => {
78+
const result = getPagesAndResourcesPermissions(courseId);
79+
80+
expect(result.canViewPagesAndResources.action).toBe(COURSE_PERMISSIONS.VIEW_PAGES_AND_RESOURCES);
81+
expect(result.canViewPagesAndResources.scope).toBe(courseId);
82+
expect(result.canManagePagesAndResources.action).toBe(COURSE_PERMISSIONS.MANAGE_PAGES_AND_RESOURCES);
83+
expect(result.canManagePagesAndResources.scope).toBe(courseId);
84+
});
85+
});
86+
87+
describe('getAdvancedSettingsPermissions', () => {
88+
it('returns MANAGE permission with the correct action and scope', () => {
89+
const result = getAdvancedSettingsPermissions(courseId);
90+
91+
expect(result.canManageAdvancedSettings.action).toBe(COURSE_PERMISSIONS.MANAGE_ADVANCED_SETTINGS);
92+
expect(result.canManageAdvancedSettings.scope).toBe(courseId);
93+
});
94+
95+
it('uses the provided courseId as scope', () => {
96+
const otherId = 'course-v1:another+test+run';
97+
const result = getAdvancedSettingsPermissions(otherId);
98+
99+
expect(result.canManageAdvancedSettings.scope).toBe(otherId);
100+
});
46101
});
47-
});
48102

49-
describe('getPagesAndResourcesPermissions', () => {
50-
it('returns VIEW and MANAGE permissions with the correct actions and scope', () => {
51-
const result = getPagesAndResourcesPermissions(courseId);
103+
it('should use correct COURSE_PERMISSIONS constants for each action', () => {
104+
const result = getFilesPermissions(mockCourseId);
52105

53-
expect(result.canViewPagesAndResources.action).toBe(COURSE_PERMISSIONS.VIEW_PAGES_AND_RESOURCES);
54-
expect(result.canViewPagesAndResources.scope).toBe(courseId);
55-
expect(result.canManagePagesAndResources.action).toBe(COURSE_PERMISSIONS.MANAGE_PAGES_AND_RESOURCES);
56-
expect(result.canManagePagesAndResources.scope).toBe(courseId);
106+
expect(result.canViewFiles.action).toBe(COURSE_PERMISSIONS.VIEW_FILES);
107+
expect(result.canCreateFiles.action).toBe(COURSE_PERMISSIONS.CREATE_FILES);
108+
expect(result.canDeleteFiles.action).toBe(COURSE_PERMISSIONS.DELETE_FILES);
109+
expect(result.canEditFiles.action).toBe(COURSE_PERMISSIONS.EDIT_FILES);
57110
});
58111
});
59112

60-
describe('getAdvancedSettingsPermissions', () => {
61-
it('returns MANAGE permission with the correct action and scope', () => {
62-
const result = getAdvancedSettingsPermissions(courseId);
113+
describe('getGradingPermissions', () => {
114+
it('should return correct permission structure for grading operations', () => {
115+
const result = getGradingPermissions(mockCourseId);
116+
117+
expect(result).toEqual({
118+
canViewGradingSettings: {
119+
action: COURSE_PERMISSIONS.VIEW_GRADING_SETTINGS,
120+
scope: mockCourseId,
121+
},
122+
canEditGradingSettings: {
123+
action: COURSE_PERMISSIONS.EDIT_GRADING_SETTINGS,
124+
scope: mockCourseId,
125+
},
126+
});
127+
});
128+
129+
it('should use the provided courseId as scope for all permissions', () => {
130+
const customCourseId = 'course-v1:TestOrg+TestCourse+2024';
131+
const result = getGradingPermissions(customCourseId);
63132

64-
expect(result.canManageAdvancedSettings.action).toBe(COURSE_PERMISSIONS.MANAGE_ADVANCED_SETTINGS);
65-
expect(result.canManageAdvancedSettings.scope).toBe(courseId);
133+
Object.values(result).forEach(permission => {
134+
expect(permission.scope).toBe(customCourseId);
135+
});
66136
});
67137

68-
it('uses the provided courseId as scope', () => {
69-
const otherId = 'course-v1:another+test+run';
70-
const result = getAdvancedSettingsPermissions(otherId);
138+
it('should use correct COURSE_PERMISSIONS constants for each action', () => {
139+
const result = getGradingPermissions(mockCourseId);
71140

72-
expect(result.canManageAdvancedSettings.scope).toBe(otherId);
141+
expect(result.canViewGradingSettings.action).toBe(COURSE_PERMISSIONS.VIEW_GRADING_SETTINGS);
142+
expect(result.canEditGradingSettings.action).toBe(COURSE_PERMISSIONS.EDIT_GRADING_SETTINGS);
73143
});
74144
});
75145
});

src/authz/permissionHelpers.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,3 +54,21 @@ export const getAdvancedSettingsPermissions = (courseId: string) => ({
5454
scope: courseId,
5555
},
5656
});
57+
export const getFilesPermissions = (courseId: string) => ({
58+
canViewFiles: {
59+
action: COURSE_PERMISSIONS.VIEW_FILES,
60+
scope: courseId,
61+
},
62+
canCreateFiles: {
63+
action: COURSE_PERMISSIONS.CREATE_FILES,
64+
scope: courseId,
65+
},
66+
canDeleteFiles: {
67+
action: COURSE_PERMISSIONS.DELETE_FILES,
68+
scope: courseId,
69+
},
70+
canEditFiles: {
71+
action: COURSE_PERMISSIONS.EDIT_FILES,
72+
scope: courseId,
73+
},
74+
});

src/files-and-videos/files-page/CourseFilesTable.tsx

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ import { getFileSizeToClosestByte } from '@src/utils';
2828
import React from 'react';
2929
import { useDispatch, useSelector } from 'react-redux';
3030
import { useParams } from 'react-router-dom';
31+
import { useCourseUserPermissions } from '@src/authz/hooks';
32+
import { getFilesPermissions } from '@src/authz/permissionHelpers';
3133

3234
export const CourseFilesTable = () => {
3335
const intl = useIntl();
@@ -40,6 +42,12 @@ export const CourseFilesTable = () => {
4042
errors: errorMessages,
4143
} = useSelector((state: DeprecatedReduxState) => state.assets);
4244

45+
const {
46+
canCreateFiles,
47+
canDeleteFiles,
48+
canEditFiles,
49+
} = useCourseUserPermissions(courseId, getFilesPermissions(courseId));
50+
4351
const handleErrorReset = (error) => dispatch(resetErrors(error));
4452
const handleDeleteFile = (id) => dispatch(deleteAssetFile(courseId, id));
4553
const handleDownloadFile = (selectedRows) =>
@@ -69,6 +77,7 @@ export const CourseFilesTable = () => {
6977
FileInfoModalSidebar({
7078
asset,
7179
handleLockedAsset: handleLockFile,
80+
canLockFile: canEditFiles,
7281
});
7382

7483
const assets = useModels('assets', assetIds);
@@ -180,6 +189,11 @@ export const CourseFilesTable = () => {
180189
thumbnailPreview,
181190
infoModalSidebar,
182191
files: assets,
192+
permissions: {
193+
canCreateFiles,
194+
canDeleteFiles,
195+
canEditFiles,
196+
},
183197
}}
184198
/>
185199
<FileValidationModal {...{ handleFileOverwrite }} />

0 commit comments

Comments
 (0)