Skip to content

Feat/recovery codes management#145

Closed
romanetar wants to merge 30 commits into
mainfrom
feat/recovery-codes-management
Closed

Feat/recovery codes management#145
romanetar wants to merge 30 commits into
mainfrom
feat/recovery-codes-management

Conversation

@romanetar

@romanetar romanetar commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

ref https://app.clickup.com/t/9014802374/86ba2zp66

Summary by CodeRabbit

  • New Features

    • Added two-factor authentication flows for login, including verification, resend, and recovery-code options.
    • Added trusted-device support so recognized devices can reduce repeated prompts.
    • Added profile controls to enable 2FA and regenerate recovery codes.
    • Added recovery-code viewing, copying, and download support.
  • Bug Fixes

    • Improved login, signup, and session handling for security-related prompts and redirects.
    • Added rate limiting and clearer error handling for MFA actions.
  • Documentation

    • Updated setup and testing guidance for the new authentication flows.

matiasperrone-exo and others added 30 commits June 8, 2026 19:55
feat(2fa): add migration for 2FA schema foundation (Phase I)
feat(2fa): add UserTrustedDevice entity
feat(2fa): add TwoFactorAuditLog entity
feat(2fa): add UserRecoveryCode entity
feat(2fa): add repository interfaces for 2FA entities
feat(2fa): add Doctrine implementations for 2FA repositories
feat(2fa): register 2FA repositories in service container
test(2fa): add repository round-trip tests for 2FA entities
- test: cover canLogin()=false branch in validateCredentials() unit tests
- docs: document known double-query cost in validateCredentials()
- fix: use consistent error message in validateCredentials()
Add tests changes with suggestion
Signed-off-by: romanetar <roman_ag@hotmail.com>
Signed-off-by: romanetar <roman_ag@hotmail.com>
Signed-off-by: romanetar <roman_ag@hotmail.com>
Comment out login-mfa-flow.spec.ts and register.spec.ts so CI runs
login.spec.ts alone to verify it now passes without account lockout
interference. Also fix the MFA beforeEach mock: fulfill() must run
before unroute(), otherwise Playwright auto-resolves the in-flight
route on unroute and the later fulfill() throws "Route is already
handled" - which was letting the real POST through with a wrong
password and locking out test@test.com.
login.spec.ts verified green in isolation; re-enable the MFA flow
suite (route-ordering fix already applied) and the registration
suite now that the account-lockout cascade is gone.

Signed-off-by: romanetar <roman_ag@hotmail.com>
Signed-off-by: romanetar <roman_ag@hotmail.com>
@romanetar romanetar closed this Jul 8, 2026
@coderabbitai

coderabbitai Bot commented Jul 8, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5d4be514-3b26-4142-9868-ff34e22168da

📥 Commits

Reviewing files that changed from the base of the PR and between 24c00ca and 15aee43.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (120)
  • .github/workflows/pull_request_frontend_tests.yml
  • .github/workflows/pull_request_unit_tests.yml
  • .github/workflows/push.yml
  • .github/workflows/push_frontend_tests.yml
  • .gitignore
  • app/Console/Commands/CreateRawUser.php
  • app/Console/Kernel.php
  • app/Http/Controllers/Api/UserApiController.php
  • app/Http/Controllers/Auth/RegisterController.php
  • app/Http/Controllers/Traits/MFACookieManager.php
  • app/Http/Controllers/UserController.php
  • app/Http/Kernel.php
  • app/Http/Middleware/EncryptCookies.php
  • app/Http/Middleware/TwoFactorRateLimitMiddleware.php
  • app/Providers/AppServiceProvider.php
  • app/Repositories/DoctrineTwoFactorAuditLogRepository.php
  • app/Repositories/DoctrineUserRecoveryCodeRepository.php
  • app/Repositories/DoctrineUserTrustedDeviceRepository.php
  • app/Repositories/RepositoriesProvider.php
  • app/Services/Auth/DeviceTrustService.php
  • app/Services/Auth/IDeviceTrustService.php
  • app/Services/Auth/IRecoveryCodeService.php
  • app/Services/Auth/ITwoFactorAuditService.php
  • app/Services/Auth/ITwoFactorGateService.php
  • app/Services/Auth/MFAGateService.php
  • app/Services/Auth/RecoveryCodeService.php
  • app/Services/Auth/TwoFactorAuditService.php
  • app/Services/Auth/TwoFactorServiceProvider.php
  • app/Strategies/MFA/AbstractMFAChallengeStrategy.php
  • app/Strategies/MFA/EmailOTPMFAChallengeStrategy.php
  • app/Strategies/MFA/IMFAChallengeStrategy.php
  • app/Strategies/MFA/MFAChallengeStrategyFactory.php
  • app/libs/Auth/AuthService.php
  • app/libs/Auth/Models/TwoFactorAuditLog.php
  • app/libs/Auth/Models/User.php
  • app/libs/Auth/Models/UserRecoveryCode.php
  • app/libs/Auth/Models/UserTrustedDevice.php
  • app/libs/Auth/Repositories/ITwoFactorAuditLogRepository.php
  • app/libs/Auth/Repositories/IUserRecoveryCodeRepository.php
  • app/libs/Auth/Repositories/IUserTrustedDeviceRepository.php
  • app/libs/Utils/Services/IAuthService.php
  • babel.config.js
  • config/app.php
  • config/auth.php
  • config/session.php
  • config/two_factor.php
  • database/migrations/Version20260416194357.php
  • doc/adrs/0001-recovery-code-generation-not-triggered-by-group-enforcement.md
  • doc/mfa-test-gap-report.md
  • docker-compose.yml
  • jest.config.js
  • package.json
  • phpunit.xml
  • playwright.config.ts
  • readme.md
  • resources/js/base_actions.js
  • resources/js/components/recovery_code_display.js
  • resources/js/components/recovery_code_modal.js
  • resources/js/components/recovery_codes.module.scss
  • resources/js/components/recovery_codes_panel.js
  • resources/js/components/two_factor_section.js
  • resources/js/login/actions.js
  • resources/js/login/components/email_error_actions.js
  • resources/js/login/components/email_input_form.js
  • resources/js/login/components/existing_account_actions.js
  • resources/js/login/components/help_links.js
  • resources/js/login/components/otp_help_links.js
  • resources/js/login/components/otp_input_form.js
  • resources/js/login/components/password_input_form.js
  • resources/js/login/components/recovery_code_form.js
  • resources/js/login/components/third_party_identity_providers.js
  • resources/js/login/components/two_factor_form.js
  • resources/js/login/constants.js
  • resources/js/login/login.js
  • resources/js/login/login.module.scss
  • resources/js/profile/actions.js
  • resources/js/profile/profile.js
  • resources/js/profile/profile.module.scss
  • resources/js/shared/HTMLRender.js
  • resources/js/signup/signup.js
  • resources/js/utils.js
  • resources/views/auth/login.blade.php
  • resources/views/profile.blade.php
  • routes/web.php
  • start_local_server.sh
  • storage/framework/cache/data/.gitignore
  • tests/AuthServiceValidateCredentialsIntegrationTest.php
  • tests/DeviceTrustServiceTest.php
  • tests/RecoveryCodeRegenerationTest.php
  • tests/TurnstileProtectedControllersTest.php
  • tests/TwoFactorLoginFlowTest.php
  • tests/TwoFactorRepositoriesTest.php
  • tests/e2e/fixtures/index.ts
  • tests/e2e/pages/LoginPage.ts
  • tests/e2e/pages/RegisterPage.ts
  • tests/e2e/tests/auth/login-mfa-flow.spec.ts
  • tests/e2e/tests/auth/login.spec.ts
  • tests/e2e/tsconfig.json
  • tests/js/__mocks__/fileMock.js
  • tests/js/components/Banner.test.js
  • tests/js/components/CustomSnackbar.test.js
  • tests/js/components/DividerWithText.test.js
  • tests/js/components/recovery_code_display.test.js
  • tests/js/components/recovery_code_modal.test.js
  • tests/js/components/recovery_codes_panel.test.js
  • tests/js/components/two_factor_section.test.js
  • tests/js/login/components/two-factor-form.test.js
  • tests/js/login/login.mfa.test.js
  • tests/js/setup.js
  • tests/js/validator/validator.test.js
  • tests/unit/AuthServiceValidateCredentialsTest.php
  • tests/unit/DisqusSSOProfileMappingTest.php
  • tests/unit/MFA/AbstractMFAChallengeStrategyTest.php
  • tests/unit/MFA/EmailOTPMFAChallengeStrategyTest.php
  • tests/unit/MFA/MFAChallengeStrategyFactoryTest.php
  • tests/unit/MFAGateServiceTest.php
  • tests/unit/OAuth2LoginStrategyTest.php
  • tests/unit/TwoFactorAuditServiceTest.php
  • tests/unit/UserTwoFactorTest.php
  • webpack.common.js

📝 Walkthrough

Walkthrough

Introduces a full two-factor authentication (MFA) feature: new database entities/migration for trusted devices, audit logs, and recovery codes; backend services and strategies for OTP challenge issuance/verification; controller/route wiring with rate limiting and cookie management; frontend login and profile UI; new Jest/Playwright test infrastructure and CI workflows; plus unrelated Turnstile/SSL/session config fixes.

Changes

Two-Factor Authentication Feature

Layer / File(s) Summary
2FA data model, repositories, migration and config
app/libs/Auth/Models/User.php, TwoFactorAuditLog.php, UserRecoveryCode.php, UserTrustedDevice.php, app/libs/Auth/Repositories/*, app/Repositories/Doctrine*Repository.php, RepositoriesProvider.php, config/two_factor.php, config/auth.php, database/migrations/Version20260416194357.php, config/app.php, doc/adrs/...
Adds 2FA fields to User, new TwoFactorAuditLog/UserRecoveryCode/UserTrustedDevice entities and repositories, a migration creating the supporting tables, 2FA/rate-limit config, and service provider registration.
Device trust, recovery code, and audit services
app/Services/Auth/*
Adds DeviceTrustService, RecoveryCodeService, TwoFactorAuditService, MFAGateService and interfaces bound via TwoFactorServiceProvider.
MFA challenge strategy abstraction
app/Strategies/MFA/*
Adds pending-state/recovery-code handling in AbstractMFAChallengeStrategy, email OTP issuance/verification in EmailOTPMFAChallengeStrategy, and a factory selecting strategies by method.
AuthService MFA wrapper methods
app/libs/Auth/AuthService.php, app/libs/Utils/Services/IAuthService.php
Adds validateCredentials, loginUser, and transactional MFA issue/verify/resend wrapper methods.
Login controller MFA gating, cookie trait, rate limiting, routes
app/Http/Controllers/UserController.php, MFACookieManager.php, EncryptCookies.php, TwoFactorRateLimitMiddleware.php, Http/Kernel.php, routes/web.php, resources/views/auth/login.blade.php
Gates postLogin behind an MFA challenge, adds verify2FA/verify2FARecovery/resend2FA endpoints, trusted-device cookie handling, and rate-limited 2FA routes.
Recovery code/2FA enrollment API
app/Http/Controllers/Api/UserApiController.php, routes/web.php, resources/views/profile.blade.php
Adds enableTwoFactor/regenerateRecoveryCodes admin API endpoints and profile view config/endpoints.
Login page MFA/OTP/recovery UI
resources/js/login/*, resources/js/shared/HTMLRender.js
Refactors login.js into a flow-driven UI with dedicated password/OTP/MFA/recovery form components and action helpers.
Profile recovery codes and enrollment UI
resources/js/components/recovery_code*, two_factor_section.js, resources/js/profile/*
Adds recovery-code display/modal/panel components, TwoFactorSection, and profile wiring for enabling 2FA.
Backend/frontend/E2E tests
tests/**, tests/js/**, tests/e2e/**, app/Console/Commands/CreateRawUser.php
Adds comprehensive PHPUnit, Jest, and Playwright test coverage for the MFA feature plus a raw-user creation command for test setup.

Estimated code review effort: 5 (Critical) | ~150 minutes

CI/E2E Test Infrastructure and Unrelated Fixes

Layer / File(s) Summary
GitHub Actions frontend/E2E workflows
.github/workflows/*
Adds Jest/Playwright CI workflows and Turnstile secret env vars to existing workflows.
Local Jest/Playwright tooling and docker/build config
jest.config.js, playwright.config.ts, babel.config.js, package.json, webpack.common.js, docker-compose.yml, .gitignore, start_local_server.sh, readme.md
Configures local test tooling, Playwright docker service, and updated dev/test documentation.
Unrelated Turnstile/SSL/session and test fixes
RegisterController.php, AppServiceProvider.php, config/session.php, tests/TurnstileProtectedControllersTest.php, tests/unit/DisqusSSOProfileMappingTest.php, tests/unit/OAuth2LoginStrategyTest.php
Makes Turnstile validation conditional, config-driven SSL/session cookie settings, and fixes test idempotency/facade cleanup issues.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

Suggested reviewers: smarcet, martinquiroga-exo, caseylocker

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/recovery-codes-management

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

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

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown

📘 OpenAPI / Swagger preview

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants