kodev-org
/
vikunja
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

docs(auth): trim verbose comments on OIDC logout to the non-obvious why

This commit is contained in:
kolaente 2026-06-19 18:06:49 +02:00
parent 6f4a21ddad
commit e321bc8939
11 changed files with 48 additions and 98 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
frontend/src/stores/auth.ts
View File

@ -533,8 +533,6 @@ export const useAuthStore = defineStore('auth', () => {
// Revoke the server session so the refresh token can't be reused.
// Best-effort: if the network call fails, still clean up locally.
// The server builds the OIDC RP-Initiated Logout URL (with id_token_hint,
// post_logout_redirect_uri and client_id) and returns it here.
let oidcLogoutUrl = ''
try {
const HTTP = AuthenticatedHTTPFactory()
@ -551,10 +549,8 @@ export const useAuthStore = defineStore('auth', () => {
await router.push({name: 'user.login'})
await checkAuth()
// Redirect to the OIDC provider's end-session endpoint so the provider
// session is ended too. Prefer the server-built URL (RP-Initiated Logout
// with id_token_hint), falling back to the static logout url if the
// server did not return one.
// Redirect to the OIDC provider to end its session too. Prefer the
// server-built RP-Initiated Logout URL, falling back to the static one.
if (oidcLogoutUrl) {
window.location.href = oidcLogoutUrl
return

4
pkg/migration/20260619155410.go
View File

@ -23,9 +23,7 @@ import (
"xorm.io/xorm"
)
// Mirrors models.Session; the two new columns store what is needed to build an
// RP-Initiated Logout request (id_token_hint + which provider to log out from)
// when the session was created via OpenID Connect.
// Mirrors models.Session; adds the two columns RP-Initiated Logout needs.
type sessionOIDCLogout20260619155410 struct {
ID string `xorm:"varchar(36) not null unique pk"`
UserID int64 `xorm:"bigint not null index"`

15
pkg/models/sessions.go
View File

@ -49,12 +49,9 @@ type Session struct {
IPAddress string `xorm:"varchar(100)" json:"ip_address" readOnly:"true" doc:"IP address captured from the login request."`
// Whether this is a "remember me" session (controls max refresh lifetime).
IsLongSession bool `xorm:"not null default false" json:"-"`
// The raw OpenID Connect ID token, kept only for sessions created via OIDC so
// it can be replayed as the id_token_hint in an RP-Initiated Logout request.
// Never exposed over the API. Empty for non-OIDC sessions.
// Raw OIDC ID token, kept so logout can replay it as id_token_hint. Empty for non-OIDC sessions.
OIDCIDToken string `xorm:"text" json:"-"`
// The key of the OIDC provider that created this session, used to look up its
// end-session endpoint at logout. Empty for non-OIDC sessions.
// OIDC provider that created this session, used to find its end-session endpoint at logout.
OIDCProviderKey string `xorm:"varchar(250)" json:"-"`
// When this session was last refreshed.
LastActive time.Time `xorm:"not null" json:"last_active" readOnly:"true" doc:"When this session was last refreshed."`
@ -88,9 +85,8 @@ func generateHashedToken() (rawToken, hash string, err error) {
return rawToken, HashSessionToken(rawToken), nil
}
// SessionOIDCData carries the OpenID Connect metadata persisted on a session so
// an RP-Initiated Logout request can be built later. It is nil for non-OIDC
// logins.
// SessionOIDCData carries the OIDC metadata persisted on a session so an
// RP-Initiated Logout request can be built later. Nil for non-OIDC logins.
type SessionOIDCData struct {
IDToken string
ProviderKey string
@ -98,8 +94,7 @@ type SessionOIDCData struct {
// CreateSession creates a new session record and generates a refresh token.
// Returns the session with RefreshToken populated (cleartext, shown only once).
// oidc is optional: pass it for OpenID Connect logins so the raw id_token and
// provider key are stored for RP-Initiated Logout; pass nil otherwise.
// Pass oidc for OpenID Connect logins to persist the logout data; nil otherwise.
func CreateSession(s *xorm.Session, userID int64, deviceInfo, ipAddress string, isLongSession bool, oidc *SessionOIDCData) (*Session, error) {
rawToken, hash, err := generateHashedToken()
if err != nil {

7
pkg/modules/auth/auth.go
View File

@ -112,9 +112,8 @@ type IssuedUserToken struct {
// IssueUserToken creates a session for the user and mints a JWT access token plus
// a refresh token for it. It is the transport-agnostic core both v1 (which writes
// the echo response) and v2 (Huma) call; callers set the refresh cookie and the
// Cache-Control header themselves via WriteUserAuthCookies. oidc is optional:
// pass it for OpenID Connect logins so the id_token is stored for RP-Initiated
// Logout, nil otherwise.
// Cache-Control header themselves via WriteUserAuthCookies. Pass oidc for
// OpenID Connect logins to store the logout data; nil otherwise.
func IssueUserToken(ctx context.Context, u *user.User, deviceInfo, ipAddress string, long bool, oidc *models.SessionOIDCData) (*IssuedUserToken, error) {
s := db.NewSession()
defer s.Close()
@ -163,7 +162,7 @@ func WriteUserAuthCookies(c *echo.Context, token *IssuedUserToken) {
}
// NewUserAuthTokenResponse creates a new user auth token response from a user object.
// oidc is optional OpenID Connect session metadata for RP-Initiated Logout; pass nil for local/LDAP logins.
// Pass oidc for OpenID Connect logins to store the logout data; nil otherwise.
func NewUserAuthTokenResponse(u *user.User, c *echo.Context, long bool, oidc *models.SessionOIDCData) error {
token, err := IssueUserToken(c.Request().Context(), u, c.Request().UserAgent(), c.RealIP(), long, oidc)
if err != nil {

47
pkg/modules/auth/openid/logout.go
View File

@ -24,15 +24,10 @@ import (
"code.vikunja.io/api/pkg/models"
)
// 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).
// EndSessionEndpoint returns the provider's RP-Initiated Logout endpoint
// (discovery's end_session_endpoint, cached at init), falling back to the static
// logouturl. Never triggers discovery so logout stays responsive when the OP is
// unreachable.
func (p *Provider) EndSessionEndpoint() string {
if p.EndSessionURL != "" {
return p.EndSessionURL
@ -40,9 +35,9 @@ func (p *Provider) EndSessionEndpoint() string {
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.
// discoveredEndSessionEndpoint reads end_session_endpoint from the discovery
// document already cached on the *oidc.Provider, so Claims unmarshals in memory
// without a request.
func (p *Provider) discoveredEndSessionEndpoint() string {
if p.openIDProvider == nil {
return ""
@ -58,25 +53,14 @@ func (p *Provider) discoveredEndSessionEndpoint() string {
return meta.EndSessionEndpoint
}
// BuildEndSessionURL constructs an OpenID Connect RP-Initiated Logout 1.0 request
// URL for the given provider key and stored session OIDC data.
//
// Per RP-Initiated Logout 1.0 §2 it appends:
// - id_token_hint: the ID token previously issued to this session. RECOMMENDED;
// it lets the OP skip the logout-confirmation prompt and is what makes the OP
// honor post_logout_redirect_uri (the OP MAY require it, §3).
// - post_logout_redirect_uri: where the OP redirects the user agent after
// logout. MUST be pre-registered with the OP. Defaults to service.publicurl
// (the Vikunja frontend) so the user lands back on Vikunja's login page.
// - client_id: the RP's client identifier (§2). Always sent; the OP verifies it
// matches the one in id_token_hint.
//
// It returns "" (and the caller skips the redirect) when neither an
// BuildEndSessionURL builds an OpenID Connect RP-Initiated Logout 1.0 request URL
// (id_token_hint + post_logout_redirect_uri + client_id; see RP-Initiated Logout
// 1.0 §2). post_logout_redirect_uri defaults to service.publicurl, and the OP
// only honors it when id_token_hint is present. Returns "" when neither an
// end_session_endpoint nor a static logouturl is configured.
func BuildEndSessionURL(providerKey string, oidc *models.SessionOIDCData) (string, error) {
// Read the cached provider rather than GetProvider: logout must never trigger
// OIDC discovery (a live HTTP GET that retries/blocks when the OP is down), and
// the static EndSessionURL/LogoutURL/ClientID needed here are already cached.
// GetProvider would trigger OIDC discovery (a live HTTP GET that blocks when
// the OP is down); the cached static fields are all logout needs.
provider, err := getCachedProvider(providerKey)
if err != nil {
return "", err
@ -98,9 +82,8 @@ func BuildEndSessionURL(providerKey string, oidc *models.SessionOIDCData) (strin
)
}
// buildEndSessionURL assembles the RP-Initiated Logout query string onto the
// given end-session endpoint. Empty optional params are omitted. Returns "" when
// no endpoint is configured.
// buildEndSessionURL appends the logout query params onto endpoint, omitting
// empty ones, and returns "" for an empty endpoint.
func buildEndSessionURL(endpoint, clientID, idToken, postLogoutRedirectURI string) (string, error) {
if endpoint == "" {
return "", nil

18
pkg/modules/auth/openid/logout_test.go
View File

@ -31,8 +31,8 @@ import (
"github.com/stretchr/testify/require"
)
// newMockOIDCServerWithEndSession serves a discovery document that includes an
// end_session_endpoint, exercising the RP-Initiated Logout discovery path.
// newMockOIDCServerWithEndSession publishes a discovery document with an
// end_session_endpoint.
func newMockOIDCServerWithEndSession() *httptest.Server {
var server *httptest.Server
mux := http.NewServeMux()
@ -137,9 +137,8 @@ func TestBuildEndSessionURLFromCachedProviderWithoutLiveObject(t *testing.T) {
config.AuthOpenIDEnabled.Set(true)
config.ServicePublicURL.Set("https://vikunja.example.com/")
// Seed only the cached static fields, mimicking a provider restored from
// keyvalue whose OP is unreachable (no live openIDProvider). BuildEndSessionURL
// must build the logout URL from the cache without triggering discovery.
// Seed only the cached static fields (no live openIDProvider), mimicking a
// provider restored from keyvalue whose OP is unreachable.
_ = keyvalue.Del("openid_providers")
require.NoError(t, keyvalue.Put("openid_provider_provider1", &Provider{
Key: "provider1",
@ -163,10 +162,8 @@ func TestBuildEndSessionURLFromCachedProviderWithoutLiveObject(t *testing.T) {
}
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.
// A nil openIDProvider models a provider restored from cache (or an
// unreachable OP): EndSessionEndpoint must answer from the cached URL.
p := &Provider{
Key: "provider1",
LogoutURL: "https://op.example.com/static-logout",
@ -212,8 +209,7 @@ func TestEndSessionEndpointCachedFromDiscoveryOnInit(t *testing.T) {
func TestEndSessionEndpointFallsBackToStaticLogoutURL(t *testing.T) {
defer CleanupSavedOpenIDProviders()
// This mock server publishes no end_session_endpoint, so the provider must
// fall back to the statically configured logouturl.
// newMockOIDCServer publishes no end_session_endpoint, forcing the logouturl fallback.
server := newMockOIDCServer()
defer server.Close()

10
pkg/modules/auth/openid/openid.go
View File

@ -69,10 +69,9 @@ type Provider struct {
ForceUserInfo bool `json:"force_user_info"`
RequireAvailability bool `json:"-"`
ClientSecret string `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).
// RP-Initiated Logout endpoint, cached at init so logout never fetches.
// Exported so it survives the gob keyvalue round-trip (gob skips unexported
// fields like openIDProvider); json:"-" keeps it out of /info.
EndSessionURL string `json:"-"`
openIDProvider *oidc.Provider
Oauth2Config *oauth2.Config `json:"-"`
@ -210,8 +209,7 @@ func AuthenticateCallback(ctx context.Context, cb *Callback, providerKey string)
return nil, nil, err
}
// Stored so logout can replay it as id_token_hint in an RP-Initiated Logout
// request. See pkg/modules/auth/openid/logout.go.
// Stored so logout can replay it as id_token_hint in an RP-Initiated Logout.
oidcData := &models.SessionOIDCData{
IDToken: rawIDToken,
ProviderKey: providerKey,

6
pkg/modules/auth/openid/providers.go
View File

@ -180,10 +180,8 @@ func GetProvider(key string) (provider *Provider, err error) {
return
}
// getCachedProvider returns the provider from keyvalue WITHOUT re-establishing the
// live OIDC connection (setOicdProvider). The logout path only needs the cached
// static fields (EndSessionURL/LogoutURL/ClientID) and must not block on an
// unreachable OP. The cache is populated at init / first GetAllProviders.
// getCachedProvider returns the provider from keyvalue without re-establishing
// the live OIDC connection, so the logout path never blocks on an unreachable OP.
func getCachedProvider(key string) (provider *Provider, err error) {
provider = &Provider{}
exists, err := keyvalue.GetWithValue("openid_provider_"+key, provider)

18
pkg/routes/api/shared/auth.go
View File

@ -197,13 +197,10 @@ func DeleteSession(sid string) error {
return err
}
// LogoutSession reads the session, builds an OpenID Connect RP-Initiated Logout
// URL when the session was created via OIDC, then deletes the session. It
// returns the end-session URL (empty for non-OIDC sessions or when no logout
// endpoint is configured) so the frontend can redirect the user agent to the
// identity provider's end_session_endpoint with id_token_hint and
// post_logout_redirect_uri. An empty sid is a no-op. The caller clears the
// refresh cookie.
// LogoutSession deletes the session and returns its OIDC RP-Initiated Logout URL
// for the frontend to redirect to (empty for non-OIDC sessions or when no logout
// endpoint is configured). An empty sid is a no-op. The caller clears the refresh
// cookie.
func LogoutSession(sid string) (endSessionURL string, err error) {
if sid == "" {
return "", nil
@ -212,8 +209,8 @@ func LogoutSession(sid string) (endSessionURL string, err error) {
s := db.NewSession()
defer s.Close()
// Read the session before deleting so the stored id_token can be replayed as
// id_token_hint. A missing session just means there is nothing to log out.
// Read before deleting so the stored id_token survives for the logout URL.
// A missing session just means there is nothing to log out.
session, err := models.GetSessionByID(s, sid)
if err != nil && !models.IsErrSessionNotFound(err) {
_ = s.Rollback()
@ -225,8 +222,7 @@ func LogoutSession(sid string) (endSessionURL string, err error) {
ProviderKey: session.OIDCProviderKey,
})
if buildErr != nil {
// Don't fail logout just because the logout URL could not be built;
// the session is still destroyed server-side below.
// A failed URL build must not block logout; the session is still deleted below.
log.Errorf("Could not build OIDC end-session URL for session %s: %v", sid, buildErr)
} else {
endSessionURL = url

7
pkg/routes/api/v1/login.go
View File

@ -150,14 +150,9 @@ func RefreshToken(c *echo.Context) (err error) {
return c.JSON(http.StatusOK, auth.Token{Token: result.AccessToken})
}
// LogoutResponse confirms a successful logout and, for sessions created via
// OpenID Connect, carries the provider's RP-Initiated Logout URL the frontend
// should redirect the user agent to so the OP session is ended too.
type LogoutResponse struct {
Message string `json:"message"`
// OIDCLogoutURL is the fully-built end_session_endpoint URL (with
// id_token_hint, post_logout_redirect_uri and client_id). Empty for non-OIDC
// sessions.
// RP-Initiated Logout URL the frontend redirects to. Empty for non-OIDC sessions.
OIDCLogoutURL string `json:"oidc_logout_url,omitempty"`
}

6
pkg/routes/api/v2/auth_login.go
View File

@ -45,11 +45,7 @@ type authTokenBody struct {
// logoutBody confirms a successful logout.
type logoutBody struct {
Body struct {
Message string `json:"message" readOnly:"true" doc:"A human-readable confirmation message."`
// OIDCLogoutURL is the provider's RP-Initiated Logout URL (with
// id_token_hint, post_logout_redirect_uri and client_id) for sessions
// created via OpenID Connect. The client should redirect the user agent
// to it to end the provider session too. Empty for non-OIDC sessions.
Message string `json:"message" readOnly:"true" doc:"A human-readable confirmation message."`
OIDCLogoutURL string `json:"oidc_logout_url,omitempty" readOnly:"true" doc:"RP-Initiated Logout URL to redirect to for OpenID Connect sessions; empty otherwise."`
}
}