feat(server,web): migrate oauth settings from env to system config (#1061)

server/apps/immich/src/api-v1/oauth/oauth.module.ts

@@ -1,3 +1,4 @@
+import { ImmichConfigModule } from '@app/immich-config';
 import { Module } from '@nestjs/common';
 import { ImmichJwtModule } from '../../modules/immich-jwt/immich-jwt.module';
 import { UserModule } from '../user/user.module';
@@ -5,7 +6,7 @@ import { OAuthController } from './oauth.controller';
 import { OAuthService } from './oauth.service';
 
 @Module({
-  imports: [UserModule, ImmichJwtModule],
+  imports: [UserModule, ImmichJwtModule, ImmichConfigModule],
   controllers: [OAuthController],
   providers: [OAuthService],
   exports: [OAuthService],

server/apps/immich/src/api-v1/oauth/oauth.service.spec.ts

@@ -1,24 +1,13 @@
+import { SystemConfig } from '@app/database/entities/system-config.entity';
 import { UserEntity } from '@app/database/entities/user.entity';
+import { ImmichConfigService } from '@app/immich-config';
 import { BadRequestException } from '@nestjs/common';
-import { ConfigService } from '@nestjs/config';
 import { generators, Issuer } from 'openid-client';
 import { ImmichJwtService } from '../../modules/immich-jwt/immich-jwt.service';
 import { LoginResponseDto } from '../auth/response-dto/login-response.dto';
 import { OAuthService } from '../oauth/oauth.service';
 import { IUserRepository } from '../user/user-repository';
 
-interface OAuthConfig {
-  OAUTH_ENABLED: boolean;
-  OAUTH_AUTO_REGISTER: boolean;
-  OAUTH_ISSUER_URL: string;
-  OAUTH_SCOPE: string;
-  OAUTH_BUTTON_TEXT: string;
-}
-
-const mockConfig = (config: Partial<OAuthConfig>) => {
-  return (value: keyof OAuthConfig, defaultValue: any) => config[value] ?? defaultValue ?? null;
-};
-
 const email = 'user@immich.com';
 const sub = 'my-auth-user-sub';
 
@@ -39,7 +28,7 @@ const loginResponse = {
 describe('OAuthService', () => {
   let sut: OAuthService;
   let userRepositoryMock: jest.Mocked<IUserRepository>;
-  let configServiceMock: jest.Mocked<ConfigService>;
+  let immichConfigServiceMock: jest.Mocked<ImmichConfigService>;
   let immichJwtServiceMock: jest.Mocked<ImmichJwtService>;
 
   beforeEach(async () => {
@@ -80,11 +69,11 @@ describe('OAuthService', () => {
       extractJwtFromCookie: jest.fn(),
     } as unknown as jest.Mocked<ImmichJwtService>;
 
-    configServiceMock = {
-      get: jest.fn(),
-    } as unknown as jest.Mocked<ConfigService>;
+    immichConfigServiceMock = {
+      getConfig: jest.fn().mockResolvedValue({ oauth: { enabled: false } }),
+    } as unknown as jest.Mocked<ImmichConfigService>;
 
-    sut = new OAuthService(immichJwtServiceMock, configServiceMock, userRepositoryMock);
+    sut = new OAuthService(immichJwtServiceMock, immichConfigServiceMock, userRepositoryMock);
   });
 
   it('should be defined', () => {
@@ -94,17 +83,17 @@ describe('OAuthService', () => {
   describe('generateConfig', () => {
     it('should work when oauth is not configured', async () => {
       await expect(sut.generateConfig({ redirectUri: 'http://callback' })).resolves.toEqual({ enabled: false });
-      expect(configServiceMock.get).toHaveBeenCalled();
+      expect(immichConfigServiceMock.getConfig).toHaveBeenCalled();
     });
 
     it('should generate the config', async () => {
-      configServiceMock.get.mockImplementation(
-        mockConfig({
-          OAUTH_ENABLED: true,
-          OAUTH_BUTTON_TEXT: 'OAuth',
-        }),
-      );
-      sut = new OAuthService(immichJwtServiceMock, configServiceMock, userRepositoryMock);
+      immichConfigServiceMock.getConfig.mockResolvedValue({
+        oauth: {
+          enabled: true,
+          buttonText: 'OAuth',
+        },
+      } as SystemConfig);
+      sut = new OAuthService(immichJwtServiceMock, immichConfigServiceMock, userRepositoryMock);
       await expect(sut.generateConfig({ redirectUri: 'http://redirect' })).resolves.toEqual({
         enabled: true,
         buttonText: 'OAuth',
@@ -119,13 +108,13 @@ describe('OAuthService', () => {
     });
 
     it('should not allow auto registering', async () => {
-      configServiceMock.get.mockImplementation(
-        mockConfig({
-          OAUTH_ENABLED: true,
-          OAUTH_AUTO_REGISTER: false,
-        }),
-      );
-      sut = new OAuthService(immichJwtServiceMock, configServiceMock, userRepositoryMock);
+      immichConfigServiceMock.getConfig.mockResolvedValue({
+        oauth: {
+          enabled: true,
+          autoRegister: false,
+        },
+      } as SystemConfig);
+      sut = new OAuthService(immichJwtServiceMock, immichConfigServiceMock, userRepositoryMock);
       jest.spyOn(sut['logger'], 'debug').mockImplementation(() => null);
       jest.spyOn(sut['logger'], 'warn').mockImplementation(() => null);
       userRepositoryMock.getByEmail.mockResolvedValue(null);
@@ -136,13 +125,13 @@ describe('OAuthService', () => {
     });
 
     it('should link an existing user', async () => {
-      configServiceMock.get.mockImplementation(
-        mockConfig({
-          OAUTH_ENABLED: true,
-          OAUTH_AUTO_REGISTER: false,
-        }),
-      );
-      sut = new OAuthService(immichJwtServiceMock, configServiceMock, userRepositoryMock);
+      immichConfigServiceMock.getConfig.mockResolvedValue({
+        oauth: {
+          enabled: true,
+          autoRegister: false,
+        },
+      } as SystemConfig);
+      sut = new OAuthService(immichJwtServiceMock, immichConfigServiceMock, userRepositoryMock);
       jest.spyOn(sut['logger'], 'debug').mockImplementation(() => null);
       jest.spyOn(sut['logger'], 'warn').mockImplementation(() => null);
       userRepositoryMock.getByEmail.mockResolvedValue(user);
@@ -156,8 +145,13 @@ describe('OAuthService', () => {
     });
 
     it('should allow auto registering by default', async () => {
-      configServiceMock.get.mockImplementation(mockConfig({ OAUTH_ENABLED: true }));
-      sut = new OAuthService(immichJwtServiceMock, configServiceMock, userRepositoryMock);
+      immichConfigServiceMock.getConfig.mockResolvedValue({
+        oauth: {
+          enabled: true,
+          autoRegister: true,
+        },
+      } as SystemConfig);
+      sut = new OAuthService(immichJwtServiceMock, immichConfigServiceMock, userRepositoryMock);
       jest.spyOn(sut['logger'], 'debug').mockImplementation(() => null);
       jest.spyOn(sut['logger'], 'log').mockImplementation(() => null);
       userRepositoryMock.getByEmail.mockResolvedValue(null);
@@ -178,13 +172,13 @@ describe('OAuthService', () => {
     });
 
     it('should get the session endpoint from the discovery document', async () => {
-      configServiceMock.get.mockImplementation(
-        mockConfig({
-          OAUTH_ENABLED: true,
-          OAUTH_ISSUER_URL: 'http://issuer',
-        }),
-      );
-      sut = new OAuthService(immichJwtServiceMock, configServiceMock, userRepositoryMock);
+      immichConfigServiceMock.getConfig.mockResolvedValue({
+        oauth: {
+          enabled: true,
+          issuerUrl: 'http://issuer,',
+        },
+      } as SystemConfig);
+      sut = new OAuthService(immichJwtServiceMock, immichConfigServiceMock, userRepositoryMock);
 
       await expect(sut.getLogoutEndpoint()).resolves.toBe('http://end-session-endpoint');
     });

server/apps/immich/src/api-v1/oauth/oauth.service.ts

@@ -1,5 +1,5 @@
+import { ImmichConfigService } from '@app/immich-config';
 import { BadRequestException, Inject, Injectable, Logger } from '@nestjs/common';
-import { ConfigService } from '@nestjs/config';
 import { ClientMetadata, generators, Issuer, UserinfoResponse } from 'openid-client';
 import { ImmichJwtService } from '../../modules/immich-jwt/immich-jwt.service';
 import { LoginResponseDto } from '../auth/response-dto/login-response.dto';
@@ -16,43 +16,26 @@ type OAuthProfile = UserinfoResponse & {
 export class OAuthService {
   private readonly logger = new Logger(OAuthService.name);
 
-  private readonly enabled: boolean;
-  private readonly autoRegister: boolean;
-  private readonly buttonText: string;
-  private readonly issuerUrl: string;
-  private readonly clientMetadata: ClientMetadata;
-  private readonly scope: string;
-
   constructor(
     private immichJwtService: ImmichJwtService,
-    configService: ConfigService,
+    private immichConfigService: ImmichConfigService,
     @Inject(USER_REPOSITORY) private userRepository: IUserRepository,
-  ) {
-    this.enabled = configService.get('OAUTH_ENABLED', false);
-    this.autoRegister = configService.get('OAUTH_AUTO_REGISTER', true);
-    this.issuerUrl = configService.get<string>('OAUTH_ISSUER_URL', '');
-    this.scope = configService.get<string>('OAUTH_SCOPE', '');
-    this.buttonText = configService.get<string>('OAUTH_BUTTON_TEXT', '');
-
-    this.clientMetadata = {
-      // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-      client_id: configService.get('OAUTH_CLIENT_ID')!,
-      client_secret: configService.get('OAUTH_CLIENT_SECRET'),
-      response_types: ['code'],
-    };
-  }
+  ) {}
 
   public async generateConfig(dto: OAuthConfigDto): Promise<OAuthConfigResponseDto> {
-    if (!this.enabled) {
+    const config = await this.immichConfigService.getConfig();
+    const { enabled, scope, buttonText } = config.oauth;
+
+    if (!enabled) {
       return { enabled: false };
     }
 
     const url = (await this.getClient()).authorizationUrl({
       redirect_uri: dto.redirectUri,
-      scope: this.scope,
+      scope,
       state: generators.state(),
     });
-    return { enabled: true, buttonText: this.buttonText, url };
+    return { enabled: true, buttonText, url };
   }
 
   public async callback(dto: OAuthCallbackDto): Promise<LoginResponseDto> {
@@ -75,9 +58,11 @@ export class OAuthService {
 
     // register new user
     if (!user) {
-      if (!this.autoRegister) {
+      const config = await this.immichConfigService.getConfig();
+      const { autoRegister } = config.oauth;
+      if (!autoRegister) {
         this.logger.warn(
-          `Unable to register ${profile.email}. To enable auto registering, set OAUTH_AUTO_REGISTER=true.`,
+          `Unable to register ${profile.email}. To enable set OAuth Auto Register to true in admin settings.`,
         );
         throw new BadRequestException(`User does not exist and auto registering is disabled.`);
       }
@@ -95,20 +80,31 @@ export class OAuthService {
   }
 
   public async getLogoutEndpoint(): Promise<string | null> {
-    if (!this.enabled) {
+    const config = await this.immichConfigService.getConfig();
+    const { enabled } = config.oauth;
+
+    if (!enabled) {
       return null;
     }
     return (await this.getClient()).issuer.metadata.end_session_endpoint || null;
   }
 
   private async getClient() {
-    if (!this.enabled) {
+    const config = await this.immichConfigService.getConfig();
+    const { enabled, clientId, clientSecret, issuerUrl } = config.oauth;
+
+    if (!enabled) {
       throw new BadRequestException('OAuth2 is not enabled');
     }
 
-    const issuer = await Issuer.discover(this.issuerUrl);
+    const metadata: ClientMetadata = {
+      client_id: clientId,
+      client_secret: clientSecret,
+      response_types: ['code'],
+    };
+
+    const issuer = await Issuer.discover(issuerUrl);
     const algorithms = (issuer.id_token_signing_alg_values_supported || []) as string[];
-    const metadata = { ...this.clientMetadata };
     if (algorithms[0] === 'HS256') {
       metadata.id_token_signed_response_alg = algorithms[0];
     }

server/apps/immich/src/api-v1/system-config/dto/system-config-ffmpeg.dto.ts

@@ -0,0 +1,18 @@
+import { IsString } from 'class-validator';
+
+export class SystemConfigFFmpegDto {
+  @IsString()
+  crf!: string;
+
+  @IsString()
+  preset!: string;
+
+  @IsString()
+  targetVideoCodec!: string;
+
+  @IsString()
+  targetAudioCodec!: string;
+
+  @IsString()
+  targetScaling!: string;
+}

server/apps/immich/src/api-v1/system-config/dto/system-config-oauth.dto.ts

@@ -0,0 +1,32 @@
+import { IsBoolean, IsNotEmpty, IsString, ValidateIf } from 'class-validator';
+
+const isEnabled = (config: SystemConfigOAuthDto) => config.enabled;
+
+export class SystemConfigOAuthDto {
+  @IsBoolean()
+  enabled!: boolean;
+
+  @ValidateIf(isEnabled)
+  @IsNotEmpty()
+  @IsString()
+  issuerUrl!: string;
+
+  @ValidateIf(isEnabled)
+  @IsNotEmpty()
+  @IsString()
+  clientId!: string;
+
+  @ValidateIf(isEnabled)
+  @IsNotEmpty()
+  @IsString()
+  clientSecret!: string;
+
+  @IsString()
+  scope!: string;
+
+  @IsString()
+  buttonText!: string;
+
+  @IsBoolean()
+  autoRegister!: boolean;
+}

server/apps/immich/src/api-v1/system-config/dto/system-config.dto.ts

@@ -0,0 +1,16 @@
+import { SystemConfig } from '@app/database/entities/system-config.entity';
+import { ValidateNested } from 'class-validator';
+import { SystemConfigFFmpegDto } from './system-config-ffmpeg.dto';
+import { SystemConfigOAuthDto } from './system-config-oauth.dto';
+
+export class SystemConfigDto {
+  @ValidateNested()
+  ffmpeg!: SystemConfigFFmpegDto;
+
+  @ValidateNested()
+  oauth!: SystemConfigOAuthDto;
+}
+
+export function mapConfig(config: SystemConfig): SystemConfigDto {
+  return config;
+}

server/apps/immich/src/api-v1/system-config/dto/update-system-config.ts

@@ -1,20 +0,0 @@
-import { SystemConfigKey, SystemConfigValue } from '@app/database/entities/system-config.entity';
-import { ApiProperty } from '@nestjs/swagger';
-import { IsEnum, IsNotEmpty, ValidateNested } from 'class-validator';
-
-export class UpdateSystemConfigDto {
-  @IsNotEmpty()
-  @ValidateNested({ each: true })
-  config!: SystemConfigItem[];
-}
-
-export class SystemConfigItem {
-  @IsNotEmpty()
-  @IsEnum(SystemConfigKey)
-  @ApiProperty({
-    enum: SystemConfigKey,
-    enumName: 'SystemConfigKey',
-  })
-  key!: SystemConfigKey;
-  value!: SystemConfigValue;
-}

server/apps/immich/src/api-v1/system-config/response-dto/system-config-response.dto.ts

@@ -1,20 +0,0 @@
-import { SystemConfigKey, SystemConfigValue } from '@app/database/entities/system-config.entity';
-import { ApiProperty } from '@nestjs/swagger';
-
-export class SystemConfigResponseDto {
-  config!: SystemConfigResponseItem[];
-}
-
-export class SystemConfigResponseItem {
-  @ApiProperty({ type: 'string' })
-  name!: string;
-
-  @ApiProperty({ enumName: 'SystemConfigKey', enum: SystemConfigKey })
-  key!: SystemConfigKey;
-
-  @ApiProperty({ type: 'string' })
-  value!: SystemConfigValue;
-
-  @ApiProperty({ type: 'string' })
-  defaultValue!: SystemConfigValue;
-}

server/apps/immich/src/api-v1/system-config/system-config.controller.ts

@@ -1,8 +1,7 @@
 import { Body, Controller, Get, Put, ValidationPipe } from '@nestjs/common';
 import { ApiBearerAuth, ApiTags } from '@nestjs/swagger';
 import { Authenticated } from '../../decorators/authenticated.decorator';
-import { UpdateSystemConfigDto } from './dto/update-system-config';
-import { SystemConfigResponseDto } from './response-dto/system-config-response.dto';
+import { SystemConfigDto } from './dto/system-config.dto';
 import { SystemConfigService } from './system-config.service';
 
 @ApiTags('System Config')
@@ -13,12 +12,17 @@ export class SystemConfigController {
   constructor(private readonly systemConfigService: SystemConfigService) {}
 
   @Get()
-  getConfig(): Promise<SystemConfigResponseDto> {
+  public getConfig(): Promise<SystemConfigDto> {
     return this.systemConfigService.getConfig();
   }
 
+  @Get('defaults')
+  public getDefaults(): SystemConfigDto {
+    return this.systemConfigService.getDefaults();
+  }
+
   @Put()
-  async updateConfig(@Body(ValidationPipe) dto: UpdateSystemConfigDto): Promise<SystemConfigResponseDto> {
+  public updateConfig(@Body(ValidationPipe) dto: SystemConfigDto): Promise<SystemConfigDto> {
     return this.systemConfigService.updateConfig(dto);
   }
 }

server/apps/immich/src/api-v1/system-config/system-config.service.ts

@@ -1,20 +1,23 @@
 import { Injectable } from '@nestjs/common';
 import { ImmichConfigService } from 'libs/immich-config/src';
-import { UpdateSystemConfigDto } from './dto/update-system-config';
-import { SystemConfigResponseDto } from './response-dto/system-config-response.dto';
+import { mapConfig, SystemConfigDto } from './dto/system-config.dto';
 
 @Injectable()
 export class SystemConfigService {
   constructor(private immichConfigService: ImmichConfigService) {}
 
-  async getConfig(): Promise<SystemConfigResponseDto> {
-    const config = await this.immichConfigService.getSystemConfig();
-    return { config };
+  public async getConfig(): Promise<SystemConfigDto> {
+    const config = await this.immichConfigService.getConfig();
+    return mapConfig(config);
   }
 
-  async updateConfig(dto: UpdateSystemConfigDto): Promise<SystemConfigResponseDto> {
-    await this.immichConfigService.updateSystemConfig(dto.config);
-    const config = await this.immichConfigService.getSystemConfig();
-    return { config };
+  public getDefaults(): SystemConfigDto {
+    const config = this.immichConfigService.getDefaults();
+    return mapConfig(config);
+  }
+
+  public async updateConfig(dto: SystemConfigDto): Promise<SystemConfigDto> {
+    await this.immichConfigService.updateConfig(dto);
+    return this.getConfig();
   }
 }