diff --git a/frontend/src/stores/auth.ts b/frontend/src/stores/auth.ts index e2576760f..e9a7c8ee0 100644 --- a/frontend/src/stores/auth.ts +++ b/frontend/src/stores/auth.ts @@ -533,9 +533,11 @@ 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. + let oidcLogoutUrl = '' try { const HTTP = AuthenticatedHTTPFactory() - await HTTP.post('user/logout') + const {data} = await HTTP.post('user/logout') + oidcLogoutUrl = data?.oidc_logout_url ?? '' } catch (_e) { // Ignore — session will expire naturally } @@ -547,7 +549,12 @@ export const useAuthStore = defineStore('auth', () => { await router.push({name: 'user.login'}) await checkAuth() - // if configured, redirect to OIDC Provider on logout + // 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 + } const fullProvider: IProvider|undefined = configStore.auth.openidConnect.providers?.find((p: IProvider) => p.key === loggedInVia) if (fullProvider) { redirectToProviderOnLogout(fullProvider) diff --git a/pkg/migration/20260619155410.go b/pkg/migration/20260619155410.go new file mode 100644 index 000000000..97f91bb5e --- /dev/null +++ b/pkg/migration/20260619155410.go @@ -0,0 +1,55 @@ +// Vikunja is a to-do list application to facilitate your life. +// Copyright 2018-present Vikunja and contributors. All rights reserved. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package migration + +import ( + "time" + + "src.techknowlogick.com/xormigrate" + "xorm.io/xorm" +) + +// 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"` + TokenHash string `xorm:"varchar(64) not null unique index"` + DeviceInfo string `xorm:"text"` + IPAddress string `xorm:"varchar(100)"` + IsLongSession bool `xorm:"not null default false"` + OIDCIDToken string `xorm:"text"` + OIDCProviderKey string `xorm:"varchar(250)"` + LastActive time.Time `xorm:"not null"` + Created time.Time `xorm:"created not null"` +} + +func (sessionOIDCLogout20260619155410) TableName() string { + return "sessions" +} + +func init() { + migrations = append(migrations, &xormigrate.Migration{ + ID: "20260619155410", + Description: "Add oidc_id_token and oidc_provider_key columns to sessions for RP-Initiated Logout", + Migrate: func(tx *xorm.Engine) error { + return tx.Sync(sessionOIDCLogout20260619155410{}) + }, + Rollback: func(tx *xorm.Engine) error { + return nil + }, + }) +} diff --git a/pkg/models/sessions.go b/pkg/models/sessions.go index 9c7a5d1f1..9824cf0c3 100644 --- a/pkg/models/sessions.go +++ b/pkg/models/sessions.go @@ -49,6 +49,10 @@ 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:"-"` + // Raw OIDC ID token, kept so logout can replay it as id_token_hint. Empty for non-OIDC sessions. + OIDCIDToken string `xorm:"text" json:"-"` + // 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."` // When this session was created (login time). @@ -81,9 +85,17 @@ func generateHashedToken() (rawToken, hash string, err error) { return rawToken, HashSessionToken(rawToken), nil } +// 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 +} + // CreateSession creates a new session record and generates a refresh token. // Returns the session with RefreshToken populated (cleartext, shown only once). -func CreateSession(s *xorm.Session, userID int64, deviceInfo, ipAddress string, isLongSession bool) (*Session, error) { +// 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 { return nil, err @@ -98,6 +110,10 @@ func CreateSession(s *xorm.Session, userID int64, deviceInfo, ipAddress string, IsLongSession: isLongSession, LastActive: time.Now(), } + if oidc != nil { + session.OIDCIDToken = oidc.IDToken + session.OIDCProviderKey = oidc.ProviderKey + } _, err = s.Insert(session) if err != nil { diff --git a/pkg/modules/auth/auth.go b/pkg/modules/auth/auth.go index f94537158..87c89e5aa 100644 --- a/pkg/modules/auth/auth.go +++ b/pkg/modules/auth/auth.go @@ -112,12 +112,13 @@ 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. -func IssueUserToken(ctx context.Context, u *user.User, deviceInfo, ipAddress string, long bool) (*IssuedUserToken, error) { +// 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() - session, err := models.CreateSession(s, u.ID, deviceInfo, ipAddress, long) + session, err := models.CreateSession(s, u.ID, deviceInfo, ipAddress, long, oidc) if err != nil { _ = s.Rollback() return nil, err @@ -161,8 +162,9 @@ func WriteUserAuthCookies(c *echo.Context, token *IssuedUserToken) { } // NewUserAuthTokenResponse creates a new user auth token response from a user object. -func NewUserAuthTokenResponse(u *user.User, c *echo.Context, long bool) error { - token, err := IssueUserToken(c.Request().Context(), u, c.Request().UserAgent(), c.RealIP(), long) +// 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 { return err } diff --git a/pkg/modules/auth/oauth2server/token.go b/pkg/modules/auth/oauth2server/token.go index 11f85772e..97978c0bf 100644 --- a/pkg/modules/auth/oauth2server/token.go +++ b/pkg/modules/auth/oauth2server/token.go @@ -114,7 +114,7 @@ func exchangeAuthorizationCode(ctx context.Context, req *TokenRequest, deviceInf } // Create a session (reuses existing session infrastructure) - session, err := models.CreateSession(s, oauthCode.UserID, deviceInfo, ipAddress, false) + session, err := models.CreateSession(s, oauthCode.UserID, deviceInfo, ipAddress, false, nil) if err != nil { _ = s.Rollback() return nil, err diff --git a/pkg/modules/auth/openid/logout.go b/pkg/modules/auth/openid/logout.go new file mode 100644 index 000000000..958ea8765 --- /dev/null +++ b/pkg/modules/auth/openid/logout.go @@ -0,0 +1,110 @@ +// Vikunja is a to-do list application to facilitate your life. +// Copyright 2018-present Vikunja and contributors. All rights reserved. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package openid + +import ( + "net/url" + + "code.vikunja.io/api/pkg/config" + "code.vikunja.io/api/pkg/log" + "code.vikunja.io/api/pkg/models" +) + +// 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 + } + return p.LogoutURL +} + +// 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 "" + } + + var meta struct { + EndSessionEndpoint string `json:"end_session_endpoint"` + } + if err := p.openIDProvider.Claims(&meta); err != nil { + log.Debugf("Could not read end_session_endpoint for provider %s: %v", p.Key, err) + return "" + } + return meta.EndSessionEndpoint +} + +// 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) { + // 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 + } + if provider == nil { + return "", nil + } + + idToken := "" + if oidc != nil { + idToken = oidc.IDToken + } + + return buildEndSessionURL( + provider.EndSessionEndpoint(), + provider.ClientID, + idToken, + config.ServicePublicURL.GetString(), + ) +} + +// 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 + } + + u, err := url.Parse(endpoint) + if err != nil { + return "", err + } + + q := u.Query() + if clientID != "" { + q.Set("client_id", clientID) + } + if idToken != "" { + q.Set("id_token_hint", idToken) + } + if postLogoutRedirectURI != "" { + q.Set("post_logout_redirect_uri", postLogoutRedirectURI) + } + u.RawQuery = q.Encode() + + return u.String(), nil +} diff --git a/pkg/modules/auth/openid/logout_test.go b/pkg/modules/auth/openid/logout_test.go new file mode 100644 index 000000000..57e3ea6a2 --- /dev/null +++ b/pkg/modules/auth/openid/logout_test.go @@ -0,0 +1,234 @@ +// Vikunja is a to-do list application to facilitate your life. +// Copyright 2018-present Vikunja and contributors. All rights reserved. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package openid + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "net/url" + "testing" + + "code.vikunja.io/api/pkg/config" + "code.vikunja.io/api/pkg/models" + "code.vikunja.io/api/pkg/modules/keyvalue" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// newMockOIDCServerWithEndSession publishes a discovery document with an +// end_session_endpoint. +func newMockOIDCServerWithEndSession() *httptest.Server { + var server *httptest.Server + mux := http.NewServeMux() + mux.HandleFunc("/.well-known/openid-configuration", func(w http.ResponseWriter, _ *http.Request) { + discovery := map[string]interface{}{ + "issuer": server.URL, + "authorization_endpoint": server.URL + "/auth", + "token_endpoint": server.URL + "/token", + "jwks_uri": server.URL + "/jwks", + "end_session_endpoint": server.URL + "/logout", + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(discovery) + }) + server = httptest.NewServer(mux) + return server +} + +func TestBuildEndSessionURLAssembly(t *testing.T) { + t.Run("all params", func(t *testing.T) { + got, err := buildEndSessionURL("https://op.example.com/logout", "my-client", "the-id-token", "https://vikunja.example.com/") + require.NoError(t, err) + + u, err := url.Parse(got) + require.NoError(t, err) + q := u.Query() + assert.Equal(t, "https", u.Scheme) + assert.Equal(t, "op.example.com", u.Host) + assert.Equal(t, "/logout", u.Path) + assert.Equal(t, "the-id-token", q.Get("id_token_hint")) + assert.Equal(t, "https://vikunja.example.com/", q.Get("post_logout_redirect_uri")) + assert.Equal(t, "my-client", q.Get("client_id")) + }) + + t.Run("preserves existing endpoint query params", func(t *testing.T) { + got, err := buildEndSessionURL("https://op.example.com/logout?foo=bar", "my-client", "the-id-token", "https://vikunja.example.com/") + require.NoError(t, err) + + u, err := url.Parse(got) + require.NoError(t, err) + q := u.Query() + assert.Equal(t, "bar", q.Get("foo")) + assert.Equal(t, "the-id-token", q.Get("id_token_hint")) + }) + + t.Run("omits id_token_hint when no token", func(t *testing.T) { + got, err := buildEndSessionURL("https://op.example.com/logout", "my-client", "", "https://vikunja.example.com/") + require.NoError(t, err) + + u, err := url.Parse(got) + require.NoError(t, err) + q := u.Query() + assert.False(t, q.Has("id_token_hint")) + assert.Equal(t, "https://vikunja.example.com/", q.Get("post_logout_redirect_uri")) + assert.Equal(t, "my-client", q.Get("client_id")) + }) + + t.Run("empty endpoint returns empty", func(t *testing.T) { + got, err := buildEndSessionURL("", "my-client", "the-id-token", "https://vikunja.example.com/") + require.NoError(t, err) + assert.Empty(t, got) + }) +} + +func TestBuildEndSessionURLFromDiscovery(t *testing.T) { + defer CleanupSavedOpenIDProviders() + + server := newMockOIDCServerWithEndSession() + defer server.Close() + + config.AuthOpenIDEnabled.Set(true) + config.ServicePublicURL.Set("https://vikunja.example.com/") + 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") + + got, err := BuildEndSessionURL("provider1", &models.SessionOIDCData{ + IDToken: "raw-id-token", + ProviderKey: "provider1", + }) + require.NoError(t, err) + + u, err := url.Parse(got) + require.NoError(t, err) + q := u.Query() + assert.Equal(t, server.URL+"/logout", u.Scheme+"://"+u.Host+u.Path) + assert.Equal(t, "raw-id-token", q.Get("id_token_hint")) + assert.Equal(t, "https://vikunja.example.com/", q.Get("post_logout_redirect_uri")) + assert.Equal(t, "client1", q.Get("client_id")) +} + +func TestBuildEndSessionURLFromCachedProviderWithoutLiveObject(t *testing.T) { + defer CleanupSavedOpenIDProviders() + + config.AuthOpenIDEnabled.Set(true) + config.ServicePublicURL.Set("https://vikunja.example.com/") + + // 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", + ClientID: "client1", + EndSessionURL: "https://op.example.com/end-session", + })) + + got, err := BuildEndSessionURL("provider1", &models.SessionOIDCData{ + IDToken: "raw-id-token", + ProviderKey: "provider1", + }) + require.NoError(t, err) + + u, err := url.Parse(got) + require.NoError(t, err) + q := u.Query() + assert.Equal(t, "https://op.example.com/end-session", u.Scheme+"://"+u.Host+u.Path) + assert.Equal(t, "raw-id-token", q.Get("id_token_hint")) + assert.Equal(t, "https://vikunja.example.com/", q.Get("post_logout_redirect_uri")) + assert.Equal(t, "client1", q.Get("client_id")) +} + +func TestEndSessionEndpointUsesCachedURLWithoutDiscovery(t *testing.T) { + // 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", + 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() + + // newMockOIDCServer publishes no end_session_endpoint, forcing the logouturl fallback. + server := newMockOIDCServer() + 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", + "logouturl": "https://op.example.com/static-logout", + }, + }) + _ = keyvalue.Del("openid_providers") + _ = keyvalue.Del("openid_provider_provider1") + + provider, err := GetProvider("provider1") + require.NoError(t, err) + require.NotNil(t, provider) + + assert.Equal(t, "https://op.example.com/static-logout", provider.EndSessionEndpoint()) +} diff --git a/pkg/modules/auth/openid/openid.go b/pkg/modules/auth/openid/openid.go index 47ade7dc9..3b82379ac 100644 --- a/pkg/modules/auth/openid/openid.go +++ b/pkg/modules/auth/openid/openid.go @@ -69,8 +69,12 @@ type Provider struct { ForceUserInfo bool `json:"force_user_info"` RequireAvailability bool `json:"-"` ClientSecret string `json:"-"` - openIDProvider *oidc.Provider - Oauth2Config *oauth2.Config `json:"-"` + // 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:"-"` } type claims struct { @@ -173,7 +177,7 @@ func HandleCallback(c *echo.Context) error { return &models.ErrOpenIDBadRequest{Message: "Bad data"} } - u, err := AuthenticateCallback(c.Request().Context(), cb, c.Param("provider")) + u, oidcData, err := AuthenticateCallback(c.Request().Context(), cb, c.Param("provider")) if err != nil { var detailedErr *models.ErrOpenIDBadRequestWithDetails if errors.As(err, &detailedErr) { @@ -186,7 +190,7 @@ func HandleCallback(c *echo.Context) error { } // Create token - return auth.NewUserAuthTokenResponse(u, c, false) + return auth.NewUserAuthTokenResponse(u, c, false, oidcData) } // AuthenticateCallback resolves an OpenID Connect callback to an authenticated @@ -196,18 +200,24 @@ func HandleCallback(c *echo.Context) error { // handler and the v2 Huma handler; the caller issues the auth token. The // ErrOpenIDBadRequestWithDetails error keeps its provider detail so v1 can render // its bespoke body and v2 can map it to RFC 9457. -func AuthenticateCallback(ctx context.Context, cb *Callback, providerKey string) (*user.User, error) { +func AuthenticateCallback(ctx context.Context, cb *Callback, providerKey string) (*user.User, *models.SessionOIDCData, error) { // ctx is threaded through only to dispatch the login event; the OIDC token // exchange, claim verification and user/avatar sync run on their own // background contexts, exactly as the v1 callback always did. - provider, oauthToken, idToken, err := exchangeOidcTokens(cb, providerKey) //nolint:contextcheck + provider, oauthToken, idToken, rawIDToken, err := exchangeOidcTokens(cb, providerKey) //nolint:contextcheck if err != nil { - return nil, err + return nil, nil, err + } + + // Stored so logout can replay it as id_token_hint in an RP-Initiated Logout. + oidcData := &models.SessionOIDCData{ + IDToken: rawIDToken, + ProviderKey: providerKey, } cl, err := getClaims(provider, oauthToken, idToken) //nolint:contextcheck if err != nil { - return nil, err + return nil, nil, err } s := db.NewSession() @@ -221,16 +231,16 @@ func AuthenticateCallback(ctx context.Context, cb *Callback, providerKey string) if err != nil { _ = s.Rollback() log.Errorf("Error creating new user for provider %s: %v", provider.Name, err) - return nil, err + return nil, nil, err } if u.Status == user.StatusDisabled { _ = s.Rollback() - return nil, &user.ErrAccountDisabled{UserID: u.ID} + return nil, nil, &user.ErrAccountDisabled{UserID: u.ID} } if u.Status == user.StatusAccountLocked { _ = s.Rollback() - return nil, &user.ErrAccountLocked{UserID: u.ID} + return nil, nil, &user.ErrAccountLocked{UserID: u.ID} } // Must run before team sync so a failed 2FA attempt cannot mutate team @@ -247,26 +257,26 @@ func AuthenticateCallback(ctx context.Context, cb *Callback, providerKey string) if user.IsErrInvalidTOTPPasscode(err) { user.HandleFailedTOTPAuth(u) } - return nil, err + return nil, nil, err } teamData := getTeamDataFromToken(cl.VikunjaGroups, provider) err = models.SyncExternalTeamsForUser(s, u, teamData, idToken.Issuer, provider.Name) if err != nil { - return nil, err + return nil, nil, err } err = s.Commit() if err != nil { _ = s.Rollback() log.Errorf("Error creating new team for provider %s: %v", provider.Name, err) - return nil, err + return nil, nil, err } events.DispatchPending(ctx, s) - return u, nil + return u, oidcData, nil } func getTeamDataFromToken(groups []map[string]interface{}, provider *Provider) (teamData []*models.Team) { @@ -543,13 +553,13 @@ func getClaims(provider *Provider, oauth2Token *oauth2.Token, idToken *oidc.IDTo // and verifies the returned ID token. It takes an already-bound Callback so it // can be shared by the v1 echo handler (which binds from the request) and the v2 // Huma handler (which binds via its typed body). -func exchangeOidcTokens(cb *Callback, providerKey string) (*Provider, *oauth2.Token, *oidc.IDToken, error) { +func exchangeOidcTokens(cb *Callback, providerKey string) (*Provider, *oauth2.Token, *oidc.IDToken, string, error) { provider, err := GetProvider(providerKey) if err != nil { - return nil, nil, nil, err + return nil, nil, nil, "", err } if provider == nil { - return nil, nil, nil, &models.ErrOpenIDBadRequest{Message: "Provider does not exist"} + return nil, nil, nil, "", &models.ErrOpenIDBadRequest{Message: "Provider does not exist"} } log.Debugf("Trying to authenticate user using provider: %s", provider.Key) @@ -565,25 +575,25 @@ func exchangeOidcTokens(cb *Callback, providerKey string) (*Provider, *oauth2.To if err := json.Unmarshal(rerr.Body, &details); err != nil { log.Errorf("Error unmarshalling token for provider %s: %v", provider.Name, err) log.Debugf("Raw token value is %s", rerr.Body) - return nil, nil, nil, err + return nil, nil, nil, "", err } log.Errorf("Error retrieving token: %s", err) log.Debugf("Raw token value is %s", rerr.Body) - return nil, nil, nil, &models.ErrOpenIDBadRequestWithDetails{ + return nil, nil, nil, "", &models.ErrOpenIDBadRequestWithDetails{ Message: "Could not authenticate against third party.", Details: details, } } - return nil, nil, nil, err + return nil, nil, nil, "", err } // Extract the ID Token from OAuth2 token. rawIDToken, ok := oauth2Token.Extra("id_token").(string) if !ok { log.Debugf("Could not get id_token, raw token is %v", oauth2Token) - return nil, nil, nil, &models.ErrOpenIDBadRequest{Message: "Missing token"} + return nil, nil, nil, "", &models.ErrOpenIDBadRequest{Message: "Missing token"} } verifier := provider.openIDProvider.Verifier(&oidc.Config{ClientID: provider.ClientID}) @@ -592,8 +602,8 @@ func exchangeOidcTokens(cb *Callback, providerKey string) (*Provider, *oauth2.To idToken, err := verifier.Verify(context.Background(), rawIDToken) if err != nil { log.Errorf("Error verifying token for provider %s: %v", provider.Name, err) - return nil, nil, nil, err + return nil, nil, nil, "", err } - return provider, oauth2Token, idToken, nil + return provider, oauth2Token, idToken, rawIDToken, nil } diff --git a/pkg/modules/auth/openid/providers.go b/pkg/modules/auth/openid/providers.go index 30f534ad9..710870b7d 100644 --- a/pkg/modules/auth/openid/providers.go +++ b/pkg/modules/auth/openid/providers.go @@ -180,6 +180,29 @@ func GetProvider(key string) (provider *Provider, err error) { return } +// 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) + if err != nil { + return nil, err + } + if !exists { + _, err = GetAllProviders() // This will put all providers in cache + if err != nil { + return nil, err + } + + _, err = keyvalue.GetWithValue("openid_provider_"+key, provider) + if err != nil { + return nil, err + } + } + + return provider, nil +} + // parseBoolField reads a boolean-valued config field from a provider map, // tolerating both native bools (from YAML/JSON) and strings (from env vars or // the GetConfigValueFromFile path, which always return strings). Missing or @@ -313,6 +336,8 @@ func getProviderFromMap(pi map[string]interface{}, key string) (provider *Provid provider.AuthURL = provider.Oauth2Config.Endpoint.AuthURL + provider.EndSessionURL = provider.discoveredEndSessionEndpoint() + return } diff --git a/pkg/routes/api/shared/auth.go b/pkg/routes/api/shared/auth.go index 153d851e8..560ae0f47 100644 --- a/pkg/routes/api/shared/auth.go +++ b/pkg/routes/api/shared/auth.go @@ -27,6 +27,7 @@ import ( "code.vikunja.io/api/pkg/models" "code.vikunja.io/api/pkg/modules/auth" "code.vikunja.io/api/pkg/modules/auth/ldap" + "code.vikunja.io/api/pkg/modules/auth/openid" "code.vikunja.io/api/pkg/modules/keyvalue" "code.vikunja.io/api/pkg/user" @@ -192,24 +193,53 @@ func enforceLoginTOTP(s *xorm.Session, u *user.User, passcode string) error { // API token or a link share), matching v1. Shared by v1 and v2; the caller is // responsible for clearing the refresh cookie. func DeleteSession(sid string) error { + _, err := LogoutSession(sid) + return err +} + +// 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 + return "", nil } s := db.NewSession() defer s.Close() + // 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() + return "", err + } + if session != nil && session.OIDCProviderKey != "" { + url, buildErr := openid.BuildEndSessionURL(session.OIDCProviderKey, &models.SessionOIDCData{ + IDToken: session.OIDCIDToken, + ProviderKey: session.OIDCProviderKey, + }) + if buildErr != nil { + // 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 + } + } + if _, err := s.Where("id = ?", sid).Delete(&models.Session{}); err != nil { _ = s.Rollback() - return err + return "", err } if err := s.Commit(); err != nil { _ = s.Rollback() - return err + return "", err } - return nil + return endSessionURL, nil } // ResetPassword resets a user's password from a previously issued reset token diff --git a/pkg/routes/api/v1/login.go b/pkg/routes/api/v1/login.go index 2d740ffdf..6a4662ae2 100644 --- a/pkg/routes/api/v1/login.go +++ b/pkg/routes/api/v1/login.go @@ -56,7 +56,7 @@ func Login(c *echo.Context) (err error) { } // Create token - return auth.NewUserAuthTokenResponse(user, c, u.LongToken) + return auth.NewUserAuthTokenResponse(user, c, u.LongToken, nil) } // RenewToken renews a link share token only. User tokens must use @@ -150,12 +150,18 @@ func RefreshToken(c *echo.Context) (err error) { return c.JSON(http.StatusOK, auth.Token{Token: result.AccessToken}) } +type LogoutResponse struct { + Message string `json:"message"` + // RP-Initiated Logout URL the frontend redirects to. Empty for non-OIDC sessions. + OIDCLogoutURL string `json:"oidc_logout_url,omitempty"` +} + // Logout deletes the current session from the server. // @Summary Logout -// @Description Destroys the current session and clears the refresh token cookie. +// @Description Destroys the current session and clears the refresh token cookie. For OpenID Connect sessions the response includes an `oidc_logout_url` the client should redirect to so the provider session is ended too. // @tags auth // @Produce json -// @Success 200 {object} models.Message "Successfully logged out." +// @Success 200 {object} v1.LogoutResponse "Successfully logged out." // @Router /user/logout [post] func Logout(c *echo.Context) (err error) { auth.ClearRefreshTokenCookie(c) @@ -177,7 +183,8 @@ func Logout(c *echo.Context) (err error) { } } - if err := shared.DeleteSession(sid); err != nil { + oidcLogoutURL, err := shared.LogoutSession(sid) + if err != nil { return err } @@ -187,5 +194,8 @@ func Logout(c *echo.Context) (err error) { } } - return c.JSON(http.StatusOK, models.Message{Message: "Successfully logged out."}) + return c.JSON(http.StatusOK, LogoutResponse{ + Message: "Successfully logged out.", + OIDCLogoutURL: oidcLogoutURL, + }) } diff --git a/pkg/routes/api/v2/auth_login.go b/pkg/routes/api/v2/auth_login.go index d6ff0ff19..519fcaef1 100644 --- a/pkg/routes/api/v2/auth_login.go +++ b/pkg/routes/api/v2/auth_login.go @@ -45,7 +45,8 @@ 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."` + 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."` } } @@ -86,7 +87,7 @@ func authLogin(ctx context.Context, in *struct{ Body user.Login }) (*authTokenBo } deviceInfo, ipAddress := requestClientInfo(ctx) - token, err := auth.IssueUserToken(ctx, u, deviceInfo, ipAddress, in.Body.LongToken) + token, err := auth.IssueUserToken(ctx, u, deviceInfo, ipAddress, in.Body.LongToken, nil) if err != nil { return nil, translateDomainError(err) } @@ -107,12 +108,14 @@ func authLogout(ctx context.Context, _ *struct{}) (*logoutBody, error) { sid = auth.SessionIDFromContext(ec) } - if err := shared.DeleteSession(sid); err != nil { + oidcLogoutURL, err := shared.LogoutSession(sid) //nolint:contextcheck // OIDC provider discovery resolves from a cached, context-less map and runs on its own background context, like the OIDC callback. + if err != nil { return nil, translateDomainError(err) } out := &logoutBody{} out.Body.Message = "Successfully logged out." + out.Body.OIDCLogoutURL = oidcLogoutURL return out, nil } diff --git a/pkg/routes/api/v2/auth_openid.go b/pkg/routes/api/v2/auth_openid.go index b52d7dca1..5e029a184 100644 --- a/pkg/routes/api/v2/auth_openid.go +++ b/pkg/routes/api/v2/auth_openid.go @@ -55,14 +55,14 @@ func authOpenIDCallback(ctx context.Context, in *struct { Provider string `path:"provider" doc:"The OpenID Connect provider key as returned by the /info endpoint."` Body openid.Callback `doc:"The provider callback, carrying the authorization code."` }) (*authTokenBody, error) { - u, err := openid.AuthenticateCallback(ctx, &in.Body, in.Provider) //nolint:contextcheck // resolves providers from a cached, context-less map and runs OIDC discovery on its own background context, like the v1 callback. + u, oidcData, err := openid.AuthenticateCallback(ctx, &in.Body, in.Provider) //nolint:contextcheck // resolves providers from a cached, context-less map and runs OIDC discovery on its own background context, like the v1 callback. if err != nil { return nil, translateOpenIDError(err) } deviceInfo, ipAddress := requestClientInfo(ctx) // OIDC logins are not "remember me" sessions; v1 always issues a short one. - token, err := auth.IssueUserToken(ctx, u, deviceInfo, ipAddress, false) + token, err := auth.IssueUserToken(ctx, u, deviceInfo, ipAddress, false, oidcData) if err != nil { return nil, translateDomainError(err) } diff --git a/pkg/webtests/huma_auth_login_test.go b/pkg/webtests/huma_auth_login_test.go index ad83fc811..48931effa 100644 --- a/pkg/webtests/huma_auth_login_test.go +++ b/pkg/webtests/huma_auth_login_test.go @@ -130,7 +130,7 @@ func TestHumaLogout(t *testing.T) { // Create a session so logout has something to delete, then mint a JWT whose // sid claim points at it. s := db.NewSession() - session, err := models.CreateSession(s, testuser1.ID, "test", "127.0.0.1", false) + session, err := models.CreateSession(s, testuser1.ID, "test", "127.0.0.1", false, nil) require.NoError(t, err) require.NoError(t, s.Commit()) require.NoError(t, s.Close())