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

feat(core): Add rsa option to ssh key generation #7154

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
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
9 changes: 9 additions & 0 deletions packages/cli/src/config/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1204,4 +1204,13 @@ export const schema = {
env: 'N8N_AI_ENABLED',
},
},

sourceControl: {
defaultKeyPairType: {
doc: 'Default SSH key type to use when generating SSH keys',
format: ['rsa', 'ed25519'] as const,
default: 'ed25519',
env: 'N8N_SOURCECONTROL_DEFAULT_SSH_KEY_TYPE',
},
},
};
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ export class SourceControlController {
) {
await this.sourceControlService.setBranch(sanitizedPreferences.branchName);
}
if (sanitizedPreferences.branchColor || sanitizedPreferences.branchReadOnly !== undefined) {
if (sanitizedPreferences.branchColor ?? sanitizedPreferences.branchReadOnly !== undefined) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't this assign the value of sanitizedPreferences.branchReadOnly !== undefined (which should be a boolean) to sanitizedPreferences.branchColor if the latter is nullish? Is this the intended behavior?

await this.sourceControlPreferencesService.setPreferences(
{
branchColor: sanitizedPreferences.branchColor,
Expand Down Expand Up @@ -237,9 +237,12 @@ export class SourceControlController {

@Authorized(['global', 'owner'])
@Post('/generate-key-pair', { middlewares: [sourceControlLicensedMiddleware] })
async generateKeyPair(): Promise<SourceControlPreferences> {
async generateKeyPair(
req: SourceControlRequest.GenerateKeyPair,
): Promise<SourceControlPreferences> {
try {
const result = await this.sourceControlPreferencesService.generateAndSaveKeyPair();
const keyPairType = req.body.keyGeneratorType;
const result = await this.sourceControlPreferencesService.generateAndSaveKeyPair(keyPairType);
return result;
} catch (error) {
throw new BadRequestError((error as { message: string }).message);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
} from './constants';
import type { SourceControlledFile } from './types/sourceControlledFile';
import path from 'path';
import type { KeyPairType } from './types/keyPairType';

export function stringContainsExpression(testString: string): boolean {
return /^=.*\{\{.*\}\}/.test(testString);
Expand Down Expand Up @@ -63,7 +64,7 @@ export function isSourceControlLicensed() {
return license.isSourceControlLicensed();
}

export async function generateSshKeyPair(keyType: 'ed25519' | 'rsa' = 'ed25519') {
export async function generateSshKeyPair(keyType: KeyPairType) {
const sshpk = await import('sshpk');
const keyPair: KeyPair = {
publicKey: '',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import {
SOURCE_CONTROL_PREFERENCES_DB_KEY,
} from './constants';
import path from 'path';
import type { KeyPairType } from './types/keyPairType';
import config from '@/config';

@Service()
export class SourceControlPreferencesService {
Expand Down Expand Up @@ -86,9 +88,15 @@ export class SourceControlPreferencesService {
* Will generate an ed25519 key pair and save it to the database and the file system
* Note: this will overwrite any existing key pair
*/
async generateAndSaveKeyPair(): Promise<SourceControlPreferences> {
async generateAndSaveKeyPair(keyPairType?: KeyPairType): Promise<SourceControlPreferences> {
sourceControlFoldersExistCheck([this.gitFolder, this.sshFolder]);
const keyPair = await generateSshKeyPair('ed25519');
if (!keyPairType) {
keyPairType =
this.getPreferences().keyGeneratorType ??
(config.get('sourceControl.defaultKeyPairType') as KeyPairType) ??
'ed25519';
}
const keyPair = await generateSshKeyPair(keyPairType);
if (keyPair.publicKey && keyPair.privateKey) {
try {
await fsWriteFile(this.sshKeyName + '.pub', keyPair.publicKey, {
Expand All @@ -100,6 +108,10 @@ export class SourceControlPreferencesService {
throw Error(`Failed to save key pair: ${(error as Error).message}`);
}
}
// update preferences only after generating key pair to prevent endless loop
if (keyPairType !== this.getPreferences().keyGeneratorType) {
await this.setPreferences({ keyGeneratorType: keyPairType });
}
return this.getPreferences();
}

Expand Down Expand Up @@ -146,8 +158,11 @@ export class SourceControlPreferencesService {
): Promise<SourceControlPreferences> {
sourceControlFoldersExistCheck([this.gitFolder, this.sshFolder]);
if (!this.hasKeyPairFiles()) {
LoggerProxy.debug('No key pair files found, generating new pair');
await this.generateAndSaveKeyPair();
const keyPairType =
preferences.keyGeneratorType ??
(config.get('sourceControl.defaultKeyPairType') as KeyPairType);
LoggerProxy.debug(`No key pair files found, generating new pair using type: ${keyPairType}`);
await this.generateAndSaveKeyPair(keyPairType);
}
this.sourceControlPreferences = preferences;
if (saveToDb) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export type KeyPairType = 'ed25519' | 'rsa';
2 changes: 2 additions & 0 deletions packages/cli/src/environments/sourceControl/types/requests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import type { SourceControlPullWorkFolder } from './sourceControlPullWorkFolder'
import type { SourceControlDisconnect } from './sourceControlDisconnect';
import type { SourceControlSetReadOnly } from './sourceControlSetReadOnly';
import type { SourceControlGetStatus } from './sourceControlGetStatus';
import type { SourceControlGenerateKeyPair } from './sourceControlGenerateKeyPair';

export declare namespace SourceControlRequest {
type UpdatePreferences = AuthenticatedRequest<{}, {}, Partial<SourceControlPreferences>, {}>;
Expand All @@ -21,4 +22,5 @@ export declare namespace SourceControlRequest {
type PushWorkFolder = AuthenticatedRequest<{}, {}, SourceControlPushWorkFolder, {}>;
type PullWorkFolder = AuthenticatedRequest<{}, {}, SourceControlPullWorkFolder, {}>;
type GetStatus = AuthenticatedRequest<{}, {}, {}, SourceControlGetStatus>;
type GenerateKeyPair = AuthenticatedRequest<{}, {}, SourceControlGenerateKeyPair, {}>;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { IsOptional, IsString } from 'class-validator';
import { KeyPairType } from './keyPairType';

export class SourceControlGenerateKeyPair {
@IsOptional()
@IsString()
readonly keyGeneratorType?: KeyPairType;
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { IsBoolean, IsHexColor, IsOptional, IsString } from 'class-validator';
import { KeyPairType } from './keyPairType';

export class SourceControlPreferences {
constructor(preferences: Partial<SourceControlPreferences> | undefined = undefined) {
Expand Down Expand Up @@ -28,6 +29,10 @@ export class SourceControlPreferences {
@IsBoolean()
readonly initRepo?: boolean;

@IsOptional()
@IsString()
readonly keyGeneratorType?: KeyPairType;

static fromJSON(json: Partial<SourceControlPreferences>): SourceControlPreferences {
return new SourceControlPreferences(json);
}
Expand All @@ -42,6 +47,7 @@ export class SourceControlPreferences {
branchName: preferences.branchName ?? defaultPreferences.branchName,
branchReadOnly: preferences.branchReadOnly ?? defaultPreferences.branchReadOnly,
branchColor: preferences.branchColor ?? defaultPreferences.branchColor,
keyGeneratorType: preferences.keyGeneratorType ?? defaultPreferences.keyGeneratorType,
});
}
}
18 changes: 18 additions & 0 deletions packages/cli/test/integration/environments/SourceControl.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import * as utils from '../shared/utils/';
import type { User } from '@db/entities/User';
import * as UserManagementHelpers from '@/UserManagement/UserManagementHelper';
import Container from 'typedi';
import config from '@/config';
import { License } from '@/License';
import { SourceControlPreferencesService } from '@/environments/sourceControl/sourceControlPreferences.service.ee';
import { SourceControlService } from '@/environments/sourceControl/sourceControl.service.ee';
Expand Down Expand Up @@ -69,4 +70,21 @@ describe('GET /sourceControl/preferences', () => {
expect(data[0].id).toBe('haQetoXq9GxHSkft');
});
});

test('refreshing key pairsshould return new rsa key', async () => {
config.set('sourceControl.defaultKeyPairType', 'rsa');
await authOwnerAgent
.post(`/${SOURCE_CONTROL_API_ROOT}/generate-key-pair`)
.send()
.expect(200)
.expect((res) => {
expect(
Container.get(SourceControlPreferencesService).getPreferences().keyGeneratorType,
).toBe('rsa');
expect(res.body.data).toHaveProperty('publicKey');
expect(res.body.data).toHaveProperty('keyGeneratorType');
expect(res.body.data.keyGeneratorType).toBe('rsa');
expect(res.body.data.publicKey).toContain('ssh-rsa');
});
});
});
13 changes: 11 additions & 2 deletions packages/cli/test/unit/SourceControl.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { LoggerProxy } from 'n8n-workflow';
import { getLogger } from '@/Logger';
import { constants as fsConstants, accessSync } from 'fs';
import type { SourceControlledFile } from '@/environments/sourceControl/types/sourceControlledFile';
import type { SourceControlPreferences } from '@/environments/sourceControl/types/sourceControlPreferences';

const pushResult: SourceControlledFile[] = [
{
Expand Down Expand Up @@ -167,13 +168,21 @@ beforeAll(async () => {

describe('Source Control', () => {
it('should generate an SSH key pair', async () => {
const keyPair = await generateSshKeyPair();
const keyPair = await generateSshKeyPair('ed25519');
expect(keyPair.privateKey).toBeTruthy();
expect(keyPair.privateKey).toContain('BEGIN OPENSSH PRIVATE KEY');
expect(keyPair.publicKey).toBeTruthy();
expect(keyPair.publicKey).toContain('ssh-ed25519');
});

it('should generate an RSA key pair', async () => {
const keyPair = await generateSshKeyPair('rsa');
expect(keyPair.privateKey).toBeTruthy();
expect(keyPair.privateKey).toContain('BEGIN OPENSSH PRIVATE KEY');
expect(keyPair.publicKey).toBeTruthy();
expect(keyPair.publicKey).toContain('ssh-rsa');
});

it('should check for git and ssh folders and create them if required', async () => {
const userFolder = UserSettings.getUserN8nFolderPath();
const sshFolder = path.join(userFolder, SOURCE_CONTROL_SSH_FOLDER);
Expand Down Expand Up @@ -242,7 +251,7 @@ describe('Source Control', () => {
});

it('should class validate correct preferences', async () => {
const validPreferences = {
const validPreferences: Partial<SourceControlPreferences> = {
branchName: 'main',
repositoryUrl: 'git@example.com:n8ntest/n8n_testrepo.git',
branchReadOnly: false,
Expand Down