From 93d2d30fe026457c0be089e89883a4f6178bd509 Mon Sep 17 00:00:00 2001 From: Brandon Wees Date: Tue, 18 Aug 2026 16:07:39 -0400 Subject: [PATCH] feat: rotate an API key (#30801) * feat: rotate an API key * fix: test response * chore: create new api key scope * chore: code review --- e2e/src/specs/server/api/api-key.e2e-spec.ts | 10 +++ i18n/en.json | 2 + open-api/immich-openapi-specs.json | 53 +++++++++++++++ packages/sdk/src/fetch-client.ts | 15 ++++ .../controllers/api-key.controller.spec.ts | 8 +++ server/src/controllers/api-key.controller.ts | 12 ++++ server/src/enum.ts | 1 + server/src/services/api-key.service.spec.ts | 45 ++++++++++++ server/src/services/api-key.service.ts | 19 ++++++ server/test/medium.factory.ts | 2 + .../specs/services/api-key.service.spec.ts | 68 +++++++++++++++++++ web/src/lib/services/api-key.service.ts | 29 +++++++- .../user-settings/UserApiKeyList.svelte | 3 +- 13 files changed, 264 insertions(+), 3 deletions(-) create mode 100644 server/test/medium/specs/services/api-key.service.spec.ts diff --git a/e2e/src/specs/server/api/api-key.e2e-spec.ts b/e2e/src/specs/server/api/api-key.e2e-spec.ts index 28d134a664..2339889afe 100644 --- a/e2e/src/specs/server/api/api-key.e2e-spec.ts +++ b/e2e/src/specs/server/api/api-key.e2e-spec.ts @@ -169,6 +169,16 @@ describe('/api-keys', () => { }); }); + describe('POST /api-keys/:id/rotate', () => { + it('should not work without permission', async () => { + const { apiKey } = await create(user.accessToken, [Permission.ApiKeyUpdate]); + const { secret } = await create(user.accessToken, [Permission.ApiKeyUpdate]); + const { status, body } = await request(app).post(`/api-keys/${apiKey.id}/rotate`).set('x-api-key', secret); + expect(status).toBe(403); + expect(body).toEqual(errorDto.missingPermission('apiKey.rotate')); + }); + }); + describe('DELETE /api-keys/:id', () => { it('should require authorization', async () => { const { apiKey } = await create(user.accessToken, [Permission.All]); diff --git a/i18n/en.json b/i18n/en.json index 0acfe68fa3..1afd6fb6a2 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -1796,6 +1796,8 @@ "role": "Role", "role_editor": "Editor", "role_viewer": "Viewer", + "rotate_api_key_prompt": "Are you sure you want to rotate this API key? The current key will stop working immediately.", + "rotate_key": "Rotate key", "running": "Running", "save": "Save", "saved": "Saved", diff --git a/open-api/immich-openapi-specs.json b/open-api/immich-openapi-specs.json index d00faab9a0..02d7fc80ef 100644 --- a/open-api/immich-openapi-specs.json +++ b/open-api/immich-openapi-specs.json @@ -3264,6 +3264,58 @@ "x-immich-state": "Deprecated" } }, + "/api-keys/{id}/rotate": { + "post": { + "description": "Generates a new secret for an API key, immediately invalidating the previous one. The current user must own this API key.", + "operationId": "rotateApiKey", + "parameters": [ + { + "name": "id", + "required": true, + "in": "path", + "schema": { + "format": "uuid", + "pattern": "^([0-9a-fA-F]{8}-[0-9a-fA-F]{4}-4[0-9a-fA-F]{3}-[89abAB][0-9a-fA-F]{3}-[0-9a-fA-F]{12})$", + "type": "string" + } + } + ], + "responses": { + "201": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ApiKeyCreateResponseDto" + } + } + }, + "description": "" + } + }, + "security": [ + { + "bearer": [] + }, + { + "cookie": [] + }, + { + "api_key": [] + } + ], + "summary": "Rotate an API key", + "tags": [ + "API keys" + ], + "x-immich-history": [ + { + "version": "v3", + "state": "Added" + } + ], + "x-immich-permission": "apiKey.rotate" + } + }, "/assets": { "delete": { "description": "Deletes multiple assets at the same time.", @@ -21016,6 +21068,7 @@ "apiKey.read", "apiKey.update", "apiKey.delete", + "apiKey.rotate", "asset.read", "asset.update", "asset.delete", diff --git a/packages/sdk/src/fetch-client.ts b/packages/sdk/src/fetch-client.ts index b2f98b58ef..9a1f93cb30 100644 --- a/packages/sdk/src/fetch-client.ts +++ b/packages/sdk/src/fetch-client.ts @@ -4112,6 +4112,20 @@ export function updateApiKey({ id, apiKeyUpdateDto }: { body: apiKeyUpdateDto }))); } +/** + * Rotate an API key + */ +export function rotateApiKey({ id }: { + id: string; +}, opts?: Oazapfts.RequestOpts) { + return oazapfts.ok(oazapfts.fetchJson<{ + status: 201; + data: ApiKeyCreateResponseDto; + }>(`/api-keys/${encodeURIComponent(id)}/rotate`, { + ...opts, + method: "POST" + })); +} /** * Delete assets */ @@ -7215,6 +7229,7 @@ export enum Permission { ApiKeyRead = "apiKey.read", ApiKeyUpdate = "apiKey.update", ApiKeyDelete = "apiKey.delete", + ApiKeyRotate = "apiKey.rotate", AssetRead = "asset.read", AssetUpdate = "asset.update", AssetDelete = "asset.delete", diff --git a/server/src/controllers/api-key.controller.spec.ts b/server/src/controllers/api-key.controller.spec.ts index 08963362b6..815e00a9a5 100644 --- a/server/src/controllers/api-key.controller.spec.ts +++ b/server/src/controllers/api-key.controller.spec.ts @@ -44,6 +44,14 @@ describe(ApiKeyController.name, () => { }); }); + describe('POST /api-keys/:id/rotate', () => { + it('should require a valid uuid', async () => { + const { status, body } = await request(ctx.getHttpServer()).post(`/api-keys/123/rotate`); + expect(status).toBe(400); + expect(body).toEqual(factory.responses.validationError([{ path: ['id'], message: 'Invalid UUID' }])); + }); + }); + describe('DELETE /api-keys/:id', () => { it('should require a valid uuid', async () => { const { status, body } = await request(ctx.getHttpServer()).delete(`/api-keys/123`); diff --git a/server/src/controllers/api-key.controller.ts b/server/src/controllers/api-key.controller.ts index b7e3627faa..a179e253de 100644 --- a/server/src/controllers/api-key.controller.ts +++ b/server/src/controllers/api-key.controller.ts @@ -87,6 +87,18 @@ export class ApiKeyController { return this.service.update(auth, id, dto); } + @Post(':id/rotate') + @Authenticated({ permission: Permission.ApiKeyRotate }) + @Endpoint({ + summary: 'Rotate an API key', + description: + 'Generates a new secret for an API key, immediately invalidating the previous one. The current user must own this API key.', + history: new HistoryBuilder().added('v3'), + }) + rotateApiKey(@Auth() auth: AuthDto, @Param() { id }: UUIDParamDto): Promise { + return this.service.rotate(auth, id); + } + @Delete(':id') @Authenticated({ permission: Permission.ApiKeyDelete }) @HttpCode(HttpStatus.NO_CONTENT) diff --git a/server/src/enum.ts b/server/src/enum.ts index e88a9a667c..b6e2f6e468 100644 --- a/server/src/enum.ts +++ b/server/src/enum.ts @@ -117,6 +117,7 @@ export enum Permission { ApiKeyRead = 'apiKey.read', ApiKeyUpdate = 'apiKey.update', ApiKeyDelete = 'apiKey.delete', + ApiKeyRotate = 'apiKey.rotate', // ASSET_CREATE = 'asset.create', AssetRead = 'asset.read', diff --git a/server/src/services/api-key.service.spec.ts b/server/src/services/api-key.service.spec.ts index 68165d642f..64ae09a0aa 100644 --- a/server/src/services/api-key.service.spec.ts +++ b/server/src/services/api-key.service.spec.ts @@ -188,6 +188,51 @@ describe(ApiKeyService.name, () => { }); }); + describe('rotate', () => { + it('should throw an error if the key is not found', async () => { + const auth = AuthFactory.create(); + const id = newUuid(); + + mocks.apiKey.getById.mockResolvedValue(void 0); + + await expect(sut.rotate(auth, id)).rejects.toBeInstanceOf(BadRequestException); + + expect(mocks.apiKey.update).not.toHaveBeenCalled(); + }); + + it('should replace the secret of a key', async () => { + const auth = AuthFactory.create(); + const apiKey = ApiKeyFactory.create({ userId: auth.user.id }); + + mocks.crypto.randomBytesAsText.mockReturnValue('super-secret'); + mocks.apiKey.getById.mockResolvedValue(apiKey); + mocks.apiKey.update.mockResolvedValue(apiKey); + + await expect(sut.rotate(auth, apiKey.id)).resolves.toEqual( + expect.objectContaining({ secret: 'super-secret', apiKey: expect.objectContaining({ id: apiKey.id }) }), + ); + + expect(mocks.apiKey.update).toHaveBeenCalledWith(auth.user.id, apiKey.id, { + key: Buffer.from('super-secret (hashed)'), + }); + }); + + it('should not rotate a key with permissions the caller does not have', async () => { + const auth = AuthFactory.from() + .apiKey({ permissions: [Permission.ApiKeyRotate] }) + .build(); + const apiKey = ApiKeyFactory.create({ userId: auth.user.id, permissions: [Permission.All] }); + + mocks.apiKey.getById.mockResolvedValue(apiKey); + + await expect(sut.rotate(auth, apiKey.id)).rejects.toThrow( + 'Cannot rotate an API Key with permissions you do not have', + ); + + expect(mocks.apiKey.update).not.toHaveBeenCalled(); + }); + }); + describe('delete', () => { it('should throw an error if the key is not found', async () => { const auth = AuthFactory.create(); diff --git a/server/src/services/api-key.service.ts b/server/src/services/api-key.service.ts index acd3b17e94..2b9e1a814c 100644 --- a/server/src/services/api-key.service.ts +++ b/server/src/services/api-key.service.ts @@ -5,6 +5,7 @@ import { AuthDto } from 'src/dtos/auth.dto'; import { Permission } from 'src/enum'; import { BaseService } from 'src/services/base.service'; import { isGranted } from 'src/utils/access'; +import { findOrFail } from 'src/utils/misc'; @Injectable() export class ApiKeyService extends BaseService { @@ -45,6 +46,24 @@ export class ApiKeyService extends BaseService { return this.map(key); } + async rotate(auth: AuthDto, id: string): Promise { + const existing = await findOrFail(() => this.apiKeyRepository.getById(auth.user.id, id), 'API Key not found'); + + if ( + auth.apiKey && + !isGranted({ requested: existing.permissions as Permission[], current: auth.apiKey.permissions }) + ) { + throw new BadRequestException('Cannot rotate an API Key with permissions you do not have'); + } + + const token = this.cryptoRepository.randomBytesAsText(32); + const hashed = this.cryptoRepository.hashSha256(token); + + const newKey = await this.apiKeyRepository.update(auth.user.id, id, { key: hashed }); + + return { secret: token, apiKey: this.map(newKey) }; + } + async delete(auth: AuthDto, id: string): Promise { const exists = await this.apiKeyRepository.getById(auth.user.id, id); if (!exists) { diff --git a/server/test/medium.factory.ts b/server/test/medium.factory.ts index 860189d9a9..dc3bc34e3c 100644 --- a/server/test/medium.factory.ts +++ b/server/test/medium.factory.ts @@ -22,6 +22,7 @@ import { AccessRepository } from 'src/repositories/access.repository'; import { ActivityRepository } from 'src/repositories/activity.repository'; import { AlbumUserRepository } from 'src/repositories/album-user.repository'; import { AlbumRepository } from 'src/repositories/album.repository'; +import { ApiKeyRepository } from 'src/repositories/api-key.repository'; import { AssetEditRepository } from 'src/repositories/asset-edit.repository'; import { AssetJobRepository } from 'src/repositories/asset-job.repository'; import { AssetRepository } from 'src/repositories/asset.repository'; @@ -443,6 +444,7 @@ const newRealRepository = (key: T, db: Kysely case AlbumRepository: case AlbumUserRepository: case ActivityRepository: + case ApiKeyRepository: case AssetRepository: case AssetEditRepository: case AssetJobRepository: diff --git a/server/test/medium/specs/services/api-key.service.spec.ts b/server/test/medium/specs/services/api-key.service.spec.ts new file mode 100644 index 0000000000..1a5842dab4 --- /dev/null +++ b/server/test/medium/specs/services/api-key.service.spec.ts @@ -0,0 +1,68 @@ +import { Kysely } from 'kysely'; +import { Permission } from 'src/enum'; +import { ApiKeyRepository } from 'src/repositories/api-key.repository'; +import { CryptoRepository } from 'src/repositories/crypto.repository'; +import { LoggingRepository } from 'src/repositories/logging.repository'; +import { DB } from 'src/schema'; +import { ApiKeyService } from 'src/services/api-key.service'; +import { newMediumService } from 'test/medium.factory'; +import { factory } from 'test/small.factory'; +import { getKyselyDB } from 'test/utils'; + +let defaultDatabase: Kysely; + +const setup = (db?: Kysely) => { + return newMediumService(ApiKeyService, { + database: db || defaultDatabase, + real: [ApiKeyRepository, CryptoRepository], + mock: [LoggingRepository], + }); +}; + +beforeAll(async () => { + defaultDatabase = await getKyselyDB(); +}); + +describe(ApiKeyService.name, () => { + describe('rotate', () => { + it('should not rotate an api key of another user', async () => { + const { sut, ctx } = setup(); + const { user } = await ctx.newUser(); + const { user: otherUser } = await ctx.newUser(); + const { apiKey } = await sut.create(factory.auth({ user }), { permissions: [Permission.All] }); + + await expect(sut.rotate(factory.auth({ user: otherUser }), apiKey.id)).rejects.toThrow('API Key not found'); + }); + + it('should not rotate a key with permissions the caller does not have', async () => { + const { sut, ctx } = setup(); + const { user } = await ctx.newUser(); + const { apiKey } = await sut.create(factory.auth({ user }), { permissions: [Permission.All] }); + const auth = factory.auth({ user, apiKey: { permissions: [Permission.ApiKeyRotate] } }); + + await expect(sut.rotate(auth, apiKey.id)).rejects.toThrow( + 'Cannot rotate an API Key with permissions you do not have', + ); + }); + + it('should replace the secret of an api key', async () => { + const { sut, ctx } = setup(); + const apiKeyRepo = ctx.get(ApiKeyRepository); + const crypto = ctx.get(CryptoRepository); + const { user } = await ctx.newUser(); + const auth = factory.auth({ user }); + const { apiKey, secret } = await sut.create(auth, { permissions: [Permission.All] }); + + const rotated = await sut.rotate(auth, apiKey.id); + + expect(rotated.secret).not.toEqual(secret); + expect(rotated.apiKey).toEqual( + expect.objectContaining({ id: apiKey.id, name: apiKey.name, permissions: [Permission.All] }), + ); + await expect(apiKeyRepo.getKey(crypto.hashSha256(secret))).resolves.toBeUndefined(); + await expect(apiKeyRepo.getKey(crypto.hashSha256(rotated.secret))).resolves.toEqual( + expect.objectContaining({ id: apiKey.id }), + ); + }); + }); +}); diff --git a/web/src/lib/services/api-key.service.ts b/web/src/lib/services/api-key.service.ts index 1908bf3b65..e5346627ff 100644 --- a/web/src/lib/services/api-key.service.ts +++ b/web/src/lib/services/api-key.service.ts @@ -1,16 +1,18 @@ import { createApiKey, deleteApiKey, + rotateApiKey, updateApiKey, type ApiKeyCreateDto, type ApiKeyResponseDto, type ApiKeyUpdateDto, } from '@immich/sdk'; import { modalManager, toastManager, type ActionItem } from '@immich/ui'; -import { mdiPencilOutline, mdiPlus, mdiTrashCanOutline } from '@mdi/js'; +import { mdiAutorenew, mdiPencilOutline, mdiPlus, mdiTrashCanOutline } from '@mdi/js'; import type { MessageFormatter } from 'svelte-i18n'; import { eventManager } from '$lib/managers/event-manager.svelte'; import ApiKeyCreateModal from '$lib/modals/ApiKeyCreateModal.svelte'; +import ApiKeySecretModal from '$lib/modals/ApiKeySecretModal.svelte'; import ApiKeyUpdateModal from '$lib/modals/ApiKeyUpdateModal.svelte'; import { handleError } from '$lib/utils/handle-error'; import { getFormatter } from '$lib/utils/i18n'; @@ -32,13 +34,19 @@ export const getApiKeyActions = ($t: MessageFormatter, apiKey: ApiKeyResponseDto onAction: () => modalManager.show(ApiKeyUpdateModal, { apiKey }), }; + const Rotate: ActionItem = { + title: $t('rotate_key'), + icon: mdiAutorenew, + onAction: () => handleRotateApiKey(apiKey), + }; + const Delete: ActionItem = { title: $t('delete_key'), icon: mdiTrashCanOutline, onAction: () => handleDeleteApiKey(apiKey), }; - return { Update, Delete }; + return { Update, Rotate, Delete }; }; export const handleCreateApiKey = async (dto: ApiKeyCreateDto) => { @@ -87,6 +95,23 @@ export const handleUpdateApiKey = async (apiKey: { id: string }, dto: ApiKeyUpda } }; +export const handleRotateApiKey = async (apiKey: ApiKeyResponseDto) => { + const $t = await getFormatter(); + + const confirmed = await modalManager.showDialog({ prompt: $t('rotate_api_key_prompt') }); + if (!confirmed) { + return; + } + + try { + const response = await rotateApiKey({ id: apiKey.id }); + eventManager.emit('ApiKeyUpdate', response.apiKey); + await modalManager.show(ApiKeySecretModal, { secret: response.secret }); + } catch (error) { + handleError(error, $t('errors.something_went_wrong')); + } +}; + export const handleDeleteApiKey = async (apiKey: ApiKeyResponseDto) => { const $t = await getFormatter(); diff --git a/web/src/routes/(user)/user-settings/UserApiKeyList.svelte b/web/src/routes/(user)/user-settings/UserApiKeyList.svelte index 441b01f9dd..d9f89f1765 100644 --- a/web/src/routes/(user)/user-settings/UserApiKeyList.svelte +++ b/web/src/routes/(user)/user-settings/UserApiKeyList.svelte @@ -51,7 +51,7 @@ {#each keys as key (key.id)} - {@const { Update, Delete } = getApiKeyActions($t, key)} + {@const { Update, Rotate, Delete } = getApiKeyActions($t, key)} {key.name} @@ -64,6 +64,7 @@ {new Date(key.createdAt).toLocaleDateString($locale, dateFormats.settings)} +