refactor: api key spec to use factories (#16776)

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 })

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;

server/src/repositories/api-key.repository.ts

@@ -12,7 +12,7 @@ export class ApiKeyRepository {
   constructor(@InjectKysely() private db: Kysely<DB>) {}
 
   create(dto: Insertable<ApiKeys>) {
-    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<ApiKeys>) {
@@ -21,7 +21,7 @@ export class ApiKeyRepository {
       .set(dto)
       .where('api_keys.userId', '=', userId)
       .where('id', '=', asUuid(id))
-      .returningAll()
+      .returning(columns.apiKey)
       .executeTakeFirstOrThrow();
   }
 

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);
     });
   });
 });

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<APIKeyCreateResponseDto> {
     const secret = this.cryptoRepository.newPassword(32);
 

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: '' });
     });
   });
 });

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,

server/test/fixtures/api-key.stub.ts

@@ -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),
-};

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<AuthUser> = {}) => ({
-  user: authUserFactory(user),
+const authFactory = ({ apiKey, ...user }: Partial<AuthUser> & { apiKey?: Partial<AuthApiKey> } = {}) => {
+  const auth: AuthDto = {
+    user: authUserFactory(user),
+  };
+
+  if (apiKey) {
+    auth.apiKey = authApiKeyFactory(apiKey);
+  }
+
+  return auth;
+};
+
+const authApiKeyFactory = (apiKey: Partial<AuthApiKey> = {}) => ({
+  id: newUuid(),
+  permissions: [Permission.ALL],
+  ...apiKey,
 });
 
-const authUserFactory = (authUser: Partial<AuthUser>) => ({
+const authUserFactory = (authUser: Partial<AuthUser> = {}) => ({
   id: newUuid(),
   isAdmin: false,
   name: 'Test User',
@@ -86,6 +101,17 @@ const activityFactory = (activity: Partial<ActivityItem> = {}) => {
   };
 };
 
+const apiKeyFactory = (apiKey: Partial<ApiKey> = {}) => ({
+  id: newUuid(),
+  userId: newUuid(),
+  createdAt: newDate(),
+  updatedAt: newDate(),
+  updateId: newUpdateId(),
+  name: 'Api Key',
+  permissions: [Permission.ALL],
+  ...apiKey,
+});
+
 const libraryFactory = (library: Partial<Library> = {}) => ({
   id: newUuid(),
   createdAt: newDate(),
@@ -121,8 +147,10 @@ const memoryFactory = (memory: Partial<MemoryItem> = {}) => ({
 
 export const factory = {
   activity: activityFactory,
+  apiKey: apiKeyFactory,
   asset: assetFactory,
   auth: authFactory,
+  authApiKey: authApiKeyFactory,
   authUser: authUserFactory,
   library: libraryFactory,
   memory: memoryFactory,