Ensure that appended roles are unique. Update the documentation.

.traefik.yml

@@ -35,11 +35,8 @@ testData:
   logoutURL: /oauth2/logout # Path for handling logout requests (if not provided, it will be set to callbackURL + "/logout")
   postLogoutRedirectURI: /oidc/different-logout # URL to redirect to after logout (default: "/")
 
-  scopes: # OAuth 2.0 scopes to request (default: ["openid", "email", "profile"])
+  scopes: # Additional scopes to append to defaults ["openid", "profile", "email"]
-    - openid
+    - roles # Result: ["openid", "profile", "email", "roles"]
-    - email
-    - profile
-    - roles # Include this to get role information from the provider
 
   allowedUserDomains: # Restricts access to specific email domains (if not provided, relies on OIDC provider)
     - company.com
@@ -153,11 +150,15 @@ configuration:
   scopes:
     type: array
     description: |
-      The OAuth 2.0 scopes to request from the OIDC provider.
+      Additional OAuth 2.0 scopes to append to the default scopes.
-      Default: ["openid", "profile", "email"]
+      Default scopes are always included: ["openid", "profile", "email"]
+      
+      User-provided scopes are appended to defaults with automatic deduplication.
+      For example, specifying ["roles", "custom_scope"] results in:
+      ["openid", "profile", "email", "roles", "custom_scope"]
 
       Include "roles" or similar scope if you need role/group information.
-      Note: For Google OAuth, the middleware automatically handles the 
+      Note: For Google OAuth, the middleware automatically handles the
       proper authentication parameters and does NOT require the "offline_access"
       scope (which Google rejects as invalid). See documentation for details.
     required: false

README.md

@@ -67,7 +67,7 @@ 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` | The OAuth 2.0 scopes to request | `["openid", "profile", "email"]` | `["openid", "email", "profile", "roles"]` |
+| `scopes` | Additional OAuth 2.0 scopes to append to defaults | `["openid", "profile", "email"]` (always included) | `["roles", "custom_scope"]` (appended to defaults) |
 | `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` |
@@ -81,6 +81,50 @@ The middleware supports the following configuration options:
 | `refreshGracePeriodSeconds` | Seconds before token expiry to attempt proactive refresh | `60` | `120` |
 | `headers` | Custom HTTP headers with templates that can access OIDC claims and tokens | none | See "Templated Headers" section |
 
+## Scope Configuration
+
+### Scope Append Behavior
+
+The middleware uses an **append** behavior for OAuth 2.0 scopes, not replacement. This means:
+
+- **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:
+
+**No custom scopes specified:**
+```yaml
+# No scopes field specified
+# Result: ["openid", "profile", "email"]
+```
+
+**Custom scopes appended:**
+```yaml
+scopes:
+  - roles
+  - custom_scope
+# Result: ["openid", "profile", "email", "roles", "custom_scope"]
+```
+
+**Overlapping scopes (automatic deduplication):**
+```yaml
+scopes:
+  - openid      # Duplicate - will be deduplicated
+  - roles
+  - profile     # Duplicate - will be deduplicated
+  - permissions
+# Result: ["openid", "profile", "email", "roles", "permissions"]
+```
+
+**Empty scopes list:**
+```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.
+
 ## Usage Examples
 
 ### Basic Configuration
@@ -101,9 +145,7 @@ spec:
       callbackURL: /oauth2/callback
       logoutURL: /oauth2/logout
       scopes:
-        - openid
+        - roles  # Appended to defaults: ["openid", "profile", "email", "roles"]
-        - email
-        - profile
 ```
 
 ### With Excluded URLs (Public Access Paths)
@@ -124,9 +166,7 @@ spec:
       callbackURL: /oauth2/callback
       logoutURL: /oauth2/logout
       scopes:
-        - openid
+        - roles  # Appended to defaults: ["openid", "profile", "email", "roles"]
-        - email
-        - profile
       excludedURLs:
         - /login        # covers /login, /login/me, /login/reminder etc.
         - /public-data
@@ -152,9 +192,7 @@ spec:
       callbackURL: /oauth2/callback
       logoutURL: /oauth2/logout
       scopes:
-        - openid
+        - roles  # Appended to defaults: ["openid", "profile", "email", "roles"]
-        - email
-        - profile
       allowedUserDomains:
         - company.com
         - subsidiary.com
@@ -178,9 +216,7 @@ spec:
       callbackURL: /oauth2/callback
       logoutURL: /oauth2/logout
       scopes:
-        - openid
+        - roles  # Appended to defaults: ["openid", "profile", "email", "roles"]
-        - email
-        - profile
       allowedUsers:
         - [email protected]
         - [email protected]
@@ -204,9 +240,7 @@ spec:
       callbackURL: /oauth2/callback
       logoutURL: /oauth2/logout
       scopes:
-        - openid
+        - roles  # Appended to defaults: ["openid", "profile", "email", "roles"]
-        - email
-        - profile
       allowedUserDomains:
         - company.com
       allowedUsers:
@@ -239,10 +273,7 @@ spec:
       callbackURL: /oauth2/callback
       logoutURL: /oauth2/logout
       scopes:
-        - openid
+        - roles  # Appended to defaults: ["openid", "profile", "email", "roles"]
-        - email
-        - profile
-        - roles     # Include this to get role information from the provider
       allowedRolesAndGroups:
         - admin
         - developer
@@ -269,9 +300,7 @@ spec:
       rateLimit: 500     # Requests per second (default: 100)
       forceHTTPS: false  # Default is true for security
       scopes:
-        - openid
+        - roles  # Appended to defaults: ["openid", "profile", "email", "roles"]
-        - email
-        - profile
 ```
 
 ### With Custom Post-Logout Redirect
@@ -293,9 +322,7 @@ spec:
       logoutURL: /oauth2/logout
       postLogoutRedirectURI: /logged-out-page  # Where to redirect after logout
       scopes:
-        - openid
+        - roles  # Appended to defaults: ["openid", "profile", "email", "roles"]
-        - email
-        - profile
 ```
 
 ### With Templated Headers
@@ -316,10 +343,7 @@ spec:
       callbackURL: /oauth2/callback
       logoutURL: /oauth2/logout
       scopes:
-        - openid
+        - roles  # Appended to defaults: ["openid", "profile", "email", "roles"]
-        - email
-        - profile
-        - roles
       headers:
         - name: "X-User-Email"
           value: "{{.Claims.email}}"
@@ -352,9 +376,7 @@ spec:
       logoutURL: /oauth2/logout
       enablePKCE: true  # Enables PKCE for added security
       scopes:
-        - openid
+        - roles  # Appended to defaults: ["openid", "profile", "email", "roles"]
-        - email
-        - profile
 ```
 
 ### Google OIDC Configuration Example
@@ -377,9 +399,7 @@ spec:
       callbackURL: /oauth2/callback                             # Adjust if needed
       logoutURL: /oauth2/logout                                 # Optional: Adjust if needed
       scopes:
-        - openid
+        - roles  # Appended to defaults: ["openid", "profile", "email", "roles"]
-        - email
-        - profile
         # Note: DO NOT manually add offline_access scope for Google
         # The middleware automatically handles Google-specific requirements
       refreshGracePeriodSeconds: 300  # Optional: Start refresh 5 min before expiry (default 60)
@@ -408,9 +428,7 @@ spec:
       callbackURL: /oauth2/callback
       logoutURL: /oauth2/logout
       scopes:
-        - openid
+        - roles  # Appended to defaults: ["openid", "profile", "email", "roles"]
-        - email
-        - profile
 ```
 
 Don't forget to create the secret:
@@ -509,9 +527,7 @@ http:
           postLogoutRedirectURI: /logged-out-page
           sessionEncryptionKey: potato-secret-is-at-least-32-bytes-long
           scopes:
-            - openid
+            - roles  # Appended to defaults: ["openid", "profile", "email", "roles"]
-            - email
-            - profile
           allowedUserDomains:
             - company.com
           allowedUsers:

main_test.go

@@ -3647,6 +3647,211 @@ func TestScopeMerging(t *testing.T) {
 	}
 }
 
+// TestScopeMergingEdgeCases tests additional edge cases for scope deduplication
+func TestScopeMergingEdgeCases(t *testing.T) {
+	// Helper function to compare string slices
+	equalSlices := func(a, b []string) bool {
+		if len(a) != len(b) {
+			return false
+		}
+		for i, v := range a {
+			if v != b[i] {
+				return false
+			}
+		}
+		return true
+	}
+
+	tests := []struct {
+		name           string
+		defaultScopes  []string
+		userScopes     []string
+		expectedScopes []string
+		description    string
+	}{
+		{
+			name:           "Case sensitivity preserved",
+			defaultScopes:  []string{"openid", "profile", "email"},
+			userScopes:     []string{"OpenID", "PROFILE", "custom"},
+			expectedScopes: []string{"openid", "profile", "email", "OpenID", "PROFILE", "custom"},
+			description:    "OAuth scopes are case-sensitive, so different cases should be preserved",
+		},
+		{
+			name:           "Empty strings in user scopes",
+			defaultScopes:  []string{"openid", "profile", "email"},
+			userScopes:     []string{"", "custom", "", "another"},
+			expectedScopes: []string{"openid", "profile", "email", "", "custom", "another"},
+			description:    "Empty strings should be preserved (though invalid in OAuth)",
+		},
+		{
+			name:           "Whitespace scopes",
+			defaultScopes:  []string{"openid", "profile", "email"},
+			userScopes:     []string{" ", "custom", "  ", "another"},
+			expectedScopes: []string{"openid", "profile", "email", " ", "custom", "  ", "another"},
+			description:    "Whitespace-only scopes should be preserved as distinct",
+		},
+		{
+			name:          "Large number of scopes",
+			defaultScopes: []string{"openid", "profile", "email"},
+			userScopes:    generateLargeUserScopes(),
+			expectedScopes: func() []string {
+				// Manually calculate expected result with proper deduplication
+				defaults := []string{"openid", "profile", "email"}
+				userScopes := generateLargeUserScopes()
+				return mergeScopes(defaults, userScopes)
+			}(),
+			description: "Performance test with larger scope lists",
+		},
+		{
+			name:           "Complex OAuth scopes with special characters",
+			defaultScopes:  []string{"openid", "profile", "email"},
+			userScopes:     []string{"read:users", "write:users", "admin:*", "scope/with/slashes", "scope-with-dashes"},
+			expectedScopes: []string{"openid", "profile", "email", "read:users", "write:users", "admin:*", "scope/with/slashes", "scope-with-dashes"},
+			description:    "Real-world OAuth scopes with colons, slashes, and special characters",
+		},
+		{
+			name:           "Duplicate defaults in user scopes multiple times",
+			defaultScopes:  []string{"openid", "profile", "email"},
+			userScopes:     []string{"openid", "profile", "openid", "custom", "email", "profile", "custom"},
+			expectedScopes: []string{"openid", "profile", "email", "custom"},
+			description:    "Multiple duplicates of default scopes should be completely deduplicated",
+		},
+		{
+			name:           "All user scopes are duplicates of defaults",
+			defaultScopes:  []string{"openid", "profile", "email"},
+			userScopes:     []string{"email", "openid", "profile", "openid"},
+			expectedScopes: []string{"openid", "profile", "email"},
+			description:    "When all user scopes duplicate defaults, result should be just defaults",
+		},
+		{
+			name:           "Single scope scenarios",
+			defaultScopes:  []string{"openid"},
+			userScopes:     []string{"custom"},
+			expectedScopes: []string{"openid", "custom"},
+			description:    "Minimal case with single scopes",
+		},
+		{
+			name:           "Identical scopes in same order",
+			defaultScopes:  []string{"openid", "profile", "email"},
+			userScopes:     []string{"openid", "profile", "email"},
+			expectedScopes: []string{"openid", "profile", "email"},
+			description:    "When user scopes exactly match defaults, no duplication",
+		},
+		{
+			name:           "Identical scopes in different order",
+			defaultScopes:  []string{"openid", "profile", "email"},
+			userScopes:     []string{"email", "profile", "openid"},
+			expectedScopes: []string{"openid", "profile", "email"},
+			description:    "Order of defaults is preserved when user scopes are reordered duplicates",
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			// Test the mergeScopes function directly
+			result := mergeScopes(tc.defaultScopes, tc.userScopes)
+			if !equalSlices(result, tc.expectedScopes) {
+				t.Errorf("Expected %v, got %v\nDescription: %s", tc.expectedScopes, result, tc.description)
+			}
+		})
+	}
+}
+
+// generateLargeUserScopes creates a large list of user scopes for performance testing
+func generateLargeUserScopes() []string {
+	scopes := make([]string, 100)
+	for i := 0; i < 100; i++ {
+		scopes[i] = fmt.Sprintf("scope_%d", i)
+	}
+	// Add some duplicates to test deduplication performance
+	scopes = append(scopes, "scope_1", "scope_5", "scope_10", "openid") // Include a default duplicate
+	return scopes
+}
+
+// TestScopeMergingPerformance tests performance with large scope lists
+func TestScopeMergingPerformance(t *testing.T) {
+	// Create large scope lists
+	defaultScopes := []string{"openid", "profile", "email"}
+
+	// Create 1000 user scopes with some duplicates
+	userScopes := make([]string, 1000)
+	for i := 0; i < 1000; i++ {
+		if i%10 == 0 {
+			// Add some duplicates of defaults
+			userScopes[i] = defaultScopes[i%len(defaultScopes)]
+		} else if i%7 == 0 {
+			// Add some internal duplicates
+			userScopes[i] = fmt.Sprintf("scope_%d", i%50)
+		} else {
+			userScopes[i] = fmt.Sprintf("scope_%d", i)
+		}
+	}
+
+	// Measure performance
+	start := time.Now()
+	result := mergeScopes(defaultScopes, userScopes)
+	duration := time.Since(start)
+
+	// Verify result correctness
+	if len(result) < len(defaultScopes) {
+		t.Errorf("Result should contain at least the default scopes")
+	}
+
+	// Verify no duplicates exist
+	seen := make(map[string]bool)
+	for _, scope := range result {
+		if seen[scope] {
+			t.Errorf("Duplicate scope found in result: %s", scope)
+		}
+		seen[scope] = true
+	}
+
+	// Performance assertion (should be very fast)
+	if duration > time.Millisecond*10 {
+		t.Logf("Performance note: mergeScopes took %v for 1000+ scopes (still acceptable)", duration)
+	}
+
+	t.Logf("Performance: processed %d user scopes in %v, result has %d unique scopes",
+		len(userScopes), duration, len(result))
+}
+
+// TestScopeMergingMemoryEfficiency tests memory efficiency of the mergeScopes function
+func TestScopeMergingMemoryEfficiency(t *testing.T) {
+	defaultScopes := []string{"openid", "profile", "email"}
+	userScopes := []string{"custom1", "custom2"}
+
+	// Test that the function doesn't modify input slices
+	originalDefaults := make([]string, len(defaultScopes))
+	copy(originalDefaults, defaultScopes)
+	originalUser := make([]string, len(userScopes))
+	copy(originalUser, userScopes)
+
+	result := mergeScopes(defaultScopes, userScopes)
+
+	// Verify input slices are unchanged
+	for i, scope := range defaultScopes {
+		if scope != originalDefaults[i] {
+			t.Errorf("Default scopes were modified: expected %s, got %s", originalDefaults[i], scope)
+		}
+	}
+	for i, scope := range userScopes {
+		if scope != originalUser[i] {
+			t.Errorf("User scopes were modified: expected %s, got %s", originalUser[i], scope)
+		}
+	}
+
+	// Verify result is independent
+	result[0] = "modified"
+	if defaultScopes[0] == "modified" {
+		t.Error("Modifying result affected input defaults")
+	}
+
+	expectedLength := len(defaultScopes) + len(userScopes)
+	if len(result) != expectedLength {
+		t.Errorf("Expected result length %d, got %d", expectedLength, len(result))
+	}
+}
+
 // TestNewWithScopeAppending tests that the New function properly merges scopes
 func TestNewWithScopeAppending(t *testing.T) {
 	// Create mock provider metadata server