From d4e69a566b7d1bbd27c9ab577fd15bd3e8e8f151 Mon Sep 17 00:00:00 2001 From: xarmian Date: Wed, 14 Oct 2026 03:30:20 -0400 Subject: [PATCH] fix(auth): reset-password, register and bootstrap refuse a cross-site form, as login does (BUG-3680) (#2146) * fix(auth): reset-password, register and bootstrap refuse a cross-site form, as login does (BUG-3680) /api/v1/auth/reset-password is CSRF-exempt and a completed reset sets a web session cookie. decodeJSON does not check the content type, so a cross-site text/plain form whose name=value serializes as JSON could complete a reset of the attacker's own account in the victim's browser and sign the victim into it (login CSRF). Only login and 2FA login-verify called refuseCrossSiteSignIn (BUG-3350). The population, by every handler that creates a session: - login, 2fa/login-verify: guarded already. - reset-password: unguarded. Guarded now. - register: CSRF-exempt without a session, and an anonymous signup signs the browser into the new account. Same class. Guarded now. - bootstrap: CSRF-exempt and signs the browser into the admin it creates. On a fresh instance reachable by loopback, or in open-bootstrap mode, a cross-site form could plant the attacker's admin. Guarded now. - oauth-login: requires the sidecar's cloud_secret, so not browser- reachable. Left as is. - rotateSessionsAfterCredentialChange callers are authenticated, CSRF-checked writes; the CLI approval session is not a browser cookie. Every legitimate client of the three already sends application/json with no Origin (CLI) or this server's origin (the SPA), and pad-cloud calls none of them. Tests, one per door: text/plain, urlencoded, foreign Origin and null Origin are refused, set no session cookie and change nothing (the reset token survives and then completes; no account is created; no admin is created). Same-origin and headless JSON still complete. Removing one door's guard fails exactly that door's test. TestBUG3350's login cases still pass. docs/api.md names the refusal for all five sign-in doors. Claude-Session: https://claude.ai/code/session_01Rj4CMyK2EqxerMihbGs3Kr * test(auth): the cross-site sign-in guard keeps an allowed CORS origin, a remote token bootstrap and an admin's SPA register working (BUG-3680) Codex r2 on #2146 found the new tests did not exercise these legitimate callers, so a guard that broke them could pass. Each control is paired with a refusal on the same door, so it proves the guard's discrimination and not a door that ignores Origin. Mutants: dropping the CORS list fails the CORS test; requiring an Origin fails the remote token bootstrap. Claude-Session: https://claude.ai/code/session_01Rj4CMyK2EqxerMihbGs3Kr --- docs/api.md | 1 + .../bug3680_cross_site_signin_doors_test.go | 255 ++++++++++++++++++ internal/server/handlers_auth.go | 19 ++ 3 files changed, 275 insertions(+) create mode 100644 internal/server/bug3680_cross_site_signin_doors_test.go diff --git a/docs/api.md b/docs/api.md index 3e685ba58..72bcf290f 100644 --- a/docs/api.md +++ b/docs/api.md @@ -99,6 +99,7 @@ One browser tab's content writes to an item are ordered by the server, also **pe - `POST /api/v1/auth/register` — create account (admin-created or invitation-based after setup). An `invitation_code` signup claims the invitation in the same transaction as the membership (BUG-3438). If the invitation was declined, cancelled or replaced since the code was read: when the code was the signup's ADMISSION (registration is otherwise closed) the account is rolled back and the answer is `404 not_found` (`500` if the rollback itself fails, since the account then still holds the email); where the signup would have been admitted anyway (Pad Cloud self-serve, or an admin creating the account) the account is KEPT, joins nothing, and the 201 carries `invitation_not_joined: {code, message}` (`invitation_gone`, `inviter_scope_changed` when the inviter is now collection-restricted (BUG-3634; `403` with that code in the admission case), or `invitation_not_applied` for another refused claim) instead of `accepted_invitation`. Additive, omitempty - `POST /api/v1/auth/login` — email/password login (returns session token) - `POST /api/v1/auth/oauth-login`, `/oauth-link` (cloud mode, pad-cloud only, cloud secret) — a social sign-in or link from the sidecar, carrying `provider`, a verified `email`, and optionally the provider's stable account `subject` (Google/Apple `sub`, GitHub user id). A subject is stored in `user_oauth_identities` (TASK-3351): the first subject-bearing sign-in of a linked provider binds it, and a different subject is refused `403 oauth_subject_mismatch` (with `details.recovery: password_reset`). **Since BUG-3671 a sign-in WITHOUT a subject is refused the same way once the account has one bound for that provider**, so an email match alone never reaches an account tied to a provider account. A sidecar that sends no subjects binds none and keeps the email path. Deploy order: the pad server before a pad-cloud that sends subjects. +- The doors that sign a browser in (`login`, `2fa/login-verify`, `reset-password`, `register`, `bootstrap`) are CSRF-exempt and refuse a cross-site form instead: a body that is not `application/json` is `415 unsupported_media_type`, and a browser `Origin` that is not this server or an allowed CORS origin is `403 forbidden`. No `Origin` (the CLI, curl) is accepted (BUG-3350, BUG-3680) - `POST /api/v1/auth/logout` — destroy session - `GET/PATCH /api/v1/auth/me` — current user profile (GET) and update name/password (PATCH) - `GET /api/v1/auth/export` — the caller's account export (GDPR portability), streamed as `pad-account-export.json` (not `pad-export.json`, which is the workspace bundle's file): `{format: "pad-account-export", version: 1, user, workspaces: [...], complete: true}`, where owned workspaces carry their collections and items. `complete: true` is written last and only after every workspace (BUG-3626). The status line goes out before the body, so a timeout (60s) or a store error mid-export cannot become an error status. The server cuts the connection instead, and the body never reaches the trailer. A reader must refuse a file without `complete: true`; the web client does. 403 `forbidden` when the caller is collection-restricted in any owned workspace (BUG-1945) diff --git a/internal/server/bug3680_cross_site_signin_doors_test.go b/internal/server/bug3680_cross_site_signin_doors_test.go new file mode 100644 index 000000000..632afbbe4 --- /dev/null +++ b/internal/server/bug3680_cross_site_signin_doors_test.go @@ -0,0 +1,255 @@ +package server + +import ( + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/PerpetualSoftware/pad/internal/store" +) + +// BUG-3680: every CSRF-exempt door that signs the browser in refuses a +// cross-site form, as login does (BUG-3350). Reset, register and bootstrap +// each set a web session cookie and were unguarded, so a cross-site +// text/plain form could sign a visitor into an account the attacker controls. +// Each door gets a refused leg that must change nothing, and a same-origin +// control that must complete. + +type crossSiteCase struct { + name, ct, origin string + want int +} + +var crossSiteRefusals = []crossSiteCase{ + {"text/plain form", "text/plain", "", http.StatusUnsupportedMediaType}, + {"urlencoded form", "application/x-www-form-urlencoded", "", http.StatusUnsupportedMediaType}, + {"JSON from a foreign origin", "application/json", "https://evil.example", http.StatusForbidden}, + {"JSON from a null origin", "application/json", "null", http.StatusForbidden}, +} + +func postSignInDoor(srv *Server, path, body, ct, origin, remoteAddr string) *httptest.ResponseRecorder { + req := httptest.NewRequest("POST", path, strings.NewReader(body)) + req.Host = "pad.example.test" + req.RemoteAddr = remoteAddr + if ct != "" { + req.Header.Set("Content-Type", ct) + } + if origin != "" { + req.Header.Set("Origin", origin) + } + rr := httptest.NewRecorder() + srv.ServeHTTP(rr, req) + return rr +} + +func hasSessionCookie(rr *httptest.ResponseRecorder) bool { + for _, c := range rr.Result().Cookies() { + if strings.Contains(c.Name, "session") && c.Value != "" { + return true + } + } + return false +} + +func jsonBody(t *testing.T, v any) string { + t.Helper() + b, err := json.Marshal(v) + if err != nil { + t.Fatal(err) + } + return string(b) +} + +func TestBUG3680_ResetPasswordRefusesCrossSiteForms(t *testing.T) { + t.Setenv("PAD_DISABLE_RATE_LIMITS", "1") + srv := testServer(t) + bootstrapFirstUser(t, srv, "reset-3680@example.com", "R") + user, err := srv.store.GetUserByEmail("reset-3680@example.com") + if err != nil || user == nil { + t.Fatalf("GetUserByEmail: %v", err) + } + for _, c := range crossSiteRefusals { + tok, err := srv.store.CreatePasswordReset(user.ID) + if err != nil { + t.Fatalf("CreatePasswordReset: %v", err) + } + body := jsonBody(t, map[string]string{"token": tok, "password": "attacker-chosen-passphrase-1"}) + rr := postSignInDoor(srv, "/api/v1/auth/reset-password", body, c.ct, c.origin, "192.0.2.7:1") + if rr.Code != c.want { + t.Errorf("%s: %d, want %d (%s)", c.name, rr.Code, c.want, rr.Body.String()) + } + if hasSessionCookie(rr) { + t.Errorf("%s: a refused reset set a session cookie", c.name) + } + // The refusal consumed nothing: the same token still completes a + // same-origin reset. + ctl := postSignInDoor(srv, "/api/v1/auth/reset-password", body, "application/json", "http://pad.example.test", "192.0.2.7:1") + if ctl.Code != http.StatusOK || !hasSessionCookie(ctl) { + t.Errorf("%s: the token did not survive the refusal: same-origin reset %d (%s)", c.name, ctl.Code, ctl.Body.String()) + } + } + // CONTROL: a headless client (no Origin, JSON) still resets. + tok, err := srv.store.CreatePasswordReset(user.ID) + if err != nil { + t.Fatalf("CreatePasswordReset: %v", err) + } + body := jsonBody(t, map[string]string{"token": tok, "password": "headless-chosen-passphrase-2"}) + if rr := postSignInDoor(srv, "/api/v1/auth/reset-password", body, "application/json", "", "192.0.2.7:1"); rr.Code != http.StatusOK { + t.Errorf("headless reset: %d, want 200 (%s)", rr.Code, rr.Body.String()) + } +} + +func TestBUG3680_RegisterRefusesCrossSiteForms(t *testing.T) { + t.Setenv("PAD_DISABLE_RATE_LIMITS", "1") + f := newAccessFixture(t, store.DriverSQLite) + for i, c := range crossSiteRefusals { + email := fmt.Sprintf("signup%d-3680@example.com", i) + inv := f.invite(email, "editor") + body := jsonBody(t, map[string]string{ + "email": email, "name": "Invitee", "password": "correct-horse-battery-staple", "invitation_code": inv.Code, + }) + rr := postSignInDoor(f.srv, "/api/v1/auth/register", body, c.ct, c.origin, "192.0.2.7:1") + if rr.Code != c.want { + t.Errorf("%s: %d, want %d (%s)", c.name, rr.Code, c.want, rr.Body.String()) + } + if hasSessionCookie(rr) { + t.Errorf("%s: a refused signup set a session cookie", c.name) + } + if u, _ := f.srv.store.GetUserByEmail(email); u != nil { + t.Errorf("%s: a refused signup created the account", c.name) + } + // CONTROL on the same invitation: same-origin JSON completes, so the + // refusal above was the guard and not a broken fixture. + if i == 0 { + ctl := postSignInDoor(f.srv, "/api/v1/auth/register", body, "application/json", "http://pad.example.test", "192.0.2.7:1") + if ctl.Code != http.StatusCreated || !hasSessionCookie(ctl) { + t.Errorf("same-origin signup: %d, want 201 with a session (%s)", ctl.Code, ctl.Body.String()) + } + } + } +} + +func TestBUG3680_BootstrapRefusesCrossSiteForms(t *testing.T) { + t.Setenv("PAD_DISABLE_RATE_LIMITS", "1") + srv := testServer(t) + body := jsonBody(t, map[string]string{"email": "boot-3680@example.com", "name": "B", "password": "correct-horse-battery-staple"}) + for _, c := range crossSiteRefusals { + rr := postSignInDoor(srv, "/api/v1/auth/bootstrap", body, c.ct, c.origin, "127.0.0.1:1") + if rr.Code != c.want { + t.Errorf("%s: %d, want %d (%s)", c.name, rr.Code, c.want, rr.Body.String()) + } + if hasSessionCookie(rr) { + t.Errorf("%s: a refused bootstrap set a session cookie", c.name) + } + if n, err := srv.store.UserCount(); err != nil || n != 0 { + t.Fatalf("%s: a refused bootstrap created an admin (count %d, err %v)", c.name, n, err) + } + } + // CONTROL: the CLI's shape (loopback, JSON, no Origin) still bootstraps. + if rr := postSignInDoor(srv, "/api/v1/auth/bootstrap", body, "application/json", "", "127.0.0.1:1"); rr.Code != http.StatusCreated { + t.Errorf("headless loopback bootstrap: %d, want 201 (%s)", rr.Code, rr.Body.String()) + } +} + +// The legitimate callers the guard must keep working beyond the same-origin +// and headless controls above (Codex r2 on #2146): an allowed CORS origin, +// the X-Bootstrap-Token bootstrap from a remote host, and an admin creating +// an account from the SPA with its session. + +func TestBUG3680_ResetPasswordAcceptsAllowedCORSOrigin(t *testing.T) { + t.Setenv("PAD_DISABLE_RATE_LIMITS", "1") + srv := testServer(t) + srv.SetCORSOrigins("https://app.example.test") + bootstrapFirstUser(t, srv, "cors-3680@example.com", "C") + user, err := srv.store.GetUserByEmail("cors-3680@example.com") + if err != nil || user == nil { + t.Fatalf("GetUserByEmail: %v", err) + } + tok, err := srv.store.CreatePasswordReset(user.ID) + if err != nil { + t.Fatalf("CreatePasswordReset: %v", err) + } + body := jsonBody(t, map[string]string{"token": tok, "password": "cors-chosen-passphrase-3"}) + // Refused first from an origin NOT on the list, so the accept below is the + // list doing it and not a guard that ignores Origin once CORS is set. + if rr := postSignInDoor(srv, "/api/v1/auth/reset-password", body, "application/json", "https://other.example.test", "192.0.2.7:1"); rr.Code != http.StatusForbidden { + t.Errorf("unlisted origin: %d, want 403 (%s)", rr.Code, rr.Body.String()) + } + if rr := postSignInDoor(srv, "/api/v1/auth/reset-password", body, "application/json", "https://app.example.test", "192.0.2.7:1"); rr.Code != http.StatusOK || !hasSessionCookie(rr) { + t.Errorf("allowed CORS origin: %d, want 200 with a session (%s)", rr.Code, rr.Body.String()) + } +} + +func TestBUG3680_BootstrapTokenHeaderFromRemoteStillWorks(t *testing.T) { + t.Setenv("PAD_DISABLE_RATE_LIMITS", "1") + srv := testServer(t) + srv.SetBootstrapToken("bug3680-bootstrap-token", t.TempDir()+"/bootstrap-token") + body := jsonBody(t, map[string]string{"email": "remote-boot-3680@example.com", "name": "R", "password": "correct-horse-battery-staple"}) + post := func(ct, origin string) *httptest.ResponseRecorder { + req := httptest.NewRequest("POST", "/api/v1/auth/bootstrap", strings.NewReader(body)) + req.Host = "pad.example.test" + req.RemoteAddr = "198.51.100.9:1" + req.Header.Set("Content-Type", ct) + req.Header.Set(BootstrapTokenHeader, "bug3680-bootstrap-token") + if origin != "" { + req.Header.Set("Origin", origin) + } + rr := httptest.NewRecorder() + srv.ServeHTTP(rr, req) + return rr + } + // The token does not lift the guard: a cross-site form carrying it is + // still refused, and consumes nothing. + if rr := post("text/plain", "https://evil.example"); rr.Code != http.StatusUnsupportedMediaType { + t.Errorf("cross-site form with the token: %d, want 415 (%s)", rr.Code, rr.Body.String()) + } + rr := post("application/json", "") + if rr.Code != http.StatusCreated || !hasSessionCookie(rr) { + t.Errorf("remote bootstrap with the token header: %d, want 201 with a session (%s)", rr.Code, rr.Body.String()) + } + if u, err := srv.store.GetUserByEmail("remote-boot-3680@example.com"); err != nil { + t.Fatalf("GetUserByEmail: %v", err) + } else if u == nil { + t.Error("the remote token bootstrap did not create the admin") + } +} + +func TestBUG3680_AdminRegisterFromTheSPA(t *testing.T) { + t.Setenv("PAD_DISABLE_RATE_LIMITS", "1") + srv := testServer(t) + adminToken := bootstrapFirstUser(t, srv, "admin-3680@example.com", "A") + const testCSRF = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" + post := func(email, origin string) *httptest.ResponseRecorder { + body := jsonBody(t, map[string]string{"email": email, "name": "New", "password": "correct-horse-battery-staple"}) + req := httptest.NewRequest("POST", "/api/v1/auth/register", strings.NewReader(body)) + req.Host = "pad.example.test" + req.RemoteAddr = "192.0.2.7:1" + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Origin", origin) + req.AddCookie(&http.Cookie{Name: "pad_session", Value: adminToken}) + req.AddCookie(&http.Cookie{Name: "pad_csrf", Value: testCSRF}) + req.Header.Set("X-CSRF-Token", testCSRF) + rr := httptest.NewRecorder() + srv.ServeHTTP(rr, req) + return rr + } + if rr := post("admin-made-1-3680@example.com", "https://evil.example"); rr.Code != http.StatusForbidden { + t.Errorf("admin register from a foreign origin: %d, want 403 (%s)", rr.Code, rr.Body.String()) + } + if u, err := srv.store.GetUserByEmail("admin-made-1-3680@example.com"); err != nil { + t.Fatalf("GetUserByEmail: %v", err) + } else if u != nil { + t.Error("a refused admin register created the account") + } + if rr := post("admin-made-2-3680@example.com", "http://pad.example.test"); rr.Code != http.StatusCreated { + t.Errorf("admin register from the SPA (same origin): %d, want 201 (%s)", rr.Code, rr.Body.String()) + } + if u, err := srv.store.GetUserByEmail("admin-made-2-3680@example.com"); err != nil { + t.Fatalf("GetUserByEmail: %v", err) + } else if u == nil { + t.Error("the admin's SPA register did not create the account") + } +} diff --git a/internal/server/handlers_auth.go b/internal/server/handlers_auth.go index 23a2e001d..2cb202546 100644 --- a/internal/server/handlers_auth.go +++ b/internal/server/handlers_auth.go @@ -492,6 +492,13 @@ func (s *Server) destroyReplacedSessions(r *http.Request) { // requests with different emails could each pass validation and end up // creating two admins from one token. func (s *Server) handleBootstrap(w http.ResponseWriter, r *http.Request) { + // Bootstrap signs the browser into the admin it creates, and is + // CSRF-exempt. On a fresh instance a cross-site form from a browser that + // can reach it (loopback, the LAN in open-bootstrap mode) could plant the + // attacker's admin and sign the visitor in (BUG-3680). + if s.refuseCrossSiteSignIn(w, r) { + return + } s.bootstrapMu.Lock() defer s.bootstrapMu.Unlock() @@ -648,6 +655,13 @@ func (s *Server) rollbackSignup(userID string) error { // Registration is restricted to admins or users with a valid invitation code // so invitees can create an account via the /join/[code] flow. func (s *Server) handleRegister(w http.ResponseWriter, r *http.Request) { + // An anonymous signup signs the browser into the new account, and the + // path is CSRF-exempt without a session: refuse a cross-site form, as + // login does, or a form could sign a visitor into an account the attacker + // made (BUG-3680). + if s.refuseCrossSiteSignIn(w, r) { + return + } var input struct { Email string `json:"email"` Username string `json:"username"` @@ -1831,6 +1845,11 @@ func (s *Server) handleLocalReset(w http.ResponseWriter, r *http.Request) { // handleResetPassword validates a reset token and sets a new password. func (s *Server) handleResetPassword(w http.ResponseWriter, r *http.Request) { + // A completed reset signs the browser in (below), so it is a sign-in + // door, CSRF-exempt like login: refuse a cross-site form (BUG-3680). + if s.refuseCrossSiteSignIn(w, r) { + return + } var input struct { Token string `json:"token"` Password string `json:"password"`