From d2ffc98f3c033abc9ae5bbf8b3cff208646f0f07 Mon Sep 17 00:00:00 2001 From: Steve Cliff Date: Tue, 5 May 2026 13:34:47 +0100 Subject: [PATCH] =?UTF-8?q?http:=20logout=20=E2=80=94=20303=20to=20end=5Fs?= =?UTF-8?q?ession=5Fendpoint=20with=20id=5Ftoken=5Fhint=20for=20OIDC=20ses?= =?UTF-8?q?sions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- internal/server/http/oidc_handlers_test.go | 62 ++++++++++++++++++++++ internal/server/http/server.go | 4 +- internal/server/http/ui_handlers.go | 36 +++++++++++-- 3 files changed, 96 insertions(+), 6 deletions(-) diff --git a/internal/server/http/oidc_handlers_test.go b/internal/server/http/oidc_handlers_test.go index 7b48c20..542ea4a 100644 --- a/internal/server/http/oidc_handlers_test.go +++ b/internal/server/http/oidc_handlers_test.go @@ -200,3 +200,65 @@ func TestOIDCCallbackReturningUserRefreshesRole(t *testing.T) { t.Errorf("role refresh: got %q want admin", u.Role) } } + +func TestOIDCLogoutRedirectsToEndSession(t *testing.T) { + t.Parallel() + srv, ts, stub := newTestServerWithOIDC(t) + endSessionURL := stub.URL() + "/logout-end" + stub.SetEndSessionEndpoint(endSessionURL) + + // Rebuild the OIDC client because end_session_endpoint is read at + // New() time from the discovery doc. + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + cfg := &config.OIDCConfig{ + Issuer: stub.URL(), ClientID: "test-client", ClientSecret: "x", + Scopes: []string{"openid"}, RoleClaim: "groups", + RoleMapping: map[string]string{"rm-admins": "admin"}, + } + newClient, err := oidc.New(ctx, cfg, "http://test") + if err != nil { + t.Fatalf("rebuild client: %v", err) + } + srv.deps.OIDC = newClient + + // Sign in via the OIDC flow. + res := runCallback(t, ts, stub, map[string]any{ + "sub": "logout-sub", + "preferred_username": "lo", + "groups": []string{"rm-admins"}, + "aud": "test-client", + }) + res.Body.Close() + cookies := res.Cookies() + if len(cookies) == 0 { + t.Fatal("expected session cookie after sign-in") + } + sessionCookie := cookies[0] + + // POST /logout — should 303 to the end_session endpoint with + // id_token_hint + post_logout_redirect_uri. + c := &stdhttp.Client{CheckRedirect: func(*stdhttp.Request, []*stdhttp.Request) error { + return stdhttp.ErrUseLastResponse + }} + req, _ := stdhttp.NewRequest("POST", ts.URL+"/logout", nil) + req.AddCookie(sessionCookie) + res, err = c.Do(req) + if err != nil { + t.Fatalf("logout: %v", err) + } + defer res.Body.Close() + if res.StatusCode != stdhttp.StatusSeeOther { + t.Errorf("status: got %d want 303", res.StatusCode) + } + loc := res.Header.Get("Location") + if !strings.Contains(loc, "/logout-end") { + t.Errorf("location not at end_session: %q", loc) + } + if !strings.Contains(loc, "id_token_hint=") { + t.Errorf("location missing id_token_hint: %q", loc) + } + if !strings.Contains(loc, "post_logout_redirect_uri=") { + t.Errorf("location missing post_logout_redirect_uri: %q", loc) + } +} diff --git a/internal/server/http/server.go b/internal/server/http/server.go index a8eddd5..41048ea 100644 --- a/internal/server/http/server.go +++ b/internal/server/http/server.go @@ -137,10 +137,12 @@ func (s *Server) routes(r chi.Router) { r.Get("/ws/agent/pending", s.handlePendingWS) r.Mount("/static/", staticHandler()) + // POST /logout is always mounted — it handles both local and OIDC + // sessions and doesn't require the UI renderer. + r.Post("/logout", s.handleUILogoutPost) if s.deps.UI != nil { r.Get("/login", s.handleUILoginGet) r.Post("/login", s.handleUILoginPost) - r.Post("/logout", s.handleUILogoutPost) r.Get("/setup", s.handleUISetupGet) r.Post("/setup", s.handleUISetupPost) } diff --git a/internal/server/http/ui_handlers.go b/internal/server/http/ui_handlers.go index 734bada..c87df78 100644 --- a/internal/server/http/ui_handlers.go +++ b/internal/server/http/ui_handlers.go @@ -8,6 +8,7 @@ import ( "io/fs" "log/slog" stdhttp "net/http" + "net/url" "strings" "time" @@ -956,12 +957,37 @@ func (s *Server) handleUILoginPost(w stdhttp.ResponseWriter, r *stdhttp.Request) stdhttp.Redirect(w, r, "/", stdhttp.StatusSeeOther) } -// handleUILogoutPost is the form-submit twin of /api/auth/logout. It -// drops the session cookie and redirects to /login. +// handleUILogoutPost is the form-submit twin of /api/auth/logout. For +// local sessions it drops the cookie and redirects to /login. For OIDC +// sessions, if the IdP advertised an end_session_endpoint it performs +// RP-initiated logout by redirecting there with id_token_hint and +// post_logout_redirect_uri. func (s *Server) handleUILogoutPost(w stdhttp.ResponseWriter, r *stdhttp.Request) { - if c, err := r.Cookie(sessionCookieName); err == nil { - _ = s.deps.Store.DeleteSession(r.Context(), auth.HashToken(c.Value)) + c, err := r.Cookie(sessionCookieName) + if err != nil { + stdhttp.Redirect(w, r, "/login", stdhttp.StatusSeeOther) + return } + hash := auth.HashToken(c.Value) + sess, _ := s.deps.Store.LookupSession(r.Context(), hash) + _ = s.deps.Store.DeleteSession(r.Context(), hash) + + // Default: drop session, go to /login. + dest := "/login" + + // OIDC session with a discovered end_session_endpoint? Compose + // the IdP logout URL with id_token_hint + post_logout_redirect_uri. + if sess != nil && sess.IDToken != "" && s.deps.OIDC != nil && + s.deps.OIDC.EndSessionEndpoint() != "" { + v := url.Values{} + v.Set("id_token_hint", sess.IDToken) + if base := strings.TrimRight(s.deps.Cfg.BaseURL, "/"); base != "" { + v.Set("post_logout_redirect_uri", base+"/login") + } + dest = s.deps.OIDC.EndSessionEndpoint() + "?" + v.Encode() + } + + // Clear the cookie. stdhttp.SetCookie(w, &stdhttp.Cookie{ Name: sessionCookieName, Value: "", @@ -971,5 +997,5 @@ func (s *Server) handleUILogoutPost(w stdhttp.ResponseWriter, r *stdhttp.Request Secure: s.deps.Cfg.CookieSecure, SameSite: stdhttp.SameSiteLaxMode, }) - stdhttp.Redirect(w, r, "/login", stdhttp.StatusSeeOther) + stdhttp.Redirect(w, r, dest, stdhttp.StatusSeeOther) }