Skip to content

Commit 24e28e3

Browse files
committed
fix: validate user manage team action and scopes
1 parent e3d90d8 commit 24e28e3

6 files changed

Lines changed: 189 additions & 29 deletions

File tree

src/authz-module/components/AddRoleButton.test.tsx

Lines changed: 62 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@ import { screen } from '@testing-library/react';
22
import userEvent from '@testing-library/user-event';
33
import { useNavigate } from 'react-router-dom';
44
import { initializeMockApp } from '@edx/frontend-platform/testing';
5-
import { renderWrapper } from '@src/setupTest';
5+
import { renderWithAllProviders } from '@src/setupTest';
6+
import { useValidateUserPermissionsNonSuspense } from '@src/data/hooks';
7+
import { CONTENT_COURSE_PERMISSIONS, CONTENT_LIBRARY_PERMISSIONS } from '../roles-permissions';
68
import AddRoleButton from './AddRoleButton';
79

810
// Mock react-router-dom navigation
@@ -11,6 +13,20 @@ jest.mock('react-router-dom', () => ({
1113
useNavigate: jest.fn(),
1214
}));
1315

16+
jest.mock('@src/data/hooks', () => ({
17+
...jest.requireActual('@src/data/hooks'),
18+
useValidateUserPermissionsNonSuspense: jest.fn(),
19+
}));
20+
21+
const mockUseValidatePermissions = useValidateUserPermissionsNonSuspense as jest.Mock;
22+
const allowAllPermissions = {
23+
data: [
24+
{ action: CONTENT_LIBRARY_PERMISSIONS.MANAGE_LIBRARY_TEAM, allowed: true },
25+
{ action: CONTENT_COURSE_PERMISSIONS.MANAGE_COURSE_TEAM, allowed: true },
26+
],
27+
isLoading: false,
28+
};
29+
1430
describe('AddRoleButton', () => {
1531
const mockNavigate = jest.fn();
1632

@@ -26,6 +42,7 @@ describe('AddRoleButton', () => {
2642

2743
beforeEach(() => {
2844
(useNavigate as jest.Mock).mockReturnValue(mockNavigate);
45+
mockUseValidatePermissions.mockReturnValue(allowAllPermissions);
2946
});
3047

3148
afterEach(() => {
@@ -34,21 +51,21 @@ describe('AddRoleButton', () => {
3451

3552
describe('rendering', () => {
3653
it('renders the assign role button with correct text', () => {
37-
renderWrapper(<AddRoleButton />);
54+
renderWithAllProviders(<AddRoleButton />);
3855

3956
const button = screen.getByRole('button', { name: /assign role/i });
4057
expect(button).toBeInTheDocument();
4158
});
4259

4360
it('displays the plus icon', () => {
44-
renderWrapper(<AddRoleButton />);
61+
renderWithAllProviders(<AddRoleButton />);
4562

4663
const button = screen.getByRole('button', { name: /assign role/i });
4764
expect(button.querySelector('svg')).toBeInTheDocument();
4865
});
4966

5067
it('renders correctly when presetUsername is provided', () => {
51-
renderWrapper(<AddRoleButton presetUsername="testuser123" />);
68+
renderWithAllProviders(<AddRoleButton presetUsername="testuser123" />);
5269

5370
const button = screen.getByRole('button', { name: /assign role/i });
5471
expect(button).toBeInTheDocument();
@@ -58,7 +75,7 @@ describe('AddRoleButton', () => {
5875
describe('navigation behavior', () => {
5976
it('navigates to assign role page without username when clicked', async () => {
6077
const user = userEvent.setup();
61-
renderWrapper(<AddRoleButton />);
78+
renderWithAllProviders(<AddRoleButton />);
6279

6380
const button = screen.getByRole('button', { name: /assign role/i });
6481
await user.click(button);
@@ -70,7 +87,7 @@ describe('AddRoleButton', () => {
7087
it('navigates to assign role page with username query parameter when presetUsername is provided', async () => {
7188
const user = userEvent.setup();
7289
const presetUsername = 'john.doe';
73-
renderWrapper(<AddRoleButton presetUsername={presetUsername} />);
90+
renderWithAllProviders(<AddRoleButton presetUsername={presetUsername} />);
7491

7592
const button = screen.getByRole('button', { name: /assign role/i });
7693
await user.click(button);
@@ -82,7 +99,7 @@ describe('AddRoleButton', () => {
8299
it('handles special characters in presetUsername correctly', async () => {
83100
const user = userEvent.setup();
84101
const presetUsername = 'user@example.com';
85-
renderWrapper(<AddRoleButton presetUsername={presetUsername} />);
102+
renderWithAllProviders(<AddRoleButton presetUsername={presetUsername} />);
86103

87104
const button = screen.getByRole('button', { name: /assign role/i });
88105
await user.click(button);
@@ -95,7 +112,7 @@ describe('AddRoleButton', () => {
95112
describe('user interactions', () => {
96113
it('responds to keyboard navigation', async () => {
97114
const user = userEvent.setup();
98-
renderWrapper(<AddRoleButton />);
115+
renderWithAllProviders(<AddRoleButton />);
99116

100117
const button = screen.getByRole('button', { name: /assign role/i });
101118

@@ -108,7 +125,7 @@ describe('AddRoleButton', () => {
108125

109126
it('responds to spacebar activation', async () => {
110127
const user = userEvent.setup();
111-
renderWrapper(<AddRoleButton />);
128+
renderWithAllProviders(<AddRoleButton />);
112129

113130
const button = screen.getByRole('button', { name: /assign role/i });
114131
button.focus();
@@ -119,7 +136,7 @@ describe('AddRoleButton', () => {
119136

120137
it('handles multiple clicks gracefully', async () => {
121138
const user = userEvent.setup();
122-
renderWrapper(<AddRoleButton presetUsername="testuser" />);
139+
renderWithAllProviders(<AddRoleButton presetUsername="testuser" />);
123140

124141
const button = screen.getByRole('button', { name: /assign role/i });
125142

@@ -131,4 +148,39 @@ describe('AddRoleButton', () => {
131148
expect(mockNavigate).toHaveBeenCalledWith('/authz/assign-role?users=testuser');
132149
});
133150
});
151+
152+
describe('permission gating', () => {
153+
it('does not render the button when the user cannot manage any team', () => {
154+
mockUseValidatePermissions.mockReturnValue({
155+
data: [
156+
{ action: CONTENT_LIBRARY_PERMISSIONS.MANAGE_LIBRARY_TEAM, allowed: false },
157+
{ action: CONTENT_COURSE_PERMISSIONS.MANAGE_COURSE_TEAM, allowed: false },
158+
],
159+
isLoading: false,
160+
});
161+
renderWithAllProviders(<AddRoleButton />);
162+
163+
expect(screen.queryByRole('button', { name: /assign role/i })).not.toBeInTheDocument();
164+
});
165+
166+
it('renders the button when at least one team-management permission is allowed', () => {
167+
mockUseValidatePermissions.mockReturnValue({
168+
data: [
169+
{ action: CONTENT_LIBRARY_PERMISSIONS.MANAGE_LIBRARY_TEAM, allowed: false },
170+
{ action: CONTENT_COURSE_PERMISSIONS.MANAGE_COURSE_TEAM, allowed: true },
171+
],
172+
isLoading: false,
173+
});
174+
renderWithAllProviders(<AddRoleButton />);
175+
176+
expect(screen.getByRole('button', { name: /assign role/i })).toBeInTheDocument();
177+
});
178+
179+
it('does not render the button while permissions are loading', () => {
180+
mockUseValidatePermissions.mockReturnValue({ data: undefined, isLoading: true });
181+
renderWithAllProviders(<AddRoleButton />);
182+
183+
expect(screen.queryByRole('button', { name: /assign role/i })).not.toBeInTheDocument();
184+
});
185+
});
134186
});

src/authz-module/components/AddRoleButton.tsx

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import { useNavigate } from 'react-router-dom';
55

66
import baseMessages from '@src/authz-module/messages';
77
import { buildWizardPath } from '@src/authz-module/constants';
8+
import { useValidateUserPermissionsNonSuspense } from '@src/data/hooks';
9+
import { CONTENT_COURSE_PERMISSIONS, CONTENT_LIBRARY_PERMISSIONS } from '../roles-permissions';
810

911
interface AddRoleButtonProps {
1012
presetUsername?: string;
@@ -15,19 +17,26 @@ const AddRoleButton = ({ presetUsername, from }: AddRoleButtonProps) => {
1517
const intl = useIntl();
1618
const navigate = useNavigate();
1719

20+
const { data: permissionValidationResponse } = useValidateUserPermissionsNonSuspense([
21+
{ action: CONTENT_LIBRARY_PERMISSIONS.MANAGE_LIBRARY_TEAM },
22+
{ action: CONTENT_COURSE_PERMISSIONS.MANAGE_COURSE_TEAM },
23+
]);
24+
const canAssignRole = permissionValidationResponse?.some((permission) => permission.allowed);
25+
1826
const handleClick = () => {
1927
const path = buildWizardPath({ from, users: presetUsername });
2028
navigate(path);
2129
};
2230

2331
return (
24-
<Button
25-
iconBefore={Plus}
26-
onClick={handleClick}
27-
>
28-
{intl.formatMessage(baseMessages['authz.management.assign.role.title'])}
29-
</Button>
30-
);
32+
canAssignRole ? (
33+
<Button
34+
iconBefore={Plus}
35+
onClick={handleClick}
36+
>
37+
{intl.formatMessage(baseMessages['authz.management.assign.role.title'])}
38+
</Button>
39+
) : null);
3140
};
3241

3342
export default AddRoleButton;

src/authz-module/role-assignation-wizard/AssignRoleWizard.test.tsx

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,11 @@ import {
77
useValidateUsers, useAssignTeamMembersRole, useScopes, useOrgs,
88
} from '../data/hooks';
99
import useScopePermissions from './hooks/useScopePermissions';
10+
import { courseRolesMetadata, libraryRolesMetadata } from '../roles-permissions';
1011
import AssignRoleWizard from './AssignRoleWizard';
1112

13+
const allRolesMetadata = [...courseRolesMetadata, ...libraryRolesMetadata];
14+
1215
jest.mock('@edx/frontend-platform/auth', () => ({
1316
getAuthenticatedUser: jest.fn(),
1417
}));
@@ -76,7 +79,7 @@ const oneOrg = [
7679

7780
const renderWizard = (props = {}) => renderWrapper(
7881
<ToastManagerProvider>
79-
<AssignRoleWizard onClose={jest.fn()} {...props} />
82+
<AssignRoleWizard onClose={jest.fn()} roles={allRolesMetadata} {...props} />
8083
</ToastManagerProvider>,
8184
);
8285

@@ -117,6 +120,12 @@ describe('AssignRoleWizard — Step 1', () => {
117120
expect(getNextButton()).toBeDisabled();
118121
});
119122

123+
it('renders no role options and keeps Next disabled when roles is empty', () => {
124+
renderWizard({ roles: [] });
125+
expect(screen.queryByRole('radio')).not.toBeInTheDocument();
126+
expect(getNextButton()).toBeDisabled();
127+
});
128+
120129
it('selecting a different role replaces the previous selection', async () => {
121130
const user = userEvent.setup();
122131
renderWizard();

src/authz-module/role-assignation-wizard/AssignRoleWizard.tsx

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,10 @@ import { RoleMetadata } from '@src/types';
1010
import { useToastManager } from '@src/components/ToastManager/ToastManagerContext';
1111
import SelectUsersAndRoleStep from './components/SelectUsersAndRoleStep';
1212
import DefineApplicationScopeStep from './components/DefineApplicationScopeStep';
13-
import { libraryRolesMetadata } from '../roles-permissions/library/constants';
14-
import { courseRolesMetadata } from '../roles-permissions/course/constants';
1513
import { useValidateUsers, useAssignTeamMembersRole } from '../data/hooks';
1614
import messages from './messages';
1715
import { formatRoleAssignmentError } from './utils';
1816

19-
const allRolesMetadata = [...courseRolesMetadata, ...libraryRolesMetadata];
20-
2117
const STEPS = {
2218
SELECT_USERS_AND_ROLE: 'select-users-and-role',
2319
DEFINE_APPLICATION_SCOPE: 'define-application-scope',
@@ -28,8 +24,6 @@ type StepKey = typeof STEPS[keyof typeof STEPS];
2824
interface AssignRoleWizardProps {
2925
onClose: () => void;
3026
initialUsers?: string;
31-
/** Filtered role list. Defaults to all roles (course + library). Pass a subset
32-
* once a permission-lookup API is available to hide groups the caller cannot assign. */
3327
roles?: RoleMetadata[];
3428
}
3529

@@ -48,7 +42,7 @@ const getInitialState = (initialUsers: string) => ({
4842
});
4943

5044
const AssignRoleWizard = ({
51-
onClose, initialUsers = '', roles = allRolesMetadata,
45+
onClose, initialUsers = '', roles = [],
5246
}: AssignRoleWizardProps) => {
5347
const intl = useIntl();
5448
const { showToast, showErrorToast } = useToastManager();

src/authz-module/role-assignation-wizard/AssignRoleWizardPage.test.tsx

Lines changed: 83 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,15 @@
11
import { screen } from '@testing-library/react';
22
import userEvent from '@testing-library/user-event';
3-
import { renderWrapper } from '@src/setupTest';
3+
import { renderWithAllProviders } from '@src/setupTest';
44
import { ToastManagerProvider } from '@src/components/ToastManager/ToastManagerContext';
5+
import { useValidateUserPermissionsNonSuspense } from '@src/data/hooks';
56
import { useValidateUsers } from '../data/hooks';
7+
import {
8+
CONTENT_COURSE_PERMISSIONS,
9+
CONTENT_LIBRARY_PERMISSIONS,
10+
courseRolesMetadata,
11+
libraryRolesMetadata,
12+
} from '../roles-permissions';
613
import AssignRoleWizardPage from './AssignRoleWizardPage';
714

815
jest.mock('@edx/frontend-platform/logging');
@@ -27,7 +34,20 @@ jest.mock('@edx/frontend-component-header', () => ({
2734
StudioHeader: () => null,
2835
}));
2936

37+
jest.mock('@src/data/hooks', () => ({
38+
...jest.requireActual('@src/data/hooks'),
39+
useValidateUserPermissionsNonSuspense: jest.fn(),
40+
}));
41+
3042
const mockUseValidateUsers = useValidateUsers as jest.Mock;
43+
const mockUseValidatePermissions = useValidateUserPermissionsNonSuspense as jest.Mock;
44+
const allowAllPermissions = {
45+
data: [
46+
{ action: CONTENT_LIBRARY_PERMISSIONS.MANAGE_LIBRARY_TEAM, allowed: true },
47+
{ action: CONTENT_COURSE_PERMISSIONS.MANAGE_COURSE_TEAM, allowed: true },
48+
],
49+
isLoading: false,
50+
};
3151

3252
const setupMocks = ({ users = '', from = '' } = {}) => {
3353
const { useSearchParams, useNavigate } = jest.requireMock('react-router-dom');
@@ -40,7 +60,7 @@ const setupMocks = ({ users = '', from = '' } = {}) => {
4060
return { navigate };
4161
};
4262

43-
const renderPage = () => renderWrapper(
63+
const renderPage = () => renderWithAllProviders(
4464
<ToastManagerProvider>
4565
<AssignRoleWizardPage />
4666
</ToastManagerProvider>,
@@ -53,6 +73,7 @@ describe('AssignRoleWizardPage', () => {
5373
mutateAsync: jest.fn(),
5474
isPending: false,
5575
});
76+
mockUseValidatePermissions.mockReturnValue(allowAllPermissions);
5677
});
5778

5879
it('renders the page with the wizard and title', () => {
@@ -107,4 +128,64 @@ describe('AssignRoleWizardPage', () => {
107128
await user.click(screen.getByRole('button', { name: /Cancel/i }));
108129
expect(navigate).toHaveBeenCalledWith('/authz/team');
109130
});
131+
132+
describe('assignable roles', () => {
133+
// Each entry maps an allowed permission to the role metadata the wizard should
134+
// surface for it. Add a row here as more scopes/roles are introduced.
135+
const scopeRoles = [
136+
{ action: CONTENT_LIBRARY_PERMISSIONS.MANAGE_LIBRARY_TEAM, roles: libraryRolesMetadata },
137+
{ action: CONTENT_COURSE_PERMISSIONS.MANAGE_COURSE_TEAM, roles: courseRolesMetadata },
138+
];
139+
const allRoles = scopeRoles.flatMap(({ roles }) => roles);
140+
141+
it('shows the roles for every allowed scope', () => {
142+
setupMocks();
143+
renderPage();
144+
allRoles.forEach((role) => {
145+
expect(screen.getByText(role.name)).toBeInTheDocument();
146+
});
147+
});
148+
149+
it.each(scopeRoles)('shows only the roles for the allowed scope %#', ({ action, roles }) => {
150+
mockUseValidatePermissions.mockReturnValue({
151+
data: scopeRoles.map((scope) => ({ action: scope.action, allowed: scope.action === action })),
152+
isLoading: false,
153+
});
154+
setupMocks();
155+
renderPage();
156+
157+
roles.forEach((role) => {
158+
expect(screen.getByText(role.name)).toBeInTheDocument();
159+
});
160+
allRoles
161+
.filter((role) => !roles.includes(role))
162+
.forEach((role) => {
163+
expect(screen.queryByText(role.name)).not.toBeInTheDocument();
164+
});
165+
});
166+
167+
it('shows no roles when no scope is allowed', () => {
168+
mockUseValidatePermissions.mockReturnValue({
169+
data: scopeRoles.map(({ action }) => ({ action, allowed: false })),
170+
isLoading: false,
171+
});
172+
setupMocks();
173+
renderPage();
174+
allRoles.forEach((role) => {
175+
expect(screen.queryByText(role.name)).not.toBeInTheDocument();
176+
});
177+
});
178+
179+
it('ignores allowed permissions whose action is not a known role scope', () => {
180+
mockUseValidatePermissions.mockReturnValue({
181+
data: [{ action: 'some.unrelated.permission', allowed: true }],
182+
isLoading: false,
183+
});
184+
setupMocks();
185+
renderPage();
186+
allRoles.forEach((role) => {
187+
expect(screen.queryByText(role.name)).not.toBeInTheDocument();
188+
});
189+
});
190+
});
110191
});

0 commit comments

Comments
 (0)