From da9c21d2a6aad8d2f2f6e1dabdcde05228610353 Mon Sep 17 00:00:00 2001 From: Lukasz Raczylo Date: Thu, 13 Aug 2026 15:23:13 +0100 Subject: [PATCH] R51: guarantee openid scope in buildAuthURL when filter drops it --- url_helpers.go | 10 +++++++++ url_helpers_ultra_test.go | 45 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/url_helpers.go b/url_helpers.go index 3d3f649..341ec28 100644 --- a/url_helpers.go +++ b/url_helpers.go @@ -163,6 +163,16 @@ func (t *TraefikOidc) buildAuthURL(redirectURL, state, nonce, codeChallenge stri if t.scopeFilter != nil && len(scopesSupported) > 0 { scopes = t.scopeFilter.FilterSupportedScopes(scopes, scopesSupported, t.providerURL) t.logger.Debugf("TraefikOidc.buildAuthURL: After final filtering: %v", scopes) + + // OIDC Core §3.1.2.1 requires the 'openid' scope for an authorization + // (ID-token) request, even when scopes_supported is optional and happens + // to omit it. FilterSupportedScopes only restores openid when EVERY + // requested scope is dropped, so openid can be absent here if the + // provider omits openid from scopes_supported while leaving other + // configured scopes supported. Guarantee its presence. (When the + // filter is inactive — no discovery scopes_supported — an explicit + // operator scope override, e.g. omitting openid, is respected as-is.) + scopes = t.scopeFilter.EnsureOpenIDScope(scopes) } if len(scopes) > 0 { diff --git a/url_helpers_ultra_test.go b/url_helpers_ultra_test.go index 5bcd231..c028653 100644 --- a/url_helpers_ultra_test.go +++ b/url_helpers_ultra_test.go @@ -605,3 +605,48 @@ func TestBuildAuthURLExtraAuthParams(t *testing.T) { assert.Contains(t, authURL, "response_type=code") }) } + +func extractScopeParam(t *testing.T, authURL string) string { + u, err := url.Parse(authURL) + require.NoError(t, err) + return u.Query().Get("scope") +} + +// Review regression: buildAuthURL must guarantee the mandatory OIDC 'openid' +// scope even when the provider's scopes_supported omits it whilst leaving +// other configured scopes supported (FilterSupportedScopes only restores +// openid when EVERY scope is dropped). +func TestBuildAuthURLGuaranteesOpenIDScope(t *testing.T) { + t.Run("openid omitted from scopes_supported but other scopes kept", func(t *testing.T) { + m := createMinimalMiddleware() + m.providerURL = "https://provider.example.com" + m.scopes = []string{"email", "profile"} // no openid configured + m.scopesSupported = []string{"email", "profile"} // provider omits openid + m.scopeFilter = NewScopeFilter(newNoOpLogger()) + + scope := extractScopeParam(t, m.buildAuthURL("https://app.com/callback", "state123", "nonce456", "")) + assert.NotEmpty(t, scope) + assert.Contains(t, scope, "openid", "buildAuthURL must include the mandatory openid scope") + }) + + t.Run("openid dropped by filter when provider omits it", func(t *testing.T) { + m := createMinimalMiddleware() + m.providerURL = "https://provider.example.com" + m.scopes = []string{"openid", "email"} + m.scopesSupported = []string{"email"} // provider omits openid -> filter would drop it + m.scopeFilter = NewScopeFilter(newNoOpLogger()) + + scope := extractScopeParam(t, m.buildAuthURL("https://app.com/callback", "state123", "nonce456", "")) + assert.Contains(t, scope, "openid", "openid dropped by scope filter must be restored") + }) + + t.Run("openid preserved when no scope filter configured", func(t *testing.T) { + m := createMinimalMiddleware() + m.providerURL = "https://provider.example.com" + m.scopes = []string{"openid", "email"} + m.scopesSupported = []string{"email"} + + scope := extractScopeParam(t, m.buildAuthURL("https://app.com/callback", "state123", "nonce456", "")) + assert.Contains(t, scope, "openid") + }) +}