diff --git a/packages/@n8n/permissions/src/roles/__tests__/custom-role-scopes.test.ts b/packages/@n8n/permissions/src/roles/__tests__/custom-role-scopes.test.ts index 6df63f5fbb4..4ed5c9eaff0 100644 --- a/packages/@n8n/permissions/src/roles/__tests__/custom-role-scopes.test.ts +++ b/packages/@n8n/permissions/src/roles/__tests__/custom-role-scopes.test.ts @@ -4,6 +4,7 @@ import { GLOBAL_CUSTOM_ROLE_SCOPES, PROJECT_CUSTOM_ROLE_SCOPES, } from '@/roles/custom-role-scopes.ee'; +import { GLOBAL_MEMBER_SCOPES } from '@/roles/scopes/global-scopes.ee'; import { ALL_SCOPES } from '@/scope-information'; describe('custom role scope whitelists', () => { @@ -53,20 +54,64 @@ describe('custom role scope whitelists', () => { expect(bundle).toContain('chatHub:message'); }); - it('includes AI Assistant and n8n Agent scopes in the settings.Manage bundle', () => { - const bundle = GLOBAL_CUSTOM_ROLE_SCOPE_GROUPS.settings.Manage; + it('exposes AI Assistant and n8n Agent scopes as their own use/manage options', () => { + const { 'AiAssistant use': use, 'AiAssistant manage': manage } = + GLOBAL_CUSTOM_ROLE_SCOPE_GROUPS.settings; - expect(bundle).toContain('aiAssistant:manage'); - expect(bundle).toContain('instanceAi:manage'); - expect(bundle).toContain('instanceAi:message'); + expect(use).toContain('instanceAi:message'); + expect(use).toContain('instanceAi:gateway'); + expect(manage).toContain('aiAssistant:manage'); + expect(manage).toContain('instanceAi:manage'); + expect(manage).toContain('instanceAi:message'); + expect(manage).toContain('instanceAi:gateway'); }); - it('includes instance-level MCP scopes in the settings.Manage bundle', () => { + it('"AiAssistant use" matches GLOBAL_MEMBER_SCOPES\' instanceAi:* grants exactly', () => { + // Member's baseline AI Assistant access is `instanceAi:message` + + // `instanceAi:gateway` (computer-use gateway pairing). A custom role built + // to mirror Member must get both, or it ends up strictly weaker than Member. + const use = GLOBAL_CUSTOM_ROLE_SCOPE_GROUPS.settings['AiAssistant use']; + const memberInstanceAiScopes = GLOBAL_MEMBER_SCOPES.filter((s) => s.startsWith('instanceAi:')); + expect(new Set(use)).toEqual(new Set(memberInstanceAiScopes)); + }); + + it('exposes instance-level MCP scopes as their own use/manage options', () => { + const { 'Mcp use': use, 'Mcp manage': manage } = GLOBAL_CUSTOM_ROLE_SCOPE_GROUPS.settings; + + expect(use).toContain('mcp:oauth'); + expect(use).toContain('mcpApiKey:create'); + expect(use).toContain('mcpApiKey:rotate'); + expect(manage).toContain('mcp:manage'); + expect(manage).toContain('mcp:oauth'); + expect(manage).toContain('mcpApiKey:create'); + expect(manage).toContain('mcpApiKey:rotate'); + }); + + it('includes MCP and AI Assistant scopes in the general settings.Manage bundle, as a superset of their own options', () => { const bundle = GLOBAL_CUSTOM_ROLE_SCOPE_GROUPS.settings.Manage; - expect(bundle).toContain('mcp:manage'); - expect(bundle).toContain('mcp:oauth'); - expect(bundle).toContain('mcpApiKey:create'); - expect(bundle).toContain('mcpApiKey:rotate'); + for (const scope of [ + 'mcp:manage', + 'mcp:oauth', + 'mcpApiKey:create', + 'mcpApiKey:rotate', + 'aiAssistant:manage', + 'instanceAi:manage', + 'instanceAi:message', + 'instanceAi:gateway', + ]) { + expect(bundle).toContain(scope); + } + }); + + it('exposes "Users: View" as exactly user:list, matching GLOBAL_MEMBER_SCOPES', () => { + // "Users: View" is granted to every instance role by default (see + // instanceRoleScopes.ts). It must never exceed what the built-in Member + // role already has, or a custom role mirroring Member ends up more + // privileged than Member itself + expect(GLOBAL_CUSTOM_ROLE_SCOPE_GROUPS.user.View).toEqual(['user:list']); + for (const scope of GLOBAL_CUSTOM_ROLE_SCOPE_GROUPS.user.View) { + expect(GLOBAL_MEMBER_SCOPES).toContain(scope); + } }); }); diff --git a/packages/@n8n/permissions/src/roles/custom-role-scopes.ee.ts b/packages/@n8n/permissions/src/roles/custom-role-scopes.ee.ts index f18a836709c..b09fa4aac1d 100644 --- a/packages/@n8n/permissions/src/roles/custom-role-scopes.ee.ts +++ b/packages/@n8n/permissions/src/roles/custom-role-scopes.ee.ts @@ -73,8 +73,12 @@ type InstanceScopeGroups = { export const GLOBAL_CUSTOM_ROLE_SCOPE_GROUPS = { settings: { - // Grants access to every instance Settings page. Each scope below gates a - // specific page; granting all of them lets the role see and manage all of them. + // Grants access to every instance Settings page, including MCP and AI + // Assistant management. MCP and AI Assistant also have their own narrower + // use/manage options below so a role can be given just those without the + // rest of instance Settings — Manage's bundle is a strict superset of all + // four, so checking Manage checks them too, and unchecking any one of them + // drops Manage out of the fully-checked state. Manage: [ 'securitySettings:manage', // Security & Policies 'credentialResolver:read', // Resolvers (requires the full CRUD set) @@ -103,13 +107,31 @@ export const GLOBAL_CUSTOM_ROLE_SCOPE_GROUPS = { 'aiAssistant:manage', // AI Assistant 'instanceAi:manage', 'instanceAi:message', + 'instanceAi:gateway', // computer-use gateway pairing 'mcp:manage', // Instance-level MCP 'mcp:oauth', // MCP OAuth clients 'mcpApiKey:create', // MCP personal API key 'mcpApiKey:rotate', ], + 'Mcp use': ['mcp:oauth', 'mcpApiKey:create', 'mcpApiKey:rotate'], + 'Mcp manage': ['mcp:manage', 'mcp:oauth', 'mcpApiKey:create', 'mcpApiKey:rotate'], + 'AiAssistant use': ['instanceAi:message', 'instanceAi:gateway'], + 'AiAssistant manage': [ + 'aiAssistant:manage', + 'instanceAi:manage', + 'instanceAi:message', + 'instanceAi:gateway', + ], }, user: { + // Lets a role look up other users (e.g. the member-search box a Project Admin + // uses to add users to a project) without granting the Settings > Users page + // or the ability to change anyone's role. Only `user:list` — the scope the + // search endpoint actually checks — matches GLOBAL_MEMBER_SCOPES exactly, so + // a custom role built to mirror Member never ends up with more than Member. + // `user:read` only gates the Public API's single-user lookup, which Member + // doesn't have either, so it stays out of this bundle. + View: ['user:list'], Manage: [ 'user:create', 'user:update', diff --git a/packages/frontend/@n8n/i18n/src/locales/en.json b/packages/frontend/@n8n/i18n/src/locales/en.json index 92a8e6cd4b9..38dff4ec195 100644 --- a/packages/frontend/@n8n/i18n/src/locales/en.json +++ b/packages/frontend/@n8n/i18n/src/locales/en.json @@ -3013,8 +3013,19 @@ "instanceRoles.option.manageAll": "Manage others", "instanceRoles.option.manageProjectRoles": "Manage project roles", "instanceRoles.option.manageAllRoles": "Manage all roles (instance and project)", + "instanceRoles.option.manageAllSettings": "Manage all settings", + "instanceRoles.option.mcpUse": "MCP use", + "instanceRoles.option.mcpManage": "MCP manage", + "instanceRoles.option.aiAssistantUse": "AI assistant use", + "instanceRoles.option.aiAssistantManage": "AI assistant manage", "instanceRoles.option.includedIn": "Included in {option}", + "instanceRoles.option.mandatory": "Every role can look up other users", "instanceRoles.description.settings.manage": "View and change instance-wide settings", + "instanceRoles.description.settings.mcpUse": "Connect MCP clients and agents to the instance", + "instanceRoles.description.settings.mcpManage": "Enable or disable instance-level MCP and manage which workflows and agents are exposed", + "instanceRoles.description.settings.aiAssistantUse": "Use the AI assistant", + "instanceRoles.description.settings.aiAssistantManage": "Enable or disable the AI assistant and manage its settings", + "instanceRoles.description.user.view": "Look up other users, e.g. to search for and add project members", "instanceRoles.description.user.manage": "Invite, remove, and update users across the instance", "instanceRoles.description.role.manageProjectRoles": "Create, edit, and delete custom project roles only", "instanceRoles.description.role.manage": "Create, edit, and delete all custom roles (instance and project).", diff --git a/packages/frontend/editor-ui/src/features/roles/composables/useRoleEditorForm.ts b/packages/frontend/editor-ui/src/features/roles/composables/useRoleEditorForm.ts index 84e17a01136..226c200d9c0 100644 --- a/packages/frontend/editor-ui/src/features/roles/composables/useRoleEditorForm.ts +++ b/packages/frontend/editor-ui/src/features/roles/composables/useRoleEditorForm.ts @@ -25,10 +25,18 @@ export interface UseRoleEditorFormOptions { defaultScopes?: () => string[]; /** * Filter applied to every scope set entering the form (default seed, fetched role, - * reset). Keeps the editor — and anything it saves — limited to scopes it exposes, - * so a role loaded with non-assignable scopes is sanitized rather than forwarded. + * reset) and to the persisted snapshot. Keeps the editor — and anything it saves — + * limited to scopes it exposes, so a role loaded with non-assignable scopes is + * sanitized rather than forwarded. Must only strip; adding scopes here would + * rewrite `initialState` and hide the difference from what is stored. */ filterScopes?: (scopes: string[]) => string[]; + /** + * Applied to form scopes after `filterScopes`, but not to `initialState`. + * Inject required scopes that should persist on the next save without treating + * a legacy role as already up to date. + */ + ensureScopes?: (scopes: string[]) => string[]; /** Error message shown when the initial role fetch fails. */ fetchError: string; } @@ -38,6 +46,7 @@ export function useRoleEditorForm({ viewRoute, defaultScopes, filterScopes, + ensureScopes, fetchError, }: UseRoleEditorFormOptions) { const rolesStore = useRolesStore(); @@ -60,10 +69,13 @@ export function useRoleEditorForm({ const sanitizeScopes = (scopes: string[]): string[] => filterScopes ? filterScopes(scopes) : scopes; + const formScopes = (scopes: string[]): string[] => + ensureScopes ? ensureScopes(sanitizeScopes(scopes)) : sanitizeScopes(scopes); + const defaultForm = (): RoleEditorForm => ({ displayName: '', description: '', - scopes: sanitizeScopes(defaultScopes?.() ?? []), + scopes: formScopes(defaultScopes?.() ?? []), }); const initialState = ref(); @@ -77,13 +89,14 @@ export function useRoleEditorForm({ try { const role = await rolesStore.fetchRoleBySlug({ slug }); - const scopes = sanitizeScopes(role.scopes); - // Sanitize initialState too so the form isn't falsely dirty on load. - initialState.value = structuredClone({ ...role, scopes }); + // Snapshot is stripped only. Required scopes go on the form so a stored + // role missing them stays unsaved until the next save. + const persistedScopes = sanitizeScopes(role.scopes); + initialState.value = structuredClone({ ...role, scopes: persistedScopes }); return { displayName: role.displayName, description: role.description, - scopes, + scopes: formScopes(role.scopes), }; } catch (error) { showError(error, fetchError); @@ -149,7 +162,7 @@ export function useRoleEditorForm({ ? { displayName: payload.displayName, description: payload.description, - scopes: sanitizeScopes(payload.scopes), + scopes: formScopes(payload.scopes), } : defaultForm(); } diff --git a/packages/frontend/editor-ui/src/features/roles/instance/InstanceRoleView.test.ts b/packages/frontend/editor-ui/src/features/roles/instance/InstanceRoleView.test.ts index 7480d119e5e..fc5554b1485 100644 --- a/packages/frontend/editor-ui/src/features/roles/instance/InstanceRoleView.test.ts +++ b/packages/frontend/editor-ui/src/features/roles/instance/InstanceRoleView.test.ts @@ -42,7 +42,7 @@ const mockCustomRole = { displayName: 'Support', slug: 'support', description: 'A custom instance role', - scopes: ['user:read', 'user:list'], + scopes: ['user:list'], licensed: true, systemRole: false, roleType: 'global' as const, @@ -95,7 +95,8 @@ describe('InstanceRoleView', () => { expect(rolesStore.createRole).toHaveBeenCalledWith({ displayName: 'Support', description: '', - scopes: [], + // "Users: View" is mandatory on every instance role — see instanceRoleScopes.ts. + scopes: ['user:list'], roleType: 'global', }); }); @@ -189,7 +190,7 @@ describe('InstanceRoleView', () => { expect(rolesStore.updateRole).toHaveBeenCalledWith('support', { displayName: 'Support 2', description: 'A custom instance role', - scopes: ['user:read', 'user:list'], + scopes: ['user:list'], }); }); expect(mockShowMessage).toHaveBeenCalledWith({ @@ -225,7 +226,7 @@ describe('InstanceRoleView', () => { expect(rolesStore.updateRole).toHaveBeenCalledWith('support', { displayName: 'Support 2', description: 'A custom instance role', - scopes: ['user:read', 'user:list'], + scopes: ['user:list'], }); }); }); @@ -301,6 +302,66 @@ describe('InstanceRoleView', () => { await waitFor(() => expect(rolesStore.updateRole).toHaveBeenCalled()); }); + it('does not mark a role unsaved when Users: View is already stored', async () => { + rolesStore.fetchRoleBySlug.mockResolvedValue(mockCustomRole); + + const { getByRole, container } = renderComponent({ props: { roleSlug: 'support' } }); + + await waitFor(() => { + const { nameInput } = getFormElements(container); + expect(nameInput?.value).toBe('Support'); + }); + + expect(getByRole('button', { name: 'Save' })).toBeDisabled(); + }); + + it('does not mark a role unsaved when only non-editor scopes were stripped', async () => { + rolesStore.fetchRoleBySlug.mockResolvedValue({ + ...mockCustomRole, + scopes: ['user:list', 'workflow:read'], + }); + + const { getByRole, container } = renderComponent({ props: { roleSlug: 'support' } }); + + await waitFor(() => { + const { nameInput } = getFormElements(container); + expect(nameInput?.value).toBe('Support'); + }); + + expect(getByRole('button', { name: 'Save' })).toBeDisabled(); + }); + + it('enables save when a stored role is missing mandatory Users: View scopes', async () => { + const legacyRole = { + ...mockCustomRole, + scopes: ['tag:read', 'tag:list', 'tag:create', 'tag:update', 'tag:delete'], + }; + rolesStore.fetchRoleBySlug.mockResolvedValue(legacyRole); + rolesStore.updateRole.mockResolvedValueOnce({ + ...legacyRole, + scopes: [...legacyRole.scopes, 'user:list'], + }); + + const { getByRole, getByTestId } = renderComponent({ props: { roleSlug: 'support' } }); + + await waitFor(() => { + expect(getByTestId('scope-option-user-view').getAttribute('aria-checked')).toBe('true'); + }); + + const save = getByRole('button', { name: 'Save' }); + expect(save).toBeEnabled(); + + await userEvent.click(save); + + await waitFor(() => { + expect(rolesStore.updateRole).toHaveBeenCalledWith('support', { + displayName: 'Support', + description: 'A custom instance role', + scopes: ['tag:read', 'tag:list', 'tag:create', 'tag:update', 'tag:delete', 'user:list'], + }); + }); + }); + it('falls back to the count from page load when the pre-save fetch fails', async () => { rolesStore.fetchRoleBySlug .mockResolvedValueOnce({ ...mockCustomRole, usedByUsers: 8 }) diff --git a/packages/frontend/editor-ui/src/features/roles/instance/InstanceRoleView.vue b/packages/frontend/editor-ui/src/features/roles/instance/InstanceRoleView.vue index 283ceccb629..5ab04ebb4fc 100644 --- a/packages/frontend/editor-ui/src/features/roles/instance/InstanceRoleView.vue +++ b/packages/frontend/editor-ui/src/features/roles/instance/InstanceRoleView.vue @@ -17,7 +17,7 @@ import { useRoleEditorForm } from '../composables/useRoleEditorForm'; import InstanceRoleAssignmentsTab from './InstanceRoleAssignmentsTab.vue'; import DeleteInstanceRoleModal from './components/DeleteInstanceRoleModal.vue'; import ScopeGroupSelector from './components/ScopeGroupSelector.vue'; -import { ALL_INSTANCE_SCOPES } from './instanceRoleScopes'; +import { ALL_INSTANCE_SCOPES, withMandatoryInstanceScopes } from './instanceRoleScopes'; const rolesStore = useRolesStore(); const router = useRouter(); @@ -50,6 +50,7 @@ const { viewRoute: VIEWS.INSTANCE_ROLE_VIEW, filterScopes: (scopes) => scopes.filter((s) => (ALL_INSTANCE_SCOPES as readonly string[]).includes(s)), + ensureScopes: withMandatoryInstanceScopes, fetchError: i18n.baseText('roles.instance.action.fetch.error'), }); @@ -92,8 +93,10 @@ function setPreset(slug: string) { // Only keep scopes the editor knows about; system roles may carry internal scopes // (e.g. chatHub:*) that the UI doesn't expose and shouldn't be silently forwarded. - form.value.scopes = structuredClone(toRaw(preset.scopes)).filter((s) => - (ALL_INSTANCE_SCOPES as readonly string[]).includes(s), + form.value.scopes = withMandatoryInstanceScopes( + structuredClone(toRaw(preset.scopes)).filter((s) => + (ALL_INSTANCE_SCOPES as readonly string[]).includes(s), + ), ); } diff --git a/packages/frontend/editor-ui/src/features/roles/instance/components/ScopeGroupSelector.test.ts b/packages/frontend/editor-ui/src/features/roles/instance/components/ScopeGroupSelector.test.ts index 32da7dcc526..e4a74cfb594 100644 --- a/packages/frontend/editor-ui/src/features/roles/instance/components/ScopeGroupSelector.test.ts +++ b/packages/frontend/editor-ui/src/features/roles/instance/components/ScopeGroupSelector.test.ts @@ -179,4 +179,108 @@ describe('ScopeGroupSelector', () => { ); }); }); + + describe('settings "Manage all settings" select-all behaviour', () => { + it('checks MCP and AI Assistant use/manage when "Manage all settings" is toggled on', async () => { + const { getByTestId, emitted } = renderComponent(ScopeGroupSelector, { + props: { modelValue: [] }, + }); + + await userEvent.click(getByTestId('scope-option-settings-manage')); + + await waitFor(() => expect(emitted()['update:modelValue']).toBeTruthy()); + const [scopes] = emitted()['update:modelValue'][0] as [string[]]; + expect(scopes).toEqual( + expect.arrayContaining([ + 'mcp:manage', + 'mcp:oauth', + 'mcpApiKey:create', + 'mcpApiKey:rotate', + 'aiAssistant:manage', + 'instanceAi:manage', + 'instanceAi:message', + ]), + ); + }); + + it('keeps all four MCP/AI Assistant checkboxes enabled (not implied) when "Manage all settings" is checked', () => { + const { getByTestId } = renderComponent(ScopeGroupSelector, { + props: { modelValue: [...INSTANCE_SCOPE_GROUPS.settings.Manage] }, + }); + for (const testId of [ + 'scope-option-settings-mcp-use', + 'scope-option-settings-mcp-manage', + 'scope-option-settings-aiassistant-use', + 'scope-option-settings-aiassistant-manage', + ]) { + const checkbox = getByTestId(testId); + expect(checkbox.getAttribute('aria-checked')).toBe('true'); + expect(checkbox.hasAttribute('disabled')).toBe(false); + } + }); + + it('unchecking "Mcp use" (not just "Mcp manage") turns "Manage all settings" off', async () => { + const { getByTestId, emitted, rerender } = renderComponent(ScopeGroupSelector, { + props: { modelValue: [...INSTANCE_SCOPE_GROUPS.settings.Manage] }, + }); + + await userEvent.click(getByTestId('scope-option-settings-mcp-use')); + + await waitFor(() => expect(emitted()['update:modelValue']).toBeTruthy()); + const [scopes] = emitted()['update:modelValue'][0] as [string[]]; + expect(scopes).not.toContain('mcp:oauth'); + expect(scopes).toContain('securitySettings:manage'); + + // v-model doesn't auto-sync in tests — re-render with the emitted value to + // prove the effect the title claims: the checkbox itself loses its checked state. + await rerender({ modelValue: scopes }); + expect(getByTestId('scope-option-settings-manage').getAttribute('aria-checked')).not.toBe( + 'true', + ); + }); + + it('unchecking "Mcp manage" turns "Manage all settings" off while "Manage all settings" was checked', async () => { + const { getByTestId, emitted, rerender } = renderComponent(ScopeGroupSelector, { + props: { modelValue: [...INSTANCE_SCOPE_GROUPS.settings.Manage] }, + }); + + await userEvent.click(getByTestId('scope-option-settings-mcp-manage')); + + await waitFor(() => expect(emitted()['update:modelValue']).toBeTruthy()); + const [scopes] = emitted()['update:modelValue'][0] as [string[]]; + expect(scopes).not.toContain('mcp:manage'); + expect(scopes).toContain('securitySettings:manage'); + + await rerender({ modelValue: scopes }); + expect(getByTestId('scope-option-settings-manage').getAttribute('aria-checked')).not.toBe( + 'true', + ); + }); + }); + + describe('mandatory "Users: View" option', () => { + // The caller (InstanceRoleView's `withMandatoryInstanceScopes`) is what + // guarantees these scopes are always in `modelValue` — the selector itself + // stays a pure function of its props, same as every other option. + const withUserView = [...INSTANCE_SCOPE_GROUPS.user.View]; + + it('renders checked and disabled', () => { + const { getByTestId } = renderComponent(ScopeGroupSelector, { + props: { modelValue: withUserView }, + }); + const userView = getByTestId('scope-option-user-view'); + expect(userView.getAttribute('aria-checked')).toBe('true'); + expect(userView.hasAttribute('disabled')).toBe(true); + }); + + it('does not emit an update when clicked', async () => { + const { getByTestId, emitted } = renderComponent(ScopeGroupSelector, { + props: { modelValue: withUserView }, + }); + + await userEvent.click(getByTestId('scope-option-user-view')); + + expect(emitted()['update:modelValue']).toBeFalsy(); + }); + }); }); diff --git a/packages/frontend/editor-ui/src/features/roles/instance/components/ScopeGroupSelector.vue b/packages/frontend/editor-ui/src/features/roles/instance/components/ScopeGroupSelector.vue index 184c0fd9260..17d51d8d260 100644 --- a/packages/frontend/editor-ui/src/features/roles/instance/components/ScopeGroupSelector.vue +++ b/packages/frontend/editor-ui/src/features/roles/instance/components/ScopeGroupSelector.vue @@ -8,8 +8,10 @@ import { SUPERSEDED_BY, getEscalationWarningKey, isOptionImplied, + isOptionMandatory, resolveOptionState, toggleOptionInGroup, + type InstanceResource, type InstanceScopeOption, } from '../instanceRoleScopes'; @@ -48,12 +50,21 @@ function impliedTooltip(option: InstanceScopeOption, groupOptions: InstanceScope /** * Tooltip shown for a permission option. When the option is implied by another * (e.g. "Manage own" under a checked "Manage all") the "Included in …" note - * takes precedence; otherwise it explains what the permission grants. + * takes precedence; a mandatory option (granted to every role, see + * `isOptionMandatory`) explains why it can't be turned off; otherwise it + * explains what the permission grants. */ -function optionTooltip(option: InstanceScopeOption, groupOptions: InstanceScopeOption[]): string { +function optionTooltip( + resource: InstanceResource, + option: InstanceScopeOption, + groupOptions: InstanceScopeOption[], +): string { if (isOptionImplied(option, groupOptions, props.modelValue)) { return impliedTooltip(option, groupOptions); } + if (isOptionMandatory(resource, option)) { + return i18n.baseText('instanceRoles.option.mandatory'); + } return option.descriptionKey ? i18n.baseText(option.descriptionKey) : ''; } @@ -76,8 +87,8 @@ function onToggle(option: InstanceScopeOption, groupOptions: InstanceScopeOption diff --git a/packages/frontend/editor-ui/src/features/roles/instance/instanceRoleScopes.test.ts b/packages/frontend/editor-ui/src/features/roles/instance/instanceRoleScopes.test.ts index db297e8485c..e498655be4b 100644 --- a/packages/frontend/editor-ui/src/features/roles/instance/instanceRoleScopes.test.ts +++ b/packages/frontend/editor-ui/src/features/roles/instance/instanceRoleScopes.test.ts @@ -7,8 +7,10 @@ import { getOptionState, getEscalationWarningKey, isOptionImplied, + isOptionMandatory, resolveOptionState, toggleOptionInGroup, + withMandatoryInstanceScopes, } from './instanceRoleScopes'; const ALL_SCOPES_SET = new Set(ALL_SCOPES as string[]); @@ -81,7 +83,13 @@ describe('instanceRoleScopes config', () => { expect(Object.keys(INSTANCE_SCOPE_GROUPS.tag)).toEqual(['Manage']); expect(Object.keys(INSTANCE_SCOPE_GROUPS.role)).toEqual(['Manage project roles', 'Manage']); expect(Object.keys(INSTANCE_SCOPE_GROUPS.project)).toEqual(['Create']); - expect(Object.keys(INSTANCE_SCOPE_GROUPS.settings)).toEqual(['Manage']); + expect(Object.keys(INSTANCE_SCOPE_GROUPS.settings)).toEqual([ + 'Manage', + 'Mcp use', + 'Mcp manage', + 'AiAssistant use', + 'AiAssistant manage', + ]); }); }); }); @@ -236,6 +244,10 @@ describe('getEscalationWarningKey', () => { it('returns undefined for role when only the non-escalating role:read scope is present', () => { expect(getEscalationWarningKey('role', ['role:read'])).toBeUndefined(); }); + + it('returns undefined for user when only the non-escalating View scope (user:list) is present', () => { + expect(getEscalationWarningKey('user', ['user:list'])).toBeUndefined(); + }); }); describe('toggleOptionInGroup', () => { @@ -293,4 +305,84 @@ describe('toggleOptionInGroup', () => { toggleOptionInGroup(input, manageAll, apiKeyGroup.options); expect(input).toEqual([...manageAll.scopes]); }); + + describe('settings "Manage all settings" acts as a select-all over MCP/AI Assistant', () => { + const settingsGroup = INSTANCE_SCOPE_GROUP_LIST.find((g) => g.resource === 'settings')!; + const manageAllSettings = settingsGroup.options.find((o) => o.key === 'Manage')!; + const mcpUse = settingsGroup.options.find((o) => o.key === 'Mcp use')!; + const mcpManage = settingsGroup.options.find((o) => o.key === 'Mcp manage')!; + const aiAssistantUse = settingsGroup.options.find((o) => o.key === 'AiAssistant use')!; + const aiAssistantManage = settingsGroup.options.find((o) => o.key === 'AiAssistant manage')!; + + it('checking "Manage all settings" checks MCP and AI Assistant use/manage too', () => { + const scopes = toggleOptionInGroup([], manageAllSettings, settingsGroup.options); + expect(getOptionState(scopes, mcpUse.scopes)).toBe('checked'); + expect(getOptionState(scopes, mcpManage.scopes)).toBe('checked'); + expect(getOptionState(scopes, aiAssistantUse.scopes)).toBe('checked'); + expect(getOptionState(scopes, aiAssistantManage.scopes)).toBe('checked'); + }); + + it('all four MCP/AI Assistant options stay independently toggleable while "Manage all settings" is checked (none implied/disabled by it)', () => { + // Unlike apiKey's "Manage own"/"Manage all" tiering, none of these four + // are superseded by another option in this group — "Manage all settings" + // checks them via plain scope-superset arithmetic, not implication, so + // unchecking any one of the four must stay a single, direct click. + const scopes = toggleOptionInGroup([], manageAllSettings, settingsGroup.options); + expect(isOptionImplied(mcpUse, settingsGroup.options, scopes)).toBe(false); + expect(isOptionImplied(mcpManage, settingsGroup.options, scopes)).toBe(false); + expect(isOptionImplied(aiAssistantUse, settingsGroup.options, scopes)).toBe(false); + expect(isOptionImplied(aiAssistantManage, settingsGroup.options, scopes)).toBe(false); + }); + + it('unchecking "Mcp manage" while "Manage all settings" is checked drops it out of the checked state', () => { + const fullyChecked = toggleOptionInGroup([], manageAllSettings, settingsGroup.options); + const afterUncheck = toggleOptionInGroup(fullyChecked, mcpManage, settingsGroup.options); + expect(resolveOptionState(manageAllSettings, settingsGroup.options, afterUncheck)).not.toBe( + 'checked', + ); + // The rest of the "Manage all settings" bundle survives the uncheck. + expect(afterUncheck).toContain('securitySettings:manage'); + }); + + it('unchecking "AiAssistant use" while "Manage all settings" is checked drops it out of the checked state', () => { + const fullyChecked = toggleOptionInGroup([], manageAllSettings, settingsGroup.options); + const afterUncheck = toggleOptionInGroup(fullyChecked, aiAssistantUse, settingsGroup.options); + expect(resolveOptionState(manageAllSettings, settingsGroup.options, afterUncheck)).not.toBe( + 'checked', + ); + }); + }); +}); + +describe('mandatory instance options', () => { + const userGroup = INSTANCE_SCOPE_GROUP_LIST.find((g) => g.resource === 'user')!; + const userView = userGroup.options.find((o) => o.key === 'View')!; + const userManage = userGroup.options.find((o) => o.key === 'Manage')!; + + it('flags "Users: View" as mandatory and every other option as not', () => { + expect(isOptionMandatory('user', userView)).toBe(true); + expect(isOptionMandatory('user', userManage)).toBe(false); + + for (const group of INSTANCE_SCOPE_GROUP_LIST) { + for (const option of group.options) { + if (group.resource === 'user' && option.key === 'View') continue; + expect(isOptionMandatory(group.resource, option)).toBe(false); + } + } + }); + + it('withMandatoryInstanceScopes adds "Users: View" scopes to an empty list', () => { + expect(new Set(withMandatoryInstanceScopes([]))).toEqual(new Set(userView.scopes)); + }); + + it('withMandatoryInstanceScopes does not duplicate scopes already present', () => { + const withDuplicate = withMandatoryInstanceScopes(['user:list', 'tag:read']); + expect(withDuplicate.filter((s) => s === 'user:list')).toHaveLength(1); + expect(withDuplicate).toEqual(expect.arrayContaining(['user:list', 'tag:read'])); + }); + + it('withMandatoryInstanceScopes preserves unrelated scopes untouched', () => { + const result = withMandatoryInstanceScopes(['tag:read', 'tag:list']); + expect(result).toEqual(expect.arrayContaining(['tag:read', 'tag:list', ...userView.scopes])); + }); }); diff --git a/packages/frontend/editor-ui/src/features/roles/instance/instanceRoleScopes.ts b/packages/frontend/editor-ui/src/features/roles/instance/instanceRoleScopes.ts index 51c6f9ade0e..5c7866c130d 100644 --- a/packages/frontend/editor-ui/src/features/roles/instance/instanceRoleScopes.ts +++ b/packages/frontend/editor-ui/src/features/roles/instance/instanceRoleScopes.ts @@ -46,6 +46,10 @@ export const INSTANCE_OPTION_LABEL_KEYS: Record = { 'Manage own': 'instanceRoles.option.manageOwn', 'Manage all': 'instanceRoles.option.manageAll', 'Manage project roles': 'instanceRoles.option.manageProjectRoles', + 'Mcp use': 'instanceRoles.option.mcpUse', + 'Mcp manage': 'instanceRoles.option.mcpManage', + 'AiAssistant use': 'instanceRoles.option.aiAssistantUse', + 'AiAssistant manage': 'instanceRoles.option.aiAssistantManage', }; /** @@ -57,6 +61,7 @@ export const INSTANCE_OPTION_LABEL_OVERRIDES: Partial< Record> > = { role: { Manage: 'instanceRoles.option.manageAllRoles' }, + settings: { Manage: 'instanceRoles.option.manageAllSettings' }, }; /** @@ -67,8 +72,17 @@ export const INSTANCE_OPTION_LABEL_OVERRIDES: Partial< export const INSTANCE_OPTION_DESCRIPTION_KEYS: Partial< Record> > = { - settings: { Manage: 'instanceRoles.description.settings.manage' }, - user: { Manage: 'instanceRoles.description.user.manage' }, + settings: { + Manage: 'instanceRoles.description.settings.manage', + 'Mcp use': 'instanceRoles.description.settings.mcpUse', + 'Mcp manage': 'instanceRoles.description.settings.mcpManage', + 'AiAssistant use': 'instanceRoles.description.settings.aiAssistantUse', + 'AiAssistant manage': 'instanceRoles.description.settings.aiAssistantManage', + }, + user: { + View: 'instanceRoles.description.user.view', + Manage: 'instanceRoles.description.user.manage', + }, role: { 'Manage project roles': 'instanceRoles.description.role.manageProjectRoles', Manage: 'instanceRoles.description.role.manage', @@ -87,6 +101,10 @@ export const INSTANCE_OPTION_ORDER: string[] = [ 'View', 'Create', 'Manage project roles', + 'Mcp use', + 'Mcp manage', + 'AiAssistant use', + 'AiAssistant manage', 'Manage', 'Manage own', 'Manage all', @@ -137,6 +155,24 @@ export const ALL_INSTANCE_SCOPES: Scope[] = [ ...new Set(INSTANCE_SCOPE_GROUP_LIST.flatMap((g) => g.options.flatMap((o) => o.scopes))), ]; +/** + * "Users: View" is baseline behavior every instance role carries — the default + * Member role already has it — not something a custom role can opt out of. + * Rendered checked and disabled in the editor. `withMandatoryInstanceScopes` + * is applied to the form (and on save), not the persisted snapshot, so a + * stored role that is missing these scopes stays unsaved until the next save. + */ +export function isOptionMandatory(resource: InstanceResource, option: InstanceScopeOption) { + return resource === 'user' && option.key === 'View'; +} + +const MANDATORY_INSTANCE_SCOPES: readonly Scope[] = GLOBAL_CUSTOM_ROLE_SCOPE_GROUPS.user.View; + +/** Unions in the mandatory scopes (see `isOptionMandatory`) on top of an already-filtered scope list. */ +export function withMandatoryInstanceScopes(scopes: readonly string[]): string[] { + return [...new Set([...scopes, ...MANDATORY_INSTANCE_SCOPES])]; +} + export type OptionState = 'checked' | 'indeterminate' | 'unchecked'; /** @@ -148,6 +184,7 @@ export type OptionState = 'checked' | 'indeterminate' | 'unchecked'; export const SUPERSEDED_BY: Partial> = { 'Manage own': 'Manage all', 'Manage project roles': 'Manage', + View: 'Manage', }; /** @@ -223,14 +260,16 @@ export function resolveOptionState( /** * Find the option that `option` supersedes within its group, if any. SUPERSEDED_BY * maps a sub-option to its superseding option, so the subordinate of a superseding - * option is the key that points back to it. + * option is the key that points back to it. Different resources can reuse the same + * superseding key (e.g. "Manage" backs both role's "Manage project roles" and user's + * "View"), so the reverse lookup must only consider keys present in this group. */ export function findSubordinateOption( option: InstanceScopeOption, groupOptions: InstanceScopeOption[], ): InstanceScopeOption | undefined { const subordinateKey = Object.keys(SUPERSEDED_BY).find( - (key) => SUPERSEDED_BY[key] === option.key, + (key) => SUPERSEDED_BY[key] === option.key && groupOptions.some((o) => o.key === key), ); return subordinateKey ? groupOptions.find((o) => o.key === subordinateKey) : undefined; } @@ -266,6 +305,8 @@ export function toggleOptionInGroup( return [...next]; } +const userViewScopes: ReadonlySet = new Set(GLOBAL_CUSTOM_ROLE_SCOPE_GROUPS.user.View); + /** * Resource groups whose scopes enable privilege escalation, with the warning to show. * Entries are checked in order; the first matching scope's message wins. @@ -275,7 +316,11 @@ export const ESCALATION_WARNING_SCOPES: Partial< > = { user: [ { - scopes: [...GLOBAL_CUSTOM_ROLE_SCOPE_GROUPS.user.Manage], + // Excludes View's `user:read`/`user:list` — looking users up isn't an + // escalation risk on its own, only Manage's write scopes are. + scopes: GLOBAL_CUSTOM_ROLE_SCOPE_GROUPS.user.Manage.filter( + (scope: Scope) => !userViewScopes.has(scope), + ), messageKey: 'instanceRoles.warning.manageMembers', }, ],