Unverified Commit 99532759 authored by Minseong Choi's avatar Minseong Choi 💬
Browse files

fix(api): bound webauthn_challenges growth by superseding all prior rows

The supersede DELETE in CreatePasskeyChallenge filtered consumed_at IS NULL, so it only reaped the prior LIVE challenge; the row that each finish stamps consumed_at on was left behind. A begin->finish loop therefore accumulated one dead row per cycle, unbounded. Drop the consumed_at clause so a fresh begin reaps ALL prior rows for (user, purpose), bounding the table at one row per (user, purpose) with zero net growth per cycle. Deleting an already-consumed row is safe: it has been redeemed and nothing reads it. The fake mirrors the widened supersede.
parent cdbb5abc
Loading
Loading
Loading
Loading
+6 −5
Changes for internal/api/api_test.go: 6 added lines, 5 removed lines.
Original line number Diff line number Diff line
@@ -285,12 +285,13 @@ func (f *fakeRepo) VerifyEmailOTP(_ context.Context, userID, purpose, codeHash s
}

// CreatePasskeyChallenge / ConsumePasskeyChallengeByUser mirror PGRepo's contract so
// the hermetic tests exercise the same semantics: a fresh begin supersedes the prior
// live challenge for (user, purpose), and the consume path redeems the newest live one
// (expiry checked before consuming), single-use.
// the hermetic tests exercise the same semantics: a fresh begin supersedes ALL prior
// rows for (user, purpose) — live, consumed, or expired — so the table holds at most one
// row per (user, purpose), and the consume path redeems the newest live one (expiry
// checked before consuming), single-use.
func (f *fakeRepo) CreatePasskeyChallenge(_ context.Context, id, userID, purpose string, sessionData []byte, expiresAt time.Time) error {
	for k, c := range f.passkeyChallenges { // supersede prior live (DELETE ... consumed_at IS NULL)
		if c.userID == userID && c.purpose == purpose && !c.consumed {
	for k, c := range f.passkeyChallenges { // supersede all prior (DELETE ... user_id=$1 AND purpose=$2)
		if c.userID == userID && c.purpose == purpose {
			delete(f.passkeyChallenges, k)
		}
	}
+8 −5
Changes for internal/api/pgrepo.go: 8 added lines, 5 removed lines.
Original line number Diff line number Diff line
@@ -846,10 +846,13 @@ func (p *PGRepo) SetSetting(ctx context.Context, key string, value []byte) error

// ---- player passkey enrollment (spec §14 WebAuthn / Phase 6 bind, migration 0007) ----

// CreatePasskeyChallenge supersedes any prior live challenge for (user, purpose) and
// inserts the fresh one, in one transaction (mirrors CreateEmailOTP). The supersede
// DELETE means a re-begin invalidates the earlier ceremony, so only the most recent
// challenge can ever finish — at most one outstanding challenge per (user, purpose).
// CreatePasskeyChallenge supersedes any prior challenge for (user, purpose) and inserts
// the fresh one, in one transaction (mirrors CreateEmailOTP). The supersede DELETE
// removes ALL prior rows for (user, purpose) — not just the live one — so a re-begin
// invalidates the earlier ceremony AND reaps any already-consumed or expired row it left
// behind. That bounds the table at one row per (user, purpose): the begin→finish loop
// nets zero growth, since each begin sweeps the consumed row the previous finish stamped.
// (Deleting a consumed row is safe: it has already been redeemed and nothing reads it.)
// The opaque SessionData is held server-side so the client cannot forge the challenge
// it must answer at finish.
func (p *PGRepo) CreatePasskeyChallenge(ctx context.Context, id, userID, purpose string, sessionData []byte, expiresAt time.Time) error {
@@ -860,7 +863,7 @@ func (p *PGRepo) CreatePasskeyChallenge(ctx context.Context, id, userID, purpose
	defer tx.Rollback() //nolint:errcheck // no-op after commit

	if _, err := tx.ExecContext(ctx,
		`DELETE FROM webauthn_challenges WHERE user_id = $1 AND purpose = $2 AND consumed_at IS NULL`,
		`DELETE FROM webauthn_challenges WHERE user_id = $1 AND purpose = $2`,
		userID, purpose); err != nil {
		return fmt.Errorf("supersede prior passkey challenge: %w", err)
	}