fix(auth): reject login when recovery-code consumption cannot persist

[cristim]

Change

Recovery-code login now returns invalid_mfa_code when persisting the consumed code fails, before creating a session or recording a successful login. Previously, each fresh database read restored the unburned hash and the same code could repeatedly authorize a session during a write failure.

Closes #228.

Verification

• Fresh-snapshot mock regression verifies repeated write failures produce no response, session, or login bookkeeping.
• Real local PostgreSQL with synthetic accounts and a scoped CHECK failure verifies repeated denial, unchanged recovery hash/bookkeeping, and zero sessions. Removing the test constraint allows one successful login and usable session; repeating the consumed code is denied.
• The retained PostgreSQL regression fails against the old service code. Independent final-SHA positive verification passed under race in 6.271s; reverting only the production service through an overlay failed in 2.011s with forbidden sessions and altered bookkeeping.
• Full short race suite, go build ./..., CI-pinned golangci-lint 2.10.1 for root and integration-tagged auth, and normal commit hooks passed.

Independent adversarial review: approved, no actionable findings, reviewed commit cd62a3b4507f18f5fc90b43bdf5b69e8e0a578cf. Two implementation passes plus final committed-SHA verification used the user-authorized Astra reviewer in place of unavailable Opus. Local evidence covers synthetic accounts on real PostgreSQL, not production services. CodeRabbit is replaced by the authorized independent local review for this SHA; required CI remains a merge gate.

Scope

The guarantee is that failed recovery-code persistence cannot authorize a login. Pre-existing concurrent consumption and stale whole-row recovery writes are separate residual risks; this change does not claim atomic consumption. The recovery-persistence test item in #263 is covered, but its other test gaps remain open.

Summary by CodeRabbit

• Bug Fixes
  • Login with a multi-factor recovery code now fails safely if the code cannot be recorded as used. Previously, login could continue despite that failure.
  • When code-use recording is unavailable, no session is created. Once recording works again, a valid unused recovery code can be used to log in; a code already used cannot be reused.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: LeanerCloud/cloud-commitments-platform/.coderabbit.yaml

Review profile: CHILL

Plan: Essentials

Run ID: 5f96aa1f-bb42-412e-856f-b686db32939f

📥 Commits

Reviewing files that changed from the base of the PR and between 6d9a70f and cd62a3b.

📒 Files selected for processing (3)

• internal/auth/service.go
• internal/auth/service_mfa_test.go
• internal/auth/service_recovery_db_test.go

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

---

📝 Walkthrough

Walkthrough

Recovery-code login now fails if the service cannot persist code consumption. Unit and integration tests cover failed persistence, successful consumption, and reuse of a consumed code.

Changes

Recovery code login

Layer / File(s)	Summary
Reject recovery codes that cannot be persisted internal/auth/service.go	The service returns ErrInvalidMFACode when it cannot persist recovery-code consumption. The method documentation describes this failure case.
Verify persistence failure and successful consumption internal/auth/service_mfa_test.go, internal/auth/service_recovery_db_test.go	Tests check that failed updates reject login without creating sessions. Integration tests also check successful consumption and rejection of a consumed code.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to cd62a

Recovery-code login now fails closed when consumption cannot be saved. The regressions cover failure and recovery behavior; the change is mergeable subject to required CI passing.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: authentication now rejects login when recovery-code consumption cannot be persisted.
Linked Issues check	✅ Passed	Issue #228 requires login denial when recovery-code consumption cannot persist. verifyPasswordAndMFA now returns ErrInvalidMFACode when UpdateUser fails. The return occurs before session creatio…
Out of Scope Changes check	✅ Passed	The changes stay within Issue #228. The service change implements the required failure behavior. The unit and integration tests verify persistence failure, recovery, session protection, and single-use…

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

_{Comment @coderabbitai help to get the list of available commands.}