feat(core): Show Public API key value only once (no-changelog) (#10126)

This commit is contained in:
Ricardo Espinoza 2024-07-29 14:13:54 -04:00 committed by GitHub
parent de50ef7590
commit cf70b06545
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 42 additions and 14 deletions

View file

@ -25,6 +25,8 @@ import { UserRepository } from '@/databases/repositories/user.repository';
import { isApiEnabled } from '@/PublicApi'; import { isApiEnabled } from '@/PublicApi';
import { EventService } from '@/eventbus/event.service'; import { EventService } from '@/eventbus/event.service';
export const API_KEY_PREFIX = 'n8n_api_';
export const isApiEnabledMiddleware: RequestHandler = (_, res, next) => { export const isApiEnabledMiddleware: RequestHandler = (_, res, next) => {
if (isApiEnabled()) { if (isApiEnabled()) {
next(); next();
@ -208,7 +210,8 @@ export class MeController {
*/ */
@Get('/api-key', { middlewares: [isApiEnabledMiddleware] }) @Get('/api-key', { middlewares: [isApiEnabledMiddleware] })
async getAPIKey(req: AuthenticatedRequest) { 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; 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)
);
}
} }

View file

@ -104,7 +104,7 @@ export class User extends WithTimestamps implements IUser {
@Column({ type: String, nullable: true }) @Column({ type: String, nullable: true })
@Index({ unique: true }) @Index({ unique: true })
apiKey?: string | null; apiKey: string | null;
@Column({ type: Boolean, default: false }) @Column({ type: Boolean, default: false })
mfaEnabled: boolean; mfaEnabled: boolean;

View file

@ -175,14 +175,14 @@ describe('Owner shell', () => {
expect(storedShellOwner.apiKey).toEqual(response.body.data.apiKey); 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'); const response = await authOwnerShellAgent.get('/me/api-key');
expect(response.statusCode).toBe(200); 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'); const response = await authOwnerShellAgent.delete('/me/api-key');
expect(response.statusCode).toBe(200); expect(response.statusCode).toBe(200);
@ -327,14 +327,14 @@ describe('Member', () => {
expect(storedMember.apiKey).toEqual(response.body.data.apiKey); 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'); const response = await testServer.authAgentFor(member).get('/me/api-key');
expect(response.statusCode).toBe(200); 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'); const response = await testServer.authAgentFor(member).delete('/me/api-key');
expect(response.statusCode).toBe(200); expect(response.statusCode).toBe(200);

View file

@ -4,7 +4,7 @@ import jwt from 'jsonwebtoken';
import { mock, anyObject } from 'jest-mock-extended'; import { mock, anyObject } from 'jest-mock-extended';
import type { PublicUser } from '@/Interfaces'; import type { PublicUser } from '@/Interfaces';
import type { User } from '@db/entities/User'; 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 { AUTH_COOKIE_NAME } from '@/constants';
import type { AuthenticatedRequest, MeRequest } from '@/requests'; import type { AuthenticatedRequest, MeRequest } from '@/requests';
import { UserService } from '@/services/user.service'; import { UserService } from '@/services/user.service';
@ -223,7 +223,7 @@ describe('MeController', () => {
describe('API Key methods', () => { describe('API Key methods', () => {
let req: AuthenticatedRequest; let req: AuthenticatedRequest;
beforeAll(() => { beforeAll(() => {
req = mock({ user: mock<Partial<User>>({ id: '123', apiKey: 'test-key' }) }); req = mock({ user: mock<Partial<User>>({ id: '123', apiKey: `${API_KEY_PREFIX}test-key` }) });
}); });
describe('createAPIKey', () => { describe('createAPIKey', () => {
@ -234,9 +234,9 @@ describe('MeController', () => {
}); });
describe('getAPIKey', () => { 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); const { apiKey } = await controller.getAPIKey(req);
expect(apiKey).toEqual(req.user.apiKey); expect(apiKey).not.toEqual(req.user.apiKey);
}); });
}); });

View file

@ -6,13 +6,14 @@
[$style.copyText]: true, [$style.copyText]: true,
[$style[size]]: true, [$style[size]]: true,
[$style.collapsed]: collapse, [$style.collapsed]: collapse,
[$style.noHover]: disableCopy,
'ph-no-capture': redactValue, 'ph-no-capture': redactValue,
}" }"
data-test-id="copy-input" data-test-id="copy-input"
@click="copy" @click="copy"
> >
<span ref="copyInputValue">{{ value }}</span> <span ref="copyInputValue">{{ value }}</span>
<div :class="$style.copyButton"> <div v-if="!disableCopy" :class="$style.copyButton">
<span>{{ copyButtonText }}</span> <span>{{ copyButtonText }}</span>
</div> </div>
</div> </div>
@ -36,6 +37,7 @@ type Props = {
size?: 'medium' | 'large'; size?: 'medium' | 'large';
collapse?: boolean; collapse?: boolean;
redactValue?: boolean; redactValue?: boolean;
disableCopy: boolean;
}; };
const props = withDefaults(defineProps<Props>(), { const props = withDefaults(defineProps<Props>(), {
@ -46,6 +48,7 @@ const props = withDefaults(defineProps<Props>(), {
size: 'medium', size: 'medium',
copyButtonText: useI18n().baseText('generic.copy'), copyButtonText: useI18n().baseText('generic.copy'),
toastTitle: useI18n().baseText('generic.copiedToClipboard'), toastTitle: useI18n().baseText('generic.copiedToClipboard'),
disableCopy: false,
}); });
const emit = defineEmits<{ const emit = defineEmits<{
copy: []; copy: [];
@ -55,6 +58,8 @@ const clipboard = useClipboard();
const { showMessage } = useToast(); const { showMessage } = useToast();
function copy() { function copy() {
if (props.disableCopy) return;
emit('copy'); emit('copy');
void clipboard.copy(props.value ?? ''); void clipboard.copy(props.value ?? '');
@ -88,6 +93,10 @@ function copy() {
} }
} }
.noHover {
cursor: default;
}
.large { .large {
span { span {
font-size: var(--font-size-s); font-size: var(--font-size-s);

View file

@ -1724,6 +1724,7 @@
"settings.api.view.copy.toast": "API Key copied to clipboard", "settings.api.view.copy.toast": "API Key copied to clipboard",
"settings.api.view.apiPlayground": "API Playground", "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.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.api": "n8n API",
"settings.api.view.info.webhook": "webhook node", "settings.api.view.info.webhook": "webhook node",
"settings.api.view.myKey": "My API Key", "settings.api.view.myKey": "My API Key",

View file

@ -43,11 +43,13 @@
:copy-button-text="$locale.baseText('generic.clickToCopy')" :copy-button-text="$locale.baseText('generic.clickToCopy')"
:toast-title="$locale.baseText('settings.api.view.copy.toast')" :toast-title="$locale.baseText('settings.api.view.copy.toast')"
:redact-value="true" :redact-value="true"
:disable-copy="isRedactedApiKey"
:hint="!isRedactedApiKey ? $locale.baseText('settings.api.view.copy') : ''"
@copy="onCopy" @copy="onCopy"
/> />
</div> </div>
</n8n-card> </n8n-card>
<div :class="$style.hint"> <div v-if="!isRedactedApiKey" :class="$style.hint">
<n8n-text size="small"> <n8n-text size="small">
{{ {{
$locale.baseText(`settings.api.view.${swaggerUIEnabled ? 'tryapi' : 'more-details'}`) $locale.baseText(`settings.api.view.${swaggerUIEnabled ? 'tryapi' : 'more-details'}`)
@ -146,6 +148,9 @@ export default defineComponent({
isPublicApiEnabled(): boolean { isPublicApiEnabled(): boolean {
return this.settingsStore.isPublicApiEnabled; return this.settingsStore.isPublicApiEnabled;
}, },
isRedactedApiKey(): boolean {
return this.apiKey.includes('*');
},
}, },
methods: { methods: {
onUpgrade() { onUpgrade() {