From a597b0edb19cf329d778f55e3376df30a7e50cea Mon Sep 17 00:00:00 2001 From: izzy Date: Thu, 6 Nov 2025 17:50:48 +0000 Subject: [PATCH] refactor: always validate login (i.e. check cookie) --- .../lib/model/maintenance_login_dto.dart | 19 +++++++++--- open-api/immich-openapi-specs.json | 25 --------------- open-api/typescript-sdk/src/fetch-client.ts | 2 +- .../maintenance-worker.controller.ts | 5 +-- .../src/controllers/maintenance.controller.ts | 10 ++---- server/src/dtos/maintenance.dto.ts | 2 +- web/src/lib/utils/maintenance.ts | 31 ++++++------------- 7 files changed, 31 insertions(+), 63 deletions(-) diff --git a/mobile/openapi/lib/model/maintenance_login_dto.dart b/mobile/openapi/lib/model/maintenance_login_dto.dart index ef01e97456..45f56bd3ba 100644 --- a/mobile/openapi/lib/model/maintenance_login_dto.dart +++ b/mobile/openapi/lib/model/maintenance_login_dto.dart @@ -13,10 +13,16 @@ part of openapi.api; class MaintenanceLoginDto { /// Returns a new [MaintenanceLoginDto] instance. MaintenanceLoginDto({ - required this.token, + this.token, }); - String token; + /// + /// Please note: This property should have been non-nullable! Since the specification file + /// does not include a default value (using the "default:" property), however, the generated + /// source code must fall back to having a nullable type. + /// Consider adding a "default:" property in the specification file to hide this note. + /// + String? token; @override bool operator ==(Object other) => identical(this, other) || other is MaintenanceLoginDto && @@ -25,14 +31,18 @@ class MaintenanceLoginDto { @override int get hashCode => // ignore: unnecessary_parenthesis - (token.hashCode); + (token == null ? 0 : token!.hashCode); @override String toString() => 'MaintenanceLoginDto[token=$token]'; Map toJson() { final json = {}; + if (this.token != null) { json[r'token'] = this.token; + } else { + // json[r'token'] = null; + } return json; } @@ -45,7 +55,7 @@ class MaintenanceLoginDto { final json = value.cast(); return MaintenanceLoginDto( - token: mapValueOfType(json, r'token')!, + token: mapValueOfType(json, r'token'), ); } return null; @@ -93,7 +103,6 @@ class MaintenanceLoginDto { /// The list of required keys that must be present in a JSON. static const requiredKeys = { - 'token', }; } diff --git a/open-api/immich-openapi-specs.json b/open-api/immich-openapi-specs.json index 093bb7ecda..0f2f1fa0cb 100644 --- a/open-api/immich-openapi-specs.json +++ b/open-api/immich-openapi-specs.json @@ -248,13 +248,6 @@ "parameters": [], "responses": { "201": { - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/MaintenanceModeResponseDto" - } - } - }, "description": "" } }, @@ -314,13 +307,6 @@ "parameters": [], "responses": { "201": { - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/MaintenanceModeResponseDto" - } - } - }, "description": "" } }, @@ -12695,17 +12681,6 @@ }, "type": "object" }, - "MaintenanceModeResponseDto": { - "properties": { - "isMaintenanceMode": { - "type": "boolean" - } - }, - "required": [ - "isMaintenanceMode" - ], - "type": "object" - }, "ManualJobName": { "enum": [ "person-cleanup", diff --git a/open-api/typescript-sdk/src/fetch-client.ts b/open-api/typescript-sdk/src/fetch-client.ts index 668b667aba..670e3b405f 100644 --- a/open-api/typescript-sdk/src/fetch-client.ts +++ b/open-api/typescript-sdk/src/fetch-client.ts @@ -44,7 +44,7 @@ export type MaintenanceModeResponseDto = { isMaintenanceMode: boolean; }; export type MaintenanceLoginDto = { - token: string; + token?: string; }; export type MaintenanceAuthDto = { username: string; diff --git a/server/src/controllers/maintenance-worker.controller.ts b/server/src/controllers/maintenance-worker.controller.ts index 42371a47ea..f2eb64aaa2 100644 --- a/server/src/controllers/maintenance-worker.controller.ts +++ b/server/src/controllers/maintenance-worker.controller.ts @@ -23,8 +23,9 @@ export class MaintenanceWorkerController { @Body() dto: MaintenanceLoginDto, @Res({ passthrough: true }) response: Response, ): Promise { - const auth = await this.service.login(dto.token ?? request.cookies[ImmichCookie.MaintenanceToken]); - response.cookie(ImmichCookie.MaintenanceToken, dto.token); + const token = dto.token ?? request.cookies[ImmichCookie.MaintenanceToken]; + const auth = await this.service.login(token); + response.cookie(ImmichCookie.MaintenanceToken, token); return auth; } diff --git a/server/src/controllers/maintenance.controller.ts b/server/src/controllers/maintenance.controller.ts index 2b3baa713f..77ccf91cd9 100644 --- a/server/src/controllers/maintenance.controller.ts +++ b/server/src/controllers/maintenance.controller.ts @@ -2,7 +2,7 @@ import { BadRequestException, Body, Controller, Post, Res } from '@nestjs/common import { ApiTags } from '@nestjs/swagger'; import { Response } from 'express'; import { AuthDto } from 'src/dtos/auth.dto'; -import { MaintenanceAuthDto, MaintenanceLoginDto, MaintenanceModeResponseDto } from 'src/dtos/maintenance.dto'; +import { MaintenanceAuthDto, MaintenanceLoginDto } from 'src/dtos/maintenance.dto'; import { ImmichCookie, Permission } from 'src/enum'; import { Auth, Authenticated } from 'src/middleware/auth.guard'; import { MaintenanceRepository } from 'src/repositories/maintenance.repository'; @@ -20,22 +20,18 @@ export class MaintenanceController { @Post('start') @Authenticated({ permission: Permission.Maintenance, admin: true }) - async startMaintenance( - @Auth() auth: AuthDto, - @Res({ passthrough: true }) response: Response, - ): Promise { + async startMaintenance(@Auth() auth: AuthDto, @Res({ passthrough: true }) response: Response): Promise { const { secret } = await this.service.startMaintenance(); const jwt = await MaintenanceRepository.createJwt(secret, { username: auth.user.name, }); response.cookie(ImmichCookie.MaintenanceToken, jwt); - return { isMaintenanceMode: true }; } @Post('end') @Authenticated({ permission: Permission.Maintenance, admin: true }) - endMaintenance(): Promise { + endMaintenance(): void { throw new BadRequestException('Not in maintenance mode'); } } diff --git a/server/src/dtos/maintenance.dto.ts b/server/src/dtos/maintenance.dto.ts index a4c3724c30..df21921327 100644 --- a/server/src/dtos/maintenance.dto.ts +++ b/server/src/dtos/maintenance.dto.ts @@ -6,7 +6,7 @@ export class MaintenanceModeResponseDto { } export class MaintenanceLoginDto { - @ValidateString() + @ValidateString({ optional: true }) token?: string; } diff --git a/web/src/lib/utils/maintenance.ts b/web/src/lib/utils/maintenance.ts index 6f91b2813c..2afa74c742 100644 --- a/web/src/lib/utils/maintenance.ts +++ b/web/src/lib/utils/maintenance.ts @@ -20,31 +20,18 @@ export function maintenanceShouldRedirect(maintenanceMode: boolean, currentUrl: export const loadMaintenanceAuth = async () => { try { const maintenanceAuth = get(maintenanceAuth$); - const query = new URLSearchParams(location.search); - const queryToken = query.get('token'); - if (!maintenanceAuth || queryToken) { - const cookie = document.cookie - .split(';') - .map((cookie) => cookie.split('=', 2).map((value) => value.trim())) - .find(([name]) => name === 'immich_maintenance_token'); + try { + const auth = await maintenanceLogin({ + maintenanceLoginDto: { + token: query.get('token') ?? undefined, + }, + }); - const token = queryToken ?? cookie?.[1]; - - if (token) { - try { - const auth = await maintenanceLogin({ - maintenanceLoginDto: { - token, - }, - }); - - maintenanceAuth$.set(auth); - } catch (error) { - void error; - } - } + maintenanceAuth$.set(auth); + } catch (error) { + void error; } return maintenanceAuth;