diff --git a/packages/@n8n/permissions/src/constants.ee.ts b/packages/@n8n/permissions/src/constants.ee.ts index ed2270fb97..3e984b26d3 100644 --- a/packages/@n8n/permissions/src/constants.ee.ts +++ b/packages/@n8n/permissions/src/constants.ee.ts @@ -30,7 +30,7 @@ export const RESOURCES = { export const API_KEY_RESOURCES = { tag: [...DEFAULT_OPERATIONS] as const, workflow: [...DEFAULT_OPERATIONS, 'move', 'activate', 'deactivate'] as const, - variable: DEFAULT_OPERATIONS, + variable: ['create', 'update', 'delete', 'list'] as const, securityAudit: ['generate'] as const, project: ['create', 'update', 'delete', 'list'] as const, user: ['read', 'list', 'create', 'changeRole', 'delete'] as const, diff --git a/packages/@n8n/permissions/src/public-api-permissions.ee.ts b/packages/@n8n/permissions/src/public-api-permissions.ee.ts index 92bf4e3d4f..c05e999809 100644 --- a/packages/@n8n/permissions/src/public-api-permissions.ee.ts +++ b/packages/@n8n/permissions/src/public-api-permissions.ee.ts @@ -15,6 +15,7 @@ export const OWNER_API_KEY_SCOPES: ApiKeyScope[] = [ 'variable:create', 'variable:delete', 'variable:list', + 'variable:update', 'tag:create', 'tag:read', 'tag:update', diff --git a/packages/cli/test/integration/public-api/endpoints-with-scopes-enabled.test.ts b/packages/cli/test/integration/public-api/endpoints-with-scopes-enabled.test.ts index 45db327cf1..6978734649 100644 --- a/packages/cli/test/integration/public-api/endpoints-with-scopes-enabled.test.ts +++ b/packages/cli/test/integration/public-api/endpoints-with-scopes-enabled.test.ts @@ -1,4 +1,4 @@ -import type { TagEntity } from '@n8n/db'; +import type { TagEntity, Variables } from '@n8n/db'; import { ApiKeyRepository } from '@n8n/db'; import { CredentialsRepository } from '@n8n/db'; import { ProjectRepository } from '@n8n/db'; @@ -22,7 +22,7 @@ import { createUser, getUserById, } from '@test-integration/db/users'; -import { createVariable, getVariableOrFail } from '@test-integration/db/variables'; +import { createVariable, getVariableByIdOrFail } from '@test-integration/db/variables'; import { createWorkflow } from '@test-integration/db/workflows'; import { randomName } from '@test-integration/random'; import type { CredentialPayload, SaveCredentialFunction } from '@test-integration/types'; @@ -57,6 +57,7 @@ describe('Public API endpoints with feat:apiKeyScopes enabled', () => { 'quota:maxTeamProjects': -1, }, }); + let apiKeyRepository: ApiKeyRepository; beforeAll(async () => { @@ -979,7 +980,7 @@ describe('Public API endpoints with feat:apiKeyScopes enabled', () => { * Assert */ expect(response.status).toBe(201); - await expect(getVariableOrFail(response.body.id)).resolves.toEqual( + await expect(getVariableByIdOrFail(response.body.id)).resolves.toEqual( expect.objectContaining(variablePayload), ); }); @@ -1005,6 +1006,36 @@ describe('Public API endpoints with feat:apiKeyScopes enabled', () => { expect(response.status).toBe(403); }); }); + + describe('PUT /variables/:id', () => { + const variablePayload = { key: 'updatedKey', value: 'updatedValue' }; + let variable: Variables; + beforeEach(async () => { + variable = await createVariable(); + }); + + it('should update a variable if API key has scope "variable:update"', async () => { + const owner = await createOwnerWithApiKey({ scopes: ['variable:update'] }); + + const response = await testServer + .publicApiAgentFor(owner) + .put(`/variables/${variable.id}`) + .send(variablePayload); + + expect(response.status).toBe(204); + const updatedVariable = await getVariableByIdOrFail(variable.id); + expect(updatedVariable).toEqual(expect.objectContaining(variablePayload)); + }); + + test('should fail to update variable when API key doesn\'t have "variable:update" scope', async () => { + const owner = await createOwnerWithApiKey({ scopes: ['variable:list'] }); + + await testServer + .publicApiAgentFor(owner) + .put(`/variables/${variable.id}`) + .send(variablePayload); + }); + }); }); describe('projects', () => { diff --git a/packages/cli/test/integration/public-api/variables.test.ts b/packages/cli/test/integration/public-api/variables.test.ts index 98befd8745..d3b46a5b31 100644 --- a/packages/cli/test/integration/public-api/variables.test.ts +++ b/packages/cli/test/integration/public-api/variables.test.ts @@ -2,7 +2,7 @@ import type { User, Variables } from '@n8n/db'; import { FeatureNotLicensedError } from '@/errors/feature-not-licensed.error'; import { createOwnerWithApiKey } from '@test-integration/db/users'; -import { createVariable, getVariableOrFail } from '@test-integration/db/variables'; +import { createVariable, getVariableByIdOrFail } from '@test-integration/db/variables'; import { setupTestServer } from '@test-integration/utils'; import * as testDb from '../shared/test-db'; @@ -83,7 +83,7 @@ describe('Variables in Public API', () => { * Assert */ expect(response.status).toBe(201); - await expect(getVariableOrFail(response.body.id)).resolves.toEqual( + await expect(getVariableByIdOrFail(response.body.id)).resolves.toEqual( expect.objectContaining(variablePayload), ); }); @@ -126,7 +126,7 @@ describe('Variables in Public API', () => { .send(variablePayload); expect(response.status).toBe(204); - const updatedVariable = await getVariableOrFail(variable.id); + const updatedVariable = await getVariableByIdOrFail(variable.id); expect(updatedVariable).toEqual(expect.objectContaining(variablePayload)); }); @@ -164,7 +164,7 @@ describe('Variables in Public API', () => { * Assert */ expect(response.status).toBe(204); - await expect(getVariableOrFail(variable.id)).rejects.toThrow(); + await expect(getVariableByIdOrFail(variable.id)).rejects.toThrow(); }); it('if not licensed, should reject', async () => { diff --git a/packages/cli/test/integration/shared/db/variables.ts b/packages/cli/test/integration/shared/db/variables.ts index 9f1835533b..c703c02443 100644 --- a/packages/cli/test/integration/shared/db/variables.ts +++ b/packages/cli/test/integration/shared/db/variables.ts @@ -3,10 +3,34 @@ import { VariablesRepository } from '@n8n/db'; import { Container } from '@n8n/di'; import { randomString } from 'n8n-workflow'; +import { VariablesService } from '@/environments.ee/variables/variables.service.ee'; + export async function createVariable(key = randomString(5), value = randomString(5)) { - return await Container.get(VariablesRepository).save({ id: generateNanoId(), key, value }); + const result = await Container.get(VariablesRepository).save({ + id: generateNanoId(), + key, + value, + }); + await Container.get(VariablesService).updateCache(); + return result; } -export async function getVariableOrFail(id: string) { +export async function getVariableByIdOrFail(id: string) { return await Container.get(VariablesRepository).findOneOrFail({ where: { id } }); } + +export async function getVariableByKey(key: string) { + return await Container.get(VariablesRepository).findOne({ + where: { + key, + }, + }); +} + +export async function getVariableById(id: string) { + return await Container.get(VariablesRepository).findOne({ + where: { + id, + }, + }); +} diff --git a/packages/cli/test/integration/variables.test.ts b/packages/cli/test/integration/variables.test.ts index 526a3ba682..c8a28155bd 100644 --- a/packages/cli/test/integration/variables.test.ts +++ b/packages/cli/test/integration/variables.test.ts @@ -1,10 +1,8 @@ import type { Variables } from '@n8n/db'; -import { generateNanoId } from '@n8n/db'; -import { VariablesRepository } from '@n8n/db'; import { Container } from '@n8n/di'; -import { VariablesService } from '@/environments.ee/variables/variables.service.ee'; import { CacheService } from '@/services/cache/cache.service'; +import { createVariable, getVariableById, getVariableByKey } from '@test-integration/db/variables'; import { createOwner, createUser } from './shared/db/users'; import * as testDb from './shared/test-db'; @@ -17,32 +15,6 @@ let authMemberAgent: SuperAgentTest; const testServer = utils.setupTestServer({ endpointGroups: ['variables'] }); const license = testServer.license; -async function createVariable(key: string, value: string) { - const result = await Container.get(VariablesRepository).save({ - id: generateNanoId(), - key, - value, - }); - await Container.get(VariablesService).updateCache(); - return result; -} - -async function getVariableByKey(key: string) { - return await Container.get(VariablesRepository).findOne({ - where: { - key, - }, - }); -} - -async function getVariableById(id: string) { - return await Container.get(VariablesRepository).findOne({ - where: { - id, - }, - }); -} - beforeAll(async () => { const owner = await createOwner(); authOwnerAgent = testServer.authAgentFor(owner);