fix(auth): cache OIDC end-session endpoint at init so logout never blocks on discovery

EndSessionEndpoint() previously called setOicdProvider() (which retries with
backoff) and re-unmarshalled the discovery document on every logout. Read the
end_session_endpoint once at provider init, store it on the provider, and make
the accessor fail-fast (cached value, else the static logouturl) so /user/logout
stays responsive even when the OP is unreachable.
This commit is contained in:
kolaente 2026-06-19 17:23:32 +02:00
parent 970f3c3733
commit 37cd45d565
4 changed files with 74 additions and 11 deletions

View File

@ -24,16 +24,28 @@ import (
"code.vikunja.io/api/pkg/models"
)
// EndSessionEndpoint returns the provider's RP-Initiated Logout endpoint as
// published in its OpenID Connect discovery document (the REQUIRED
// EndSessionEndpoint returns the provider's RP-Initiated Logout endpoint, read
// from the discovery document once at init time (EndSessionURL, the REQUIRED
// `end_session_endpoint` metadata, RP-Initiated Logout 1.0 §2.1). When the
// provider does not publish one, it falls back to the statically configured
// `logouturl` so existing setups keep working.
//
// It deliberately never triggers discovery: logout must stay responsive even
// when the OP is unreachable (e.g. right after an API restart, before the
// provider has been re-discovered).
func (p *Provider) EndSessionEndpoint() string {
if p.EndSessionURL != "" {
return p.EndSessionURL
}
return p.LogoutURL
}
// discoveredEndSessionEndpoint reads the `end_session_endpoint` from the already
// fetched discovery document. The discovery JSON is cached on the *oidc.Provider
// by oidc.NewProvider, so Claims only unmarshals in memory and makes no request.
func (p *Provider) discoveredEndSessionEndpoint() string {
if p.openIDProvider == nil {
if err := p.setOicdProvider(); err != nil {
return p.LogoutURL
}
return ""
}
var meta struct {
@ -41,10 +53,7 @@ func (p *Provider) EndSessionEndpoint() string {
}
if err := p.openIDProvider.Claims(&meta); err != nil {
log.Debugf("Could not read end_session_endpoint for provider %s: %v", p.Key, err)
return p.LogoutURL
}
if meta.EndSessionEndpoint == "" {
return p.LogoutURL
return ""
}
return meta.EndSessionEndpoint
}

View File

@ -131,6 +131,53 @@ func TestBuildEndSessionURLFromDiscovery(t *testing.T) {
assert.Equal(t, "client1", q.Get("client_id"))
}
func TestEndSessionEndpointUsesCachedURLWithoutDiscovery(t *testing.T) {
// A nil openIDProvider models a provider restored from the keyvalue cache
// (or one whose OP is currently unreachable). EndSessionEndpoint must answer
// from the cached EndSessionURL and never attempt discovery, so logout stays
// responsive.
p := &Provider{
Key: "provider1",
LogoutURL: "https://op.example.com/static-logout",
EndSessionURL: "https://op.example.com/end-session",
}
assert.Equal(t, "https://op.example.com/end-session", p.EndSessionEndpoint())
}
func TestEndSessionEndpointFallsBackToLogoutURLWhenNotCached(t *testing.T) {
p := &Provider{
Key: "provider1",
LogoutURL: "https://op.example.com/static-logout",
}
assert.Equal(t, "https://op.example.com/static-logout", p.EndSessionEndpoint())
}
func TestEndSessionEndpointCachedFromDiscoveryOnInit(t *testing.T) {
defer CleanupSavedOpenIDProviders()
server := newMockOIDCServerWithEndSession()
defer server.Close()
config.AuthOpenIDEnabled.Set(true)
config.AuthOpenIDProviders.Set(map[string]interface{}{
"provider1": map[string]interface{}{
"name": "Provider One",
"authurl": server.URL,
"clientid": "client1",
"clientsecret": "secret1",
},
})
_ = keyvalue.Del("openid_providers")
_ = keyvalue.Del("openid_provider_provider1")
provider, err := GetProvider("provider1")
require.NoError(t, err)
require.NotNil(t, provider)
assert.Equal(t, server.URL+"/logout", provider.EndSessionURL)
assert.Equal(t, server.URL+"/logout", provider.EndSessionEndpoint())
}
func TestEndSessionEndpointFallsBackToStaticLogoutURL(t *testing.T) {
defer CleanupSavedOpenIDProviders()

View File

@ -69,8 +69,13 @@ type Provider struct {
ForceUserInfo bool `json:"force_user_info"`
RequireAvailability bool `json:"-"`
ClientSecret string `json:"-"`
openIDProvider *oidc.Provider
Oauth2Config *oauth2.Config `json:"-"`
// EndSessionURL is the provider's RP-Initiated Logout endpoint, read once
// from the discovery document at init time so logout never triggers a fetch.
// Exported so it survives the gob-encoded redis keyvalue round-trip (gob
// skips unexported fields, like openIDProvider).
EndSessionURL string `json:"-"`
openIDProvider *oidc.Provider
Oauth2Config *oauth2.Config `json:"-"`
}
type claims struct {

View File

@ -313,6 +313,8 @@ func getProviderFromMap(pi map[string]interface{}, key string) (provider *Provid
provider.AuthURL = provider.Oauth2Config.Endpoint.AuthURL
provider.EndSessionURL = provider.discoveredEndSessionEndpoint()
return
}