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 <a href> 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 <form method="post"> 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 <noreply@anthropic.com>
This commit is contained in:
@@ -64,6 +64,7 @@ func main() {
|
|||||||
Secret: cfg.SessionSecret,
|
Secret: cfg.SessionSecret,
|
||||||
Duration: cfg.SessionDuration,
|
Duration: cfg.SessionDuration,
|
||||||
Secure: cfg.SessionSecure,
|
Secure: cfg.SessionSecure,
|
||||||
|
PublicBaseURL: cfg.PublicBaseURL,
|
||||||
})
|
})
|
||||||
|
|
||||||
ctx, stop := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM)
|
ctx, stop := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM)
|
||||||
|
|||||||
@@ -26,6 +26,7 @@ var testSessionConfig = SessionConfig{
|
|||||||
Secret: []byte("test-session-secret-at-least-32-bytes-long"),
|
Secret: []byte("test-session-secret-at-least-32-bytes-long"),
|
||||||
Duration: time.Hour,
|
Duration: time.Hour,
|
||||||
Secure: false,
|
Secure: false,
|
||||||
|
PublicBaseURL: "https://geniusrun.example.com",
|
||||||
}
|
}
|
||||||
|
|
||||||
func newTestServer(t *testing.T) (*Server, *store.DB) {
|
func newTestServer(t *testing.T) (*Server, *store.DB) {
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
package api
|
package api
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"log"
|
||||||
"net/http"
|
"net/http"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
@@ -14,6 +15,12 @@ type SessionConfig struct {
|
|||||||
Secret []byte
|
Secret []byte
|
||||||
Duration time.Duration
|
Duration time.Duration
|
||||||
Secure bool
|
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 {
|
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) {
|
func (s *Server) handleSessionCallback(w http.ResponseWriter, r *http.Request) {
|
||||||
txnCookie, err := r.Cookie(auth.TxnCookieName)
|
txnCookie, err := r.Cookie(auth.TxnCookieName)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
log.Printf("session callback: missing txn cookie: %v", err)
|
||||||
http.Redirect(w, r, "/?auth_error=failed", http.StatusFound)
|
http.Redirect(w, r, "/?auth_error=failed", http.StatusFound)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
@@ -46,12 +54,14 @@ func (s *Server) handleSessionCallback(w http.ResponseWriter, r *http.Request) {
|
|||||||
|
|
||||||
txn, err := auth.ParseTxnCookie(txnCookie, s.Session.Secret)
|
txn, err := auth.ParseTxnCookie(txnCookie, s.Session.Secret)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
log.Printf("session callback: failed to parse txn cookie: %v", err)
|
||||||
http.Redirect(w, r, "/?auth_error=failed", http.StatusFound)
|
http.Redirect(w, r, "/?auth_error=failed", http.StatusFound)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
result, err := s.Auth.HandleCallback(r.Context(), txn, r.URL.Query())
|
result, err := s.Auth.HandleCallback(r.Context(), txn, r.URL.Query())
|
||||||
if err != nil {
|
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)
|
http.Redirect(w, r, "/?auth_error=failed", http.StatusFound)
|
||||||
return
|
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) {
|
func (s *Server) handleSessionLogout(w http.ResponseWriter, r *http.Request) {
|
||||||
http.SetCookie(w, auth.ClearCookie(auth.SessionCookieName, s.Session.Secure))
|
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) {
|
func (s *Server) handleSessionMe(w http.ResponseWriter, r *http.Request) {
|
||||||
|
|||||||
@@ -94,9 +94,14 @@ body {
|
|||||||
}
|
}
|
||||||
|
|
||||||
.logout-link {
|
.logout-link {
|
||||||
|
background: none;
|
||||||
|
border: none;
|
||||||
|
padding: 0;
|
||||||
|
font: inherit;
|
||||||
color: #9aa0ab;
|
color: #9aa0ab;
|
||||||
font-size: 0.85rem;
|
font-size: 0.85rem;
|
||||||
text-decoration: none;
|
text-decoration: none;
|
||||||
|
cursor: pointer;
|
||||||
}
|
}
|
||||||
|
|
||||||
.logout-link:hover {
|
.logout-link:hover {
|
||||||
|
|||||||
@@ -1,5 +1,5 @@
|
|||||||
import { useEffect, useState } from "react";
|
import { useEffect, useState } from "react";
|
||||||
import { api } from "./api/client";
|
import { api, BASE_URL } from "./api/client";
|
||||||
import "./App.css";
|
import "./App.css";
|
||||||
import { Dashboard } from "./pages/Dashboard";
|
import { Dashboard } from "./pages/Dashboard";
|
||||||
import { Plan } from "./pages/Plan";
|
import { Plan } from "./pages/Plan";
|
||||||
@@ -56,9 +56,11 @@ function App({ session }: { session: SessionInfo }) {
|
|||||||
>
|
>
|
||||||
{profileName ?? "Profile"}
|
{profileName ?? "Profile"}
|
||||||
</button>
|
</button>
|
||||||
<a className="logout-link" href="/api/session/logout" title={session.email}>
|
<form method="post" action={`${BASE_URL}/api/session/logout`} title={session.email}>
|
||||||
|
<button type="submit" className="logout-link">
|
||||||
Log out
|
Log out
|
||||||
</a>
|
</button>
|
||||||
|
</form>
|
||||||
</div>
|
</div>
|
||||||
</header>
|
</header>
|
||||||
<main>{showProfile ? <Profile onSaved={(p) => setProfileName(p.Name)} /> : <Active />}</main>
|
<main>{showProfile ? <Profile onSaved={(p) => setProfileName(p.Name)} /> : <Active />}</main>
|
||||||
|
|||||||
@@ -1,5 +1,5 @@
|
|||||||
import { useEffect, useState } from "react";
|
import { useEffect, useState } from "react";
|
||||||
import { api } from "./api/client";
|
import { api, BASE_URL } from "./api/client";
|
||||||
import "./LoginGate.css";
|
import "./LoginGate.css";
|
||||||
import App from "./App";
|
import App from "./App";
|
||||||
import type { SessionInfo } from "./types/api";
|
import type { SessionInfo } from "./types/api";
|
||||||
@@ -39,7 +39,7 @@ export function LoginGate() {
|
|||||||
<div className="login-gate">
|
<div className="login-gate">
|
||||||
<h1>🧞♀️ geniusrun</h1>
|
<h1>🧞♀️ geniusrun</h1>
|
||||||
{authError && <p className="login-gate-error">{AUTH_ERROR_MESSAGES[authError] ?? "Login failed, please try again."}</p>}
|
{authError && <p className="login-gate-error">{AUTH_ERROR_MESSAGES[authError] ?? "Login failed, please try again."}</p>}
|
||||||
<a className="login-gate-button" href="/api/session/login">
|
<a className="login-gate-button" href={`${BASE_URL}/api/session/login`}>
|
||||||
Log in
|
Log in
|
||||||
</a>
|
</a>
|
||||||
</div>
|
</div>
|
||||||
|
|||||||
@@ -13,7 +13,7 @@ import type {
|
|||||||
WorkoutKind,
|
WorkoutKind,
|
||||||
} from "../types/api";
|
} 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<T>(path: string, init?: RequestInit): Promise<T> {
|
async function request<T>(path: string, init?: RequestInit): Promise<T> {
|
||||||
const res = await fetch(`${BASE_URL}${path}`, {
|
const res = await fetch(`${BASE_URL}${path}`, {
|
||||||
@@ -39,9 +39,12 @@ async function request<T>(path: string, init?: RequestInit): Promise<T> {
|
|||||||
|
|
||||||
export const api = {
|
export const api = {
|
||||||
// Session (app login via OIDC -- distinct from the Garmin credential
|
// Session (app login via OIDC -- distinct from the Garmin credential
|
||||||
// login below). No logout()/login() methods: those are plain <a href>
|
// login below). No logout()/login() methods: login is a plain <a href>
|
||||||
// full-page navigations (see LoginGate.tsx), not fetches, since the OIDC
|
// and logout is a <form method="post"> submit button (see App.tsx /
|
||||||
// flow and Keycloak's own logout redirect need real browser navigation.
|
// 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
|
||||||
|
// <a href> can't do, hence the form.
|
||||||
getSessionInfo: () => request<SessionInfo>("/api/session/me"),
|
getSessionInfo: () => request<SessionInfo>("/api/session/me"),
|
||||||
|
|
||||||
// Auth
|
// Auth
|
||||||
|
|||||||
Reference in New Issue
Block a user