cheapnud/CheapHelpers
Watch
1
0
Fork
You've already forked CheapHelpers
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Harden sign-in flow: antiforgery, TOTP challenge, generic 2FA pages #7

Merged
cheapnud merged 2 commits from d/auth-hardening into master 2026-08-17 12:29:00 +00:00
Conversation 1 Commits 2 Files changed 8 +233 -310
cheapnud commented 2026-08-17 12:23:08 +00:00
Owner
Copy link

Sign-in POSTs now validate antiforgery tokens and the shipped login forms supply them. Adds the SignInWith2fa TOTP challenge action and reworks the 2FA pages to SSR form posts so auth cookies are set reliably and derived user types resolve.

Sign-in POSTs now validate antiforgery tokens and the shipped login forms supply them. Adds the SignInWith2fa TOTP challenge action and reworks the 2FA pages to SSR form posts so auth cookies are set reliably and derived user types resolve.
antiforgery on sign-in posts, totp challenge action, genericize 2fa pages
All checks were successful
Claude PR Review / AI Code Review (pull_request) Successful in 17s
Details
Build / Build and Test (pull_request) Successful in 2m26s
Details
bd9cdf71b6
forgejo-actions commented 2026-08-17 12:23:25 +00:00
Copy link

🤖 Claude AI Code Review

  1. Summary
    Hardens the sign-in flow with antiforgery validation, adds a dedicated SignInWith2fa action, and reworks the 2FA/recovery pages to SSR form posts so auth cookies set reliably across derived user types.

  2. Code Quality ⭐⭐⭐⭐☆
    Solid architectural fix — the SSR-post-to-controller approach correctly addresses the broken cookie-setting on interactive circuits, and the non-generic IAccountIdentityService bridge is a clean solution for derived TUser.

  3. Issues

🟡 AccountController.cs — SignInWith2fa uses isPersistent: true while the original interactive flow used model.RememberMachine for persistence. This is documented as intentional (matching SignIn), but note it silently changes 2FA login to always-persistent regardless of user choice. Confirm this is desired product behavior.

🟢 LoginWith2fa.razor / LoginWithRecoveryCode.razor — the removed pages previously validated the two-factor session in OnInitializedAsync and redirected to login if absent. Now the pages render unconditionally; the session check only happens on POST. Acceptable (controller redirects), but users landing directly without a 2FA session will see a form that just bounces to login on submit.

  1. Verdict: APPROVE

The antiforgery hardening, null-guarding of form fields, and SSR rework are all correct and improve security. The persistence-behavior change is worth a quick confirmation but isn't a blocker.

📊 Tokens: 14575 input + 529 output | 💰 Cost: ~$0.0861
Generated by Claude Opus 4.8 via Gitea Actions

## 🤖 Claude AI Code Review 1. **Summary** Hardens the sign-in flow with antiforgery validation, adds a dedicated `SignInWith2fa` action, and reworks the 2FA/recovery pages to SSR form posts so auth cookies set reliably across derived user types. 2. **Code Quality** ⭐⭐⭐⭐☆ Solid architectural fix — the SSR-post-to-controller approach correctly addresses the broken cookie-setting on interactive circuits, and the non-generic `IAccountIdentityService` bridge is a clean solution for derived `TUser`. 3. **Issues** 🟡 AccountController.cs — `SignInWith2fa` uses `isPersistent: true` while the original interactive flow used `model.RememberMachine` for persistence. This is documented as intentional (matching `SignIn`), but note it silently changes 2FA login to always-persistent regardless of user choice. Confirm this is desired product behavior. 🟢 LoginWith2fa.razor / LoginWithRecoveryCode.razor — the removed pages previously validated the two-factor session in `OnInitializedAsync` and redirected to login if absent. Now the pages render unconditionally; the session check only happens on POST. Acceptable (controller redirects), but users landing directly without a 2FA session will see a form that just bounces to login on submit. 4. **Verdict**: **APPROVE** The antiforgery hardening, null-guarding of form fields, and SSR rework are all correct and improve security. The persistence-behavior change is worth a quick confirmation but isn't a blocker. --- 📊 **Tokens**: 14575 input + 529 output | 💰 **Cost**: ~$0.0861 *Generated by Claude Opus 4.8 via Gitea Actions*
guard form fields against malformed posts, clarify persistence comment
All checks were successful
Claude PR Review / AI Code Review (pull_request) Successful in 17s
Details
Build / Build and Test (pull_request) Successful in 2m21s
Details
5ef7c5bb3c
cheapnud merged commit af57c2697f into master 2026-08-17 12:29:00 +00:00
cheapnud deleted branch d/auth-hardening 2026-08-17 12:29:00 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
cheapnud/CheapHelpers!7
No description provided.