From a21b008773bd839ab2fa8628dc6f16a160961cfd Mon Sep 17 00:00:00 2001 From: Jason Rasmussen Date: Fri, 10 Jan 2025 14:02:12 -0500 Subject: [PATCH] feat: migration api keys to use kysely (#15206) --- server/src/dtos/auth.dto.ts | 4 +- server/src/interfaces/api-key.interface.ts | 7 +- server/src/queries/api.key.repository.sql | 122 ++++++++---------- server/src/repositories/api-key.repository.ts | 93 +++++++++---- server/src/services/api-key.service.spec.ts | 5 +- server/src/services/auth.service.spec.ts | 8 +- server/src/services/auth.service.ts | 2 +- server/src/types.ts | 9 ++ server/test/fixtures/api-key.stub.ts | 8 ++ 9 files changed, 151 insertions(+), 107 deletions(-) create mode 100644 server/src/types.ts diff --git a/server/src/dtos/auth.dto.ts b/server/src/dtos/auth.dto.ts index b2bf1b8bccc86d..d6b73f584a8de9 100644 --- a/server/src/dtos/auth.dto.ts +++ b/server/src/dtos/auth.dto.ts @@ -1,11 +1,11 @@ import { ApiProperty } from '@nestjs/swagger'; import { Transform } from 'class-transformer'; import { IsEmail, IsNotEmpty, IsString, MinLength } from 'class-validator'; -import { APIKeyEntity } from 'src/entities/api-key.entity'; import { SessionEntity } from 'src/entities/session.entity'; import { SharedLinkEntity } from 'src/entities/shared-link.entity'; import { UserEntity } from 'src/entities/user.entity'; import { ImmichCookie } from 'src/enum'; +import { AuthApiKey } from 'src/types'; import { toEmail } from 'src/validation'; export type CookieResponse = { @@ -16,7 +16,7 @@ export type CookieResponse = { export class AuthDto { user!: UserEntity; - apiKey?: APIKeyEntity; + apiKey?: AuthApiKey; sharedLink?: SharedLinkEntity; session?: SessionEntity; } diff --git a/server/src/interfaces/api-key.interface.ts b/server/src/interfaces/api-key.interface.ts index 731b7ff6fbb752..473a2b8019a55a 100644 --- a/server/src/interfaces/api-key.interface.ts +++ b/server/src/interfaces/api-key.interface.ts @@ -1,16 +1,19 @@ +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: Partial): Promise; + 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; + getKey(hashedToken: string): Promise; getById(userId: string, id: string): Promise; getByUserId(userId: string): Promise; } diff --git a/server/src/queries/api.key.repository.sql b/server/src/queries/api.key.repository.sql index f4989d355e8808..e1ed8a3dd614ac 100644 --- a/server/src/queries/api.key.repository.sql +++ b/server/src/queries/api.key.repository.sql @@ -1,77 +1,59 @@ -- NOTE: This file is auto generated by ./sql-generator -- ApiKeyRepository.getKey -SELECT DISTINCT - "distinctAlias"."APIKeyEntity_id" AS "ids_APIKeyEntity_id" -FROM - ( - SELECT - "APIKeyEntity"."id" AS "APIKeyEntity_id", - "APIKeyEntity"."key" AS "APIKeyEntity_key", - "APIKeyEntity"."userId" AS "APIKeyEntity_userId", - "APIKeyEntity"."permissions" AS "APIKeyEntity_permissions", - "APIKeyEntity__APIKeyEntity_user"."id" AS "APIKeyEntity__APIKeyEntity_user_id", - "APIKeyEntity__APIKeyEntity_user"."name" AS "APIKeyEntity__APIKeyEntity_user_name", - "APIKeyEntity__APIKeyEntity_user"."isAdmin" AS "APIKeyEntity__APIKeyEntity_user_isAdmin", - "APIKeyEntity__APIKeyEntity_user"."email" AS "APIKeyEntity__APIKeyEntity_user_email", - "APIKeyEntity__APIKeyEntity_user"."storageLabel" AS "APIKeyEntity__APIKeyEntity_user_storageLabel", - "APIKeyEntity__APIKeyEntity_user"."oauthId" AS "APIKeyEntity__APIKeyEntity_user_oauthId", - "APIKeyEntity__APIKeyEntity_user"."profileImagePath" AS "APIKeyEntity__APIKeyEntity_user_profileImagePath", - "APIKeyEntity__APIKeyEntity_user"."shouldChangePassword" AS "APIKeyEntity__APIKeyEntity_user_shouldChangePassword", - "APIKeyEntity__APIKeyEntity_user"."createdAt" AS "APIKeyEntity__APIKeyEntity_user_createdAt", - "APIKeyEntity__APIKeyEntity_user"."deletedAt" AS "APIKeyEntity__APIKeyEntity_user_deletedAt", - "APIKeyEntity__APIKeyEntity_user"."status" AS "APIKeyEntity__APIKeyEntity_user_status", - "APIKeyEntity__APIKeyEntity_user"."updatedAt" AS "APIKeyEntity__APIKeyEntity_user_updatedAt", - "APIKeyEntity__APIKeyEntity_user"."quotaSizeInBytes" AS "APIKeyEntity__APIKeyEntity_user_quotaSizeInBytes", - "APIKeyEntity__APIKeyEntity_user"."quotaUsageInBytes" AS "APIKeyEntity__APIKeyEntity_user_quotaUsageInBytes", - "APIKeyEntity__APIKeyEntity_user"."profileChangedAt" AS "APIKeyEntity__APIKeyEntity_user_profileChangedAt", - "7f5f7a38bf327bfbbf826778460704c9a50fe6f4"."userId" AS "7f5f7a38bf327bfbbf826778460704c9a50fe6f4_userId", - "7f5f7a38bf327bfbbf826778460704c9a50fe6f4"."key" AS "7f5f7a38bf327bfbbf826778460704c9a50fe6f4_key", - "7f5f7a38bf327bfbbf826778460704c9a50fe6f4"."value" AS "7f5f7a38bf327bfbbf826778460704c9a50fe6f4_value" - FROM - "api_keys" "APIKeyEntity" - LEFT JOIN "users" "APIKeyEntity__APIKeyEntity_user" ON "APIKeyEntity__APIKeyEntity_user"."id" = "APIKeyEntity"."userId" - AND ( - "APIKeyEntity__APIKeyEntity_user"."deletedAt" IS NULL - ) - LEFT JOIN "user_metadata" "7f5f7a38bf327bfbbf826778460704c9a50fe6f4" ON "7f5f7a38bf327bfbbf826778460704c9a50fe6f4"."userId" = "APIKeyEntity__APIKeyEntity_user"."id" - WHERE - (("APIKeyEntity"."key" = $1)) - ) "distinctAlias" -ORDER BY - "APIKeyEntity_id" ASC -LIMIT - 1 +select + "api_keys"."id", + "api_keys"."key", + "api_keys"."userId", + "api_keys"."permissions", + to_json("user") as "user" +from + "api_keys" + inner join lateral ( + select + "users".*, + ( + select + array_agg("user_metadata") as "metadata" + from + "user_metadata" + where + "users"."id" = "user_metadata"."userId" + ) as "metadata" + from + "users" + where + "users"."id" = "api_keys"."userId" + and "users"."deletedAt" is null + ) as "user" on true +where + "api_keys"."key" = $1 -- ApiKeyRepository.getById -SELECT - "APIKeyEntity"."id" AS "APIKeyEntity_id", - "APIKeyEntity"."name" AS "APIKeyEntity_name", - "APIKeyEntity"."userId" AS "APIKeyEntity_userId", - "APIKeyEntity"."permissions" AS "APIKeyEntity_permissions", - "APIKeyEntity"."createdAt" AS "APIKeyEntity_createdAt", - "APIKeyEntity"."updatedAt" AS "APIKeyEntity_updatedAt" -FROM - "api_keys" "APIKeyEntity" -WHERE - ( - ("APIKeyEntity"."userId" = $1) - AND ("APIKeyEntity"."id" = $2) - ) -LIMIT - 1 +select + "id", + "name", + "userId", + "createdAt", + "updatedAt", + "permissions" +from + "api_keys" +where + "id" = $1::uuid + and "userId" = $2 -- ApiKeyRepository.getByUserId -SELECT - "APIKeyEntity"."id" AS "APIKeyEntity_id", - "APIKeyEntity"."name" AS "APIKeyEntity_name", - "APIKeyEntity"."userId" AS "APIKeyEntity_userId", - "APIKeyEntity"."permissions" AS "APIKeyEntity_permissions", - "APIKeyEntity"."createdAt" AS "APIKeyEntity_createdAt", - "APIKeyEntity"."updatedAt" AS "APIKeyEntity_updatedAt" -FROM - "api_keys" "APIKeyEntity" -WHERE - (("APIKeyEntity"."userId" = $1)) -ORDER BY - "APIKeyEntity"."createdAt" DESC +select + "id", + "name", + "userId", + "createdAt", + "updatedAt", + "permissions" +from + "api_keys" +where + "userId" = $1 +order by + "createdAt" desc diff --git a/server/src/repositories/api-key.repository.ts b/server/src/repositories/api-key.repository.ts index bb37390de1df35..c0fc7532137c26 100644 --- a/server/src/repositories/api-key.repository.ts +++ b/server/src/repositories/api-key.repository.ts @@ -1,52 +1,97 @@ 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) {} + constructor( + @InjectRepository(APIKeyEntity) private repository: Repository, + @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(); - async create(dto: Partial): Promise { - return this.repository.save(dto); + return { id, name, createdAt, updatedAt, permissions } as APIKeyEntity; } - async update(userId: string, id: string, dto: Partial): Promise { - await this.repository.update({ userId, id }, dto); - return this.repository.findOneOrFail({ where: { id: dto.id } }); + async update(userId: string, id: string, dto: Updateable): Promise { + return this.db + .updateTable('api_keys') + .set(dto) + .where('api_keys.userId', '=', userId) + .where('id', '=', asUuid(id)) + .returningAll() + .executeTakeFirstOrThrow() as unknown as Promise; } async delete(userId: string, id: string): Promise { - await this.repository.delete({ userId, id }); + await this.db.deleteFrom('api_keys').where('userId', '=', userId).where('id', '=', asUuid(id)).execute(); } @GenerateSql({ params: [DummyValue.STRING] }) - getKey(hashedToken: string): Promise { - return this.repository.findOne({ - select: { - id: true, - key: true, - userId: true, - permissions: true, - }, - where: { key: hashedToken }, - relations: { - user: { - metadata: true, - }, - }, - }); + getKey(hashedToken: string): Promise { + return this.db + .selectFrom('api_keys') + .innerJoinLateral( + (eb) => + eb + .selectFrom('users') + .selectAll('users') + .select((eb) => + eb + .selectFrom('user_metadata') + .whereRef('users.id', '=', 'user_metadata.userId') + .select((eb) => eb.fn('array_agg', [eb.table('user_metadata')]).as('metadata')) + .as('metadata'), + ) + .whereRef('users.id', '=', 'api_keys.userId') + .where('users.deletedAt', 'is', null) + .as('user'), + (join) => join.onTrue(), + ) + .select((eb) => [ + 'api_keys.id', + 'api_keys.key', + 'api_keys.userId', + 'api_keys.permissions', + eb.fn.toJson('user').as('user'), + ]) + .where('api_keys.key', '=', hashedToken) + .executeTakeFirst() as Promise; } @GenerateSql({ params: [DummyValue.UUID, DummyValue.UUID] }) getById(userId: string, id: string): Promise { - return this.repository.findOne({ where: { userId, id } }); + return this.db + .selectFrom('api_keys') + .select(columns) + .where('id', '=', asUuid(id)) + .where('userId', '=', userId) + .executeTakeFirst() as unknown as Promise; } @GenerateSql({ params: [DummyValue.UUID] }) getByUserId(userId: string): Promise { - return this.repository.find({ where: { userId }, order: { createdAt: 'DESC' } }); + return this.db + .selectFrom('api_keys') + .select(columns) + .where('userId', '=', userId) + .orderBy('createdAt', 'desc') + .execute() as unknown as Promise; } } diff --git a/server/src/services/api-key.service.spec.ts b/server/src/services/api-key.service.spec.ts index 3841ba1be97565..8d07985440ae0c 100644 --- a/server/src/services/api-key.service.spec.ts +++ b/server/src/services/api-key.service.spec.ts @@ -49,10 +49,7 @@ describe(APIKeyService.name, () => { it('should throw an error if the api key does not have sufficient permissions', async () => { await expect( - sut.create( - { ...authStub.admin, apiKey: { ...keyStub.admin, permissions: [] } }, - { permissions: [Permission.ASSET_READ] }, - ), + sut.create({ ...authStub.admin, apiKey: keyStub.authKey }, { permissions: [Permission.ASSET_READ] }), ).rejects.toBeInstanceOf(BadRequestException); }); }); diff --git a/server/src/services/auth.service.spec.ts b/server/src/services/auth.service.spec.ts index d34e2673f56eff..06035b03a29375 100644 --- a/server/src/services/auth.service.spec.ts +++ b/server/src/services/auth.service.spec.ts @@ -405,7 +405,7 @@ describe('AuthService', () => { describe('validate - api key', () => { it('should throw an error if no api key is found', async () => { - keyMock.getKey.mockResolvedValue(null); + keyMock.getKey.mockResolvedValue(void 0); await expect( sut.authenticate({ headers: { 'x-api-key': 'auth_token' }, @@ -417,7 +417,7 @@ describe('AuthService', () => { }); it('should throw an error if api key has insufficient permissions', async () => { - keyMock.getKey.mockResolvedValue({ ...keyStub.admin, permissions: [] }); + keyMock.getKey.mockResolvedValue(keyStub.authKey); await expect( sut.authenticate({ headers: { 'x-api-key': 'auth_token' }, @@ -428,14 +428,14 @@ describe('AuthService', () => { }); it('should return an auth dto', async () => { - keyMock.getKey.mockResolvedValue(keyStub.admin); + keyMock.getKey.mockResolvedValue(keyStub.authKey); await expect( sut.authenticate({ headers: { 'x-api-key': 'auth_token' }, queryParams: {}, metadata: { adminRoute: false, sharedLinkRoute: false, uri: 'test' }, }), - ).resolves.toEqual({ user: userStub.admin, apiKey: keyStub.admin }); + ).resolves.toEqual({ user: userStub.admin, apiKey: keyStub.authKey }); expect(keyMock.getKey).toHaveBeenCalledWith('auth_token (hashed)'); }); }); diff --git a/server/src/services/auth.service.ts b/server/src/services/auth.service.ts index 0d44fa05622351..d6154976f9ffe1 100644 --- a/server/src/services/auth.service.ts +++ b/server/src/services/auth.service.ts @@ -308,7 +308,7 @@ export class AuthService extends BaseService { private async validateApiKey(key: string): Promise { const hashedKey = this.cryptoRepository.hashSha256(key); const apiKey = await this.keyRepository.getKey(hashedKey); - if (apiKey?.user) { + if (apiKey) { return { user: apiKey.user, apiKey }; } diff --git a/server/src/types.ts b/server/src/types.ts new file mode 100644 index 00000000000000..c55de4160dd156 --- /dev/null +++ b/server/src/types.ts @@ -0,0 +1,9 @@ +import { UserEntity } from 'src/entities/user.entity'; +import { Permission } from 'src/enum'; + +export type AuthApiKey = { + id: string; + key: string; + user: UserEntity; + permissions: Permission[]; +}; diff --git a/server/test/fixtures/api-key.stub.ts b/server/test/fixtures/api-key.stub.ts index f8b1832c84e37d..248d30c2eccda4 100644 --- a/server/test/fixtures/api-key.stub.ts +++ b/server/test/fixtures/api-key.stub.ts @@ -1,8 +1,16 @@ 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'; export const keyStub = { + authKey: Object.freeze({ + id: 'my-random-guid', + key: 'my-api-key (hashed)', + user: userStub.admin, + permissions: [], + } as AuthApiKey), + admin: Object.freeze({ id: 'my-random-guid', name: 'My Key',