From cf70b06545b4f7dc97abf04f9dd63a2408bbd861 Mon Sep 17 00:00:00 2001 From: Ricardo Espinoza Date: Mon, 29 Jul 2024 14:13:54 -0400 Subject: [PATCH] feat(core): Show Public API key value only once (no-changelog) (#10126) --- packages/cli/src/controllers/me.controller.ts | 15 ++++++++++++++- packages/cli/src/databases/entities/User.ts | 2 +- packages/cli/test/integration/me.api.test.ts | 12 ++++++------ .../test/unit/controllers/me.controller.test.ts | 8 ++++---- packages/editor-ui/src/components/CopyInput.vue | 11 ++++++++++- .../editor-ui/src/plugins/i18n/locales/en.json | 1 + packages/editor-ui/src/views/SettingsApiView.vue | 7 ++++++- 7 files changed, 42 insertions(+), 14 deletions(-) diff --git a/packages/cli/src/controllers/me.controller.ts b/packages/cli/src/controllers/me.controller.ts index 1e3fc22ca2..62a2e101ff 100644 --- a/packages/cli/src/controllers/me.controller.ts +++ b/packages/cli/src/controllers/me.controller.ts @@ -25,6 +25,8 @@ import { UserRepository } from '@/databases/repositories/user.repository'; import { isApiEnabled } from '@/PublicApi'; import { EventService } from '@/eventbus/event.service'; +export const API_KEY_PREFIX = 'n8n_api_'; + export const isApiEnabledMiddleware: RequestHandler = (_, res, next) => { if (isApiEnabled()) { next(); @@ -208,7 +210,8 @@ export class MeController { */ @Get('/api-key', { middlewares: [isApiEnabledMiddleware] }) async getAPIKey(req: AuthenticatedRequest) { - return { apiKey: req.user.apiKey }; + const apiKey = this.redactApiKey(req.user.apiKey); + return { apiKey }; } /** @@ -242,4 +245,14 @@ 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) + ); + } } diff --git a/packages/cli/src/databases/entities/User.ts b/packages/cli/src/databases/entities/User.ts index 5755f24d84..d24af19742 100644 --- a/packages/cli/src/databases/entities/User.ts +++ b/packages/cli/src/databases/entities/User.ts @@ -104,7 +104,7 @@ export class User extends WithTimestamps implements IUser { @Column({ type: String, nullable: true }) @Index({ unique: true }) - apiKey?: string | null; + apiKey: string | null; @Column({ type: Boolean, default: false }) mfaEnabled: boolean; diff --git a/packages/cli/test/integration/me.api.test.ts b/packages/cli/test/integration/me.api.test.ts index 30e2ff27d1..ef9757dc44 100644 --- a/packages/cli/test/integration/me.api.test.ts +++ b/packages/cli/test/integration/me.api.test.ts @@ -175,14 +175,14 @@ describe('Owner shell', () => { expect(storedShellOwner.apiKey).toEqual(response.body.data.apiKey); }); - test('GET /me/api-key should fetch the api key', async () => { + test('GET /me/api-key should fetch the api key redacted', async () => { const response = await authOwnerShellAgent.get('/me/api-key'); expect(response.statusCode).toBe(200); - expect(response.body.data.apiKey).toEqual(ownerShell.apiKey); + expect(response.body.data.apiKey).not.toEqual(ownerShell.apiKey); }); - test('DELETE /me/api-key should fetch the api key', async () => { + test('DELETE /me/api-key should delete the api key', async () => { const response = await authOwnerShellAgent.delete('/me/api-key'); expect(response.statusCode).toBe(200); @@ -327,14 +327,14 @@ describe('Member', () => { expect(storedMember.apiKey).toEqual(response.body.data.apiKey); }); - test('GET /me/api-key should fetch the api key', async () => { + test('GET /me/api-key should fetch the api key redacted', async () => { const response = await testServer.authAgentFor(member).get('/me/api-key'); expect(response.statusCode).toBe(200); - expect(response.body.data.apiKey).toEqual(member.apiKey); + expect(response.body.data.apiKey).not.toEqual(member.apiKey); }); - test('DELETE /me/api-key should fetch the api key', async () => { + test('DELETE /me/api-key should delete the api key', async () => { const response = await testServer.authAgentFor(member).delete('/me/api-key'); expect(response.statusCode).toBe(200); diff --git a/packages/cli/test/unit/controllers/me.controller.test.ts b/packages/cli/test/unit/controllers/me.controller.test.ts index 6a9bff1251..2416ecafbd 100644 --- a/packages/cli/test/unit/controllers/me.controller.test.ts +++ b/packages/cli/test/unit/controllers/me.controller.test.ts @@ -4,7 +4,7 @@ import jwt from 'jsonwebtoken'; import { mock, anyObject } from 'jest-mock-extended'; import type { PublicUser } from '@/Interfaces'; import type { User } from '@db/entities/User'; -import { MeController } from '@/controllers/me.controller'; +import { API_KEY_PREFIX, MeController } from '@/controllers/me.controller'; import { AUTH_COOKIE_NAME } from '@/constants'; import type { AuthenticatedRequest, MeRequest } from '@/requests'; import { UserService } from '@/services/user.service'; @@ -223,7 +223,7 @@ describe('MeController', () => { describe('API Key methods', () => { let req: AuthenticatedRequest; beforeAll(() => { - req = mock({ user: mock>({ id: '123', apiKey: 'test-key' }) }); + req = mock({ user: mock>({ id: '123', apiKey: `${API_KEY_PREFIX}test-key` }) }); }); describe('createAPIKey', () => { @@ -234,9 +234,9 @@ describe('MeController', () => { }); describe('getAPIKey', () => { - it('should return the users api key', async () => { + it('should return the users api key redacted', async () => { const { apiKey } = await controller.getAPIKey(req); - expect(apiKey).toEqual(req.user.apiKey); + expect(apiKey).not.toEqual(req.user.apiKey); }); }); diff --git a/packages/editor-ui/src/components/CopyInput.vue b/packages/editor-ui/src/components/CopyInput.vue index e25a37007d..8171885b26 100644 --- a/packages/editor-ui/src/components/CopyInput.vue +++ b/packages/editor-ui/src/components/CopyInput.vue @@ -6,13 +6,14 @@ [$style.copyText]: true, [$style[size]]: true, [$style.collapsed]: collapse, + [$style.noHover]: disableCopy, 'ph-no-capture': redactValue, }" data-test-id="copy-input" @click="copy" > {{ value }} -
+
{{ copyButtonText }}
@@ -36,6 +37,7 @@ type Props = { size?: 'medium' | 'large'; collapse?: boolean; redactValue?: boolean; + disableCopy: boolean; }; const props = withDefaults(defineProps(), { @@ -46,6 +48,7 @@ const props = withDefaults(defineProps(), { size: 'medium', copyButtonText: useI18n().baseText('generic.copy'), toastTitle: useI18n().baseText('generic.copiedToClipboard'), + disableCopy: false, }); const emit = defineEmits<{ copy: []; @@ -55,6 +58,8 @@ const clipboard = useClipboard(); const { showMessage } = useToast(); function copy() { + if (props.disableCopy) return; + emit('copy'); void clipboard.copy(props.value ?? ''); @@ -88,6 +93,10 @@ function copy() { } } +.noHover { + cursor: default; +} + .large { span { font-size: var(--font-size-s); diff --git a/packages/editor-ui/src/plugins/i18n/locales/en.json b/packages/editor-ui/src/plugins/i18n/locales/en.json index 5823850ff7..b86ebe0f3e 100644 --- a/packages/editor-ui/src/plugins/i18n/locales/en.json +++ b/packages/editor-ui/src/plugins/i18n/locales/en.json @@ -1724,6 +1724,7 @@ "settings.api.view.copy.toast": "API Key copied to clipboard", "settings.api.view.apiPlayground": "API Playground", "settings.api.view.info": "Use your API Key to control n8n programmatically using the {apiAction}. But if you only want to trigger workflows, consider using the {webhookAction} instead.", + "settings.api.view.copy": "Make sure to copy your API key now as you will not be able to see this again.", "settings.api.view.info.api": "n8n API", "settings.api.view.info.webhook": "webhook node", "settings.api.view.myKey": "My API Key", diff --git a/packages/editor-ui/src/views/SettingsApiView.vue b/packages/editor-ui/src/views/SettingsApiView.vue index 7681f872b9..22219ef339 100644 --- a/packages/editor-ui/src/views/SettingsApiView.vue +++ b/packages/editor-ui/src/views/SettingsApiView.vue @@ -43,11 +43,13 @@ :copy-button-text="$locale.baseText('generic.clickToCopy')" :toast-title="$locale.baseText('settings.api.view.copy.toast')" :redact-value="true" + :disable-copy="isRedactedApiKey" + :hint="!isRedactedApiKey ? $locale.baseText('settings.api.view.copy') : ''" @copy="onCopy" /> -
+
{{ $locale.baseText(`settings.api.view.${swaggerUIEnabled ? 'tryapi' : 'more-details'}`) @@ -146,6 +148,9 @@ export default defineComponent({ isPublicApiEnabled(): boolean { return this.settingsStore.isPublicApiEnabled; }, + isRedactedApiKey(): boolean { + return this.apiKey.includes('*'); + }, }, methods: { onUpgrade() {