From af695ebf934526d926ea87fe87df61aa73d70979 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Tue, 30 Jul 2024 14:58:07 +0200 Subject: [PATCH] feat: Support create, read, delete variables in Public API (#10241) --- .../variables/spec/paths/variables.id.yml | 16 ++ .../variables/spec/paths/variables.yml | 40 +++++ .../spec/schemas/parameters/variableId.yml | 6 + .../variables/spec/schemas/variable.yml | 17 ++ .../variables/spec/schemas/variableList.yml | 11 ++ .../handlers/variables/variables.handler.ts | 55 ++++++ packages/cli/src/PublicApi/v1/openapi.yml | 6 + .../shared/middlewares/global.middleware.ts | 10 ++ .../integration/publicApi/variables.test.ts | 167 ++++++++++++++++++ .../cli/test/integration/shared/db/users.ts | 6 +- .../test/integration/shared/db/variables.ts | 12 ++ 11 files changed, 345 insertions(+), 1 deletion(-) create mode 100644 packages/cli/src/PublicApi/v1/handlers/variables/spec/paths/variables.id.yml create mode 100644 packages/cli/src/PublicApi/v1/handlers/variables/spec/paths/variables.yml create mode 100644 packages/cli/src/PublicApi/v1/handlers/variables/spec/schemas/parameters/variableId.yml create mode 100644 packages/cli/src/PublicApi/v1/handlers/variables/spec/schemas/variable.yml create mode 100644 packages/cli/src/PublicApi/v1/handlers/variables/spec/schemas/variableList.yml create mode 100644 packages/cli/src/PublicApi/v1/handlers/variables/variables.handler.ts create mode 100644 packages/cli/test/integration/publicApi/variables.test.ts create mode 100644 packages/cli/test/integration/shared/db/variables.ts diff --git a/packages/cli/src/PublicApi/v1/handlers/variables/spec/paths/variables.id.yml b/packages/cli/src/PublicApi/v1/handlers/variables/spec/paths/variables.id.yml new file mode 100644 index 0000000000..79c65416b2 --- /dev/null +++ b/packages/cli/src/PublicApi/v1/handlers/variables/spec/paths/variables.id.yml @@ -0,0 +1,16 @@ +delete: + x-eov-operation-id: deleteVariable + x-eov-operation-handler: v1/handlers/variables/variables.handler + tags: + - Variables + summary: Delete a variable + description: Delete a variable from your instance. + parameters: + - $ref: '../schemas/parameters/variableId.yml' + responses: + '204': + description: Operation successful. + '401': + $ref: '../../../../shared/spec/responses/unauthorized.yml' + '404': + $ref: '../../../../shared/spec/responses/notFound.yml' diff --git a/packages/cli/src/PublicApi/v1/handlers/variables/spec/paths/variables.yml b/packages/cli/src/PublicApi/v1/handlers/variables/spec/paths/variables.yml new file mode 100644 index 0000000000..7418c2fe05 --- /dev/null +++ b/packages/cli/src/PublicApi/v1/handlers/variables/spec/paths/variables.yml @@ -0,0 +1,40 @@ +post: + x-eov-operation-id: createVariable + x-eov-operation-handler: v1/handlers/variables/variables.handler + tags: + - Variables + summary: Create a variable + description: Create a variable in your instance. + requestBody: + description: Payload for variable to create. + content: + application/json: + schema: + $ref: '../schemas/variable.yml' + required: true + responses: + '201': + description: Operation successful. + '400': + $ref: '../../../../shared/spec/responses/badRequest.yml' + '401': + $ref: '../../../../shared/spec/responses/unauthorized.yml' +get: + x-eov-operation-id: getVariables + x-eov-operation-handler: v1/handlers/variables/variables.handler + tags: + - Variables + summary: Retrieve variables + description: Retrieve variables from your instance. + parameters: + - $ref: '../../../../shared/spec/parameters/limit.yml' + - $ref: '../../../../shared/spec/parameters/cursor.yml' + responses: + '200': + description: Operation successful. + content: + application/json: + schema: + $ref: '../schemas/variableList.yml' + '401': + $ref: '../../../../shared/spec/responses/unauthorized.yml' diff --git a/packages/cli/src/PublicApi/v1/handlers/variables/spec/schemas/parameters/variableId.yml b/packages/cli/src/PublicApi/v1/handlers/variables/spec/schemas/parameters/variableId.yml new file mode 100644 index 0000000000..a886039d8e --- /dev/null +++ b/packages/cli/src/PublicApi/v1/handlers/variables/spec/schemas/parameters/variableId.yml @@ -0,0 +1,6 @@ +name: id +in: path +description: The ID of the variable. +required: true +schema: + type: string diff --git a/packages/cli/src/PublicApi/v1/handlers/variables/spec/schemas/variable.yml b/packages/cli/src/PublicApi/v1/handlers/variables/spec/schemas/variable.yml new file mode 100644 index 0000000000..319ad8d440 --- /dev/null +++ b/packages/cli/src/PublicApi/v1/handlers/variables/spec/schemas/variable.yml @@ -0,0 +1,17 @@ +type: object +additionalProperties: false +required: + - key + - value +properties: + id: + type: string + readOnly: true + key: + type: string + value: + type: string + example: test + type: + type: string + readOnly: true diff --git a/packages/cli/src/PublicApi/v1/handlers/variables/spec/schemas/variableList.yml b/packages/cli/src/PublicApi/v1/handlers/variables/spec/schemas/variableList.yml new file mode 100644 index 0000000000..95e66e180f --- /dev/null +++ b/packages/cli/src/PublicApi/v1/handlers/variables/spec/schemas/variableList.yml @@ -0,0 +1,11 @@ +type: object +properties: + data: + type: array + items: + $ref: './variable.yml' + nextCursor: + type: string + description: Paginate through variables by setting the cursor parameter to a nextCursor attribute returned by a previous request. Default value fetches the first "page" of the collection. + nullable: true + example: MTIzZTQ1NjctZTg5Yi0xMmQzLWE0NTYtNDI2NjE0MTc0MDA diff --git a/packages/cli/src/PublicApi/v1/handlers/variables/variables.handler.ts b/packages/cli/src/PublicApi/v1/handlers/variables/variables.handler.ts new file mode 100644 index 0000000000..3073044eac --- /dev/null +++ b/packages/cli/src/PublicApi/v1/handlers/variables/variables.handler.ts @@ -0,0 +1,55 @@ +import Container from 'typedi'; +import { VariablesRepository } from '@/databases/repositories/variables.repository'; +import { VariablesController } from '@/environments/variables/variables.controller.ee'; +import { globalScope, isLicensed, validCursor } from '../../shared/middlewares/global.middleware'; +import { encodeNextCursor } from '../../shared/services/pagination.service'; +import type { Response } from 'express'; +import type { VariablesRequest } from '@/requests'; +import type { PaginatedRequest } from '@/PublicApi/types'; + +type Create = VariablesRequest.Create; +type Delete = VariablesRequest.Delete; +type GetAll = PaginatedRequest; + +export = { + createVariable: [ + isLicensed('feat:variables'), + globalScope('variable:create'), + async (req: Create, res: Response) => { + await Container.get(VariablesController).createVariable(req); + + res.status(201).send(); + }, + ], + deleteVariable: [ + isLicensed('feat:variables'), + globalScope('variable:delete'), + async (req: Delete, res: Response) => { + await Container.get(VariablesController).deleteVariable(req); + + res.status(204).send(); + }, + ], + getVariables: [ + isLicensed('feat:variables'), + globalScope('variable:list'), + validCursor, + async (req: GetAll, res: Response) => { + const { offset = 0, limit = 100 } = req.query; + + const [variables, count] = await Container.get(VariablesRepository).findAndCount({ + skip: offset, + take: limit, + }); + + return res.json({ + data: variables, + nextCursor: encodeNextCursor({ + offset, + limit, + numberOfTotalRecords: count, + }), + }); + }, + ], +}; diff --git a/packages/cli/src/PublicApi/v1/openapi.yml b/packages/cli/src/PublicApi/v1/openapi.yml index 9d82499835..91eacd5376 100644 --- a/packages/cli/src/PublicApi/v1/openapi.yml +++ b/packages/cli/src/PublicApi/v1/openapi.yml @@ -30,6 +30,8 @@ tags: description: Operations about tags - name: SourceControl description: Operations about source control + - name: Variables + description: Operations about variables paths: /audit: @@ -64,6 +66,10 @@ paths: $ref: './handlers/users/spec/paths/users.id.yml' /source-control/pull: $ref: './handlers/sourceControl/spec/paths/sourceControl.yml' + /variables: + $ref: './handlers/variables/spec/paths/variables.yml' + /variables/{id}: + $ref: './handlers/variables/spec/paths/variables.id.yml' components: schemas: $ref: './shared/spec/schemas/_index.yml' diff --git a/packages/cli/src/PublicApi/v1/shared/middlewares/global.middleware.ts b/packages/cli/src/PublicApi/v1/shared/middlewares/global.middleware.ts index 6fa9bed113..7e8c39bb91 100644 --- a/packages/cli/src/PublicApi/v1/shared/middlewares/global.middleware.ts +++ b/packages/cli/src/PublicApi/v1/shared/middlewares/global.middleware.ts @@ -9,6 +9,8 @@ import type { PaginatedRequest } from '../../../types'; import { decodeCursor } from '../services/pagination.service'; import type { Scope } from '@n8n/permissions'; import { userHasScope } from '@/permissions/checkAccess'; +import type { BooleanLicenseFeature } from '@/Interfaces'; +import { FeatureNotLicensedError } from '@/errors/feature-not-licensed.error'; const UNLIMITED_USERS_QUOTA = -1; @@ -86,3 +88,11 @@ export const validLicenseWithUserQuota = ( return next(); }; + +export const isLicensed = (feature: BooleanLicenseFeature) => { + return async (_: AuthenticatedRequest, res: express.Response, next: express.NextFunction) => { + if (Container.get(License).isFeatureEnabled(feature)) return next(); + + return res.status(403).json({ message: new FeatureNotLicensedError(feature).message }); + }; +}; diff --git a/packages/cli/test/integration/publicApi/variables.test.ts b/packages/cli/test/integration/publicApi/variables.test.ts new file mode 100644 index 0000000000..b97c2467eb --- /dev/null +++ b/packages/cli/test/integration/publicApi/variables.test.ts @@ -0,0 +1,167 @@ +import { setupTestServer } from '@test-integration/utils'; +import { createOwner } from '@test-integration/db/users'; +import { createVariable, getVariableOrFail } from '@test-integration/db/variables'; +import * as testDb from '../shared/testDb'; +import { FeatureNotLicensedError } from '@/errors/feature-not-licensed.error'; + +describe('Variables in Public API', () => { + const testServer = setupTestServer({ endpointGroups: ['publicApi'] }); + + beforeAll(async () => { + await testDb.init(); + }); + + beforeEach(async () => { + await testDb.truncate(['Variables', 'User']); + }); + + describe('GET /variables', () => { + it('if licensed, should return all variables with pagination', async () => { + /** + * Arrange + */ + testServer.license.enable('feat:variables'); + const owner = await createOwner({ withApiKey: true }); + const variables = await Promise.all([createVariable(), createVariable(), createVariable()]); + + /** + * Act + */ + const response = await testServer.publicApiAgentFor(owner).get('/variables'); + + /** + * Assert + */ + expect(response.status).toBe(200); + expect(response.body).toHaveProperty('data'); + expect(response.body).toHaveProperty('nextCursor'); + expect(Array.isArray(response.body.data)).toBe(true); + expect(response.body.data.length).toBe(variables.length); + + variables.forEach(({ id, key, value }) => { + expect(response.body.data).toContainEqual(expect.objectContaining({ id, key, value })); + }); + }); + + it('if not licensed, should reject', async () => { + /** + * Arrange + */ + const owner = await createOwner({ withApiKey: true }); + + /** + * Act + */ + const response = await testServer.publicApiAgentFor(owner).get('/variables'); + + /** + * Assert + */ + expect(response.status).toBe(403); + expect(response.body).toHaveProperty( + 'message', + new FeatureNotLicensedError('feat:variables').message, + ); + }); + }); + + describe('POST /variables', () => { + it('if licensed, should create a new variable', async () => { + /** + * Arrange + */ + testServer.license.enable('feat:variables'); + const owner = await createOwner({ withApiKey: true }); + const variablePayload = { key: 'key', value: 'value' }; + + /** + * Act + */ + const response = await testServer + .publicApiAgentFor(owner) + .post('/variables') + .send(variablePayload); + + /** + * Assert + */ + expect(response.status).toBe(201); + await expect(getVariableOrFail(response.body.id)).resolves.toEqual( + expect.objectContaining(variablePayload), + ); + }); + + it('if not licensed, should reject', async () => { + /** + * Arrange + */ + const owner = await createOwner({ withApiKey: true }); + const variablePayload = { key: 'key', value: 'value' }; + + /** + * Act + */ + const response = await testServer + .publicApiAgentFor(owner) + .post('/variables') + .send(variablePayload); + + /** + * Assert + */ + expect(response.status).toBe(403); + expect(response.body).toHaveProperty( + 'message', + new FeatureNotLicensedError('feat:variables').message, + ); + }); + }); + + describe('DELETE /variables/:id', () => { + it('if licensed, should delete a variable', async () => { + /** + * Arrange + */ + testServer.license.enable('feat:variables'); + const owner = await createOwner({ withApiKey: true }); + const variable = await createVariable(); + + /** + * Act + */ + const response = await testServer + .publicApiAgentFor(owner) + .delete(`/variables/${variable.id}`); + + /** + * Assert + */ + expect(response.status).toBe(204); + await expect(getVariableOrFail(variable.id)).rejects.toThrow(); + }); + + it('if not licensed, should reject', async () => { + /** + * Arrange + */ + const owner = await createOwner({ withApiKey: true }); + const variable = await createVariable(); + + /** + * Act + */ + const response = await testServer + .publicApiAgentFor(owner) + .delete(`/variables/${variable.id}`); + + /** + * Assert + */ + expect(response.status).toBe(403); + expect(response.body).toHaveProperty( + 'message', + new FeatureNotLicensedError('feat:variables').message, + ); + }); + }); +}); diff --git a/packages/cli/test/integration/shared/db/users.ts b/packages/cli/test/integration/shared/db/users.ts index aa7a3baaa7..f125f5ccde 100644 --- a/packages/cli/test/integration/shared/db/users.ts +++ b/packages/cli/test/integration/shared/db/users.ts @@ -78,7 +78,11 @@ export async function createUserWithMfaEnabled( }; } -export async function createOwner() { +export async function createOwner({ withApiKey } = { withApiKey: false }) { + if (withApiKey) { + return await addApiKey(await createUser({ role: 'global:owner' })); + } + return await createUser({ role: 'global:owner' }); } diff --git a/packages/cli/test/integration/shared/db/variables.ts b/packages/cli/test/integration/shared/db/variables.ts new file mode 100644 index 0000000000..68495da3b1 --- /dev/null +++ b/packages/cli/test/integration/shared/db/variables.ts @@ -0,0 +1,12 @@ +import { VariablesRepository } from '@/databases/repositories/variables.repository'; +import { generateNanoId } from '@/databases/utils/generators'; +import { randomString } from 'n8n-workflow'; +import Container from 'typedi'; + +export async function createVariable(key = randomString(5), value = randomString(5)) { + return await Container.get(VariablesRepository).save({ id: generateNanoId(), key, value }); +} + +export async function getVariableOrFail(id: string) { + return await Container.get(VariablesRepository).findOneOrFail({ where: { id } }); +}