Improve sign-in process (#12790)

[yasmoradi]

closes #12790

Summary by CodeRabbit

• Bug Fixes
  • Expired or incomplete one-time passwords are now rejected reliably.
  • User creation requests without an email address or phone number are rejected.
  • Email and phone confirmation sign-ins now use the correct authentication flow.
  • Failed two-factor attempts are tracked consistently and may trigger account lockout.
  • Refresh-token handling now applies stricter rotation and access validation rules.
  • Privileged-session limits correctly support unlimited access settings.

[coderabbitai]

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 356811ba-58ba-4450-b36b-4e5494d0d809

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

Walkthrough

The identity flow now validates sign-in inputs, handles missing OTP timestamps, uses automatic sign-in links for confirmations, centralizes two-factor lockout handling, and applies stricter elevated-session refresh validation.

Changes

Identity authentication

Layer / File(s)	Summary
Sign-in entry and confirmation src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Features/Identity/...	Sign-in creation validates email or phone input. OTPs without request timestamps expire. Email and phone confirmation use GenerateAutomaticSignInLink.
Two-factor validation and lockout src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Features/Identity/IdentityController.cs	Two-factor verification checks recovery, phone, and authenticator codes. Failed attempts can return UserLockedOutException.
Privileged refresh controls src/Templates/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Features/Identity/IdentityController.cs	Refresh rotation checks timing. Elevated access validation applies lockout and failure tracking, resets successful failures, preserves unlimited limits, and renews session expiry.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant IdentityController
  participant PhoneProvider
  participant AuthenticatorProvider
  IdentityController->>PhoneProvider: Validate phone token
  PhoneProvider-->>IdentityController: Return validation result
  IdentityController->>AuthenticatorProvider: Validate authenticator token
  AuthenticatorProvider-->>IdentityController: Return validation result
  IdentityController->>IdentityController: Record failure or return lockout

Loading

Poem

I’m a rabbit checking codes at night,
OTP timestamps must be right.
Tokens guide the sign-in way,
Lockouts guard the gate today.
Refresh sessions hop in line—
Secure and tidy, carrot-fine.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 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 sign-in improvements implemented by the pull request.
Linked Issues check	✅ Passed	The changes directly address sign-in improvements described in linked issue #12790, including OTP, confirmation, lockout, and token handling.
Out of Scope Changes check	✅ Passed	All changes are within the identity sign-in flow and support the objectives of linked issue #12790.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Features/Identity/IdentityController.cs (1)

145-155: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Good consolidation; reuse the new helper in the confirmation flows too.

Centralizing lockout handling into UserLockedOutException and FailedTwoFactorSignIn is a correct improvement. Two follow-ups worth considering, both enabled by this change:

• IdentityController.EmailConfirmation.cs (lines 36-40) and IdentityController.PhoneConfirmation.cs (lines 38-42) still build the lockout message inline using (TimeProvider.GetUtcNow() - user.LockoutEnd!).Value.Humanize(...). That expression is the negative of tryAgainIn, while their WithExtensionData("TryAgainIn", tryAgainIn) uses the positive value. UserLockedOutException computes both consistently from the positive tryAgainIn. Since both files are partial classes of IdentityController, they can call the shared private helper directly and drop the duplicated, inconsistent logic.
• OtpSignIn in SignInManagerExtensions.cs (lines 65-69) still returns SignInResult.Failed after AccessFailedAsync without checking IsLockedOutAsync, unlike the new FailedTwoFactorSignIn. If a failed OTP attempt is the one that crosses the lockout threshold, the caller reports a generic invalid-credentials error instead of the lockout message until the next request.

Neither point is required by this PR, but both are low-effort follow-ups that remove duplication and fix a real display bug.

Also applies to: 172-217

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Features/Identity/IdentityController.cs`
around lines 145 - 155, Update the email and phone confirmation flows in
IdentityController to call the shared UserLockedOutException helper instead of
constructing lockout messages and tryAgainIn values inline. Update OtpSignIn in
SignInManagerExtensions to check IsLockedOutAsync after AccessFailedAsync and
return the same FailedTwoFactorSignIn lockout result when the failed OTP attempt
triggers lockout; preserve the existing failed-sign-in result otherwise.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Features/Identity/IdentityController.cs`:
- Around line 316-321: Update the refresh-token reuse logic in
IdentityController to reject stale tokens based on the presented iat claim
relative to the session’s allowed rotation state, rather than requiring both the
issued-at gap and 30 seconds since lastRotatedOn. Remove the post-rotation
time-window dependency while preserving acceptance of the current legitimate
refresh token.

---

Nitpick comments:
In
`@src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Features/Identity/IdentityController.cs`:
- Around line 145-155: Update the email and phone confirmation flows in
IdentityController to call the shared UserLockedOutException helper instead of
constructing lockout messages and tryAgainIn values inline. Update OtpSignIn in
SignInManagerExtensions to check IsLockedOutAsync after AccessFailedAsync and
return the same FailedTwoFactorSignIn lockout result when the failed OTP attempt
triggers lockout; preserve the existing failed-sign-in result otherwise.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 551b3944-2e97-45c6-a780-c0402001f4c9

📥 Commits

Reviewing files that changed from the base of the PR and between 44cb1a1 and efad364.

📒 Files selected for processing (5)

• src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Features/Identity/Extensions/SignInManagerExtensions.cs
• src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Features/Identity/Extensions/UserManagerExtensions.cs
• src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Features/Identity/IdentityController.EmailConfirmation.cs
• src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Features/Identity/IdentityController.PhoneConfirmation.cs
• src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Features/Identity/IdentityController.cs