feat: require @Authenticated decorator everywhere

server/src/controllers/app.controller.ts

@@ -1,5 +1,6 @@
 import { Controller, Get, Header } from '@nestjs/common';
 import { ApiExcludeEndpoint } from '@nestjs/swagger';
+import { Authenticated } from 'src/middleware/auth.guard';
 import { SystemConfigService } from 'src/services/system-config.service';
 
 @Controller()
@@ -8,6 +9,7 @@ export class AppController {
 
   @ApiExcludeEndpoint()
   @Get('.well-known/immich')
+  @Authenticated({ public: true })
   getImmichWellKnown() {
     return {
       api: {
@@ -18,6 +20,7 @@ export class AppController {
 
   @ApiExcludeEndpoint()
   @Get('custom.css')
+  @Authenticated({ public: true })
   @Header('Content-Type', 'text/css')
   getCustomCss() {
     return this.service.getCustomCss();

server/src/controllers/auth.controller.spec.ts

@@ -1,3 +1,4 @@
+import { BadRequestException } from '@nestjs/common';
 import { AuthController } from 'src/controllers/auth.controller';
 import { LoginResponseDto } from 'src/dtos/auth.dto';
 import { AuthService } from 'src/services/auth.service';
@@ -76,6 +77,18 @@ describe(AuthController.name, () => {
         .send({ name: 'admin', password: 'password', email: 'admin@local' });
       expect(status).toEqual(201);
     });
+
+    it('should not sign up an admin when setup is unavailable', async () => {
+      ctx.requireSetupAvailable.mockRejectedValue(new BadRequestException('Admin setup is not available'));
+
+      const { status, body } = await request(ctx.getHttpServer())
+        .post('/auth/admin-sign-up')
+        .send({ name, email, password });
+
+      expect(status).toEqual(400);
+      expect(body).toEqual(errorDto.badRequest('Admin setup is not available'));
+      expect(service.adminSignUp).not.toHaveBeenCalled();
+    });
   });
 
   describe('POST /auth/login', () => {

server/src/controllers/auth.controller.ts

@@ -33,6 +33,7 @@ export class AuthController {
     description: 'Login with username and password and receive a session token.',
     history: new HistoryBuilder().added('v1').beta('v1').stable('v2'),
   })
+  @Authenticated({ public: true })
   async login(
     @Res({ passthrough: true }) res: Response,
     @Body() loginCredential: LoginCredentialDto,
@@ -55,6 +56,7 @@ export class AuthController {
     description: 'Create the first admin user in the system.',
     history: new HistoryBuilder().added('v1').beta('v1').stable('v2'),
   })
+  @Authenticated({ public: true, setup: true })
   signUpAdmin(@Body() dto: SignUpDto): Promise<UserAdminResponseDto> {
     return this.service.adminSignUp(dto);
   }

server/src/controllers/database-backup.controller.spec.ts

@@ -0,0 +1,55 @@
+import { BadRequestException } from '@nestjs/common';
+import { DatabaseBackupController } from 'src/controllers/database-backup.controller';
+import { DatabaseBackupService } from 'src/services/database-backup.service';
+import { MaintenanceService } from 'src/services/maintenance.service';
+import request from 'supertest';
+import { errorDto } from 'test/medium/responses';
+import { automock, ControllerContext, controllerSetup, mockBaseService } from 'test/utils';
+
+describe(DatabaseBackupController.name, () => {
+  let ctx: ControllerContext;
+  const service = automock(DatabaseBackupService, { args: [{ setContext: () => {} }], strict: false });
+  const maintenanceService = mockBaseService(MaintenanceService);
+
+  beforeAll(async () => {
+    ctx = await controllerSetup(DatabaseBackupController, [
+      { provide: DatabaseBackupService, useValue: service },
+      { provide: MaintenanceService, useValue: maintenanceService },
+    ]);
+    return () => ctx.close();
+  });
+
+  beforeEach(() => {
+    service.resetAllMocks();
+    maintenanceService.resetAllMocks();
+    ctx.reset();
+  });
+
+  describe('GET /admin/database-backups', () => {
+    it('should be an authenticated route', async () => {
+      await request(ctx.getHttpServer()).get('/admin/database-backups').send();
+      expect(ctx.authenticate).toHaveBeenCalled();
+    });
+  });
+
+  describe('POST /admin/database-backups/start-restore', () => {
+    it('should not be an authenticated route', async () => {
+      maintenanceService.startRestoreFlow.mockResolvedValue({ jwt: 'jwt' });
+
+      await request(ctx.getHttpServer()).post('/admin/database-backups/start-restore').send();
+
+      expect(ctx.authenticate).not.toHaveBeenCalled();
+      expect(ctx.requireSetupAvailable).toHaveBeenCalled();
+    });
+
+    it('should not start a restore when setup is unavailable', async () => {
+      ctx.requireSetupAvailable.mockRejectedValue(new BadRequestException('Admin setup is not available'));
+
+      const { status, body } = await request(ctx.getHttpServer()).post('/admin/database-backups/start-restore').send();
+
+      expect(status).toEqual(400);
+      expect(body).toEqual(errorDto.badRequest('Admin setup is not available'));
+      expect(maintenanceService.startRestoreFlow).not.toHaveBeenCalled();
+    });
+  });
+});

server/src/controllers/database-backup.controller.ts

@@ -71,6 +71,7 @@ export class DatabaseBackupController {
     description: 'Put Immich into maintenance mode to restore a backup (Immich must not be configured)',
     history: new HistoryBuilder().added('v2.5.0').alpha('v2.5.0'),
   })
+  @Authenticated({ public: true, setup: true })
   async startDatabaseRestoreFlow(
     @GetLoginDetails() loginDetails: LoginDetails,
     @Res({ passthrough: true }) res: Response,

server/src/controllers/index.spec.ts

@@ -53,12 +53,10 @@ describe('controllers', () => {
     expect(new Set(reachableByNonAdmins)).toEqual(UNAUTHENTICATED_ADMIN_ROUTES);
   });
 
-  it('should not authenticate the bootstrap routes under admin/', () => {
-    const authenticated = routes
-      .filter((route) => UNAUTHENTICATED_ADMIN_ROUTES.has(route.id) && route.auth !== undefined)
-      .map((route) => route.label);
+  it('should declare authentication on every route', () => {
+    const undeclared = routes.filter((route) => route.auth === undefined).map((route) => route.label);
 
-    expect(authenticated).toEqual([]);
+    expect(undeclared).toEqual([]);
   });
 
   it('should require admin access for routes with an admin permission', () => {

server/src/controllers/maintenance.controller.ts

@@ -27,6 +27,7 @@ export class MaintenanceController {
     description: 'Fetch information about the currently running maintenance action.',
     history: new HistoryBuilder().added('v2.5.0').alpha('v2.5.0'),
   })
+  @Authenticated({ public: true })
   getMaintenanceStatus(): MaintenanceStatusResponseDto {
     return this.service.getMaintenanceStatus();
   }
@@ -48,6 +49,7 @@ export class MaintenanceController {
     description: 'Login with maintenance token or cookie to receive current information and perform further actions.',
     history: new HistoryBuilder().added('v2.3.0').alpha('v2.3.0'),
   })
+  @Authenticated({ public: true })
   maintenanceLogin(@Body() _dto: MaintenanceLoginDto): MaintenanceAuthDto {
     throw new BadRequestException('Not in maintenance mode');
   }

server/src/controllers/oauth.controller.ts

@@ -22,6 +22,7 @@ export class OAuthController {
   constructor(private service: AuthService) {}
 
   @Get('mobile-redirect')
+  @Authenticated({ public: true })
   @Redirect()
   @Endpoint({
     summary: 'Redirect OAuth to mobile',
@@ -37,6 +38,7 @@ export class OAuthController {
   }
 
   @Post('authorize')
+  @Authenticated({ public: true })
   @Endpoint({
     summary: 'Start OAuth',
     description: 'Initiate the OAuth authorization process.',
@@ -62,6 +64,7 @@ export class OAuthController {
   }
 
   @Post('callback')
+  @Authenticated({ public: true })
   @Endpoint({
     summary: 'Finish OAuth',
     description: 'Complete the OAuth authorization process by exchanging the authorization code for a session token.',
@@ -115,6 +118,7 @@ export class OAuthController {
   }
 
   @Post('backchannel-logout')
+  @Authenticated({ public: true })
   @HttpCode(HttpStatus.OK)
   @ApiConsumes('application/x-www-form-urlencoded')
   @Endpoint({

server/src/controllers/server.controller.ts

@@ -64,6 +64,7 @@ export class ServerController {
   }
 
   @Get('ping')
+  @Authenticated({ public: true })
   @Endpoint({
     summary: 'Ping',
     description: 'Pong',
@@ -74,6 +75,7 @@ export class ServerController {
   }
 
   @Get('version')
+  @Authenticated({ public: true })
   @Endpoint({
     summary: 'Get server version',
     description: 'Retrieve the current server version in semantic versioning (semver) format.',
@@ -84,6 +86,7 @@ export class ServerController {
   }
 
   @Get('version-history')
+  @Authenticated({ public: true })
   @Endpoint({
     summary: 'Get version history',
     description: 'Retrieve a list of past versions the server has been on.',
@@ -94,6 +97,7 @@ export class ServerController {
   }
 
   @Get('features')
+  @Authenticated({ public: true })
   @Endpoint({
     summary: 'Get features',
     description: 'Retrieve available features supported by this server.',
@@ -104,6 +108,7 @@ export class ServerController {
   }
 
   @Get('config')
+  @Authenticated({ public: true })
   @Endpoint({
     summary: 'Get config',
     description: 'Retrieve the current server configuration.',
@@ -125,6 +130,7 @@ export class ServerController {
   }
 
   @Get('media-types')
+  @Authenticated({ public: true })
   @Endpoint({
     summary: 'Get supported media types',
     description: 'Retrieve all media types supported by the server.',

server/src/middleware/auth.guard.spec.ts

@@ -0,0 +1,82 @@
+import { ExecutionContext } from '@nestjs/common';
+import { Reflector } from '@nestjs/core';
+import { Authenticated, AuthGuard } from 'src/middleware/auth.guard';
+import { LoggingRepository } from 'src/repositories/logging.repository';
+import { AuthService } from 'src/services/auth.service';
+import { mockEnvData } from 'test/repositories/config.repository.mock';
+import { newTestService, ServiceMocks } from 'test/utils';
+
+class TestController {
+  @Authenticated({ public: true, setup: true })
+  setupRoute() {}
+
+  @Authenticated({ public: true })
+  publicRoute() {}
+
+  undecoratedRoute() {}
+}
+
+describe(AuthGuard.name, () => {
+  let sut: AuthGuard;
+  let authService: AuthService;
+  let mocks: ServiceMocks;
+
+  const contextFor = (handler: () => void) =>
+    ({
+      getHandler: () => handler,
+      switchToHttp: () => ({ getRequest: () => ({ headers: {}, query: {}, path: '/' }) }),
+    }) as unknown as ExecutionContext;
+
+  beforeEach(() => {
+    ({ sut: authService, mocks } = newTestService(AuthService));
+    sut = new AuthGuard(mocks.logger as unknown as LoggingRepository, new Reflector(), authService);
+  });
+
+  describe('setup routes', () => {
+    it('should allow access while the server is awaiting its first admin', async () => {
+      mocks.user.hasAdmin.mockResolvedValue(false);
+      const authenticate = vitest.spyOn(authService, 'authenticate');
+
+      await expect(sut.canActivate(contextFor(TestController.prototype.setupRoute))).resolves.toBe(true);
+
+      expect(authenticate).not.toHaveBeenCalled();
+    });
+
+    it('should reject when setup is disabled', async () => {
+      mocks.config.getEnv.mockReturnValue(mockEnvData({ setup: { allow: false } }));
+      mocks.user.hasAdmin.mockResolvedValue(false);
+
+      await expect(sut.canActivate(contextFor(TestController.prototype.setupRoute))).rejects.toThrowError(
+        'Admin setup is not available',
+      );
+    });
+
+    it('should reject when the server already has an admin', async () => {
+      mocks.user.hasAdmin.mockResolvedValue(true);
+
+      await expect(sut.canActivate(contextFor(TestController.prototype.setupRoute))).rejects.toThrowError(
+        'Admin setup is not available',
+      );
+    });
+  });
+
+  describe('public routes', () => {
+    it('should not require setup availability', async () => {
+      mocks.user.hasAdmin.mockResolvedValue(true);
+
+      await expect(sut.canActivate(contextFor(TestController.prototype.publicRoute))).resolves.toBe(true);
+    });
+  });
+
+  describe('undecorated routes', () => {
+    it('should be rejected', async () => {
+      const authenticate = vitest.spyOn(authService, 'authenticate');
+
+      await expect(sut.canActivate(contextFor(TestController.prototype.undecoratedRoute))).rejects.toThrowError(
+        'does not declare @Authenticated()',
+      );
+
+      expect(authenticate).not.toHaveBeenCalled();
+    });
+  });
+});

server/src/middleware/auth.guard.ts

@@ -17,23 +17,26 @@ import { getUserAgentDetails } from 'src/utils/request';
 
 type AdminRoute = { admin?: true };
 type SharedLinkRoute = { sharedLink?: true };
-export type AuthenticatedOptions = { permission?: Permission | false } & (AdminRoute | SharedLinkRoute);
+type AuthorizedRoute = { permission?: Permission | false; public?: never; setup?: never } & (
+  AdminRoute | SharedLinkRoute
+);
+type PublicRoute = { public: true; setup?: true; permission?: never; admin?: never; sharedLink?: never };
+export type AuthenticatedOptions = AuthorizedRoute | PublicRoute;
 
 type ReflectorTarget = Parameters<Reflector['get']>[1];
 
 /** Resolves the `@Authenticated()` options of a route handler, with the defaults applied. */
 export const getAuthenticatedOptions = (reflector: Reflector, target: ReflectorTarget) => {
   const options = reflector.getAllAndOverride<AuthenticatedOptions | undefined>(MetadataKey.AuthRoute, [target]);
-  return options && { sharedLink: false, admin: false, ...options };
+  return options && { sharedLink: false, admin: false, public: false, setup: false, ...options };
 };
 
 export const Authenticated = (options: AuthenticatedOptions = {}): MethodDecorator => {
-  const decorators: MethodDecorator[] = [
-    ApiBearerAuth(),
-    ApiCookieAuth(),
-    ApiSecurity(MetadataKey.ApiKeySecurity),
-    SetMetadata(MetadataKey.AuthRoute, options),
-  ];
+  const decorators: MethodDecorator[] = [SetMetadata(MetadataKey.AuthRoute, options)];
+
+  if (!options.public) {
+    decorators.push(ApiBearerAuth(), ApiCookieAuth(), ApiSecurity(MetadataKey.ApiKeySecurity));
+  }
 
   if ((options as AdminRoute).admin) {
     decorators.push(ApiExtension(ApiCustomExtension.AdminOnly, true));
@@ -96,6 +99,14 @@ export class AuthGuard implements CanActivate {
   async canActivate(context: ExecutionContext): Promise<boolean> {
     const options = getAuthenticatedOptions(this.reflector, context.getHandler());
     if (!options) {
+      throw new Error(`Route ${context.getHandler().name} does not declare @Authenticated()`);
+    }
+
+    if (options.setup) {
+      await this.authService.requireSetupAvailable();
+    }
+
+    if (options.public) {
       return true;
     }
 

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

@@ -14,7 +14,6 @@ import { UserFactory } from 'test/factories/user.factory';
 import { sharedLinkStub } from 'test/fixtures/shared-link.stub';
 import { systemConfigStub } from 'test/fixtures/system-config.stub';
 import { userStub } from 'test/fixtures/user.stub';
-import { mockEnvData } from 'test/repositories/config.repository.mock';
 import { newUuid } from 'test/small.factory';
 import { newTestService, ServiceMocks } from 'test/utils';
 
@@ -308,26 +307,7 @@ describe(AuthService.name, () => {
   describe('adminSignUp', () => {
     const dto: SignUpDto = { email: 'test@immich.com', password: 'password', name: 'immich admin' };
 
-    it('should only allow one admin', async () => {
-      mocks.user.hasAdmin.mockResolvedValue(true);
-
-      await expect(sut.adminSignUp(dto)).rejects.toBeInstanceOf(BadRequestException);
-
-      expect(mocks.user.hasAdmin).toHaveBeenCalled();
-      expect(mocks.user.create).not.toHaveBeenCalled();
-    });
-
-    it('should not allow signup when setup is disabled', async () => {
-      mocks.config.getEnv.mockReturnValue(mockEnvData({ setup: { allow: false } }));
-      mocks.user.hasAdmin.mockResolvedValue(false);
-
-      await expect(sut.adminSignUp(dto)).rejects.toBeInstanceOf(BadRequestException);
-
-      expect(mocks.user.create).not.toHaveBeenCalled();
-    });
-
     it('should sign up the admin', async () => {
-      mocks.user.hasAdmin.mockResolvedValue(false);
       mocks.user.create.mockResolvedValue({
         ...userStub.admin,
         ...dto,
@@ -345,7 +325,6 @@ describe(AuthService.name, () => {
         name: 'immich admin',
       });
 
-      expect(mocks.user.hasAdmin).toHaveBeenCalled();
       expect(mocks.user.create).toHaveBeenCalled();
     });
   });

server/src/services/auth.service.ts

@@ -197,8 +197,6 @@ export class AuthService extends BaseService {
   }
 
   async adminSignUp(dto: SignUpDto): Promise<UserAdminResponseDto> {
-    await this.requireSetupAvailable();
-
     const admin = await this.createUser({
       isAdmin: true,
       email: dto.email,

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

@@ -1,6 +1,5 @@
 import { MaintenanceAction, SystemMetadataKey } from 'src/enum';
 import { MaintenanceService } from 'src/services/maintenance.service';
-import { mockEnvData } from 'test/repositories/config.repository.mock';
 import { newTestService, ServiceMocks } from 'test/utils';
 
 describe(MaintenanceService.name, () => {
@@ -136,27 +135,7 @@ describe(MaintenanceService.name, () => {
   });
 
   describe('startRestoreFlow', () => {
-    it('should fail if setup is disabled', async () => {
-      mocks.config.getEnv.mockReturnValue(mockEnvData({ setup: { allow: false } }));
-      mocks.user.hasAdmin.mockResolvedValue(false);
-
-      await expect(sut.startRestoreFlow()).rejects.toThrowError('Admin setup is not available');
-
-      expect(mocks.systemMetadata.set).not.toHaveBeenCalled();
-      expect(mocks.event.emit).not.toHaveBeenCalled();
-    });
-
-    it('should fail if the server already has an admin', async () => {
-      mocks.user.hasAdmin.mockResolvedValue(true);
-
-      await expect(sut.startRestoreFlow()).rejects.toThrowError('Admin setup is not available');
-
-      expect(mocks.systemMetadata.set).not.toHaveBeenCalled();
-      expect(mocks.event.emit).not.toHaveBeenCalled();
-    });
-
     it('should start maintenance mode and return a jwt', async () => {
-      mocks.user.hasAdmin.mockResolvedValue(false);
       mocks.systemMetadata.get.mockResolvedValue({ isMaintenanceMode: false });
 
       await expect(sut.startRestoreFlow()).resolves.toMatchObject({ jwt: expect.any(String) });

server/src/services/maintenance.service.ts

@@ -59,8 +59,6 @@ export class MaintenanceService extends BaseService {
   }
 
   async startRestoreFlow(): Promise<{ jwt: string }> {
-    await this.requireSetupAvailable();
-
     return this.startMaintenance(
       {
         action: MaintenanceAction.SelectDatabaseRestore,

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

@@ -1,5 +1,6 @@
 import { SystemMetadataKey } from 'src/enum';
 import { ServerService } from 'src/services/server.service';
+import { mockEnvData } from 'test/repositories/config.repository.mock';
 import { newTestService, ServiceMocks } from 'test/utils';
 
 describe(ServerService.name, () => {
@@ -172,6 +173,19 @@ describe(ServerService.name, () => {
       });
       expect(mocks.systemMetadata.get).toHaveBeenCalled();
     });
+
+    it('should be initialized once an admin exists', async () => {
+      mocks.user.hasAdmin.mockResolvedValue(true);
+
+      await expect(sut.getSystemConfig()).resolves.toMatchObject({ isInitialized: true });
+    });
+
+    it('should be initialized when setup is disabled', async () => {
+      mocks.config.getEnv.mockReturnValue(mockEnvData({ setup: { allow: false } }));
+      mocks.user.hasAdmin.mockResolvedValue(false);
+
+      await expect(sut.getSystemConfig()).resolves.toMatchObject({ isInitialized: true });
+    });
   });
 
   describe('getStats', () => {

server/test/medium/specs/services/auth.service.spec.ts

@@ -16,7 +16,6 @@ import { UserRepository } from 'src/repositories/user.repository';
 import { DB } from 'src/schema';
 import { AuthService } from 'src/services/auth.service';
 import { mediumFactory, newMediumService } from 'test/medium.factory';
-import { mockEnvData } from 'test/repositories/config.repository.mock';
 import { factory } from 'test/small.factory';
 import { getKyselyDB } from 'test/utils';
 
@@ -58,29 +57,6 @@ describe(AuthService.name, () => {
         }),
       );
     });
-
-    it('should not allow a second admin to sign up', async () => {
-      const { sut, ctx } = setup();
-      await ctx.newUser({ isAdmin: true });
-      const dto = { name: 'Admin', email: 'admin@immich.cloud', password: 'password' };
-
-      const response = sut.adminSignUp(dto);
-      await expect(response).rejects.toThrow(BadRequestException);
-      await expect(response).rejects.toThrow('Admin setup is not available');
-    });
-
-    it('should not allow signup when setup is disabled', async () => {
-      // a database without an admin, so that the env var is the only thing rejecting the signup
-      const { sut, ctx } = setup(await getKyselyDB());
-      vi.spyOn(ctx.get(ConfigRepository), 'getEnv').mockReturnValue(mockEnvData({ setup: { allow: false } }));
-      const dto = { name: 'Admin', email: 'admin@immich.cloud', password: 'password' };
-
-      const response = sut.adminSignUp(dto);
-      await expect(response).rejects.toThrow(BadRequestException);
-      await expect(response).rejects.toThrow('Admin setup is not available');
-
-      await expect(ctx.get(UserRepository).hasAdmin()).resolves.toBe(false);
-    });
   });
 
   describe('login', () => {

server/test/utils.ts

@@ -89,6 +89,7 @@ import { assert, Mock, Mocked, vitest } from 'vitest';
 
 export type ControllerContext = {
   authenticate: Mock;
+  requireSetupAvailable: Mock;
   getHttpServer: () => any;
   reset: () => void;
   close: () => Promise<void>;
@@ -124,7 +125,7 @@ export const controllerSetup = async (controller: new (...args: any[]) => unknow
       { provide: APP_GUARD, useClass: AuthGuard },
       { provide: LoggingRepository, useValue: LoggingRepository.create() },
       { provide: ClsService, useValue: { getId: vi.fn() } },
-      { provide: AuthService, useValue: { authenticate: vi.fn() } },
+      { provide: AuthService, useValue: { authenticate: vi.fn(), requireSetupAvailable: vi.fn() } },
       ...providers,
     ],
   })
@@ -137,13 +138,17 @@ export const controllerSetup = async (controller: new (...args: any[]) => unknow
   await app.init();
 
   // allow the AuthController to override the AuthService itself
-  const authenticate = app.get<Mocked<AuthService>>(AuthService).authenticate as Mock;
+  const resolvedAuthService = app.get<Mocked<AuthService>>(AuthService);
+  const authenticate = resolvedAuthService.authenticate as Mock;
+  const requireSetupAvailable = resolvedAuthService.requireSetupAvailable as Mock;
 
   return {
     authenticate,
+    requireSetupAvailable,
     getHttpServer: () => app.getHttpServer(),
     reset: () => {
       authenticate.mockReset();
+      requireSetupAvailable.mockReset();
     },
     close: async () => {
       await app.close();
@@ -184,10 +189,12 @@ export const automock = <T>(
   const mocks: Mock[] = [];
 
   const instance = new Dependency(...args);
-  const propertyNames = new Set([
-    ...Object.getOwnPropertyNames(Dependency.prototype),
-    ...Object.getOwnPropertyNames(instance),
-  ]);
+  const propertyNames = new Set(Object.getOwnPropertyNames(instance));
+  for (let proto = Dependency.prototype; proto && proto !== Object.prototype; proto = Object.getPrototypeOf(proto)) {
+    for (const property of Object.getOwnPropertyNames(proto)) {
+      propertyNames.add(property);
+    }
+  }
   for (const property of propertyNames) {
     if (property === 'constructor') {
       continue;