diff --git a/backend/internal/api/api_test.go b/backend/internal/api/api_test.go index e8b70b6..9edfb48 100644 --- a/backend/internal/api/api_test.go +++ b/backend/internal/api/api_test.go @@ -1013,3 +1013,72 @@ func TestSessionLogout_PostLogoutRedirectUsesFrontendURL(t *testing.T) { t.Fatalf("status = %d, Location = %q, want post_logout_redirect_uri = %q", rec.Code, rec.Header().Get("Location"), testSessionConfig.FrontendURL+"/") } } + +// TestSessionLogout_PassesIDTokenHintFromSessionCookie confirms the raw ID +// token carried in the session cookie (minted at callback time) is handed +// back to EndSessionURL on logout, so Keycloak can skip its own +// logout-confirmation prompt instead of leaving the user a chance to cancel +// out of it after their geniusrun account is already deleted. +func TestSessionLogout_PassesIDTokenHintFromSessionCookie(t *testing.T) { + verifier := &authmock.Verifier{EndSessionResult: "https://keycloak.example.com/logout"} + s, _ := newTestServerWithAuth(t, verifier) + + cookie, err := auth.MintSessionCookie( + auth.Claims{Sub: "test-user", Name: "Test User", Email: "test@example.com", IDToken: "raw-id-token-jwt"}, + testSessionConfig.Secret, testSessionConfig.Duration, testSessionConfig.Secure, + ) + if err != nil { + t.Fatalf("mint session cookie: %v", err) + } + req := httptest.NewRequest(http.MethodPost, "/api/session/logout", nil) + req.AddCookie(cookie) + rec := httptest.NewRecorder() + s.Router().ServeHTTP(rec, req) + + if rec.Code != http.StatusFound { + t.Fatalf("status = %d, want 302, body = %s", rec.Code, rec.Body.String()) + } + if verifier.LastIDTokenHint != "raw-id-token-jwt" { + t.Errorf("LastIDTokenHint = %q, want %q", verifier.LastIDTokenHint, "raw-id-token-jwt") + } +} + +// TestSessionCallback_MintsSessionCookieCarryingIDToken confirms the raw ID +// token from a completed OIDC callback ends up in the session cookie (not +// just Sub/Name/Email), since that's the only place logout can later read +// it back from to build id_token_hint. +func TestSessionCallback_MintsSessionCookieCarryingIDToken(t *testing.T) { + verifier := &authmock.Verifier{ + CallbackResult: auth.LoginResult{ + Claims: auth.Claims{Sub: "u1", Name: "Alice", Email: "alice@example.com", IDToken: "raw-id-token-jwt"}, + Authorized: true, + }, + } + s, _ := newTestServerWithAuth(t, verifier) + + txnCookie, err := auth.MintTxnCookie(auth.TxnState{State: "s1", CodeVerifier: "v1"}, testSessionConfig.Secret, false) + if err != nil { + t.Fatalf("mint txn cookie: %v", err) + } + req := httptest.NewRequest(http.MethodGet, "/api/session/callback?code=abc&state=s1", nil) + req.AddCookie(txnCookie) + rec := httptest.NewRecorder() + s.Router().ServeHTTP(rec, req) + + var sessionCookie *http.Cookie + for _, c := range rec.Result().Cookies() { + if c.Name == auth.SessionCookieName { + sessionCookie = c + } + } + if sessionCookie == nil { + t.Fatal("expected a session cookie to be set") + } + claims, err := auth.ParseSessionCookie(sessionCookie, testSessionConfig.Secret) + if err != nil { + t.Fatalf("parse session cookie: %v", err) + } + if claims.IDToken != "raw-id-token-jwt" { + t.Errorf("claims.IDToken = %q, want %q", claims.IDToken, "raw-id-token-jwt") + } +} diff --git a/backend/internal/api/session.go b/backend/internal/api/session.go index 9ec3e07..3376e36 100644 --- a/backend/internal/api/session.go +++ b/backend/internal/api/session.go @@ -89,9 +89,16 @@ func (s *Server) handleSessionCallback(w http.ResponseWriter, r *http.Request) { http.Redirect(w, r, s.Session.FrontendURL+"/", http.StatusFound) } +// handleSessionLogout clears geniusrun's own session cookie and redirects +// through Keycloak's end-session endpoint, passing the session's ID token +// as id_token_hint (see auth.Claims.IDToken) so Keycloak can skip its own +// logout-confirmation prompt -- otherwise a user could cancel out of it and +// land back on the app with a Keycloak SSO session but no geniusrun profile +// (already deleted, in the profile-deletion case this exists for). func (s *Server) handleSessionLogout(w http.ResponseWriter, r *http.Request) { + claims, _ := auth.ClaimsFromContext(r.Context()) http.SetCookie(w, auth.ClearCookie(auth.SessionCookieName, s.Session.Secure)) - http.Redirect(w, r, s.Auth.EndSessionURL(s.Session.FrontendURL+"/"), http.StatusFound) + http.Redirect(w, r, s.Auth.EndSessionURL(s.Session.FrontendURL+"/", claims.IDToken), http.StatusFound) } func (s *Server) handleSessionMe(w http.ResponseWriter, r *http.Request) { diff --git a/backend/internal/auth/mock/mock.go b/backend/internal/auth/mock/mock.go index 91811e3..094d864 100644 --- a/backend/internal/auth/mock/mock.go +++ b/backend/internal/auth/mock/mock.go @@ -18,6 +18,7 @@ type Verifier struct { CallbackResult auth.LoginResult CallbackErr error EndSessionResult string // if empty, EndSessionURL returns postLogoutRedirectURL unchanged + LastIDTokenHint string // records the idTokenHint passed to the last EndSessionURL call } var _ auth.Verifier = (*Verifier)(nil) @@ -33,7 +34,8 @@ func (v *Verifier) HandleCallback(ctx context.Context, txn auth.TxnState, query return v.CallbackResult, nil } -func (v *Verifier) EndSessionURL(postLogoutRedirectURL string) string { +func (v *Verifier) EndSessionURL(postLogoutRedirectURL, idTokenHint string) string { + v.LastIDTokenHint = idTokenHint if v.EndSessionResult != "" { return v.EndSessionResult } diff --git a/backend/internal/auth/oidc.go b/backend/internal/auth/oidc.go index ee22aff..50c389a 100644 --- a/backend/internal/auth/oidc.go +++ b/backend/internal/auth/oidc.go @@ -6,6 +6,7 @@ import ( "encoding/base64" "fmt" "net/url" + "slices" "github.com/coreos/go-oidc/v3/oidc" "golang.org/x/oauth2" @@ -24,7 +25,11 @@ type Verifier interface { HandleCallback(ctx context.Context, txn TxnState, query url.Values) (LoginResult, error) // EndSessionURL builds the identity provider's logout URL, redirecting // back to postLogoutRedirectURL once Keycloak's own session is cleared. - EndSessionURL(postLogoutRedirectURL string) string + // idTokenHint, if non-empty, is passed as id_token_hint so Keycloak can + // positively identify the session being ended and skip its own + // logout-confirmation prompt (which would otherwise let the user cancel + // out of logout after their geniusrun account is already deleted). + EndSessionURL(postLogoutRedirectURL, idTokenHint string) string } // LoginResult is what a completed callback exchange resolves to. @@ -55,12 +60,7 @@ type idTokenClaims struct { } func (c idTokenClaims) hasRole(required string) bool { - for _, r := range c.RealmAccess.Roles { - if r == required { - return true - } - } - return false + return slices.Contains(c.RealmAccess.Roles, required) } type oidcVerifier struct { @@ -135,12 +135,12 @@ func (v *oidcVerifier) HandleCallback(ctx context.Context, txn TxnState, query u return LoginResult{}, fmt.Errorf("decode id_token claims: %w", err) } return LoginResult{ - Claims: Claims{Sub: claims.Sub, Name: claims.Name, Email: claims.Email}, + Claims: Claims{Sub: claims.Sub, Name: claims.Name, Email: claims.Email, IDToken: rawIDToken}, Authorized: claims.hasRole(v.requiredRole), }, nil } -func (v *oidcVerifier) EndSessionURL(postLogoutRedirectURL string) string { +func (v *oidcVerifier) EndSessionURL(postLogoutRedirectURL, idTokenHint string) string { var discovery struct { EndSessionEndpoint string `json:"end_session_endpoint"` } @@ -154,6 +154,9 @@ func (v *oidcVerifier) EndSessionURL(postLogoutRedirectURL string) string { q := u.Query() q.Set("client_id", v.oauth2Config.ClientID) q.Set("post_logout_redirect_uri", postLogoutRedirectURL) + if idTokenHint != "" { + q.Set("id_token_hint", idTokenHint) + } u.RawQuery = q.Encode() return u.String() } diff --git a/backend/internal/auth/oidc_test.go b/backend/internal/auth/oidc_test.go index d6f51cc..27c9024 100644 --- a/backend/internal/auth/oidc_test.go +++ b/backend/internal/auth/oidc_test.go @@ -1,10 +1,83 @@ package auth import ( + "context" "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "net/url" "testing" ) +// newTestOIDCVerifier spins up a fake OIDC discovery endpoint (just enough +// for oidc.NewProvider's discovery GET to succeed) and returns a real +// oidcVerifier backed by it, for tests that exercise EndSessionURL without +// a live Keycloak. +func newTestOIDCVerifier(t *testing.T) Verifier { + t.Helper() + var issuerURL string + mux := http.NewServeMux() + mux.HandleFunc("/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + fmt.Fprintf(w, `{ + "issuer": %[1]q, + "authorization_endpoint": "%[1]s/auth", + "token_endpoint": "%[1]s/token", + "end_session_endpoint": "%[1]s/logout", + "jwks_uri": "%[1]s/certs" + }`, issuerURL) + }) + mux.HandleFunc("/certs", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + fmt.Fprint(w, `{"keys":[]}`) + }) + srv := httptest.NewServer(mux) + t.Cleanup(srv.Close) + issuerURL = srv.URL + + verifier, err := NewOIDCVerifier(context.Background(), OIDCConfig{ + IssuerURL: issuerURL, ClientID: "geniusrun", ClientSecret: "secret", RedirectURL: issuerURL + "/callback", + }) + if err != nil { + t.Fatalf("NewOIDCVerifier: %v", err) + } + return verifier +} + +func TestEndSessionURL_IncludesIDTokenHintWhenProvided(t *testing.T) { + verifier := newTestOIDCVerifier(t) + + got := verifier.EndSessionURL("https://app.example.com/", "raw-id-token-jwt") + u, err := url.Parse(got) + if err != nil { + t.Fatalf("parse EndSessionURL result %q: %v", got, err) + } + q := u.Query() + if q.Get("id_token_hint") != "raw-id-token-jwt" { + t.Errorf("id_token_hint = %q, want %q", q.Get("id_token_hint"), "raw-id-token-jwt") + } + if q.Get("client_id") != "geniusrun" { + t.Errorf("client_id = %q, want geniusrun", q.Get("client_id")) + } + if q.Get("post_logout_redirect_uri") != "https://app.example.com/" { + t.Errorf("post_logout_redirect_uri = %q, want https://app.example.com/", q.Get("post_logout_redirect_uri")) + } +} + +func TestEndSessionURL_OmitsIDTokenHintWhenEmpty(t *testing.T) { + verifier := newTestOIDCVerifier(t) + + got := verifier.EndSessionURL("https://app.example.com/", "") + u, err := url.Parse(got) + if err != nil { + t.Fatalf("parse EndSessionURL result %q: %v", got, err) + } + if u.Query().Has("id_token_hint") { + t.Errorf("expected no id_token_hint param when idTokenHint is empty, got %q", got) + } +} + func TestIDTokenClaims_HasRole(t *testing.T) { cases := []struct { name string diff --git a/backend/internal/auth/session.go b/backend/internal/auth/session.go index 5332918..c251341 100644 --- a/backend/internal/auth/session.go +++ b/backend/internal/auth/session.go @@ -1,9 +1,15 @@ // Package auth implements geniusrun's own login gate: an OIDC Authorization // Code flow against an existing Keycloak realm (see oidc.go), backed by a // signed session cookie geniusrun mints itself (this file) and a chi -// middleware that checks it (middleware.go). Keycloak's own tokens are never -// stored or refreshed -- once HandleCallback verifies the ID token and role, -// only this package's own cookie matters for subsequent requests. +// middleware that checks it (middleware.go). Keycloak's own access/refresh +// tokens are never stored or refreshed -- once HandleCallback verifies the +// ID token and role, only this package's own cookie matters for subsequent +// requests. The one exception is the raw ID token itself, carried opaquely +// inside the signed session cookie (Claims.IDToken) solely so a later +// logout can pass it back to Keycloak as id_token_hint -- letting Keycloak +// skip its own logout-confirmation prompt for a session it can positively +// identify, rather than leaving the user a chance to cancel out of it after +// their geniusrun account (and profile) is already gone. package auth import ( @@ -26,17 +32,21 @@ const ( ) // Claims identifies the authenticated user, carried in the signed session -// cookie. +// cookie. IDToken is the raw Keycloak ID token JWT from the OIDC callback, +// carried opaquely so a later logout can pass it back to Keycloak as +// id_token_hint (see EndSessionURL) -- geniusrun never inspects it itself. type Claims struct { - Sub string - Name string - Email string + Sub string + Name string + Email string + IDToken string } type sessionClaims struct { - Sub string `json:"sub"` - Name string `json:"name"` - Email string `json:"email"` + Sub string `json:"sub"` + Name string `json:"name"` + Email string `json:"email"` + IDToken string `json:"id_token,omitempty"` jwt.RegisteredClaims } @@ -58,9 +68,10 @@ type txnClaims struct { func MintSessionCookie(claims Claims, secret []byte, duration time.Duration, secure bool) (*http.Cookie, error) { now := time.Now() token := jwt.NewWithClaims(jwt.SigningMethodHS256, sessionClaims{ - Sub: claims.Sub, - Name: claims.Name, - Email: claims.Email, + Sub: claims.Sub, + Name: claims.Name, + Email: claims.Email, + IDToken: claims.IDToken, RegisteredClaims: jwt.RegisteredClaims{ IssuedAt: jwt.NewNumericDate(now), ExpiresAt: jwt.NewNumericDate(now.Add(duration)), @@ -91,7 +102,7 @@ func ParseSessionCookie(cookie *http.Cookie, secret []byte) (Claims, error) { if _, err := jwt.ParseWithClaims(cookie.Value, &sc, keyfunc(secret), jwt.WithValidMethods([]string{"HS256"})); err != nil { return Claims{}, fmt.Errorf("parse session token: %w", err) } - return Claims{Sub: sc.Sub, Name: sc.Name, Email: sc.Email}, nil + return Claims{Sub: sc.Sub, Name: sc.Name, Email: sc.Email, IDToken: sc.IDToken}, nil } // MintTxnCookie signs an OIDC login transaction into a short-lived cookie. diff --git a/backend/internal/auth/session_test.go b/backend/internal/auth/session_test.go index 8a645c3..fbfaf5a 100644 --- a/backend/internal/auth/session_test.go +++ b/backend/internal/auth/session_test.go @@ -8,7 +8,7 @@ import ( const testSecret = "test-secret-at-least-32-bytes-long!" func TestMintAndParseSessionCookie(t *testing.T) { - claims := Claims{Sub: "u1", Name: "Alice", Email: "alice@example.com"} + claims := Claims{Sub: "u1", Name: "Alice", Email: "alice@example.com", IDToken: "raw-id-token-jwt"} cookie, err := MintSessionCookie(claims, []byte(testSecret), time.Hour, true) if err != nil { t.Fatalf("mint: %v", err)