From 3356e81c85eddf7b145a333e786542970bf0669e Mon Sep 17 00:00:00 2001 From: bo0tzz Date: Thu, 16 Apr 2026 16:11:58 +0200 Subject: [PATCH] fix!: do not allow insecure oauth requests by default (#27844) * fix!: do not allow insecure oauth requests by default * fix: format * fix: make open-api * fix: tests * nit: casing * chore: migration to allow insecure if current oauth uses http --- e2e/src/specs/server/api/oauth.e2e-spec.ts | 20 +++++++++++++++++ i18n/en.json | 2 ++ .../lib/model/system_config_o_auth_dto.dart | 11 +++++++++- open-api/immich-openapi-specs.json | 5 +++++ open-api/typescript-sdk/src/fetch-client.ts | 2 ++ server/src/config.ts | 2 ++ server/src/dtos/system-config.dto.ts | 1 + server/src/repositories/oauth.repository.ts | 6 +++-- ...332807985-SetOAuthAllowInsecureRequests.ts | 22 +++++++++++++++++++ .../services/system-config.service.spec.ts | 1 + .../admin-settings/AuthSettings.svelte | 8 +++++++ 11 files changed, 77 insertions(+), 3 deletions(-) create mode 100644 server/src/schema/migrations/1776332807985-SetOAuthAllowInsecureRequests.ts diff --git a/e2e/src/specs/server/api/oauth.e2e-spec.ts b/e2e/src/specs/server/api/oauth.e2e-spec.ts index a0ae1dc819..e4127438f7 100644 --- a/e2e/src/specs/server/api/oauth.e2e-spec.ts +++ b/e2e/src/specs/server/api/oauth.e2e-spec.ts @@ -76,6 +76,7 @@ const setupOAuth = async (token: string, dto: Partial) => ...defaults.oauth, buttonText: 'Login with Immich', issuerUrl: `${authServer.internal}/.well-known/openid-configuration`, + allowInsecureRequests: true, ...dto, }; await updateConfig({ systemConfigDto: { ...defaults, oauth: merged } }, options); @@ -399,4 +400,23 @@ describe(`/oauth`, () => { }); }); }); + + describe('allowInsecureRequests: false', () => { + beforeAll(async () => { + await setupOAuth(admin.accessToken, { + enabled: true, + clientId: OAuthClient.DEFAULT, + clientSecret: OAuthClient.DEFAULT, + allowInsecureRequests: false, + }); + }); + + it('should reject OAuth discovery over HTTP', async () => { + const { status, body } = await request(app) + .post('/oauth/authorize') + .send({ redirectUri: 'http://127.0.0.1:2285/auth/login' }); + expect(status).toBe(500); + expect(body).toMatchObject({ statusCode: 500 }); + }); + }); }); diff --git a/i18n/en.json b/i18n/en.json index 351b97f00c..4f608f890d 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -267,6 +267,8 @@ "notification_enable_email_notifications": "Enable email notifications", "notification_settings": "Notification Settings", "notification_settings_description": "Manage notification settings, including email", + "oauth_allow_insecure_requests": "Allow insecure requests", + "oauth_allow_insecure_requests_description": "WARNING: This disables TLS certificate validation for OAuth requests and may expose you to MITM attacks.", "oauth_auto_launch": "Auto launch", "oauth_auto_launch_description": "Start the OAuth login flow automatically upon navigating to the login page", "oauth_auto_register": "Auto register", diff --git a/mobile/openapi/lib/model/system_config_o_auth_dto.dart b/mobile/openapi/lib/model/system_config_o_auth_dto.dart index 88dddbb4d3..cc8c38e503 100644 --- a/mobile/openapi/lib/model/system_config_o_auth_dto.dart +++ b/mobile/openapi/lib/model/system_config_o_auth_dto.dart @@ -13,6 +13,7 @@ part of openapi.api; class SystemConfigOAuthDto { /// Returns a new [SystemConfigOAuthDto] instance. SystemConfigOAuthDto({ + required this.allowInsecureRequests, required this.autoLaunch, required this.autoRegister, required this.buttonText, @@ -33,6 +34,9 @@ class SystemConfigOAuthDto { required this.tokenEndpointAuthMethod, }); + /// Allow insecure requests + bool allowInsecureRequests; + /// Auto launch bool autoLaunch; @@ -93,6 +97,7 @@ class SystemConfigOAuthDto { @override bool operator ==(Object other) => identical(this, other) || other is SystemConfigOAuthDto && + other.allowInsecureRequests == allowInsecureRequests && other.autoLaunch == autoLaunch && other.autoRegister == autoRegister && other.buttonText == buttonText && @@ -115,6 +120,7 @@ class SystemConfigOAuthDto { @override int get hashCode => // ignore: unnecessary_parenthesis + (allowInsecureRequests.hashCode) + (autoLaunch.hashCode) + (autoRegister.hashCode) + (buttonText.hashCode) + @@ -135,10 +141,11 @@ class SystemConfigOAuthDto { (tokenEndpointAuthMethod.hashCode); @override - String toString() => 'SystemConfigOAuthDto[autoLaunch=$autoLaunch, autoRegister=$autoRegister, buttonText=$buttonText, clientId=$clientId, clientSecret=$clientSecret, defaultStorageQuota=$defaultStorageQuota, enabled=$enabled, issuerUrl=$issuerUrl, mobileOverrideEnabled=$mobileOverrideEnabled, mobileRedirectUri=$mobileRedirectUri, profileSigningAlgorithm=$profileSigningAlgorithm, roleClaim=$roleClaim, scope=$scope, signingAlgorithm=$signingAlgorithm, storageLabelClaim=$storageLabelClaim, storageQuotaClaim=$storageQuotaClaim, timeout=$timeout, tokenEndpointAuthMethod=$tokenEndpointAuthMethod]'; + String toString() => 'SystemConfigOAuthDto[allowInsecureRequests=$allowInsecureRequests, autoLaunch=$autoLaunch, autoRegister=$autoRegister, buttonText=$buttonText, clientId=$clientId, clientSecret=$clientSecret, defaultStorageQuota=$defaultStorageQuota, enabled=$enabled, issuerUrl=$issuerUrl, mobileOverrideEnabled=$mobileOverrideEnabled, mobileRedirectUri=$mobileRedirectUri, profileSigningAlgorithm=$profileSigningAlgorithm, roleClaim=$roleClaim, scope=$scope, signingAlgorithm=$signingAlgorithm, storageLabelClaim=$storageLabelClaim, storageQuotaClaim=$storageQuotaClaim, timeout=$timeout, tokenEndpointAuthMethod=$tokenEndpointAuthMethod]'; Map toJson() { final json = {}; + json[r'allowInsecureRequests'] = this.allowInsecureRequests; json[r'autoLaunch'] = this.autoLaunch; json[r'autoRegister'] = this.autoRegister; json[r'buttonText'] = this.buttonText; @@ -173,6 +180,7 @@ class SystemConfigOAuthDto { final json = value.cast(); return SystemConfigOAuthDto( + allowInsecureRequests: mapValueOfType(json, r'allowInsecureRequests')!, autoLaunch: mapValueOfType(json, r'autoLaunch')!, autoRegister: mapValueOfType(json, r'autoRegister')!, buttonText: mapValueOfType(json, r'buttonText')!, @@ -240,6 +248,7 @@ class SystemConfigOAuthDto { /// The list of required keys that must be present in a JSON. static const requiredKeys = { + 'allowInsecureRequests', 'autoLaunch', 'autoRegister', 'buttonText', diff --git a/open-api/immich-openapi-specs.json b/open-api/immich-openapi-specs.json index 33a692474a..175d609bd3 100644 --- a/open-api/immich-openapi-specs.json +++ b/open-api/immich-openapi-specs.json @@ -24302,6 +24302,10 @@ }, "SystemConfigOAuthDto": { "properties": { + "allowInsecureRequests": { + "description": "Allow insecure requests", + "type": "boolean" + }, "autoLaunch": { "description": "Auto launch", "type": "boolean" @@ -24379,6 +24383,7 @@ } }, "required": [ + "allowInsecureRequests", "autoLaunch", "autoRegister", "buttonText", diff --git a/open-api/typescript-sdk/src/fetch-client.ts b/open-api/typescript-sdk/src/fetch-client.ts index 0875715beb..2164b1acb4 100644 --- a/open-api/typescript-sdk/src/fetch-client.ts +++ b/open-api/typescript-sdk/src/fetch-client.ts @@ -2502,6 +2502,8 @@ export type SystemConfigNotificationsDto = { smtp: SystemConfigSmtpDto; }; export type SystemConfigOAuthDto = { + /** Allow insecure requests */ + allowInsecureRequests: boolean; /** Auto launch */ autoLaunch: boolean; /** Auto register */ diff --git a/server/src/config.ts b/server/src/config.ts index b4feeca4dc..bf408fcca7 100644 --- a/server/src/config.ts +++ b/server/src/config.ts @@ -111,6 +111,7 @@ export type SystemConfig = { profileSigningAlgorithm: string; tokenEndpointAuthMethod: OAuthTokenEndpointAuthMethod; timeout: number; + allowInsecureRequests: boolean; storageLabelClaim: string; storageQuotaClaim: string; roleClaim: string; @@ -305,6 +306,7 @@ export const defaults = Object.freeze({ roleClaim: 'immich_role', tokenEndpointAuthMethod: OAuthTokenEndpointAuthMethod.ClientSecretPost, timeout: 30_000, + allowInsecureRequests: false, }, passwordLogin: { enabled: true, diff --git a/server/src/dtos/system-config.dto.ts b/server/src/dtos/system-config.dto.ts index 8160ab12cd..4e5bef2627 100644 --- a/server/src/dtos/system-config.dto.ts +++ b/server/src/dtos/system-config.dto.ts @@ -179,6 +179,7 @@ const SystemConfigOAuthSchema = z clientSecret: z.string().describe('Client secret'), tokenEndpointAuthMethod: OAuthTokenEndpointAuthMethodSchema, timeout: z.int().min(1).describe('Timeout'), + allowInsecureRequests: configBool.describe('Allow insecure requests'), defaultStorageQuota: z.number().min(0).nullable().describe('Default storage quota'), enabled: configBool.describe('Enabled'), issuerUrl: z diff --git a/server/src/repositories/oauth.repository.ts b/server/src/repositories/oauth.repository.ts index b2e72e470a..8fb969a233 100644 --- a/server/src/repositories/oauth.repository.ts +++ b/server/src/repositories/oauth.repository.ts @@ -1,6 +1,6 @@ import { Injectable, InternalServerErrorException } from '@nestjs/common'; import { - allowInsecureRequests, + allowInsecureRequests as allowInsecureRequestsExecute, authorizationCodeGrant, buildAuthorizationUrl, calculatePKCECodeChallenge, @@ -28,6 +28,7 @@ export type OAuthConfig = { signingAlgorithm: string; tokenEndpointAuthMethod: OAuthTokenEndpointAuthMethod; timeout: number; + allowInsecureRequests: boolean; }; export type OAuthProfile = UserInfoResponse; @@ -133,6 +134,7 @@ export class OAuthRepository { signingAlgorithm, tokenEndpointAuthMethod, timeout, + allowInsecureRequests, }: OAuthConfig) { try { return await discovery( @@ -146,7 +148,7 @@ export class OAuthRepository { }, this.getTokenAuthMethod(tokenEndpointAuthMethod, clientSecret), { - execute: [allowInsecureRequests], + execute: allowInsecureRequests ? [allowInsecureRequestsExecute] : [], timeout, }, ); diff --git a/server/src/schema/migrations/1776332807985-SetOAuthAllowInsecureRequests.ts b/server/src/schema/migrations/1776332807985-SetOAuthAllowInsecureRequests.ts new file mode 100644 index 0000000000..f5e8fcf197 --- /dev/null +++ b/server/src/schema/migrations/1776332807985-SetOAuthAllowInsecureRequests.ts @@ -0,0 +1,22 @@ +import { Kysely, sql } from 'kysely'; + +export async function up(db: Kysely): Promise { + await sql` + UPDATE system_metadata + SET value = jsonb_set( + value, + '{oauth,allowInsecureRequests}', + 'true'::jsonb + ) + WHERE key = 'system-config' + AND value->'oauth'->>'issuerUrl' LIKE 'http://%' + `.execute(db); +} + +export async function down(db: Kysely): Promise { + await sql` + UPDATE system_metadata + SET value = value #- '{oauth,allowInsecureRequests}' + WHERE key = 'system-config' + `.execute(db); +} diff --git a/server/src/services/system-config.service.spec.ts b/server/src/services/system-config.service.spec.ts index a9d2e35507..992dc4a177 100644 --- a/server/src/services/system-config.service.spec.ts +++ b/server/src/services/system-config.service.spec.ts @@ -145,6 +145,7 @@ const updatedConfig = Object.freeze({ profileSigningAlgorithm: 'none', tokenEndpointAuthMethod: OAuthTokenEndpointAuthMethod.ClientSecretPost, timeout: 30_000, + allowInsecureRequests: false, storageLabelClaim: 'preferred_username', storageQuotaClaim: 'immich_quota', roleClaim: 'immich_role', diff --git a/web/src/lib/components/admin-settings/AuthSettings.svelte b/web/src/lib/components/admin-settings/AuthSettings.svelte index ae0e1c97c5..1dd8aff25b 100644 --- a/web/src/lib/components/admin-settings/AuthSettings.svelte +++ b/web/src/lib/components/admin-settings/AuthSettings.svelte @@ -174,6 +174,14 @@ isEdited={!(configToEdit.oauth.timeout === config.oauth.timeout)} /> + +