mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-10-26 06:19:49 +00:00
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
This commit is contained in:
xarmian
@@ -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)
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
}
|
||||
@@ -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"`
|
||||
|
||||
Reference in New Issue
Block a user