From d9ead47d19451b256336b7d0eb46606a236928f3 Mon Sep 17 00:00:00 2001 From: Elias Schneider Date: Mon, 13 Jul 2026 08:35:02 +0200 Subject: [PATCH] fix: allow insecure callback URLs by default until next major release --- .env.example | 2 +- .../internal/bootstrap/services_bootstrap.go | 7 +- backend/internal/common/env_config.go | 56 ++++++++-------- backend/internal/common/env_config_test.go | 2 + backend/internal/oidc/module.go | 7 +- backend/internal/oidc/provider.go | 12 ++++ backend/internal/oidc/provider_test.go | 65 +++++++++++++++++++ 7 files changed, 117 insertions(+), 34 deletions(-) diff --git a/.env.example b/.env.example index 0afd1375..fd3cfb2f 100644 --- a/.env.example +++ b/.env.example @@ -15,4 +15,4 @@ ENCRYPTION_KEY= TRUST_PROXY=false MAXMIND_LICENSE_KEY= PUID=1000 -PGID=1000 \ No newline at end of file +PGID=1000 diff --git a/backend/internal/bootstrap/services_bootstrap.go b/backend/internal/bootstrap/services_bootstrap.go index cfc8ac8d..d05edf06 100644 --- a/backend/internal/bootstrap/services_bootstrap.go +++ b/backend/internal/bootstrap/services_bootstrap.go @@ -88,9 +88,10 @@ func initServices(ctx context.Context, db *gorm.DB, instanceID string, httpClien DB: db, HTTPClient: httpClient, Config: oidc.Config{ - BaseURL: common.EnvConfig.AppURL, - TokenBaseURL: common.EnvConfig.AppURL, - Secret: common.EnvConfig.EncryptionKey, + BaseURL: common.EnvConfig.AppURL, + TokenBaseURL: common.EnvConfig.AppURL, + Secret: common.EnvConfig.EncryptionKey, + AllowInsecureCallbackURLs: common.EnvConfig.AllowInsecureCallbackURLs, }, Signer: svc.jwtService, CustomClaims: svc.customClaimService, diff --git a/backend/internal/common/env_config.go b/backend/internal/common/env_config.go index a98738fc..1f040596 100644 --- a/backend/internal/common/env_config.go +++ b/backend/internal/common/env_config.go @@ -38,21 +38,22 @@ const ( ) type EnvConfigSchema struct { - AppEnv AppEnv `env:"APP_ENV" options:"toLower"` - EncryptionKey []byte `env:"ENCRYPTION_KEY" options:"file"` - AppURL string `env:"APP_URL" options:"toLower,trimTrailingSlash"` - DbProvider DbProvider - DbConnectionString string `env:"DB_CONNECTION_STRING" options:"file"` - TrustProxy bool `env:"TRUST_PROXY"` - TrustedPlatform string `env:"TRUSTED_PLATFORM"` - AuditLogRetentionDays int `env:"AUDIT_LOG_RETENTION_DAYS"` - AnalyticsDisabled bool `env:"ANALYTICS_DISABLED"` - AllowDowngrade bool `env:"ALLOW_DOWNGRADE"` - InternalAppURL string `env:"INTERNAL_APP_URL"` - UiConfigDisabled bool `env:"UI_CONFIG_DISABLED"` - DisableRateLimiting bool `env:"DISABLE_RATE_LIMITING"` - VersionCheckDisabled bool `env:"VERSION_CHECK_DISABLED"` - StaticApiKey string `env:"STATIC_API_KEY" options:"file"` + AppEnv AppEnv `env:"APP_ENV" options:"toLower"` + EncryptionKey []byte `env:"ENCRYPTION_KEY" options:"file"` + AppURL string `env:"APP_URL" options:"toLower,trimTrailingSlash"` + DbProvider DbProvider + DbConnectionString string `env:"DB_CONNECTION_STRING" options:"file"` + TrustProxy bool `env:"TRUST_PROXY"` + TrustedPlatform string `env:"TRUSTED_PLATFORM"` + AuditLogRetentionDays int `env:"AUDIT_LOG_RETENTION_DAYS"` + AnalyticsDisabled bool `env:"ANALYTICS_DISABLED"` + AllowDowngrade bool `env:"ALLOW_DOWNGRADE"` + AllowInsecureCallbackURLs bool `env:"ALLOW_INSECURE_CALLBACK_URLS"` + InternalAppURL string `env:"INTERNAL_APP_URL"` + UiConfigDisabled bool `env:"UI_CONFIG_DISABLED"` + DisableRateLimiting bool `env:"DISABLE_RATE_LIMITING"` + VersionCheckDisabled bool `env:"VERSION_CHECK_DISABLED"` + StaticApiKey string `env:"STATIC_API_KEY" options:"file"` FileBackend string `env:"FILE_BACKEND" options:"toLower"` UploadPath string `env:"UPLOAD_PATH"` @@ -97,18 +98,19 @@ func init() { func defaultConfig() EnvConfigSchema { return EnvConfigSchema{ - AppEnv: AppEnvProduction, - LogLevel: "info", - DbProvider: "sqlite", - FileBackend: "filesystem", - AuditLogRetentionDays: 90, - AppURL: AppUrl, - Port: "1411", - Host: "0.0.0.0", - ActorsPort: "1414", - ActorsHost: "0.0.0.0", - GeoLiteDBPath: "data/GeoLite2-City.mmdb", - GeoLiteDBUrl: MaxMindGeoLiteCityUrl, + AppEnv: AppEnvProduction, + LogLevel: "info", + DbProvider: "sqlite", + FileBackend: "filesystem", + AuditLogRetentionDays: 90, + AllowInsecureCallbackURLs: true, // TODO: Default to false in major v3 + AppURL: AppUrl, + Port: "1411", + Host: "0.0.0.0", + ActorsPort: "1414", + ActorsHost: "0.0.0.0", + GeoLiteDBPath: "data/GeoLite2-City.mmdb", + GeoLiteDBUrl: MaxMindGeoLiteCityUrl, } } diff --git a/backend/internal/common/env_config_test.go b/backend/internal/common/env_config_test.go index a64ae75d..a28c13ce 100644 --- a/backend/internal/common/env_config_test.go +++ b/backend/internal/common/env_config_test.go @@ -119,12 +119,14 @@ func TestParseEnvConfig(t *testing.T) { t.Setenv("TRACING_ENABLED", "false") t.Setenv("TRUST_PROXY", "true") t.Setenv("ANALYTICS_DISABLED", "false") + t.Setenv("ALLOW_INSECURE_CALLBACK_URLS", "false") err := parseAndValidateEnvConfig(t) require.NoError(t, err) assert.True(t, EnvConfig.UiConfigDisabled) assert.True(t, EnvConfig.TrustProxy) assert.False(t, EnvConfig.AnalyticsDisabled) + assert.False(t, EnvConfig.AllowInsecureCallbackURLs) }) t.Run("should default audit log retention days to 90", func(t *testing.T) { diff --git a/backend/internal/oidc/module.go b/backend/internal/oidc/module.go index 7ba3688c..9a4a5b52 100644 --- a/backend/internal/oidc/module.go +++ b/backend/internal/oidc/module.go @@ -13,9 +13,10 @@ import ( ) type Config struct { - BaseURL string - TokenBaseURL string - Secret []byte + BaseURL string + TokenBaseURL string + Secret []byte + AllowInsecureCallbackURLs bool } type TokenSigner interface { diff --git a/backend/internal/oidc/provider.go b/backend/internal/oidc/provider.go index 76827bfb..0604700d 100644 --- a/backend/internal/oidc/provider.go +++ b/backend/internal/oidc/provider.go @@ -49,6 +49,7 @@ func newProvider(store *Store, authenticator *federatedClientAuthenticator, sign IgnoreUnknownScopes: true, AudienceMatchingStrategy: fosite.ExactAudienceMatchingStrategy, RedirectURIMatcher: matchRedirectURI, + RedirectSecureChecker: redirectSecureChecker(config.AllowInsecureCallbackURLs), EnforcePKCEForPublicClients: true, EnablePKCEPlainChallengeMethod: true, SupportedRequestObjectSigningAlgorithms: []string{"none"}, @@ -111,6 +112,17 @@ func newProvider(store *Store, authenticator *federatedClientAuthenticator, sign }, nil } +func redirectSecureChecker(allowInsecureCallbackURLs bool) func(context.Context, *url.URL) bool { + return func(ctx context.Context, redirectURI *url.URL) bool { + if allowInsecureCallbackURLs || fosite.IsRedirectURISecure(ctx, redirectURI) { + return true + } + + slog.InfoContext(ctx, "HTTP callback URL rejected; set ALLOW_INSECURE_CALLBACK_URLS=true to allow it", "callback_url", redirectURI.Redacted()) + return false + } +} + func matchRedirectURI(rawurl string, client fosite.Client) (*url.URL, error) { redirectURI, err := fosite.MatchRedirectURIWithClientRedirectURIs(rawurl, client) if err == nil || rawurl == "" { diff --git a/backend/internal/oidc/provider_test.go b/backend/internal/oidc/provider_test.go index 6f25a3e7..e60c6279 100644 --- a/backend/internal/oidc/provider_test.go +++ b/backend/internal/oidc/provider_test.go @@ -11,6 +11,7 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "net/url" "strings" "testing" "time" @@ -85,6 +86,70 @@ func TestProviderIssuesJWTAccessTokens(t *testing.T) { require.Equal(t, "test-key-id", header["kid"]) } +func TestRedirectSecureChecker(t *testing.T) { + loopbackRedirectURI, err := url.Parse("http://127.0.0.1:49813/callback") + require.NoError(t, err) + + checker := redirectSecureChecker(false) + require.True(t, checker(t.Context(), loopbackRedirectURI)) +} + +func TestProviderInsecureCallbackURLCompatibility(t *testing.T) { + tests := []struct { + name string + allowInsecureCallbackURLs bool + expectSuccess bool + }{ + { + name: "allows HTTP callback URLs when compatibility is enabled", + allowInsecureCallbackURLs: true, + expectSuccess: true, + }, + { + name: "rejects HTTP callback URLs when compatibility is disabled", + allowInsecureCallbackURLs: false, + expectSuccess: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + db := testutils.NewDatabaseForTest(t) + signerKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + require.NoError(t, db.Create(&model.OidcClient{ + Base: model.Base{ID: "test-client"}, + Name: "Test Client", + CallbackURLs: model.UrlList{"http://client.example.com/callback"}, + }).Error) + + provider, err := newProvider(NewStore(db, nil), nil, testTokenSigner{key: signerKey}, Config{ //nolint:gosec // static test-only provider secret + BaseURL: "https://issuer.example.com", + TokenBaseURL: "https://issuer.example.com", + Secret: []byte("test-secret"), + AllowInsecureCallbackURLs: tt.allowInsecureCallbackURLs, + }) + require.NoError(t, err) + + req := httptest.NewRequestWithContext( + t.Context(), + http.MethodGet, + "/api/oidc/authorize?client_id=test-client&response_type=code&scope=openid&state=state-with-enough-entropy&redirect_uri=http://client.example.com/callback", + nil, + ) + authorizeRequest, err := provider.NewAuthorizeRequest(req.Context(), req) + require.NoError(t, err) + + _, err = provider.NewAuthorizeResponse(t.Context(), authorizeRequest, NewEmptySession()) + if tt.expectSuccess { + require.NoError(t, err) + } else { + require.ErrorIs(t, err, fosite.ErrInvalidRequest) + } + }) + } +} + func TestProviderAcceptsWildcardRedirectURI(t *testing.T) { db := testutils.NewDatabaseForTest(t) signerKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)