diff --git a/README.md b/README.md index 9f5cf13..f7745bc 100644 --- a/README.md +++ b/README.md @@ -67,7 +67,8 @@ The middleware supports the following configuration options: |-----------|-------------|---------|---------| | `logoutURL` | The path for handling logout requests | `callbackURL + "/logout"` | `/oauth2/logout` | | `postLogoutRedirectURI` | The URL to redirect to after logout | `/` | `/logged-out-page` | -| `scopes` | Additional OAuth 2.0 scopes to append to defaults | `["openid", "profile", "email"]` (always included) | `["roles", "custom_scope"]` (appended to defaults) | +| `scopes` | OAuth 2.0 scopes to use for authentication | `["openid", "profile", "email"]` (always included by default) | `["roles", "custom_scope"]` (appended to defaults) | +| `overrideScopes` | When true, replaces default scopes with provided scopes instead of appending | `false` | `true` (use only the scopes explicitly provided) | | `logLevel` | Sets the logging verbosity | `info` | `debug`, `info`, `error` | | `forceHTTPS` | Forces the use of HTTPS for all URLs | `true` | `true`, `false` | | `rateLimit` | Sets the maximum number of requests per second | `100` | `500` | @@ -83,23 +84,35 @@ The middleware supports the following configuration options: ## Scope Configuration -### Scope Append Behavior +### Scope Behavior -The middleware uses an **append** behavior for OAuth 2.0 scopes, not replacement. This means: +The middleware supports two modes for handling OAuth 2.0 scopes, controlled by the `overrideScopes` parameter: + +#### Default Append Mode (`overrideScopes: false`) + +By default, the middleware uses an **append** behavior for OAuth 2.0 scopes: - **Default scopes** are always included: `["openid", "profile", "email"]` - **User-provided scopes** are appended to the defaults with automatic deduplication - The final scope list maintains the order: defaults first, then user scopes -#### Examples: +#### Override Mode (`overrideScopes: true`) -**No custom scopes specified:** +When `overrideScopes` is set to `true`, the middleware uses **replacement** behavior: + +- Default scopes are **not** automatically included +- Only the scopes explicitly provided in the `scopes` field are used +- You must include all required scopes explicitly, including `openid` if needed + +### Examples: + +**Default behavior (no custom scopes):** ```yaml # No scopes field specified # Result: ["openid", "profile", "email"] ``` -**Custom scopes appended:** +**Default append behavior:** ```yaml scopes: - roles @@ -107,7 +120,7 @@ scopes: # Result: ["openid", "profile", "email", "roles", "custom_scope"] ``` -**Overlapping scopes (automatic deduplication):** +**Overlapping scopes with append (automatic deduplication):** ```yaml scopes: - openid # Duplicate - will be deduplicated @@ -117,13 +130,30 @@ scopes: # Result: ["openid", "profile", "email", "roles", "permissions"] ``` -**Empty scopes list:** +**Using override mode:** +```yaml +overrideScopes: true +scopes: + - openid + - profile + - custom_scope +# Result: ["openid", "profile", "custom_scope"] +``` + +**Empty scopes list with default behavior:** ```yaml scopes: [] # Result: ["openid", "profile", "email"] ``` -This append behavior ensures that essential OIDC scopes are always present while allowing you to add provider-specific or application-specific scopes as needed. +**Empty scopes list with override mode:** +```yaml +overrideScopes: true +scopes: [] +# Result: [] (Warning: empty scopes may cause authentication to fail) +``` + +The default append behavior ensures essential OIDC scopes are always present, while the override mode gives you complete control over the exact scopes requested from the provider. ## Usage Examples diff --git a/main.go b/main.go index f82ad68..290c7ba 100644 --- a/main.go +++ b/main.go @@ -240,6 +240,7 @@ type TraefikOidc struct { shutdownOnce sync.Once forceHTTPS bool enablePKCE bool + overrideScopes bool suppressDiagnosticLogs bool } @@ -652,14 +653,20 @@ func New(ctx context.Context, next http.Handler, config *Config, name string) (h } return config.PostLogoutRedirectURI }(), - tokenBlacklist: cacheManager.GetSharedTokenBlacklist(), - jwkCache: cacheManager.GetSharedJWKCache(), - metadataCache: cacheManager.GetSharedMetadataCache(), - clientID: config.ClientID, - clientSecret: config.ClientSecret, - forceHTTPS: config.ForceHTTPS, - enablePKCE: config.EnablePKCE, - scopes: mergeScopes([]string{"openid", "profile", "email"}, config.Scopes), + tokenBlacklist: cacheManager.GetSharedTokenBlacklist(), + jwkCache: cacheManager.GetSharedJWKCache(), + metadataCache: cacheManager.GetSharedMetadataCache(), + clientID: config.ClientID, + clientSecret: config.ClientSecret, + forceHTTPS: config.ForceHTTPS, + enablePKCE: config.EnablePKCE, + overrideScopes: config.OverrideScopes, + scopes: func() []string { + if config.OverrideScopes { + return append([]string(nil), config.Scopes...) + } + return mergeScopes([]string{"openid", "profile", "email"}, config.Scopes) + }(), limiter: rate.NewLimiter(rate.Every(time.Second), config.RateLimit), tokenCache: cacheManager.GetSharedTokenCache(), httpClient: httpClient, diff --git a/scopes_test.go b/scopes_test.go new file mode 100644 index 0000000..2ba6837 --- /dev/null +++ b/scopes_test.go @@ -0,0 +1,124 @@ +package traefikoidc + +import ( + "reflect" + "testing" +) + +func TestMergeScopes(t *testing.T) { + testCases := []struct { + name string + defaultScopes []string + userScopes []string + expectedScopes []string + }{ + { + name: "Empty user scopes", + defaultScopes: []string{"openid", "profile", "email"}, + userScopes: []string{}, + expectedScopes: []string{"openid", "profile", "email"}, + }, + { + name: "Non-overlapping scopes", + defaultScopes: []string{"openid", "profile", "email"}, + userScopes: []string{"roles", "custom_scope"}, + expectedScopes: []string{"openid", "profile", "email", "roles", "custom_scope"}, + }, + { + name: "Overlapping scopes", + defaultScopes: []string{"openid", "profile", "email"}, + userScopes: []string{"openid", "roles", "profile", "permissions"}, + expectedScopes: []string{"openid", "profile", "email", "roles", "permissions"}, + }, + { + name: "Nil user scopes", + defaultScopes: []string{"openid", "profile", "email"}, + userScopes: nil, + expectedScopes: []string{"openid", "profile", "email"}, + }, + { + name: "Nil default scopes", + defaultScopes: nil, + userScopes: []string{"roles", "custom_scope"}, + expectedScopes: []string{"roles", "custom_scope"}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + result := mergeScopes(tc.defaultScopes, tc.userScopes) + if !reflect.DeepEqual(result, tc.expectedScopes) { + t.Errorf("Expected %v, got %v", tc.expectedScopes, result) + } + }) + } +} + +func TestScopesConfiguration(t *testing.T) { + defaultScopes := []string{"openid", "profile", "email"} + userScopes := []string{"roles", "custom_scope"} + + t.Run("Default Append Behavior", func(t *testing.T) { + // Create config with user scopes but overrideScopes=false + config := &Config{ + Scopes: userScopes, + OverrideScopes: false, + } + + // Simulate middleware initialization + var result []string + if config.OverrideScopes { + result = append([]string(nil), config.Scopes...) + } else { + result = mergeScopes(defaultScopes, config.Scopes) + } + + // Expect defaultScopes + userScopes with deduplication + expectedScopes := []string{"openid", "profile", "email", "roles", "custom_scope"} + if !reflect.DeepEqual(result, expectedScopes) { + t.Errorf("Expected %v, got %v", expectedScopes, result) + } + }) + + t.Run("Override Behavior", func(t *testing.T) { + // Create config with user scopes and overrideScopes=true + config := &Config{ + Scopes: userScopes, + OverrideScopes: true, + } + + // Simulate middleware initialization + var result []string + if config.OverrideScopes { + result = append([]string(nil), config.Scopes...) + } else { + result = mergeScopes(defaultScopes, config.Scopes) + } + + // Expect only userScopes + if !reflect.DeepEqual(result, userScopes) { + t.Errorf("Expected %v, got %v", userScopes, result) + } + }) + + t.Run("Empty Scopes with Override", func(t *testing.T) { + // Create config with empty scopes and overrideScopes=true + config := &Config{ + Scopes: []string{}, + OverrideScopes: true, + } + + // Simulate middleware initialization + var result []string + if config.OverrideScopes { + result = append([]string(nil), config.Scopes...) + } else { + result = mergeScopes(defaultScopes, config.Scopes) + } + + // Expect empty scopes - check length instead of DeepEqual + if len(result) != 0 { + t.Errorf("Expected empty slice, got %v with length %d", result, len(result)) + } + }) +} diff --git a/settings.go b/settings.go index 3dca6d2..49eca6e 100644 --- a/settings.go +++ b/settings.go @@ -47,6 +47,7 @@ type Config struct { RefreshGracePeriodSeconds int `json:"refreshGracePeriodSeconds"` ForceHTTPS bool `json:"forceHTTPS"` EnablePKCE bool `json:"enablePKCE"` + OverrideScopes bool `json:"overrideScopes"` } const ( @@ -87,6 +88,7 @@ func CreateConfig() *Config { RateLimit: DefaultRateLimit, ForceHTTPS: true, // Secure by default EnablePKCE: false, // PKCE is opt-in + OverrideScopes: false, // Default to appending scopes, not overriding RefreshGracePeriodSeconds: 60, // Default grace period of 60 seconds } diff --git a/settings_test.go b/settings_test.go index edafc01..6758caa 100644 --- a/settings_test.go +++ b/settings_test.go @@ -49,6 +49,11 @@ func TestCreateConfig(t *testing.T) { if !config.ForceHTTPS { t.Error("Expected ForceHTTPS to be true by default") } + + // Check OverrideScopes default + if config.OverrideScopes { + t.Error("Expected OverrideScopes to be false by default") + } }) t.Run("Config Can Hold Custom Values", func(t *testing.T) { @@ -57,6 +62,7 @@ func TestCreateConfig(t *testing.T) { config.LogLevel = "debug" config.RateLimit = 50 config.ForceHTTPS = false + config.OverrideScopes = true // Verify config struct can hold custom values if len(config.Scopes) != 1 || config.Scopes[0] != "custom_scope" { @@ -71,6 +77,9 @@ func TestCreateConfig(t *testing.T) { if config.ForceHTTPS { t.Error("Config struct cannot hold custom ForceHTTPS value") } + if !config.OverrideScopes { + t.Error("Config struct cannot hold custom OverrideScopes value") + } }) }