mirror of
https://github.com/immich-app/immich.git
synced 2026-05-18 03:10:24 +03:00
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
This commit is contained in:
@@ -76,6 +76,7 @@ const setupOAuth = async (token: string, dto: Partial<SystemConfigOAuthDto>) =>
|
|||||||
...defaults.oauth,
|
...defaults.oauth,
|
||||||
buttonText: 'Login with Immich',
|
buttonText: 'Login with Immich',
|
||||||
issuerUrl: `${authServer.internal}/.well-known/openid-configuration`,
|
issuerUrl: `${authServer.internal}/.well-known/openid-configuration`,
|
||||||
|
allowInsecureRequests: true,
|
||||||
...dto,
|
...dto,
|
||||||
};
|
};
|
||||||
await updateConfig({ systemConfigDto: { ...defaults, oauth: merged } }, options);
|
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 });
|
||||||
|
});
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -267,6 +267,8 @@
|
|||||||
"notification_enable_email_notifications": "Enable email notifications",
|
"notification_enable_email_notifications": "Enable email notifications",
|
||||||
"notification_settings": "Notification Settings",
|
"notification_settings": "Notification Settings",
|
||||||
"notification_settings_description": "Manage notification settings, including email",
|
"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": "Auto launch",
|
||||||
"oauth_auto_launch_description": "Start the OAuth login flow automatically upon navigating to the login page",
|
"oauth_auto_launch_description": "Start the OAuth login flow automatically upon navigating to the login page",
|
||||||
"oauth_auto_register": "Auto register",
|
"oauth_auto_register": "Auto register",
|
||||||
|
|||||||
+10
-1
@@ -13,6 +13,7 @@ part of openapi.api;
|
|||||||
class SystemConfigOAuthDto {
|
class SystemConfigOAuthDto {
|
||||||
/// Returns a new [SystemConfigOAuthDto] instance.
|
/// Returns a new [SystemConfigOAuthDto] instance.
|
||||||
SystemConfigOAuthDto({
|
SystemConfigOAuthDto({
|
||||||
|
required this.allowInsecureRequests,
|
||||||
required this.autoLaunch,
|
required this.autoLaunch,
|
||||||
required this.autoRegister,
|
required this.autoRegister,
|
||||||
required this.buttonText,
|
required this.buttonText,
|
||||||
@@ -33,6 +34,9 @@ class SystemConfigOAuthDto {
|
|||||||
required this.tokenEndpointAuthMethod,
|
required this.tokenEndpointAuthMethod,
|
||||||
});
|
});
|
||||||
|
|
||||||
|
/// Allow insecure requests
|
||||||
|
bool allowInsecureRequests;
|
||||||
|
|
||||||
/// Auto launch
|
/// Auto launch
|
||||||
bool autoLaunch;
|
bool autoLaunch;
|
||||||
|
|
||||||
@@ -93,6 +97,7 @@ class SystemConfigOAuthDto {
|
|||||||
|
|
||||||
@override
|
@override
|
||||||
bool operator ==(Object other) => identical(this, other) || other is SystemConfigOAuthDto &&
|
bool operator ==(Object other) => identical(this, other) || other is SystemConfigOAuthDto &&
|
||||||
|
other.allowInsecureRequests == allowInsecureRequests &&
|
||||||
other.autoLaunch == autoLaunch &&
|
other.autoLaunch == autoLaunch &&
|
||||||
other.autoRegister == autoRegister &&
|
other.autoRegister == autoRegister &&
|
||||||
other.buttonText == buttonText &&
|
other.buttonText == buttonText &&
|
||||||
@@ -115,6 +120,7 @@ class SystemConfigOAuthDto {
|
|||||||
@override
|
@override
|
||||||
int get hashCode =>
|
int get hashCode =>
|
||||||
// ignore: unnecessary_parenthesis
|
// ignore: unnecessary_parenthesis
|
||||||
|
(allowInsecureRequests.hashCode) +
|
||||||
(autoLaunch.hashCode) +
|
(autoLaunch.hashCode) +
|
||||||
(autoRegister.hashCode) +
|
(autoRegister.hashCode) +
|
||||||
(buttonText.hashCode) +
|
(buttonText.hashCode) +
|
||||||
@@ -135,10 +141,11 @@ class SystemConfigOAuthDto {
|
|||||||
(tokenEndpointAuthMethod.hashCode);
|
(tokenEndpointAuthMethod.hashCode);
|
||||||
|
|
||||||
@override
|
@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<String, dynamic> toJson() {
|
Map<String, dynamic> toJson() {
|
||||||
final json = <String, dynamic>{};
|
final json = <String, dynamic>{};
|
||||||
|
json[r'allowInsecureRequests'] = this.allowInsecureRequests;
|
||||||
json[r'autoLaunch'] = this.autoLaunch;
|
json[r'autoLaunch'] = this.autoLaunch;
|
||||||
json[r'autoRegister'] = this.autoRegister;
|
json[r'autoRegister'] = this.autoRegister;
|
||||||
json[r'buttonText'] = this.buttonText;
|
json[r'buttonText'] = this.buttonText;
|
||||||
@@ -173,6 +180,7 @@ class SystemConfigOAuthDto {
|
|||||||
final json = value.cast<String, dynamic>();
|
final json = value.cast<String, dynamic>();
|
||||||
|
|
||||||
return SystemConfigOAuthDto(
|
return SystemConfigOAuthDto(
|
||||||
|
allowInsecureRequests: mapValueOfType<bool>(json, r'allowInsecureRequests')!,
|
||||||
autoLaunch: mapValueOfType<bool>(json, r'autoLaunch')!,
|
autoLaunch: mapValueOfType<bool>(json, r'autoLaunch')!,
|
||||||
autoRegister: mapValueOfType<bool>(json, r'autoRegister')!,
|
autoRegister: mapValueOfType<bool>(json, r'autoRegister')!,
|
||||||
buttonText: mapValueOfType<String>(json, r'buttonText')!,
|
buttonText: mapValueOfType<String>(json, r'buttonText')!,
|
||||||
@@ -240,6 +248,7 @@ class SystemConfigOAuthDto {
|
|||||||
|
|
||||||
/// The list of required keys that must be present in a JSON.
|
/// The list of required keys that must be present in a JSON.
|
||||||
static const requiredKeys = <String>{
|
static const requiredKeys = <String>{
|
||||||
|
'allowInsecureRequests',
|
||||||
'autoLaunch',
|
'autoLaunch',
|
||||||
'autoRegister',
|
'autoRegister',
|
||||||
'buttonText',
|
'buttonText',
|
||||||
|
|||||||
@@ -24302,6 +24302,10 @@
|
|||||||
},
|
},
|
||||||
"SystemConfigOAuthDto": {
|
"SystemConfigOAuthDto": {
|
||||||
"properties": {
|
"properties": {
|
||||||
|
"allowInsecureRequests": {
|
||||||
|
"description": "Allow insecure requests",
|
||||||
|
"type": "boolean"
|
||||||
|
},
|
||||||
"autoLaunch": {
|
"autoLaunch": {
|
||||||
"description": "Auto launch",
|
"description": "Auto launch",
|
||||||
"type": "boolean"
|
"type": "boolean"
|
||||||
@@ -24379,6 +24383,7 @@
|
|||||||
}
|
}
|
||||||
},
|
},
|
||||||
"required": [
|
"required": [
|
||||||
|
"allowInsecureRequests",
|
||||||
"autoLaunch",
|
"autoLaunch",
|
||||||
"autoRegister",
|
"autoRegister",
|
||||||
"buttonText",
|
"buttonText",
|
||||||
|
|||||||
@@ -2502,6 +2502,8 @@ export type SystemConfigNotificationsDto = {
|
|||||||
smtp: SystemConfigSmtpDto;
|
smtp: SystemConfigSmtpDto;
|
||||||
};
|
};
|
||||||
export type SystemConfigOAuthDto = {
|
export type SystemConfigOAuthDto = {
|
||||||
|
/** Allow insecure requests */
|
||||||
|
allowInsecureRequests: boolean;
|
||||||
/** Auto launch */
|
/** Auto launch */
|
||||||
autoLaunch: boolean;
|
autoLaunch: boolean;
|
||||||
/** Auto register */
|
/** Auto register */
|
||||||
|
|||||||
@@ -111,6 +111,7 @@ export type SystemConfig = {
|
|||||||
profileSigningAlgorithm: string;
|
profileSigningAlgorithm: string;
|
||||||
tokenEndpointAuthMethod: OAuthTokenEndpointAuthMethod;
|
tokenEndpointAuthMethod: OAuthTokenEndpointAuthMethod;
|
||||||
timeout: number;
|
timeout: number;
|
||||||
|
allowInsecureRequests: boolean;
|
||||||
storageLabelClaim: string;
|
storageLabelClaim: string;
|
||||||
storageQuotaClaim: string;
|
storageQuotaClaim: string;
|
||||||
roleClaim: string;
|
roleClaim: string;
|
||||||
@@ -305,6 +306,7 @@ export const defaults = Object.freeze<SystemConfig>({
|
|||||||
roleClaim: 'immich_role',
|
roleClaim: 'immich_role',
|
||||||
tokenEndpointAuthMethod: OAuthTokenEndpointAuthMethod.ClientSecretPost,
|
tokenEndpointAuthMethod: OAuthTokenEndpointAuthMethod.ClientSecretPost,
|
||||||
timeout: 30_000,
|
timeout: 30_000,
|
||||||
|
allowInsecureRequests: false,
|
||||||
},
|
},
|
||||||
passwordLogin: {
|
passwordLogin: {
|
||||||
enabled: true,
|
enabled: true,
|
||||||
|
|||||||
@@ -179,6 +179,7 @@ const SystemConfigOAuthSchema = z
|
|||||||
clientSecret: z.string().describe('Client secret'),
|
clientSecret: z.string().describe('Client secret'),
|
||||||
tokenEndpointAuthMethod: OAuthTokenEndpointAuthMethodSchema,
|
tokenEndpointAuthMethod: OAuthTokenEndpointAuthMethodSchema,
|
||||||
timeout: z.int().min(1).describe('Timeout'),
|
timeout: z.int().min(1).describe('Timeout'),
|
||||||
|
allowInsecureRequests: configBool.describe('Allow insecure requests'),
|
||||||
defaultStorageQuota: z.number().min(0).nullable().describe('Default storage quota'),
|
defaultStorageQuota: z.number().min(0).nullable().describe('Default storage quota'),
|
||||||
enabled: configBool.describe('Enabled'),
|
enabled: configBool.describe('Enabled'),
|
||||||
issuerUrl: z
|
issuerUrl: z
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
import { Injectable, InternalServerErrorException } from '@nestjs/common';
|
import { Injectable, InternalServerErrorException } from '@nestjs/common';
|
||||||
import {
|
import {
|
||||||
allowInsecureRequests,
|
allowInsecureRequests as allowInsecureRequestsExecute,
|
||||||
authorizationCodeGrant,
|
authorizationCodeGrant,
|
||||||
buildAuthorizationUrl,
|
buildAuthorizationUrl,
|
||||||
calculatePKCECodeChallenge,
|
calculatePKCECodeChallenge,
|
||||||
@@ -28,6 +28,7 @@ export type OAuthConfig = {
|
|||||||
signingAlgorithm: string;
|
signingAlgorithm: string;
|
||||||
tokenEndpointAuthMethod: OAuthTokenEndpointAuthMethod;
|
tokenEndpointAuthMethod: OAuthTokenEndpointAuthMethod;
|
||||||
timeout: number;
|
timeout: number;
|
||||||
|
allowInsecureRequests: boolean;
|
||||||
};
|
};
|
||||||
export type OAuthProfile = UserInfoResponse;
|
export type OAuthProfile = UserInfoResponse;
|
||||||
|
|
||||||
@@ -133,6 +134,7 @@ export class OAuthRepository {
|
|||||||
signingAlgorithm,
|
signingAlgorithm,
|
||||||
tokenEndpointAuthMethod,
|
tokenEndpointAuthMethod,
|
||||||
timeout,
|
timeout,
|
||||||
|
allowInsecureRequests,
|
||||||
}: OAuthConfig) {
|
}: OAuthConfig) {
|
||||||
try {
|
try {
|
||||||
return await discovery(
|
return await discovery(
|
||||||
@@ -146,7 +148,7 @@ export class OAuthRepository {
|
|||||||
},
|
},
|
||||||
this.getTokenAuthMethod(tokenEndpointAuthMethod, clientSecret),
|
this.getTokenAuthMethod(tokenEndpointAuthMethod, clientSecret),
|
||||||
{
|
{
|
||||||
execute: [allowInsecureRequests],
|
execute: allowInsecureRequests ? [allowInsecureRequestsExecute] : [],
|
||||||
timeout,
|
timeout,
|
||||||
},
|
},
|
||||||
);
|
);
|
||||||
|
|||||||
@@ -0,0 +1,22 @@
|
|||||||
|
import { Kysely, sql } from 'kysely';
|
||||||
|
|
||||||
|
export async function up(db: Kysely<any>): Promise<void> {
|
||||||
|
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<any>): Promise<void> {
|
||||||
|
await sql`
|
||||||
|
UPDATE system_metadata
|
||||||
|
SET value = value #- '{oauth,allowInsecureRequests}'
|
||||||
|
WHERE key = 'system-config'
|
||||||
|
`.execute(db);
|
||||||
|
}
|
||||||
@@ -145,6 +145,7 @@ const updatedConfig = Object.freeze<SystemConfig>({
|
|||||||
profileSigningAlgorithm: 'none',
|
profileSigningAlgorithm: 'none',
|
||||||
tokenEndpointAuthMethod: OAuthTokenEndpointAuthMethod.ClientSecretPost,
|
tokenEndpointAuthMethod: OAuthTokenEndpointAuthMethod.ClientSecretPost,
|
||||||
timeout: 30_000,
|
timeout: 30_000,
|
||||||
|
allowInsecureRequests: false,
|
||||||
storageLabelClaim: 'preferred_username',
|
storageLabelClaim: 'preferred_username',
|
||||||
storageQuotaClaim: 'immich_quota',
|
storageQuotaClaim: 'immich_quota',
|
||||||
roleClaim: 'immich_role',
|
roleClaim: 'immich_role',
|
||||||
|
|||||||
@@ -174,6 +174,14 @@
|
|||||||
isEdited={!(configToEdit.oauth.timeout === config.oauth.timeout)}
|
isEdited={!(configToEdit.oauth.timeout === config.oauth.timeout)}
|
||||||
/>
|
/>
|
||||||
|
|
||||||
|
<SettingSwitch
|
||||||
|
title={$t('admin.oauth_allow_insecure_requests')}
|
||||||
|
subtitle={$t('admin.oauth_allow_insecure_requests_description')}
|
||||||
|
bind:checked={configToEdit.oauth.allowInsecureRequests}
|
||||||
|
disabled={disabled || !configToEdit.oauth.enabled}
|
||||||
|
isEdited={!(configToEdit.oauth.allowInsecureRequests === config.oauth.allowInsecureRequests)}
|
||||||
|
/>
|
||||||
|
|
||||||
<SettingInputField
|
<SettingInputField
|
||||||
inputType={SettingInputFieldType.TEXT}
|
inputType={SettingInputFieldType.TEXT}
|
||||||
label={$t('admin.oauth_storage_label_claim')}
|
label={$t('admin.oauth_storage_label_claim')}
|
||||||
|
|||||||
Reference in New Issue
Block a user