fix(auth): a verification link verifies only for its account's session; the mailbox owner can claim (BUG-3382) (#1758)

* fix(auth): a verification link verifies only for its account's session; the mailbox owner can claim (BUG-3382)

Registration mails the verification link to the address, and the address
may belong to someone other than whoever registered it. Clicking it used to
verify the squatter's account in place, keeping the squatter's password and
sessions.

- POST /auth/verify-email now verifies only when the request is signed in as
  the account the token names. Otherwise it answers 409 verify_needs_session
  with {email, can_claim} and leaves the token unspent.
- New POST /auth/verify-email/claim {token}: the mailbox owner claims a
  never-verified account. One transaction spends the token, resets every
  credential (an unusable password, 2FA, sessions, PATs, OAuth, CLI
  handoffs, reset and verification tokens), marks the address verified,
  strips memberships, grants, watches and tabs in workspaces the account
  does not own, and soft-deletes the ones it owns (TASK-3351 ruling 3). An
  account with a billing customer is refused with
  account_claim_needs_support (ruling 4). The answer carries a
  set-password path and the affected workspace names.
- users.credential_epoch (SQLite 110 / PG 084): bumped by a password change,
  a disable and a claim. Every session mint is fenced on the epoch read
  when the credential was checked (login, 2FA via the challenge token, CLI
  approval, reset completion, rotation, OAuth login), so a sign-in checked
  before a claim mints nothing after it (401 credentials_changed).
- The web verify page offers "Sign in to verify" (back to the link), and,
  for an unverified address, a confirmed claim listing what was removed.
- A regression test shows that an unverified account's cookie cannot use
  the cookie-fallback routes.

Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC

* fix(auth): fence password writes on the checked epoch; own verify bucket; switch account (BUG-3382)

Codex round 1:
- A password change (PATCH /me) or a reset completion that started before
  a claim could still overwrite the claim's reset. UserUpdate.ExpectedEpoch
  makes the write conditional on the epoch its authority was checked under:
  the current password's read, or the reset token's spend. The spend now
  reads the epoch in the same statement, and a test hook puts a claim
  between the spend and the read-back. A mismatch writes nothing
  (credentials_changed).
- verify-email and its claim move to their own EmailVerify bucket (10/hour),
  so opening the link, refreshing and claiming no longer spends the
  PasswordReset budget the claimant needs to set a password.
- The verify page, signed in as a different account, signs out before
  sending the user to sign in. A plain /login link bounced straight back.

Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC
This commit is contained in:
xarmian
2026-10-03 09:48:19 -04:00
committed by GitHub
parent 3bcef0e04d
commit 98871c7e01
23 changed files with 1239 additions and 70 deletions
+4
View File
@@ -44,6 +44,10 @@ const (
// ActionEmailVerified is logged when a user confirms their email address
// via a verification link (POST /auth/verify-email). PLAN-1933 / TASK-1936.
ActionEmailVerified = "email_verified"
// ActionAccountClaimed is logged when the owner of an address claims a
// never-verified account registered with it, through the verification
// link (BUG-3382): every credential is reset.
ActionAccountClaimed = "account_claimed"
// ActionEmailVerifiedByAdmin is logged when an admin force-verifies a
// user's email from the admin console (DR-7). PLAN-1933 / TASK-1936.
ActionEmailVerifiedByAdmin = "email_verified_by_admin"
+33 -23
View File
@@ -7,29 +7,33 @@ import (
// User represents a registered user in the system.
type User struct {
ID string `json:"id"`
Email string `json:"email"`
Username string `json:"username"` // Unique handle; empty until set
Name string `json:"name"`
PasswordHash string `json:"-"` // Never serialized
Role string `json:"role"` // "admin" or "member"
AvatarURL string `json:"avatar_url,omitempty"`
TOTPSecret string `json:"-"` // Never serialized
TOTPEnabled bool `json:"totp_enabled"`
RecoveryCodes string `json:"-"` // Never serialized
Plan string `json:"plan"` // "free", "pro", or "self-hosted"
PlanExpiresAt string `json:"plan_expires_at,omitempty"`
PlanSource string `json:"-"` // Who set Plan: "manual" or "stripe" (TASK-3295); surfaced only on admin user responses
StripeCustomerID string `json:"-"` // Never serialized
PlanOverrides string `json:"plan_overrides,omitempty"` // JSON overrides for per-user limits
OAuthProviders string `json:"-"` // JSON array of linked providers, e.g. ["github","google"]
PasswordSet bool `json:"password_set"` // True if the user explicitly set a password (vs. OAuth placeholder hash)
DisabledAt string `json:"disabled_at,omitempty"` // Non-empty = account disabled
EmailVerifiedAt string `json:"email_verified_at,omitempty"` // Non-empty = email verified (mirror DisabledAt). Empty/NULL = unverified. PLAN-1933 / TASK-1935.
LastActiveAt string `json:"last_active_at,omitempty"` // Last authenticated API request (any read or write)
LastWriteAt string `json:"last_write_at,omitempty"` // Last mutating action (item/comment/attachment); see Store.TouchUserWrite. PLAN-1542 / TASK-1543.
CreatedAt time.Time `json:"created_at"`
UpdatedAt time.Time `json:"updated_at"`
ID string `json:"id"`
Email string `json:"email"`
Username string `json:"username"` // Unique handle; empty until set
Name string `json:"name"`
PasswordHash string `json:"-"` // Never serialized
Role string `json:"role"` // "admin" or "member"
AvatarURL string `json:"avatar_url,omitempty"`
TOTPSecret string `json:"-"` // Never serialized
TOTPEnabled bool `json:"totp_enabled"`
RecoveryCodes string `json:"-"` // Never serialized
Plan string `json:"plan"` // "free", "pro", or "self-hosted"
PlanExpiresAt string `json:"plan_expires_at,omitempty"`
PlanSource string `json:"-"` // Who set Plan: "manual" or "stripe" (TASK-3295); surfaced only on admin user responses
StripeCustomerID string `json:"-"` // Never serialized
PlanOverrides string `json:"plan_overrides,omitempty"` // JSON overrides for per-user limits
OAuthProviders string `json:"-"` // JSON array of linked providers, e.g. ["github","google"]
PasswordSet bool `json:"password_set"` // True if the user explicitly set a password (vs. OAuth placeholder hash)
// CredentialEpoch is bumped by every credential change (password reset or
// change, disable, account claim). A sign-in mints a session only if the
// epoch it read when checking the credential is still current (BUG-3382).
CredentialEpoch int64 `json:"-"`
DisabledAt string `json:"disabled_at,omitempty"` // Non-empty = account disabled
EmailVerifiedAt string `json:"email_verified_at,omitempty"` // Non-empty = email verified (mirror DisabledAt). Empty/NULL = unverified. PLAN-1933 / TASK-1935.
LastActiveAt string `json:"last_active_at,omitempty"` // Last authenticated API request (any read or write)
LastWriteAt string `json:"last_write_at,omitempty"` // Last mutating action (item/comment/attachment); see Store.TouchUserWrite. PLAN-1542 / TASK-1543.
CreatedAt time.Time `json:"created_at"`
UpdatedAt time.Time `json:"updated_at"`
}
// IsDisabled returns true if the user account has been disabled.
@@ -123,6 +127,12 @@ type UserUpdate struct {
Username *string `json:"username,omitempty"`
Password *string `json:"password,omitempty"` // Plaintext, will be hashed
AvatarURL *string `json:"avatar_url,omitempty"`
// ExpectedEpoch, when set with Password, makes the password write
// conditional on the account's credential_epoch still being this value:
// the epoch the caller's authority (the current password, a reset token)
// was checked under (BUG-3382). A mismatch writes nothing and returns
// store.ErrCredentialsChanged.
ExpectedEpoch *int64 `json:"-"`
}
// Session represents a database-backed authentication session.
@@ -0,0 +1,305 @@
package server
import (
"net/http"
"strings"
"testing"
"time"
"github.com/PerpetualSoftware/pad/internal/models"
"github.com/PerpetualSoftware/pad/internal/store"
)
// BUG-3382: a squatter registers the victim's address; the verification
// link goes to the victim. Clicking it used to verify the SQUATTER's account
// (keeping the squatter's password and session). Now a link verifies only for
// a request signed in as that account, and the mailbox owner can instead
// CLAIM the address: every credential is reset and they set their own.
type squatFixture struct {
srv *Server
mails chan capturedMail
squatter *models.User
sessTok string // the squatter's session
token string // the verification token, as mailed to the victim
adminTok string
adminWS *models.Workspace
adminItem *models.Item
}
func newSquatFixture(t *testing.T) squatFixture {
t.Helper()
srv := testServer(t)
adminTok := bootstrapFirstUser(t, srv, "admin@pad.test", "Admin")
sender, mails := newMailSink(t)
srv.baseURL = "https://app.getpad.dev"
srv.SetEmailSender(sender)
srv.cloudMode = true
rr := doRequest(srv, "POST", "/api/v1/auth/register", map[string]string{
"email": "victim@example.com", "name": "Squatter", "password": "squatter-password-123",
})
if rr.Code != http.StatusCreated {
t.Fatalf("squatter register: %d %s", rr.Code, rr.Body.String())
}
var reg struct {
Token string `json:"token"`
}
parseJSON(t, rr, &reg)
token := extractVerifyToken(t, waitMail(t, mails))
squatter, err := srv.store.GetUserByEmail("victim@example.com")
if err != nil || squatter == nil {
t.Fatalf("lookup: %v", err)
}
admin, _ := srv.store.GetUserByEmail("admin@pad.test")
ws, err := srv.store.CreateWorkspace(models.WorkspaceCreate{Name: "Admin WS", OwnerID: admin.ID})
if err != nil {
t.Fatal(err)
}
if err := srv.store.AddWorkspaceMember(ws.ID, admin.ID, "owner"); err != nil {
t.Fatal(err)
}
item := mustItem(t, srv, ws.ID, mustCollection(t, srv, ws.ID, "Tasks").ID, "shared")
return squatFixture{srv, mails, squatter, reg.Token, token, adminTok, ws, item}
}
// What a squatter can plant before the claim: membership elsewhere with a
// grant, a watch and a tab there, and credentials of every kind.
func (f squatFixture) plant(t *testing.T) {
t.Helper()
s := f.srv.store
admin, _ := s.GetUserByEmail("admin@pad.test")
if err := s.AddWorkspaceMember(f.adminWS.ID, f.squatter.ID, "viewer"); err != nil {
t.Fatal(err)
}
if _, err := s.CreateItemGrant(f.adminWS.ID, f.adminItem.ID, f.squatter.ID, "view", admin.ID); err != nil {
t.Fatal(err)
}
if _, err := s.CreateWatch(f.adminWS.ID, f.squatter.ID, f.adminItem.ID, ""); err != nil {
t.Fatal(err)
}
if _, err := s.OpenWorkspaceTab(f.squatter.ID, f.adminWS.ID, false); err != nil {
t.Fatal(err)
}
if _, err := s.CreateAPIToken(f.squatter.ID, models.APITokenCreate{Name: "planted"}, 30, 0); err != nil {
t.Fatal(err)
}
cli, err := s.CreateCLIAuthSession()
if err != nil {
t.Fatal(err)
}
cliTok, err := s.CreateSession(f.squatter.ID, "cli-browser-auth", "192.0.2.1", "", time.Hour)
if err != nil {
t.Fatal(err)
}
if err := s.ApproveCLIAuthSession(cli.Code, cliTok, f.squatter.ID); err != nil {
t.Fatal(err)
}
}
func count(t *testing.T, srv *Server, query string, args ...any) int {
t.Helper()
var n int
if err := srv.store.DB().QueryRow(query, args...).Scan(&n); err != nil {
t.Fatalf("%s: %v", query, err)
}
return n
}
func TestBUG3382_LinkWithoutTheAccountsSessionDoesNotVerify(t *testing.T) {
f := newSquatFixture(t)
// The victim clicks with no session: nothing is verified, the token is
// not spent, and the squatter still holds what they held.
rr := doRequest(f.srv, "POST", "/api/v1/auth/verify-email", map[string]string{"token": f.token})
if rr.Code != http.StatusConflict || !strings.Contains(rr.Body.String(), "verify_needs_session") {
t.Fatalf("verify without a session: %d %s", rr.Code, rr.Body.String())
}
if u, _ := f.srv.store.GetUser(f.squatter.ID); u.IsEmailVerified() {
t.Fatal("a link clicked without the account's session verified it")
}
// Signed in as someone else: the same answer.
rr = doRequestWithCookie(f.srv, "POST", "/api/v1/auth/verify-email", map[string]string{"token": f.token}, f.adminTok)
if rr.Code != http.StatusConflict {
t.Fatalf("verify as another account: %d %s", rr.Code, rr.Body.String())
}
// Control: the account's own session verifies, so the token survived.
rr = doRequestWithCookie(f.srv, "POST", "/api/v1/auth/verify-email", map[string]string{"token": f.token}, f.sessTok)
if rr.Code != http.StatusOK {
t.Fatalf("verify with the account's session: %d %s", rr.Code, rr.Body.String())
}
}
func TestBUG3382_MailboxOwnerClaimsTheAddress(t *testing.T) {
f := newSquatFixture(t)
f.plant(t)
s := f.srv.store
owned := count(t, f.srv, `SELECT COUNT(*) FROM workspaces WHERE owner_id = ? AND deleted_at IS NULL`, f.squatter.ID)
if owned == 0 {
t.Fatal("precondition: signup should have created the squatter's workspace")
}
rr := doRequest(f.srv, "POST", "/api/v1/auth/verify-email/claim", map[string]string{"token": f.token})
if rr.Code != http.StatusOK {
t.Fatalf("claim: %d %s", rr.Code, rr.Body.String())
}
var claim struct {
ResetPath string `json:"reset_path"`
StrippedWorkspaces []string `json:"stripped_workspaces"`
DeletedWorkspaces []string `json:"deleted_workspaces"`
}
parseJSON(t, rr, &claim)
if !strings.HasPrefix(claim.ResetPath, "/reset-password/") {
t.Fatalf("no set-password path: %+v", claim)
}
if len(claim.StrippedWorkspaces) != 1 || claim.StrippedWorkspaces[0] != "Admin WS" || len(claim.DeletedWorkspaces) != owned {
t.Errorf("claim reported %+v", claim)
}
u, _ := s.GetUser(f.squatter.ID)
if !u.IsEmailVerified() || u.PasswordSet || u.TOTPEnabled {
t.Errorf("after the claim: verified=%v password_set=%v totp=%v", u.IsEmailVerified(), u.PasswordSet, u.TOTPEnabled)
}
// The squatter's session, password, PAT and CLI handoff are dead.
if rr := doRequestWithCookie(f.srv, "GET", "/api/v1/auth/me", nil, f.sessTok); rr.Code == http.StatusOK {
t.Error("the squatter's session survived the claim")
}
if ok, _ := s.ValidatePassword("victim@example.com", "squatter-password-123"); ok != nil {
t.Error("the squatter's password survived the claim")
}
for what, q := range map[string]string{
"sessions": `SELECT COUNT(*) FROM sessions WHERE user_id = ?`,
"api tokens": `SELECT COUNT(*) FROM api_tokens WHERE user_id = ?`,
"cli handoffs": `SELECT COUNT(*) FROM cli_auth_sessions WHERE user_id = ?`,
"memberships": `SELECT COUNT(*) FROM workspace_members WHERE user_id = ? AND workspace_id = '` + f.adminWS.ID + `'`,
"item grants": `SELECT COUNT(*) FROM item_grants WHERE user_id = ?`,
"watches": `SELECT COUNT(*) FROM watches WHERE user_id = ?`,
"tabs": `SELECT COUNT(*) FROM user_workspace_tabs WHERE user_id = ?`,
"owned live ws": `SELECT COUNT(*) FROM workspaces WHERE owner_id = ? AND deleted_at IS NULL`,
} {
if n := count(t, f.srv, q, f.squatter.ID); n != 0 {
t.Errorf("%s survived the claim: %d", what, n)
}
}
// The claimant sets a password through the link and is signed in.
resetToken := strings.TrimPrefix(claim.ResetPath, "/reset-password/")
rr = doRequest(f.srv, "POST", "/api/v1/auth/reset-password", map[string]string{"token": resetToken, "password": "the-real-owners-password-9"})
if rr.Code != http.StatusOK {
t.Fatalf("set password: %d %s", rr.Code, rr.Body.String())
}
var cookie string
for _, c := range rr.Result().Cookies() {
if c.Name == sessionCookieName(f.srv.secureCookies) {
cookie = c.Value
}
}
if rr := doRequestWithCookie(f.srv, "GET", "/api/v1/auth/me", nil, cookie); rr.Code != http.StatusOK {
t.Errorf("the claimant's new session does not work: %d", rr.Code)
}
// The token is spent: a second claim is refused.
if rr := doRequest(f.srv, "POST", "/api/v1/auth/verify-email/claim", map[string]string{"token": f.token}); rr.Code != http.StatusBadRequest {
t.Errorf("a spent token claimed again: %d", rr.Code)
}
}
func TestBUG3382_ClaimRefusals(t *testing.T) {
t.Run("billing attached", func(t *testing.T) {
f := newSquatFixture(t)
if _, err := f.srv.store.DB().Exec(`UPDATE users SET stripe_customer_id = 'cus_x' WHERE id = ?`, f.squatter.ID); err != nil {
t.Fatal(err)
}
rr := doRequest(f.srv, "POST", "/api/v1/auth/verify-email/claim", map[string]string{"token": f.token})
if rr.Code != http.StatusConflict || !strings.Contains(rr.Body.String(), "account_claim_needs_support") {
t.Fatalf("claim with billing: %d %s", rr.Code, rr.Body.String())
}
if rr := doRequestWithCookie(f.srv, "GET", "/api/v1/auth/me", nil, f.sessTok); rr.Code != http.StatusOK {
t.Error("a refused claim still reset the account")
}
})
t.Run("already verified", func(t *testing.T) {
f := newSquatFixture(t)
if err := f.srv.store.SetUserEmailVerified(f.squatter.ID); err != nil {
t.Fatal(err)
}
if rr := doRequest(f.srv, "POST", "/api/v1/auth/verify-email/claim", map[string]string{"token": f.token}); rr.Code != http.StatusBadRequest {
t.Fatalf("claim of a verified account: %d %s", rr.Code, rr.Body.String())
}
})
}
// The credential epoch: a sign-in that checked its credential before a
// credential change mints nothing after it.
func TestBUG3382_StaleCredentialCheckMintsNoSession(t *testing.T) {
f := newSquatFixture(t)
s := f.srv.store
checked, _ := s.GetUser(f.squatter.ID) // a sign-in read the account here
if _, err := s.UpdateUser(f.squatter.ID, models.UserUpdate{Password: ptr("a-new-password-123")}); err != nil {
t.Fatal(err)
}
if _, err := s.CreateSessionFenced(checked.ID, checked.CredentialEpoch, "web", "192.0.2.1", "", time.Hour, time.Time{}); err != store.ErrCredentialsChanged {
t.Errorf("a session minted on a stale credential check: err=%v", err)
}
fresh, _ := s.GetUser(f.squatter.ID)
if _, err := s.CreateSessionFenced(fresh.ID, fresh.CredentialEpoch, "web", "192.0.2.1", "", time.Hour, time.Time{}); err != nil {
t.Errorf("control: a current check could not mint: %v", err)
}
// The 2FA challenge carries the first factor's epoch.
ch := generateTwoFAChallengeAt(checked.ID, "192.0.2.1", checked.CredentialEpoch, []byte("k"))
if id, epoch, err := validateTwoFAChallengeEpoch(ch, "192.0.2.1", []byte("k")); err != nil || id != checked.ID || epoch != checked.CredentialEpoch {
t.Errorf("challenge round trip: %q %d %v", id, epoch, err)
}
}
// The cookie fallbacks in PATCH /auth/me and CLI approval do not let an
// unverified account write (codex: SessionAuth resolves the cookie first).
func TestBUG3382_UnverifiedCookieCannotUseFallbackRoutes(t *testing.T) {
f := newSquatFixture(t)
rr := doRequestWithCookie(f.srv, "PATCH", "/api/v1/auth/me", map[string]string{"name": "Renamed"}, f.sessTok)
if rr.Code != http.StatusForbidden || veErrorCode(rr) != "email_not_verified" {
t.Errorf("PATCH /auth/me as an unverified account: %d %s", rr.Code, rr.Body.String())
}
cli, err := f.srv.store.CreateCLIAuthSession()
if err != nil {
t.Fatal(err)
}
rr = doRequestWithCookie(f.srv, "POST", "/api/v1/auth/cli/sessions/"+cli.Code+"/approve", nil, f.sessTok)
if rr.Code != http.StatusForbidden || veErrorCode(rr) != "email_not_verified" {
t.Errorf("CLI approval as an unverified account: %d %s", rr.Code, rr.Body.String())
}
}
func ptr[T any](v T) *T { return &v }
// Opening the link a few times, claiming, then setting the password must fit
// one address's budget: the claim has already reset the old credentials, so
// a 429 on the set-password step strands the claimant.
func TestBUG3382_ClaimThenSetPasswordFitsTheBudget(t *testing.T) {
f := newSquatFixture(t)
const addr = "198.51.100.77:1234"
for i := 0; i < 3; i++ {
rr := doRequestFromRemoteAddr(f.srv, "POST", "/api/v1/auth/verify-email", map[string]string{"token": f.token}, addr)
if rr.Code != http.StatusConflict {
t.Fatalf("open %d: %d %s", i, rr.Code, rr.Body.String())
}
}
rr := doRequestFromRemoteAddr(f.srv, "POST", "/api/v1/auth/verify-email/claim", map[string]string{"token": f.token}, addr)
if rr.Code != http.StatusOK {
t.Fatalf("claim: %d %s", rr.Code, rr.Body.String())
}
var claim struct {
ResetPath string `json:"reset_path"`
}
parseJSON(t, rr, &claim)
rr = doRequestFromRemoteAddr(f.srv, "POST", "/api/v1/auth/reset-password", map[string]string{
"token": strings.TrimPrefix(claim.ResetPath, "/reset-password/"), "password": "the-real-owners-password-9",
}, addr)
if rr.Code != http.StatusOK {
t.Fatalf("set password: %d %s", rr.Code, rr.Body.String())
}
}
+7 -2
View File
@@ -265,7 +265,7 @@ func (s *Server) handleTOTPLoginVerify(w http.ResponseWriter, r *http.Request) {
}
// Validate the challenge token (proves password was verified, checks IP + expiry)
userID, err := validateTwoFAChallenge(input.ChallengeToken, clientIP(r), s.twoFAChallengeSecret)
userID, challengeEpoch, err := validateTwoFAChallengeEpoch(input.ChallengeToken, clientIP(r), s.twoFAChallengeSecret)
if err != nil {
time.Sleep(500 * time.Millisecond)
writeError(w, http.StatusUnauthorized, "unauthorized", "Invalid or expired 2FA challenge")
@@ -393,7 +393,12 @@ func (s *Server) handleTOTPLoginVerify(w http.ResponseWriter, r *http.Request) {
return
}
// Create full session
// Create full session, fenced on the credential epoch the FIRST factor
// was checked under (BUG-3382): a password reset or account claim since
// then makes this challenge mint nothing.
if challengeEpoch >= 0 {
user.CredentialEpoch = challengeEpoch
}
token, err := s.createAuthSession(w, r, user, webSessionTTL)
if err != nil {
return
+121 -8
View File
@@ -348,7 +348,9 @@ func (s *Server) rotateSessionsAfterCredentialChange(w http.ResponseWriter, r *h
if info.Kind == store.SessionKindCLI {
device = "cli"
}
token, err := s.store.CreateSessionIssuedAt(user.ID, device, clientIP(r), r.UserAgent(), webSessionTTL, info.CreatedAt)
// Fenced on the epoch of the user the caller passes, which is the
// credential state this rotation follows (BUG-3382).
token, err := s.store.CreateSessionFenced(user.ID, user.CredentialEpoch, device, clientIP(r), r.UserAgent(), webSessionTTL, info.CreatedAt)
if errors.Is(err, store.ErrUserDisabled) {
writeError(w, http.StatusForbidden, "account_disabled", "Your account has been disabled. Contact an administrator.")
return "", false
@@ -382,7 +384,14 @@ func (s *Server) createAuthSession(w http.ResponseWriter, r *http.Request, user
if isCLIClient(r) {
device = "cli"
}
token, err := s.store.CreateSession(user.ID, device, clientIP(r), r.UserAgent(), ttl)
// Fenced on the credential epoch read when the sign-in checked its
// credential (BUG-3382): user is the account as that check loaded it.
token, err := s.store.CreateSessionFenced(user.ID, user.CredentialEpoch, device, clientIP(r), r.UserAgent(), ttl, time.Time{})
if errors.Is(err, store.ErrCredentialsChanged) {
writeError(w, http.StatusUnauthorized, "credentials_changed",
"Your sign-in details changed while you were signing in. Sign in again.")
return "", err
}
if err != nil {
writeError(w, http.StatusInternalServerError, "internal_error", "Failed to create session")
return "", err
@@ -1158,7 +1167,7 @@ func (s *Server) handleLogin(w http.ResponseWriter, r *http.Request) {
// If 2FA is enabled, return a challenge token instead of a full session.
// The challenge token is HMAC-signed, IP-bound, and expires in 5 minutes.
if user.TOTPEnabled {
challenge := generateTwoFAChallenge(user.ID, clientIP(r), s.twoFAChallengeSecret)
challenge := generateTwoFAChallengeAt(user.ID, clientIP(r), user.CredentialEpoch, s.twoFAChallengeSecret)
writeJSON(w, http.StatusOK, map[string]interface{}{
"requires_2fa": true,
"challenge_token": challenge,
@@ -1403,6 +1412,7 @@ func (s *Server) handleUpdateCurrentUser(w http.ResponseWriter, r *http.Request)
}
// Validate password change
var passwordEpoch int64
if input.NewPassword != "" {
if input.CurrentPassword == "" {
writeError(w, http.StatusBadRequest, "validation_error", "Current password is required to set a new password")
@@ -1438,6 +1448,7 @@ func (s *Server) handleUpdateCurrentUser(w http.ResponseWriter, r *http.Request)
writeError(w, http.StatusForbidden, "invalid_password", "Current password is incorrect")
return
}
passwordEpoch = valid.CredentialEpoch
}
// Build update
@@ -1447,9 +1458,17 @@ func (s *Server) handleUpdateCurrentUser(w http.ResponseWriter, r *http.Request)
}
if input.NewPassword != "" {
update.Password = &input.NewPassword
// Fenced on the epoch the current password was checked under, so a
// claim or reset that lands in between is not overwritten (BUG-3382).
update.ExpectedEpoch = &passwordEpoch
}
updated, err := s.store.UpdateUser(user.ID, update)
if errors.Is(err, store.ErrCredentialsChanged) {
writeError(w, http.StatusUnauthorized, "credentials_changed",
"Your sign-in details changed while this request was in progress. Sign in again.")
return
}
if err != nil {
writeError(w, http.StatusInternalServerError, "internal_error", "Failed to update profile")
return
@@ -1713,9 +1732,18 @@ func (s *Server) handleResetPassword(w http.ResponseWriter, r *http.Request) {
}
// Update password
// Fenced on the epoch read as the token was spent (BUG-3382): a claim or
// another password change that landed since wins, and this reset writes
// nothing.
password := input.Password
update := models.UserUpdate{Password: &password}
if _, err := s.store.UpdateUser(user.ID, update); err != nil {
update := models.UserUpdate{Password: &password, ExpectedEpoch: &user.CredentialEpoch}
updated, err := s.store.UpdateUser(user.ID, update)
if errors.Is(err, store.ErrCredentialsChanged) {
writeError(w, http.StatusConflict, "credentials_changed",
"This account's sign-in details changed after this link was opened. Request a new reset link.")
return
}
if err != nil {
writeError(w, http.StatusInternalServerError, "internal_error", "Failed to update password")
return
}
@@ -1741,8 +1769,9 @@ func (s *Server) handleResetPassword(w http.ResponseWriter, r *http.Request) {
return
}
// Create a fresh session so the user is logged in
sessionToken, err := s.store.CreateSession(user.ID, "web", clientIP(r), r.UserAgent(), webSessionTTL)
// Create a fresh session so the user is logged in, fenced on the epoch
// this reset produced (BUG-3382).
sessionToken, err := s.store.CreateSessionFenced(updated.ID, updated.CredentialEpoch, "web", clientIP(r), r.UserAgent(), webSessionTTL, time.Time{})
if err != nil {
writeError(w, http.StatusInternalServerError, "internal_error", "Password updated but failed to create session")
return
@@ -1806,12 +1835,32 @@ func (s *Server) handleVerifyEmail(w http.ResponseWriter, r *http.Request) {
return
}
pending, err := s.store.LookupEmailVerification(input.Token)
if err != nil {
writeError(w, http.StatusInternalServerError, "internal_error", "Failed to verify email")
return
}
// A disabled account does not act, here either (BUG-3349): refuse before
// the token is consumed, so it still works if the account is re-enabled.
if pending, err := s.store.LookupEmailVerification(input.Token); err == nil && pending != nil && pending.IsDisabled() {
if pending != nil && pending.IsDisabled() {
writeError(w, http.StatusForbidden, "account_disabled", "Your account has been disabled. Contact an administrator.")
return
}
// BUG-3382: the link verifies only for a request signed in as the
// account it names. Registration mails this link to the ADDRESS, and the
// address may belong to someone other than whoever registered it: a
// squatter's account, verified by the real owner's click, would keep the
// squatter's password and sessions. A clicker who is not signed in as
// that account is told to sign in, or, if it was not them, to claim the
// address (handleClaimByVerification). The token is not spent.
if pending != nil {
if cu := currentUser(r); cu == nil || cu.ID != pending.ID {
writeError2(w, http.StatusConflict, "verify_needs_session",
"Sign in to the account registered with this address to verify it. If you didn't register it, you can claim the address.",
map[string]interface{}{"email": pending.Email, "can_claim": !pending.IsEmailVerified()})
return
}
}
user, err := s.store.ConsumeEmailVerification(input.Token)
if errors.Is(err, store.ErrUserDisabled) {
writeError(w, http.StatusForbidden, "account_disabled", "Your account has been disabled. Contact an administrator.")
@@ -1846,6 +1895,70 @@ func (s *Server) handleVerifyEmail(w http.ResponseWriter, r *http.Request) {
})
}
// handleClaimByVerification: POST /api/v1/auth/verify-email/claim {token}.
//
// The verification link was mailed to the address, so whoever holds it owns
// the mailbox. When the account registered with that address was never
// verified, its credentials may be a squatter's (BUG-3382): the claim resets
// every one of them (store.ClaimAccountByVerification) and answers with a
// password-reset path, where the claimant sets a password and is signed in.
// The previous holder's live connections are kicked now.
func (s *Server) handleClaimByVerification(w http.ResponseWriter, r *http.Request) {
var input struct {
Token string `json:"token"`
}
if err := decodeJSON(r, &input); err != nil {
writeError(w, http.StatusBadRequest, "bad_request", "Invalid request body")
return
}
input.Token = strings.TrimSpace(input.Token)
if input.Token == "" {
writeError(w, http.StatusBadRequest, "validation_error", "Verification token is required")
return
}
claim, err := s.store.ClaimAccountByVerification(input.Token)
if errors.Is(err, store.ErrClaimNotEligible) {
writeError(w, http.StatusBadRequest, "invalid_token",
"This link is invalid or has expired, or the address is already verified. Request a new link, or sign in.")
return
}
if errors.Is(err, store.ErrClaimNeedsSupport) {
writeError(w, http.StatusConflict, "account_claim_needs_support",
"This account has billing attached, so it can't be claimed automatically. Contact support@getpad.dev and we'll sort it out.")
return
}
if err != nil {
writeInternalError(w, err)
return
}
// Every session of the account is gone; its live connections end now.
s.invalidateUserAccess(claim.UserID)
s.logAuditEventForUser(models.ActionAccountClaimed, r, claim.UserID, auditMeta(map[string]string{
"stripped_workspaces": strconv.Itoa(len(claim.StrippedWorkspaces)),
"deleted_workspaces": strconv.Itoa(len(claim.DeletedWorkspaces)),
}))
resetToken, err := s.store.CreatePasswordReset(claim.UserID)
if err != nil {
// The account is claimed and safe; the claimant can still use
// forgot-password to set a password.
slog.Error("account claim: could not mint the set-password link", "error", err, "user_id", claim.UserID)
writeJSON(w, http.StatusOK, map[string]interface{}{
"claimed": true,
"reset_path": "",
"stripped_workspaces": nonNilStrings(claim.StrippedWorkspaces),
"deleted_workspaces": nonNilStrings(claim.DeletedWorkspaces),
})
return
}
writeJSON(w, http.StatusOK, map[string]interface{}{
"claimed": true,
"reset_path": "/reset-password/" + resetToken,
"stripped_workspaces": nonNilStrings(claim.StrippedWorkspaces),
"deleted_workspaces": nonNilStrings(claim.DeletedWorkspaces),
})
}
// handleResendVerification re-sends an email-verification link for an
// UNVERIFIED account (PLAN-1933 DR-5).
//
+3 -1
View File
@@ -154,7 +154,9 @@ func (s *Server) handleApproveCLIAuthSession(w http.ResponseWriter, r *http.Requ
writeError(w, http.StatusUnauthorized, "unauthorized", "You must be logged in to approve a CLI session")
return
}
token, err := s.store.CreateSessionIssuedAt(user.ID, "cli-browser-auth", clientIP(r), "", 30*24*time.Hour, approver.CreatedAt)
// Fenced on the approver's credential epoch as this request was admitted
// (BUG-3382): a claim or reset since then makes the approval mint nothing.
token, err := s.store.CreateSessionFenced(user.ID, user.CredentialEpoch, "cli-browser-auth", clientIP(r), "", 30*24*time.Hour, approver.CreatedAt)
if errors.Is(err, store.ErrUserDisabled) {
// Disabled between this request's admission and the mint (BUG-3349).
writeError(w, http.StatusForbidden, "account_disabled", "Your account has been disabled. Contact an administrator.")
+1 -1
View File
@@ -291,7 +291,7 @@ func (s *Server) handleOAuthLogin(w http.ResponseWriter, r *http.Request) {
"email": input.Email,
"reason": "two_factor_required",
}))
challenge := generateTwoFAChallenge(user.ID, clientIP(r), s.twoFAChallengeSecret)
challenge := generateTwoFAChallengeAt(user.ID, clientIP(r), user.CredentialEpoch, s.twoFAChallengeSecret)
writeError2(w, http.StatusForbidden, "two_factor_required",
"This account has two-factor authentication turned on. Sign in on the web to finish with your 2FA code.",
map[string]interface{}{"challenge_token": challenge})
+10 -4
View File
@@ -181,8 +181,9 @@ func TestCloudSignup_UnverifiedThenVerifyUnblocksSameSession(t *testing.T) {
t.Fatalf("pre-verify item-create: expected 403 email_not_verified, got %d: %s", rr.Code, rr.Body.String())
}
// Verify the email.
rr = doRequest(srv, "POST", "/api/v1/auth/verify-email", map[string]string{"token": token})
// Verify the email, from the session that registered it (BUG-3382: a
// link verifies only for a request signed in as its account).
rr = doRequestWithCookie(srv, "POST", "/api/v1/auth/verify-email", map[string]string{"token": token}, reg.Token)
if rr.Code != http.StatusOK {
t.Fatalf("verify-email: expected 200, got %d: %s", rr.Code, rr.Body.String())
}
@@ -251,6 +252,10 @@ func TestResendVerification_InvalidatesPriorTokenAndIsEnumerationSafe(t *testing
if rr.Code != http.StatusCreated {
t.Fatalf("register: expected 201, got %d: %s", rr.Code, rr.Body.String())
}
var reg struct {
Token string `json:"token"`
}
parseJSON(t, rr, &reg)
firstToken := extractVerifyToken(t, waitMail(t, mails))
// Resend → always 200, and a NEW token lands in the inbox.
@@ -269,8 +274,9 @@ func TestResendVerification_InvalidatesPriorTokenAndIsEnumerationSafe(t *testing
t.Fatalf("stale first token: expected 400 (invalidated by resend), got %d: %s", rr.Code, rr.Body.String())
}
// The new token still verifies.
rr = post("/api/v1/auth/verify-email", map[string]string{"token": secondToken})
// The new token still verifies, for the account's own session (BUG-3382).
ipN++
rr = doRequestWithCookieFrom(srv, "POST", "/api/v1/auth/verify-email", map[string]string{"token": secondToken}, reg.Token, "203.0.113."+strconv.Itoa(ipN)+":1234")
if rr.Code != http.StatusOK {
t.Fatalf("new token verify: expected 200, got %d: %s", rr.Code, rr.Body.String())
}
+1
View File
@@ -75,6 +75,7 @@ var authCSRFUnconditionalExemptPaths = map[string]bool{
"/api/v1/auth/reset-password": true,
"/api/v1/auth/local-reset": true,
"/api/v1/auth/verify-email": true,
"/api/v1/auth/verify-email/claim": true, // BUG-3382: pre-auth, proven by the mailed token
"/api/v1/auth/resend-verification": true,
"/api/v1/auth/2fa/login-verify": true,
"/api/v1/auth/oauth-login": true,
+19 -5
View File
@@ -259,6 +259,8 @@ type RateLimiters struct {
AuthEmail *ipRateLimiter
// Password reset: per-IP
PasswordReset *ipRateLimiter
// Email-verification link consume and claim: per-IP (BUG-3382)
EmailVerify *ipRateLimiter
// Registration: per-IP
Register *ipRateLimiter
// OAuth login: per-IP (higher limit since pad-cloud sidecar calls this)
@@ -420,6 +422,15 @@ func NewRateLimiters() *RateLimiters {
Rate: rate.Limit(3.0 / 3600.0),
Burst: 3,
}),
// Verification link consume + claim: 10 per hour per IP, burst 10
// (BUG-3382). Separate from PasswordReset so one verification page
// load, a refresh and a claim do not spend the budget the claimant
// then needs to set a password. The token is a 256-bit secret, so
// this bounds load, not guessing.
EmailVerify: newIPRateLimiter(rateLimitConfig{
Rate: rate.Limit(10.0 / 3600.0),
Burst: 10,
}),
// Registration: 5 per hour per IP (= 5/3600 per second, burst 5)
Register: newIPRateLimiter(rateLimitConfig{
Rate: rate.Limit(5.0 / 3600.0),
@@ -589,6 +600,7 @@ func (rls *RateLimiters) Stop() {
rls.Auth,
rls.AuthEmail,
rls.PasswordReset,
rls.EmailVerify,
rls.Register,
rls.OAuthLogin,
rls.CloudAdmin,
@@ -665,12 +677,14 @@ func (s *Server) RateLimit(next http.Handler) http.Handler {
case path == "/api/v1/auth/login" || path == "/api/v1/auth/bootstrap" || path == "/api/v1/auth/2fa/login-verify":
limiter = s.rateLimiters.Auth
case path == "/api/v1/auth/forgot-password" || path == "/api/v1/auth/reset-password" || path == "/api/v1/auth/local-reset" ||
path == "/api/v1/auth/verify-email" || path == "/api/v1/auth/resend-verification":
// Email-verification endpoints (PLAN-1933 DR-5) reuse the
// PasswordReset bucket — same low-frequency, enumeration-safe
// shape as forgot/reset-password. Without an entry here they'd
// fall through to the looser default API limiter.
path == "/api/v1/auth/resend-verification":
// resend-verification (PLAN-1933 DR-5) reuses the
// PasswordReset bucket — same low-frequency, enumeration-safe,
// mail-sending shape as forgot/reset-password. Without an entry
// here it would fall through to the looser default API limiter.
limiter = s.rateLimiters.PasswordReset
case path == "/api/v1/auth/verify-email" || path == "/api/v1/auth/verify-email/claim":
limiter = s.rateLimiters.EmailVerify
case path == "/api/v1/auth/register":
limiter = s.rateLimiters.Register
case path == "/api/v1/auth/oauth-login" || path == "/api/v1/auth/oauth-link":
@@ -89,6 +89,7 @@ func verifiedEmailExemptPath(path string) bool {
switch path {
case "/api/v1/auth/logout",
"/api/v1/auth/verify-email",
"/api/v1/auth/verify-email/claim",
"/api/v1/auth/resend-verification",
"/api/v1/auth/delete-account":
return true
+3
View File
@@ -1686,6 +1686,9 @@ func (s *Server) setupRouter() {
// RequireVerifiedEmail's exempt list so an unverified user
// can reach them to clear their own unverified state.
r.Post("/verify-email", s.handleVerifyEmail)
// BUG-3382: the mailbox owner claims a never-verified
// account someone else registered with their address.
r.Post("/verify-email/claim", s.handleClaimByVerification)
r.Post("/resend-verification", s.handleResendVerification)
// Two-factor authentication
+33 -16
View File
@@ -22,12 +22,19 @@ func generateTwoFASecret() ([]byte, error) {
return b, nil
}
// generateTwoFAChallenge creates a short-lived, HMAC-signed challenge token
// generateTwoFAChallengeAt creates a short-lived, HMAC-signed challenge token
// that proves the user already passed password verification. The token is
// bound to the user ID and client IP, and expires after 5 minutes.
func generateTwoFAChallenge(userID, clientIP string, secret []byte) string {
// bound to the user ID and client IP, and expires after 5 minutes. It also
// carries the account's credential_epoch as of the FIRST factor's check
// (BUG-3382), so the session minted when the second
// factor completes is fenced on the epoch the password (or provider) was
// checked under, not the one read minutes later. A negative epoch omits it.
func generateTwoFAChallengeAt(userID, clientIP string, epoch int64, secret []byte) string {
expires := time.Now().UTC().Add(twoFAChallengeExpiry).Unix()
payload := fmt.Sprintf("%s|%s|%d", userID, clientIP, expires)
if epoch >= 0 {
payload += fmt.Sprintf("|%d", epoch)
}
encoded := base64.RawURLEncoding.EncodeToString([]byte(payload))
mac := hmac.New(sha256.New, secret)
@@ -37,12 +44,14 @@ func generateTwoFAChallenge(userID, clientIP string, secret []byte) string {
return encoded + "." + sig
}
// validateTwoFAChallenge verifies a challenge token's signature, expiry, and
// IP binding. Returns the user ID if valid, or an error describing the failure.
func validateTwoFAChallenge(token, clientIP string, secret []byte) (string, error) {
// validateTwoFAChallengeEpoch verifies a challenge token's signature, expiry,
// and IP binding. Returns the user ID and the credential epoch the challenge
// was issued under, or -1 for a token that carries none (issued before
// BUG-3382; they expire within minutes), or an error describing the failure.
func validateTwoFAChallengeEpoch(token, clientIP string, secret []byte) (string, int64, error) {
parts := strings.SplitN(token, ".", 2)
if len(parts) != 2 {
return "", fmt.Errorf("malformed challenge token")
return "", -1, fmt.Errorf("malformed challenge token")
}
encoded, sig := parts[0], parts[1]
@@ -52,19 +61,19 @@ func validateTwoFAChallenge(token, clientIP string, secret []byte) (string, erro
mac.Write([]byte(encoded))
expectedSig := base64.RawURLEncoding.EncodeToString(mac.Sum(nil))
if !hmac.Equal([]byte(sig), []byte(expectedSig)) {
return "", fmt.Errorf("invalid challenge signature")
return "", -1, fmt.Errorf("invalid challenge signature")
}
// Decode payload
payloadBytes, err := base64.RawURLEncoding.DecodeString(encoded)
if err != nil {
return "", fmt.Errorf("invalid challenge encoding")
return "", -1, fmt.Errorf("invalid challenge encoding")
}
payload := string(payloadBytes)
fields := strings.SplitN(payload, "|", 3)
if len(fields) != 3 {
return "", fmt.Errorf("invalid challenge payload")
fields := strings.SplitN(payload, "|", 4)
if len(fields) != 3 && len(fields) != 4 {
return "", -1, fmt.Errorf("invalid challenge payload")
}
userID := fields[0]
@@ -74,16 +83,24 @@ func validateTwoFAChallenge(token, clientIP string, secret []byte) (string, erro
// Check expiry
expiryUnix, err := strconv.ParseInt(expiryStr, 10, 64)
if err != nil {
return "", fmt.Errorf("invalid challenge expiry")
return "", -1, fmt.Errorf("invalid challenge expiry")
}
if time.Now().UTC().Unix() > expiryUnix {
return "", fmt.Errorf("challenge token expired")
return "", -1, fmt.Errorf("challenge token expired")
}
// Check IP binding
if tokenIP != clientIP {
return "", fmt.Errorf("challenge IP mismatch")
return "", -1, fmt.Errorf("challenge IP mismatch")
}
return userID, nil
epoch := int64(-1)
if len(fields) == 4 {
e, err := strconv.ParseInt(fields[3], 10, 64)
if err != nil || e < 0 {
return "", -1, fmt.Errorf("invalid challenge epoch")
}
epoch = e
}
return userID, epoch, nil
}
+179
View File
@@ -0,0 +1,179 @@
package store
import (
"crypto/rand"
"crypto/sha256"
"database/sql"
"encoding/hex"
"errors"
"fmt"
"golang.org/x/crypto/bcrypt"
)
// ErrClaimNotEligible: the token is invalid, spent or expired, or its account
// is already verified, disabled or gone. Nothing was changed.
var ErrClaimNotEligible = errors.New("account claim: not eligible")
// ErrClaimNeedsSupport: the account has a billing customer, which a claim
// must not transfer or cancel silently (TASK-3351 ruling (4)). Nothing was
// changed; the token is still unspent.
var ErrClaimNeedsSupport = errors.New("account claim: needs support")
// AccountClaim reports what a claim changed.
type AccountClaim struct {
UserID string
// Epoch is the account's credential_epoch after the claim, for minting
// the claimant's first session.
Epoch int64
// StrippedWorkspaces names the workspaces the account was a member of but
// did not own, from which its membership, grants, watches and tabs were
// removed.
StrippedWorkspaces []string
// DeletedWorkspaces names the workspaces the account owned, now
// soft-deleted: they were made by whoever held the account before.
DeletedWorkspaces []string
}
// ClaimAccountByVerification hands a never-verified account to whoever holds
// its mailbox, proven by the email-verification token (BUG-3382). The token
// was mailed to the address, so the clicker owns the mailbox; the account's
// current credentials may belong to someone who registered the address
// first (a squatter). One transaction:
//
// 1. spend the token (same first step, and lock order, as
// ConsumeEmailVerification);
// 2. lock the account and require it unverified, live and with no billing
// customer;
// 3. reset every credential: an unusable password, no 2FA, every session,
// PAT, OAuth grant, CLI handoff, reset and verification token gone, and
// credential_epoch bumped so no sign-in checked before this can mint;
// 4. mark the address verified;
// 5. remove the account from workspaces it does not own (membership,
// grants, watches, tabs), and soft-delete the ones it owns (TASK-3351
// ruling (3)): what the previous holder joined or built is not the
// mailbox owner's.
//
// The caller then gives the claimant a way to set a password and kicks the
// account's live connections.
func (s *Store) ClaimAccountByVerification(token string) (*AccountClaim, error) {
sum := sha256.Sum256([]byte(token))
tokenHash := hex.EncodeToString(sum[:])
// An unusable password: a random secret nobody keeps.
raw := make([]byte, 32)
if _, err := rand.Read(raw); err != nil {
return nil, fmt.Errorf("account claim: random password: %w", err)
}
unusable, err := bcrypt.GenerateFromPassword([]byte(hex.EncodeToString(raw)), bcryptCost)
if err != nil {
return nil, fmt.Errorf("account claim: hash password: %w", err)
}
tx, err := s.db.Begin()
if err != nil {
return nil, fmt.Errorf("account claim: begin: %w", err)
}
defer func() { _ = tx.Rollback() }()
ts := now()
var userID string
err = tx.QueryRow(s.q(`
UPDATE email_verification_tokens SET used_at = ?
WHERE token_hash = ? AND used_at IS NULL AND expires_at > ?
RETURNING user_id`), ts, tokenHash, ts).Scan(&userID)
if errors.Is(err, sql.ErrNoRows) {
return nil, ErrClaimNotEligible
}
if err != nil {
return nil, fmt.Errorf("account claim: spend token: %w", err)
}
lockQ := `SELECT COALESCE(email_verified_at, ''), CASE WHEN disabled_at IS NULL THEN 0 ELSE 1 END, COALESCE(stripe_customer_id, '') FROM users WHERE id = ?`
if s.dialect.Driver() == DriverPostgres {
lockQ += ` FOR NO KEY UPDATE`
}
var verifiedAt, customer string
var disabled int
err = tx.QueryRow(s.q(lockQ), userID).Scan(&verifiedAt, &disabled, &customer)
if errors.Is(err, sql.ErrNoRows) {
return nil, ErrClaimNotEligible
}
if err != nil {
return nil, fmt.Errorf("account claim: lock user: %w", err)
}
if verifiedAt != "" || disabled == 1 {
return nil, ErrClaimNotEligible
}
if customer != "" {
return nil, ErrClaimNeedsSupport
}
claim := &AccountClaim{UserID: userID}
names := func(query string) ([]string, error) {
rows, err := tx.Query(s.q(query), userID)
if err != nil {
return nil, err
}
defer rows.Close()
var out []string
for rows.Next() {
var n string
if err := rows.Scan(&n); err != nil {
return nil, err
}
out = append(out, n)
}
return out, rows.Err()
}
if claim.StrippedWorkspaces, err = names(`
SELECT w.name FROM workspace_members m JOIN workspaces w ON w.id = m.workspace_id
WHERE m.user_id = ? AND w.owner_id <> m.user_id AND w.deleted_at IS NULL ORDER BY w.name`); err != nil {
return nil, fmt.Errorf("account claim: list joined workspaces: %w", err)
}
if claim.DeletedWorkspaces, err = names(`
SELECT name FROM workspaces WHERE owner_id = ? AND deleted_at IS NULL ORDER BY name`); err != nil {
return nil, fmt.Errorf("account claim: list owned workspaces: %w", err)
}
notOwned := `(SELECT id FROM workspaces WHERE owner_id <> ?)`
stmts := []struct {
what, query string
args []any
}{
{"reset credentials", `UPDATE users SET password_hash = ?, password_set = ?, totp_secret = '', totp_enabled = ?, recovery_codes = '',
email_verified_at = ?, updated_at = ?, credential_epoch = credential_epoch + 1 WHERE id = ?`,
[]any{string(unusable), s.dialect.BoolToInt(false), s.dialect.BoolToInt(false), ts, ts, userID}},
{"delete sessions", `DELETE FROM sessions WHERE user_id = ?`, []any{userID}},
{"delete api tokens", `DELETE FROM api_tokens WHERE user_id = ?`, []any{userID}},
{"revoke oauth access tokens", `UPDATE oauth_access_tokens SET active = ? WHERE subject = ?`, []any{s.dialect.BoolToInt(false), userID}},
{"revoke oauth refresh tokens", `UPDATE oauth_refresh_tokens SET active = ? WHERE subject = ?`, []any{s.dialect.BoolToInt(false), userID}},
{"revoke oauth authorization codes", `UPDATE oauth_authorization_codes SET active = ? WHERE request_id IN (SELECT request_id FROM oauth_connections WHERE user_id = ?)`, []any{s.dialect.BoolToInt(false), userID}},
{"delete oauth pkce requests", `DELETE FROM oauth_pkce_requests WHERE request_id IN (SELECT request_id FROM oauth_connections WHERE user_id = ?)`, []any{userID}},
{"delete oauth connections", `DELETE FROM oauth_connections WHERE user_id = ?`, []any{userID}},
{"delete cli handoffs", `DELETE FROM cli_auth_sessions WHERE user_id = ?`, []any{userID}},
{"delete reset tokens", `DELETE FROM password_reset_tokens WHERE user_id = ?`, []any{userID}},
{"delete verification tokens", `DELETE FROM email_verification_tokens WHERE user_id = ?`, []any{userID}},
{"delete grants elsewhere (collections)", `DELETE FROM collection_grants WHERE user_id = ? AND workspace_id IN ` + notOwned, []any{userID, userID}},
{"delete grants elsewhere (items)", `DELETE FROM item_grants WHERE user_id = ? AND workspace_id IN ` + notOwned, []any{userID, userID}},
{"delete watches elsewhere", `DELETE FROM watches WHERE user_id = ? AND workspace_id IN ` + notOwned, []any{userID, userID}},
{"delete tabs elsewhere", `DELETE FROM user_workspace_tabs WHERE user_id = ? AND workspace_id IN ` + notOwned, []any{userID, userID}},
{"delete memberships elsewhere", `DELETE FROM workspace_members WHERE user_id = ? AND workspace_id IN ` + notOwned, []any{userID, userID}},
// Owned workspaces go as account deletion takes them: soft-deleted,
// with every holder's tab on them (users.go deleteAccountAtomic).
{"delete tabs of owned workspaces", `DELETE FROM user_workspace_tabs WHERE workspace_id IN (SELECT id FROM workspaces WHERE owner_id = ? AND deleted_at IS NULL)`, []any{userID}},
{"soft-delete owned workspaces", `UPDATE workspaces SET deleted_at = ?, updated_at = ? WHERE owner_id = ? AND deleted_at IS NULL`, []any{ts, ts, userID}},
}
for _, st := range stmts {
if _, err := tx.Exec(s.q(st.query), st.args...); err != nil {
return nil, fmt.Errorf("account claim: %s: %w", st.what, err)
}
}
if err := tx.QueryRow(s.q(`SELECT credential_epoch FROM users WHERE id = ?`), userID).Scan(&claim.Epoch); err != nil {
return nil, fmt.Errorf("account claim: read epoch: %w", err)
}
if err := tx.Commit(); err != nil {
return nil, fmt.Errorf("account claim: commit: %w", err)
}
return claim, nil
}
@@ -0,0 +1,107 @@
package store
import (
"errors"
"testing"
"github.com/PerpetualSoftware/pad/internal/models"
)
// BUG-3382: a password write is fenced on the credential_epoch its authority
// was checked under, so one that started before a claim cannot overwrite the
// claim's reset afterwards.
func TestBUG3382_ResetSpentBeforeAClaimWritesNothingAfterIt(t *testing.T) {
s := testStore(t)
u := createUnverifiedUser(t, s, "squat@example.com")
resetTok, err := s.CreatePasswordReset(u.ID)
if err != nil {
t.Fatal(err)
}
spent, err := s.ConsumePasswordReset(resetTok)
if err != nil || spent == nil {
t.Fatalf("consume reset: %v %v", spent, err)
}
// The mailbox owner claims between the reset's spend and its write.
vTok, err := s.CreateEmailVerification(u.ID)
if err != nil {
t.Fatal(err)
}
if _, err := s.ClaimAccountByVerification(vTok); err != nil {
t.Fatalf("claim: %v", err)
}
pwd := "squatters-new-password-1"
_, err = s.UpdateUser(u.ID, models.UserUpdate{Password: &pwd, ExpectedEpoch: &spent.CredentialEpoch})
if !errors.Is(err, ErrCredentialsChanged) {
t.Fatalf("stale reset write: got %v, want ErrCredentialsChanged", err)
}
if got, _ := s.ValidatePassword(u.Email, pwd); got != nil {
t.Fatal("the stale reset's password was stored over the claim")
}
}
func TestBUG3382_FencedPasswordWriteReturnsItsOwnEpoch(t *testing.T) {
s := testStore(t)
u := createUnverifiedUser(t, s, "own@example.com")
resetTok, err := s.CreatePasswordReset(u.ID)
if err != nil {
t.Fatal(err)
}
spent, err := s.ConsumePasswordReset(resetTok)
if err != nil || spent == nil {
t.Fatalf("consume reset: %v %v", spent, err)
}
pwd := "a-fresh-password-2"
updated, err := s.UpdateUser(u.ID, models.UserUpdate{Password: &pwd, ExpectedEpoch: &spent.CredentialEpoch})
if err != nil {
t.Fatalf("fenced write: %v", err)
}
if updated.CredentialEpoch != spent.CredentialEpoch+1 {
t.Fatalf("epoch = %d, want %d", updated.CredentialEpoch, spent.CredentialEpoch+1)
}
if got, _ := s.ValidatePassword(u.Email, pwd); got == nil {
t.Fatal("the fenced write did not store the password")
}
}
// The claim lands INSIDE the spend, after the token is marked used and before
// the user is read back: the epoch the reset is fenced on must be the one from
// the spend, not the claim's.
func TestBUG3382_ClaimInsideTheResetSpendStillFencesIt(t *testing.T) {
s := testStore(t)
u := createUnverifiedUser(t, s, "inside@example.com")
resetTok, err := s.CreatePasswordReset(u.ID)
if err != nil {
t.Fatal(err)
}
vTok, err := s.CreateEmailVerification(u.ID)
if err != nil {
t.Fatal(err)
}
claimed := false
passwordResetSpentHook = func(string) {
if _, err := s.ClaimAccountByVerification(vTok); err != nil {
t.Errorf("claim: %v", err)
return
}
claimed = true
}
t.Cleanup(func() { passwordResetSpentHook = nil })
spent, err := s.ConsumePasswordReset(resetTok)
if err != nil || spent == nil {
t.Fatalf("consume reset: %v %v", spent, err)
}
if !claimed {
t.Fatal("control: the claim did not run inside the spend")
}
pwd := "squatters-new-password-3"
_, err = s.UpdateUser(u.ID, models.UserUpdate{Password: &pwd, ExpectedEpoch: &spent.CredentialEpoch})
if !errors.Is(err, ErrCredentialsChanged) {
t.Fatalf("reset spent before the claim: got %v, want ErrCredentialsChanged", err)
}
}
@@ -0,0 +1,11 @@
-- Migration 110: a per-account credential epoch (BUG-3382, TASK-3351).
--
-- Every sign-in checks a credential (a password, a provider assertion, a 2FA
-- code, an approving session) and THEN mints a session, in a separate step. A
-- credential change landing between the two (a reset, a change, a disable, an
-- account claim) used to be invisible to the mint: CreateSession checked only
-- disabled_at, so a login that validated the OLD password could still mint a
-- session just after every session was deleted. The epoch is bumped by every
-- credential change; a mint carries the epoch it read when it checked the
-- credential, and inserts only if it is unchanged.
ALTER TABLE users ADD COLUMN credential_epoch INTEGER NOT NULL DEFAULT 0;
+23 -2
View File
@@ -82,13 +82,22 @@ func (s *Store) ConsumePasswordReset(token string) (*models.User, error) {
// Atomically mark the token as used and return it, only if it's
// currently unused and not expired. The WHERE clause ensures only
// one concurrent caller can succeed.
//
// The account's credential_epoch is read in the SAME statement
// (BUG-3382): the password write that follows is fenced on it, and a
// claim that committed after this token was spent must refuse that
// write. A later read could return the claim's epoch instead. The token
// row this statement locks is one the claim deletes, so a claim in
// flight either commits before this statement (the token is gone) or
// after it (its epoch bump is not seen).
var userID string
var epoch int64
err := s.db.QueryRow(s.q(`
UPDATE password_reset_tokens
SET used_at = ?
WHERE token_hash = ? AND used_at IS NULL AND expires_at > ?
RETURNING user_id
`), now(), tokenHash, now()).Scan(&userID)
RETURNING user_id, (SELECT credential_epoch FROM users WHERE users.id = password_reset_tokens.user_id)
`), now(), tokenHash, now()).Scan(&userID, &epoch)
if err == sql.ErrNoRows {
return nil, nil // Invalid, expired, or already used
@@ -97,15 +106,27 @@ func (s *Store) ConsumePasswordReset(token string) (*models.User, error) {
return nil, fmt.Errorf("consume reset token: %w", err)
}
if passwordResetSpentHook != nil {
passwordResetSpentHook(userID)
}
// Fetch the user
user, err := s.GetUser(userID)
if err != nil {
return nil, fmt.Errorf("get user: %w", err)
}
if user != nil {
user.CredentialEpoch = epoch
}
return user, nil
}
// passwordResetSpentHook, when set by a test, runs after the reset token is
// spent and before the user is read back, which is where a claim can land
// (BUG-3382).
var passwordResetSpentHook func(userID string)
// CleanExpiredPasswordResets removes old reset tokens.
func (s *Store) CleanExpiredPasswordResets() error {
_, err := s.db.Exec(s.q(`
@@ -0,0 +1,3 @@
-- Migration 084: a per-account credential epoch (BUG-3382, TASK-3351).
-- Mirrors SQLite migration 110; see it for the rationale.
ALTER TABLE users ADD COLUMN IF NOT EXISTS credential_epoch BIGINT NOT NULL DEFAULT 0;
+35
View File
@@ -64,6 +64,29 @@ func (s *Store) CreateSession(userID, deviceInfo, ipAddress, userAgent string, t
// measured from created_at. The expiry is capped at that lifetime too. A
// zero issuedAt means a new sign-in, now.
func (s *Store) CreateSessionIssuedAt(userID, deviceInfo, ipAddress, userAgent string, ttl time.Duration, issuedAt time.Time) (string, error) {
return s.createSession(userID, -1, deviceInfo, ipAddress, userAgent, ttl, issuedAt)
}
// ErrCredentialsChanged refuses a session mint whose credential check is
// stale: the account's credential_epoch moved between the check and the mint
// (a password reset or change, a disable, an account claim). The caller
// answers as for a failed sign-in (BUG-3382).
var ErrCredentialsChanged = errors.New("credentials changed since the sign-in was checked")
// CreateSessionFenced is CreateSessionIssuedAt for a sign-in: it mints only if
// the account's credential_epoch still equals epoch, the value read when the
// credential was checked (models.User.CredentialEpoch on the user the check
// loaded), else ErrCredentialsChanged. Every production sign-in uses it
// (BUG-3382); CreateSession / CreateSessionIssuedAt carry no fence and are
// for tests and tooling.
func (s *Store) CreateSessionFenced(userID string, epoch int64, deviceInfo, ipAddress, userAgent string, ttl time.Duration, issuedAt time.Time) (string, error) {
if epoch < 0 {
return "", fmt.Errorf("insert session: negative credential epoch %d", epoch)
}
return s.createSession(userID, epoch, deviceInfo, ipAddress, userAgent, ttl, issuedAt)
}
func (s *Store) createSession(userID string, epoch int64, deviceInfo, ipAddress, userAgent string, ttl time.Duration, issuedAt time.Time) (string, error) {
// Generate 32 random bytes → hex → prefix
raw := make([]byte, 32)
if _, err := rand.Read(raw); err != nil {
@@ -106,6 +129,18 @@ func (s *Store) CreateSessionIssuedAt(userID, deviceInfo, ipAddress, userAgent s
}
return "", fmt.Errorf("insert session: %w", err)
}
// The epoch is read under requireActiveUserTx's lock (FOR SHARE on
// Postgres; SQLite serializes writers), which a credential change's
// UPDATE of the row conflicts with, so the two serialize.
if epoch >= 0 {
var current int64
if err := tx.QueryRow(s.q(`SELECT credential_epoch FROM users WHERE id = ?`), userID).Scan(&current); err != nil {
return "", fmt.Errorf("insert session: read credential epoch: %w", err)
}
if current != epoch {
return "", ErrCredentialsChanged
}
}
if _, err := tx.Exec(s.q(`
INSERT INTO sessions (id, user_id, token_hash, device_info, ip_address, ua_hash, expires_at, created_at, renew_ttl_seconds, kind)
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
+25 -4
View File
@@ -30,7 +30,7 @@ var usernameCleanRe = regexp.MustCompile(`[^a-z0-9-]+`)
var bcryptCost = 12
// user SELECT columns — used by all user queries.
const userColumns = `id, email, username, name, password_hash, role, avatar_url, totp_secret, totp_enabled, recovery_codes, plan, plan_expires_at, plan_source, stripe_customer_id, plan_overrides, oauth_providers, password_set, disabled_at, email_verified_at, last_active_at, last_write_at, created_at, updated_at`
const userColumns = `id, email, username, name, password_hash, role, avatar_url, totp_secret, totp_enabled, recovery_codes, plan, plan_expires_at, plan_source, stripe_customer_id, plan_overrides, oauth_providers, password_set, disabled_at, email_verified_at, last_active_at, last_write_at, created_at, updated_at, credential_epoch`
// scanUser scans a user row into a User struct.
// Note: does NOT decrypt the TOTP secret — call store.decryptUserTOTP() after
@@ -46,6 +46,7 @@ func scanUser(row interface{ Scan(...interface{}) error }) (*models.User, error)
&u.Plan, &u.PlanExpiresAt, &u.PlanSource, &u.StripeCustomerID, &u.PlanOverrides, &u.OAuthProviders,
&u.PasswordSet,
&disabledAt, &emailVerifiedAt, &lastActiveAt, &lastWriteAt, &createdAt, &updatedAt,
&u.CredentialEpoch,
)
if disabledAt.Valid {
u.DisabledAt = disabledAt.String
@@ -190,6 +191,9 @@ func (s *Store) UpdateUser(id string, input models.UserUpdate) (*models.User, er
// (clears the OAuth placeholder-hash state set by CreateOAuthUser).
sets = append(sets, "password_set = ?")
args = append(args, true)
// A new password is a credential change: a sign-in that checked the
// old one must not mint a session after it (BUG-3382).
sets = append(sets, "credential_epoch = credential_epoch + 1")
}
if input.AvatarURL != nil {
sets = append(sets, "avatar_url = ?")
@@ -205,16 +209,32 @@ func (s *Store) UpdateUser(id string, input models.UserUpdate) (*models.User, er
args = append(args, id)
query := fmt.Sprintf("UPDATE users SET %s WHERE id = ?", strings.Join(sets, ", "))
fenced := input.Password != nil && input.ExpectedEpoch != nil
if fenced {
query += " AND credential_epoch = ?"
args = append(args, *input.ExpectedEpoch)
}
result, err := s.db.Exec(s.q(query), args...)
if err != nil {
return nil, fmt.Errorf("update user: %w", err)
}
n, _ := result.RowsAffected()
if n == 0 {
if fenced {
if u, err := s.GetUser(id); err == nil && u != nil {
return nil, ErrCredentialsChanged
}
}
return nil, sql.ErrNoRows
}
return s.GetUser(id)
u, err := s.GetUser(id)
if err == nil && u != nil && fenced {
// The epoch THIS write produced, not whatever a later change left:
// a session minted on it must not survive a change after this one.
u.CredentialEpoch = *input.ExpectedEpoch + 1
}
return u, err
}
// ValidatePassword checks an email/password combination. Returns the user
@@ -495,6 +515,7 @@ func (s *Store) SearchUsers(params AdminUserSearchParams) (*AdminUserSearchResul
&entry.Plan, &entry.PlanExpiresAt, &entry.PlanSource, &entry.StripeCustomerID, &entry.PlanOverrides, &entry.OAuthProviders,
&entry.PasswordSet,
&disabledAt, &emailVerifiedAt, &lastActiveAt, &lastWriteAt, &createdAt, &updatedAt,
&entry.CredentialEpoch,
&workspaceCount, &storageBytes,
); err != nil {
return nil, fmt.Errorf("search users scan: %w", err)
@@ -915,7 +936,7 @@ func throttleTime(ts string) string {
// DisableUser soft-disables a user account by setting disabled_at.
func (s *Store) DisableUser(userID string) error {
_, err := s.db.Exec(s.q(`UPDATE users SET disabled_at = ?, updated_at = ? WHERE id = ?`),
_, err := s.db.Exec(s.q(`UPDATE users SET disabled_at = ?, updated_at = ?, credential_epoch = credential_epoch + 1 WHERE id = ?`),
now(), now(), userID)
if err != nil {
return fmt.Errorf("disable user: %w", err)
@@ -979,7 +1000,7 @@ func (s *Store) DisableUserAndRevokeAccess(userID string) error {
what, query string
args []any
}{
{"disable", `UPDATE users SET disabled_at = ?, updated_at = ? WHERE id = ?`, []any{ts, ts, userID}},
{"disable", `UPDATE users SET disabled_at = ?, updated_at = ?, credential_epoch = credential_epoch + 1 WHERE id = ?`, []any{ts, ts, userID}},
{"delete sessions", `DELETE FROM sessions WHERE user_id = ?`, []any{userID}},
{"delete api tokens", `DELETE FROM api_tokens WHERE user_id = ?`, []any{userID}},
{"revoke oauth access tokens", `UPDATE oauth_access_tokens SET active = ? WHERE subject = ?`, []any{s.dialect.BoolToInt(false), userID}},
+13 -1
View File
@@ -2747,12 +2747,24 @@ export const api = {
// The token in the link is the credential, so this is a bare POST with
// no form. Returns the freshly-verified user so the SPA can refresh its
// auth store without a second round-trip; 400 invalid_token when the
// link is invalid, expired, or already used.
// link is invalid, expired, or already used; 409 verify_needs_session
// (details {email, can_claim}) when the request has no session for the
// account the link names (BUG-3382).
verifyEmailToken: (token: string) =>
request<{ ok: boolean; user: { id: string; email: string; username: string; name: string; role: string } }>('/auth/verify-email', {
method: 'POST',
body: JSON.stringify({ token })
}),
// Claim the address the verification link was mailed to (BUG-3382).
// Offered when the link is opened without a session for its account
// (409 verify_needs_session with details.can_claim). Resets every
// credential on the account and answers with a set-password path;
// 409 account_claim_needs_support when billing is attached.
claimByVerification: (token: string) =>
request<{ claimed: boolean; reset_path: string; stripped_workspaces: string[]; deleted_workspaces: string[] }>('/auth/verify-email/claim', {
method: 'POST',
body: JSON.stringify({ token })
}),
me: () => request<User>('/auth/me'),
updateProfile: (data: UserProfileUpdate) =>
request<User>('/auth/me', {
@@ -2,17 +2,28 @@
import { onMount } from 'svelte';
import { page } from '$app/state';
import { goto } from '$app/navigation';
import { api } from '$lib/api/client';
import { api, PadApiError } from '$lib/api/client';
import { authStore } from '$lib/stores/auth.svelte';
import AuthHeader from '$lib/components/auth/AuthHeader.svelte';
import AuthFooter from '$lib/components/auth/AuthFooter.svelte';
let token = $derived(page.params.token ?? '');
// verifying → success (terminal, redirects) | error (terminal, offers resend).
let status = $state<'verifying' | 'success' | 'error'>('verifying');
// verifying → success (terminal, redirects) | error (terminal, offers resend)
// | needs-session (BUG-3382: the link was opened without a session for the
// account it names; offers sign-in, or a claim when it wasn't them).
let status = $state<'verifying' | 'success' | 'error' | 'needs-session'>('verifying');
let error = $state('');
// needs-session details, from the 409's details.
let pendingEmail = $state('');
let canClaim = $state(false);
// idle → confirming → claiming → claimed (terminal) | failed (retryable).
let claimState = $state<'idle' | 'confirming' | 'claiming' | 'claimed' | 'failed'>('idle');
let claimError = $state('');
let claimed = $state<{ reset_path: string; stripped_workspaces: string[]; deleted_workspaces: string[] } | null>(null);
let signInHref = $derived(`/login?redirect=${encodeURIComponent(`/verify-email/${token}`)}`);
// Mirrors VerifyEmailBanner's resend flow: idle → sending → sent (terminal) ;
// error is retryable.
let resendState = $state<'idle' | 'sending' | 'sent' | 'error'>('idle');
@@ -44,6 +55,12 @@
status = 'success';
await goto('/console', { replaceState: true });
} catch (err: unknown) {
if (err instanceof PadApiError && err.code === 'verify_needs_session') {
pendingEmail = typeof err.details?.email === 'string' ? err.details.email : '';
canClaim = err.details?.can_claim === true;
status = 'needs-session';
return;
}
if (err instanceof Error) {
error = err.message || 'This verification link is invalid or has expired.';
} else {
@@ -53,6 +70,42 @@
}
}
// Signed in as a DIFFERENT account: /login would bounce straight back
// here still signed in as it, so sign out first (the CLI-approval page's
// switch-account pattern).
let switching = $state(false);
let switchError = $state('');
async function switchAccount() {
if (switching) return;
switching = true;
switchError = '';
try {
await api.auth.logout();
} catch (err: unknown) {
switching = false;
switchError =
err instanceof Error && err.message
? `Couldn't sign you out: ${err.message}`
: "Couldn't sign you out. Please try again.";
return;
}
await goto(signInHref, { replaceState: true });
}
async function claim() {
if (claimState === 'claiming') return;
claimState = 'claiming';
claimError = '';
try {
claimed = await api.auth.claimByVerification(token);
claimState = 'claimed';
} catch (err: unknown) {
claimError =
err instanceof Error && err.message ? err.message : 'Could not claim this address. Try again.';
claimState = 'failed';
}
}
async function resend() {
const email = authStore.user?.email;
if (!email || resendState === 'sending') return;
@@ -87,6 +140,71 @@
<div class="status" role="status">
<span>Email verified — redirecting…</span>
</div>
{:else if status === 'needs-session'}
{#if claimState === 'claimed' && claimed}
<p class="subtitle">Address claimed</p>
<div class="form claim">
<p>
Every password, session and token on this account has been reset. Set a password to
sign in.
</p>
{#if claimed.deleted_workspaces.length > 0}
<p>Workspaces created by whoever registered this address were deleted:</p>
<ul>
{#each claimed.deleted_workspaces as name, i (i)}<li>{name}</li>{/each}
</ul>
{/if}
{#if claimed.stripped_workspaces.length > 0}
<p>The account was removed from workspaces it had joined:</p>
<ul>
{#each claimed.stripped_workspaces as name, i (i)}<li>{name}</li>{/each}
</ul>
{/if}
{#if claimed.reset_path}
<a class="button-link" href={claimed.reset_path}>Set a password</a>
{:else}
<a class="button-link" href="/forgot-password">Set a password</a>
{/if}
</div>
{:else}
<p class="subtitle">Sign in to verify</p>
<div class="form claim">
<p>
This link verifies {pendingEmail || 'this address'}. Sign in to that account to finish
verifying.
</p>
{#if authStore.user}
<p>You're signed in as {authStore.user.email}.</p>
{#if switchError}
<p class="error" role="alert">{switchError}</p>
{/if}
<button onclick={switchAccount} disabled={switching}>
{switching ? 'Signing out…' : 'Switch account to verify'}
</button>
{:else}
<a class="button-link" href={signInHref}>Sign in to verify</a>
{/if}
{#if canClaim}
{#if claimState === 'idle'}
<button class="secondary" onclick={() => (claimState = 'confirming')}>
I didn't register this account
</button>
{:else}
<p>
If someone else registered your address, you can claim it. This resets every
password, session and token on the account, deletes workspaces it created, and
removes it from workspaces it joined. You then set your own password.
</p>
{#if claimState === 'failed'}
<p class="error" role="alert">{claimError}</p>
{/if}
<button onclick={claim} disabled={claimState === 'claiming'}>
{claimState === 'claiming' ? 'Claiming…' : 'Claim this address'}
</button>
{/if}
{/if}
</div>
{/if}
{:else}
<p class="subtitle">This verification link is invalid or expired.</p>
<div class="form">
@@ -220,6 +338,35 @@
button:hover:not(:disabled) { opacity: 0.9; }
button:disabled { opacity: 0.6; cursor: not-allowed; }
.claim {
text-align: left;
color: var(--text-secondary);
font-size: 0.85rem;
}
.claim ul {
margin: 0;
padding-left: var(--space-5);
}
.button-link {
display: block;
text-align: center;
padding: var(--space-3) var(--space-4);
background: var(--accent-blue);
color: #fff;
border-radius: var(--radius);
font-size: 0.95rem;
font-weight: 500;
text-decoration: none;
}
button.secondary {
background: transparent;
color: var(--accent-blue);
border: 1px solid var(--border);
}
.signin-link {
display: inline-block;
color: var(--accent-blue);
@@ -0,0 +1,152 @@
// BUG-3382: a verification link opened without a session for the account it
// names does not verify. The page offers sign-in (back to this link), and,
// when the address is unverified, a claim that resets the account and lists
// what the claim removed.
import { describe, it, expect, vi, afterEach, beforeEach } from 'vitest';
import { cleanup, fireEvent, render, screen } from '@testing-library/svelte';
const calls = vi.hoisted(() => ({ verify: [] as string[], claim: [] as string[], logout: 0, goto: [] as string[] }));
const signedIn = vi.hoisted(() => ({ user: null as null | { email: string } }));
const next = vi.hoisted(() => ({ verify: null as unknown, claim: null as unknown }));
vi.mock('$lib/api/client', () => {
class PadApiError extends Error {
code: string;
details?: Record<string, unknown>;
constructor(o: { code: string; message: string; details?: Record<string, unknown> }) {
super(o.message);
this.code = o.code;
this.details = o.details;
}
}
const settle = (v: unknown) => (v instanceof Error ? Promise.reject(v) : Promise.resolve(v));
return {
PadApiError,
api: {
auth: {
verifyEmailToken: vi.fn(async (t: string) => {
calls.verify.push(t);
return settle(next.verify);
}),
claimByVerification: vi.fn(async (t: string) => {
calls.claim.push(t);
return settle(next.claim);
}),
resendVerification: vi.fn(async () => ({})),
logout: vi.fn(async () => {
calls.logout++;
}),
},
},
};
});
vi.mock('$lib/stores/auth.svelte', () => ({
authStore: {
cloudMode: true,
get user() {
return signedIn.user;
},
ensureLoaded: async () => {},
load: async () => {},
},
}));
vi.mock('$app/navigation', () => ({
goto: vi.fn(async (url: string) => {
calls.goto.push(url);
}),
}));
import { page } from '$app/state';
import { PadApiError } from '$lib/api/client';
import VerifyPage from './+page.svelte';
async function settle() {
for (let i = 0; i < 20; i++) await Promise.resolve();
}
const needsSession = (canClaim: boolean) =>
new (PadApiError as unknown as new (o: object) => Error)({
code: 'verify_needs_session',
message: 'Sign in',
details: { email: 'victim@example.com', can_claim: canClaim },
});
beforeEach(() => {
calls.verify.length = 0;
calls.claim.length = 0;
calls.logout = 0;
calls.goto.length = 0;
signedIn.user = null;
(page as { params: Record<string, string> }).params = { token: 'tok123' };
});
afterEach(() => cleanup());
describe('verify link without the account’s session (BUG-3382)', () => {
it('offers sign-in back to this link, and a confirmed claim', async () => {
next.verify = needsSession(true);
next.claim = {
claimed: true,
reset_path: '/reset-password/rst',
stripped_workspaces: ['Joined WS'],
deleted_workspaces: ['Squat WS'],
};
render(VerifyPage);
await settle();
expect(screen.getByText(/victim@example\.com/)).toBeTruthy();
const signIn = screen.getByRole('link', { name: 'Sign in to verify' });
expect(signIn.getAttribute('href')).toBe('/login?redirect=%2Fverify-email%2Ftok123');
// The first click only explains; nothing is claimed without the second.
await fireEvent.click(screen.getByRole('button', { name: "I didn't register this account" }));
await settle();
expect(calls.claim).toEqual([]);
await fireEvent.click(screen.getByRole('button', { name: 'Claim this address' }));
await settle();
expect(calls.claim).toEqual(['tok123']);
expect(screen.getByText('Squat WS')).toBeTruthy();
expect(screen.getByText('Joined WS')).toBeTruthy();
expect(screen.getByRole('link', { name: 'Set a password' }).getAttribute('href')).toBe(
'/reset-password/rst',
);
});
it('offers no claim for an already-verified address', async () => {
next.verify = needsSession(false);
render(VerifyPage);
await settle();
expect(screen.getByRole('link', { name: 'Sign in to verify' })).toBeTruthy();
expect(screen.queryByRole('button', { name: "I didn't register this account" })).toBeNull();
});
it('shows a refused claim and lets it be retried', async () => {
next.verify = needsSession(true);
next.claim = new (PadApiError as unknown as new (o: object) => Error)({
code: 'account_claim_needs_support',
message: 'Contact support@getpad.dev',
});
render(VerifyPage);
await settle();
await fireEvent.click(screen.getByRole('button', { name: "I didn't register this account" }));
await fireEvent.click(screen.getByRole('button', { name: 'Claim this address' }));
await settle();
expect(screen.getByRole('alert').textContent).toContain('support@getpad.dev');
expect(screen.getByRole('button', { name: 'Claim this address' })).toBeTruthy();
});
it('signed in as another account: signs out before sending to sign-in', async () => {
signedIn.user = { email: 'other@example.com' };
next.verify = needsSession(true);
render(VerifyPage);
await settle();
expect(screen.getByText(/signed in as other@example\.com/)).toBeTruthy();
// A plain link to /login would bounce back here still signed in.
expect(screen.queryByRole('link', { name: 'Sign in to verify' })).toBeNull();
await fireEvent.click(screen.getByRole('button', { name: 'Switch account to verify' }));
await settle();
expect(calls.logout).toBe(1);
expect(calls.goto).toEqual(['/login?redirect=%2Fverify-email%2Ftok123']);
});
});