refactor: Move API keys into their own table (no-changelog) (#10629)
Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <aditya@netroy.in>
This commit is contained in:
@@ -2,11 +2,14 @@ import { UserUpdateRequestDto } from '@n8n/api-types';
|
||||
import type { Response } from 'express';
|
||||
import { mock, anyObject } from 'jest-mock-extended';
|
||||
import jwt from 'jsonwebtoken';
|
||||
import { randomString } from 'n8n-workflow';
|
||||
import { Container } from 'typedi';
|
||||
|
||||
import { AUTH_COOKIE_NAME } from '@/constants';
|
||||
import { API_KEY_PREFIX, MeController } from '@/controllers/me.controller';
|
||||
import { MeController } from '@/controllers/me.controller';
|
||||
import type { ApiKey } from '@/databases/entities/api-key';
|
||||
import type { User } from '@/databases/entities/user';
|
||||
import { ApiKeyRepository } from '@/databases/repositories/api-key.repository';
|
||||
import { AuthUserRepository } from '@/databases/repositories/auth-user.repository';
|
||||
import { InvalidAuthTokenRepository } from '@/databases/repositories/invalid-auth-token.repository';
|
||||
import { UserRepository } from '@/databases/repositories/user.repository';
|
||||
@@ -18,6 +21,7 @@ import type { PublicUser } from '@/interfaces';
|
||||
import { License } from '@/license';
|
||||
import { MfaService } from '@/mfa/mfa.service';
|
||||
import type { AuthenticatedRequest, MeRequest } from '@/requests';
|
||||
import { API_KEY_PREFIX } from '@/services/public-api-key.service';
|
||||
import { UserService } from '@/services/user.service';
|
||||
import { mockInstance } from '@test/mocking';
|
||||
import { badPasswords } from '@test/test-data';
|
||||
@@ -30,6 +34,7 @@ describe('MeController', () => {
|
||||
const userService = mockInstance(UserService);
|
||||
const userRepository = mockInstance(UserRepository);
|
||||
const mockMfaService = mockInstance(MfaService);
|
||||
const apiKeysRepository = mockInstance(ApiKeyRepository);
|
||||
mockInstance(AuthUserRepository);
|
||||
mockInstance(InvalidAuthTokenRepository);
|
||||
mockInstance(License).isWithinUsersLimit.mockReturnValue(true);
|
||||
@@ -412,27 +417,63 @@ describe('MeController', () => {
|
||||
describe('API Key methods', () => {
|
||||
let req: AuthenticatedRequest;
|
||||
beforeAll(() => {
|
||||
req = mock({ user: mock<Partial<User>>({ id: '123', apiKey: `${API_KEY_PREFIX}test-key` }) });
|
||||
req = mock<AuthenticatedRequest>({ user: mock<User>({ id: '123' }) });
|
||||
});
|
||||
|
||||
describe('createAPIKey', () => {
|
||||
it('should create and save an API key', async () => {
|
||||
const { apiKey } = await controller.createAPIKey(req);
|
||||
expect(userService.update).toHaveBeenCalledWith(req.user.id, { apiKey });
|
||||
const apiKeyData = {
|
||||
id: '123',
|
||||
userId: '123',
|
||||
label: 'My API Key',
|
||||
apiKey: `${API_KEY_PREFIX}${randomString(42)}`,
|
||||
createdAt: new Date(),
|
||||
} as ApiKey;
|
||||
|
||||
apiKeysRepository.upsert.mockImplementation();
|
||||
|
||||
apiKeysRepository.findOneByOrFail.mockResolvedValue(apiKeyData);
|
||||
|
||||
const newApiKey = await controller.createAPIKey(req);
|
||||
|
||||
expect(apiKeysRepository.upsert).toHaveBeenCalled();
|
||||
expect(apiKeyData).toEqual(newApiKey);
|
||||
});
|
||||
});
|
||||
|
||||
describe('getAPIKey', () => {
|
||||
it('should return the users api key redacted', async () => {
|
||||
const { apiKey } = await controller.getAPIKey(req);
|
||||
expect(apiKey).not.toEqual(req.user.apiKey);
|
||||
describe('getAPIKeys', () => {
|
||||
it('should return the users api keys redacted', async () => {
|
||||
const apiKeyData = {
|
||||
id: '123',
|
||||
userId: '123',
|
||||
label: 'My API Key',
|
||||
apiKey: `${API_KEY_PREFIX}${randomString(42)}`,
|
||||
createdAt: new Date(),
|
||||
} as ApiKey;
|
||||
|
||||
apiKeysRepository.findBy.mockResolvedValue([apiKeyData]);
|
||||
|
||||
const apiKeys = await controller.getAPIKeys(req);
|
||||
expect(apiKeys[0].apiKey).not.toEqual(apiKeyData.apiKey);
|
||||
expect(apiKeysRepository.findBy).toHaveBeenCalledWith({ userId: req.user.id });
|
||||
});
|
||||
});
|
||||
|
||||
describe('deleteAPIKey', () => {
|
||||
it('should delete the API key', async () => {
|
||||
const user = mock<User>({
|
||||
id: '123',
|
||||
password: 'password',
|
||||
authIdentities: [],
|
||||
role: 'global:member',
|
||||
mfaEnabled: false,
|
||||
});
|
||||
const req = mock<MeRequest.DeleteAPIKey>({ user, params: { id: user.id } });
|
||||
await controller.deleteAPIKey(req);
|
||||
expect(userService.update).toHaveBeenCalledWith(req.user.id, { apiKey: null });
|
||||
expect(apiKeysRepository.delete).toHaveBeenCalledWith({
|
||||
userId: req.user.id,
|
||||
id: req.params.id,
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -4,7 +4,6 @@ import {
|
||||
UserUpdateRequestDto,
|
||||
} from '@n8n/api-types';
|
||||
import { plainToInstance } from 'class-transformer';
|
||||
import { randomBytes } from 'crypto';
|
||||
import { type RequestHandler, Response } from 'express';
|
||||
|
||||
import { AuthService } from '@/auth/auth.service';
|
||||
@@ -22,13 +21,12 @@ import { MfaService } from '@/mfa/mfa.service';
|
||||
import { isApiEnabled } from '@/public-api';
|
||||
import { AuthenticatedRequest, MeRequest } from '@/requests';
|
||||
import { PasswordUtility } from '@/services/password.utility';
|
||||
import { PublicApiKeyService } from '@/services/public-api-key.service';
|
||||
import { UserService } from '@/services/user.service';
|
||||
import { isSamlLicensedAndEnabled } from '@/sso/saml/saml-helpers';
|
||||
|
||||
import { PersonalizationSurveyAnswersV4 } from './survey-answers.dto';
|
||||
|
||||
export const API_KEY_PREFIX = 'n8n_api_';
|
||||
|
||||
export const isApiEnabledMiddleware: RequestHandler = (_, res, next) => {
|
||||
if (isApiEnabled()) {
|
||||
next();
|
||||
@@ -48,6 +46,7 @@ export class MeController {
|
||||
private readonly userRepository: UserRepository,
|
||||
private readonly eventService: EventService,
|
||||
private readonly mfaService: MfaService,
|
||||
private readonly publicApiKeyService: PublicApiKeyService,
|
||||
) {}
|
||||
|
||||
/**
|
||||
@@ -219,34 +218,32 @@ export class MeController {
|
||||
}
|
||||
|
||||
/**
|
||||
* Creates an API Key
|
||||
* Create an API Key
|
||||
*/
|
||||
@Post('/api-key', { middlewares: [isApiEnabledMiddleware] })
|
||||
@Post('/api-keys', { middlewares: [isApiEnabledMiddleware] })
|
||||
async createAPIKey(req: AuthenticatedRequest) {
|
||||
const apiKey = `n8n_api_${randomBytes(40).toString('hex')}`;
|
||||
|
||||
await this.userService.update(req.user.id, { apiKey });
|
||||
const newApiKey = await this.publicApiKeyService.createPublicApiKeyForUser(req.user);
|
||||
|
||||
this.eventService.emit('public-api-key-created', { user: req.user, publicApi: false });
|
||||
|
||||
return { apiKey };
|
||||
return newApiKey;
|
||||
}
|
||||
|
||||
/**
|
||||
* Get an API Key
|
||||
* Get API keys
|
||||
*/
|
||||
@Get('/api-key', { middlewares: [isApiEnabledMiddleware] })
|
||||
async getAPIKey(req: AuthenticatedRequest) {
|
||||
const apiKey = this.redactApiKey(req.user.apiKey);
|
||||
return { apiKey };
|
||||
@Get('/api-keys', { middlewares: [isApiEnabledMiddleware] })
|
||||
async getAPIKeys(req: AuthenticatedRequest) {
|
||||
const apiKeys = await this.publicApiKeyService.getRedactedApiKeysForUser(req.user);
|
||||
return apiKeys;
|
||||
}
|
||||
|
||||
/**
|
||||
* Deletes an API Key
|
||||
* Delete an API Key
|
||||
*/
|
||||
@Delete('/api-key', { middlewares: [isApiEnabledMiddleware] })
|
||||
async deleteAPIKey(req: AuthenticatedRequest) {
|
||||
await this.userService.update(req.user.id, { apiKey: null });
|
||||
@Delete('/api-keys/:id', { middlewares: [isApiEnabledMiddleware] })
|
||||
async deleteAPIKey(req: MeRequest.DeleteAPIKey) {
|
||||
await this.publicApiKeyService.deleteApiKeyForUser(req.user, req.params.id);
|
||||
|
||||
this.eventService.emit('public-api-key-deleted', { user: req.user, publicApi: false });
|
||||
|
||||
@@ -273,14 +270,4 @@ export class MeController {
|
||||
|
||||
return user.settings;
|
||||
}
|
||||
|
||||
private redactApiKey(apiKey: string | null) {
|
||||
if (!apiKey) return;
|
||||
const keepLength = 5;
|
||||
return (
|
||||
API_KEY_PREFIX +
|
||||
apiKey.slice(API_KEY_PREFIX.length, API_KEY_PREFIX.length + keepLength) +
|
||||
'*'.repeat(apiKey.length - API_KEY_PREFIX.length - keepLength)
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user