From 502e61e7b432be68dd6e0ecb87a6c0165d0a5e53 Mon Sep 17 00:00:00 2001 From: Christophe Vila Date: Fri, 24 Jul 2026 22:03:02 +0200 Subject: [PATCH] Fix OIDC login-gate cross-file wiring bugs from whole-branch review Four bugs slipped through per-task review since each task only saw its own diff: - Logout was a plain GET against a POST-only backend route, so it 405'd and never cleared the session cookie or hit Keycloak's end-session redirect. Now a
with a submit button styled to match the old link (still a real full-page navigation, not a fetch, so the Keycloak redirect chain still works). - Login/logout used origin-relative paths, unreachable from the Vite dev server (:5173) against the backend (:8080) with no proxy configured. Both now build their URL from client.ts's now-exported BASE_URL. - handleSessionCallback's four failure paths redirected to /?auth_error=failed with no logging, making a real OIDC failure undiagnosable in production. Added log.Printf on each failure site. - handleSessionLogout passed a bare "/" to EndSessionURL; Keycloak requires post_logout_redirect_uri to be an absolute, registered URL. Added SessionConfig.PublicBaseURL, wired from cfg.PublicBaseURL in main.go, and used to build an absolute redirect. Co-Authored-By: Claude Sonnet 5 --- backend/cmd/geniusrund/main.go | 7 ++++--- backend/internal/api/api_test.go | 7 ++++--- backend/internal/api/session.go | 12 +++++++++++- frontend/src/App.css | 5 +++++ frontend/src/App.tsx | 10 ++++++---- frontend/src/LoginGate.tsx | 4 ++-- frontend/src/api/client.ts | 11 +++++++---- 7 files changed, 39 insertions(+), 17 deletions(-) diff --git a/backend/cmd/geniusrund/main.go b/backend/cmd/geniusrund/main.go index f9ac804..126b47d 100644 --- a/backend/cmd/geniusrund/main.go +++ b/backend/cmd/geniusrund/main.go @@ -61,9 +61,10 @@ func main() { } server := api.NewServer(db, garminClient, syncSvc, authVerifier, api.SessionConfig{ - Secret: cfg.SessionSecret, - Duration: cfg.SessionDuration, - Secure: cfg.SessionSecure, + Secret: cfg.SessionSecret, + Duration: cfg.SessionDuration, + Secure: cfg.SessionSecure, + PublicBaseURL: cfg.PublicBaseURL, }) ctx, stop := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM) diff --git a/backend/internal/api/api_test.go b/backend/internal/api/api_test.go index 591cc3b..16174c7 100644 --- a/backend/internal/api/api_test.go +++ b/backend/internal/api/api_test.go @@ -23,9 +23,10 @@ import ( func newCtx() context.Context { return context.Background() } var testSessionConfig = SessionConfig{ - Secret: []byte("test-session-secret-at-least-32-bytes-long"), - Duration: time.Hour, - Secure: false, + Secret: []byte("test-session-secret-at-least-32-bytes-long"), + Duration: time.Hour, + Secure: false, + PublicBaseURL: "https://geniusrun.example.com", } func newTestServer(t *testing.T) (*Server, *store.DB) { diff --git a/backend/internal/api/session.go b/backend/internal/api/session.go index 840bf54..4d0aefe 100644 --- a/backend/internal/api/session.go +++ b/backend/internal/api/session.go @@ -1,6 +1,7 @@ package api import ( + "log" "net/http" "time" @@ -14,6 +15,12 @@ type SessionConfig struct { Secret []byte Duration time.Duration Secure bool + // PublicBaseURL is this app's own externally reachable origin (e.g. + // "https://geniusrun.example.com", no trailing slash), used to build an + // absolute post_logout_redirect_uri for the identity provider -- some + // providers, including Keycloak, require this to be an absolute URL + // matching one registered on the client, not a bare relative path. + PublicBaseURL string } type sessionMeResponse struct { @@ -39,6 +46,7 @@ func (s *Server) handleSessionLogin(w http.ResponseWriter, r *http.Request) { func (s *Server) handleSessionCallback(w http.ResponseWriter, r *http.Request) { txnCookie, err := r.Cookie(auth.TxnCookieName) if err != nil { + log.Printf("session callback: missing txn cookie: %v", err) http.Redirect(w, r, "/?auth_error=failed", http.StatusFound) return } @@ -46,12 +54,14 @@ func (s *Server) handleSessionCallback(w http.ResponseWriter, r *http.Request) { txn, err := auth.ParseTxnCookie(txnCookie, s.Session.Secret) if err != nil { + log.Printf("session callback: failed to parse txn cookie: %v", err) http.Redirect(w, r, "/?auth_error=failed", http.StatusFound) return } result, err := s.Auth.HandleCallback(r.Context(), txn, r.URL.Query()) if err != nil { + log.Printf("session callback: HandleCallback failed (state mismatch, code exchange, or ID-token verification): %v", err) http.Redirect(w, r, "/?auth_error=failed", http.StatusFound) return } @@ -71,7 +81,7 @@ func (s *Server) handleSessionCallback(w http.ResponseWriter, r *http.Request) { func (s *Server) handleSessionLogout(w http.ResponseWriter, r *http.Request) { http.SetCookie(w, auth.ClearCookie(auth.SessionCookieName, s.Session.Secure)) - http.Redirect(w, r, s.Auth.EndSessionURL("/"), http.StatusFound) + http.Redirect(w, r, s.Auth.EndSessionURL(s.Session.PublicBaseURL+"/"), http.StatusFound) } func (s *Server) handleSessionMe(w http.ResponseWriter, r *http.Request) { diff --git a/frontend/src/App.css b/frontend/src/App.css index 6bffae5..64d42dc 100644 --- a/frontend/src/App.css +++ b/frontend/src/App.css @@ -94,9 +94,14 @@ body { } .logout-link { + background: none; + border: none; + padding: 0; + font: inherit; color: #9aa0ab; font-size: 0.85rem; text-decoration: none; + cursor: pointer; } .logout-link:hover { diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index d32e67a..d5c9243 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -1,5 +1,5 @@ import { useEffect, useState } from "react"; -import { api } from "./api/client"; +import { api, BASE_URL } from "./api/client"; import "./App.css"; import { Dashboard } from "./pages/Dashboard"; import { Plan } from "./pages/Plan"; @@ -56,9 +56,11 @@ function App({ session }: { session: SessionInfo }) { > {profileName ?? "Profile"} - - Log out - + + +
{showProfile ? setProfileName(p.Name)} /> : }
diff --git a/frontend/src/LoginGate.tsx b/frontend/src/LoginGate.tsx index 62c11b7..96a3558 100644 --- a/frontend/src/LoginGate.tsx +++ b/frontend/src/LoginGate.tsx @@ -1,5 +1,5 @@ import { useEffect, useState } from "react"; -import { api } from "./api/client"; +import { api, BASE_URL } from "./api/client"; import "./LoginGate.css"; import App from "./App"; import type { SessionInfo } from "./types/api"; @@ -39,7 +39,7 @@ export function LoginGate() {

🧞‍♀️ geniusrun

{authError &&

{AUTH_ERROR_MESSAGES[authError] ?? "Login failed, please try again."}

} - + Log in
diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index c4a96f9..1d16f26 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -13,7 +13,7 @@ import type { WorkoutKind, } from "../types/api"; -const BASE_URL = import.meta.env.VITE_API_BASE_URL ?? "http://localhost:8080"; +export const BASE_URL = import.meta.env.VITE_API_BASE_URL ?? "http://localhost:8080"; async function request(path: string, init?: RequestInit): Promise { const res = await fetch(`${BASE_URL}${path}`, { @@ -39,9 +39,12 @@ async function request(path: string, init?: RequestInit): Promise { export const api = { // Session (app login via OIDC -- distinct from the Garmin credential - // login below). No logout()/login() methods: those are plain - // full-page navigations (see LoginGate.tsx), not fetches, since the OIDC - // flow and Keycloak's own logout redirect need real browser navigation. + // login below). No logout()/login() methods: login is a plain + // and logout is a
submit button (see App.tsx / + // LoginGate.tsx) -- both are real full-page navigations, not fetches, + // since the OIDC flow and Keycloak's own logout redirect need real + // browser navigation. Logout must POST (see server.go's route), which an + // can't do, hence the form. getSessionInfo: () => request("/api/session/me"), // Auth