Skip to content

Commit 5ccf39d

Browse files
rodmgwgurodmgwgu
authored
feat: Add validation for Advanced Settings permissions using openedx-authz (#2869)
* feat: Add validation for Advanced Settings permissions using openedx-authz * squash!: Increase test coverage * squash!: Fix lint issues * squash!: Validate advanced settings permission on HelpSidebar * squash!: Increase test coverage * squash!: Attend PR comments
1 parent 42f26e7 commit 5ccf39d

8 files changed

Lines changed: 265 additions & 27 deletions

File tree

‎src/advanced-settings/AdvancedSettings.jsx‎

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@ import {
66
import { CheckCircle, Info, Warning } from '@openedx/paragon/icons';
77
import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n';
88
import { useCourseAuthoringContext } from '@src/CourseAuthoringContext';
9+
import { useWaffleFlags } from '@src/data/apiHooks';
10+
import { useUserPermissions } from '@src/authz/data/apiHooks';
11+
import { COURSE_PERMISSIONS } from '@src/authz/constants';
12+
import PermissionDeniedAlert from 'CourseAuthoring/generic/PermissionDeniedAlert';
913
import Placeholder from '../editors/Placeholder';
1014

1115
import AlertProctoringError from '../generic/AlertProctoringError';
@@ -41,6 +45,15 @@ const AdvancedSettings = () => {
4145
const { courseId, courseDetails } = useCourseAuthoringContext();
4246
document.title = getPageHeadTitle(courseDetails?.name, intl.formatMessage(messages.headingTitle));
4347

48+
const waffleFlags = useWaffleFlags(courseId);
49+
const isAuthzEnabled = waffleFlags.enableAuthzCourseAuthoring;
50+
const { isLoading: isLoadingUserPermissions, data: userPermissions } = useUserPermissions({
51+
canManageAdvancedSettings: {
52+
action: COURSE_PERMISSIONS.MANAGE_ADVANCED_SETTINGS,
53+
scope: courseId,
54+
},
55+
}, isAuthzEnabled);
56+
4457
useEffect(() => {
4558
dispatch(fetchCourseAppSettings(courseId));
4659
dispatch(fetchProctoringExamErrors(courseId));
@@ -52,7 +65,7 @@ const AdvancedSettings = () => {
5265
const settingsWithSendErrors = useSelector(getSendRequestErrors) || {};
5366
const loadingSettingsStatus = useSelector(getLoadingStatus);
5467

55-
const isLoading = loadingSettingsStatus === RequestStatus.IN_PROGRESS;
68+
const isLoading = loadingSettingsStatus === RequestStatus.IN_PROGRESS || (isAuthzEnabled && isLoadingUserPermissions);
5669
const updateSettingsButtonState = {
5770
labels: {
5871
default: intl.formatMessage(messages.buttonSaveText),
@@ -128,6 +141,15 @@ const AdvancedSettings = () => {
128141
showSaveSettingsPrompt(true);
129142
};
130143

144+
// Show permission denied alert when authz is enabled and user doesn't have permission
145+
const authzIsEnabledAndNoPermission = isAuthzEnabled
146+
&& !isLoadingUserPermissions
147+
&& !userPermissions?.canManageAdvancedSettings;
148+
149+
if (authzIsEnabledAndNoPermission) {
150+
return <PermissionDeniedAlert />;
151+
}
152+
131153
return (
132154
<>
133155
<Container size="xl" className="advanced-settings px-4">
@@ -192,8 +214,8 @@ const AdvancedSettings = () => {
192214
defaultMessage="{visibility} deprecated settings"
193215
values={{
194216
visibility:
195-
showDeprecated ? intl.formatMessage(messages.deprecatedButtonHideText)
196-
: intl.formatMessage(messages.deprecatedButtonShowText),
217+
showDeprecated ? intl.formatMessage(messages.deprecatedButtonHideText)
218+
: intl.formatMessage(messages.deprecatedButtonShowText),
197219
}}
198220
/>
199221
</Button>

‎src/advanced-settings/AdvancedSettings.test.jsx‎

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
import { CourseAuthoringProvider } from '@src/CourseAuthoringContext';
2+
import { useUserPermissions } from '@src/authz/data/apiHooks';
3+
import { mockWaffleFlags } from '@src/data/apiHooks.mock';
24
import {
35
render as baseRender,
46
fireEvent,
@@ -21,11 +23,15 @@ const courseId = '123';
2123
jest.mock('react-textarea-autosize', () => jest.fn((props) => (
2224
<textarea
2325
{...props}
24-
onFocus={() => {}}
25-
onBlur={() => {}}
26+
onFocus={() => { }}
27+
onBlur={() => { }}
2628
/>
2729
)));
2830

31+
jest.mock('@src/authz/data/apiHooks', () => ({
32+
useUserPermissions: jest.fn(),
33+
}));
34+
2935
const render = () => baseRender(
3036
<CourseAuthoringProvider courseId={courseId}>
3137
<AdvancedSettings />
@@ -41,6 +47,11 @@ describe('<AdvancedSettings />', () => {
4147
axiosMock
4248
.onGet(`${getCourseAdvancedSettingsApiUrl(courseId)}?fetch_all=0`)
4349
.reply(200, advancedSettingsMock);
50+
51+
jest.mocked(useUserPermissions).mockReturnValue({
52+
isLoading: false,
53+
data: { canManageAdvancedSettings: true },
54+
});
4455
});
4556
it('should render without errors', async () => {
4657
const { getByText } = render();
@@ -144,4 +155,38 @@ describe('<AdvancedSettings />', () => {
144155
await executeThunk(updateCourseAppSetting(courseId, [3, 2, 1]), store.dispatch);
145156
expect(getByText('Your policy changes have been saved.')).toBeInTheDocument();
146157
});
158+
159+
it('should render without errors when authz.enable_course_authoring flag is enabled and the user is authorized', async () => {
160+
// Mock feature flag
161+
mockWaffleFlags({ enableAuthzCourseAuthoring: true });
162+
// Mock the useUserPermissions hook to return true for the authz.enable_course_authoring permission
163+
jest.mocked(useUserPermissions).mockReturnValue({
164+
isLoading: false,
165+
data: { canManageAdvancedSettings: true },
166+
});
167+
const { getByText } = render();
168+
await waitFor(() => {
169+
expect(getByText(messages.headingSubtitle.defaultMessage)).toBeInTheDocument();
170+
const advancedSettingsElement = getByText(messages.headingTitle.defaultMessage, {
171+
selector: 'h2.sub-header-title',
172+
});
173+
expect(advancedSettingsElement).toBeInTheDocument();
174+
expect(getByText(messages.policy.defaultMessage)).toBeInTheDocument();
175+
expect(getByText(/Do not modify these policies unless you are familiar with their purpose./i)).toBeInTheDocument();
176+
});
177+
});
178+
it('should show permission alert when authz.enable_course_authoring flag is enabled and the user is not authorized', async () => {
179+
// Mock feature flag
180+
mockWaffleFlags({ enableAuthzCourseAuthoring: true });
181+
// Mock the useUserPermissions hook to return true for the authz.enable_course_authoring permission
182+
jest.mocked(useUserPermissions).mockReturnValue({
183+
isLoading: false,
184+
data: { canManageAdvancedSettings: false },
185+
});
186+
const { getByTestId } = render();
187+
await waitFor(() => {
188+
const permissionAlert = getByTestId('permissionDeniedAlert');
189+
expect(permissionAlert).toBeInTheDocument();
190+
});
191+
});
147192
});

‎src/authz/constants.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,3 +14,7 @@ export const CONTENT_LIBRARY_PERMISSIONS = {
1414
MANAGE_LIBRARY_TEAM: 'content_libraries.manage_library_team',
1515
VIEW_LIBRARY_TEAM: 'content_libraries.view_library_team',
1616
};
17+
18+
export const COURSE_PERMISSIONS = {
19+
MANAGE_ADVANCED_SETTINGS: 'courses.manage_advanced_settings',
20+
};

‎src/data/api.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ export const waffleFlagDefaults = {
9292
useNewGroupConfigurationsPage: true,
9393
useReactMarkdownEditor: true,
9494
useVideoGalleryFlow: false,
95+
enableAuthzCourseAuthoring: false,
9596
} as const;
9697

9798
export type WaffleFlagName = keyof typeof waffleFlagDefaults;

‎src/generic/help-sidebar/HelpSidebar.jsx‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import classNames from 'classnames';
44
import { useIntl } from '@edx/frontend-platform/i18n';
55
import { getConfig } from '@edx/frontend-platform';
66

7+
import { useUserPermissions } from '@src/authz/data/apiHooks';
8+
import { COURSE_PERMISSIONS } from '@src/authz/constants';
79
import { useWaffleFlags } from '../../data/apiHooks';
810
import { otherLinkURLParams } from './constants';
911
import messages from './messages';
@@ -25,7 +27,7 @@ const HelpSidebar = ({
2527
scheduleAndDetails,
2628
groupConfigurations,
2729
} = otherLinkURLParams;
28-
const waffleFlags = useWaffleFlags();
30+
const waffleFlags = useWaffleFlags(courseId);
2931

3032
const showOtherLink = (params) => !pathname.includes(params);
3133
const generateLegacyURL = (urlParameter) => {
@@ -39,6 +41,26 @@ const HelpSidebar = ({
3941
const advancedSettingsDestination = generateLegacyURL(advancedSettings);
4042
const groupConfigurationsDestination = generateLegacyURL(groupConfigurations);
4143

44+
/*
45+
AuthZ for Course Authoring
46+
If authz.enable_course_authoring flag is enabled, validate permissions using AuthZ API.
47+
*/
48+
const isAuthzEnabled = waffleFlags.enableAuthzCourseAuthoring;
49+
const { isLoading: isLoadingUserPermissions, data: userPermissions } = useUserPermissions({
50+
canManageAdvancedSettings: {
51+
action: COURSE_PERMISSIONS.MANAGE_ADVANCED_SETTINGS,
52+
scope: courseId,
53+
},
54+
}, isAuthzEnabled);
55+
56+
// If it's still loading, don't show the Advanced Settings link, otherwise, use the permission to decide
57+
const authzCanManageAdvancedSettings = isLoadingUserPermissions
58+
? false
59+
: !!userPermissions?.canManageAdvancedSettings;
60+
61+
// When authz is enabled, use permission, otherwise it's always allowed (legacy behavior)
62+
const canManageAdvancedSettings = isAuthzEnabled ? authzCanManageAdvancedSettings : true;
63+
4264
return (
4365
<aside className={classNames('help-sidebar', className)}>
4466
<div className="help-sidebar-about">{children}</div>
@@ -90,7 +112,7 @@ const HelpSidebar = ({
90112
isNewPage={waffleFlags.useNewGroupConfigurationsPage}
91113
/>
92114
)}
93-
{showOtherLink(advancedSettings) && (
115+
{showOtherLink(advancedSettings) && canManageAdvancedSettings && (
94116
<HelpSidebarLink
95117
pathToPage={waffleFlags.useNewAdvancedSettingsPage
96118
? `/course/${courseId}/${advancedSettings}` : advancedSettingsDestination}

‎src/generic/help-sidebar/HelpSidebar.test.jsx‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,16 @@
11
// @ts-check
22

3+
import { waitFor } from '@testing-library/react';
4+
import { mockWaffleFlags } from '@src/data/apiHooks.mock';
5+
import { useUserPermissions } from '@src/authz/data/apiHooks';
36
import { initializeMocks, render } from '../../testUtils';
47
import messages from './messages';
58
import { HelpSidebar } from '.';
69

10+
jest.mock('@src/authz/data/apiHooks', () => ({
11+
useUserPermissions: jest.fn(),
12+
}));
13+
714
const mockPathname = '/foo-bar';
815

916
const renderHelpSidebar = (props) => render(
@@ -22,6 +29,11 @@ const props = {
2229
describe('HelpSidebar', () => {
2330
beforeEach(() => {
2431
initializeMocks();
32+
// @ts-ignore
33+
jest.mocked(useUserPermissions).mockReturnValue({
34+
isLoading: false,
35+
data: undefined,
36+
});
2537
});
2638

2739
it('renders children correctly', () => {
@@ -56,4 +68,41 @@ describe('HelpSidebar', () => {
5668
const { getByText } = renderHelpSidebar(initialProps);
5769
expect(getByText(messages.sidebarLinkToProctoredExamSettings.defaultMessage)).toBeTruthy();
5870
});
71+
72+
it('should render the advanced settings sidebar link when authz.enable_course_authoring flag is enabled and the user is authorized', async () => {
73+
// Mock feature flag
74+
mockWaffleFlags({ enableAuthzCourseAuthoring: true });
75+
// Mock the useUserPermissions hook to return true for the authz.enable_course_authoring permission
76+
// @ts-ignore
77+
jest.mocked(useUserPermissions).mockReturnValue({
78+
isLoading: false,
79+
data: { canManageAdvancedSettings: true },
80+
});
81+
const { queryByText } = renderHelpSidebar(props);
82+
await waitFor(() => expect(queryByText(messages.sidebarLinkToAdvancedSettings.defaultMessage)).toBeTruthy());
83+
});
84+
it('should not render the advanced settings sidebar link when authz.enable_course_authoring flag is enabled and the user is not authorized', async () => {
85+
// Mock feature flag
86+
mockWaffleFlags({ enableAuthzCourseAuthoring: true });
87+
// Mock the useUserPermissions hook to return true for the authz.enable_course_authoring permission
88+
// @ts-ignore
89+
jest.mocked(useUserPermissions).mockReturnValue({
90+
isLoading: false,
91+
data: { canManageAdvancedSettings: false },
92+
});
93+
const { queryByText } = renderHelpSidebar(props);
94+
await waitFor(() => expect(queryByText(messages.sidebarLinkToAdvancedSettings.defaultMessage)).toBeFalsy());
95+
});
96+
it('should not render the advanced settings sidebar link when authz.enable_course_authoring flag is enabled and the permissions are still loading', async () => {
97+
// Mock feature flag
98+
mockWaffleFlags({ enableAuthzCourseAuthoring: true });
99+
// Mock the useUserPermissions hook to return true for the authz.enable_course_authoring permission
100+
// @ts-ignore
101+
jest.mocked(useUserPermissions).mockReturnValue({
102+
isLoading: true,
103+
data: undefined,
104+
});
105+
const { queryByText } = renderHelpSidebar(props);
106+
await waitFor(() => expect(queryByText(messages.sidebarLinkToAdvancedSettings.defaultMessage)).toBeFalsy());
107+
});
59108
});

0 commit comments

Comments
 (0)