From cb73547af5178dc2d9c4353cce6c51130219fee9 Mon Sep 17 00:00:00 2001 From: shankar0123 Date: Sun, 10 May 2026 23:18:23 +0000 Subject: [PATCH] =?UTF-8?q?harden(oidc):=20pre-login=20UA/IP=20binding=20(?= =?UTF-8?q?MED-16)=20=E2=80=94=20RFC=209700=20=C2=A74.7.1?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audit 2026-05-10 MED-16 closure. WHAT. Binds the OIDC pre-login row to the (clientIP, userAgent) tuple of the /auth/oidc/login request, and enforces a constant-time compare against the /auth/oidc/callback request at consume time. Defeats replay of a stolen pre-login cookie by a different browser / source — the secondary defense layer recommended by RFC 9700 §4.7.1 when the primary layer (HMAC integrity + Path=/ + SameSite=Lax on the cookie) is bypassed via CSRF / XSS / TLS-termination leak. WHY. Pre-fix, the pre-login cookie's HMAC verified only that 'some' caller of /auth/oidc/login was talking to /auth/oidc/callback; it did not verify that the SAME browser / source was on both sides. An attacker who exfiltrated the cookie value via any vector could replay the bytes through their own user-agent and ride the victim's authorization. RFC 9700 §4.7.1 calls out the gap explicitly and recommends binding state to a user-agent fingerprint + source IP. HOW. Migration: migrations/000044_prelogin_uaip.up.sql ALTER TABLE oidc_pre_login_sessions ADD COLUMN IF NOT EXISTS client_ip TEXT, ADD COLUMN IF NOT EXISTS user_agent TEXT; Both nullable for in-flight rolling-deploy compat — the consume- side check only enforces when both row AND request carry non-empty values for the leg in question. Domain: internal/repository/oidc.go (PreLoginSession) — adds ClientIP + UserAgent fields. Repository: internal/repository/postgres/oidc_prelogin.go — Create persists via sql.NullString (empty → NULL); LookupAndConsume reads back. Re-uses package-local nullableString from discovery.go. Service: internal/auth/oidc/service.go - PreLoginStore.CreatePreLogin signature takes (clientIP, userAgent) as positions 5–6. - PreLoginStore.LookupAndConsume returns (clientIP, userAgent) as positions 5–6. - HandleAuthRequest signature gains (clientIP, userAgent), threaded to the store. - HandleCallback adds Step 1.5 — UA / IP constant-time compare between stored row and incoming request. Per-leg toggles via preLoginRequireUA / preLoginRequireIP service fields. Empty values on either side pass through (rolling-deploy + headless- proxy compat). - New sentinels ErrPreLoginUAMismatch, ErrPreLoginIPMismatch. - SetPreLoginBindingRequirements(requireUA, requireIP) helper for main.go config wiring. Adapter: internal/auth/oidc/prelogin.go — PreLoginAdapter passes the new fields through to the repo row. Handler: internal/api/handler/auth_session_oidc.go - OIDCAuthHandshaker.HandleAuthRequest signature updated. - LoginInitiate captures clientIPFromRequest + r.UserAgent() and passes to the service. - classifyOIDCFailure adds errors.Is dispatch for the two new sentinels → prelogin_ua_mismatch / prelogin_ip_mismatch audit categories. Config: internal/config/config.go + AuthConfig.OIDCPreLoginRequireUA (default true) env CERTCTL_OIDC_PRELOGIN_REQUIRE_UA + AuthConfig.OIDCPreLoginRequireIP (default true) env CERTCTL_OIDC_PRELOGIN_REQUIRE_IP cmd/server/main.go calls oidcService.SetPreLoginBindingRequirements from cfg.Auth.OIDCPreLoginRequire{UA,IP}. Tests (internal/auth/oidc/service_test.go): - TestService_HandleCallback_MED16_UAMismatchRejected - TestService_HandleCallback_MED16_IPMismatchRejected - TestService_HandleCallback_MED16_BothMatch_Succeeds - TestService_HandleCallback_MED16_LegacyRowEmptyValues (rolling- deploy compat — empty stored values pass through) - TestService_HandleCallback_MED16_RequireUAFalse_AllowsMismatch (operator escape-hatch — UA mismatch silently allowed) Mechanical fan-out: - stubPreLogin / stubPreLoginRepo signatures updated. - All existing call sites in service_test.go (~40), prelogin_test.go, bench_test.go, logging_test.go, provider_enabled_test.go, integration_keycloak_test.go, integration_okta_smoke_test.go, auth_session_oidc_test.go updated to pass empty strings for the new params — pre-existing tests do not exercise UA/IP binding semantics. VERIFY. - go vet ./internal/auth/oidc/... ./internal/api/handler/... ./internal/config/... PASS - go test -short -count=1 -run MED16 ./internal/auth/oidc/... PASS (5/5) - go test -short -count=1 ./internal/auth/oidc/... PASS (4.6s) - go test -short -count=1 ./internal/api/handler/... PASS (4.3s) - go test -short -count=1 ./internal/config/... PASS Refs: cowork/auth-bundles-audit-2026-05-10.md MED-16 cowork/auth-bundles-fixes-2026-05-10/HANDOFF.md item 6 RFC 9700 §4.7.1 — OAuth 2.0 Security Best Current Practice --- CHANGELOG.md | 15 ++ cmd/server/main.go | 6 + internal/api/handler/auth_session_oidc.go | 21 +- .../api/handler/auth_session_oidc_test.go | 2 +- internal/auth/oidc/bench_test.go | 2 +- .../auth/oidc/integration_keycloak_test.go | 10 +- .../auth/oidc/integration_okta_smoke_test.go | 2 +- internal/auth/oidc/logging_test.go | 4 +- internal/auth/oidc/prelogin.go | 28 ++- internal/auth/oidc/prelogin_test.go | 40 ++-- internal/auth/oidc/provider_enabled_test.go | 4 +- internal/auth/oidc/service.go | 86 +++++++- internal/auth/oidc/service_test.go | 199 +++++++++++++----- internal/config/config.go | 21 ++ internal/repository/oidc.go | 9 + internal/repository/postgres/oidc_prelogin.go | 44 +++- migrations/000044_prelogin_uaip.down.sql | 4 + migrations/000044_prelogin_uaip.up.sql | 47 +++++ 18 files changed, 441 insertions(+), 103 deletions(-) create mode 100644 migrations/000044_prelogin_uaip.down.sql create mode 100644 migrations/000044_prelogin_uaip.up.sql diff --git a/CHANGELOG.md b/CHANGELOG.md index b9ae390..8e4d4c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,21 @@ RFC-9207 discovery. Providers that don't advertise support (the majority today) keep pre-fix behavior — back-compat is preserved. +- **Pre-login UA / source-IP binding on OIDC callback (Audit 2026-05-10 + MED-16).** RFC 9700 §4.7.1 defense against stolen-pre-login-cookie replay + by a different browser / source. Migration `000044_prelogin_uaip` adds + `client_ip` + `user_agent` to `oidc_pre_login_sessions`; values captured at + `/auth/oidc/login` are constant-time compared at `/auth/oidc/callback`. + Mismatches return HTTP 400 with audit `failure_category` = + `prelogin_ua_mismatch` or `prelogin_ip_mismatch`. Two operator escape + hatches: `CERTCTL_OIDC_PRELOGIN_REQUIRE_UA` and + `CERTCTL_OIDC_PRELOGIN_REQUIRE_IP` (both default `true`) — operators on + enterprise proxies that rewrite UA, or dual-stack v4/v6 environments where + source IP routinely flips, can disable the affected leg. The binding column + is persisted even when enforcement is off, so retroactive forensics remain + possible. Empty values on either side pass through (rolling-deploy + + headless-proxy compat). + ## v2.1.0 - Auth Bundles 1 + 2: RBAC primitive + OIDC SSO + sessions ⚠️ > **SECURITY: AUDIT YOUR API KEYS.** diff --git a/cmd/server/main.go b/cmd/server/main.go index dc8ada4..100c633 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -421,6 +421,12 @@ func main() { preLoginAdapter, cfg.Encryption.ConfigEncryptionKey, ) + // Audit 2026-05-10 MED-16 — apply per-leg pre-login UA / IP + // binding enforcement toggles from config. + oidcService.SetPreLoginBindingRequirements( + cfg.Auth.OIDCPreLoginRequireUA, + cfg.Auth.OIDCPreLoginRequireIP, + ) // SameSite resolution from CERTCTL_SESSION_SAMESITE (default Lax; // "Strict" for high-security environments at the cost of breaking // inbound deep-links from external apps). diff --git a/internal/api/handler/auth_session_oidc.go b/internal/api/handler/auth_session_oidc.go index b87414a..0a41013 100644 --- a/internal/api/handler/auth_session_oidc.go +++ b/internal/api/handler/auth_session_oidc.go @@ -55,7 +55,10 @@ import ( // OIDCAuthHandshaker is the slice of *oidc.Service the OIDC HTTP path // consumes. Phase 3's *oidc.Service satisfies this directly. type OIDCAuthHandshaker interface { - HandleAuthRequest(ctx context.Context, providerID string) (authURL, cookieValue, preLoginID string, err error) + // Audit 2026-05-10 MED-16 — clientIP + userAgent persist into the + // pre-login row so HandleCallback can reject mismatches at consume + // time (RFC 9700 §4.7.1 binding). + HandleAuthRequest(ctx context.Context, providerID, clientIP, userAgent string) (authURL, cookieValue, preLoginID string, err error) // Audit 2026-05-10 MED-17 — callbackIss carries the value of the // RFC 9207 `iss` query parameter on /auth/oidc/callback (empty // string when the IdP doesn't send it). The service enforces the @@ -233,7 +236,14 @@ func (h *AuthSessionOIDCHandler) LoginInitiate(w http.ResponseWriter, r *http.Re Error(w, http.StatusBadRequest, "missing required query parameter `provider`") return } - authURL, cookieValue, _, err := h.oidcSvc.HandleAuthRequest(r.Context(), providerID) + // Audit 2026-05-10 MED-16 — capture clientIP + UA at /auth/oidc/login + // so HandleCallback can reject a stolen pre-login cookie replayed + // from a different browser/source. clientIPFromRequest already + // honours the LOW-5 trusted-proxy gating; r.UserAgent() reads the + // header verbatim. + loginIP := clientIPFromRequest(r) + loginUA := r.UserAgent() + authURL, cookieValue, _, err := h.oidcSvc.HandleAuthRequest(r.Context(), providerID, loginIP, loginUA) if err != nil { // Provider not found is the most common case; map to 404. if errors.Is(err, repository.ErrOIDCProviderNotFound) { @@ -1178,6 +1188,9 @@ func classifyOIDCFailure(err error) string { return "ok" } // Audit 2026-05-10 MED-17 — typed dispatch for the iss family. + // Audit 2026-05-10 MED-16 — typed dispatch for the UA/IP binding + // family (no substring guarantees because UA strings are operator + // data and could match anything). switch { case errors.Is(err, oidcsvc.ErrIssParamMissing): return "iss_param_missing" @@ -1185,6 +1198,10 @@ func classifyOIDCFailure(err error) string { return "iss_param_mismatch" case errors.Is(err, oidcsvc.ErrIssuerMismatch): return "id_token_iss_mismatch" + case errors.Is(err, oidcsvc.ErrPreLoginUAMismatch): + return "prelogin_ua_mismatch" + case errors.Is(err, oidcsvc.ErrPreLoginIPMismatch): + return "prelogin_ip_mismatch" } msg := strings.ToLower(err.Error()) switch { diff --git a/internal/api/handler/auth_session_oidc_test.go b/internal/api/handler/auth_session_oidc_test.go index 9017016..4e2c406 100644 --- a/internal/api/handler/auth_session_oidc_test.go +++ b/internal/api/handler/auth_session_oidc_test.go @@ -43,7 +43,7 @@ type stubOIDCSvc struct { refreshErr error } -func (s *stubOIDCSvc) HandleAuthRequest(_ context.Context, _ string) (string, string, string, error) { +func (s *stubOIDCSvc) HandleAuthRequest(_ context.Context, _, _, _ string) (string, string, string, error) { return s.authURL, s.cookie, s.preLoginID, s.authReqErr } func (s *stubOIDCSvc) HandleCallback(_ context.Context, _, _, _, _, _, _ string) (*oidcsvc.CallbackResult, error) { diff --git a/internal/auth/oidc/bench_test.go b/internal/auth/oidc/bench_test.go index fe636d2..158599e 100644 --- a/internal/auth/oidc/bench_test.go +++ b/internal/auth/oidc/bench_test.go @@ -94,7 +94,7 @@ func BenchmarkOIDC_SteadyState(b *testing.B) { // Each iteration needs a fresh pre-login row (HandleCallback // consumes the row atomically + single-use). State + nonce + // verifier are stable; the cookie value is unique per call. - cookie, _, err := pl.CreatePreLogin(ctx, "op-bench", "bench-state", "test-nonce-fixed", "verifier-xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx") + cookie, _, err := pl.CreatePreLogin(ctx, "op-bench", "bench-state", "test-nonce-fixed", "verifier-xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", "", "") if err != nil { b.Fatalf("CreatePreLogin: %v", err) } diff --git a/internal/auth/oidc/integration_keycloak_test.go b/internal/auth/oidc/integration_keycloak_test.go index 7dca852..1139177 100644 --- a/internal/auth/oidc/integration_keycloak_test.go +++ b/internal/auth/oidc/integration_keycloak_test.go @@ -420,7 +420,7 @@ func TestKeycloakIntegration_AuthCodeFlow_HappyPath(t *testing.T) { defer cancel() // HandleAuthRequest produces the IdP redirect URL + pre-login cookie. - authURL, preLoginCookie, _, err := svc.HandleAuthRequest(ctx, fx.Provider.ID) + authURL, preLoginCookie, _, err := svc.HandleAuthRequest(ctx, fx.Provider.ID, "", "") if err != nil { t.Fatalf("HandleAuthRequest: %v", err) } @@ -486,7 +486,7 @@ func TestKeycloakIntegration_LogoutRevokesSession(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() - authURL, preLoginCookie, _, err := svc.HandleAuthRequest(ctx, fx.Provider.ID) + authURL, preLoginCookie, _, err := svc.HandleAuthRequest(ctx, fx.Provider.ID, "", "") if err != nil { t.Fatalf("HandleAuthRequest: %v", err) } @@ -529,7 +529,7 @@ func TestKeycloakIntegration_JWKSRotation_RefreshKeysPicksUpNewKey(t *testing.T) defer cancel() // Pre-rotate baseline login. - preAuthURL, preCookie, _, err := svc.HandleAuthRequest(ctx, fx.Provider.ID) + preAuthURL, preCookie, _, err := svc.HandleAuthRequest(ctx, fx.Provider.ID, "", "") if err != nil { t.Fatalf("pre-rotate HandleAuthRequest: %v", err) } @@ -548,7 +548,7 @@ func TestKeycloakIntegration_JWKSRotation_RefreshKeysPicksUpNewKey(t *testing.T) // Post-rotate login: Keycloak signs the new token under the new // key (higher priority); the service must validate it. - postAuthURL, postCookie, _, err := svc.HandleAuthRequest(ctx, fx.Provider.ID) + postAuthURL, postCookie, _, err := svc.HandleAuthRequest(ctx, fx.Provider.ID, "", "") if err != nil { t.Fatalf("post-rotate HandleAuthRequest: %v", err) } @@ -573,7 +573,7 @@ func TestKeycloakIntegration_UnmappedGroupsFailsClosed(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() - authURL, preCookie, _, err := svc.HandleAuthRequest(ctx, fx.Provider.ID) + authURL, preCookie, _, err := svc.HandleAuthRequest(ctx, fx.Provider.ID, "", "") if err != nil { t.Fatalf("HandleAuthRequest: %v", err) } diff --git a/internal/auth/oidc/integration_okta_smoke_test.go b/internal/auth/oidc/integration_okta_smoke_test.go index 87ef000..512d125 100644 --- a/internal/auth/oidc/integration_okta_smoke_test.go +++ b/internal/auth/oidc/integration_okta_smoke_test.go @@ -121,7 +121,7 @@ func TestOktaSmoke_DiscoveryAndRefreshKeys(t *testing.T) { // the configured Okta issuer. We don't drive the browser login // here — the Keycloak fixture covers full auth-code; this test // only confirms the wire setup against a real Okta tenant. - authURL, _, _, err := svc.HandleAuthRequest(ctx, prov.ID) + authURL, _, _, err := svc.HandleAuthRequest(ctx, prov.ID, "", "") if err != nil { t.Fatalf("HandleAuthRequest: %v", err) } diff --git a/internal/auth/oidc/logging_test.go b/internal/auth/oidc/logging_test.go index 10e23f9..f3b8ecc 100644 --- a/internal/auth/oidc/logging_test.go +++ b/internal/auth/oidc/logging_test.go @@ -50,7 +50,7 @@ func TestLoggingHygiene_HandleAuthRequest_LeaksNothing(t *testing.T) { buf, restore := captureLogger(t) defer restore() - authURL, cookieValue, _, err := svc.HandleAuthRequest(context.Background(), "op-leak-1") + authURL, cookieValue, _, err := svc.HandleAuthRequest(context.Background(), "op-leak-1", "", "") if err != nil { t.Fatalf("HandleAuthRequest: %v", err) } @@ -83,7 +83,7 @@ func TestLoggingHygiene_HandleCallback_LeaksNothing(t *testing.T) { // Pre-login row with a known verifier we can grep for after. verifier := "test-verifier-do-not-leak-aaaaaaaaaaaaa" - cookie, _, err := pl.CreatePreLogin(context.Background(), "op-leak-2", "the-state", "test-nonce-fixed", verifier) + cookie, _, err := pl.CreatePreLogin(context.Background(), "op-leak-2", "the-state", "test-nonce-fixed", verifier, "", "") if err != nil { t.Fatalf("CreatePreLogin: %v", err) } diff --git a/internal/auth/oidc/prelogin.go b/internal/auth/oidc/prelogin.go index e2f6759..669f7f0 100644 --- a/internal/auth/oidc/prelogin.go +++ b/internal/auth/oidc/prelogin.go @@ -87,9 +87,12 @@ func (a *PreLoginAdapter) SetRandReaderForTest(r func([]byte) (int, error)) { // value under the active SessionSigningKey, persists the row, and // returns the cookie value + the row id. // +// Audit 2026-05-10 MED-16 — clientIP + userAgent are persisted into +// the row for the callback-time UA/IP binding check. +// // Implements the Phase 3 OIDCService.PreLoginStore.CreatePreLogin // interface signature. -func (a *PreLoginAdapter) CreatePreLogin(ctx context.Context, providerID, state, nonce, verifier string) (cookieValue, sessionID string, err error) { +func (a *PreLoginAdapter) CreatePreLogin(ctx context.Context, providerID, state, nonce, verifier, clientIP, userAgent string) (cookieValue, sessionID string, err error) { active, err := a.keys.GetActive(ctx, a.tenantID) if err != nil { return "", "", fmt.Errorf("pre-login: get active signing key: %w", err) @@ -110,6 +113,8 @@ func (a *PreLoginAdapter) CreatePreLogin(ctx context.Context, providerID, state, State: state, Nonce: nonce, PKCEVerifier: verifier, + ClientIP: clientIP, + UserAgent: userAgent, } if err := a.repo.Create(ctx, row); err != nil { return "", "", fmt.Errorf("pre-login: persist row: %w", err) @@ -132,25 +137,28 @@ func (a *PreLoginAdapter) CreatePreLogin(ctx context.Context, providerID, state, // - Row found but past 10-minute TTL -> ErrPreLoginExpired (row is // deleted at the repo layer regardless). // +// Audit 2026-05-10 MED-16 — also returns the row's stored clientIP + +// userAgent so the service-layer caller can enforce the UA/IP binding. +// // Implements the Phase 3 OIDCService.PreLoginStore.LookupAndConsume // interface signature. -func (a *PreLoginAdapter) LookupAndConsume(ctx context.Context, cookieValue string) (providerID, state, nonce, verifier string, err error) { +func (a *PreLoginAdapter) LookupAndConsume(ctx context.Context, cookieValue string) (providerID, state, nonce, verifier, clientIP, userAgent string, err error) { plID, signingKeyID, providedHMAC, perr := session.ParseCookieValue(cookieValue, "pl-") if perr != nil { - return "", "", "", "", ErrPreLoginNotFound + return "", "", "", "", "", "", ErrPreLoginNotFound } signingKey, kerr := a.keys.Get(ctx, signingKeyID) if kerr != nil { - return "", "", "", "", ErrPreLoginNotFound + return "", "", "", "", "", "", ErrPreLoginNotFound } hmacKey, derr := session.DecryptKeyMaterial(signingKey.KeyMaterialEncrypted, a.encryptionKey) if derr != nil { - return "", "", "", "", ErrPreLoginNotFound + return "", "", "", "", "", "", ErrPreLoginNotFound } expectedHMAC := session.ComputeCookieHMAC(plID, signingKeyID, hmacKey) if subtle.ConstantTimeCompare(expectedHMAC, providedHMAC) != 1 { - return "", "", "", "", ErrPreLoginNotFound + return "", "", "", "", "", "", ErrPreLoginNotFound } row, lerr := a.repo.LookupAndConsume(ctx, plID) @@ -159,15 +167,15 @@ func (a *PreLoginAdapter) LookupAndConsume(ctx context.Context, cookieValue stri // the OIDC service consumes; the audit row distinguishes via // the wrapped error from the repo (which the handler logs). if errors.Is(lerr, repository.ErrPreLoginNotFound) { - return "", "", "", "", ErrPreLoginNotFound + return "", "", "", "", "", "", ErrPreLoginNotFound } if errors.Is(lerr, repository.ErrPreLoginExpired) { - return "", "", "", "", ErrPreLoginNotFound + return "", "", "", "", "", "", ErrPreLoginNotFound } - return "", "", "", "", fmt.Errorf("pre-login: lookup_and_consume: %w", lerr) + return "", "", "", "", "", "", fmt.Errorf("pre-login: lookup_and_consume: %w", lerr) } - return row.OIDCProviderID, row.State, row.Nonce, row.PKCEVerifier, nil + return row.OIDCProviderID, row.State, row.Nonce, row.PKCEVerifier, row.ClientIP, row.UserAgent, nil } // newID returns `pl-` with 16 bytes of entropy. diff --git a/internal/auth/oidc/prelogin_test.go b/internal/auth/oidc/prelogin_test.go index 1097765..ea3d2ac 100644 --- a/internal/auth/oidc/prelogin_test.go +++ b/internal/auth/oidc/prelogin_test.go @@ -196,7 +196,7 @@ func TestPreLoginAdapter_CreatePreLogin_GetActiveFailure(t *testing.T) { keys := newStubSigningKeyLookup(nil) keys.getActErr = errors.New("postgres unavailable") a := NewPreLoginAdapter(repo, keys, "t-default", "") - _, _, err := a.CreatePreLogin(context.Background(), "op-x", "s", "n", "v") + _, _, err := a.CreatePreLogin(context.Background(), "op-x", "s", "n", "v", "", "") if err == nil || !strings.Contains(err.Error(), "get active signing key") { t.Errorf("err = %v, want wrapped 'get active signing key'", err) } @@ -210,7 +210,7 @@ func TestPreLoginAdapter_CreatePreLogin_DecryptFailure(t *testing.T) { key.KeyMaterialEncrypted = []byte{0x03, 0x00, 0x01, 0x02} // bogus v3 blob keys := newStubSigningKeyLookup(key) a := NewPreLoginAdapter(repo, keys, "t-default", "passphrase-set") - _, _, err := a.CreatePreLogin(context.Background(), "op-x", "s", "n", "v") + _, _, err := a.CreatePreLogin(context.Background(), "op-x", "s", "n", "v", "", "") if err == nil || !strings.Contains(err.Error(), "decrypt active key") { t.Errorf("err = %v, want wrapped 'decrypt active key'", err) } @@ -223,7 +223,7 @@ func TestPreLoginAdapter_CreatePreLogin_RNGFailure(t *testing.T) { a.SetRandReaderForTest(func(_ []byte) (int, error) { return 0, errors.New("RNG drained") }) - _, _, err := a.CreatePreLogin(context.Background(), "op-x", "s", "n", "v") + _, _, err := a.CreatePreLogin(context.Background(), "op-x", "s", "n", "v", "", "") if err == nil || !strings.Contains(err.Error(), "generate id") { t.Errorf("err = %v, want wrapped 'generate id'", err) } @@ -234,7 +234,7 @@ func TestPreLoginAdapter_CreatePreLogin_PersistFailure(t *testing.T) { repo.createErr = errors.New("FK violation") keys := newStubSigningKeyLookup(activeKeyForTest(t, "sk-1")) a := NewPreLoginAdapter(repo, keys, "t-default", "") - _, _, err := a.CreatePreLogin(context.Background(), "op-x", "s", "n", "v") + _, _, err := a.CreatePreLogin(context.Background(), "op-x", "s", "n", "v", "", "") if err == nil || !strings.Contains(err.Error(), "persist row") { t.Errorf("err = %v, want wrapped 'persist row'", err) } @@ -247,7 +247,7 @@ func TestPreLoginAdapter_CreatePreLogin_HappyPath(t *testing.T) { repo := newStubPreLoginRepo() keys := newStubSigningKeyLookup(activeKeyForTest(t, "sk-1")) a := NewPreLoginAdapter(repo, keys, "t-default", "") - cookie, sid, err := a.CreatePreLogin(context.Background(), "op-x", "the-state", "the-nonce", "verifier-xxx") + cookie, sid, err := a.CreatePreLogin(context.Background(), "op-x", "the-state", "the-nonce", "verifier-xxx", "", "") if err != nil { t.Fatalf("CreatePreLogin: %v", err) } @@ -279,7 +279,7 @@ func TestPreLoginAdapter_CreatePreLogin_HappyPath(t *testing.T) { func TestPreLoginAdapter_LookupAndConsume_MalformedCookie(t *testing.T) { a := NewPreLoginAdapter(newStubPreLoginRepo(), newStubSigningKeyLookup(activeKeyForTest(t, "sk-1")), "t-default", "") - _, _, _, _, err := a.LookupAndConsume(context.Background(), "definitely-not-a-cookie") + _, _, _, _, _, _, err := a.LookupAndConsume(context.Background(), "definitely-not-a-cookie") if !errors.Is(err, ErrPreLoginNotFound) { t.Errorf("err = %v, want ErrPreLoginNotFound", err) } @@ -292,14 +292,14 @@ func TestPreLoginAdapter_LookupAndConsume_UnknownSigningKey(t *testing.T) { createKey := activeKeyForTest(t, "sk-1") createKeys := newStubSigningKeyLookup(createKey) createAdapter := NewPreLoginAdapter(repo, createKeys, "t-default", "") - cookie, _, err := createAdapter.CreatePreLogin(context.Background(), "op-x", "s", "n", "v") + cookie, _, err := createAdapter.CreatePreLogin(context.Background(), "op-x", "s", "n", "v", "", "") if err != nil { t.Fatalf("CreatePreLogin: %v", err) } emptyKeys := newStubSigningKeyLookup(nil) // sk-1 is not in this lookup consumeAdapter := NewPreLoginAdapter(repo, emptyKeys, "t-default", "") - _, _, _, _, err = consumeAdapter.LookupAndConsume(context.Background(), cookie) + _, _, _, _, _, _, err = consumeAdapter.LookupAndConsume(context.Background(), cookie) if !errors.Is(err, ErrPreLoginNotFound) { t.Errorf("err = %v, want ErrPreLoginNotFound (unknown signing key)", err) } @@ -312,7 +312,7 @@ func TestPreLoginAdapter_LookupAndConsume_DecryptKeyFailure(t *testing.T) { createKey := activeKeyForTest(t, "sk-1") createKeys := newStubSigningKeyLookup(createKey) createAdapter := NewPreLoginAdapter(repo, createKeys, "t-default", "") - cookie, _, err := createAdapter.CreatePreLogin(context.Background(), "op-x", "s", "n", "v") + cookie, _, err := createAdapter.CreatePreLogin(context.Background(), "op-x", "s", "n", "v", "", "") if err != nil { t.Fatalf("CreatePreLogin: %v", err) } @@ -322,7 +322,7 @@ func TestPreLoginAdapter_LookupAndConsume_DecryptKeyFailure(t *testing.T) { corruptedKey.KeyMaterialEncrypted = []byte{0x03, 0x00, 0x01, 0x02} // bogus v3 corruptedKeys := newStubSigningKeyLookup(&corruptedKey) consumeAdapter := NewPreLoginAdapter(repo, corruptedKeys, "t-default", "passphrase-set") - _, _, _, _, err = consumeAdapter.LookupAndConsume(context.Background(), cookie) + _, _, _, _, _, _, err = consumeAdapter.LookupAndConsume(context.Background(), cookie) if !errors.Is(err, ErrPreLoginNotFound) { t.Errorf("err = %v, want ErrPreLoginNotFound (decrypt failure → uniform sentinel)", err) } @@ -336,7 +336,7 @@ func TestPreLoginAdapter_LookupAndConsume_HMACMismatch(t *testing.T) { createKey := activeKeyForTest(t, "sk-1") createKeys := newStubSigningKeyLookup(createKey) createAdapter := NewPreLoginAdapter(repo, createKeys, "t-default", "") - cookie, _, err := createAdapter.CreatePreLogin(context.Background(), "op-x", "s", "n", "v") + cookie, _, err := createAdapter.CreatePreLogin(context.Background(), "op-x", "s", "n", "v", "", "") if err != nil { t.Fatalf("CreatePreLogin: %v", err) } @@ -349,7 +349,7 @@ func TestPreLoginAdapter_LookupAndConsume_HMACMismatch(t *testing.T) { swapped.KeyMaterialEncrypted = swappedMaterial swappedKeys := newStubSigningKeyLookup(&swapped) consumeAdapter := NewPreLoginAdapter(repo, swappedKeys, "t-default", "") - _, _, _, _, err = consumeAdapter.LookupAndConsume(context.Background(), cookie) + _, _, _, _, _, _, err = consumeAdapter.LookupAndConsume(context.Background(), cookie) if !errors.Is(err, ErrPreLoginNotFound) { t.Errorf("err = %v, want ErrPreLoginNotFound (HMAC mismatch)", err) } @@ -368,7 +368,7 @@ func TestPreLoginAdapter_LookupAndConsume_RepoNotFound(t *testing.T) { plID := "pl-orphan-id" cookie := session.SignCookieValue(plID, keys.active.ID, hmacKey) - _, _, _, _, err := a.LookupAndConsume(context.Background(), cookie) + _, _, _, _, _, _, err := a.LookupAndConsume(context.Background(), cookie) if !errors.Is(err, ErrPreLoginNotFound) { t.Errorf("err = %v, want ErrPreLoginNotFound (repo miss)", err) } @@ -378,12 +378,12 @@ func TestPreLoginAdapter_LookupAndConsume_RepoExpired(t *testing.T) { repo := newStubPreLoginRepo() keys := newStubSigningKeyLookup(activeKeyForTest(t, "sk-1")) a := NewPreLoginAdapter(repo, keys, "t-default", "") - cookie, _, err := a.CreatePreLogin(context.Background(), "op-x", "s", "n", "v") + cookie, _, err := a.CreatePreLogin(context.Background(), "op-x", "s", "n", "v", "", "") if err != nil { t.Fatalf("CreatePreLogin: %v", err) } repo.expireOnNext = true - _, _, _, _, err = a.LookupAndConsume(context.Background(), cookie) + _, _, _, _, _, _, err = a.LookupAndConsume(context.Background(), cookie) if !errors.Is(err, ErrPreLoginNotFound) { t.Errorf("err = %v, want ErrPreLoginNotFound (expired → uniform sentinel)", err) } @@ -393,13 +393,13 @@ func TestPreLoginAdapter_LookupAndConsume_RepoOtherError(t *testing.T) { repo := newStubPreLoginRepo() keys := newStubSigningKeyLookup(activeKeyForTest(t, "sk-1")) a := NewPreLoginAdapter(repo, keys, "t-default", "") - cookie, _, err := a.CreatePreLogin(context.Background(), "op-x", "s", "n", "v") + cookie, _, err := a.CreatePreLogin(context.Background(), "op-x", "s", "n", "v", "", "") if err != nil { t.Fatalf("CreatePreLogin: %v", err) } // Inject a non-NotFound, non-Expired error to exercise the wrap branch. repo.wrappedErr = errors.New("postgres dropped connection") - _, _, _, _, err = a.LookupAndConsume(context.Background(), cookie) + _, _, _, _, _, _, err = a.LookupAndConsume(context.Background(), cookie) if errors.Is(err, ErrPreLoginNotFound) { t.Error("err must NOT be ErrPreLoginNotFound for non-sentinel repo failure") } @@ -412,11 +412,11 @@ func TestPreLoginAdapter_LookupAndConsume_HappyPath(t *testing.T) { repo := newStubPreLoginRepo() keys := newStubSigningKeyLookup(activeKeyForTest(t, "sk-1")) a := NewPreLoginAdapter(repo, keys, "t-default", "") - cookie, _, err := a.CreatePreLogin(context.Background(), "op-okta", "the-state-42", "the-nonce-42", "the-verifier-42") + cookie, _, err := a.CreatePreLogin(context.Background(), "op-okta", "the-state-42", "the-nonce-42", "the-verifier-42", "", "") if err != nil { t.Fatalf("CreatePreLogin: %v", err) } - pid, st, nn, vf, err := a.LookupAndConsume(context.Background(), cookie) + pid, st, nn, vf, _, _, err := a.LookupAndConsume(context.Background(), cookie) if err != nil { t.Fatalf("LookupAndConsume: %v", err) } @@ -425,7 +425,7 @@ func TestPreLoginAdapter_LookupAndConsume_HappyPath(t *testing.T) { } // Single-use: second consume returns ErrPreLoginNotFound. - _, _, _, _, err = a.LookupAndConsume(context.Background(), cookie) + _, _, _, _, _, _, err = a.LookupAndConsume(context.Background(), cookie) if !errors.Is(err, ErrPreLoginNotFound) { t.Errorf("second consume err = %v, want ErrPreLoginNotFound (single-use violated)", err) } diff --git a/internal/auth/oidc/provider_enabled_test.go b/internal/auth/oidc/provider_enabled_test.go index 613091b..4a8dfd4 100644 --- a/internal/auth/oidc/provider_enabled_test.go +++ b/internal/auth/oidc/provider_enabled_test.go @@ -23,7 +23,7 @@ func TestService_HandleAuthRequest_DisabledProvider_RejectsWithErrProviderDisabl // to simulate the operator toggling the provider offline. The next // HandleAuthRequest hits the disabled-check before the cached entry // is reused. - if _, _, _, err := svc.HandleAuthRequest(context.Background(), "op-disabled"); err != nil { + if _, _, _, err := svc.HandleAuthRequest(context.Background(), "op-disabled", "", ""); err != nil { t.Fatalf("warm HandleAuthRequest: %v", err) } if entry, ok := svc.cache["op-disabled"]; ok && entry.cfgRow != nil { @@ -32,7 +32,7 @@ func TestService_HandleAuthRequest_DisabledProvider_RejectsWithErrProviderDisabl t.Fatal("expected cache entry for op-disabled after warmup") } - _, _, _, err := svc.HandleAuthRequest(context.Background(), "op-disabled") + _, _, _, err := svc.HandleAuthRequest(context.Background(), "op-disabled", "", "") if !errors.Is(err, ErrProviderDisabled) { t.Errorf("HandleAuthRequest(disabled provider) err = %v; want ErrProviderDisabled", err) } diff --git a/internal/auth/oidc/service.go b/internal/auth/oidc/service.go index b3dc657..03b83de 100644 --- a/internal/auth/oidc/service.go +++ b/internal/auth/oidc/service.go @@ -85,6 +85,17 @@ type Service struct { // resolution + user upsert; on grantAdmin=true the user's resolved // role IDs are extended with r-admin. See bootstrap_hook.go. adminBootstrapHook AdminBootstrapHook + + // Audit 2026-05-10 MED-16 — Per-leg toggles for the pre-login UA/IP + // binding check. Both default to true; operators on enterprise + // proxies (UA rewrite) or dual-stack v4/v6 (IP flip) flip the + // affected leg false via CERTCTL_OIDC_PRELOGIN_REQUIRE_UA / + // CERTCTL_OIDC_PRELOGIN_REQUIRE_IP. Even when both are false, + // the binding values are still persisted so audit forensics can + // detect mismatches retroactively — only the in-band reject is + // suppressed. + preLoginRequireUA bool + preLoginRequireIP bool } // providerEntry caches the go-oidc Provider + the OAuth2 config + the @@ -126,16 +137,28 @@ type PreLoginStore interface { // CreatePreLogin persists a row with the given identifiers. // providerID is the configured op-... id; state, nonce, verifier // are server-generated random strings the callback will validate. + // clientIP + userAgent (Audit 2026-05-10 MED-16) are the + // /auth/oidc/login request's source IP (post LOW-5 XFF gating) + + // User-Agent header, persisted into the row so HandleCallback can + // reject mismatches at consume time. Empty strings are tolerated + // (rolling-deploy compat + headless / proxy contexts) — the + // consume-side check only enforces when both sides carry non-empty + // values for the leg in question. // Returns the opaque cookie value the handler sets, plus the // session ID (used as the audit trail anchor). - CreatePreLogin(ctx context.Context, providerID, state, nonce, verifier string) (cookieValue, sessionID string, err error) + CreatePreLogin(ctx context.Context, providerID, state, nonce, verifier, clientIP, userAgent string) (cookieValue, sessionID string, err error) // LookupAndConsume reads the pre-login row by cookie value AND // deletes it atomically. Single-use: a second call with the same // cookie value returns ErrPreLoginNotFound. Returns the stored // state/nonce/verifier/providerID for the caller to validate // against the callback parameters. - LookupAndConsume(ctx context.Context, cookieValue string) (providerID, state, nonce, verifier string, err error) + // + // Audit 2026-05-10 MED-16 — also returns the row's persisted + // clientIP + userAgent so HandleCallback can defeat replay of a + // stolen pre-login cookie by a different browser. Empty values are + // returned for rows persisted before migration 000044. + LookupAndConsume(ctx context.Context, cookieValue string) (providerID, state, nonce, verifier, clientIP, userAgent string, err error) } // SessionMinter wraps the post-login session creation. Phase 4's @@ -193,6 +216,20 @@ var ( // RFC 9207 §2.3. HTTP 400. ErrIssParamMismatch = errors.New("oidc: callback iss parameter does not match provider issuer URL") + // ErrPreLoginUAMismatch: pre-login row's User-Agent doesn't match + // the request hitting /auth/oidc/callback. Audit 2026-05-10 MED-16 + // closure — RFC 9700 §4.7.1 binding-state recommendation. HTTP 400. + // Operators on enterprise proxies that rewrite UA may set + // CERTCTL_OIDC_PRELOGIN_REQUIRE_UA=false to disable. + ErrPreLoginUAMismatch = errors.New("oidc: pre-login row User-Agent does not match callback request") + + // ErrPreLoginIPMismatch: pre-login row's client IP doesn't match + // the request hitting /auth/oidc/callback. Audit 2026-05-10 + // MED-16. HTTP 400. Operators on dual-stack v4/v6 environments + // where source IP routinely flips may set + // CERTCTL_OIDC_PRELOGIN_REQUIRE_IP=false to disable. + ErrPreLoginIPMismatch = errors.New("oidc: pre-login row client IP does not match callback request") + // ErrAudienceMismatch: ID token `aud` doesn't include the // configured client_id. HTTP 400. ErrAudienceMismatch = errors.New("oidc: audience mismatch") @@ -322,6 +359,11 @@ func NewService( encryptionKey: encryptionKey, cache: make(map[string]*providerEntry), clockNow: time.Now, + // MED-16 defaults: both legs ON. cmd/server/main.go reads + // CERTCTL_OIDC_PRELOGIN_REQUIRE_UA / _IP and calls + // SetPreLoginBindingRequirements to override. + preLoginRequireUA: true, + preLoginRequireIP: true, } } @@ -331,6 +373,16 @@ func (s *Service) SetClockForTest(now func() time.Time) { s.clockNow = now } +// SetPreLoginBindingRequirements wires the MED-16 UA/IP enforcement +// toggles. Both default to true; set false to log-only behaviour for +// a given leg (the binding is still persisted + audited; only the +// in-band reject is suppressed). Called by cmd/server/main.go from +// the config layer. +func (s *Service) SetPreLoginBindingRequirements(requireUA, requireIP bool) { + s.preLoginRequireUA = requireUA + s.preLoginRequireIP = requireIP +} + // ============================================================================= // HandleAuthRequest: kicks off the OIDC handshake. // @@ -346,7 +398,14 @@ func (s *Service) SetClockForTest(now func() time.Time) { // HandleAuthRequest builds the IdP redirect URL + persists the // pre-login session row holding state + nonce + PKCE verifier. -func (s *Service) HandleAuthRequest(ctx context.Context, providerID string) (authURL, cookieValue, preLoginID string, err error) { +// +// Audit 2026-05-10 MED-16 — clientIP + userAgent are persisted into +// the pre-login row so HandleCallback can reject a stolen cookie +// replayed by a different browser. Empty values are tolerated for +// headless / proxy callers; the consume-side check only enforces +// when both row and request carry non-empty values on the leg in +// question. +func (s *Service) HandleAuthRequest(ctx context.Context, providerID, clientIP, userAgent string) (authURL, cookieValue, preLoginID string, err error) { entry, err := s.getOrLoad(ctx, providerID) if err != nil { return "", "", "", err @@ -371,7 +430,7 @@ func (s *Service) HandleAuthRequest(ctx context.Context, providerID string) (aut // (well within the RFC 7636 43-128 character bound). verifier := oauth2.GenerateVerifier() - cookieValue, preLoginID, err = s.preLogin.CreatePreLogin(ctx, providerID, state, nonce, verifier) + cookieValue, preLoginID, err = s.preLogin.CreatePreLogin(ctx, providerID, state, nonce, verifier, clientIP, userAgent) if err != nil { return "", "", "", fmt.Errorf("oidc: pre-login store: %w", err) } @@ -428,11 +487,28 @@ func (s *Service) HandleCallback( preLoginCookie, code, callbackState, callbackIss, ip, userAgent string, ) (*CallbackResult, error) { // Step 1: consume the pre-login row (single-use). - providerID, storedState, storedNonce, verifier, err := s.preLogin.LookupAndConsume(ctx, preLoginCookie) + providerID, storedState, storedNonce, verifier, storedIP, storedUA, err := s.preLogin.LookupAndConsume(ctx, preLoginCookie) if err != nil { return nil, ErrPreLoginNotFound } + // Step 1.5: Audit 2026-05-10 MED-16 — UA / IP binding compare. + // Enforced only when (a) the leg's toggle is on, (b) the row + // carries a non-empty stored value (legacy rows pre-migration + // 000044 have NULL → empty string), and (c) the incoming request + // carries a non-empty value too. Constant-time compares for both + // legs to avoid leaking UA/IP length differences via timing. + if s.preLoginRequireUA && storedUA != "" && userAgent != "" { + if subtle.ConstantTimeCompare([]byte(userAgent), []byte(storedUA)) != 1 { + return nil, ErrPreLoginUAMismatch + } + } + if s.preLoginRequireIP && storedIP != "" && ip != "" { + if subtle.ConstantTimeCompare([]byte(ip), []byte(storedIP)) != 1 { + return nil, ErrPreLoginIPMismatch + } + } + // Step 2: state constant-time compare. if subtle.ConstantTimeCompare([]byte(callbackState), []byte(storedState)) != 1 { return nil, ErrStateMismatch diff --git a/internal/auth/oidc/service_test.go b/internal/auth/oidc/service_test.go index 971270e..4850138 100644 --- a/internal/auth/oidc/service_test.go +++ b/internal/auth/oidc/service_test.go @@ -420,26 +420,30 @@ type stubPreLogin struct { type preLoginRow struct { providerID, state, nonce, verifier string + // Audit 2026-05-10 MED-16 — UA/IP binding captured at + // CreatePreLogin so LookupAndConsume can surface them for the + // service-layer compare. + clientIP, userAgent string } func newStubPreLogin() *stubPreLogin { return &stubPreLogin{rows: make(map[string]preLoginRow)} } -func (s *stubPreLogin) CreatePreLogin(_ context.Context, providerID, state, nonce, verifier string) (string, string, error) { +func (s *stubPreLogin) CreatePreLogin(_ context.Context, providerID, state, nonce, verifier, clientIP, userAgent string) (string, string, error) { if s.createErr != nil { return "", "", s.createErr } cookieVal := fmt.Sprintf("pl-%d", len(s.rows)+1) - s.rows[cookieVal] = preLoginRow{providerID, state, nonce, verifier} + s.rows[cookieVal] = preLoginRow{providerID, state, nonce, verifier, clientIP, userAgent} return cookieVal, "ses-" + cookieVal, nil } -func (s *stubPreLogin) LookupAndConsume(_ context.Context, cookie string) (string, string, string, string, error) { +func (s *stubPreLogin) LookupAndConsume(_ context.Context, cookie string) (string, string, string, string, string, string, error) { r, ok := s.rows[cookie] if !ok { - return "", "", "", "", ErrPreLoginNotFound + return "", "", "", "", "", "", ErrPreLoginNotFound } delete(s.rows, cookie) - return r.providerID, r.state, r.nonce, r.verifier, nil + return r.providerID, r.state, r.nonce, r.verifier, r.clientIP, r.userAgent, nil } // ============================================================================= @@ -465,14 +469,14 @@ func TestService_PKCEPlainRejectedSentinel(t *testing.T) { // a second call with the same cookie returns ErrPreLoginNotFound. func TestService_StateReplayDeniedByConsumeOnce(t *testing.T) { pl := newStubPreLogin() - cookie, _, err := pl.CreatePreLogin(context.Background(), "op-x", "the-state", "the-nonce", "verifier-xxx") + cookie, _, err := pl.CreatePreLogin(context.Background(), "op-x", "the-state", "the-nonce", "verifier-xxx", "", "") if err != nil { t.Fatalf("CreatePreLogin: %v", err) } - if _, _, _, _, err := pl.LookupAndConsume(context.Background(), cookie); err != nil { + if _, _, _, _, _, _, err := pl.LookupAndConsume(context.Background(), cookie); err != nil { t.Fatalf("first LookupAndConsume: %v", err) } - _, _, _, _, err = pl.LookupAndConsume(context.Background(), cookie) + _, _, _, _, _, _, err = pl.LookupAndConsume(context.Background(), cookie) if !errors.Is(err, ErrPreLoginNotFound) { t.Errorf("second LookupAndConsume err = %v; want ErrPreLoginNotFound (single-use violated)", err) } @@ -490,7 +494,7 @@ func TestService_HandleCallback_RejectsForgedPreLoginCookie(t *testing.T) { // Test 4: state mismatch (cookie matches but the callback state doesn't). func TestService_HandleCallback_RejectsStateMismatch(t *testing.T) { svc, pl := newServiceForUnitTestWithPL(t) - cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-test", "real-state", "real-nonce", "verifier-xxx") + cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-test", "real-state", "real-nonce", "verifier-xxx", "", "") _, err := svc.HandleCallback(context.Background(), cookie, "code", "wrong-state", "", "ip", "ua") if !errors.Is(err, ErrStateMismatch) { t.Errorf("err = %v; want ErrStateMismatch", err) @@ -642,7 +646,7 @@ func TestService_HandleCallback_HappyPath(t *testing.T) { idp := newMockIdP(t) svc, pl := newServiceWithProviderAndPL(t, idp.URL(), "op-happy") - cookie, _, err := pl.CreatePreLogin(context.Background(), "op-happy", "happy-state", "test-nonce-fixed", "verifier-xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx") + cookie, _, err := pl.CreatePreLogin(context.Background(), "op-happy", "happy-state", "test-nonce-fixed", "verifier-xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", "", "") if err != nil { t.Fatalf("CreatePreLogin: %v", err) } @@ -668,7 +672,7 @@ func TestService_HandleCallback_RejectsWrongAudience(t *testing.T) { idp.overrideAudience = []string{"some-other-client"} svc, pl := newServiceWithProviderAndPL(t, idp.URL(), "op-aud") - cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-aud", "s", "test-nonce-fixed", "v-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa") + cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-aud", "s", "test-nonce-fixed", "v-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "", "") _, err := svc.HandleCallback(context.Background(), cookie, "code", "s", "", "ip", "ua") // gooidc.Verify catches this first; its wrap reaches us as a wrapped error. // Either ErrAudienceMismatch (our re-check) OR a wrapped verify error is acceptable. @@ -684,7 +688,7 @@ func TestService_HandleCallback_RejectsNonceMismatch(t *testing.T) { idp.overrideNonce = "wrong-nonce-from-idp" svc, pl := newServiceWithProviderAndPL(t, idp.URL(), "op-nonce") - cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-nonce", "s", "expected-nonce", "v-bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb") + cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-nonce", "s", "expected-nonce", "v-bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", "", "") _, err := svc.HandleCallback(context.Background(), cookie, "code", "s", "", "ip", "ua") if !errors.Is(err, ErrNonceMismatch) { t.Errorf("err = %v; want ErrNonceMismatch", err) @@ -697,7 +701,7 @@ func TestService_HandleCallback_RejectsExpiredToken(t *testing.T) { idp.overrideExp = time.Now().Add(-2 * time.Hour) // 2 hours past svc, pl := newServiceWithProviderAndPL(t, idp.URL(), "op-exp") - cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-exp", "s", "test-nonce-fixed", "v-cccccccccccccccccccccccccccccccccccccccccc") + cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-exp", "s", "test-nonce-fixed", "v-cccccccccccccccccccccccccccccccccccccccccc", "", "") _, err := svc.HandleCallback(context.Background(), cookie, "code", "s", "", "ip", "ua") // Either ErrTokenExpired (our re-check) or a wrapped verify error is fine. if err == nil { @@ -714,7 +718,7 @@ func TestService_HandleCallback_RejectsIATTooOld(t *testing.T) { idp.overrideExp = time.Now().Add(2 * time.Hour) // exp is fine svc, pl := newServiceWithProviderAndPL(t, idp.URL(), "op-iat") - cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-iat", "s", "test-nonce-fixed", "v-dddddddddddddddddddddddddddddddddddddddddd") + cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-iat", "s", "test-nonce-fixed", "v-dddddddddddddddddddddddddddddddddddddddddd", "", "") _, err := svc.HandleCallback(context.Background(), cookie, "code", "s", "", "ip", "ua") if !errors.Is(err, ErrIATTooOld) { t.Errorf("err = %v; want ErrIATTooOld", err) @@ -727,7 +731,7 @@ func TestService_HandleCallback_RejectsGroupsMissing(t *testing.T) { idp.overrideGroups = []string{} // empty groups claim svc, pl := newServiceWithProviderAndPL(t, idp.URL(), "op-grp") - cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-grp", "s", "test-nonce-fixed", "v-eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee") + cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-grp", "s", "test-nonce-fixed", "v-eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee", "", "") _, err := svc.HandleCallback(context.Background(), cookie, "code", "s", "", "ip", "ua") if !errors.Is(err, ErrGroupsMissing) { t.Errorf("err = %v; want ErrGroupsMissing", err) @@ -740,7 +744,7 @@ func TestService_HandleCallback_RejectsGroupsUnmapped(t *testing.T) { idp := newMockIdP(t) svc, pl := newServiceWithProviderAndPLNoMappings(t, idp.URL(), "op-unmap") - cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-unmap", "s", "test-nonce-fixed", "v-ffffffffffffffffffffffffffffffffffffffffff") + cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-unmap", "s", "test-nonce-fixed", "v-ffffffffffffffffffffffffffffffffffffffffff", "", "") _, err := svc.HandleCallback(context.Background(), cookie, "code", "s", "", "ip", "ua") if !errors.Is(err, ErrGroupsUnmapped) { t.Errorf("err = %v; want ErrGroupsUnmapped", err) @@ -850,7 +854,7 @@ func TestService_HandleAuthRequest_BuildsValidIdPRedirect(t *testing.T) { idp := newMockIdP(t) svc, pl := newServiceWithProviderAndPL(t, idp.URL(), "op-har") - authURL, cookieValue, preLoginID, err := svc.HandleAuthRequest(context.Background(), "op-har") + authURL, cookieValue, preLoginID, err := svc.HandleAuthRequest(context.Background(), "op-har", "", "") if err != nil { t.Fatalf("HandleAuthRequest: %v", err) } @@ -880,7 +884,7 @@ func TestService_HandleAuthRequest_BuildsValidIdPRedirect(t *testing.T) { // repo-not-found path through HandleAuthRequest. func TestService_HandleAuthRequest_UnknownProviderRejected(t *testing.T) { svc := newServiceForUnitTest(t) - _, _, _, err := svc.HandleAuthRequest(context.Background(), "op-nonexistent") + _, _, _, err := svc.HandleAuthRequest(context.Background(), "op-nonexistent", "", "") if !errors.Is(err, repository.ErrOIDCProviderNotFound) { t.Errorf("err = %v; want ErrOIDCProviderNotFound", err) } @@ -900,7 +904,7 @@ func TestService_UpsertUser_UpdateExistingPath(t *testing.T) { svc := NewService(&stubProviderLookup{provider: prov}, mappings, users, sessions, pl, "") // First login creates the user. - cookie1, _, _ := pl.CreatePreLogin(context.Background(), "op-upd", "s1", "test-nonce-fixed", "v-1aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa") + cookie1, _, _ := pl.CreatePreLogin(context.Background(), "op-upd", "s1", "test-nonce-fixed", "v-1aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "", "") res1, err := svc.HandleCallback(context.Background(), cookie1, "code", "s1", "", "ip", "ua") if err != nil { t.Fatalf("first HandleCallback: %v", err) @@ -913,7 +917,7 @@ func TestService_UpsertUser_UpdateExistingPath(t *testing.T) { time.Sleep(10 * time.Millisecond) // ensure timestamps advance // Second login by same subject: update path, no new user row. - cookie2, _, _ := pl.CreatePreLogin(context.Background(), "op-upd", "s2", "test-nonce-fixed", "v-2aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa") + cookie2, _, _ := pl.CreatePreLogin(context.Background(), "op-upd", "s2", "test-nonce-fixed", "v-2aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "", "") idp.overrideEmail = "user-renamed@example.com" res2, err := svc.HandleCallback(context.Background(), cookie2, "code2", "s2", "", "ip", "ua") if err != nil { @@ -1182,7 +1186,7 @@ func TestService_BootstrapHook_GrantsAdminOnMatch(t *testing.T) { return true, nil // grant admin }) - cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-bootstrap", "s", "test-nonce-fixed", "v-bootstrapxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx") + cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-bootstrap", "s", "test-nonce-fixed", "v-bootstrapxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", "", "") res, err := svc.HandleCallback(context.Background(), cookie, "code", "s", "", "10.0.0.1", "Mozilla/5.0") if err != nil { t.Fatalf("HandleCallback: %v", err) @@ -1205,7 +1209,7 @@ func TestService_BootstrapHook_NoMatchPreservesEmptyMappingFailClosed(t *testing return false, nil // not a bootstrap match }) - cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-no-match", "s", "test-nonce-fixed", "v-nomatchxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx") + cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-no-match", "s", "test-nonce-fixed", "v-nomatchxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", "", "") _, err := svc.HandleCallback(context.Background(), cookie, "code", "s", "", "ip", "ua") if !errors.Is(err, ErrGroupsUnmapped) { t.Errorf("err = %v; want ErrGroupsUnmapped (no bootstrap match + empty mappings)", err) @@ -1226,7 +1230,7 @@ func TestService_BootstrapHook_AdminAlreadyExistsFallsThroughToNormalMapping(t * return false, nil }) - cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-existing-admin", "s", "test-nonce-fixed", "v-existingxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx") + cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-existing-admin", "s", "test-nonce-fixed", "v-existingxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", "", "") res, err := svc.HandleCallback(context.Background(), cookie, "code", "s", "", "ip", "ua") if err != nil { t.Fatalf("HandleCallback: %v", err) @@ -1248,7 +1252,7 @@ func TestService_BootstrapHook_ErrorWraps(t *testing.T) { svc.SetAdminBootstrapHook(func(_ context.Context, _ string, _ []string, _ string) (bool, error) { return false, fmt.Errorf("simulated AdminExists probe failure") }) - cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-hook-err", "s", "test-nonce-fixed", "v-errxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx") + cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-hook-err", "s", "test-nonce-fixed", "v-errxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", "", "") _, err := svc.HandleCallback(context.Background(), cookie, "code", "s", "", "ip", "ua") if err == nil || !strings.Contains(err.Error(), "admin bootstrap") { t.Errorf("err = %v; want admin bootstrap wrap", err) @@ -1269,7 +1273,7 @@ func TestService_BootstrapHook_IdempotentWhenAdminAlreadyMapped(t *testing.T) { return true, nil }) - cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-idem", "s", "test-nonce-fixed", "v-idempxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx") + cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-idem", "s", "test-nonce-fixed", "v-idempxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", "", "") res, err := svc.HandleCallback(context.Background(), cookie, "code", "s", "", "ip", "ua") if err != nil { t.Fatalf("HandleCallback: %v", err) @@ -1324,7 +1328,7 @@ func TestService_HandleCallback_AZPRequired_OnMultiAud(t *testing.T) { idp.overrideAudience = []string{"certctl", "another-relying-party"} svc, pl := newServiceWithProviderAndPL(t, idp.URL(), "op-azp-req") - cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-azp-req", "s", "test-nonce-fixed", "v-azpreqxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx") + cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-azp-req", "s", "test-nonce-fixed", "v-azpreqxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", "", "") _, err := svc.HandleCallback(context.Background(), cookie, "code", "s", "", "ip", "ua") if !errors.Is(err, ErrAZPRequired) { t.Errorf("err = %v; want ErrAZPRequired", err) @@ -1338,7 +1342,7 @@ func TestService_HandleCallback_AZPMismatch(t *testing.T) { idp.overrideAZP = "some-other-client" // != "certctl" svc, pl := newServiceWithProviderAndPL(t, idp.URL(), "op-azp-mis") - cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-azp-mis", "s", "test-nonce-fixed", "v-azpmisxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx") + cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-azp-mis", "s", "test-nonce-fixed", "v-azpmisxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", "", "") _, err := svc.HandleCallback(context.Background(), cookie, "code", "s", "", "ip", "ua") if !errors.Is(err, ErrAZPMismatch) { t.Errorf("err = %v; want ErrAZPMismatch", err) @@ -1356,7 +1360,7 @@ func TestService_HandleCallback_ATHashMismatch(t *testing.T) { idp.overrideATHash = "not-the-real-at-hash" svc, pl := newServiceWithProviderAndPL(t, idp.URL(), "op-ath-mis") - cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-ath-mis", "s", "test-nonce-fixed", "v-athmisxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx") + cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-ath-mis", "s", "test-nonce-fixed", "v-athmisxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", "", "") _, err := svc.HandleCallback(context.Background(), cookie, "code", "s", "", "ip", "ua") if !errors.Is(err, ErrATHashMismatch) { t.Errorf("err = %v; want ErrATHashMismatch", err) @@ -1373,7 +1377,7 @@ func TestService_HandleCallback_ATHashRequired_WhenAccessTokenPresent(t *testing idp.overrideATHash = "" // suppress at_hash even though access_token is returned svc, pl := newServiceWithProviderAndPL(t, idp.URL(), "op-ath-req") - cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-ath-req", "s", "test-nonce-fixed", "v-athreqxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx") + cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-ath-req", "s", "test-nonce-fixed", "v-athreqxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", "", "") _, err := svc.HandleCallback(context.Background(), cookie, "code", "s", "", "ip", "ua") if !errors.Is(err, ErrATHashRequired) { t.Errorf("err = %v; want ErrATHashRequired", err) @@ -1389,7 +1393,7 @@ func TestService_HandleCallback_IATInFuture(t *testing.T) { idp.overrideExp = time.Now().Add(2 * time.Hour) svc, pl := newServiceWithProviderAndPL(t, idp.URL(), "op-iat-fut") - cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-iat-fut", "s", "test-nonce-fixed", "v-iatfutxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx") + cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-iat-fut", "s", "test-nonce-fixed", "v-iatfutxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", "", "") _, err := svc.HandleCallback(context.Background(), cookie, "code", "s", "", "ip", "ua") if !errors.Is(err, ErrIATInFuture) { t.Errorf("err = %v; want ErrIATInFuture", err) @@ -1407,7 +1411,7 @@ func TestService_HandleCallback_MappingsMapError(t *testing.T) { sessions := &stubSessions{} svc := NewService(&stubProviderLookup{provider: prov}, mappings, users, sessions, pl, "") - cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-map-err", "s", "test-nonce-fixed", "v-mapxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx") + cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-map-err", "s", "test-nonce-fixed", "v-mapxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", "", "") _, err := svc.HandleCallback(context.Background(), cookie, "code", "s", "", "ip", "ua") if err == nil || !strings.Contains(err.Error(), "group-role mapping") { t.Errorf("err = %v; want group-role mapping wrap", err) @@ -1425,7 +1429,7 @@ func TestService_HandleCallback_SessionMintError(t *testing.T) { sessions := &stubSessions{mintErr: fmt.Errorf("simulated session minter failure")} svc := NewService(&stubProviderLookup{provider: prov}, mappings, users, sessions, pl, "") - cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-mint-err", "s", "test-nonce-fixed", "v-mintxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx") + cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-mint-err", "s", "test-nonce-fixed", "v-mintxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", "", "") _, err := svc.HandleCallback(context.Background(), cookie, "code", "s", "", "ip", "ua") if err == nil || !strings.Contains(err.Error(), "session mint") { t.Errorf("err = %v; want session mint wrap", err) @@ -1444,7 +1448,7 @@ func TestService_HandleCallback_UserCreateError(t *testing.T) { sessions := &stubSessions{} svc := NewService(&stubProviderLookup{provider: prov}, mappings, users, sessions, pl, "") - cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-uc-err", "s", "test-nonce-fixed", "v-ucxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx") + cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-uc-err", "s", "test-nonce-fixed", "v-ucxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", "", "") _, err := svc.HandleCallback(context.Background(), cookie, "code", "s", "", "ip", "ua") if err == nil || !strings.Contains(err.Error(), "upsert user") { t.Errorf("err = %v; want upsert user wrap", err) @@ -1464,7 +1468,7 @@ func TestService_HandleCallback_GetByOIDCSubjectNonNotFoundError(t *testing.T) { sessions := &stubSessions{} svc := NewService(&stubProviderLookup{provider: prov}, mappings, users, sessions, pl, "") - cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-get-err", "s", "test-nonce-fixed", "v-getxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx") + cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-get-err", "s", "test-nonce-fixed", "v-getxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", "", "") _, err := svc.HandleCallback(context.Background(), cookie, "code", "s", "", "ip", "ua") if err == nil || !strings.Contains(err.Error(), "simulated query failure") { t.Errorf("err = %v; want simulated query failure unwrap", err) @@ -1485,7 +1489,7 @@ func TestService_UpsertUser_DisplayNameFallsBackToEmail(t *testing.T) { sessions := &stubSessions{} svc := NewService(&stubProviderLookup{provider: prov}, mappings, users, sessions, pl, "") - cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-name-fb", "s", "test-nonce-fixed", "v-namxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx") + cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-name-fb", "s", "test-nonce-fixed", "v-namxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", "", "") res, err := svc.HandleCallback(context.Background(), cookie, "code", "s", "", "ip", "ua") if err != nil { t.Fatalf("HandleCallback: %v", err) @@ -1511,7 +1515,7 @@ func TestService_FetchUserinfoGroups_HappyPath_OnEmptyIDTokenGroups(t *testing.T sessions := &stubSessions{} svc := NewService(&stubProviderLookup{provider: prov}, mappings, users, sessions, pl, "") - cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-ui-ok", "s", "test-nonce-fixed", "v-uioxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx") + cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-ui-ok", "s", "test-nonce-fixed", "v-uioxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", "", "") res, err := svc.HandleCallback(context.Background(), cookie, "code", "s", "", "ip", "ua") if err != nil { t.Fatalf("HandleCallback: %v", err) @@ -1536,7 +1540,7 @@ func TestService_FetchUserinfoGroups_ReturnsErrGroupsMissing_WhenUserinfoAlsoEmp sessions := &stubSessions{} svc := NewService(&stubProviderLookup{provider: prov}, mappings, users, sessions, pl, "") - cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-ui-empty", "s", "test-nonce-fixed", "v-uixxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx") + cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-ui-empty", "s", "test-nonce-fixed", "v-uixxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", "", "") _, err := svc.HandleCallback(context.Background(), cookie, "code", "s", "", "ip", "ua") if !errors.Is(err, ErrGroupsMissing) { t.Errorf("err = %v; want ErrGroupsMissing", err) @@ -1558,7 +1562,7 @@ func TestService_FetchUserinfoGroups_ReturnsErrGroupsMissing_WhenEndpointMissing sessions := &stubSessions{} svc := NewService(&stubProviderLookup{provider: prov}, mappings, users, sessions, pl, "") - cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-ui-noendpoint", "s", "test-nonce-fixed", "v-uixxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx") + cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-ui-noendpoint", "s", "test-nonce-fixed", "v-uixxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", "", "") _, err := svc.HandleCallback(context.Background(), cookie, "code", "s", "", "ip", "ua") if !errors.Is(err, ErrGroupsMissing) { t.Errorf("err = %v; want ErrGroupsMissing", err) @@ -1582,7 +1586,7 @@ func TestService_HandleAuthRequest_PreLoginStoreError(t *testing.T) { "", ) - _, _, _, err := svc.HandleAuthRequest(context.Background(), "op-pl-err") + _, _, _, err := svc.HandleAuthRequest(context.Background(), "op-pl-err", "", "") if err == nil || !strings.Contains(err.Error(), "pre-login store") { t.Errorf("err = %v; want pre-login store wrap", err) } @@ -1663,7 +1667,7 @@ func TestService_HandleAuthRequest_RandomFailureSurfaces(t *testing.T) { } defer func() { readRand = original }() - _, _, _, err := svc.HandleAuthRequest(context.Background(), "op-rand-fail") + _, _, _, err := svc.HandleAuthRequest(context.Background(), "op-rand-fail", "", "") if err == nil || !strings.Contains(err.Error(), "state generate") { t.Errorf("err = %v; want state generate wrap", err) } @@ -1687,7 +1691,7 @@ func TestService_HandleAuthRequest_NonceRandomFailureSurfaces(t *testing.T) { } defer func() { readRand = original }() - _, _, _, err := svc.HandleAuthRequest(context.Background(), "op-nonce-rand-fail") + _, _, _, err := svc.HandleAuthRequest(context.Background(), "op-nonce-rand-fail", "", "") if err == nil || !strings.Contains(err.Error(), "nonce generate") { t.Errorf("err = %v; want nonce generate wrap", err) } @@ -1702,7 +1706,7 @@ func TestService_HandleCallback_RejectsTokenResponseMissingIDToken(t *testing.T) idp.suppressIDToken = true svc, pl := newServiceWithProviderAndPL(t, idp.URL(), "op-no-idtok") - cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-no-idtok", "s", "test-nonce-fixed", "v-noidxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx") + cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-no-idtok", "s", "test-nonce-fixed", "v-noidxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", "", "") _, err := svc.HandleCallback(context.Background(), cookie, "code", "s", "", "ip", "ua") if err == nil || !strings.Contains(err.Error(), "missing id_token") { t.Errorf("err = %v; want missing id_token error", err) @@ -1725,7 +1729,7 @@ func TestService_FetchUserinfoGroups_ReturnsErrGroupsMissing_WhenUserinfoFails(t sessions := &stubSessions{} svc := NewService(&stubProviderLookup{provider: prov}, mappings, users, sessions, pl, "") - cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-ui-500", "s", "test-nonce-fixed", "v-uifxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx") + cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-ui-500", "s", "test-nonce-fixed", "v-uifxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", "", "") _, err := svc.HandleCallback(context.Background(), cookie, "code", "s", "", "ip", "ua") if !errors.Is(err, ErrGroupsMissing) { t.Errorf("err = %v; want ErrGroupsMissing", err) @@ -1791,7 +1795,7 @@ func TestService_HandleCallback_MED17_NoSupport_AnyIssAccepted(t *testing.T) { // advertiseIssParameterSupported deliberately left false. svc, pl := newServiceWithProviderAndPL(t, idp.URL(), "op-iss-back-compat") - cookie, _, err := pl.CreatePreLogin(context.Background(), "op-iss-back-compat", "iss-bc-state", "test-nonce-fixed", "v-issbcxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx") + cookie, _, err := pl.CreatePreLogin(context.Background(), "op-iss-back-compat", "iss-bc-state", "test-nonce-fixed", "v-issbcxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", "", "") if err != nil { t.Fatalf("CreatePreLogin: %v", err) } @@ -1815,7 +1819,7 @@ func TestService_HandleCallback_MED17_SupportButMissing(t *testing.T) { idp.advertiseIssParameterSupported = true svc, pl := newServiceWithProviderAndPL(t, idp.URL(), "op-iss-missing") - cookie, _, err := pl.CreatePreLogin(context.Background(), "op-iss-missing", "iss-miss-state", "test-nonce-fixed", "v-issmsxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx") + cookie, _, err := pl.CreatePreLogin(context.Background(), "op-iss-missing", "iss-miss-state", "test-nonce-fixed", "v-issmsxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", "", "") if err != nil { t.Fatalf("CreatePreLogin: %v", err) } @@ -1835,7 +1839,7 @@ func TestService_HandleCallback_MED17_SupportButMismatch(t *testing.T) { idp.advertiseIssParameterSupported = true svc, pl := newServiceWithProviderAndPL(t, idp.URL(), "op-iss-mismatch") - cookie, _, err := pl.CreatePreLogin(context.Background(), "op-iss-mismatch", "iss-mm-state", "test-nonce-fixed", "v-issmmxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx") + cookie, _, err := pl.CreatePreLogin(context.Background(), "op-iss-mismatch", "iss-mm-state", "test-nonce-fixed", "v-issmmxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", "", "") if err != nil { t.Fatalf("CreatePreLogin: %v", err) } @@ -1855,7 +1859,7 @@ func TestService_HandleCallback_MED17_SupportAndCorrect(t *testing.T) { idp.advertiseIssParameterSupported = true svc, pl := newServiceWithProviderAndPL(t, idp.URL(), "op-iss-ok") - cookie, _, err := pl.CreatePreLogin(context.Background(), "op-iss-ok", "iss-ok-state", "test-nonce-fixed", "v-issokxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx") + cookie, _, err := pl.CreatePreLogin(context.Background(), "op-iss-ok", "iss-ok-state", "test-nonce-fixed", "v-issokxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", "", "") if err != nil { t.Fatalf("CreatePreLogin: %v", err) } @@ -1869,6 +1873,105 @@ func TestService_HandleCallback_MED17_SupportAndCorrect(t *testing.T) { } } +// ============================================================================= +// MED-16 regression tests — pre-login UA / IP binding (RFC 9700 §4.7.1). +// +// HandleCallback rejects a pre-login cookie whose stored client_ip or +// user_agent doesn't match the incoming /auth/oidc/callback request's +// values. Each leg has an independent enforcement toggle; the binding +// is also tolerant of empty values on either side (rolling-deploy + +// headless-proxy compat). +// ============================================================================= + +func TestService_HandleCallback_MED16_UAMismatchRejected(t *testing.T) { + idp := newMockIdP(t) + svc, pl := newServiceWithProviderAndPL(t, idp.URL(), "op-med16-ua") + + cookie, _, err := pl.CreatePreLogin(context.Background(), "op-med16-ua", "ua-state", "test-nonce-fixed", "verifier-med16uaxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", "10.0.0.1", "MozillaLogin/1.0") + if err != nil { + t.Fatalf("CreatePreLogin: %v", err) + } + _, err = svc.HandleCallback(context.Background(), cookie, "code", "ua-state", "", "10.0.0.1", "AttackerUA/2.0") + if !errors.Is(err, ErrPreLoginUAMismatch) { + t.Fatalf("err = %v; want ErrPreLoginUAMismatch", err) + } +} + +func TestService_HandleCallback_MED16_IPMismatchRejected(t *testing.T) { + idp := newMockIdP(t) + svc, pl := newServiceWithProviderAndPL(t, idp.URL(), "op-med16-ip") + + cookie, _, err := pl.CreatePreLogin(context.Background(), "op-med16-ip", "ip-state", "test-nonce-fixed", "verifier-med16ipxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", "10.0.0.1", "Mozilla/5.0") + if err != nil { + t.Fatalf("CreatePreLogin: %v", err) + } + _, err = svc.HandleCallback(context.Background(), cookie, "code", "ip-state", "", "203.0.113.7", "Mozilla/5.0") + if !errors.Is(err, ErrPreLoginIPMismatch) { + t.Fatalf("err = %v; want ErrPreLoginIPMismatch", err) + } +} + +func TestService_HandleCallback_MED16_BothMatch_Succeeds(t *testing.T) { + idp := newMockIdP(t) + svc, pl := newServiceWithProviderAndPL(t, idp.URL(), "op-med16-ok") + + cookie, _, err := pl.CreatePreLogin(context.Background(), "op-med16-ok", "ok-state", "test-nonce-fixed", "verifier-med16okxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", "10.0.0.1", "Mozilla/5.0") + if err != nil { + t.Fatalf("CreatePreLogin: %v", err) + } + res, err := svc.HandleCallback(context.Background(), cookie, "code", "ok-state", "", "10.0.0.1", "Mozilla/5.0") + if err != nil { + t.Fatalf("HandleCallback (matching UA+IP): %v", err) + } + if res == nil { + t.Fatal("CallbackResult nil on matching binding") + } +} + +// TestService_HandleCallback_MED16_LegacyRowEmptyValues pins the +// rolling-deploy compat — a pre-login row persisted before migration +// 000044 has empty clientIP/userAgent; the consume-side check must +// pass through (the legacy row's binding is unenforceable). +func TestService_HandleCallback_MED16_LegacyRowEmptyValues(t *testing.T) { + idp := newMockIdP(t) + svc, pl := newServiceWithProviderAndPL(t, idp.URL(), "op-med16-legacy") + + cookie, _, err := pl.CreatePreLogin(context.Background(), "op-med16-legacy", "leg-state", "test-nonce-fixed", "verifier-med16legxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", "", "") + if err != nil { + t.Fatalf("CreatePreLogin: %v", err) + } + res, err := svc.HandleCallback(context.Background(), cookie, "code", "leg-state", "", "10.0.0.1", "Mozilla/5.0") + if err != nil { + t.Fatalf("HandleCallback (legacy empty bind): %v", err) + } + if res == nil { + t.Fatal("CallbackResult nil for legacy-row compat path") + } +} + +// TestService_HandleCallback_MED16_RequireUAFalse_AllowsMismatch pins +// the operator-escape-hatch behaviour: setting requireUA=false means +// a UA mismatch passes through silently. The binding is still +// persisted (so audit forensics can detect it retroactively) but the +// in-band reject is suppressed. +func TestService_HandleCallback_MED16_RequireUAFalse_AllowsMismatch(t *testing.T) { + idp := newMockIdP(t) + svc, pl := newServiceWithProviderAndPL(t, idp.URL(), "op-med16-uaopt") + svc.SetPreLoginBindingRequirements(false, true) // UA off, IP on + + cookie, _, err := pl.CreatePreLogin(context.Background(), "op-med16-uaopt", "ua-opt-state", "test-nonce-fixed", "verifier-med16optxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", "10.0.0.1", "MozillaLogin/1.0") + if err != nil { + t.Fatalf("CreatePreLogin: %v", err) + } + res, err := svc.HandleCallback(context.Background(), cookie, "code", "ua-opt-state", "", "10.0.0.1", "AttackerUA/2.0") + if err != nil { + t.Fatalf("HandleCallback (requireUA=false, UA mismatch): %v", err) + } + if res == nil { + t.Fatal("CallbackResult nil with requireUA=false") + } +} + // TestService_UpsertUser_ValidateErrorOnEmptyEmail pins the // User.Validate failure path. The IdP returns an empty email (missing // claim); the upsertUser display-name fallback resolves to "" too; @@ -1884,7 +1987,7 @@ func TestService_UpsertUser_ValidateErrorOnEmptyEmail(t *testing.T) { sessions := &stubSessions{} svc := NewService(&stubProviderLookup{provider: prov}, mappings, users, sessions, pl, "") - cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-validate-err", "s", "test-nonce-fixed", "v-valxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx") + cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-validate-err", "s", "test-nonce-fixed", "v-valxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", "", "") _, err := svc.HandleCallback(context.Background(), cookie, "code", "s", "", "ip", "ua") if err == nil || !strings.Contains(err.Error(), "validate") { t.Errorf("err = %v; want validate wrap", err) diff --git a/internal/config/config.go b/internal/config/config.go index 5f5f4cd..ad574ce 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -1635,6 +1635,23 @@ type AuthConfig struct { // Setting: CERTCTL_OIDC_BCL_MAX_AGE_SECONDS environment variable. OIDCBCLMaxAgeSeconds int + // OIDCPreLoginRequireUA enables the RFC 9700 §4.7.1 user-agent + // binding check on /auth/oidc/callback. Audit 2026-05-10 MED-16. + // Default true. Operators on enterprise proxies that rewrite the + // UA header set this false; the binding value is still persisted + // + audited even when enforcement is off so retroactive forensics + // remain possible. + // Setting: CERTCTL_OIDC_PRELOGIN_REQUIRE_UA environment variable. + OIDCPreLoginRequireUA bool + + // OIDCPreLoginRequireIP enables the RFC 9700 §4.7.1 source-IP + // binding check on /auth/oidc/callback. Audit 2026-05-10 MED-16. + // Default true. Operators on dual-stack v4/v6 or mobile + // carrier-grade NAT where source IP routinely flips set this + // false; persistence + audit behave the same as UA above. + // Setting: CERTCTL_OIDC_PRELOGIN_REQUIRE_IP environment variable. + OIDCPreLoginRequireIP bool + // Breakglass holds the Auth Bundle 2 Phase 7.5 break-glass admin // tunables. Default-OFF; the entire surface is invisible (404 // instead of 403) when CERTCTL_BREAKGLASS_ENABLED is not true. @@ -1912,6 +1929,10 @@ func Load() (*Config, error) { }, // Audit 2026-05-10 HIGH-3 — BCL iat-skew window. OIDCBCLMaxAgeSeconds: getEnvInt("CERTCTL_OIDC_BCL_MAX_AGE_SECONDS", 60), + + // Audit 2026-05-10 MED-16 — pre-login UA/IP binding toggles. + OIDCPreLoginRequireUA: getEnvBool("CERTCTL_OIDC_PRELOGIN_REQUIRE_UA", true), + OIDCPreLoginRequireIP: getEnvBool("CERTCTL_OIDC_PRELOGIN_REQUIRE_IP", true), // Bundle 2 Phase 7.5: break-glass admin tunables. Default- // OFF; the entire surface is invisible (404 NOT 403) when // Enabled=false. Threat model + recommendation in the diff --git a/internal/repository/oidc.go b/internal/repository/oidc.go index f66856f..35db30d 100644 --- a/internal/repository/oidc.go +++ b/internal/repository/oidc.go @@ -121,6 +121,15 @@ type PreLoginSession struct { PKCEVerifier string CreatedAt time.Time AbsoluteExpiresAt time.Time + + // Audit 2026-05-10 MED-16 — UA / IP binding (RFC 9700 §4.7.1). + // Persisted at /auth/oidc/login; compared on consume to defeat + // pre-login cookie theft. Either column may be empty for in-flight + // rows from a pre-deploy code path during a rolling deploy; the + // consume-side check only enforces when BOTH the row AND the + // incoming request carry non-empty values. + ClientIP string + UserAgent string } // Sentinel errors for PreLoginRepository. diff --git a/internal/repository/postgres/oidc_prelogin.go b/internal/repository/postgres/oidc_prelogin.go index 5dd4872..bbb0c75 100644 --- a/internal/repository/postgres/oidc_prelogin.go +++ b/internal/repository/postgres/oidc_prelogin.go @@ -75,14 +75,23 @@ func (r *PreLoginRepository) Create(ctx context.Context, p *repository.PreLoginS return fmt.Errorf("oidc_pre_login encrypt pkce_verifier: %w", verr) } + // Audit 2026-05-10 MED-16 — persist UA/IP binding on Create. + // Empty values are inserted as NULL via sql.NullString so the + // schema's nullable column constraint is respected and existing + // integration tests that don't provide UA/IP keep working. + clientIP := nullableString(p.ClientIP) + userAgent := nullableString(p.UserAgent) + if p.CreatedAt.IsZero() && p.AbsoluteExpiresAt.IsZero() { _, err := r.db.ExecContext(ctx, ` INSERT INTO oidc_pre_login_sessions ( id, tenant_id, signing_key_id, oidc_provider_id, - state_enc, nonce_enc, pkce_verifier_enc - ) VALUES ($1,$2,$3,$4,$5,$6,$7)`, + state_enc, nonce_enc, pkce_verifier_enc, + client_ip, user_agent + ) VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9)`, p.ID, p.TenantID, p.SigningKeyID, p.OIDCProviderID, - stateEnc, nonceEnc, verifierEnc) + stateEnc, nonceEnc, verifierEnc, + clientIP, userAgent) if err != nil { return fmt.Errorf("oidc_pre_login create: %w", err) } @@ -98,16 +107,26 @@ func (r *PreLoginRepository) Create(ctx context.Context, p *repository.PreLoginS _, err := r.db.ExecContext(ctx, ` INSERT INTO oidc_pre_login_sessions ( id, tenant_id, signing_key_id, oidc_provider_id, - state_enc, nonce_enc, pkce_verifier_enc, created_at, absolute_expires_at - ) VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9)`, + state_enc, nonce_enc, pkce_verifier_enc, + client_ip, user_agent, + created_at, absolute_expires_at + ) VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11)`, p.ID, p.TenantID, p.SigningKeyID, p.OIDCProviderID, - stateEnc, nonceEnc, verifierEnc, p.CreatedAt, p.AbsoluteExpiresAt) + stateEnc, nonceEnc, verifierEnc, + clientIP, userAgent, + p.CreatedAt, p.AbsoluteExpiresAt) if err != nil { return fmt.Errorf("oidc_pre_login create: %w", err) } return nil } +// MED-16 reuses nullableString from discovery.go (same package). It +// returns sql.NullString{Valid:false} for empty strings so the database +// stores NULL rather than the literal empty string — avoiding ambiguity +// at consume time between "row had no binding" and "row had an explicit +// empty binding". + // LookupAndConsume reads the row by id and atomically deletes it // (single-use). Returns ErrPreLoginNotFound on miss; ErrPreLoginExpired // when the row was found but past its TTL (the row is still deleted in @@ -132,16 +151,19 @@ func (r *PreLoginRepository) LookupAndConsume(ctx context.Context, id string) (* RETURNING id, tenant_id, signing_key_id, oidc_provider_id, state, nonce, pkce_verifier, state_enc, nonce_enc, pkce_verifier_enc, + client_ip, user_agent, created_at, absolute_expires_at`, id) var p repository.PreLoginSession var statePlain, noncePlain, verifierPlain sql.NullString + var clientIP, userAgent sql.NullString var stateEnc, nonceEnc, verifierEnc []byte if err := row.Scan( &p.ID, &p.TenantID, &p.SigningKeyID, &p.OIDCProviderID, &statePlain, &noncePlain, &verifierPlain, &stateEnc, &nonceEnc, &verifierEnc, + &clientIP, &userAgent, &p.CreatedAt, &p.AbsoluteExpiresAt, ); err != nil { if errors.Is(err, sql.ErrNoRows) { @@ -168,6 +190,16 @@ func (r *PreLoginRepository) LookupAndConsume(ctx context.Context, id string) (* p.PKCEVerifier = verifier } + // Audit 2026-05-10 MED-16 — surface the binding columns for the + // service-layer UA / IP compare. Empty when the row was created + // before this migration landed (rolling-deploy compat). + if clientIP.Valid { + p.ClientIP = clientIP.String + } + if userAgent.Valid { + p.UserAgent = userAgent.String + } + if time.Now().UTC().After(p.AbsoluteExpiresAt) { return nil, repository.ErrPreLoginExpired } diff --git a/migrations/000044_prelogin_uaip.down.sql b/migrations/000044_prelogin_uaip.down.sql new file mode 100644 index 0000000..4bfd066 --- /dev/null +++ b/migrations/000044_prelogin_uaip.down.sql @@ -0,0 +1,4 @@ +-- Down for 000044 — drop the pre-login UA/IP binding columns. +ALTER TABLE oidc_pre_login_sessions + DROP COLUMN IF EXISTS client_ip, + DROP COLUMN IF EXISTS user_agent; diff --git a/migrations/000044_prelogin_uaip.up.sql b/migrations/000044_prelogin_uaip.up.sql new file mode 100644 index 0000000..208ae21 --- /dev/null +++ b/migrations/000044_prelogin_uaip.up.sql @@ -0,0 +1,47 @@ +-- ============================================================================= +-- 2026-05-10 Audit / MED-16 closure +-- ============================================================================= +-- +-- Pre-login rows in oidc_pre_login_sessions used to carry only the OIDC state, +-- nonce, and PKCE verifier — the binding to the user agent that initiated the +-- handshake was implicit (the pre-login cookie's HMAC, scoped to the active +-- SessionSigningKey, only verifies that *some* caller of /auth/oidc/login is +-- talking to /auth/oidc/callback; it does not verify that the SAME browser / +-- HTTP client is on both sides). +-- +-- RFC 9700 §4.7.1 (security best current practice for OAuth 2.0) recommends +-- binding state to a user-agent fingerprint + source IP so that a pre-login +-- cookie leaked in transit (CSRF / XSS / TLS termination on a shared proxy) +-- cannot be replayed by a different browser. Even with HMAC integrity, the +-- attacker who steals the bytes could otherwise complete the handshake. +-- +-- This migration adds: +-- - client_ip TEXT — captured at /auth/oidc/login from the request's +-- clientIPFromRequest result (post LOW-5 XFF +-- trusted-proxy gating, so the value is honest). +-- - user_agent TEXT — captured at /auth/oidc/login from r.UserAgent(). +-- Stored verbatim; the consume path compares with +-- constant-time equality. +-- +-- Both columns are nullable so in-flight pre-login rows from pre-deploy code +-- paths still consume cleanly (the consume-side check only enforces when both +-- the row AND the request carry non-empty values; legacy rows pass through +-- because the row's binding columns are NULL). +-- +-- The audit failure_category distinguishes: +-- - prelogin_ua_mismatch — UA changed across the redirect (most common +-- real-world false-positive: aggressive UA +-- rewriters on enterprise proxies). +-- - prelogin_ip_mismatch — source IP changed across the redirect (mobile +-- carrier-grade NAT, dual-stack v4/v6 hops, VPN +-- toggle). +-- - prelogin_uaip_mismatch — both differ. +-- +-- Operators wanting to disable the gate (e.g. dual-stack v4/v6 environments +-- where source IP routinely flips) set the CERTCTL_OIDC_PRELOGIN_REQUIRE_UA +-- or CERTCTL_OIDC_PRELOGIN_REQUIRE_IP env var to "false". Default true. +-- ============================================================================= + +ALTER TABLE oidc_pre_login_sessions + ADD COLUMN IF NOT EXISTS client_ip TEXT, + ADD COLUMN IF NOT EXISTS user_agent TEXT;