From 909508000f7c8f4148a73a8932ccb871054fa9da Mon Sep 17 00:00:00 2001 From: Valya Bullions Date: Thu, 24 Oct 2024 18:16:51 +0100 Subject: [PATCH] work in progress --- packages/@n8n/api-types/src/dto/index.ts | 1 + .../@n8n/api-types/src/dto/variables/base.ts | 20 ++++ .../variables/create-variable-request.dto.ts | 17 +--- .../variables/update-variable-request.dto.ts | 3 + packages/@n8n/permissions/src/constants.ts | 1 + packages/@n8n/permissions/src/types.ts | 52 +--------- .../variables/variables.controller.ee.ts | 30 +++--- .../variables/variables.service.ee.ts | 94 ++++++++++++++++++- .../response-errors/missing-scope.error.ts | 9 ++ packages/cli/src/permissions/global-roles.ts | 9 +- packages/cli/src/permissions/project-roles.ts | 9 ++ 11 files changed, 165 insertions(+), 80 deletions(-) create mode 100644 packages/@n8n/api-types/src/dto/variables/base.ts create mode 100644 packages/@n8n/api-types/src/dto/variables/update-variable-request.dto.ts create mode 100644 packages/cli/src/errors/response-errors/missing-scope.error.ts diff --git a/packages/@n8n/api-types/src/dto/index.ts b/packages/@n8n/api-types/src/dto/index.ts index 34ee681342..8f1c85cc28 100644 --- a/packages/@n8n/api-types/src/dto/index.ts +++ b/packages/@n8n/api-types/src/dto/index.ts @@ -4,3 +4,4 @@ export { SettingsUpdateRequestDto } from './user/settings-update-request.dto'; export { UserUpdateRequestDto } from './user/user-update-request.dto'; export { CommunityRegisteredRequestDto } from './license/community-registered-request.dto'; export { CreateVariableRequestDto } from './variables/create-variable-request.dto'; +export { UpdateVariableRequestDto } from './variables/update-variable-request.dto'; diff --git a/packages/@n8n/api-types/src/dto/variables/base.ts b/packages/@n8n/api-types/src/dto/variables/base.ts new file mode 100644 index 0000000000..e7d7cc17d1 --- /dev/null +++ b/packages/@n8n/api-types/src/dto/variables/base.ts @@ -0,0 +1,20 @@ +import { z } from 'zod'; +import { Z } from 'zod-class'; + +export const KEY_NAME_REGEX = /^[A-Za-z0-9_]+$/; +export const KEY_MAX_LENGTH = 50; +export const VALUE_MAX_LENGTH = 255; +export const TYPE_ENUM = ['string'] as const; +export const TYPE_DEFAULT: (typeof TYPE_ENUM)[number] = 'string'; + +export class BaseVariableRequestDto extends Z.class({ + key: z + .string() + .min(1, 'key must be at least 1 character long') + .max(50, 'key cannot be longer than 50 characters') + .regex(KEY_NAME_REGEX, 'key can only contain characters A-Za-z0-9_'), + type: z.enum(TYPE_ENUM).default(TYPE_DEFAULT), + value: z + .string() + .max(VALUE_MAX_LENGTH, `value cannot be longer than ${VALUE_MAX_LENGTH} characters`), +}) {} diff --git a/packages/@n8n/api-types/src/dto/variables/create-variable-request.dto.ts b/packages/@n8n/api-types/src/dto/variables/create-variable-request.dto.ts index 2c13ebf6ba..5eae0a03ec 100644 --- a/packages/@n8n/api-types/src/dto/variables/create-variable-request.dto.ts +++ b/packages/@n8n/api-types/src/dto/variables/create-variable-request.dto.ts @@ -1,20 +1,7 @@ import { z } from 'zod'; -import { Z } from 'zod-class'; -export const KEY_NAME_REGEX = /^[A-Za-z0-9_]+$/; -export const VALUE_MAX_LENGTH = 255; -export const TYPE_ENUM = ['string'] as const; -export const TYPE_DEFAULT: (typeof TYPE_ENUM)[number] = 'string'; +import { BaseVariableRequestDto } from './base'; -export class CreateVariableRequestDto extends Z.class({ - key: z - .string() - .min(1, 'key must be at least 1 character long') - .max(50, 'key cannot be longer than 50 characters') - .regex(KEY_NAME_REGEX, 'key can only contain characters A-Za-z0-9_'), - type: z.enum(TYPE_ENUM).default(TYPE_DEFAULT), - value: z - .string() - .max(VALUE_MAX_LENGTH, `value cannot be longer than ${VALUE_MAX_LENGTH} characters`), +export class CreateVariableRequestDto extends BaseVariableRequestDto.extend({ projectId: z.string().length(36).optional().nullable().default(null), }) {} diff --git a/packages/@n8n/api-types/src/dto/variables/update-variable-request.dto.ts b/packages/@n8n/api-types/src/dto/variables/update-variable-request.dto.ts new file mode 100644 index 0000000000..f13d3d4f18 --- /dev/null +++ b/packages/@n8n/api-types/src/dto/variables/update-variable-request.dto.ts @@ -0,0 +1,3 @@ +import { BaseVariableRequestDto } from './base'; + +export class UpdateVariableRequestDto extends BaseVariableRequestDto {} diff --git a/packages/@n8n/permissions/src/constants.ts b/packages/@n8n/permissions/src/constants.ts index 7a0ebf2cb1..cd8d3bd725 100644 --- a/packages/@n8n/permissions/src/constants.ts +++ b/packages/@n8n/permissions/src/constants.ts @@ -9,6 +9,7 @@ export const RESOURCES = { externalSecretsProvider: ['sync', ...DEFAULT_OPERATIONS] as const, externalSecret: ['list', 'use'] as const, eventBusDestination: ['test', ...DEFAULT_OPERATIONS] as const, + globalVariable: [...DEFAULT_OPERATIONS] as const, ldap: ['sync', 'manage'] as const, license: ['manage'] as const, logStreaming: ['manage'] as const, diff --git a/packages/@n8n/permissions/src/types.ts b/packages/@n8n/permissions/src/types.ts index f32c00ad41..972bd6924c 100644 --- a/packages/@n8n/permissions/src/types.ts +++ b/packages/@n8n/permissions/src/types.ts @@ -1,6 +1,5 @@ -import type { DEFAULT_OPERATIONS, RESOURCES } from './constants'; +import type { RESOURCES } from './constants'; -export type DefaultOperations = (typeof DEFAULT_OPERATIONS)[number]; export type Resource = keyof typeof RESOURCES; export type ResourceScope< @@ -10,52 +9,11 @@ export type ResourceScope< export type WildcardScope = `${Resource}:*` | '*'; -export type AnnotationTagScope = ResourceScope<'annotationTag'>; -export type AuditLogsScope = ResourceScope<'auditLogs'>; -export type BannerScope = ResourceScope<'banner'>; -export type CommunityScope = ResourceScope<'community'>; -export type CommunityPackageScope = ResourceScope<'communityPackage'>; -export type CredentialScope = ResourceScope<'credential'>; -export type ExternalSecretScope = ResourceScope<'externalSecret'>; -export type ExternalSecretProviderScope = ResourceScope<'externalSecretsProvider'>; -export type EventBusDestinationScope = ResourceScope<'eventBusDestination'>; -export type LdapScope = ResourceScope<'ldap'>; -export type LicenseScope = ResourceScope<'license'>; -export type LogStreamingScope = ResourceScope<'logStreaming'>; -export type OrchestrationScope = ResourceScope<'orchestration'>; -export type ProjectScope = ResourceScope<'project'>; -export type SamlScope = ResourceScope<'saml'>; -export type SecurityAuditScope = ResourceScope<'securityAudit'>; -export type SourceControlScope = ResourceScope<'sourceControl'>; -export type TagScope = ResourceScope<'tag'>; -export type UserScope = ResourceScope<'user'>; -export type VariableScope = ResourceScope<'variable'>; -export type WorkersViewScope = ResourceScope<'workersView'>; -export type WorkflowScope = ResourceScope<'workflow'>; +type AllScopesObject = { + [R in Resource]: ResourceScope; +}; -export type Scope = - | AnnotationTagScope - | AuditLogsScope - | BannerScope - | CommunityScope - | CommunityPackageScope - | CredentialScope - | ExternalSecretProviderScope - | ExternalSecretScope - | EventBusDestinationScope - | LdapScope - | LicenseScope - | LogStreamingScope - | OrchestrationScope - | ProjectScope - | SamlScope - | SecurityAuditScope - | SourceControlScope - | TagScope - | UserScope - | VariableScope - | WorkersViewScope - | WorkflowScope; +export type Scope = AllScopesObject[K]; export type ScopeLevel = 'global' | 'project' | 'resource'; export type GetScopeLevel = Record; diff --git a/packages/cli/src/environments/variables/variables.controller.ee.ts b/packages/cli/src/environments/variables/variables.controller.ee.ts index 94cc543d75..ac89bd14b3 100644 --- a/packages/cli/src/environments/variables/variables.controller.ee.ts +++ b/packages/cli/src/environments/variables/variables.controller.ee.ts @@ -1,4 +1,4 @@ -import { CreateVariableRequestDto } from '@n8n/api-types'; +import { CreateVariableRequestDto, UpdateVariableRequestDto } from '@n8n/api-types'; import { Body, @@ -10,6 +10,7 @@ import { Post, RestController, } from '@/decorators'; +import { Param } from '@/decorators/args'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { NotFoundError } from '@/errors/response-errors/not-found.error'; import { VariableCountLimitReachedError } from '@/errors/variable-count-limit-reached.error'; @@ -50,24 +51,29 @@ export class VariablesController { @Get('/:id') @GlobalScope('variable:read') - async getVariable(req: VariablesRequest.Get) { - const id = req.params.id; - const variable = await this.variablesService.getCached(id); + async getVariable( + req: AuthenticatedRequest, + _res: Response, + @Param('variableId') variableId: string, + ) { + const variable = await this.variablesService.getCached(variableId); if (variable === null) { - throw new NotFoundError(`Variable with id ${req.params.id} not found`); + throw new NotFoundError(`Variable with id ${variableId} not found`); } return variable; } - @Patch('/:id') + @Patch('/:variableId') @Licensed('feat:variables') @GlobalScope('variable:update') - async updateVariable(req: VariablesRequest.Update) { - const id = req.params.id; - const variable = req.body; - delete variable.id; + async updateVariable( + req: AuthenticatedRequest, + _res: Response, + @Param('variableId') variableId: string, + @Body variable: UpdateVariableRequestDto, + ) { try { - return await this.variablesService.update(id, variable); + return await this.variablesService.update(variableId, variable, req.user); } catch (error) { if (error instanceof VariableCountLimitReachedError) { throw new BadRequestError(error.message); @@ -78,7 +84,7 @@ export class VariablesController { } } - @Delete('/:id(\\w+)') + @Delete('/:id') @GlobalScope('variable:delete') async deleteVariable(req: VariablesRequest.Delete) { const id = req.params.id; diff --git a/packages/cli/src/environments/variables/variables.service.ee.ts b/packages/cli/src/environments/variables/variables.service.ee.ts index e83c280ccb..b35e25f32e 100644 --- a/packages/cli/src/environments/variables/variables.service.ee.ts +++ b/packages/cli/src/environments/variables/variables.service.ee.ts @@ -1,16 +1,19 @@ -import type { CreateVariableRequestDto } from '@n8n/api-types'; +import type { CreateVariableRequestDto, UpdateVariableRequestDto } from '@n8n/api-types'; import { Container, Service } from 'typedi'; +import type { User } from '@/databases/entities/user'; import type { Variables } from '@/databases/entities/variables'; import { VariablesRepository } from '@/databases/repositories/variables.repository'; import { generateNanoId } from '@/databases/utils/generators'; +import { MissingScopeError } from '@/errors/response-errors/missing-scope.error'; import { VariableCountLimitReachedError } from '@/errors/variable-count-limit-reached.error'; import { VariableValidationError } from '@/errors/variable-validation.error'; import { EventService } from '@/events/event.service'; import { CacheService } from '@/services/cache/cache.service'; +// TODO: figure out how to avoid this cycle +import { ProjectService } from '@/services/project.service'; import { canCreateNewVariable } from './environment-helpers'; -import { User } from '@/databases/entities/user'; @Service() export class VariablesService { @@ -18,6 +21,7 @@ export class VariablesService { protected cacheService: CacheService, protected variablesRepository: VariablesRepository, private readonly eventService: EventService, + private readonly projectService: ProjectService, ) {} async getAllCached(): Promise { @@ -29,6 +33,29 @@ export class VariablesService { return (variables as Array>).map((v) => this.variablesRepository.create(v)); } + async getAllForUser(id: string, user: User): Promise { + const projects = await this.projectService.getAccessibleProjects(user); + const projectIds = projects.map((p) => p.id); + + const unfilteredVariables = await this.getAllCached(); + const canReadGlobalVariables = user.hasGlobalScope('globalVariable:read'); + + const foundVariables = unfilteredVariables.filter((variable) => { + if (variable.id !== id) { + return false; + } else if (!variable.projectId && canReadGlobalVariables) { + return true; + } else if (variable.projectId && projectIds.includes(variable.projectId)) { + return true; + } + return false; + }); + + return (foundVariables as Array>).map((v) => + this.variablesRepository.create(v), + ); + } + async getCount(): Promise { return (await this.getAllCached()).length; } @@ -42,6 +69,30 @@ export class VariablesService { return this.variablesRepository.create(foundVariable as Partial); } + async getForUser(id: string, user: User): Promise { + const projects = await this.projectService.getAccessibleProjects(user); + const projectIds = projects.map((p) => p.id); + + const variables = await this.getAllCached(); + const canReadGlobalVariables = user.hasGlobalScope('globalVariable:read'); + + const foundVariable = variables.find((variable) => { + if (variable.id !== id) { + return false; + } else if (!variable.projectId && canReadGlobalVariables) { + return true; + } else if (variable.projectId && projectIds.includes(variable.projectId)) { + return true; + } + return false; + }); + + if (!foundVariable) { + return null; + } + return this.variablesRepository.create(foundVariable as Partial); + } + async delete(id: string): Promise { await this.variablesRepository.delete(id); await this.updateCache(); @@ -74,6 +125,21 @@ export class VariablesService { throw new VariableCountLimitReachedError('Variables limit reached'); } + // Creating a global variable + if (!variable.projectId && !user.hasGlobalScope('globalVariable:create')) { + throw new MissingScopeError(); + } + + // Creating a project variable + if ( + variable.projectId && + !(await this.projectService.getProjectWithScope(user, variable.projectId, [ + 'variable:create', + ])) + ) { + throw new MissingScopeError(); + } + this.eventService.emit('variable-created'); const saveResult = await this.variablesRepository.save( { @@ -86,8 +152,28 @@ export class VariablesService { return saveResult; } - async update(id: string, variable: Omit): Promise { - this.validateVariable(variable); + async update(id: string, variable: UpdateVariableRequestDto, user: User): Promise { + const originalVariable = await this.variablesRepository.findOneOrFail({ + where: { + id, + }, + }); + + // Updating a global variable + if (!originalVariable.projectId && !user.hasGlobalScope('globalVariable:create')) { + throw new MissingScopeError(); + } + + // Updating a project variable + if ( + originalVariable.projectId && + !(await this.projectService.getProjectWithScope(user, originalVariable.projectId, [ + 'variable:create', + ])) + ) { + throw new MissingScopeError(); + } + await this.variablesRepository.update(id, variable); await this.updateCache(); return (await this.getCached(id))!; diff --git a/packages/cli/src/errors/response-errors/missing-scope.error.ts b/packages/cli/src/errors/response-errors/missing-scope.error.ts new file mode 100644 index 0000000000..32e3f380d8 --- /dev/null +++ b/packages/cli/src/errors/response-errors/missing-scope.error.ts @@ -0,0 +1,9 @@ +import { RESPONSE_ERROR_MESSAGES } from '@/constants'; + +import { ResponseError } from './abstract/response.error'; + +export class MissingScopeError extends ResponseError { + constructor(message = RESPONSE_ERROR_MESSAGES.MISSING_SCOPE, hint?: string) { + super(message, 403, 403, hint); + } +} diff --git a/packages/cli/src/permissions/global-roles.ts b/packages/cli/src/permissions/global-roles.ts index 7ea1b575da..595216430c 100644 --- a/packages/cli/src/permissions/global-roles.ts +++ b/packages/cli/src/permissions/global-roles.ts @@ -61,6 +61,11 @@ export const GLOBAL_OWNER_SCOPES: Scope[] = [ 'variable:update', 'variable:delete', 'variable:list', + 'globalVariable:create', + 'globalVariable:read', + 'globalVariable:update', + 'globalVariable:delete', + 'globalVariable:list', 'workflow:create', 'workflow:read', 'workflow:update', @@ -92,6 +97,6 @@ export const GLOBAL_MEMBER_SCOPES: Scope[] = [ 'tag:update', 'tag:list', 'user:list', - 'variable:list', - 'variable:read', + 'globalVariable:list', + 'globalVariable:read', ]; diff --git a/packages/cli/src/permissions/project-roles.ts b/packages/cli/src/permissions/project-roles.ts index d05507c47c..deca344189 100644 --- a/packages/cli/src/permissions/project-roles.ts +++ b/packages/cli/src/permissions/project-roles.ts @@ -25,6 +25,11 @@ export const REGULAR_PROJECT_ADMIN_SCOPES: Scope[] = [ 'project:read', 'project:update', 'project:delete', + 'variable:create', + 'variable:read', + 'variable:update', + 'variable:delete', + 'variable:list', ]; export const PERSONAL_PROJECT_OWNER_SCOPES: Scope[] = [ @@ -61,6 +66,8 @@ export const PROJECT_EDITOR_SCOPES: Scope[] = [ 'credential:list', 'project:list', 'project:read', + 'variable:list', + 'variable:read', ]; export const PROJECT_VIEWER_SCOPES: Scope[] = [ @@ -70,4 +77,6 @@ export const PROJECT_VIEWER_SCOPES: Scope[] = [ 'project:read', 'workflow:list', 'workflow:read', + 'variable:list', + 'variable:read', ];