fix(service): verify-totp keeps the verified second factor when the session insert fails (OC-0378)

VerifyTOTP consumed the partial challenge before issueSession, so a store
fault on the session insert discarded a verified second factor and sent the
user back to the password step; the code was also marked used, so an
immediate retry would have been refused as a replay.

The claim stays atomic and first (two concurrent verifies can never both
reach issueSession). On issueSession failure the challenge is restored under
the same partial token — the client still holds it — and the accepted code
is released, so the retry completes the login without another password
step. auth gains PartialAuthStore.Restore and UsedTOTPCodeStore.Unmark, each
tested in the leaf package.

Characterization row flipped in the same commit: `VerifyTOTPFailurePaths/
session insert fails -> 500, the challenge and the code survive` — once the
trigger is dropped the same token and the same code answer 200 with a token.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A17Uq3d2C36rN82Jitf3wo
This commit is contained in:
J3vb
2026-08-30 10:31:38 +02:00
co-authored by Claude Fable 5
parent f70158096a
commit be37d7ee2d
4 changed files with 121 additions and 8 deletions
+19 -6
View File
@@ -824,18 +824,31 @@ func TestAuthCharacterization_VerifyTOTPFailurePaths(t *testing.T) {
}
wantErr(t, verify(t, router, pt, totpCode(t, secret), ""), http.StatusInternalServerError, "INTERNAL_ERROR", "failed to verify two-factor code")
})
t.Run("session insert fails -> 500 and the challenge is consumed", func(t *testing.T) {
t.Run("session insert fails -> 500, the challenge and the code survive", func(t *testing.T) {
database, router, _, secret := setup(t)
pt := loginPartial(t, router, "two", "correctPass1", "", "")
failWrite(t, database, "INSERT", "sessions")
wantErr(t, verify(t, router, pt, totpCode(t, secret), ""), http.StatusInternalServerError, "INTERNAL_ERROR", "failed to create session")
// known: the partial token was consumed before the session insert, so
// the user must repeat the password step after a transient store
// failure (ledger OC-0378). Pinned as-is; the code is also marked used.
code := totpCode(t, secret)
wantErr(t, verify(t, router, pt, code, ""), http.StatusInternalServerError, "INTERNAL_ERROR", "failed to create session")
// OC-0378 (fixed in B3-9): the verified second factor is not discarded
// by a store fault — the challenge is restored under the same partial
// token and the accepted code is released, so once the store is back
// the same token and the same code complete the login. (Restore alone
// would refuse the retry as a replay.)
if _, err := database.ExecContext(context.Background(), `DROP TRIGGER fault_insert_sessions`); err != nil {
t.Fatalf("drop trigger: %v", err)
}
wantErr(t, verify(t, router, pt, totpCode(t, secret), "203.0.113.2"), http.StatusUnauthorized, "UNAUTHORIZED", challengeGone)
rr := verify(t, router, pt, code, "203.0.113.2")
if rr.Code != http.StatusOK {
t.Fatalf("retry status = %d, want 200; body = %s", rr.Code, rr.Body.String())
}
var res struct {
Token string `json:"token"`
Requires2FA bool `json:"requires_2fa"`
}
if err := json.Unmarshal(rr.Body.Bytes(), &res); err != nil || res.Token == "" || res.Requires2FA {
t.Fatalf("retry body = %s (err %v), want a session token", rr.Body.String(), err)
}
})
t.Run("per-user failure cap spans challenges -> 429 on the 11th attempt", func(t *testing.T) {
_, router, _, secret := setup(t)
+19
View File
@@ -108,6 +108,16 @@ func (s *PartialAuthStore) Consume(token string) (PartialAuthChallenge, bool) {
return entry, true
}
// Restore puts a challenge Consume returned back under its original token —
// the recovery path for a caller that claimed the challenge and then could
// not finish the login (OC-0378). The entry keeps its expiry and failure
// count, so a challenge that expired meanwhile is dropped by the next Lookup.
func (s *PartialAuthStore) Restore(token string, challenge PartialAuthChallenge) {
s.mu.Lock()
defer s.mu.Unlock()
s.entries[token] = challenge
}
func (s *PartialAuthStore) RegisterFailure(token string, maxFailures int) bool {
s.mu.Lock()
defer s.mu.Unlock()
@@ -198,6 +208,15 @@ func (s *UsedTOTPCodeStore) MarkUsed(userID int64, code string) bool {
return true
}
// Unmark forgets a code MarkUsed recorded so it can be accepted once more —
// the companion of PartialAuthStore.Restore: a verification the caller could
// not complete is released together with its challenge.
func (s *UsedTOTPCodeStore) Unmark(userID int64, code string) {
s.mu.Lock()
defer s.mu.Unlock()
delete(s.entries, fmt.Sprintf("%d:%s", userID, code))
}
func (s *UsedTOTPCodeStore) cleanupExpiredLocked() {
now := time.Now()
for key, expiry := range s.entries {
+70
View File
@@ -376,3 +376,73 @@ func TestBuildTOTPURI_ContainsIssuerAndSecret(t *testing.T) {
t.Fatalf("issuer = %q, want OwnCord", query.Get("issuer"))
}
}
// ─── OC-0378: Restore / Unmark ──────────────────────────────────────────────
func TestPartialAuthStore_RestoreAfterConsume(t *testing.T) {
store := auth.NewPartialAuthStore(time.Minute)
token, err := store.Issue(7, "device", "203.0.113.7")
if err != nil {
t.Fatalf("Issue: %v", err)
}
store.RegisterFailure(token, 5) // Failures=1 must survive the round trip
claimed, ok := store.Consume(token)
if !ok {
t.Fatal("Consume: challenge not found")
}
if _, ok := store.Lookup(token); ok {
t.Fatal("consumed token still resolves")
}
store.Restore(token, claimed)
got, ok := store.Lookup(token)
if !ok {
t.Fatal("restored token does not resolve")
}
if got != claimed {
t.Fatalf("restored challenge = %+v, want %+v", got, claimed)
}
if got.Failures != 1 {
t.Fatalf("Failures = %d, want 1 (restore keeps the count)", got.Failures)
}
}
func TestPartialAuthStore_RestoreExpiredStaysGone(t *testing.T) {
store := auth.NewPartialAuthStore(20 * time.Millisecond)
token, err := store.Issue(7, "device", "203.0.113.7")
if err != nil {
t.Fatalf("Issue: %v", err)
}
claimed, ok := store.Consume(token)
if !ok {
t.Fatal("Consume: challenge not found")
}
time.Sleep(40 * time.Millisecond)
store.Restore(token, claimed)
if _, ok := store.Lookup(token); ok {
t.Fatal("an expired challenge came back to life")
}
}
func TestUsedTOTPCodeStore_UnmarkAllowsReuse(t *testing.T) {
store := auth.NewUsedTOTPCodeStore()
if !store.MarkUsed(1, "123456") {
t.Fatal("first MarkUsed refused")
}
if store.MarkUsed(1, "123456") {
t.Fatal("replay accepted before Unmark")
}
if !store.MarkUsed(2, "123456") {
t.Fatal("another user's identical code refused")
}
store.Unmark(1, "123456")
if !store.MarkUsed(1, "123456") {
t.Fatal("code still marked after Unmark")
}
if store.MarkUsed(2, "123456") {
t.Fatal("Unmark for user 1 released user 2's code")
}
}
+13 -2
View File
@@ -470,6 +470,7 @@ func (s *AuthService) authenticate(ctx context.Context, in LoginInput) (*db.User
// VerifyTOTP completes a challenge Login started and issues the session,
// bound to the login request's device and IP rather than this one's.
func (s *AuthService) VerifyTOTP(ctx context.Context, partialToken, code string) (*AuthResult, error) {
code = strings.TrimSpace(code)
challenge, ok := s.partial.Lookup(partialToken)
if !ok {
return nil, ErrTOTPChallengeInvalid
@@ -500,7 +501,7 @@ func (s *AuthService) VerifyTOTP(ctx context.Context, partialToken, code string)
return nil, ErrTooManyAttempts
}
if !auth.VerifyTOTPCodeOnce(secret, strings.TrimSpace(code), time.Now().UTC(), user.ID, s.usedCodes) {
if !auth.VerifyTOTPCodeOnce(secret, code, time.Now().UTC(), user.ID, s.usedCodes) {
// The attempt was already recorded atomically up-front via
// limiter.Allow; only the per-partial-token counter is advanced here.
s.partial.RegisterFailure(partialToken, partialAuthMaxFailures)
@@ -509,12 +510,22 @@ func (s *AuthService) VerifyTOTP(ctx context.Context, partialToken, code string)
s.limiter.Reset(ctx, totpRateLimitKey)
if _, ok := s.partial.Consume(partialToken); !ok {
claimed, ok := s.partial.Consume(partialToken)
if !ok {
return nil, ErrTOTPChallengeInvalid
}
token, err := issueSession(ctx, s.st, user.ID, challenge.Device, challenge.IP)
if err != nil {
// The second factor was verified; a store fault must not discard it
// (OC-0378). The claim stays atomic and first — two concurrent
// verifies can never both reach issueSession — so on failure put the
// challenge back under the same token (the client still holds it) and
// release the accepted code: the retry completes the login without
// another password step. Code first, then token, so a concurrent
// retry never finds a live token with a dead code.
s.usedCodes.Unmark(user.ID, code)
s.partial.Restore(partialToken, claimed)
return nil, ErrSessionIssue
}