diff --git a/errors/errors.go b/errors/errors.go index aca6d020..3ff10514 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -208,4 +208,5 @@ var ( ErrOIDCAudienceMismatch = errors.New("token audience does not match any of the expected audiences") ErrCertificateNotLoaded = errors.New("tls certificate not yet loaded") ErrCertificateWatcherAlreadyRunning = errors.New("certificate watcher is already running") + ErrInvalidEndSessionEndpoint = errors.New("end_session_endpoint must be an absolute http(s) URL") ) diff --git a/pkg/api/authn.go b/pkg/api/authn.go index 3b00fa75..8528b148 100644 --- a/pkg/api/authn.go +++ b/pkg/api/authn.go @@ -204,7 +204,7 @@ func (amw *AuthnMiddleware) basicAuthn(ctlr *Controller, userAc *reqCtx.UserAcce // saved logged session only if the request comes from web (has UI session header value) if hasSessionHeader(request) { secure := ctlr.Config.UseSecureSession() - if err := saveUserLoggedSession(cookieStore, response, request, identity, secure, ctlr.Log); err != nil { + if err := saveUserLoggedSession(cookieStore, response, request, identity, "", secure, ctlr.Log); err != nil { return false, err } } @@ -243,7 +243,7 @@ func (amw *AuthnMiddleware) basicAuthn(ctlr *Controller, userAc *reqCtx.UserAcce // saved logged session only if the request comes from web (has UI session header value) if hasSessionHeader(request) { secure := ctlr.Config.UseSecureSession() - if err := saveUserLoggedSession(cookieStore, response, request, identity, secure, ctlr.Log); err != nil { + if err := saveUserLoggedSession(cookieStore, response, request, identity, "", secure, ctlr.Log); err != nil { return false, err } } @@ -901,6 +901,24 @@ func NewRelyingPartyGithub(config *config.Config, provider string, hashKey, encr return relyingParty } +// originFromConfig returns the server's base URL (scheme + host[:port], no trailing +// slash) used as the origin for OIDC redirect URIs. It prefers ExternalURL; otherwise +// it derives the origin from cfg.HTTP.Address, cfg.HTTP.Port, and cfg.HTTP.TLS. Using +// a single source for both the login redirect_uri and the logout +// post_logout_redirect_uri ensures the IdP sees matching origins. +func originFromConfig(cfg *config.Config) string { + if trimmed := strings.TrimSuffix(cfg.HTTP.ExternalURL, "/"); trimmed != "" { + return trimmed + } + + scheme := constants.SchemeHTTP + if cfg.HTTP.TLS != nil { + scheme = constants.SchemeHTTPS + } + + return scheme + "://" + net.JoinHostPort(cfg.HTTP.Address, cfg.HTTP.Port) +} + func getRelyingPartyArgs(cfg *config.Config, provider string, hashKey, encryptKey []byte, log log.Logger) ( string, string, string, string, []string, []rp.Option, ) { @@ -918,26 +936,11 @@ func getRelyingPartyArgs(cfg *config.Config, provider string, hashKey, encryptKe scopes = append([]string{oidc.ScopeOpenID}, scopes...) } - port := cfg.HTTP.Port issuer := providerConfig.Issuer keyPath := providerConfig.KeyPath - baseURL := net.JoinHostPort(cfg.HTTP.Address, port) callback := constants.CallbackBasePath + "/" + provider - - var redirectURI string - - if cfg.HTTP.ExternalURL != "" { - externalURL := strings.TrimSuffix(cfg.HTTP.ExternalURL, "/") - redirectURI = fmt.Sprintf("%s%s", externalURL, callback) - } else { - scheme := constants.SchemeHTTP - if cfg.HTTP.TLS != nil { - scheme = constants.SchemeHTTPS - } - - redirectURI = fmt.Sprintf("%s://%s%s", scheme, baseURL, callback) - } + redirectURI := originFromConfig(cfg) + callback options := []rp.Option{ rp.WithVerifierOpts(rp.WithIssuedAtOffset(issuedAtOffset)), @@ -1066,7 +1069,7 @@ func GetGithubUserInfo(ctx context.Context, client *github.Client, log log.Logge } func saveUserLoggedSession(cookieStore sessions.Store, response http.ResponseWriter, - request *http.Request, identity string, secure bool, log log.Logger, + request *http.Request, identity, provider string, secure bool, log log.Logger, ) error { session, _ := cookieStore.Get(request, "session") @@ -1076,6 +1079,12 @@ func saveUserLoggedSession(cookieStore sessions.Store, response http.ResponseWri session.Values["authStatus"] = true session.Values["user"] = identity + if provider != "" { + session.Values["provider"] = provider + } else { + delete(session.Values, "provider") + } + // let the session set its own id err := session.Save(request, response) if err != nil { @@ -1098,7 +1107,7 @@ func saveUserLoggedSession(cookieStore sessions.Store, response http.ResponseWri } // OAuth2Callback is the callback logic where openid/oauth2 will redirect back to our app. -func OAuth2Callback(ctlr *Controller, w http.ResponseWriter, r *http.Request, state, email string, +func OAuth2Callback(ctlr *Controller, w http.ResponseWriter, r *http.Request, state, email, provider string, groups []string, ) (string, error) { stateCookie, _ := ctlr.CookieStore.Get(r, "statecookie") @@ -1126,7 +1135,7 @@ func OAuth2Callback(ctlr *Controller, w http.ResponseWriter, r *http.Request, st // if this line has been reached, then a new session should be created // if the `session` key is already on the cookie, it's not a valid one secure := ctlr.Config.UseSecureSession() - if err := saveUserLoggedSession(ctlr.CookieStore, w, r, email, secure, ctlr.Log); err != nil { + if err := saveUserLoggedSession(ctlr.CookieStore, w, r, email, provider, secure, ctlr.Log); err != nil { return "", err } diff --git a/pkg/api/logout_internal_test.go b/pkg/api/logout_internal_test.go new file mode 100644 index 00000000..68101e60 --- /dev/null +++ b/pkg/api/logout_internal_test.go @@ -0,0 +1,527 @@ +package api + +import ( + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "net/url" + "testing" + + "github.com/gorilla/sessions" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/zitadel/oidc/v3/pkg/client/rp" + "golang.org/x/oauth2" + + "zotregistry.dev/zot/v2/pkg/api/config" + "zotregistry.dev/zot/v2/pkg/log" +) + +func TestComposeEndSessionURL(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + endpoint string + clientID string + redirectURI string + wantEmpty bool + wantErr bool + wantQuery url.Values + wantPath string + }{ + { + name: "empty endpoint yields empty URL and no error", + endpoint: "", + clientID: "id", + wantEmpty: true, + }, + { + name: "client_id only when redirect is empty", + endpoint: "https://idp.example.com/logout", + clientID: "zot-client", + wantPath: "/logout", + wantQuery: url.Values{ + "client_id": []string{"zot-client"}, + }, + }, + { + name: "client_id and post_logout_redirect_uri merged", + endpoint: "https://idp.example.com/realms/zot/protocol/openid-connect/logout", + clientID: "zot-client", + redirectURI: "https://zot.example.com/login", + wantPath: "/realms/zot/protocol/openid-connect/logout", + wantQuery: url.Values{ + "client_id": []string{"zot-client"}, + "post_logout_redirect_uri": []string{"https://zot.example.com/login"}, + }, + }, + { + name: "preserves pre-existing query parameters", + endpoint: "https://idp.example.com/logout?ui_locales=en", + clientID: "zot-client", + redirectURI: "https://zot.example.com/login", + wantPath: "/logout", + wantQuery: url.Values{ + "ui_locales": []string{"en"}, + "client_id": []string{"zot-client"}, + "post_logout_redirect_uri": []string{"https://zot.example.com/login"}, + }, + }, + { + name: "invalid endpoint returns error", + endpoint: "://not-a-url", + wantErr: true, + }, + { + name: "relative endpoint is rejected", + endpoint: "/realms/zot/protocol/openid-connect/logout", + wantErr: true, + }, + { + name: "scheme-less endpoint with host is rejected", + endpoint: "//idp.example.com/logout", + wantErr: true, + }, + { + name: "non-http scheme is rejected", + endpoint: "javascript:alert(1)", + wantErr: true, + }, + { + name: "ftp scheme is rejected", + endpoint: "ftp://idp.example.com/logout", + wantErr: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + got, err := composeEndSessionURL(tc.endpoint, tc.clientID, tc.redirectURI) + + if tc.wantErr { + require.Error(t, err) + + return + } + + require.NoError(t, err) + + if tc.wantEmpty { + assert.Empty(t, got) + + return + } + + parsed, err := url.Parse(got) + require.NoError(t, err) + assert.Equal(t, tc.wantPath, parsed.Path) + assert.Equal(t, tc.wantQuery, parsed.Query()) + }) + } +} + +func TestPostLogoutRedirectURI(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + externalURL string + address string + port string + tls bool + want string + }{ + { + name: "plain http from address:port when no external URL", + address: "zot.example.com", + port: "5000", + want: "http://zot.example.com:5000/login", + }, + { + name: "https when TLS is configured", + address: "zot.example.com", + port: "5000", + tls: true, + want: "https://zot.example.com:5000/login", + }, + { + name: "externalURL wins over address:port", + externalURL: "https://zot.example.com", + address: "internal.local", + port: "5000", + want: "https://zot.example.com/login", + }, + { + name: "externalURL trailing slash is trimmed", + externalURL: "https://zot.example.com/", + address: "internal.local", + port: "5000", + want: "https://zot.example.com/login", + }, + { + name: "externalURL with subpath is preserved", + externalURL: "https://example.com/zot", + want: "https://example.com/zot/login", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + cfg := &config.Config{HTTP: config.HTTPConfig{ + ExternalURL: tc.externalURL, + Address: tc.address, + Port: tc.port, + }} + if tc.tls { + cfg.HTTP.TLS = &config.TLSConfig{} + } + + assert.Equal(t, tc.want, postLogoutRedirectURI(cfg)) + }) + } +} + +// newTestCookieStore builds a cookie store suitable for in-process handler tests. +// MaxAge is short-circuited to 0 so the underlying cookie is encoded as a plain session +// cookie, which httptest recorders can round-trip without worrying about expiry skew. +func newTestCookieStore(t *testing.T) *CookieStore { + t.Helper() + + memStore := sessions.NewCookieStore( + []byte("test-hash-key-0123456789abcdef01"), + []byte("test-encryption-key-0123456789ab"), + ) + memStore.MaxAge(0) + + return &CookieStore{Store: memStore} +} + +// newTestRouteHandler builds the minimum RouteHandler needed to exercise the Logout +// handler directly, bypassing NewController/SetupRoutes which require much more plumbing. +func newTestRouteHandler(t *testing.T) *RouteHandler { + t.Helper() + + logger := log.NewLogger("debug", "") + ctlr := &Controller{ + Config: &config.Config{HTTP: config.HTTPConfig{}}, + Log: logger, + CookieStore: newTestCookieStore(t), + RelyingParties: map[string]rp.RelyingParty{}, + } + + return &RouteHandler{c: ctlr} +} + +// seedSessionProvider writes a value into session.Values["provider"] and returns the +// cookies produced. Callers re-attach those cookies to the subsequent request so the +// handler sees the same session. +func seedSessionProvider(t *testing.T, rh *RouteHandler, provider string) []*http.Cookie { + t.Helper() + + req := httptest.NewRequest(http.MethodPost, "/zot/auth/logout", nil) + rec := httptest.NewRecorder() + + session, err := rh.c.CookieStore.Get(req, "session") + require.NoError(t, err) + + session.Values["provider"] = provider + require.NoError(t, session.Save(req, rec)) + + return rec.Result().Cookies() +} + +func TestLogoutHandler(t *testing.T) { + t.Parallel() + + t.Run("no session → 200 OK with empty LogoutResponse JSON", func(t *testing.T) { + t.Parallel() + + rh := newTestRouteHandler(t) + req := httptest.NewRequest(http.MethodPost, "/zot/auth/logout", nil) + rec := httptest.NewRecorder() + + rh.Logout(rec, req) + + res := rec.Result() + defer res.Body.Close() + + assert.Equal(t, http.StatusOK, res.StatusCode) + assert.Equal(t, "application/json", res.Header.Get("Content-Type")) + assert.JSONEq(t, `{}`, rec.Body.String(), + "non-OIDC logout should still return the LogoutResponse shape (endSessionUrl omitted)") + }) + + t.Run("OPTIONS preflight is a no-op", func(t *testing.T) { + t.Parallel() + + rh := newTestRouteHandler(t) + req := httptest.NewRequest(http.MethodOptions, "/zot/auth/logout", nil) + rec := httptest.NewRecorder() + + rh.Logout(rec, req) + + // The handler returns before touching the response, so status stays at the + // recorder default (200) and body is empty. + assert.Equal(t, http.StatusOK, rec.Result().StatusCode) + assert.Empty(t, rec.Body.Bytes()) + }) + + t.Run("session has provider but no matching RelyingParty → JSON with no endSessionUrl", func(t *testing.T) { + t.Parallel() + + rh := newTestRouteHandler(t) + cookies := seedSessionProvider(t, rh, "oidc") // supported provider name, but map is empty + + req := httptest.NewRequest(http.MethodPost, "/zot/auth/logout", nil) + for _, c := range cookies { + req.AddCookie(c) + } + + rec := httptest.NewRecorder() + rh.Logout(rec, req) + + res := rec.Result() + defer res.Body.Close() + + assert.Equal(t, http.StatusOK, res.StatusCode) + assert.Equal(t, "application/json", res.Header.Get("Content-Type")) + assert.JSONEq(t, `{}`, rec.Body.String(), + "absent RelyingParty should degrade to an empty LogoutResponse (local-only logout)") + }) + + t.Run("session has provider not in OIDC-supported list → JSON with no endSessionUrl", func(t *testing.T) { + t.Parallel() + + rh := newTestRouteHandler(t) + cookies := seedSessionProvider(t, rh, "github") // github is oauth2-only, not OIDC + + req := httptest.NewRequest(http.MethodPost, "/zot/auth/logout", nil) + for _, c := range cookies { + req.AddCookie(c) + } + + rec := httptest.NewRecorder() + rh.Logout(rec, req) + + res := rec.Result() + defer res.Body.Close() + + assert.Equal(t, http.StatusOK, res.StatusCode) + assert.Equal(t, "application/json", res.Header.Get("Content-Type")) + assert.JSONEq(t, `{}`, rec.Body.String(), + "non-OIDC provider should degrade to an empty LogoutResponse") + }) + + t.Run("LogoutResponse JSON shape round-trips endSessionUrl", func(t *testing.T) { + t.Parallel() + + // The full Logout→IdP path requires a live OIDC discovery server; the pure URL + // construction is covered by TestComposeEndSessionURL. Here we just verify the + // JSON contract the zui client relies on: a "LogoutResponse" with the field name + // "endSessionUrl" encodes as expected and decodes back to the same struct. + orig := LogoutResponse{EndSessionURL: "https://idp.example.com/logout?client_id=zot"} + + encoded, err := json.Marshal(orig) + require.NoError(t, err) + assert.JSONEq(t, `{"endSessionUrl":"https://idp.example.com/logout?client_id=zot"}`, string(encoded)) + + var decoded LogoutResponse + + require.NoError(t, json.Unmarshal(encoded, &decoded)) + assert.Equal(t, orig, decoded) + + // And verify the omitempty behavior: missing URL should yield an empty object. + empty, err := json.Marshal(LogoutResponse{}) + require.NoError(t, err) + assert.JSONEq(t, `{}`, string(empty)) + }) +} + +// TestSaveUserLoggedSessionClearsStaleProvider guards against a re-login regression: if +// a previous OIDC session left provider="oidc" on the cookie and the user re-authenticates +// via a non-OIDC path (basic/LDAP/GitHub), the provider key must be cleared, otherwise a +// later Logout would return an endSessionUrl pointing at an IdP the user is no longer +// authenticated against. +func TestSaveUserLoggedSessionClearsStaleProvider(t *testing.T) { + t.Parallel() + + rh := newTestRouteHandler(t) + cookies := seedSessionProvider(t, rh, "oidc") + + req := httptest.NewRequest(http.MethodGet, "/", nil) + for _, c := range cookies { + req.AddCookie(c) + } + + rec := httptest.NewRecorder() + + require.NoError(t, + saveUserLoggedSession(rh.c.CookieStore, rec, req, "alice", "", false, rh.c.Log)) + + // Round-trip the new cookies and assert the provider key is gone. + readReq := httptest.NewRequest(http.MethodGet, "/", nil) + for _, c := range rec.Result().Cookies() { + readReq.AddCookie(c) + } + + session, err := rh.c.CookieStore.Get(readReq, "session") + require.NoError(t, err) + _, hasProvider := session.Values["provider"] + assert.False(t, hasProvider, "stale provider must be cleared after non-OIDC re-login") +} + +// failingStore wraps a real sessions.Store and forces Save to return a supplied error, +// so we can exercise the error branch that plain in-memory stores never trigger. +type failingStore struct { + sessions.Store + + saveErr error +} + +// Get routes through the request-scoped session registry but registers THIS wrapper as +// the session's owning store. Without that, gorilla would cache the inner sessions.Store +// under the session's .store field and later Save calls would bypass our saveErr gate. +func (f *failingStore) Get(r *http.Request, name string) (*sessions.Session, error) { + return sessions.GetRegistry(r).Get(f, name) +} + +func (f *failingStore) New(r *http.Request, name string) (*sessions.Session, error) { + return f.Store.New(r, name) +} + +func (f *failingStore) Save(r *http.Request, w http.ResponseWriter, s *sessions.Session) error { + if f.saveErr != nil { + return f.saveErr + } + + return f.Store.Save(r, w, s) +} + +// newTestRouteHandlerWithStore is newTestRouteHandler but lets the caller inject a +// custom CookieStore — used to drive the failingStore paths. +func newTestRouteHandlerWithStore(t *testing.T, store *CookieStore) *RouteHandler { + t.Helper() + + return &RouteHandler{c: &Controller{ + Config: &config.Config{HTTP: config.HTTPConfig{}}, + Log: log.NewLogger("debug", ""), + CookieStore: store, + RelyingParties: map[string]rp.RelyingParty{}, + }} +} + +// fakeEndSessionRP satisfies the rp.RelyingParty interface just enough for +// buildEndSessionURL, which only calls GetEndSessionEndpoint and OAuthConfig. Any other +// method on the embedded interface remains nil and would panic if called — keeping the +// fake honest: tests will blow up if a future change starts calling a new RP method. +type fakeEndSessionRP struct { + rp.RelyingParty + + endSessionURL string + clientID string +} + +func (f *fakeEndSessionRP) GetEndSessionEndpoint() string { + return f.endSessionURL +} + +func (f *fakeEndSessionRP) OAuthConfig() *oauth2.Config { + return &oauth2.Config{ClientID: f.clientID} +} + +func TestLogoutSessionSaveError(t *testing.T) { + t.Parallel() + + saveErr := errors.New("cookie store save failed") + memStore := sessions.NewCookieStore( + []byte("test-hash-key-0123456789abcdef01"), + []byte("test-encryption-key-0123456789ab"), + ) + rh := newTestRouteHandlerWithStore(t, + &CookieStore{Store: &failingStore{Store: memStore, saveErr: saveErr}}) + + req := httptest.NewRequest(http.MethodPost, "/zot/auth/logout", nil) + rec := httptest.NewRecorder() + + rh.Logout(rec, req) + + assert.Equal(t, http.StatusInternalServerError, rec.Result().StatusCode, + "a failing session.Save should surface as 500 rather than a partial body") +} + +func TestLogoutReturnsEndSessionURL(t *testing.T) { + t.Parallel() + + // Happy path: a RelyingParty with a well-formed end_session_endpoint produces a + // LogoutResponse whose endSessionUrl carries client_id and the resolved + // post_logout_redirect_uri. + rh := newTestRouteHandler(t) + rh.c.Config.HTTP.ExternalURL = "https://zot.example.com" + rh.c.RelyingParties["oidc"] = &fakeEndSessionRP{ + endSessionURL: "https://idp.example.com/logout", + clientID: "zot-client", + } + + cookies := seedSessionProvider(t, rh, "oidc") + + req := httptest.NewRequest(http.MethodPost, "/zot/auth/logout", nil) + for _, c := range cookies { + req.AddCookie(c) + } + + rec := httptest.NewRecorder() + rh.Logout(rec, req) + + res := rec.Result() + defer res.Body.Close() + + assert.Equal(t, http.StatusOK, res.StatusCode) + assert.Equal(t, "application/json", res.Header.Get("Content-Type")) + + var body LogoutResponse + + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &body)) + + parsed, err := url.Parse(body.EndSessionURL) + require.NoError(t, err) + assert.Equal(t, "https://idp.example.com/logout", parsed.Scheme+"://"+parsed.Host+parsed.Path) + assert.Equal(t, "zot-client", parsed.Query().Get("client_id")) + assert.Equal(t, "https://zot.example.com/login", parsed.Query().Get("post_logout_redirect_uri")) +} + +func TestLogoutBuildEndSessionURLComposeError(t *testing.T) { + t.Parallel() + + // A RelyingParty whose GetEndSessionEndpoint returns a relative URL forces + // composeEndSessionURL to reject it; buildEndSessionURL logs the error and returns + // "", so the handler degrades to an empty LogoutResponse. + rh := newTestRouteHandler(t) + rh.c.RelyingParties["oidc"] = &fakeEndSessionRP{ + endSessionURL: "/realms/zot/logout", // relative, IsAbs() == false + clientID: "zot-client", + } + + cookies := seedSessionProvider(t, rh, "oidc") + + req := httptest.NewRequest(http.MethodPost, "/zot/auth/logout", nil) + for _, c := range cookies { + req.AddCookie(c) + } + + rec := httptest.NewRecorder() + rh.Logout(rec, req) + + res := rec.Result() + defer res.Body.Close() + + assert.Equal(t, http.StatusOK, res.StatusCode) + assert.Equal(t, "application/json", res.Header.Get("Content-Type")) + assert.JSONEq(t, `{}`, rec.Body.String(), + "a malformed end_session_endpoint should not leak into the response") +} diff --git a/pkg/api/routes.go b/pkg/api/routes.go index a17961ae..822aaa0f 100644 --- a/pkg/api/routes.go +++ b/pkg/api/routes.go @@ -2049,20 +2049,38 @@ func (rh *RouteHandler) ListExtensions(w http.ResponseWriter, r *http.Request) { // The following routes are specific to zot and NOT part of the OCI dist-spec +// LogoutResponse is returned by POST /zot/auth/logout. When the session was established +// via an OIDC provider that advertises an `end_session_endpoint` in its discovery +// metadata, EndSessionURL is set to the URL the client should navigate to in order to +// terminate the session at the IdP. For local, basic, LDAP, or GitHub OAuth2 sessions +// the field is omitted from the JSON body. +type LogoutResponse struct { + EndSessionURL string `json:"endSessionUrl,omitempty"` +} + // Logout godoc // @Summary Logout by removing current session -// @Description Logout by removing current session +// @Description Logout by removing current session. For OIDC providers that advertise an +// @Description `end_session_endpoint` in their discovery metadata (OpenID Connect +// @Description RP-Initiated Logout 1.0), the response body contains an `endSessionUrl` +// @Description the client should navigate to in order to terminate the session at the IdP. // @Router /zot/auth/logout [post] // @Accept json // @Produce json -// @Success 200 {string} string "ok" +// @Success 200 {object} api.LogoutResponse // @Failure 500 {string} string "internal server error" func (rh *RouteHandler) Logout(response http.ResponseWriter, request *http.Request) { if request.Method == http.MethodOptions { return } - session, _ := rh.c.CookieStore.Get(request, "session") + session, sessionErr := rh.c.CookieStore.Get(request, "session") + if sessionErr != nil { + rh.c.Log.Warn().Err(sessionErr). + Msg("session cookie could not be decoded; falling back to local-only logout") + } + + provider, _ := session.Values["provider"].(string) session.Options.MaxAge = -1 err := session.Save(request, response) @@ -2072,7 +2090,84 @@ func (rh *RouteHandler) Logout(response http.ResponseWriter, request *http.Reque return } - response.WriteHeader(http.StatusOK) + zcommon.WriteJSON(response, http.StatusOK, LogoutResponse{ + EndSessionURL: rh.buildEndSessionURL(provider), + }) +} + +// buildEndSessionURL returns the OIDC provider's RP-Initiated Logout URL for the given +// provider name, or an empty string if the provider is not OIDC or does not advertise an +// `end_session_endpoint`. No `id_token_hint` is used — `client_id` identifies the RP per +// https://openid.net/specs/openid-connect-rpinitiated-1_0.html#RPLogout. The +// `post_logout_redirect_uri` is resolved by postLogoutRedirectURI and must be pre- +// registered with the IdP client, otherwise the IdP will ignore the parameter and keep +// the user on its own "logged out" page. +func (rh *RouteHandler) buildEndSessionURL(provider string) string { + if provider == "" || !config.IsOpenIDSupported(provider) { + return "" + } + + relyingParty, ok := rh.c.RelyingParties[provider] + if !ok { + return "" + } + + endSessionURL, err := composeEndSessionURL( + relyingParty.GetEndSessionEndpoint(), + relyingParty.OAuthConfig().ClientID, + postLogoutRedirectURI(rh.c.Config), + ) + if err != nil { + rh.c.Log.Error().Err(err).Str("provider", provider). + Str("endpoint", relyingParty.GetEndSessionEndpoint()). + Msg("failed to parse OIDC end_session_endpoint") + + return "" + } + + return endSessionURL +} + +// composeEndSessionURL parses `endpoint` and returns it with `client_id` and, if non-empty, +// `post_logout_redirect_uri` query parameters merged in. An empty endpoint yields an empty +// URL with no error. Relative URLs or non-http(s) schemes are rejected to prevent a client +// from being redirected to an unexpected location when the IdP discovery document is +// malformed. +func composeEndSessionURL(endpoint, clientID, redirectURI string) (string, error) { + if endpoint == "" { + return "", nil + } + + logoutURL, err := url.Parse(endpoint) + if err != nil { + return "", err + } + + if !logoutURL.IsAbs() || logoutURL.Host == "" || + (logoutURL.Scheme != "http" && logoutURL.Scheme != "https") { + return "", fmt.Errorf("%w: got %q", zerr.ErrInvalidEndSessionEndpoint, endpoint) + } + + query := logoutURL.Query() + query.Set("client_id", clientID) + + if redirectURI != "" { + query.Set("post_logout_redirect_uri", redirectURI) + } + + logoutURL.RawQuery = query.Encode() + + return logoutURL.String(), nil +} + +// postLogoutRedirectURI returns the URI where the IdP should send the user after logout. +// It derives the origin from the server configuration via originFromConfig — the same +// helper used to build the login redirect_uri — so that the IdP sees matching origins +// for both login and logout. The resulting URI must be pre-registered with the IdP +// client; otherwise the IdP will ignore the parameter and keep the user on its own +// "logged out" page. +func postLogoutRedirectURI(cfg *config.Config) string { + return originFromConfig(cfg) + "/login" } // GithubCodeExchangeCallback is a github Oauth2 CodeExchange callback. @@ -2091,13 +2186,17 @@ func (rh *RouteHandler) GithubCodeExchangeCallback() rp.CodeExchangeCallback[*oi return } - callbackUI, err := OAuth2Callback(rh.c, w, r, state, email, groups) //nolint: contextcheck + callbackUI, err := OAuth2Callback(rh.c, w, r, state, email, "", groups) //nolint: contextcheck if err != nil { if errors.Is(err, zerr.ErrInvalidStateCookie) { w.WriteHeader(http.StatusUnauthorized) + + return } w.WriteHeader(http.StatusInternalServerError) + + return } if callbackUI != "" { @@ -2205,13 +2304,17 @@ func (rh *RouteHandler) OpenIDCodeExchangeCallbackWithProvider(providerName stri slices.Sort(groups) groups = slices.Compact(groups) - callbackUI, err := OAuth2Callback(rh.c, w, r, state, username, groups) + callbackUI, err := OAuth2Callback(rh.c, w, r, state, username, providerName, groups) if err != nil { if errors.Is(err, zerr.ErrInvalidStateCookie) { w.WriteHeader(http.StatusUnauthorized) + + return } w.WriteHeader(http.StatusInternalServerError) + + return } if callbackUI != "" { diff --git a/pkg/api/routes_test.go b/pkg/api/routes_test.go index 6c13ca9a..1e5a26f5 100644 --- a/pkg/api/routes_test.go +++ b/pkg/api/routes_test.go @@ -155,7 +155,7 @@ func TestRoutes(t *testing.T) { request, _ := http.NewRequestWithContext(ctx, http.MethodGet, baseURL, nil) response := httptest.NewRecorder() - _, err := api.OAuth2Callback(ctlr, response, request, "state", "email", []string{"group"}) + _, err := api.OAuth2Callback(ctlr, response, request, "state", "email", "", []string{"group"}) So(err, ShouldEqual, zerr.ErrInvalidStateCookie) session, _ := ctlr.CookieStore.Get(request, "statecookie") @@ -172,7 +172,7 @@ func TestRoutes(t *testing.T) { err = session.Save(request, response) So(err, ShouldBeNil) - _, err = api.OAuth2Callback(ctlr, response, request, "state", "email", []string{"group"}) + _, err = api.OAuth2Callback(ctlr, response, request, "state", "email", "", []string{"group"}) So(err, ShouldEqual, zerr.ErrInvalidStateCookie) }) diff --git a/swagger/docs.go b/swagger/docs.go index dba617e8..871d25bd 100644 --- a/swagger/docs.go +++ b/swagger/docs.go @@ -1173,7 +1173,7 @@ const docTemplate = `{ }, "/zot/auth/logout": { "post": { - "description": "Logout by removing current session", + "description": "Logout by removing current session. For OIDC providers that advertise an\n` + "`" + `end_session_endpoint` + "`" + ` in their discovery metadata (OpenID Connect\nRP-Initiated Logout 1.0), the response body contains an ` + "`" + `endSessionUrl` + "`" + `\nthe client should navigate to in order to terminate the session at the IdP.", "consumes": [ "application/json" ], @@ -1183,9 +1183,9 @@ const docTemplate = `{ "summary": "Logout by removing current session", "responses": { "200": { - "description": "ok", + "description": "OK", "schema": { - "type": "string" + "$ref": "#/definitions/api.LogoutResponse" } }, "500": { @@ -1313,6 +1313,14 @@ const docTemplate = `{ } } }, + "api.LogoutResponse": { + "type": "object", + "properties": { + "endSessionUrl": { + "type": "string" + } + } + }, "api.RepositoryList": { "type": "object", "properties": { diff --git a/swagger/swagger.json b/swagger/swagger.json index 110bbb14..2fee657f 100644 --- a/swagger/swagger.json +++ b/swagger/swagger.json @@ -1165,7 +1165,7 @@ }, "/zot/auth/logout": { "post": { - "description": "Logout by removing current session", + "description": "Logout by removing current session. For OIDC providers that advertise an\n`end_session_endpoint` in their discovery metadata (OpenID Connect\nRP-Initiated Logout 1.0), the response body contains an `endSessionUrl`\nthe client should navigate to in order to terminate the session at the IdP.", "consumes": [ "application/json" ], @@ -1175,9 +1175,9 @@ "summary": "Logout by removing current session", "responses": { "200": { - "description": "ok", + "description": "OK", "schema": { - "type": "string" + "$ref": "#/definitions/api.LogoutResponse" } }, "500": { @@ -1305,6 +1305,14 @@ } } }, + "api.LogoutResponse": { + "type": "object", + "properties": { + "endSessionUrl": { + "type": "string" + } + } + }, "api.RepositoryList": { "type": "object", "properties": { diff --git a/swagger/swagger.yaml b/swagger/swagger.yaml index c08f418d..09f78efb 100644 --- a/swagger/swagger.yaml +++ b/swagger/swagger.yaml @@ -83,6 +83,11 @@ definitions: manifest forming an association between the image manifest and the other manifest. type: object + api.LogoutResponse: + properties: + endSessionUrl: + type: string + type: object api.RepositoryList: properties: repositories: @@ -1025,14 +1030,18 @@ paths: post: consumes: - application/json - description: Logout by removing current session + description: |- + Logout by removing current session. For OIDC providers that advertise an + `end_session_endpoint` in their discovery metadata (OpenID Connect + RP-Initiated Logout 1.0), the response body contains an `endSessionUrl` + the client should navigate to in order to terminate the session at the IdP. produces: - application/json responses: "200": - description: ok + description: OK schema: - type: string + $ref: '#/definitions/api.LogoutResponse' "500": description: internal server error schema: