mirror of
https://github.com/pocket-id/pocket-id.git
synced 2026-07-15 21:48:13 +03:00
fix: allow insecure callback URLs by default until next major release
This commit is contained in:
@@ -15,4 +15,4 @@ ENCRYPTION_KEY=
|
||||
TRUST_PROXY=false
|
||||
MAXMIND_LICENSE_KEY=
|
||||
PUID=1000
|
||||
PGID=1000
|
||||
PGID=1000
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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 == "" {
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user