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 97f14e95c47..5a130d3a742 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 @@ -117,6 +117,37 @@ describe('custom role scope whitelists', () => { expect(manage.length).toBeGreaterThan(view.length); }); + it('exposes "Variables: View" as exactly the global variable list/read pair', () => { + expect(GLOBAL_CUSTOM_ROLE_SCOPE_GROUPS.variable.View).toEqual([ + 'variable:list', + 'variable:read', + ]); + }); + + it('keeps "Variables: View" within GLOBAL_MEMBER_SCOPES', () => { + // A custom role built to mirror Member must never end up with more than Member. + for (const scope of GLOBAL_CUSTOM_ROLE_SCOPE_GROUPS.variable.View) { + expect(GLOBAL_MEMBER_SCOPES).toContain(scope); + } + }); + + it('keeps variable "Manage" a strict superset of variable "View"', () => { + // The editor's implied/downgrade arithmetic (SUPERSEDED_BY: View -> Manage) + // only holds while Manage contains everything View grants. + const view = GLOBAL_CUSTOM_ROLE_SCOPE_GROUPS.variable.View; + const manage = GLOBAL_CUSTOM_ROLE_SCOPE_GROUPS.variable.Manage; + expect(view.every((scope) => (manage as readonly string[]).includes(scope))).toBe(true); + expect(manage.length).toBeGreaterThan(view.length); + }); + + it('keeps variable scopes out of the settings.Manage bundle', () => { + // Global variables have their own group, so granting instance Settings no + // longer grants them. Every scope must live under exactly one group. + const bundle = GLOBAL_CUSTOM_ROLE_SCOPE_GROUPS.settings.Manage as readonly string[]; + expect(bundle).not.toContain('variable:list'); + expect(bundle).not.toContain('variable:read'); + }); + it('keeps "Tags: View" within GLOBAL_MEMBER_SCOPES', () => { // "Tags: View" is granted to every instance role by default (see // instanceRoleScopes.ts), so it must never exceed what the built-in Member 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 3d6adfcef65..17f36143d7a 100644 --- a/packages/@n8n/permissions/src/roles/custom-role-scopes.ee.ts +++ b/packages/@n8n/permissions/src/roles/custom-role-scopes.ee.ts @@ -100,8 +100,6 @@ export const GLOBAL_CUSTOM_ROLE_SCOPE_GROUPS = { 'eventBusDestination:delete', 'eventBusDestination:list', 'eventBusDestination:test', - 'variable:list', - 'variable:read', 'dataTable:list', 'aiPreference:create', // Context (instance-wide AI preferences) 'aiPreference:read', @@ -169,6 +167,21 @@ export const GLOBAL_CUSTOM_ROLE_SCOPE_GROUPS = { View: ['tag:read', 'tag:list'], Manage: ['tag:read', 'tag:list', 'tag:create', 'tag:update', 'tag:delete'], }, + variable: { + // Global (instance-level) variables only; project variables are granted per + // project via `projectVariable:*` on a project role. View mirrors + // GLOBAL_MEMBER_SCOPES exactly, so a custom role built to match Member never + // exceeds it. The list response carries each variable's value, so View reads + // values and is not only discovery — hence it is opt-in, not mandatory. + View: ['variable:list', 'variable:read'], + Manage: [ + 'variable:list', + 'variable:read', + 'variable:create', + 'variable:update', + 'variable:delete', + ], + }, project: { Create: ['project:create'], }, diff --git a/packages/cli/test/integration/variables.test.ts b/packages/cli/test/integration/variables.test.ts index fbe89ef9684..bc1a3251b46 100644 --- a/packages/cli/test/integration/variables.test.ts +++ b/packages/cli/test/integration/variables.test.ts @@ -10,6 +10,7 @@ import { getVariableByKey, } from '@test-integration/db/variables'; +import { createCustomRoleWithScopeSlugs } from './shared/db/roles'; import { createOwner, createUser } from './shared/db/users'; import type { SuperAgentTest } from './shared/types'; import * as utils from './shared/utils/'; @@ -119,6 +120,78 @@ describe('GET /variables/:id', () => { }); }); +// ---------------------------------------- +// Custom instance roles - global variables +// ---------------------------------------- +describe('GET /variables - custom instance roles', () => { + // A custom instance role only sees global variables when it holds `variable:list`. + // `variable:list` is now its own permission option, so a role can read global + // variables without also holding every instance settings scope. + let authNoVariableScopesAgent: SuperAgentTest; + let authVariableViewAgent: SuperAgentTest; + let globalVariable: Variables; + let projectVariable: Variables; + + beforeAll(async () => { + const roleWithoutVariableScopes = await createCustomRoleWithScopeSlugs(['user:list'], { + roleType: 'global', + }); + const roleWithVariableView = await createCustomRoleWithScopeSlugs( + ['user:list', 'variable:list', 'variable:read'], + { roleType: 'global' }, + ); + + const userWithoutVariableScopes = await createUser({ role: roleWithoutVariableScopes }); + const userWithVariableView = await createUser({ role: roleWithVariableView }); + await Promise.all([ + linkUserToProject(userWithoutVariableScopes, project, 'project:admin'), + linkUserToProject(userWithVariableView, project, 'project:admin'), + ]); + + authNoVariableScopesAgent = testServer.authAgentFor(userWithoutVariableScopes); + authVariableViewAgent = testServer.authAgentFor(userWithVariableView); + }); + + beforeEach(async () => { + [globalVariable, projectVariable] = await Promise.all([ + createVariable('globalVar', 'globalValue'), + createProjectVariable('projectVar', 'projectValue', project), + ]); + }); + + test('should filter out global variables for a role without variable:list', async () => { + const response = await authNoVariableScopesAgent.get('/variables'); + + // Filtered out silently, not a 403 - the list still returns the project variables. + expect(response.statusCode).toBe(200); + expect(response.body.data.map((variable: Variables) => variable.key)).toEqual([ + projectVariable.key, + ]); + }); + + test('should return global variables for a role with variable:list', async () => { + const response = await authVariableViewAgent.get('/variables'); + + expect(response.statusCode).toBe(200); + expect(response.body.data.map((variable: Variables) => variable.key).sort()).toEqual( + [globalVariable.key, projectVariable.key].sort(), + ); + }); + + test('should return a single global variable for a role with variable:read', async () => { + const response = await authVariableViewAgent.get(`/variables/${globalVariable.id}`); + + expect(response.statusCode).toBe(200); + expect(response.body.data.key).toBe(globalVariable.key); + }); + + test('should deny a single global variable to a role without variable:read', async () => { + const response = await authNoVariableScopesAgent.get(`/variables/${globalVariable.id}`); + + expect(response.statusCode).toBe(403); + }); +}); + // ---------------------------------------- // POST /variables - create a new variable // ---------------------------------------- diff --git a/packages/frontend/@n8n/i18n/src/locales/en.json b/packages/frontend/@n8n/i18n/src/locales/en.json index f82df500c12..39ce62e2e61 100644 --- a/packages/frontend/@n8n/i18n/src/locales/en.json +++ b/packages/frontend/@n8n/i18n/src/locales/en.json @@ -3030,6 +3030,7 @@ "instanceRoles.resource.role": "Roles", "instanceRoles.resource.apiKey": "API keys", "instanceRoles.resource.tag": "Tags", + "instanceRoles.resource.variable": "Variables", "instanceRoles.resource.project": "Projects", "instanceRoles.resource.insights": "Insights", "instanceRoles.option.view": "View", @@ -3059,6 +3060,8 @@ "instanceRoles.description.apiKey.manageAll": "View and delete other users' API keys.", "instanceRoles.description.tag.view": "Users always see and can select any tags associated with workflows they have access to.", "instanceRoles.description.tag.manage": "Create, edit, and delete tags", + "instanceRoles.description.variable.view": "View global variables and their values", + "instanceRoles.description.variable.manage": "Create, edit, and delete global variables", "instanceRoles.description.project.create": "Create new projects", "instanceRoles.description.insights.view": "View instance-level insights", "instanceRoles.warning.manageMembers": "A user with this permission can give themselves or an account they invite permissions they don't currently hold. {link}", 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 41dc30dbaa3..a8aa5169eed 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 @@ -82,6 +82,7 @@ describe('instanceRoleScopes config', () => { it('exposes the configured option labels per resource', () => { expect(Object.keys(INSTANCE_SCOPE_GROUPS.apiKey)).toEqual(['Manage own', 'Manage all']); expect(Object.keys(INSTANCE_SCOPE_GROUPS.tag)).toEqual(['View', 'Manage']); + expect(Object.keys(INSTANCE_SCOPE_GROUPS.variable)).toEqual(['View', '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([ @@ -154,6 +155,14 @@ describe('isOptionImplied', () => { it('returns false when the superseding option is present but "Manage own" is checked via own scopes only', () => { expect(isOptionImplied(manageOwn, apiKeyGroup.options, ownScopes)).toBe(false); }); + + it('renders variable "View" as implied under a fully-checked variable "Manage"', () => { + const variableGroup = INSTANCE_SCOPE_GROUP_LIST.find((g) => g.resource === 'variable')!; + const view = variableGroup.options.find((o) => o.key === 'View')!; + const manage = variableGroup.options.find((o) => o.key === 'Manage')!; + expect(isOptionImplied(view, variableGroup.options, [...manage.scopes])).toBe(true); + expect(isOptionImplied(view, variableGroup.options, [...view.scopes])).toBe(false); + }); }); describe('resolveOptionState', () => { @@ -306,6 +315,17 @@ describe('toggleOptionInGroup', () => { expect(result).toEqual(['user:read']); }); + it('downgrades to "View" when unchecking variable "Manage"', () => { + const variableGroup = INSTANCE_SCOPE_GROUP_LIST.find((g) => g.resource === 'variable')!; + const manageVariables = variableGroup.options.find((o) => o.key === 'Manage')!; + const result = toggleOptionInGroup( + ['user:list', ...manageVariables.scopes], + manageVariables, + variableGroup.options, + ); + expect(result).toEqual(['user:list', 'variable:list', 'variable:read']); + }); + it('downgrades to "View" when unchecking tag "Manage", keeping the mandatory scopes', () => { const tagGroup = INSTANCE_SCOPE_GROUP_LIST.find((g) => g.resource === 'tag')!; const manageTags = tagGroup.options.find((o) => o.key === 'Manage')!; 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 5e0a5ab1bfd..25a385b65d3 100644 --- a/packages/frontend/editor-ui/src/features/roles/instance/instanceRoleScopes.ts +++ b/packages/frontend/editor-ui/src/features/roles/instance/instanceRoleScopes.ts @@ -20,6 +20,7 @@ export const INSTANCE_RESOURCE_ORDER: InstanceResource[] = [ 'role', 'apiKey', 'tag', + 'variable', 'project', 'insights', ]; @@ -31,6 +32,7 @@ export const INSTANCE_RESOURCE_LABEL_KEYS: Record role: 'instanceRoles.resource.role', apiKey: 'instanceRoles.resource.apiKey', tag: 'instanceRoles.resource.tag', + variable: 'instanceRoles.resource.variable', project: 'instanceRoles.resource.project', insights: 'instanceRoles.resource.insights', }; @@ -95,6 +97,10 @@ export const INSTANCE_OPTION_DESCRIPTION_KEYS: Partial< View: 'instanceRoles.description.tag.view', Manage: 'instanceRoles.description.tag.manage', }, + variable: { + View: 'instanceRoles.description.variable.view', + Manage: 'instanceRoles.description.variable.manage', + }, project: { Create: 'instanceRoles.description.project.create' }, insights: { View: 'instanceRoles.description.insights.view' }, };