From 86a4eb8042183ab73ead47b152fc22e518788a84 Mon Sep 17 00:00:00 2001 From: ricardo Date: Sun, 27 Mar 2022 10:48:43 -0400 Subject: [PATCH] :zap: Improvements --- packages/cli/jest.config.js | 2 +- packages/cli/src/PublicApi/helpers.ts | 4 + packages/cli/src/PublicApi/v1/index.ts | 106 +++++----- .../src/PublicApi/v1/routes/Users/index.ts | 4 +- packages/cli/src/Server.ts | 4 +- .../publicApi/users.endpoints.test-api.ts | 183 ++++++++++++++++++ .../cli/test/integration/shared/constants.ts | 2 + .../cli/test/integration/shared/random.ts | 4 + .../cli/test/integration/shared/testDb.ts | 5 +- .../cli/test/integration/shared/types.d.ts | 4 +- packages/cli/test/integration/shared/utils.ts | 38 ++-- 11 files changed, 284 insertions(+), 72 deletions(-) create mode 100644 packages/cli/test/integration/publicApi/users.endpoints.test-api.ts diff --git a/packages/cli/jest.config.js b/packages/cli/jest.config.js index 267cc93266..0e0213d31d 100644 --- a/packages/cli/jest.config.js +++ b/packages/cli/jest.config.js @@ -4,7 +4,7 @@ module.exports = { '^.+\\.ts?$': 'ts-jest', }, testURL: 'http://localhost/', - testRegex: '(/__tests__/.*|(\\.|/)(test))\\.ts$', + testRegex: '(/__tests__/.*|(\\.|/)(test-api))\\.ts$', testPathIgnorePatterns: ['/dist/', '/node_modules/'], moduleFileExtensions: ['ts', 'js', 'json'], globals: { diff --git a/packages/cli/src/PublicApi/helpers.ts b/packages/cli/src/PublicApi/helpers.ts index d00ec1622a..94f3835b4a 100644 --- a/packages/cli/src/PublicApi/helpers.ts +++ b/packages/cli/src/PublicApi/helpers.ts @@ -47,3 +47,7 @@ export const getSelectableProperties = (table: string) => { ], }[table]; }; + +export const connectionName = () => { + return 'default'; +}; diff --git a/packages/cli/src/PublicApi/v1/index.ts b/packages/cli/src/PublicApi/v1/index.ts index e2013a63be..e6b74c8592 100644 --- a/packages/cli/src/PublicApi/v1/index.ts +++ b/packages/cli/src/PublicApi/v1/index.ts @@ -9,73 +9,73 @@ import path = require('path'); import express = require('express'); -import { OpenAPIV3 } from 'express-openapi-validator/dist/framework/types'; -import { Db } from '../..'; +import { HttpError, OpenAPIV3 } from 'express-openapi-validator/dist/framework/types'; +import { Db, ResponseHelper } from '../..'; +import config = require('../../../config'); export interface N8nApp { app: Application; } -const publicApiController = express.Router(); +export const publicApiController = express.Router(); -export const getRoutes = (): express.Router => { +publicApiController.use(`/v1`, + OpenApiValidator.middleware({ + apiSpec: path.join(__dirname, 'openapi.yml'), + operationHandlers: path.join(__dirname), + validateRequests: true, // (default) + validateApiSpec: true, + validateSecurity: { + handlers: { + ApiKeyAuth: async (req, scopes, schema: OpenAPIV3.ApiKeySecurityScheme) => { - publicApiController.use(`/v1`, - OpenApiValidator.middleware({ - apiSpec: path.join(__dirname, 'openapi.yml'), - operationHandlers: path.join(__dirname), - validateRequests: true, // (default) - validateApiSpec: true, - validateSecurity: { - handlers: { - ApiKeyAuth: async (req, scopes, schema: OpenAPIV3.ApiKeySecurityScheme) => { + const apiKey = req.headers[schema.name.toLowerCase()]; - const apiKey = req.headers[schema.name.toLowerCase()]; + const user = await Db.collections.User!.find({ + where: { + apiKey, + }, + relations: ['globalRole'], + }); - const user = await Db.collections.User!.find({ - where: { - apiKey, - }, - relations: ['globalRole'], - }); + if (!user.length) { + return false; + } - if (!user.length) { - return false; - } + if (!config.get('userManagement.isInstanceOwnerSetUp')) { + throw { + message: 'asasasas', + status: 400, + }; + } - // if (!config.get('userManagement.isInstanceOwnerSetUp')) { - // Logger.debug( - // 'Request to send email invite(s) to user(s) failed because the owner account is not set up', - // ); - // throw new ResponseHelper.ResponseError( - // 'You must set up your own account before inviting others', - // undefined, - // 400, - // ); - // } + if (user[0].globalRole.name !== 'owner') { + throw { + status: 403, + }; + } - // if (req.user.globalRole.name === 'owner') { + req.user = user[0]; - - // } - - req.user = user[0]; - - return true; - }, + return true; }, - }, - })); - - //add error handler - //@ts-ignore - publicApiController.use((err, req, res, next) => { - res.status(err.status || 500).json({ - message: err.message, - errors: err.errors, - }); + }, + }, + })); +//add error handler +//@ts-ignore +publicApiController.use((err: HttpError, req, res, next) => { + res.status(err.status || 500).json({ + message: err.message, + errors: err.errors, }); - return publicApiController; -}; \ No newline at end of file +}); + +// export const getRoutes = (): express.Router => { + + + +// return publicApiController; +// }; \ No newline at end of file diff --git a/packages/cli/src/PublicApi/v1/routes/Users/index.ts b/packages/cli/src/PublicApi/v1/routes/Users/index.ts index 0794558175..f1decfd29c 100644 --- a/packages/cli/src/PublicApi/v1/routes/Users/index.ts +++ b/packages/cli/src/PublicApi/v1/routes/Users/index.ts @@ -4,7 +4,7 @@ import { getConnection } from 'typeorm'; import { UserRequest } from '../../../../requests'; import { User } from '../../../../databases/entities/User'; -import { decodeCursor, getNextCursor, getSelectableProperties } from '../../../helpers'; +import { connectionName, decodeCursor, getNextCursor, getSelectableProperties } from '../../../helpers'; import * as config from '../../../../../config'; @@ -28,7 +28,7 @@ export = { ({ offset, limit } = decodeCursor(cursor)); } - const query = getConnection() + const query = getConnection(connectionName()) .getRepository(User) .createQueryBuilder() .leftJoinAndSelect('User.globalRole', 'Role') diff --git a/packages/cli/src/Server.ts b/packages/cli/src/Server.ts index 42a1adf9e2..073aff41ec 100644 --- a/packages/cli/src/Server.ts +++ b/packages/cli/src/Server.ts @@ -172,7 +172,7 @@ import { SharedWorkflow } from './databases/entities/SharedWorkflow'; import { AUTH_COOKIE_NAME, RESPONSE_ERROR_MESSAGES } from './constants'; import { credentialsController } from './api/credentials.api'; import { getInstanceBaseUrl, isEmailSetUp } from './UserManagement/UserManagementHelper'; -import * as publicApiv1Routes from './PublicApi/v1'; +import { publicApiController } from './PublicApi/v1'; require('body-parser-xml')(bodyParser); @@ -582,7 +582,7 @@ class App { }); - this.app.use(`/${this.publicApiEndpoint}`, publicApiv1Routes.getRoutes()); + this.app.use(`/${this.publicApiEndpoint}/`, publicApiController); // Parse cookies for easier access this.app.use(cookieParser()); diff --git a/packages/cli/test/integration/publicApi/users.endpoints.test-api.ts b/packages/cli/test/integration/publicApi/users.endpoints.test-api.ts new file mode 100644 index 0000000000..5247c655d8 --- /dev/null +++ b/packages/cli/test/integration/publicApi/users.endpoints.test-api.ts @@ -0,0 +1,183 @@ +import express = require('express'); +import validator from 'validator'; +import { v4 as uuid } from 'uuid'; +import { compare } from 'bcryptjs'; + +import { Db } from '../../../src'; +import config = require('../../../config'); +import { SUCCESS_RESPONSE_BODY } from './../shared/constants'; +import { Role } from '../../../src/databases/entities/Role'; +import { + randomApiKey, + randomEmail, + randomInvalidPassword, + randomName, + randomValidPassword, +} from './../shared/random'; + +import * as utils from './../shared/utils'; +import * as testDb from './../shared/testDb'; + +// import * from './../../../src/PublicApi/helpers' + +let app: express.Application; +let testDbName = ''; +let globalOwnerRole: Role; +let globalMemberRole: Role; +let workflowOwnerRole: Role; +let credentialOwnerRole: Role; + +beforeAll(async () => { + app = utils.initTestServer({ endpointGroups: ['publicApi'], applyAuth: false }); + const initResult = await testDb.init(); + testDbName = initResult.testDbName; + + const [ + fetchedGlobalOwnerRole, + fetchedGlobalMemberRole, + fetchedWorkflowOwnerRole, + fetchedCredentialOwnerRole, + ] = await testDb.getAllRoles(); + + globalOwnerRole = fetchedGlobalOwnerRole; + globalMemberRole = fetchedGlobalMemberRole; + workflowOwnerRole = fetchedWorkflowOwnerRole; + credentialOwnerRole = fetchedCredentialOwnerRole; + + utils.initTestTelemetry(); + utils.initTestLogger(); +}); + +beforeEach(async () => { + // do not combine calls - shared tables must be cleared first and separately + await testDb.truncate(['SharedCredentials', 'SharedWorkflow'], testDbName); + await testDb.truncate(['User', 'Workflow', 'Credentials'], testDbName); + + jest.isolateModules(() => { + jest.mock('../../../config'); + jest.mock('./../../../src/PublicApi/helpers', () => ({ + ...jest.requireActual('./../../../src/PublicApi/helpers'), + connectionName: jest.fn(() => testDbName), + })); + }); + + await testDb.createUser({ + id: INITIAL_TEST_USER.id, + email: INITIAL_TEST_USER.email, + password: INITIAL_TEST_USER.password, + firstName: INITIAL_TEST_USER.firstName, + lastName: INITIAL_TEST_USER.lastName, + globalRole: globalOwnerRole, + apiKey: INITIAL_TEST_USER.apiKey, + }); + + config.set('userManagement.disabled', false); + config.set('userManagement.isInstanceOwnerSetUp', true); + config.set('userManagement.emails.mode', ''); +}); + +afterAll(async () => { + await testDb.terminate(testDbName); +}); + +test('GET /users should fail due to missing API Key', async () => { + const owner = await Db.collections.User!.findOneOrFail(); + + const authOwnerAgent = utils.createAgent(app, { apiPath: 'public', auth: false, user: owner }); + + await testDb.createUser(); + + const response = await authOwnerAgent.get('/v1/users'); + + expect(response.statusCode).toBe(401); + +}); + +test('GET /users should fail due to invalid API Key', async () => { + const owner = await Db.collections.User!.findOneOrFail(); + + owner.apiKey = null; + + const authOwnerAgent = utils.createAgent(app, { apiPath: 'public', auth: false, user: owner }); + + const response = await authOwnerAgent.get('/v1/users'); + + expect(response.statusCode).toBe(401); +}); + +test('GET /users should fail due to member trying to access owner only endpoint', async () => { + const member = await testDb.createUser(); + + const authOwnerAgent = utils.createAgent(app, { apiPath: 'public', auth: true, user: member }); + + const response = await authOwnerAgent.get('/v1/users'); + + expect(response.statusCode).toBe(403); +}); + +test('GET /users should fail due no instance owner not setup', async () => { + + config.set('userManagement.isInstanceOwnerSetUp', false); + + const owner = await Db.collections.User!.findOneOrFail(); + + const authOwnerAgent = utils.createAgent(app, { apiPath: 'public', auth: true, user: owner }); + + const response = await authOwnerAgent.get('/v1/users'); + + expect(response.statusCode).toBe(400); + +}); + +test('GET /users should return all users', async () => { + + const owner = await Db.collections.User!.findOneOrFail(); + + const authOwnerAgent = utils.createAgent(app, { apiPath: 'public', auth: true, user: owner }); + + await testDb.createUser(); + + const response = await authOwnerAgent.get('/v1/users'); + + expect(response.statusCode).toBe(200); + expect(response.body.users.length).toBe(2); + expect(response.body.nextCursor).toBeNull(); + + for (const user of response.body.users) { + const { + id, + email, + firstName, + lastName, + personalizationAnswers, + globalRole, + password, + resetPasswordToken, + isPending, + createdAt, + updatedAt, + } = user; + + expect(validator.isUUID(id)).toBe(true); + expect(email).toBeDefined(); + expect(firstName).toBeDefined(); + expect(lastName).toBeDefined(); + expect(personalizationAnswers).toBeUndefined(); + expect(password).toBeUndefined(); + expect(resetPasswordToken).toBeUndefined(); + //virtual method not working + //expect(isPending).toBe(false); + expect(globalRole).toBeUndefined(); + expect(createdAt).toBeDefined(); + expect(updatedAt).toBeDefined(); + } +}); + +const INITIAL_TEST_USER = { + id: uuid(), + email: randomEmail(), + firstName: randomName(), + lastName: randomName(), + password: randomValidPassword(), + apiKey: randomApiKey(), +}; \ No newline at end of file diff --git a/packages/cli/test/integration/shared/constants.ts b/packages/cli/test/integration/shared/constants.ts index 99c624efb4..d3add721b0 100644 --- a/packages/cli/test/integration/shared/constants.ts +++ b/packages/cli/test/integration/shared/constants.ts @@ -2,6 +2,8 @@ import config = require('../../../config'); export const REST_PATH_SEGMENT = config.get('endpoints.rest') as Readonly; +export const PUBLIC_API_REST_PATH_SEGMENT = config.get('publicApiEndpoints.path') as Readonly; + export const AUTHLESS_ENDPOINTS: Readonly = [ 'healthz', 'metrics', diff --git a/packages/cli/test/integration/shared/random.ts b/packages/cli/test/integration/shared/random.ts index 4a531aba3b..c291b48e3a 100644 --- a/packages/cli/test/integration/shared/random.ts +++ b/packages/cli/test/integration/shared/random.ts @@ -10,6 +10,10 @@ export function randomString(min: number, max: number) { return randomBytes(randomInteger / 2).toString('hex'); } +export function randomApiKey() { + return randomBytes(20).toString('hex'); +} + const chooseRandomly = (array: T[]) => array[Math.floor(Math.random() * array.length)]; const randomDigit = () => Math.floor(Math.random() * 10); diff --git a/packages/cli/test/integration/shared/testDb.ts b/packages/cli/test/integration/shared/testDb.ts index 7c64dcfd90..3710375802 100644 --- a/packages/cli/test/integration/shared/testDb.ts +++ b/packages/cli/test/integration/shared/testDb.ts @@ -4,7 +4,7 @@ import { Credentials, UserSettings } from 'n8n-core'; import config = require('../../../config'); import { BOOTSTRAP_MYSQL_CONNECTION_NAME, BOOTSTRAP_POSTGRES_CONNECTION_NAME } from './constants'; import { DatabaseType, Db, ICredentialsDb, IDatabaseCollections } from '../../../src'; -import { randomEmail, randomName, randomString, randomValidPassword } from './random'; +import { randomApiKey, randomEmail, randomName, randomString, randomValidPassword } from './random'; import { CredentialsEntity } from '../../../src/databases/entities/CredentialsEntity'; import { RESPONSE_ERROR_MESSAGES } from '../../../src/constants'; @@ -185,13 +185,14 @@ export async function saveCredential( * Store a user in the DB, defaulting to a `member`. */ export async function createUser(attributes: Partial = {}): Promise { - const { email, password, firstName, lastName, globalRole, ...rest } = attributes; + const { email, password, firstName, lastName, globalRole, apiKey, ...rest } = attributes; const user = { email: email ?? randomEmail(), password: password ?? randomValidPassword(), firstName: firstName ?? randomName(), lastName: lastName ?? randomName(), globalRole: globalRole ?? (await getGlobalMemberRole()), + apiKey: apiKey?? randomApiKey(), ...rest, }; diff --git a/packages/cli/test/integration/shared/types.d.ts b/packages/cli/test/integration/shared/types.d.ts index b160faea01..526f0887a7 100644 --- a/packages/cli/test/integration/shared/types.d.ts +++ b/packages/cli/test/integration/shared/types.d.ts @@ -13,7 +13,9 @@ export type SmtpTestAccount = { }; }; -type EndpointGroup = 'me' | 'users' | 'auth' | 'owner' | 'passwordReset' | 'credentials'; +export type ApiPath = 'internal' | 'public'; + +type EndpointGroup = 'me' | 'users' | 'auth' | 'owner' | 'passwordReset' | 'credentials' | 'publicApi'; export type CredentialPayload = { name: string; diff --git a/packages/cli/test/integration/shared/utils.ts b/packages/cli/test/integration/shared/utils.ts index 45f97cfabf..e616073a6a 100644 --- a/packages/cli/test/integration/shared/utils.ts +++ b/packages/cli/test/integration/shared/utils.ts @@ -11,7 +11,7 @@ import { INodeTypes, LoggerProxy } from 'n8n-workflow'; import { UserSettings } from 'n8n-core'; import config = require('../../../config'); -import { AUTHLESS_ENDPOINTS, REST_PATH_SEGMENT } from './constants'; +import { AUTHLESS_ENDPOINTS, PUBLIC_API_REST_PATH_SEGMENT, REST_PATH_SEGMENT } from './constants'; import { AUTH_COOKIE_NAME } from '../../../src/constants'; import { addRoutes as authMiddleware } from '../../../src/UserManagement/routes'; import { Db, ExternalHooks, InternalHooksManager } from '../../../src'; @@ -23,10 +23,10 @@ import { passwordResetNamespace as passwordResetEndpoints } from '../../../src/U import { issueJWT } from '../../../src/UserManagement/auth/jwt'; import { getLogger } from '../../../src/Logger'; import { credentialsController } from '../../../src/api/credentials.api'; - +import { publicApiController } from '../../../src/PublicApi/v1/'; import type { User } from '../../../src/databases/entities/User'; import { Telemetry } from '../../../src/telemetry'; -import type { EndpointGroup, SmtpTestAccount } from './types'; +import type { ApiPath, EndpointGroup, SmtpTestAccount } from './types'; import type { N8nApp } from '../../../src/UserManagement/Interfaces'; /** @@ -45,6 +45,7 @@ export function initTestServer({ const testServer = { app: express(), restEndpoint: REST_PATH_SEGMENT, + publicApiEndpoint: PUBLIC_API_REST_PATH_SEGMENT, ...(endpointGroups?.includes('credentials') ? { externalHooks: ExternalHooks() } : {}), }; @@ -65,10 +66,15 @@ export function initTestServer({ if (routerEndpoints.length) { const map: Record = { credentials: credentialsController, + publicApi: publicApiController, }; for (const group of routerEndpoints) { - testServer.app.use(`/${testServer.restEndpoint}/${group}`, map[group]); + if (group === 'publicApi') { + testServer.app.use(`/${testServer.publicApiEndpoint}`, map[group]); + } else { + testServer.app.use(`/${testServer.restEndpoint}/${group}`, map[group]); + } } } @@ -106,7 +112,7 @@ const classifyEndpointGroups = (endpointGroups: string[]) => { const functionEndpoints: string[] = []; endpointGroups.forEach((group) => - (group === 'credentials' ? routerEndpoints : functionEndpoints).push(group), + (group === 'credentials' || group === 'publicApi' ? routerEndpoints : functionEndpoints).push(group), ); return [routerEndpoints, functionEndpoints]; @@ -143,13 +149,24 @@ export function initConfigFile() { /** * Create a request agent, optionally with an auth cookie. */ -export function createAgent(app: express.Application, options?: { auth: true; user: User }) { +export function createAgent(app: express.Application, options?: { apiPath?: ApiPath, auth: boolean; user: User }) { const agent = request.agent(app); - agent.use(prefix(REST_PATH_SEGMENT)); - if (options?.auth && options?.user) { - const { token } = issueJWT(options.user); - agent.jar.setCookie(`${AUTH_COOKIE_NAME}=${token}`); + if (options?.apiPath === undefined || options?.apiPath === 'internal') { + agent.use(prefix(REST_PATH_SEGMENT)); + + if (options?.auth && options?.user) { + const { token } = issueJWT(options.user); + agent.jar.setCookie(`${AUTH_COOKIE_NAME}=${token}`); + } + } + + if (options?.apiPath === 'public') { + agent.use(prefix(PUBLIC_API_REST_PATH_SEGMENT)); + + if (options?.auth && options?.user.apiKey) { + agent.set({ 'X-N8N-API-KEY': options.user.apiKey }); + } } return agent; @@ -171,7 +188,6 @@ export function prefix(pathSegment: string) { url.pathname = pathSegment + url.pathname; request.url = url.toString(); - return request; }; }