mirror of
https://github.com/lukaszraczylo/traefikoidc.git
synced 2026-10-12 04:04:26 +00:00
R51: guarantee openid scope in buildAuthURL when filter drops it
This commit is contained in:
@@ -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 {
|
||||
|
||||
@@ -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")
|
||||
})
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user