refactor: auth service (#11811)

e2e/src/api/specs/api-key.e2e-spec.ts

@@ -0,0 +1,206 @@
+import { LoginResponseDto, createApiKey } from '@immich/sdk';
+import { createUserDto, uuidDto } from 'src/fixtures';
+import { errorDto } from 'src/responses';
+import { app, asBearerAuth, utils } from 'src/utils';
+import request from 'supertest';
+import { beforeAll, beforeEach, describe, expect, it } from 'vitest';
+
+const create = (accessToken: string) =>
+  createApiKey({ apiKeyCreateDto: { name: 'api key' } }, { headers: asBearerAuth(accessToken) });
+
+describe('/api-keys', () => {
+  let admin: LoginResponseDto;
+  let user: LoginResponseDto;
+
+  beforeAll(async () => {
+    await utils.resetDatabase();
+
+    admin = await utils.adminSetup();
+    user = await utils.userSetup(admin.accessToken, createUserDto.user1);
+  });
+
+  beforeEach(async () => {
+    await utils.resetDatabase(['api_keys']);
+  });
+
+  describe('POST /api-keys', () => {
+    it('should require authentication', async () => {
+      const { status, body } = await request(app).post('/api-keys').send({ name: 'API Key' });
+      expect(status).toBe(401);
+      expect(body).toEqual(errorDto.unauthorized);
+    });
+
+    it('should create an api key', async () => {
+      const { status, body } = await request(app)
+        .post('/api-keys')
+        .send({ name: 'API Key' })
+        .set('Authorization', `Bearer ${admin.accessToken}`);
+      expect(body).toEqual({
+        apiKey: {
+          id: expect.any(String),
+          name: 'API Key',
+          createdAt: expect.any(String),
+          updatedAt: expect.any(String),
+        },
+        secret: expect.any(String),
+      });
+      expect(status).toEqual(201);
+    });
+  });
+
+  describe('GET /api-keys', () => {
+    it('should require authentication', async () => {
+      const { status, body } = await request(app).get('/api-keys');
+      expect(status).toBe(401);
+      expect(body).toEqual(errorDto.unauthorized);
+    });
+
+    it('should start off empty', async () => {
+      const { status, body } = await request(app).get('/api-keys').set('Authorization', `Bearer ${admin.accessToken}`);
+      expect(body).toEqual([]);
+      expect(status).toEqual(200);
+    });
+
+    it('should return a list of api keys', async () => {
+      const [{ apiKey: apiKey1 }, { apiKey: apiKey2 }, { apiKey: apiKey3 }] = await Promise.all([
+        create(admin.accessToken),
+        create(admin.accessToken),
+        create(admin.accessToken),
+      ]);
+      const { status, body } = await request(app).get('/api-keys').set('Authorization', `Bearer ${admin.accessToken}`);
+      expect(body).toHaveLength(3);
+      expect(body).toEqual(expect.arrayContaining([apiKey1, apiKey2, apiKey3]));
+      expect(status).toEqual(200);
+    });
+  });
+
+  describe('GET /api-keys/:id', () => {
+    it('should require authentication', async () => {
+      const { status, body } = await request(app).get(`/api-keys/${uuidDto.notFound}`);
+      expect(status).toBe(401);
+      expect(body).toEqual(errorDto.unauthorized);
+    });
+
+    it('should require authorization', async () => {
+      const { apiKey } = await create(user.accessToken);
+      const { status, body } = await request(app)
+        .get(`/api-keys/${apiKey.id}`)
+        .set('Authorization', `Bearer ${admin.accessToken}`);
+      expect(status).toBe(400);
+      expect(body).toEqual(errorDto.badRequest('API Key not found'));
+    });
+
+    it('should require a valid uuid', async () => {
+      const { status, body } = await request(app)
+        .get(`/api-keys/${uuidDto.invalid}`)
+        .set('Authorization', `Bearer ${admin.accessToken}`);
+      expect(status).toBe(400);
+      expect(body).toEqual(errorDto.badRequest(['id must be a UUID']));
+    });
+
+    it('should get api key details', async () => {
+      const { apiKey } = await create(user.accessToken);
+      const { status, body } = await request(app)
+        .get(`/api-keys/${apiKey.id}`)
+        .set('Authorization', `Bearer ${user.accessToken}`);
+      expect(status).toBe(200);
+      expect(body).toEqual({
+        id: expect.any(String),
+        name: 'api key',
+        createdAt: expect.any(String),
+        updatedAt: expect.any(String),
+      });
+    });
+  });
+
+  describe('PUT /api-keys/:id', () => {
+    it('should require authentication', async () => {
+      const { status, body } = await request(app).put(`/api-keys/${uuidDto.notFound}`).send({ name: 'new name' });
+      expect(status).toBe(401);
+      expect(body).toEqual(errorDto.unauthorized);
+    });
+
+    it('should require authorization', async () => {
+      const { apiKey } = await create(user.accessToken);
+      const { status, body } = await request(app)
+        .put(`/api-keys/${apiKey.id}`)
+        .send({ name: 'new name' })
+        .set('Authorization', `Bearer ${admin.accessToken}`);
+      expect(status).toBe(400);
+      expect(body).toEqual(errorDto.badRequest('API Key not found'));
+    });
+
+    it('should require a valid uuid', async () => {
+      const { status, body } = await request(app)
+        .put(`/api-keys/${uuidDto.invalid}`)
+        .send({ name: 'new name' })
+        .set('Authorization', `Bearer ${admin.accessToken}`);
+      expect(status).toBe(400);
+      expect(body).toEqual(errorDto.badRequest(['id must be a UUID']));
+    });
+
+    it('should update api key details', async () => {
+      const { apiKey } = await create(user.accessToken);
+      const { status, body } = await request(app)
+        .put(`/api-keys/${apiKey.id}`)
+        .send({ name: 'new name' })
+        .set('Authorization', `Bearer ${user.accessToken}`);
+      expect(status).toBe(200);
+      expect(body).toEqual({
+        id: expect.any(String),
+        name: 'new name',
+        createdAt: expect.any(String),
+        updatedAt: expect.any(String),
+      });
+    });
+  });
+
+  describe('DELETE /api-keys/:id', () => {
+    it('should require authentication', async () => {
+      const { status, body } = await request(app).delete(`/api-keys/${uuidDto.notFound}`);
+      expect(status).toBe(401);
+      expect(body).toEqual(errorDto.unauthorized);
+    });
+
+    it('should require authorization', async () => {
+      const { apiKey } = await create(user.accessToken);
+      const { status, body } = await request(app)
+        .delete(`/api-keys/${apiKey.id}`)
+        .set('Authorization', `Bearer ${admin.accessToken}`);
+      expect(status).toBe(400);
+      expect(body).toEqual(errorDto.badRequest('API Key not found'));
+    });
+
+    it('should require a valid uuid', async () => {
+      const { status, body } = await request(app)
+        .delete(`/api-keys/${uuidDto.invalid}`)
+        .set('Authorization', `Bearer ${admin.accessToken}`);
+      expect(status).toBe(400);
+      expect(body).toEqual(errorDto.badRequest(['id must be a UUID']));
+    });
+
+    it('should delete an api key', async () => {
+      const { apiKey } = await create(user.accessToken);
+      const { status } = await request(app)
+        .delete(`/api-keys/${apiKey.id}`)
+        .set('Authorization', `Bearer ${user.accessToken}`);
+      expect(status).toBe(204);
+    });
+  });
+
+  describe('authentication', () => {
+    it('should work as a header', async () => {
+      const { secret } = await create(admin.accessToken);
+      const { status, body } = await request(app).get('/api-keys').set('x-api-key', secret);
+      expect(body).toHaveLength(1);
+      expect(status).toBe(200);
+    });
+
+    it('should work as a query param', async () => {
+      const { secret } = await create(admin.accessToken);
+      const { status, body } = await request(app).get(`/api-keys?apiKey=${secret}`);
+      expect(body).toHaveLength(1);
+      expect(status).toBe(200);
+    });
+  });
+});

open-api/immich-openapi-specs.json

@@ -1185,7 +1185,7 @@
           }
         ],
         "responses": {
-          "200": {
+          "204": {
             "description": ""
           }
         },

server/src/controllers/api-key.controller.ts

@@ -1,4 +1,4 @@
-import { Body, Controller, Delete, Get, Param, Post, Put } from '@nestjs/common';
+import { Body, Controller, Delete, Get, HttpCode, HttpStatus, Param, Post, Put } from '@nestjs/common';
 import { ApiTags } from '@nestjs/swagger';
 import { APIKeyCreateDto, APIKeyCreateResponseDto, APIKeyResponseDto, APIKeyUpdateDto } from 'src/dtos/api-key.dto';
 import { AuthDto } from 'src/dtos/auth.dto';
@@ -40,6 +40,7 @@ export class APIKeyController {
   }
 
   @Delete(':id')
+  @HttpCode(HttpStatus.NO_CONTENT)
   @Authenticated()
   deleteApiKey(@Auth() auth: AuthDto, @Param() { id }: UUIDParamDto): Promise<void> {
     return this.service.delete(auth, id);

server/src/dtos/auth.dto.ts

@@ -25,6 +25,8 @@ export enum ImmichHeader {
 
 export enum ImmichQuery {
   SHARED_LINK_KEY = 'key',
+  API_KEY = 'apiKey',
+  SESSION_KEY = 'sessionKey',
 }
 
 export type CookieResponse = {

server/src/middleware/auth.guard.ts

@@ -89,20 +89,14 @@ export class AuthGuard implements CanActivate {
       return true;
     }
 
+    const { admin: adminRoute, sharedLink: sharedLinkRoute } = { sharedLink: false, admin: false, ...options };
     const request = context.switchToHttp().getRequest<AuthRequest>();
 
-    const authDto = await this.authService.validate(request.headers, request.query as Record<string, string>);
-    if (authDto.sharedLink && !(options as SharedLinkRoute).sharedLink) {
-      this.logger.warn(`Denied access to non-shared route: ${request.path}`);
-      return false;
-    }
-
-    if (!authDto.user.isAdmin && (options as AdminRoute).admin) {
-      this.logger.warn(`Denied access to admin only route: ${request.path}`);
-      return false;
-    }
-
-    request.user = authDto;
+    request.user = await this.authService.authenticate({
+      headers: request.headers,
+      queryParams: request.query as Record<string, string>,
+      metadata: { adminRoute, sharedLinkRoute, uri: request.path },
+    });
 
     return true;
   }

server/src/repositories/event.repository.ts

@@ -59,7 +59,11 @@ export class EventRepository implements OnGatewayConnection, OnGatewayDisconnect
   async handleConnection(client: Socket) {
     try {
       this.logger.log(`Websocket Connect:    ${client.id}`);
-      const auth = await this.authService.validate(client.request.headers, {});
+      const auth = await this.authService.authenticate({
+        headers: client.request.headers,
+        queryParams: {},
+        metadata: { adminRoute: false, sharedLinkRoute: false, uri: '/api/socket.io' },
+      });
       await client.join(auth.user.id);
       this.serverSend(ServerEvent.WEBSOCKET_CONNECT, { userId: auth.user.id });
     } catch (error: Error | any) {

server/src/services/auth.service.spec.ts

@@ -1,7 +1,5 @@
-import { BadRequestException, UnauthorizedException } from '@nestjs/common';
-import { IncomingHttpHeaders } from 'node:http';
+import { BadRequestException, ForbiddenException, UnauthorizedException } from '@nestjs/common';
 import { Issuer, generators } from 'openid-client';
-import { Socket } from 'socket.io';
 import { AuthType } from 'src/constants';
 import { AuthDto, SignUpDto } from 'src/dtos/auth.dto';
 import { UserMetadataEntity } from 'src/entities/user-metadata.entity';
@@ -252,15 +250,26 @@ describe('AuthService', () => {
   });
 
   describe('validate - socket connections', () => {
-    it('should throw token is not provided', async () => {
-      await expect(sut.validate({}, {})).rejects.toBeInstanceOf(UnauthorizedException);
+    it('should throw when token is not provided', async () => {
+      await expect(
+        sut.authenticate({
+          headers: {},
+          queryParams: {},
+          metadata: { adminRoute: false, sharedLinkRoute: false, uri: 'test' },
+        }),
+      ).rejects.toBeInstanceOf(UnauthorizedException);
     });
 
     it('should validate using authorization header', async () => {
       userMock.get.mockResolvedValue(userStub.user1);
       sessionMock.getByToken.mockResolvedValue(sessionStub.valid);
-      const client = { request: { headers: { authorization: 'Bearer auth_token' } } };
-      await expect(sut.validate((client as Socket).request.headers, {})).resolves.toEqual({
+      await expect(
+        sut.authenticate({
+          headers: { authorization: 'Bearer auth_token' },
+          queryParams: {},
+          metadata: { adminRoute: false, sharedLinkRoute: false, uri: 'test' },
+        }),
+      ).resolves.toEqual({
         user: userStub.user1,
         session: sessionStub.valid,
       });
@@ -270,28 +279,48 @@ describe('AuthService', () => {
   describe('validate - shared key', () => {
     it('should not accept a non-existent key', async () => {
       shareMock.getByKey.mockResolvedValue(null);
-      const headers: IncomingHttpHeaders = { 'x-immich-share-key': 'key' };
-      await expect(sut.validate(headers, {})).rejects.toBeInstanceOf(UnauthorizedException);
+      await expect(
+        sut.authenticate({
+          headers: { 'x-immich-share-key': 'key' },
+          queryParams: {},
+          metadata: { adminRoute: false, sharedLinkRoute: true, uri: 'test' },
+        }),
+      ).rejects.toBeInstanceOf(UnauthorizedException);
     });
 
     it('should not accept an expired key', async () => {
       shareMock.getByKey.mockResolvedValue(sharedLinkStub.expired);
-      const headers: IncomingHttpHeaders = { 'x-immich-share-key': 'key' };
-      await expect(sut.validate(headers, {})).rejects.toBeInstanceOf(UnauthorizedException);
+      await expect(
+        sut.authenticate({
+          headers: { 'x-immich-share-key': 'key' },
+          queryParams: {},
+          metadata: { adminRoute: false, sharedLinkRoute: true, uri: 'test' },
+        }),
+      ).rejects.toBeInstanceOf(UnauthorizedException);
     });
 
     it('should not accept a key without a user', async () => {
       shareMock.getByKey.mockResolvedValue(sharedLinkStub.expired);
       userMock.get.mockResolvedValue(null);
-      const headers: IncomingHttpHeaders = { 'x-immich-share-key': 'key' };
-      await expect(sut.validate(headers, {})).rejects.toBeInstanceOf(UnauthorizedException);
+      await expect(
+        sut.authenticate({
+          headers: { 'x-immich-share-key': 'key' },
+          queryParams: {},
+          metadata: { adminRoute: false, sharedLinkRoute: true, uri: 'test' },
+        }),
+      ).rejects.toBeInstanceOf(UnauthorizedException);
     });
 
     it('should accept a base64url key', async () => {
       shareMock.getByKey.mockResolvedValue(sharedLinkStub.valid);
       userMock.get.mockResolvedValue(userStub.admin);
-      const headers: IncomingHttpHeaders = { 'x-immich-share-key': sharedLinkStub.valid.key.toString('base64url') };
-      await expect(sut.validate(headers, {})).resolves.toEqual({
+      await expect(
+        sut.authenticate({
+          headers: { 'x-immich-share-key': sharedLinkStub.valid.key.toString('base64url') },
+          queryParams: {},
+          metadata: { adminRoute: false, sharedLinkRoute: true, uri: 'test' },
+        }),
+      ).resolves.toEqual({
         user: userStub.admin,
         sharedLink: sharedLinkStub.valid,
       });
@@ -301,8 +330,13 @@ describe('AuthService', () => {
     it('should accept a hex key', async () => {
       shareMock.getByKey.mockResolvedValue(sharedLinkStub.valid);
       userMock.get.mockResolvedValue(userStub.admin);
-      const headers: IncomingHttpHeaders = { 'x-immich-share-key': sharedLinkStub.valid.key.toString('hex') };
-      await expect(sut.validate(headers, {})).resolves.toEqual({
+      await expect(
+        sut.authenticate({
+          headers: { 'x-immich-share-key': sharedLinkStub.valid.key.toString('hex') },
+          queryParams: {},
+          metadata: { adminRoute: false, sharedLinkRoute: true, uri: 'test' },
+        }),
+      ).resolves.toEqual({
         user: userStub.admin,
         sharedLink: sharedLinkStub.valid,
       });
@@ -313,24 +347,50 @@ describe('AuthService', () => {
   describe('validate - user token', () => {
     it('should throw if no token is found', async () => {
       sessionMock.getByToken.mockResolvedValue(null);
-      const headers: IncomingHttpHeaders = { 'x-immich-user-token': 'auth_token' };
-      await expect(sut.validate(headers, {})).rejects.toBeInstanceOf(UnauthorizedException);
+      await expect(
+        sut.authenticate({
+          headers: { 'x-immich-user-token': 'auth_token' },
+          queryParams: {},
+          metadata: { adminRoute: false, sharedLinkRoute: false, uri: 'test' },
+        }),
+      ).rejects.toBeInstanceOf(UnauthorizedException);
     });
 
     it('should return an auth dto', async () => {
       sessionMock.getByToken.mockResolvedValue(sessionStub.valid);
-      const headers: IncomingHttpHeaders = { cookie: 'immich_access_token=auth_token' };
-      await expect(sut.validate(headers, {})).resolves.toEqual({
+      await expect(
+        sut.authenticate({
+          headers: { cookie: 'immich_access_token=auth_token' },
+          queryParams: {},
+          metadata: { adminRoute: false, sharedLinkRoute: false, uri: 'test' },
+        }),
+      ).resolves.toEqual({
         user: userStub.user1,
         session: sessionStub.valid,
       });
     });
 
+    it('should throw if admin route and not an admin', async () => {
+      sessionMock.getByToken.mockResolvedValue(sessionStub.valid);
+      await expect(
+        sut.authenticate({
+          headers: { cookie: 'immich_access_token=auth_token' },
+          queryParams: {},
+          metadata: { adminRoute: true, sharedLinkRoute: false, uri: 'test' },
+        }),
+      ).rejects.toBeInstanceOf(ForbiddenException);
+    });
+
     it('should update when access time exceeds an hour', async () => {
       sessionMock.getByToken.mockResolvedValue(sessionStub.inactive);
       sessionMock.update.mockResolvedValue(sessionStub.valid);
-      const headers: IncomingHttpHeaders = { cookie: 'immich_access_token=auth_token' };
-      await expect(sut.validate(headers, {})).resolves.toBeDefined();
+      await expect(
+        sut.authenticate({
+          headers: { cookie: 'immich_access_token=auth_token' },
+          queryParams: {},
+          metadata: { adminRoute: false, sharedLinkRoute: false, uri: 'test' },
+        }),
+      ).resolves.toBeDefined();
       expect(sessionMock.update.mock.calls[0][0]).toMatchObject({ id: 'not_active', updatedAt: expect.any(Date) });
     });
   });
@@ -338,15 +398,25 @@ describe('AuthService', () => {
   describe('validate - api key', () => {
     it('should throw an error if no api key is found', async () => {
       keyMock.getKey.mockResolvedValue(null);
-      const headers: IncomingHttpHeaders = { 'x-api-key': 'auth_token' };
-      await expect(sut.validate(headers, {})).rejects.toBeInstanceOf(UnauthorizedException);
+      await expect(
+        sut.authenticate({
+          headers: { 'x-api-key': 'auth_token' },
+          queryParams: {},
+          metadata: { adminRoute: false, sharedLinkRoute: false, uri: 'test' },
+        }),
+      ).rejects.toBeInstanceOf(UnauthorizedException);
       expect(keyMock.getKey).toHaveBeenCalledWith('auth_token (hashed)');
     });
 
     it('should return an auth dto', async () => {
       keyMock.getKey.mockResolvedValue(keyStub.admin);
-      const headers: IncomingHttpHeaders = { 'x-api-key': 'auth_token' };
-      await expect(sut.validate(headers, {})).resolves.toEqual({ user: userStub.admin, apiKey: keyStub.admin });
+      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 });
       expect(keyMock.getKey).toHaveBeenCalledWith('auth_token (hashed)');
     });
   });

server/src/services/auth.service.ts

@@ -1,5 +1,6 @@
 import {
   BadRequestException,
+  ForbiddenException,
   Inject,
   Injectable,
   InternalServerErrorException,
@@ -19,6 +20,7 @@ import {
   ChangePasswordDto,
   ImmichCookie,
   ImmichHeader,
+  ImmichQuery,
   LoginCredentialDto,
   LogoutResponseDto,
   OAuthAuthorizeResponseDto,
@@ -53,6 +55,16 @@ interface ClaimOptions<T> {
   isValid: (value: unknown) => boolean;
 }
 
+export type ValidateRequest = {
+  headers: IncomingHttpHeaders;
+  queryParams: Record<string, string>;
+  metadata: {
+    sharedLinkRoute: boolean;
+    adminRoute: boolean;
+    uri: string;
+  };
+};
+
 @Injectable()
 export class AuthService {
   private configCore: SystemConfigCore;
@@ -143,14 +155,31 @@ export class AuthService {
     return mapUserAdmin(admin);
   }
 
-  async validate(headers: IncomingHttpHeaders, params: Record<string, string>): Promise<AuthDto> {
-    const shareKey = (headers[ImmichHeader.SHARED_LINK_KEY] || params.key) as string;
+  async authenticate({ headers, queryParams, metadata }: ValidateRequest): Promise<AuthDto> {
+    const authDto = await this.validate({ headers, queryParams });
+    const { adminRoute, sharedLinkRoute, uri } = metadata;
+
+    if (!authDto.user.isAdmin && adminRoute) {
+      this.logger.warn(`Denied access to admin only route: ${uri}`);
+      throw new ForbiddenException('Forbidden');
+    }
+
+    if (authDto.sharedLink && !sharedLinkRoute) {
+      this.logger.warn(`Denied access to non-shared route: ${uri}`);
+      throw new ForbiddenException('Forbidden');
+    }
+
+    return authDto;
+  }
+
+  private async validate({ headers, queryParams }: Omit<ValidateRequest, 'metadata'>): Promise<AuthDto> {
+    const shareKey = (headers[ImmichHeader.SHARED_LINK_KEY] || queryParams[ImmichQuery.SHARED_LINK_KEY]) as string;
     const session = (headers[ImmichHeader.USER_TOKEN] ||
       headers[ImmichHeader.SESSION_TOKEN] ||
-      params.sessionKey ||
+      queryParams[ImmichQuery.SESSION_KEY] ||
       this.getBearerToken(headers) ||
       this.getCookieToken(headers)) as string;
-    const apiKey = (headers[ImmichHeader.API_KEY] || params.apiKey) as string;
+    const apiKey = (headers[ImmichHeader.API_KEY] || queryParams[ImmichQuery.API_KEY]) as string;
 
     if (shareKey) {
       return this.validateSharedLink(shareKey);