diff --git a/server/src/controllers/api-key.controller.ts b/server/src/controllers/api-key.controller.ts index 4691ce05ef..08efd753cf 100644 --- a/server/src/controllers/api-key.controller.ts +++ b/server/src/controllers/api-key.controller.ts @@ -4,13 +4,13 @@ import { APIKeyCreateDto, APIKeyCreateResponseDto, APIKeyResponseDto, APIKeyUpda import { AuthDto } from 'src/dtos/auth.dto'; import { Permission } from 'src/enum'; import { Auth, Authenticated } from 'src/middleware/auth.guard'; -import { APIKeyService } from 'src/services/api-key.service'; +import { ApiKeyService } from 'src/services/api-key.service'; import { UUIDParamDto } from 'src/validation'; @ApiTags('API Keys') @Controller('api-keys') export class APIKeyController { - constructor(private service: APIKeyService) {} + constructor(private service: ApiKeyService) {} @Post() @Authenticated({ permission: Permission.API_KEY_CREATE }) diff --git a/server/src/database.ts b/server/src/database.ts index 4ec76fa9d2..08ed7240db 100644 --- a/server/src/database.ts +++ b/server/src/database.ts @@ -29,6 +29,15 @@ export type AuthApiKey = { permissions: Permission[]; }; +export type ApiKey = { + id: string; + name: string; + userId: string; + createdAt: Date; + updatedAt: Date; + permissions: Permission[]; +}; + export type User = { id: string; name: string; diff --git a/server/src/repositories/api-key.repository.ts b/server/src/repositories/api-key.repository.ts index 4ed463365b..0085db2337 100644 --- a/server/src/repositories/api-key.repository.ts +++ b/server/src/repositories/api-key.repository.ts @@ -12,7 +12,7 @@ export class ApiKeyRepository { constructor(@InjectKysely() private db: Kysely) {} create(dto: Insertable) { - return this.db.insertInto('api_keys').values(dto).returningAll().executeTakeFirstOrThrow(); + return this.db.insertInto('api_keys').values(dto).returning(columns.apiKey).executeTakeFirstOrThrow(); } async update(userId: string, id: string, dto: Updateable) { @@ -21,7 +21,7 @@ export class ApiKeyRepository { .set(dto) .where('api_keys.userId', '=', userId) .where('id', '=', asUuid(id)) - .returningAll() + .returning(columns.apiKey) .executeTakeFirstOrThrow(); } diff --git a/server/src/services/api-key.service.spec.ts b/server/src/services/api-key.service.spec.ts index 905bbede2a..53266b493e 100644 --- a/server/src/services/api-key.service.spec.ts +++ b/server/src/services/api-key.service.spec.ts @@ -1,112 +1,145 @@ import { BadRequestException } from '@nestjs/common'; import { Permission } from 'src/enum'; -import { APIKeyService } from 'src/services/api-key.service'; -import { keyStub } from 'test/fixtures/api-key.stub'; -import { authStub } from 'test/fixtures/auth.stub'; +import { ApiKeyService } from 'src/services/api-key.service'; +import { factory, newUuid } from 'test/small.factory'; import { newTestService, ServiceMocks } from 'test/utils'; -describe(APIKeyService.name, () => { - let sut: APIKeyService; +describe(ApiKeyService.name, () => { + let sut: ApiKeyService; let mocks: ServiceMocks; beforeEach(() => { - ({ sut, mocks } = newTestService(APIKeyService)); + ({ sut, mocks } = newTestService(ApiKeyService)); }); describe('create', () => { it('should create a new key', async () => { - mocks.apiKey.create.mockResolvedValue(keyStub.admin); - await sut.create(authStub.admin, { name: 'Test Key', permissions: [Permission.ALL] }); + const auth = factory.auth(); + const apiKey = factory.apiKey({ userId: auth.user.id, permissions: [Permission.ALL] }); + const key = 'super-secret'; + + mocks.crypto.newPassword.mockReturnValue(key); + mocks.apiKey.create.mockResolvedValue(apiKey); + + await sut.create(auth, { name: apiKey.name, permissions: apiKey.permissions }); + expect(mocks.apiKey.create).toHaveBeenCalledWith({ - key: 'cmFuZG9tLWJ5dGVz (hashed)', - name: 'Test Key', - permissions: [Permission.ALL], - userId: authStub.admin.user.id, + key: 'super-secret (hashed)', + name: apiKey.name, + permissions: apiKey.permissions, + userId: apiKey.userId, }); expect(mocks.crypto.newPassword).toHaveBeenCalled(); expect(mocks.crypto.hashSha256).toHaveBeenCalled(); }); it('should not require a name', async () => { - mocks.apiKey.create.mockResolvedValue(keyStub.admin); + const auth = factory.auth(); + const apiKey = factory.apiKey({ userId: auth.user.id }); + const key = 'super-secret'; - await sut.create(authStub.admin, { permissions: [Permission.ALL] }); + mocks.crypto.newPassword.mockReturnValue(key); + mocks.apiKey.create.mockResolvedValue(apiKey); + + await sut.create(auth, { permissions: [Permission.ALL] }); expect(mocks.apiKey.create).toHaveBeenCalledWith({ - key: 'cmFuZG9tLWJ5dGVz (hashed)', + key: 'super-secret (hashed)', name: 'API Key', permissions: [Permission.ALL], - userId: authStub.admin.user.id, + userId: auth.user.id, }); expect(mocks.crypto.newPassword).toHaveBeenCalled(); expect(mocks.crypto.hashSha256).toHaveBeenCalled(); }); it('should throw an error if the api key does not have sufficient permissions', async () => { - await expect( - sut.create({ ...authStub.admin, apiKey: keyStub.authKey }, { permissions: [Permission.ASSET_READ] }), - ).rejects.toBeInstanceOf(BadRequestException); + const auth = factory.auth({ apiKey: factory.authApiKey({ permissions: [Permission.ASSET_READ] }) }); + + await expect(sut.create(auth, { permissions: [Permission.ASSET_UPDATE] })).rejects.toBeInstanceOf( + BadRequestException, + ); }); }); describe('update', () => { it('should throw an error if the key is not found', async () => { - await expect(sut.update(authStub.admin, 'random-guid', { name: 'New Name' })).rejects.toBeInstanceOf( - BadRequestException, - ); + const id = newUuid(); + const auth = factory.auth(); - expect(mocks.apiKey.update).not.toHaveBeenCalledWith('random-guid'); + await expect(sut.update(auth, id, { name: 'New Name' })).rejects.toBeInstanceOf(BadRequestException); + + expect(mocks.apiKey.update).not.toHaveBeenCalledWith(id); }); it('should update a key', async () => { - mocks.apiKey.getById.mockResolvedValue(keyStub.admin); - mocks.apiKey.update.mockResolvedValue(keyStub.admin); + const auth = factory.auth(); + const apiKey = factory.apiKey({ userId: auth.user.id }); + const newName = 'New name'; - await sut.update(authStub.admin, 'random-guid', { name: 'New Name' }); + mocks.apiKey.getById.mockResolvedValue(apiKey); + mocks.apiKey.update.mockResolvedValue(apiKey); - expect(mocks.apiKey.update).toHaveBeenCalledWith(authStub.admin.user.id, 'random-guid', { name: 'New Name' }); + await sut.update(auth, apiKey.id, { name: newName }); + + expect(mocks.apiKey.update).toHaveBeenCalledWith(auth.user.id, apiKey.id, { name: newName }); }); }); describe('delete', () => { it('should throw an error if the key is not found', async () => { - await expect(sut.delete(authStub.admin, 'random-guid')).rejects.toBeInstanceOf(BadRequestException); + const auth = factory.auth(); + const id = newUuid(); - expect(mocks.apiKey.delete).not.toHaveBeenCalledWith('random-guid'); + await expect(sut.delete(auth, id)).rejects.toBeInstanceOf(BadRequestException); + + expect(mocks.apiKey.delete).not.toHaveBeenCalledWith(id); }); it('should delete a key', async () => { - mocks.apiKey.getById.mockResolvedValue(keyStub.admin); + const auth = factory.auth(); + const apiKey = factory.apiKey({ userId: auth.user.id }); - await sut.delete(authStub.admin, 'random-guid'); + mocks.apiKey.getById.mockResolvedValue(apiKey); - expect(mocks.apiKey.delete).toHaveBeenCalledWith(authStub.admin.user.id, 'random-guid'); + await sut.delete(auth, apiKey.id); + + expect(mocks.apiKey.delete).toHaveBeenCalledWith(auth.user.id, apiKey.id); }); }); describe('getById', () => { it('should throw an error if the key is not found', async () => { - await expect(sut.getById(authStub.admin, 'random-guid')).rejects.toBeInstanceOf(BadRequestException); + const auth = factory.auth(); + const id = newUuid(); - expect(mocks.apiKey.getById).toHaveBeenCalledWith(authStub.admin.user.id, 'random-guid'); + await expect(sut.getById(auth, id)).rejects.toBeInstanceOf(BadRequestException); + + expect(mocks.apiKey.getById).toHaveBeenCalledWith(auth.user.id, id); }); it('should get a key by id', async () => { - mocks.apiKey.getById.mockResolvedValue(keyStub.admin); + const auth = factory.auth(); + const apiKey = factory.apiKey({ userId: auth.user.id }); - await sut.getById(authStub.admin, 'random-guid'); + mocks.apiKey.getById.mockResolvedValue(apiKey); - expect(mocks.apiKey.getById).toHaveBeenCalledWith(authStub.admin.user.id, 'random-guid'); + await sut.getById(auth, apiKey.id); + + expect(mocks.apiKey.getById).toHaveBeenCalledWith(auth.user.id, apiKey.id); }); }); describe('getAll', () => { it('should return all the keys for a user', async () => { - mocks.apiKey.getByUserId.mockResolvedValue([keyStub.admin]); + const auth = factory.auth(); + const apiKey = factory.apiKey({ userId: auth.user.id }); - await expect(sut.getAll(authStub.admin)).resolves.toHaveLength(1); + mocks.apiKey.getByUserId.mockResolvedValue([apiKey]); - expect(mocks.apiKey.getByUserId).toHaveBeenCalledWith(authStub.admin.user.id); + await expect(sut.getAll(auth)).resolves.toHaveLength(1); + + expect(mocks.apiKey.getByUserId).toHaveBeenCalledWith(auth.user.id); }); }); }); diff --git a/server/src/services/api-key.service.ts b/server/src/services/api-key.service.ts index 44c3015330..5459b56889 100644 --- a/server/src/services/api-key.service.ts +++ b/server/src/services/api-key.service.ts @@ -7,7 +7,7 @@ import { ApiKeyItem } from 'src/types'; import { isGranted } from 'src/utils/access'; @Injectable() -export class APIKeyService extends BaseService { +export class ApiKeyService extends BaseService { async create(auth: AuthDto, dto: APIKeyCreateDto): Promise { const secret = this.cryptoRepository.newPassword(32); diff --git a/server/src/services/auth.service.spec.ts b/server/src/services/auth.service.spec.ts index 9fb1af128e..2a3d905275 100644 --- a/server/src/services/auth.service.spec.ts +++ b/server/src/services/auth.service.spec.ts @@ -4,12 +4,11 @@ import { UserMetadataEntity } from 'src/entities/user-metadata.entity'; import { UserEntity } from 'src/entities/user.entity'; import { AuthType, Permission } from 'src/enum'; import { AuthService } from 'src/services/auth.service'; -import { keyStub } from 'test/fixtures/api-key.stub'; -import { authStub } from 'test/fixtures/auth.stub'; import { sessionStub } from 'test/fixtures/session.stub'; import { sharedLinkStub } from 'test/fixtures/shared-link.stub'; import { systemConfigStub } from 'test/fixtures/system-config.stub'; import { userStub } from 'test/fixtures/user.stub'; +import { factory } from 'test/small.factory'; import { newTestService, ServiceMocks } from 'test/utils'; const oauthResponse = { @@ -398,7 +397,10 @@ describe('AuthService', () => { }); it('should throw an error if api key has insufficient permissions', async () => { - mocks.apiKey.getKey.mockResolvedValue(keyStub.authKey); + const authUser = factory.authUser(); + const authApiKey = factory.authApiKey({ permissions: [] }); + + mocks.apiKey.getKey.mockResolvedValue({ ...authApiKey, user: authUser }); await expect( sut.authenticate({ headers: { 'x-api-key': 'auth_token' }, @@ -409,14 +411,18 @@ describe('AuthService', () => { }); it('should return an auth dto', async () => { - mocks.apiKey.getKey.mockResolvedValue(keyStub.authKey); + const authUser = factory.authUser(); + const authApiKey = factory.authApiKey({ permissions: [] }); + + mocks.apiKey.getKey.mockResolvedValue({ ...authApiKey, user: authUser }); + 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.authKey }); + ).resolves.toEqual({ user: authUser, apiKey: expect.objectContaining(authApiKey) }); expect(mocks.apiKey.getKey).toHaveBeenCalledWith('auth_token (hashed)'); }); }); @@ -622,19 +628,27 @@ describe('AuthService', () => { describe('link', () => { it('should link an account', async () => { + const authUser = factory.authUser(); + const authApiKey = factory.authApiKey({ permissions: [] }); + const auth = { user: authUser, apiKey: authApiKey }; + mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.enabled); mocks.user.update.mockResolvedValue(userStub.user1); - await sut.link(authStub.user1, { url: 'http://immich/user-settings?code=abc123' }); + await sut.link(auth, { url: 'http://immich/user-settings?code=abc123' }); - expect(mocks.user.update).toHaveBeenCalledWith(authStub.user1.user.id, { oauthId: sub }); + expect(mocks.user.update).toHaveBeenCalledWith(auth.user.id, { oauthId: sub }); }); it('should not link an already linked oauth.sub', async () => { + const authUser = factory.authUser(); + const authApiKey = factory.authApiKey({ permissions: [] }); + const auth = { user: authUser, apiKey: authApiKey }; + mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.enabled); mocks.user.getByOAuthId.mockResolvedValue({ id: 'other-user' } as UserEntity); - await expect(sut.link(authStub.user1, { url: 'http://immich/user-settings?code=abc123' })).rejects.toBeInstanceOf( + await expect(sut.link(auth, { url: 'http://immich/user-settings?code=abc123' })).rejects.toBeInstanceOf( BadRequestException, ); @@ -644,12 +658,16 @@ describe('AuthService', () => { describe('unlink', () => { it('should unlink an account', async () => { + const authUser = factory.authUser(); + const authApiKey = factory.authApiKey({ permissions: [] }); + const auth = { user: authUser, apiKey: authApiKey }; + mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.enabled); mocks.user.update.mockResolvedValue(userStub.user1); - await sut.unlink(authStub.user1); + await sut.unlink(auth); - expect(mocks.user.update).toHaveBeenCalledWith(authStub.user1.user.id, { oauthId: '' }); + expect(mocks.user.update).toHaveBeenCalledWith(auth.user.id, { oauthId: '' }); }); }); }); diff --git a/server/src/services/index.ts b/server/src/services/index.ts index 0dd8bdae66..b214dd14f6 100644 --- a/server/src/services/index.ts +++ b/server/src/services/index.ts @@ -1,6 +1,6 @@ import { ActivityService } from 'src/services/activity.service'; import { AlbumService } from 'src/services/album.service'; -import { APIKeyService } from 'src/services/api-key.service'; +import { ApiKeyService } from 'src/services/api-key.service'; import { ApiService } from 'src/services/api.service'; import { AssetMediaService } from 'src/services/asset-media.service'; import { AssetService } from 'src/services/asset.service'; @@ -40,7 +40,7 @@ import { VersionService } from 'src/services/version.service'; import { ViewService } from 'src/services/view.service'; export const services = [ - APIKeyService, + ApiKeyService, ActivityService, AlbumService, ApiService, diff --git a/server/test/fixtures/api-key.stub.ts b/server/test/fixtures/api-key.stub.ts deleted file mode 100644 index 905bda34b4..0000000000 --- a/server/test/fixtures/api-key.stub.ts +++ /dev/null @@ -1,20 +0,0 @@ -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 any), - - admin: Object.freeze({ - id: 'my-random-guid', - name: 'My Key', - key: 'my-api-key (hashed)', - userId: authStub.admin.user.id, - user: userStub.admin, - permissions: [], - } as any), -}; diff --git a/server/test/small.factory.ts b/server/test/small.factory.ts index d93056713a..d7b710eb7d 100644 --- a/server/test/small.factory.ts +++ b/server/test/small.factory.ts @@ -1,7 +1,8 @@ import { randomUUID } from 'node:crypto'; -import { Asset, AuthUser, Library, User } from 'src/database'; +import { ApiKey, Asset, AuthApiKey, AuthUser, Library, User } from 'src/database'; +import { AuthDto } from 'src/dtos/auth.dto'; import { OnThisDayData } from 'src/entities/memory.entity'; -import { AssetStatus, AssetType, MemoryType } from 'src/enum'; +import { AssetStatus, AssetType, MemoryType, Permission } from 'src/enum'; import { ActivityItem, MemoryItem } from 'src/types'; export const newUuid = () => randomUUID() as string; @@ -13,11 +14,25 @@ export const newDate = () => new Date(); export const newUpdateId = () => 'uuid-v7'; export const newSha1 = () => Buffer.from('this is a fake hash'); -const authFactory = (user: Partial = {}) => ({ - user: authUserFactory(user), +const authFactory = ({ apiKey, ...user }: Partial & { apiKey?: Partial } = {}) => { + const auth: AuthDto = { + user: authUserFactory(user), + }; + + if (apiKey) { + auth.apiKey = authApiKeyFactory(apiKey); + } + + return auth; +}; + +const authApiKeyFactory = (apiKey: Partial = {}) => ({ + id: newUuid(), + permissions: [Permission.ALL], + ...apiKey, }); -const authUserFactory = (authUser: Partial) => ({ +const authUserFactory = (authUser: Partial = {}) => ({ id: newUuid(), isAdmin: false, name: 'Test User', @@ -86,6 +101,17 @@ const activityFactory = (activity: Partial = {}) => { }; }; +const apiKeyFactory = (apiKey: Partial = {}) => ({ + id: newUuid(), + userId: newUuid(), + createdAt: newDate(), + updatedAt: newDate(), + updateId: newUpdateId(), + name: 'Api Key', + permissions: [Permission.ALL], + ...apiKey, +}); + const libraryFactory = (library: Partial = {}) => ({ id: newUuid(), createdAt: newDate(), @@ -121,8 +147,10 @@ const memoryFactory = (memory: Partial = {}) => ({ export const factory = { activity: activityFactory, + apiKey: apiKeyFactory, asset: assetFactory, auth: authFactory, + authApiKey: authApiKeyFactory, authUser: authUserFactory, library: libraryFactory, memory: memoryFactory,