fix: IDEAS.md quickfixes — idle-timeout app config, id_token out of Claims, FormEvent import
session.idle_timeout (minutes, default 15) joins the app-config registry and drives the onboarding Garmin session eviction, distinct from session.duration (the login cookie lifetime in hours). The raw Keycloak ID token no longer rides in auth.Claims through every request context: it's minted into the session cookie separately and read back only by the logout handler via IDTokenFromSessionCookie. OnboardingWizard uses the type-imported FormEvent<HTMLFormElement> instead of the React.FormEvent namespace alias. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -65,7 +65,7 @@ func doJSON(t *testing.T, handler http.Handler, method, path string, body any) *
|
||||
}
|
||||
req := httptest.NewRequest(method, path, reader)
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
cookie, err := auth.MintSessionCookie(auth.Claims{Sub: "test-user", Name: "Test User", Email: "test@example.com"}, testSessionConfig.Secret, testSessionConfig.Duration, testSessionConfig.Secure)
|
||||
cookie, err := auth.MintSessionCookie(auth.Claims{Sub: "test-user", Name: "Test User", Email: "test@example.com"}, "", testSessionConfig.Secret, testSessionConfig.Duration, testSessionConfig.Secure)
|
||||
if err != nil {
|
||||
t.Fatalf("mint test session cookie: %v", err)
|
||||
}
|
||||
@@ -1081,7 +1081,8 @@ func TestSessionLogout_PassesIDTokenHintFromSessionCookie(t *testing.T) {
|
||||
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"},
|
||||
auth.Claims{Sub: "test-user", Name: "Test User", Email: "test@example.com"},
|
||||
"raw-id-token-jwt", // travels in the cookie apart from Claims
|
||||
testSessionConfig.Secret, testSessionConfig.Duration, testSessionConfig.Secure,
|
||||
)
|
||||
if err != nil {
|
||||
@@ -1107,8 +1108,9 @@ func TestSessionLogout_PassesIDTokenHintFromSessionCookie(t *testing.T) {
|
||||
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"},
|
||||
Claims: auth.Claims{Sub: "u1", Name: "Alice", Email: "alice@example.com"},
|
||||
Authorized: true,
|
||||
IDToken: "raw-id-token-jwt",
|
||||
},
|
||||
}
|
||||
s, _ := newTestServerWithAuth(t, verifier)
|
||||
@@ -1131,12 +1133,12 @@ func TestSessionCallback_MintsSessionCookieCarryingIDToken(t *testing.T) {
|
||||
if sessionCookie == nil {
|
||||
t.Fatal("expected a session cookie to be set")
|
||||
}
|
||||
claims, err := auth.ParseSessionCookie(sessionCookie, testSessionConfig.Secret)
|
||||
idToken, err := auth.IDTokenFromSessionCookie(sessionCookie, testSessionConfig.Secret)
|
||||
if err != nil {
|
||||
t.Fatalf("parse session cookie: %v", err)
|
||||
t.Fatalf("read id token from session cookie: %v", err)
|
||||
}
|
||||
if claims.IDToken != "raw-id-token-jwt" {
|
||||
t.Errorf("claims.IDToken = %q, want %q", claims.IDToken, "raw-id-token-jwt")
|
||||
if idToken != "raw-id-token-jwt" {
|
||||
t.Errorf("cookie id token = %q, want %q", idToken, "raw-id-token-jwt")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1177,7 +1179,7 @@ func TestRequestLoggingMiddleware_5xxLogsAtWarnLevel(t *testing.T) {
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "/api/profile", nil)
|
||||
req = req.WithContext(applog.WithLogger(req.Context(), logger))
|
||||
cookie, err := auth.MintSessionCookie(auth.Claims{Sub: "test-user", Name: "Test User", Email: "test@example.com"}, testSessionConfig.Secret, testSessionConfig.Duration, testSessionConfig.Secure)
|
||||
cookie, err := auth.MintSessionCookie(auth.Claims{Sub: "test-user", Name: "Test User", Email: "test@example.com"}, "", testSessionConfig.Secret, testSessionConfig.Duration, testSessionConfig.Secure)
|
||||
if err != nil {
|
||||
t.Fatalf("mint session cookie: %v", err)
|
||||
}
|
||||
|
||||
@@ -36,12 +36,27 @@ func TestConfig_GetDefaultsAndEnvSnapshot(t *testing.T) {
|
||||
if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil {
|
||||
t.Fatalf("unmarshal: %v", err)
|
||||
}
|
||||
if len(resp.Application) != 1 {
|
||||
t.Fatalf("expected 1 app-config entry, got %d", len(resp.Application))
|
||||
if len(resp.Application) != 2 {
|
||||
t.Fatalf("expected 2 app-config entries, got %d: %+v", len(resp.Application), resp.Application)
|
||||
}
|
||||
e := resp.Application[0]
|
||||
if e.Key != "session.duration" || e.Value != "720" || e.Default != "720" || e.Overridden || e.Description == "" {
|
||||
t.Fatalf("unexpected default entry: %+v", e)
|
||||
byKey := map[string]struct {
|
||||
value, def string
|
||||
overridden bool
|
||||
}{}
|
||||
for _, e := range resp.Application {
|
||||
if e.Description == "" {
|
||||
t.Errorf("entry %q has no description", e.Key)
|
||||
}
|
||||
byKey[e.Key] = struct {
|
||||
value, def string
|
||||
overridden bool
|
||||
}{e.Value, e.Default, e.Overridden}
|
||||
}
|
||||
if e := byKey["session.duration"]; e.value != "720" || e.def != "720" || e.overridden {
|
||||
t.Fatalf("session.duration default entry = %+v", e)
|
||||
}
|
||||
if e := byKey["session.idle_timeout"]; e.value != "15" || e.def != "15" || e.overridden {
|
||||
t.Fatalf("session.idle_timeout default entry = %+v", e)
|
||||
}
|
||||
if len(resp.Environment) != 1 || resp.Environment[0].Value != "•••• (set)" {
|
||||
t.Fatalf("env snapshot not passed through: %+v", resp.Environment)
|
||||
|
||||
@@ -31,7 +31,7 @@ func doJSONAs(t *testing.T, handler http.Handler, sub, method, path string, body
|
||||
}
|
||||
req := httptest.NewRequest(method, path, reader)
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
cookie, err := auth.MintSessionCookie(auth.Claims{Sub: sub, Name: sub, Email: sub + "@example.com"}, testSessionConfig.Secret, testSessionConfig.Duration, testSessionConfig.Secure)
|
||||
cookie, err := auth.MintSessionCookie(auth.Claims{Sub: sub, Name: sub, Email: sub + "@example.com"}, "", testSessionConfig.Secret, testSessionConfig.Duration, testSessionConfig.Secure)
|
||||
if err != nil {
|
||||
t.Fatalf("mint test session cookie: %v", err)
|
||||
}
|
||||
|
||||
@@ -53,6 +53,11 @@ type Server struct {
|
||||
// never call os.Getenv.
|
||||
EnvVars []EnvVar
|
||||
|
||||
// SetupSessionIdleTimeout is the app-config session.idle_timeout value
|
||||
// (see internal/config); zero falls back to
|
||||
// defaultSetupSessionIdleTimeout.
|
||||
SetupSessionIdleTimeout time.Duration
|
||||
|
||||
mu sync.Mutex
|
||||
userClient map[int64]garmin.Client
|
||||
userSync map[int64]*garmin.Sync
|
||||
|
||||
@@ -82,7 +82,7 @@ func (s *Server) handleSessionCallback(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
sessionCookie, err := auth.MintSessionCookie(result.Claims, s.SessionConfig.Secret, s.SessionConfig.Duration, s.SessionConfig.Secure)
|
||||
sessionCookie, err := auth.MintSessionCookie(result.Claims, result.IDToken, s.SessionConfig.Secret, s.SessionConfig.Duration, s.SessionConfig.Secure)
|
||||
if err != nil {
|
||||
writeError(w, http.StatusInternalServerError, err.Error())
|
||||
return
|
||||
@@ -93,15 +93,23 @@ func (s *Server) handleSessionCallback(w http.ResponseWriter, r *http.Request) {
|
||||
|
||||
// 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).
|
||||
// as id_token_hint (read back from the cookie via
|
||||
// auth.IDTokenFromSessionCookie -- it deliberately doesn't ride in Claims)
|
||||
// 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())
|
||||
s.removeSetupSession(claims.Sub)
|
||||
// Best-effort: an unreadable cookie just means logging out without the
|
||||
// hint, at worst showing Keycloak's own confirmation screen.
|
||||
var idToken string
|
||||
if cookie, err := r.Cookie(auth.SessionCookieName); err == nil {
|
||||
idToken, _ = auth.IDTokenFromSessionCookie(cookie, s.SessionConfig.Secret)
|
||||
}
|
||||
http.SetCookie(w, auth.ClearCookie(auth.SessionCookieName, s.SessionConfig.Secure))
|
||||
http.Redirect(w, r, s.Auth.EndSessionURL(s.SessionConfig.FrontendURL+"/", claims.IDToken), http.StatusFound)
|
||||
http.Redirect(w, r, s.Auth.EndSessionURL(s.SessionConfig.FrontendURL+"/", idToken), http.StatusFound)
|
||||
}
|
||||
|
||||
func (s *Server) handleSessionMe(w http.ResponseWriter, r *http.Request) {
|
||||
|
||||
@@ -13,11 +13,23 @@ import (
|
||||
applog "geniusrun/backend/internal/log"
|
||||
)
|
||||
|
||||
// setupSessionIdleTimeout bounds how long an onboarding Garmin session
|
||||
// survives without being touched (login, MFA, or complete) before it's
|
||||
// evicted -- long enough to check email for an MFA code, short enough that
|
||||
// an abandoned attempt doesn't leave a subprocess running indefinitely.
|
||||
const setupSessionIdleTimeout = 15 * time.Minute
|
||||
// defaultSetupSessionIdleTimeout bounds how long an onboarding Garmin
|
||||
// session survives without being touched (login, MFA, or complete) before
|
||||
// it's evicted -- long enough to check email for an MFA code, short enough
|
||||
// that an abandoned attempt doesn't leave a subprocess running
|
||||
// indefinitely. Tunable via the session.idle_timeout application-config
|
||||
// key (minutes -- distinct from session.duration, the login cookie
|
||||
// lifetime); this constant is the fallback when the Server field was never
|
||||
// wired (tests building a bare NewServer).
|
||||
const defaultSetupSessionIdleTimeout = 15 * time.Minute
|
||||
|
||||
// setupIdleTimeout returns the configured onboarding-session idle timeout.
|
||||
func (s *Server) setupIdleTimeout() time.Duration {
|
||||
if s.SetupSessionIdleTimeout > 0 {
|
||||
return s.SetupSessionIdleTimeout
|
||||
}
|
||||
return defaultSetupSessionIdleTimeout
|
||||
}
|
||||
|
||||
// setupSession is a temporary, not-yet-persisted Garmin authentication
|
||||
// attempt made during onboarding, before any users/profile row exists --
|
||||
@@ -30,7 +42,7 @@ const setupSessionIdleTimeout = 15 * time.Minute
|
||||
// the next time anything closes and restarts its subprocess; a later
|
||||
// garminFor(ctx, userID) call builds a fresh client with the correct path
|
||||
// instead. Otherwise evicted lazily (the next setup-endpoint touch for that
|
||||
// subject checks staleness first) once idle past setupSessionIdleTimeout.
|
||||
// subject checks staleness first) once idle past setupIdleTimeout().
|
||||
type setupSession struct {
|
||||
Client garmin.Client
|
||||
Email, Password string
|
||||
@@ -213,7 +225,7 @@ func (s *Server) setupSessionFor(sub string) (*setupSession, bool) {
|
||||
if !ok {
|
||||
return nil, false
|
||||
}
|
||||
if time.Since(sess.LastUsed) > setupSessionIdleTimeout {
|
||||
if time.Since(sess.LastUsed) > s.setupIdleTimeout() {
|
||||
sess.Client.Close()
|
||||
delete(s.setupSession, sub)
|
||||
if s.ClientConfig.TokenStorePath != "" {
|
||||
|
||||
Reference in New Issue
Block a user