From 9a1068c867d7cd128a1ee2f618c39ad44376cb57 Mon Sep 17 00:00:00 2001 From: Jason Rasmussen Date: Tue, 21 Jan 2025 11:45:59 -0500 Subject: [PATCH] refactor: api key repository (#15491) --- server/src/interfaces/api-key.interface.ts | 19 --------- server/src/repositories/api-key.repository.ts | 40 ++++++------------- server/src/repositories/index.ts | 3 +- server/src/services/api-key.service.spec.ts | 10 +---- server/src/services/api-key.service.ts | 7 ++-- server/src/services/auth.service.spec.ts | 4 +- server/src/services/auth.service.ts | 6 ++- server/src/services/base.service.ts | 4 +- server/src/types.ts | 7 ++++ server/test/fixtures/api-key.stub.ts | 7 ++-- .../repositories/api-key.repository.mock.ts | 4 +- server/test/utils.ts | 5 ++- 12 files changed, 44 insertions(+), 72 deletions(-) delete mode 100644 server/src/interfaces/api-key.interface.ts diff --git a/server/src/interfaces/api-key.interface.ts b/server/src/interfaces/api-key.interface.ts deleted file mode 100644 index 473a2b8019a55..0000000000000 --- a/server/src/interfaces/api-key.interface.ts +++ /dev/null @@ -1,19 +0,0 @@ -import { Insertable } from 'kysely'; -import { ApiKeys } from 'src/db'; -import { APIKeyEntity } from 'src/entities/api-key.entity'; -import { AuthApiKey } from 'src/types'; - -export const IKeyRepository = 'IKeyRepository'; - -export interface IKeyRepository { - create(dto: Insertable): Promise; - update(userId: string, id: string, dto: Partial): Promise; - delete(userId: string, id: string): Promise; - /** - * Includes the hashed `key` for verification - * @param id - */ - getKey(hashedToken: string): Promise; - getById(userId: string, id: string): Promise; - getByUserId(userId: string): Promise; -} diff --git a/server/src/repositories/api-key.repository.ts b/server/src/repositories/api-key.repository.ts index c0fc7532137c2..5422ad569e674 100644 --- a/server/src/repositories/api-key.repository.ts +++ b/server/src/repositories/api-key.repository.ts @@ -1,50 +1,36 @@ import { Injectable } from '@nestjs/common'; -import { InjectRepository } from '@nestjs/typeorm'; import { Insertable, Kysely, Updateable } from 'kysely'; import { InjectKysely } from 'nestjs-kysely'; import { ApiKeys, DB } from 'src/db'; import { DummyValue, GenerateSql } from 'src/decorators'; -import { APIKeyEntity } from 'src/entities/api-key.entity'; -import { IKeyRepository } from 'src/interfaces/api-key.interface'; -import { AuthApiKey } from 'src/types'; import { asUuid } from 'src/utils/database'; -import { Repository } from 'typeorm'; const columns = ['id', 'name', 'userId', 'createdAt', 'updatedAt', 'permissions'] as const; @Injectable() -export class ApiKeyRepository implements IKeyRepository { - constructor( - @InjectRepository(APIKeyEntity) private repository: Repository, - @InjectKysely() private db: Kysely, - ) {} +export class ApiKeyRepository { + constructor(@InjectKysely() private db: Kysely) {} - async create(dto: Insertable): Promise { - const { id, name, createdAt, updatedAt, permissions } = await this.db - .insertInto('api_keys') - .values(dto) - .returningAll() - .executeTakeFirstOrThrow(); - - return { id, name, createdAt, updatedAt, permissions } as APIKeyEntity; + create(dto: Insertable) { + return this.db.insertInto('api_keys').values(dto).returningAll().executeTakeFirstOrThrow(); } - async update(userId: string, id: string, dto: Updateable): Promise { + async update(userId: string, id: string, dto: Updateable) { return this.db .updateTable('api_keys') .set(dto) .where('api_keys.userId', '=', userId) .where('id', '=', asUuid(id)) .returningAll() - .executeTakeFirstOrThrow() as unknown as Promise; + .executeTakeFirstOrThrow(); } - async delete(userId: string, id: string): Promise { + async delete(userId: string, id: string) { await this.db.deleteFrom('api_keys').where('userId', '=', userId).where('id', '=', asUuid(id)).execute(); } @GenerateSql({ params: [DummyValue.STRING] }) - getKey(hashedToken: string): Promise { + getKey(hashedToken: string) { return this.db .selectFrom('api_keys') .innerJoinLateral( @@ -72,26 +58,26 @@ export class ApiKeyRepository implements IKeyRepository { eb.fn.toJson('user').as('user'), ]) .where('api_keys.key', '=', hashedToken) - .executeTakeFirst() as Promise; + .executeTakeFirst(); } @GenerateSql({ params: [DummyValue.UUID, DummyValue.UUID] }) - getById(userId: string, id: string): Promise { + getById(userId: string, id: string) { return this.db .selectFrom('api_keys') .select(columns) .where('id', '=', asUuid(id)) .where('userId', '=', userId) - .executeTakeFirst() as unknown as Promise; + .executeTakeFirst(); } @GenerateSql({ params: [DummyValue.UUID] }) - getByUserId(userId: string): Promise { + getByUserId(userId: string) { return this.db .selectFrom('api_keys') .select(columns) .where('userId', '=', userId) .orderBy('createdAt', 'desc') - .execute() as unknown as Promise; + .execute(); } } diff --git a/server/src/repositories/index.ts b/server/src/repositories/index.ts index 8f691ac9e706d..434efa935f865 100644 --- a/server/src/repositories/index.ts +++ b/server/src/repositories/index.ts @@ -1,6 +1,5 @@ import { IAlbumUserRepository } from 'src/interfaces/album-user.interface'; import { IAlbumRepository } from 'src/interfaces/album.interface'; -import { IKeyRepository } from 'src/interfaces/api-key.interface'; import { IAssetRepository } from 'src/interfaces/asset.interface'; import { IAuditRepository } from 'src/interfaces/audit.interface'; import { IConfigRepository } from 'src/interfaces/config.interface'; @@ -79,6 +78,7 @@ export const repositories = [ // AccessRepository, ActivityRepository, + ApiKeyRepository, ]; export const providers = [ @@ -92,7 +92,6 @@ export const providers = [ { provide: IDatabaseRepository, useClass: DatabaseRepository }, { provide: IEventRepository, useClass: EventRepository }, { provide: IJobRepository, useClass: JobRepository }, - { provide: IKeyRepository, useClass: ApiKeyRepository }, { provide: ILibraryRepository, useClass: LibraryRepository }, { provide: ILoggerRepository, useClass: LoggerRepository }, { provide: IMachineLearningRepository, useClass: MachineLearningRepository }, diff --git a/server/src/services/api-key.service.spec.ts b/server/src/services/api-key.service.spec.ts index 8d07985440ae0..928978b698f93 100644 --- a/server/src/services/api-key.service.spec.ts +++ b/server/src/services/api-key.service.spec.ts @@ -1,8 +1,8 @@ import { BadRequestException } from '@nestjs/common'; import { Permission } from 'src/enum'; -import { IKeyRepository } from 'src/interfaces/api-key.interface'; import { ICryptoRepository } from 'src/interfaces/crypto.interface'; import { APIKeyService } from 'src/services/api-key.service'; +import { IApiKeyRepository } from 'src/types'; import { keyStub } from 'test/fixtures/api-key.stub'; import { authStub } from 'test/fixtures/auth.stub'; import { newTestService } from 'test/utils'; @@ -12,7 +12,7 @@ describe(APIKeyService.name, () => { let sut: APIKeyService; let cryptoMock: Mocked; - let keyMock: Mocked; + let keyMock: Mocked; beforeEach(() => { ({ sut, cryptoMock, keyMock } = newTestService(APIKeyService)); @@ -56,8 +56,6 @@ describe(APIKeyService.name, () => { describe('update', () => { it('should throw an error if the key is not found', async () => { - keyMock.getById.mockResolvedValue(null); - await expect(sut.update(authStub.admin, 'random-guid', { name: 'New Name' })).rejects.toBeInstanceOf( BadRequestException, ); @@ -77,8 +75,6 @@ describe(APIKeyService.name, () => { describe('delete', () => { it('should throw an error if the key is not found', async () => { - keyMock.getById.mockResolvedValue(null); - await expect(sut.delete(authStub.admin, 'random-guid')).rejects.toBeInstanceOf(BadRequestException); expect(keyMock.delete).not.toHaveBeenCalledWith('random-guid'); @@ -95,8 +91,6 @@ describe(APIKeyService.name, () => { describe('getById', () => { it('should throw an error if the key is not found', async () => { - keyMock.getById.mockResolvedValue(null); - await expect(sut.getById(authStub.admin, 'random-guid')).rejects.toBeInstanceOf(BadRequestException); expect(keyMock.getById).toHaveBeenCalledWith(authStub.admin.user.id, 'random-guid'); diff --git a/server/src/services/api-key.service.ts b/server/src/services/api-key.service.ts index 303ca05537781..7d9a4f37763d2 100644 --- a/server/src/services/api-key.service.ts +++ b/server/src/services/api-key.service.ts @@ -1,8 +1,9 @@ import { BadRequestException, Injectable } from '@nestjs/common'; import { APIKeyCreateDto, APIKeyCreateResponseDto, APIKeyResponseDto, APIKeyUpdateDto } from 'src/dtos/api-key.dto'; import { AuthDto } from 'src/dtos/auth.dto'; -import { APIKeyEntity } from 'src/entities/api-key.entity'; +import { Permission } from 'src/enum'; import { BaseService } from 'src/services/base.service'; +import { ApiKeyItem } from 'src/types'; import { isGranted } from 'src/utils/access'; @Injectable() @@ -57,13 +58,13 @@ export class APIKeyService extends BaseService { return keys.map((key) => this.map(key)); } - private map(entity: APIKeyEntity): APIKeyResponseDto { + private map(entity: ApiKeyItem): APIKeyResponseDto { return { id: entity.id, name: entity.name, createdAt: entity.createdAt, updatedAt: entity.updatedAt, - permissions: entity.permissions, + permissions: entity.permissions as Permission[], }; } } diff --git a/server/src/services/auth.service.spec.ts b/server/src/services/auth.service.spec.ts index 917f3681bdac3..ffa280677aac6 100644 --- a/server/src/services/auth.service.spec.ts +++ b/server/src/services/auth.service.spec.ts @@ -3,7 +3,6 @@ import { AuthDto, SignUpDto } from 'src/dtos/auth.dto'; import { UserMetadataEntity } from 'src/entities/user-metadata.entity'; import { UserEntity } from 'src/entities/user.entity'; import { AuthType, Permission } from 'src/enum'; -import { IKeyRepository } from 'src/interfaces/api-key.interface'; import { ICryptoRepository } from 'src/interfaces/crypto.interface'; import { IEventRepository } from 'src/interfaces/event.interface'; import { IOAuthRepository } from 'src/interfaces/oauth.interface'; @@ -12,6 +11,7 @@ import { ISharedLinkRepository } from 'src/interfaces/shared-link.interface'; import { ISystemMetadataRepository } from 'src/interfaces/system-metadata.interface'; import { IUserRepository } from 'src/interfaces/user.interface'; import { AuthService } from 'src/services/auth.service'; +import { IApiKeyRepository } from 'src/types'; import { keyStub } from 'test/fixtures/api-key.stub'; import { authStub } from 'test/fixtures/auth.stub'; import { sessionStub } from 'test/fixtures/session.stub'; @@ -62,7 +62,7 @@ describe('AuthService', () => { let cryptoMock: Mocked; let eventMock: Mocked; - let keyMock: Mocked; + let keyMock: Mocked; let oauthMock: Mocked; let sessionMock: Mocked; let sharedLinkMock: Mocked; diff --git a/server/src/services/auth.service.ts b/server/src/services/auth.service.ts index 9999c16f64ba2..4c0cdbab916ba 100644 --- a/server/src/services/auth.service.ts +++ b/server/src/services/auth.service.ts @@ -21,6 +21,7 @@ import { UserEntity } from 'src/entities/user.entity'; import { AuthType, ImmichCookie, ImmichHeader, ImmichQuery, Permission } from 'src/enum'; import { OAuthProfile } from 'src/interfaces/oauth.interface'; import { BaseService } from 'src/services/base.service'; +import { AuthApiKey } from 'src/types'; import { isGranted } from 'src/utils/access'; import { HumanReadableSize } from 'src/utils/bytes'; @@ -309,7 +310,10 @@ export class AuthService extends BaseService { const hashedKey = this.cryptoRepository.hashSha256(key); const apiKey = await this.keyRepository.getKey(hashedKey); if (apiKey) { - return { user: apiKey.user, apiKey }; + return { + user: apiKey.user as unknown as UserEntity, + apiKey: apiKey as unknown as AuthApiKey, + }; } throw new UnauthorizedException('Invalid API key'); diff --git a/server/src/services/base.service.ts b/server/src/services/base.service.ts index fa77bcc38884b..adcddd8d666c5 100644 --- a/server/src/services/base.service.ts +++ b/server/src/services/base.service.ts @@ -8,7 +8,6 @@ import { Users } from 'src/db'; import { UserEntity } from 'src/entities/user.entity'; import { IAlbumUserRepository } from 'src/interfaces/album-user.interface'; import { IAlbumRepository } from 'src/interfaces/album.interface'; -import { IKeyRepository } from 'src/interfaces/api-key.interface'; import { IAssetRepository } from 'src/interfaces/asset.interface'; import { IAuditRepository } from 'src/interfaces/audit.interface'; import { IConfigRepository } from 'src/interfaces/config.interface'; @@ -45,6 +44,7 @@ import { IVersionHistoryRepository } from 'src/interfaces/version-history.interf import { IViewRepository } from 'src/interfaces/view.interface'; import { AccessRepository } from 'src/repositories/access.repository'; import { ActivityRepository } from 'src/repositories/activity.repository'; +import { ApiKeyRepository } from 'src/repositories/api-key.repository'; import { AccessRequest, checkAccess, requireAccess } from 'src/utils/access'; import { getConfig, updateConfig } from 'src/utils/config'; @@ -65,7 +65,7 @@ export class BaseService { @Inject(IDatabaseRepository) protected databaseRepository: IDatabaseRepository, @Inject(IEventRepository) protected eventRepository: IEventRepository, @Inject(IJobRepository) protected jobRepository: IJobRepository, - @Inject(IKeyRepository) protected keyRepository: IKeyRepository, + protected keyRepository: ApiKeyRepository, @Inject(ILibraryRepository) protected libraryRepository: ILibraryRepository, @Inject(IMachineLearningRepository) protected machineLearningRepository: IMachineLearningRepository, @Inject(IMapRepository) protected mapRepository: IMapRepository, diff --git a/server/src/types.ts b/server/src/types.ts index a6a070dc637f7..dd1fea710f882 100644 --- a/server/src/types.ts +++ b/server/src/types.ts @@ -2,6 +2,7 @@ import { UserEntity } from 'src/entities/user.entity'; import { Permission } from 'src/enum'; import { AccessRepository } from 'src/repositories/access.repository'; import { ActivityRepository } from 'src/repositories/activity.repository'; +import { ApiKeyRepository } from 'src/repositories/api-key.repository'; export type AuthApiKey = { id: string; @@ -14,7 +15,13 @@ export type RepositoryInterface = Pick; export type IActivityRepository = RepositoryInterface; export type IAccessRepository = { [K in keyof AccessRepository]: RepositoryInterface }; +export type IApiKeyRepository = RepositoryInterface; export type ActivityItem = | Awaited> | Awaited>[0]; + +export type ApiKeyItem = + | Awaited> + | NonNullable>> + | Awaited>[0]; diff --git a/server/test/fixtures/api-key.stub.ts b/server/test/fixtures/api-key.stub.ts index 248d30c2eccda..905bda34b410d 100644 --- a/server/test/fixtures/api-key.stub.ts +++ b/server/test/fixtures/api-key.stub.ts @@ -1,5 +1,3 @@ -import { APIKeyEntity } from 'src/entities/api-key.entity'; -import { AuthApiKey } from 'src/types'; import { authStub } from 'test/fixtures/auth.stub'; import { userStub } from 'test/fixtures/user.stub'; @@ -9,7 +7,7 @@ export const keyStub = { key: 'my-api-key (hashed)', user: userStub.admin, permissions: [], - } as AuthApiKey), + } as any), admin: Object.freeze({ id: 'my-random-guid', @@ -17,5 +15,6 @@ export const keyStub = { key: 'my-api-key (hashed)', userId: authStub.admin.user.id, user: userStub.admin, - } as APIKeyEntity), + permissions: [], + } as any), }; diff --git a/server/test/repositories/api-key.repository.mock.ts b/server/test/repositories/api-key.repository.mock.ts index a7cfb6369a8d9..8c471e520f800 100644 --- a/server/test/repositories/api-key.repository.mock.ts +++ b/server/test/repositories/api-key.repository.mock.ts @@ -1,7 +1,7 @@ -import { IKeyRepository } from 'src/interfaces/api-key.interface'; +import { IApiKeyRepository } from 'src/types'; import { Mocked, vitest } from 'vitest'; -export const newKeyRepositoryMock = (): Mocked => { +export const newKeyRepositoryMock = (): Mocked => { return { create: vitest.fn(), update: vitest.fn(), diff --git a/server/test/utils.ts b/server/test/utils.ts index d6d1cd71beb99..929fcb9da0234 100644 --- a/server/test/utils.ts +++ b/server/test/utils.ts @@ -5,8 +5,9 @@ import { ImmichWorker } from 'src/enum'; import { IMetadataRepository } from 'src/interfaces/metadata.interface'; import { AccessRepository } from 'src/repositories/access.repository'; import { ActivityRepository } from 'src/repositories/activity.repository'; +import { ApiKeyRepository } from 'src/repositories/api-key.repository'; import { BaseService } from 'src/services/base.service'; -import { IAccessRepository, IActivityRepository } from 'src/types'; +import { IAccessRepository, IActivityRepository, IApiKeyRepository } from 'src/types'; import { newAccessRepositoryMock } from 'test/repositories/access.repository.mock'; import { newActivityRepositoryMock } from 'test/repositories/activity.repository.mock'; import { newAlbumUserRepositoryMock } from 'test/repositories/album-user.repository.mock'; @@ -118,7 +119,7 @@ export const newTestService = ( databaseMock, eventMock, jobMock, - keyMock, + keyMock as IApiKeyRepository as ApiKeyRepository, libraryMock, machineLearningMock, mapMock,