Feature | UserController MFA Integration, Device Trust Cookie Management, Audit Wiring, and 2FA Rate Limiting

[matiasperrone-exo]

Task:

Ref: https://app.clickup.com/t/9014802374/86ba2zc6p

What Changed

UserController (app/Http/Controllers/UserController.php)

• Injected IDeviceTrustService, ITwoFactorAuditService, and ITwoFactorGateService via constructor.
• Password flow now validates credentials without creating a session first. If MFAGateService::requiresChallenge() returns true, issues a challenge (via
  AuthService::issueMFAChallenge) and returns { error_code: "mfa_required", ... } — no session created yet.
• Added resolveClientFromMemento() private helper to extract the OAuth2 client from a pending session memento (deduplicates logic previously repeated in password and OTP flows).
• Added postVerify2FA(), postResend2FA(), and postRecovery2FA() action methods to handle the full MFA step-up login lifecycle.
• Applied MFACookieManager trait for reading/writing the trusted-device cookie.

MFACookieManager trait (app/Http/Controllers/Traits/MFACookieManager.php) — new file

• Provides getCookieToken(): ?string (reads raw token from cookie).
• Provides queueDeviceTrustCookie(User $user): void (calls IDeviceTrustService::trustDevice and queues a secure, HttpOnly cookie).
• Contains no Doctrine/repository logic — cookie transport only.

TwoFactorRateLimitMiddleware (app/Http/Middleware/TwoFactorRateLimitMiddleware.php) — new file

• Throttles verify, recovery, and resend MFA endpoints.
• Counters stored in cache (not session) with a fixed-window TTL.
• verify / recovery: increments only on failed response (mfa_verification_failed, mfa_invalid_recovery).
• resend: increments on every request.
• Returns HTTP 429 with error_code: mfa_rate_limit when the limit is exceeded.
• Registered in app/Http/Kernel.php and wired to the new routes in routes/web.php.

AuthService (app/libs/Auth/AuthService.php) and IAuthService (app/libs/Utils/Services/IAuthService.php)

• Added validateCredentials(string $username, string $password): User — validates without creating a session.
• Added loginUser(User $user, bool $remember): void — establishes the session for an already-validated user.
• Added issueMFAChallenge(User $user, MFAChallengeStrategy $strategy, ?Client $client, bool $remember): array — triggers the challenge strategy and stores pending state.

DeviceTrustService and TwoFactorAuditService (minor fixes)

• Resolved wiring/dependency issues surfaced during integration testing.

config/two_factor.php — new file

• Centralizes MFA configuration: cookie_name, device_trust_lifetime_days, rate-limit thresholds per action.

EncryptCookies middleware

• Excluded the device_trust_token cookie from encryption (raw HMAC token must be read as-is by IDeviceTrustService).

---

Tests

File	What it covers
tests/TwoFactorLoginFlowTest.php (new)	End-to-end: password→MFA challenge→verify→session; device trust skip; rate limiting; recovery codes; resend.
tests/DeviceTrustServiceTest.php	Expanded with cookie integration assertions.
tests/Unit/TwoFactorAuditServiceTest.php	Expanded with new event types.
tests/Unit/MFA/EmailOTPMFAChallengeStrategyTest.php	Added cases for issueMFAChallenge path.
phpunit.xml	Registered Two Factor Authentication Test Suite.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 527311e1-cc70-41e0-9160-4c78d58b01b4

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/mfa---usercontroller-mfa-integration-device-trust-cookie-management-audit-wiring-and-2fa-rate-limiting---cu-86ba2zc6p

---

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 and usage tips.}

[github-actions]

📘 OpenAPI / Swagger preview

➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-136/

This page is automatically updated on each push to this PR.

[github-actions]

📘 OpenAPI / Swagger preview

➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-136/

This page is automatically updated on each push to this PR.

[github-actions]

📘 OpenAPI / Swagger preview

➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-136/

This page is automatically updated on each push to this PR.

[github-actions]

📘 OpenAPI / Swagger preview

➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-136/

This page is automatically updated on each push to this PR.

[caseylocker]

Reviewed against ClickUp 86ba2zc6p and SDS idp-mfa.md (§4.4 controller flow, §4.5 strategies, §4.9.1 AuthService MFA methods).

The transaction-context design rule is correctly implemented — the controller orchestrates only, all strategy DB-mutating calls go through AuthService::*MFA* wrappers, and no add(entity, true) self-flush remains. Every acceptance criterion is met on the happy path, and verifyChallenge now correctly loads the persisted OTP and compares its value.

However, five issues should be addressed before merge. All five are consequences of work first made live in this PR (the MFA strategies and loginUser() were previously unwired/dead code), so they are in scope here rather than the base PR. The first is a critical regression in the shared password-login path. Inline comments below.

Also, a few test gaps versus the ticket's TESTS list: concurrent OTP redeem, concurrent recovery-code redeem, client-bound MFA verification, recovery rate-limit blocking, and rollback/partial-failure after a successful redeem.

[github-actions]

📘 OpenAPI / Swagger preview

➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-136/

This page is automatically updated on each push to this PR.

[smarcet]

@matiasperrone-exo review

[github-actions]

📘 OpenAPI / Swagger preview

➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-136/

This page is automatically updated on each push to this PR.

[github-actions]

📘 OpenAPI / Swagger preview

➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-136/

This page is automatically updated on each push to this PR.

[github-actions]

📘 OpenAPI / Swagger preview

➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-136/

This page is automatically updated on each push to this PR.

[caseylocker]

LGTM