Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(core): Block Public API related REST calls when Public API is not enabled #9521

Merged
merged 3 commits into from
May 28, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions packages/cli/src/PublicApi/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,11 +153,6 @@ export const loadPublicApiVersions = async (
};
};

function isApiEnabledByLicense(): boolean {
const license = Container.get(License);
return !license.isAPIDisabled();
}

export function isApiEnabled(): boolean {
return !config.get('publicApi.disabled') && isApiEnabledByLicense();
return !config.get('publicApi.disabled') && !Container.get(License).isAPIDisabled();
}
17 changes: 13 additions & 4 deletions packages/cli/src/controllers/me.controller.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import validator from 'validator';
import { plainToInstance } from 'class-transformer';
import { Response } from 'express';
import { type RequestHandler, Response } from 'express';
import { randomBytes } from 'crypto';

import { AuthService } from '@/auth/auth.service';
Expand All @@ -22,6 +22,15 @@ import { ExternalHooks } from '@/ExternalHooks';
import { InternalHooks } from '@/InternalHooks';
import { BadRequestError } from '@/errors/response-errors/bad-request.error';
import { UserRepository } from '@/databases/repositories/user.repository';
import { isApiEnabled } from '@/PublicApi';

export const isApiEnabledMiddleware: RequestHandler = (_, res, next) => {
if (isApiEnabled()) {
next();
} else {
res.status(404).end();
}
};

@RestController('/me')
export class MeController {
Expand Down Expand Up @@ -185,7 +194,7 @@ export class MeController {
/**
* Creates an API Key
*/
@Post('/api-key')
@Post('/api-key', { middlewares: [isApiEnabledMiddleware] })
async createAPIKey(req: AuthenticatedRequest) {
const apiKey = `n8n_api_${randomBytes(40).toString('hex')}`;

Expand All @@ -202,15 +211,15 @@ export class MeController {
/**
* Get an API Key
*/
@Get('/api-key')
@Get('/api-key', { middlewares: [isApiEnabledMiddleware] })
async getAPIKey(req: AuthenticatedRequest) {
return { apiKey: req.user.apiKey };
}

/**
* Deletes an API Key
*/
@Delete('/api-key')
@Delete('/api-key', { middlewares: [isApiEnabledMiddleware] })
async deleteAPIKey(req: AuthenticatedRequest) {
await this.userService.update(req.user.id, { apiKey: null });

Expand Down
3 changes: 2 additions & 1 deletion packages/cli/src/services/frontend.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import type { CommunityPackagesService } from '@/services/communityPackages.serv
import { Logger } from '@/Logger';
import { UrlService } from './url.service';
import { InternalHooks } from '@/InternalHooks';
import { isApiEnabled } from '@/PublicApi';

@Service()
export class FrontendService {
Expand Down Expand Up @@ -143,7 +144,7 @@ export class FrontendService {
},
},
publicApi: {
enabled: !config.get('publicApi.disabled') && !this.license.isAPIDisabled(),
enabled: isApiEnabled(),
despairblue marked this conversation as resolved.
Show resolved Hide resolved
latestVersion: 1,
path: config.getEnv('publicApi.path'),
swaggerUi: {
Expand Down
22 changes: 19 additions & 3 deletions packages/cli/test/integration/me.api.test.ts
netroy marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
import { Container } from 'typedi';
import type { SuperAgentTest } from 'supertest';
import { IsNull } from '@n8n/typeorm';
import validator from 'validator';

import type { User } from '@db/entities/User';
import { UserRepository } from '@db/repositories/user.repository';
import { ProjectRepository } from '@db/repositories/project.repository';

import { SUCCESS_RESPONSE_BODY } from './shared/constants';
import {
randomApiKey,
Expand All @@ -13,14 +18,13 @@ import {
import * as testDb from './shared/testDb';
import * as utils from './shared/utils/';
import { addApiKey, createUser, createUserShell } from './shared/db/users';
import Container from 'typedi';
import { UserRepository } from '@db/repositories/user.repository';
import { ProjectRepository } from '@/databases/repositories/project.repository';
import config from '@/config';

const testServer = utils.setupTestServer({ endpointGroups: ['me'] });

beforeEach(async () => {
await testDb.truncate(['User']);
config.set('publicApi.disabled', false);
});

describe('Owner shell', () => {
Expand Down Expand Up @@ -151,13 +155,25 @@ describe('Owner shell', () => {
expect(storedShellOwner.apiKey).toEqual(response.body.data.apiKey);
});

test('POST /me/api-key 404 when the API is disabled', async () => {
config.set('publicApi.disabled', true);
const response = await authOwnerShellAgent.post('/me/api-key');
expect(response.statusCode).toBe(404);
});

test('GET /me/api-key should fetch the api key', async () => {
const response = await authOwnerShellAgent.get('/me/api-key');

expect(response.statusCode).toBe(200);
expect(response.body.data.apiKey).toEqual(ownerShell.apiKey);
});

test('GET /me/api-key should 404 when the API is disabled', async () => {
config.set('publicApi.disabled', true);
const response = await authOwnerShellAgent.get('/me/api-key');
expect(response.statusCode).toBe(404);
});

test('DELETE /me/api-key should fetch the api key', async () => {
const response = await authOwnerShellAgent.delete('/me/api-key');

Expand Down
Loading