diff --git a/.github/workflows/pull_request_frontend_tests.yml b/.github/workflows/pull_request_frontend_tests.yml new file mode 100644 index 00000000..f2f8df58 --- /dev/null +++ b/.github/workflows/pull_request_frontend_tests.yml @@ -0,0 +1,138 @@ +name: Front End Tests On Pull Request + +on: + pull_request: + types: [opened, reopened, edited, synchronize] + branches: ["main"] + +jobs: + + js-unit-tests: + runs-on: ubuntu-latest + steps: + - name: Check out repository code + uses: actions/checkout@v4 + - name: Set up Node.js + uses: actions/setup-node@v4 + with: + node-version: '22' + cache: 'yarn' + - name: Install JS dependencies + run: yarn install --frozen-lockfile + - name: Run Jest unit tests + run: yarn test:unit:ci + - name: Upload Jest coverage + uses: actions/upload-artifact@v4 + if: always() + with: + name: jest-coverage + path: tests/js/coverage + retention-days: 5 + + e2e-tests: + runs-on: ubuntu-latest + env: + APP_ENV: testing + APP_DEBUG: true + APP_KEY: base64:4vh0op/S1dAsXKQ2bbdCfWRyCI9r8NNIdPXyZWt9PX4= + APP_URL: http://localhost:8001 + DEV_EMAIL_TO: smarcet@gmail.com + DB_CONNECTION: mysql + DB_HOST: 127.0.0.1 + DB_PORT: 3306 + DB_DATABASE: idp_test + DB_USERNAME: root + DB_PASSWORD: 1qaz2wsx + REDIS_HOST: 127.0.0.1 + REDIS_PORT: 6379 + REDIS_DB: 0 + REDIS_PASSWORD: 1qaz2wsx + REDIS_DATABASES: 16 + SSL_ENABLED: false + SESSION_DRIVER: redis + SESSION_COOKIE_SECURE: false + PHP_VERSION: 8.3 + OTEL_SDK_DISABLED: true + OTEL_SERVICE_ENABLED: false + TURNSTILE_SITE_KEY: ${{ secrets.TURNSTILE_SITE_KEY }} + TURNSTILE_SECRET_KEY: ${{ secrets.TURNSTILE_SECRET_KEY }} + services: + mysql: + image: mysql:8.0 + env: + MYSQL_ROOT_PASSWORD: 1qaz2wsx + MYSQL_DATABASE: idp_test + ports: + - 3306:3306 + options: >- + --health-cmd="mysqladmin ping" + --health-interval=10s + --health-timeout=5s + --health-retries=3 + steps: + - name: Create Redis + uses: supercharge/redis-github-action@1.8.1 + with: + redis-port: 6379 + redis-password: 1qaz2wsx + - name: Check out repository code + uses: actions/checkout@v4 + - name: Install PHP + uses: shivammathur/setup-php@v2 + with: + php-version: ${{ env.PHP_VERSION }} + extensions: pdo_mysql, mbstring, exif, pcntl, bcmath, sockets, gettext, apcu + - name: Install PHP dependencies + uses: ramsey/composer-install@v3 + env: + COMPOSER_AUTH: '{"github-oauth": {"github.com": "${{ secrets.PAT }}"} }' + - name: Set up Node.js + uses: actions/setup-node@v4 + with: + node-version: '22' + cache: 'yarn' + - name: Install JS dependencies + run: yarn install --frozen-lockfile + - name: Build frontend assets + run: yarn build + - name: Prepare application + run: | + ./update_doctrine.sh + php artisan doctrine:migrations:migrate --no-interaction + php artisan db:seed --force + php artisan idp:create-super-admin test@test.com '1Qaz2wsx!' + php artisan idp:create-raw-user e2e@test.com '1Qaz2wsx!' + for i in 001 002 003 004 005 006 007 008; do + php artisan idp:create-super-admin "mfa-ts-$i@test.com" '1Qaz2wsx!' + done + php artisan idp:create-super-admin mfa-oauth2@test.com '1Qaz2wsx!' + php artisan idp:create-super-admin mfa-oauth2-consent@test.com '1Qaz2wsx!' + php artisan idp:create-super-admin mfa-oauth2-trust@test.com '1Qaz2wsx!' + php artisan idp:create-oauth2-test-client + - name: Install Playwright Chromium + run: npx playwright install --with-deps chromium + - name: Start web server + run: php artisan serve --host=127.0.0.1 --port=8001 & + - name: Wait for server to be ready + run: | + for i in $(seq 1 20); do + curl -sf http://localhost:8001 > /dev/null 2>&1 && echo "Server ready" && exit 0 + sleep 2 + done + echo "Server did not start in time" && exit 1 + - name: Run E2E tests + run: yarn test:e2e --reporter=list + - name: Upload Playwright report + uses: actions/upload-artifact@v4 + if: always() + with: + name: playwright-report + path: tests/e2e/report + retention-days: 7 + - name: Upload Playwright traces + uses: actions/upload-artifact@v4 + if: failure() + with: + name: playwright-traces + path: test-results/ + retention-days: 7 diff --git a/.github/workflows/pull_request_unit_tests.yml b/.github/workflows/pull_request_unit_tests.yml index 462317c3..45df32f3 100644 --- a/.github/workflows/pull_request_unit_tests.yml +++ b/.github/workflows/pull_request_unit_tests.yml @@ -37,6 +37,8 @@ jobs: PHP_VERSION: 8.3 OTEL_SDK_DISABLED: true OTEL_SERVICE_ENABLED: false + TURNSTILE_SITE_KEY: ${{ secrets.TURNSTILE_SITE_KEY }} + TURNSTILE_SECRET_KEY: ${{ secrets.TURNSTILE_SECRET_KEY }} services: mysql: image: mysql:8.0 diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml index ad2ede65..ec993b27 100644 --- a/.github/workflows/push.yml +++ b/.github/workflows/push.yml @@ -33,6 +33,8 @@ jobs: PHP_VERSION: 8.3 OTEL_SDK_DISABLED: true OTEL_SERVICE_ENABLED: false + TURNSTILE_SITE_KEY: ${{ secrets.TURNSTILE_SITE_KEY }} + TURNSTILE_SECRET_KEY: ${{ secrets.TURNSTILE_SECRET_KEY }} services: mysql: image: mysql:8.0 diff --git a/.github/workflows/push_frontend_tests.yml b/.github/workflows/push_frontend_tests.yml new file mode 100644 index 00000000..5376f6f0 --- /dev/null +++ b/.github/workflows/push_frontend_tests.yml @@ -0,0 +1,139 @@ +name: Front End Tests On Push + +on: push + +jobs: + + js-unit-tests: + runs-on: ubuntu-latest + steps: + - name: Check out repository code + uses: actions/checkout@v4 + - name: Set up Node.js + uses: actions/setup-node@v4 + with: + node-version: '22' + cache: 'yarn' + - name: Install JS dependencies + run: yarn install --frozen-lockfile + - name: Run Jest unit tests + run: yarn test:unit:ci + - name: Upload Jest coverage + uses: actions/upload-artifact@v4 + if: always() + with: + name: jest-coverage + path: tests/js/coverage + retention-days: 5 + + e2e-tests: + runs-on: ubuntu-latest + env: + APP_ENV: testing + APP_DEBUG: true + APP_KEY: base64:4vh0op/S1dAsXKQ2bbdCfWRyCI9r8NNIdPXyZWt9PX4= + APP_URL: http://localhost:8001 + DEV_EMAIL_TO: smarcet@gmail.com + DB_CONNECTION: mysql + DB_HOST: 127.0.0.1 + DB_PORT: 3306 + DB_DATABASE: idp_test + DB_USERNAME: root + DB_PASSWORD: 1qaz2wsx + REDIS_HOST: 127.0.0.1 + REDIS_PORT: 6379 + REDIS_DB: 0 + REDIS_PASSWORD: 1qaz2wsx + REDIS_DATABASES: 16 + SSL_ENABLED: false + SESSION_DRIVER: redis + SESSION_COOKIE_SECURE: false + PHP_VERSION: 8.3 + OTEL_SDK_DISABLED: true + OTEL_SERVICE_ENABLED: false + TURNSTILE_SITE_KEY: ${{ secrets.TURNSTILE_SITE_KEY }} + TURNSTILE_SECRET_KEY: ${{ secrets.TURNSTILE_SECRET_KEY }} + # `php artisan serve` (below) is PHP's built-in single-threaded dev server — + # it can only handle one request at a time, so Playwright workers must stay + # at 1 here or concurrent page loads queue up and blow the 30s test timeout. + PLAYWRIGHT_WORKERS: 1 + services: + mysql: + image: mysql:8.0 + env: + MYSQL_ROOT_PASSWORD: 1qaz2wsx + MYSQL_DATABASE: idp_test + ports: + - 3306:3306 + options: >- + --health-cmd="mysqladmin ping" + --health-interval=10s + --health-timeout=5s + --health-retries=3 + steps: + - name: Create Redis + uses: supercharge/redis-github-action@1.8.1 + with: + redis-port: 6379 + redis-password: 1qaz2wsx + - name: Check out repository code + uses: actions/checkout@v4 + - name: Install PHP + uses: shivammathur/setup-php@v2 + with: + php-version: ${{ env.PHP_VERSION }} + extensions: pdo_mysql, mbstring, exif, pcntl, bcmath, sockets, gettext, apcu + - name: Install PHP dependencies + uses: ramsey/composer-install@v3 + env: + COMPOSER_AUTH: '{"github-oauth": {"github.com": "${{ secrets.PAT }}"} }' + - name: Set up Node.js + uses: actions/setup-node@v4 + with: + node-version: '22' + cache: 'yarn' + - name: Install JS dependencies + run: yarn install --frozen-lockfile + - name: Build frontend assets + run: yarn build + - name: Prepare application + run: | + ./update_doctrine.sh + php artisan doctrine:migrations:migrate --no-interaction + php artisan db:seed --force + php artisan idp:create-super-admin test@test.com '1Qaz2wsx!' + php artisan idp:create-raw-user e2e@test.com '1Qaz2wsx!' + for i in 001 002 003 004 005 006 007 008; do + php artisan idp:create-super-admin "mfa-ts-$i@test.com" '1Qaz2wsx!' + done + php artisan idp:create-super-admin mfa-oauth2@test.com '1Qaz2wsx!' + php artisan idp:create-super-admin mfa-oauth2-consent@test.com '1Qaz2wsx!' + php artisan idp:create-super-admin mfa-oauth2-trust@test.com '1Qaz2wsx!' + php artisan idp:create-oauth2-test-client + - name: Install Playwright Chromium + run: npx playwright install --with-deps chromium + - name: Start web server + run: php artisan serve --host=127.0.0.1 --port=8001 & + - name: Wait for server to be ready + run: | + for i in $(seq 1 20); do + curl -sf http://localhost:8001 > /dev/null 2>&1 && echo "Server ready" && exit 0 + sleep 2 + done + echo "Server did not start in time" && exit 1 + - name: Run E2E tests + run: yarn test:e2e --reporter=list + - name: Upload Playwright report + uses: actions/upload-artifact@v4 + if: always() + with: + name: playwright-report + path: tests/e2e/report + retention-days: 7 + - name: Upload Playwright traces + uses: actions/upload-artifact@v4 + if: failure() + with: + name: playwright-traces + path: test-results/ + retention-days: 7 diff --git a/.gitignore b/.gitignore index 75aa0c64..74d9df00 100644 --- a/.gitignore +++ b/.gitignore @@ -54,4 +54,13 @@ docker-compose/mysql/model/*.sql public/assets/*.map public/assets/css/*.map .codegraph -docs/plans \ No newline at end of file +docs/plans + +# Playwright +/tests/e2e/report/ +/test-results/ + +# Jest +/tests/js/coverage/ +/playwright-report/ +/.playwright-out/ diff --git a/app/Console/Commands/CreateOAuth2TestClient.php b/app/Console/Commands/CreateOAuth2TestClient.php new file mode 100644 index 00000000..8fdaa402 --- /dev/null +++ b/app/Console/Commands/CreateOAuth2TestClient.php @@ -0,0 +1,106 @@ +findOneBy(['client_id' => self::CLIENT_ID]); + + if (is_null($client)) { + // The consent screen's getDeveloperEmail() dereferences the + // client's owner unconditionally - a client without one 500s + // as soon as a real login reaches /accounts/user/consent. + $owner = EntityManager::getRepository(User::class)->findOneBy(['email' => self::OWNER_EMAIL]); + if (is_null($owner)) { + $owner = new User(); + $owner->setEmail(self::OWNER_EMAIL); + $owner->verifyEmail(); + $owner->setPassword('1Qaz2wsx!'); + $owner->setFirstName(self::OWNER_EMAIL); + $owner->setLastName(self::OWNER_EMAIL); + $owner->setIdentifier(self::OWNER_EMAIL); + EntityManager::persist($owner); + EntityManager::flush(); + } + + $client = ClientFactory::build([ + 'app_name' => 'oauth2_test_app', + 'app_description' => 'oauth2_test_app', + 'client_id' => self::CLIENT_ID, + 'client_secret' => self::CLIENT_SECRET, + 'client_type' => IClient::ClientType_Confidential, + 'application_type' => IClient::ApplicationType_Web_App, + 'token_endpoint_auth_method' => OAuth2Protocol::TokenEndpoint_AuthMethod_ClientSecretBasic, + 'owner' => $owner, + 'rotate_refresh_token' => true, + 'use_refresh_token' => true, + 'redirect_uris' => self::REDIRECT_URI, + ]); + EntityManager::persist($client); + EntityManager::flush(); + $this->info('Created client: ' . self::CLIENT_ID); + } else { + $this->info('Client already exists: ' . self::CLIENT_ID); + } + + $scope = EntityManager::getRepository(ApiScope::class)->findOneBy(['name' => 'profile']); + if (is_null($scope)) { + $this->error("api scope 'profile' not found - run php artisan db:seed first"); + return 1; + } + + $client->addScope($scope); + EntityManager::persist($client); + EntityManager::flush(); + + return 0; + } +} diff --git a/app/Console/Commands/CreateRawUser.php b/app/Console/Commands/CreateRawUser.php new file mode 100644 index 00000000..b25450b8 --- /dev/null +++ b/app/Console/Commands/CreateRawUser.php @@ -0,0 +1,57 @@ +argument('email')); + $password = trim($this->argument('password')); + + $user = EntityManager::getRepository(User::class)->findOneBy(['email' => $email]); + if (is_null($user)) { + $user = new User(); + $user->setEmail($email); + $user->verifyEmail(); + $user->setPassword($password); + $user->setFirstName($email); + $user->setLastName($email); + $user->setIdentifier($email); + EntityManager::persist($user); + EntityManager::flush(); + $this->info("Created user: {$email}"); + } else { + $this->info("User already exists: {$email}"); + } + } +} diff --git a/app/Console/Commands/GetLatestOtp.php b/app/Console/Commands/GetLatestOtp.php new file mode 100644 index 00000000..c51a6ff4 --- /dev/null +++ b/app/Console/Commands/GetLatestOtp.php @@ -0,0 +1,52 @@ +argument('email')); + + // DoctrineOAuth2OTPRepository::getByUserNameNotRedeemed() orders by + // id DESC, so the newest not-yet-redeemed OTP is the FIRST result, + // not the last - an account with more than one pending OTP (e.g. a + // prior attempt that was never redeemed) would otherwise return a + // stale code. + $otps = $repository->getByUserNameNotRedeemed($email); + if (empty($otps)) { + $this->error("no pending otp for {$email}"); + return 1; + } + + $this->line(reset($otps)->getValue()); + return 0; + } +} diff --git a/app/Console/Kernel.php b/app/Console/Kernel.php index 89bf376a..03857599 100644 --- a/app/Console/Kernel.php +++ b/app/Console/Kernel.php @@ -29,6 +29,9 @@ class Kernel extends ConsoleKernel Commands\CleanOAuth2StaleData::class, Commands\CleanOpenIdStaleData::class, Commands\CreateSuperAdmin::class, + Commands\CreateRawUser::class, + Commands\CreateOAuth2TestClient::class, + Commands\GetLatestOtp::class, Commands\SpammerProcess\RebuildUserSpammerEstimator::class, Commands\SpammerProcess\UserSpammerProcessor::class, ]; diff --git a/app/Http/Controllers/Api/UserApiController.php b/app/Http/Controllers/Api/UserApiController.php index b32c7307..b334959b 100644 --- a/app/Http/Controllers/Api/UserApiController.php +++ b/app/Http/Controllers/Api/UserApiController.php @@ -16,6 +16,7 @@ use App\Http\Controllers\Traits\RequestProcessor; use App\Http\Controllers\UserValidationRulesFactory; use App\ModelSerializers\SerializerRegistry; +use App\Services\Auth\IRecoveryCodeService; use Auth\Repositories\IUserRepository; use Auth\User; use Exception; @@ -23,6 +24,7 @@ use Illuminate\Support\Facades\Auth; use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Request; +use Illuminate\Support\Facades\Validator; use models\exceptions\EntityNotFoundException; use models\exceptions\ValidationException; use OAuth2\Services\ITokenService; @@ -43,23 +45,31 @@ final class UserApiController extends APICRUDController */ private $token_service; + /** + * @var IRecoveryCodeService + */ + private $recovery_code_service; + /** * UserApiController constructor. * @param IUserRepository $user_repository * @param ILogService $log_service * @param IUserService $user_service * @param ITokenService $token_service + * @param IRecoveryCodeService $recovery_code_service */ public function __construct ( IUserRepository $user_repository, ILogService $log_service, IUserService $user_service, - ITokenService $token_service + ITokenService $token_service, + IRecoveryCodeService $recovery_code_service ) { parent::__construct($user_repository, $user_service, $log_service); $this->token_service = $token_service; + $this->recovery_code_service = $recovery_code_service; } /** @@ -247,6 +257,68 @@ public function updateMe() return $this->update(Auth::user()->getId()); } + /** + * Enables a 2FA method for the current user and generates the first batch of + * recovery codes for them. Plaintext codes are returned once in the response + * and never persisted. + * + * @return \Illuminate\Http\JsonResponse|mixed + */ + public function enableTwoFactor() + { + if (!Auth::check()) + return $this->error403(); + + return $this->processRequest(function () { + $data = Request::all(); + $validator = Validator::make($data, [ + 'method' => 'required|string|in:' . implode(',', User::ValidMFAMethods), + ]); + + if (!$validator->passes()) { + return $this->error412($validator->getMessageBag()->getMessages()); + } + + $user = Auth::user(); + $method = $data['method']; + + if ($user->isTwoFactorEnabled()) { + return $this->error412(['method' => ['Two-factor authentication is already enabled. Use the regenerate recovery codes endpoint to rotate your codes.']]); + } + + $codes = $this->recovery_code_service->enableTwoFactorAndGenerateCodes($user, $method); + + return $this->ok(['recovery_codes' => $codes]); + }); + } + + /** + * Invalidates the current user's recovery codes and generates a fresh batch. + * Plaintext codes are returned once in the response and never persisted. + * + * @return \Illuminate\Http\JsonResponse|mixed + */ + public function regenerateRecoveryCodes() + { + if (!Auth::check()) + return $this->error403(); + + return $this->processRequest(function () { + $data = Request::all(); + $validator = Validator::make($data, [ + 'current_password' => 'required|string', + ]); + + if (!$validator->passes()) { + return $this->error412($validator->getMessageBag()->getMessages()); + } + + $codes = $this->recovery_code_service->regenerateRecoveryCodes(Auth::user(), $data['current_password']); + + return $this->ok(['recovery_codes' => $codes]); + }); + } + public function revokeAllMyTokens() { if (!Auth::check()) diff --git a/app/Http/Controllers/Auth/RegisterController.php b/app/Http/Controllers/Auth/RegisterController.php index 4ec12ec0..f1bf24aa 100644 --- a/app/Http/Controllers/Auth/RegisterController.php +++ b/app/Http/Controllers/Auth/RegisterController.php @@ -173,15 +173,18 @@ public function showRegistrationForm(LaravelRequest $request) protected function validator(array $data) { $rules = [ - 'first_name' => 'required|string|max:100', - 'last_name' => 'required|string|max:100', - 'country_iso_code' => 'required|string|country_iso_alpha2_code', - 'email' => 'required|string|email|max:255', - 'password' => 'required|string|confirmed|password_policy', - 'cf-turnstile-response' => ['required', new Turnstile()], + 'first_name' => 'required|string|max:100', + 'last_name' => 'required|string|max:100', + 'country_iso_code' => 'required|string|country_iso_alpha2_code', + 'email' => 'required|string|email|max:255', + 'password' => 'required|string|confirmed|password_policy', ]; - if(!empty(Config::get("app.code_of_conduct_link", null))){ + if (!empty(Config::get("services.turnstile.secret", null))) { + $rules['cf-turnstile-response'] = ['required', new Turnstile()]; + } + + if (!empty(Config::get("app.code_of_conduct_link", null))) { $rules['agree_code_of_conduct'] = 'required|string|in:true'; } diff --git a/app/Http/Controllers/UserController.php b/app/Http/Controllers/UserController.php index ef3c49ce..97a54674 100644 --- a/app/Http/Controllers/UserController.php +++ b/app/Http/Controllers/UserController.php @@ -21,6 +21,7 @@ use App\libs\OAuth2\Strategies\LoginHintProcessStrategy; use App\ModelSerializers\SerializerRegistry; use App\Services\Auth\IDeviceTrustService; +use App\Services\Auth\IRecoveryCodeService; use App\Services\Auth\ITwoFactorAuditService; use App\Services\Auth\ITwoFactorGateService; use App\Services\Auth\ITwoFactorRateLimitService; @@ -160,6 +161,11 @@ final class UserController extends OpenIdController */ private $two_factor_rate_limit_service; + /** + * @var IRecoveryCodeService + */ + private $recovery_code_service; + /** * @param IMementoOpenIdSerializerService $openid_memento_service * @param IMementoOAuth2SerializerService $oauth2_memento_service @@ -200,6 +206,7 @@ public function __construct ITwoFactorAuditService $two_factor_audit_service, ITwoFactorGateService $mfa_gate_service, ITwoFactorRateLimitService $two_factor_rate_limit_service, + IRecoveryCodeService $recovery_code_service, ) { $this->openid_memento_service = $openid_memento_service; @@ -221,6 +228,7 @@ public function __construct $this->two_factor_audit_service = $two_factor_audit_service; $this->mfa_gate_service = $mfa_gate_service; $this->two_factor_rate_limit_service = $two_factor_rate_limit_service; + $this->recovery_code_service = $recovery_code_service; $this->middleware(function ($request, $next) use($login_hint_process_strategy){ @@ -919,7 +927,14 @@ public function verify2FARecovery() // See verify2FA() for rationale: return the destination as data so a real // top-level navigation (not this XHR) performs any cross-origin hop. $redirect = $this->login_strategy->postLogin(); - return $this->ok(['redirect_url' => $redirect->getTargetUrl()]); + return $this->ok([ + 'redirect_url' => $redirect->getTargetUrl(), + // CU-86ba2zp66 / sds/idp-mfa.md §4.10.3, §4.11 step 5: the login page + // must be able to warn the user when they've just burned into their + // last few recovery codes, since it may be their only way back in. + 'recovery_codes_remaining' => $this->recovery_code_service->countUnusedRecoveryCodes($user), + 'recovery_codes_low_threshold' => (int) config('auth.recovery_codes.low_threshold', 3), + ]); } catch (ValidationException $ex) { Log::warning($ex); return $this->error412($ex->getMessages()); @@ -1184,6 +1199,10 @@ public function getProfile() 'actions' => $actions, 'countries' => CountryList::getCountries(), 'languages' => $lang2Code, + 'two_factor_enabled' => $user->shouldRequire2FA(), + 'recovery_codes_remaining' => $this->recovery_code_service->countUnusedRecoveryCodes($user), + 'recovery_codes_total' => (int)config('auth.recovery_codes.count', 10), + 'recovery_codes_low_threshold' => (int)config('auth.recovery_codes.low_threshold', 3), ]); } diff --git a/app/Providers/AppServiceProvider.php b/app/Providers/AppServiceProvider.php index 09ed90d4..e25e83f7 100644 --- a/app/Providers/AppServiceProvider.php +++ b/app/Providers/AppServiceProvider.php @@ -40,7 +40,7 @@ class AppServiceProvider extends ServiceProvider */ public function boot() { - if (!App::isLocal()) + if (Config::get('server.ssl_enabled', false)) URL::forceScheme('https'); $logger = Log::getLogger(); diff --git a/app/Services/Auth/IRecoveryCodeService.php b/app/Services/Auth/IRecoveryCodeService.php new file mode 100644 index 00000000..51df966b --- /dev/null +++ b/app/Services/Auth/IRecoveryCodeService.php @@ -0,0 +1,64 @@ +checkPassword(trim($currentPassword))) { + throw new ValidationException('current_password is not correct.'); + } + + return $this->generateRecoveryCodes($user); + } + + /** + * @inheritDoc + */ + public function generateRecoveryCodes(User $user): array + { + $codes = $this->tx_service->transaction(fn() => $this->regenerateCodesForUser($user)); + + // Best-effort: the codes are already committed and about to be shown to + // the user, so an audit-logging failure must not 500 this response - a + // client retry on a 500 would regenerate and invalidate the codes it + // just received. + try { + $this->audit_service->log( + $user, + TwoFactorAuditLog::EventRecoveryCodesGenerated, + TwoFactorAuditLog::MethodRecovery, + IPHelper::getUserIp() + ); + } catch (\Throwable $ex) { + Log::warning($ex); + } + + return $codes; + } + + /** + * @inheritDoc + */ + public function enableTwoFactorAndGenerateCodes(User $user, string $method): array + { + // Everything must live in a single transaction() call: it opens/commits + // its own connection-level transaction and closes the entity manager on + // failure (see DoctrineTransactionService::transaction()), so nesting a + // second call inside it (e.g. by calling generateRecoveryCodes() here) + // would let an inner failure tear down the EM out from under this + // still-running outer transaction. + $codes = $this->tx_service->transaction(function () use ($user, $method) { + $user->enable2FA($method); + $this->user_repository->add($user, false); + + return $this->regenerateCodesForUser($user); + }); + + // Best-effort: 2FA is already enabled and the codes are already + // committed, so an audit-logging failure must not 500 this response - a + // client retry on a 500 would regenerate and invalidate the codes it + // just received. + try { + $this->audit_service->log( + $user, + TwoFactorAuditLog::EventRecoveryCodesGenerated, + TwoFactorAuditLog::MethodRecovery, + IPHelper::getUserIp() + ); + + $this->audit_service->log( + $user, + TwoFactorAuditLog::EventEnrollmentChanged, + $method, + IPHelper::getUserIp() + ); + } catch (\Throwable $ex) { + Log::warning($ex); + } + + return $codes; + } + + /** + * Invalidates every existing recovery code for the user and generates a + * fresh batch, within the caller's already-open transaction. + * + * @return string[] plaintext codes formatted as XXXX-XXXX + */ + private function regenerateCodesForUser(User $user): array + { + $count = (int)config('auth.recovery_codes.count', 10); + $length = (int)config('auth.recovery_codes.length', 8); + + $plaintext_codes = []; + + $this->repository->deleteAllForUser($user); + + for ($i = 0; $i < $count; $i++) { + $plain = Rand::getString($length, self::CODE_CHARSET, true); + $plaintext_codes[] = $plain; + + $code = new UserRecoveryCode(); + $code->setUser($user); + $code->setCodeHash(Hash::make($plain)); + $this->repository->add($code, false); + } + + return array_map(static fn(string $code) => implode('-', str_split($code, 4)), $plaintext_codes); + } + + /** + * @inheritDoc + */ + public function countUnusedRecoveryCodes(User $user): int + { + return count($this->repository->getUnusedByUser($user)); + } +} diff --git a/app/Services/Auth/TwoFactorServiceProvider.php b/app/Services/Auth/TwoFactorServiceProvider.php index 0bb18a6f..b82bdd10 100644 --- a/app/Services/Auth/TwoFactorServiceProvider.php +++ b/app/Services/Auth/TwoFactorServiceProvider.php @@ -42,6 +42,7 @@ public function register(): void $this->app->singleton(ITwoFactorAuditService::class, TwoFactorAuditService::class); $this->app->singleton(ITwoFactorGateService::class, MFAGateService::class); $this->app->singleton(ITwoFactorRateLimitService::class, TwoFactorRateLimitService::class); + $this->app->singleton(IRecoveryCodeService::class, RecoveryCodeService::class); } /** @@ -110,6 +111,7 @@ public function provides(): array ITwoFactorAuditService::class, ITwoFactorGateService::class, ITwoFactorRateLimitService::class, + IRecoveryCodeService::class, ]; } } diff --git a/app/Strategies/MFA/AbstractMFAChallengeStrategy.php b/app/Strategies/MFA/AbstractMFAChallengeStrategy.php index c2d04fea..95da987e 100644 --- a/app/Strategies/MFA/AbstractMFAChallengeStrategy.php +++ b/app/Strategies/MFA/AbstractMFAChallengeStrategy.php @@ -48,6 +48,11 @@ public function clearPendingState(): void public function verifyRecoveryCode(User $user, string $code): void { + // Recovery codes are hashed without the "-" separator; it is added only + // for on-screen readability (XXXX-XXXX). Normalize here so a code typed + // or pasted exactly as displayed still matches the stored hash. + $code = strtoupper(preg_replace('/[^A-Za-z0-9]/', '', $code)); + foreach ($this->recovery_code_repository->getUnusedByUser($user) as $recoveryCode) { if (Hash::check($code, $recoveryCode->getCodeHash())) { // Concurrency: acquire a PESSIMISTIC_WRITE row lock and re-hydrate diff --git a/app/libs/Auth/Models/TwoFactorAuditLog.php b/app/libs/Auth/Models/TwoFactorAuditLog.php index 4938345a..5258978a 100644 --- a/app/libs/Auth/Models/TwoFactorAuditLog.php +++ b/app/libs/Auth/Models/TwoFactorAuditLog.php @@ -29,6 +29,7 @@ class TwoFactorAuditLog extends BaseEntity public const EventDeviceRevoked = 'device_revoked'; public const EventRecoveryUsed = 'recovery_used'; public const EventSettingsChanged = 'settings_changed'; + public const EventRecoveryCodesGenerated = 'recovery_codes_generated'; public const MethodEmailOtp = 'email_otp'; public const MethodSmsOtp = 'sms_otp'; @@ -46,6 +47,7 @@ class TwoFactorAuditLog extends BaseEntity self::EventDeviceRevoked, self::EventRecoveryUsed, self::EventSettingsChanged, + self::EventRecoveryCodesGenerated, ]; private const ALLOWED_METHODS = [ diff --git a/babel.config.js b/babel.config.js index bc853ec6..e5afd041 100644 --- a/babel.config.js +++ b/babel.config.js @@ -21,6 +21,21 @@ module.exports = { plugins: [ "@babel/plugin-proposal-object-rest-spread", "@babel/plugin-proposal-class-properties" - ] + ], + env: { + test: { + presets: [ + [ + "@babel/preset-env", + { + "targets": { "node": "current" }, + "useBuiltIns": false + } + ], + "@babel/preset-react", + "@babel/preset-flow" + ] + } + } }; diff --git a/config/auth.php b/config/auth.php index 5da1b184..69282ef7 100644 --- a/config/auth.php +++ b/config/auth.php @@ -107,6 +107,12 @@ 'password_shape_warning' => env('AUTH_PASSWORD_SHAPE_WARNING', 'Password must include at least one uppercase letter, one lowercase letter, one number, and one special character (#?!@$%^&*+-).'), 'verification_email_lifetime' => env("AUTH_VERIFICATION_EMAIL_LIFETIME", 600), 'allows_native_auth' => env('AUTH_ALLOWS_NATIVE_AUTH', 1), + + 'recovery_codes' => [ + 'count' => env('MFA_RECOVERY_CODES_COUNT', 10), + 'length' => env('MFA_RECOVERY_CODE_LENGTH', 8), + 'low_threshold' => env('MFA_RECOVERY_CODES_LOW_THRESHOLD', 3), + ], 'allows_native_on_config' => env('AUTH_ALLOWS_NATIVE_AUTH_CONFIG', 1), 'allows_opt_auth' => env('AUTH_ALLOWS_OTP_AUTH', 1), ]; diff --git a/config/session.php b/config/session.php index 39306e12..6e18cbbe 100644 --- a/config/session.php +++ b/config/session.php @@ -148,7 +148,7 @@ | */ - 'secure' => true, + 'secure' => env('SESSION_SECURE_COOKIE', false), /* |-------------------------------------------------------------------------- @@ -176,6 +176,6 @@ | */ - 'same_site' => 'none', + 'same_site' => env('SESSION_COOKIE_SAME_SITE', 'lax'), ]; diff --git a/doc/adrs/0001-recovery-code-generation-not-triggered-by-group-enforcement.md b/doc/adrs/0001-recovery-code-generation-not-triggered-by-group-enforcement.md new file mode 100644 index 00000000..31ed8bbd --- /dev/null +++ b/doc/adrs/0001-recovery-code-generation-not-triggered-by-group-enforcement.md @@ -0,0 +1,91 @@ +# 0001. Recovery code generation is not automatically triggered by group-based 2FA enforcement + +## Status + +Accepted — 2026-07-08 + +## Context + +Ticket [CU-86ba2zp66](https://app.clickup.com/t/86ba2zp66) ("Recovery Code Management UI and Endpoints") requires: + +> Create endpoint to generate recovery codes **when 2FA is enabled or admin enforcement requires code creation**. + +Two-factor authentication in this project can become mandatory for a user in two distinct ways: + +1. **Explicit self-enrollment** — the user calls the new `UserApiController::enableTwoFactor()` endpoint + (`app/Http/Controllers/Api/UserApiController.php`), which invokes `User::enable2FA($method)` and, in the + same transaction, calls `RecoveryCodeService::generateRecoveryCodes()` to mint the user's first batch of + recovery codes. +2. **Group-based enforcement** — `User::shouldRequire2FA()` (`app/libs/Auth/Models/User.php`) returns `true` + for any user belonging to one of the groups listed in `config('two_factor.enforced_groups')` + (`config/two_factor.php`: SuperAdmin, Admin, OAuth2ServerAdmin, OpenIdServerAdmins), **independently** of + whether that user ever called `enableTwoFactor()` or has the `two_factor_enabled` column set. + +Only path (1) generates recovery codes automatically. Path (2) has no corresponding hook: there is no +listener on group-membership assignment (e.g. when a user is added to the `Admin` group via +`GroupApiController` or any other admin-management flow) that generates a recovery-code batch for that user. + +Practical consequence: a user who is force-enrolled into MFA purely by being added to an enforced group — +and who never separately visits their profile to enable 2FA or regenerate codes — has **zero recovery codes** +until they proactively open their profile's "Two-Factor Authentication" section and click "Regenerate Codes" +(`resources/js/components/recovery_codes_panel.js`). That manual path works correctly and is not gated on +prior enrollment, but it is not automatic, so the literal wording of the ticket ("or admin enforcement +requires code creation") is not satisfied for this path. + +Implementing the automatic path would require identifying every place group membership can be granted +(direct admin action, bulk import, programmatic group assignment, etc.) and wiring a listener/hook into each +one to call `RecoveryCodeService::generateRecoveryCodes()` exactly once per user, without duplicating codes +on repeated grants or interfering with a user who already regenerated codes themselves. No single +well-defined integration point for "group membership changed" was identified during implementation without +a dedicated exploration pass, and building one was judged to be a meaningfully larger change than the rest +of this ticket. + +## Decision + +We accept the gap as a documented scope limitation for this ticket. Recovery code generation for +group-enforced users remains **on-demand**: the user (or an admin acting on their behalf, e.g. via a support +flow) must visit the profile's Two-Factor Authentication section and use "Regenerate Codes" at least once +after being enrolled through group enforcement. + +No code changes accompany this decision; it documents the trade-off already present in the shipped +implementation (`feat/recovery-codes-management` branch). + +## Consequences + +**Positive** + +- No new event/listener infrastructure needed for group-membership changes, keeping the change surface of + this ticket limited to the profile self-service flow it was originally scoped around. +- The manual path is simple, already implemented, and requires no additional user-facing concept: a + group-enforced user sees the same "Two-Factor Authentication" section and the same "Regenerate Codes" + action as anyone who self-enrolled. +- Avoids the risk of generating recovery codes an admin never asked for or expects during unrelated + group-management operations (e.g. bulk group imports). + +**Negative** + +- A user who is force-enrolled by group membership and is challenged for MFA (e.g. at their next login) + before ever visiting their profile has **no recovery codes available** if they lose access to their normal + 2FA method (email, for Phase I) at that point. Their only recourse is out-of-band administrative + intervention (e.g. a server admin resetting their 2FA state directly), not a self-service recovery path. +- The literal acceptance criterion "generate recovery codes ... when admin enforcement requires code + creation" is not met for the group-enforcement path — only for explicit self-enrollment. + +**Follow-up (not scheduled)** + +If this gap needs to be closed later, the natural integration point is wherever group membership is granted +(`GroupApiController` and any other code path that adds a user to `Group`) — call +`RecoveryCodeService::generateRecoveryCodes($user)` immediately after granting membership to a group in +`config('two_factor.enforced_groups')`, guarded so it only fires when the user currently has zero unused +codes (to avoid clobbering codes on every re-grant). + +## Alternatives considered + +- **Hook into group-assignment code paths now.** Rejected for this ticket: requires auditing every place + group membership can change (there is more than one — see `GroupApiController` and related admin flows) + to guarantee the hook fires exactly once and doesn't silently invalidate codes a user already saved. Judged + to be new scope beyond "Recovery Code Management UI and Endpoints," better handled as its own ticket if + the org decides to close this gap. +- **Generate codes lazily on first MFA challenge instead of on group grant.** Rejected: the MFA challenge + screen itself has no natural place to show a one-time "here are your recovery codes" modal without + interrupting the login flow the ticket explicitly requires to remain "unaffected." diff --git a/doc/mfa-test-gap-report.md b/doc/mfa-test-gap-report.md new file mode 100644 index 00000000..5c6fd602 --- /dev/null +++ b/doc/mfa-test-gap-report.md @@ -0,0 +1,143 @@ +# MFA Test Gap Report — PR 142 + +**Branch:** `feat/mfa---login-ui-flow` +**Date:** 2026-06-30 +**Scope:** All files changed across the MFA feature branch (backend + frontend) + +--- + +## Summary + +PR 142 adds full MFA authentication support: 65 files changed, +6,900 lines. The PHP backend layer has strong coverage — 11 dedicated test files were added as part of the PR. The entire frontend refactor (15 JavaScript/JSX files, ~2,220 lines) has **zero test coverage**, and four specific PHP areas were identified as gaps in isolation-level coverage even though they are exercised indirectly by the integration suite. + +| Layer | Files Changed | Files with Tests | Coverage | +|---|---|---|---| +| PHP — services, strategies, repositories | 30 | 30 | ✅ Direct | +| PHP — HTTP / controller layer | 6 | 0 (integration only) | ⚠️ Partial | +| JavaScript — login UI | 15 | 0 | ❌ None | + +--- + +## What IS Covered — PHP Test Files Added in PR 142 + +The following 11 test files were added or substantially extended as part of this PR. They form the baseline any reviewer can rely on. + +### Integration / Feature Tests + +| File | Tests | What it covers | +|---|---|---| +| `tests/TwoFactorLoginFlowTest.php` | 19 | Full end-to-end MFA login flow via HTTP: admin/non-admin routing, OTP verify/fail/reuse, recovery codes, device trust cookie enrollment, trusted-device bypass, audit failure resilience, rate-limit enforcement on verify/recovery/resend endpoints | +| `tests/AuthServiceValidateCredentialsIntegrationTest.php` | 2 | `AuthService::validateCredentials` integration path including the MFA gate check | + +### Unit Tests + +| File | Tests | What it covers | +|-------------------------------------------------------|---|---| +| `tests/unit/AuthServiceValidateCredentialsTest.php` | 9 | Password validation, account state guards, `validateCredentials` under MFA gate (unit) | +| `tests/unit/UserTwoFactorTest.php` | 14 | User entity 2FA flag logic, enforcement rules, group-based enforcement, method availability | +| `tests/unit/MFAGateServiceTest.php` | 5 | `MFAGateService::requiresChallenge` decision tree for all trust/enforce/cookie combinations | +| `tests/unit/TwoFactorAuditServiceTest.php` | 7 | Audit event recording: challenge issued, verified, failed, device trusted | +| `tests/DeviceTrustServiceTest.php` | 15 | Full `DeviceTrustService` contract: trust/revoke/expire/validate, SHA-256 storage, audit wiring | +| `tests/unit/MFA/AbstractMFAChallengeStrategyTest.php` | 8 | Base strategy: OTP generation, expiry, session binding, reuse prevention | +| `tests/unit/MFA/EmailOTPMFAChallengeStrategyTest.php` | 5 | Email OTP dispatch, already-redeemed race, numeric-only validation | +| `tests/unit/MFA/MFAChallengeStrategyFactoryTest.php` | 2 | Factory resolves correct strategy for each `MFA_METHODS` value | + +### Repository / Model Tests + +| File | Tests | What it covers | +|---|---|---| +| `tests/TwoFactorRepositoriesTest.php` | 11 | Doctrine round-trips for `UserTrustedDevice`, `TwoFactorAuditLog`, `UserRecoveryCode`: persistence, expiry/revocation queries, uniqueness constraints, `setCodeHash` guards against plaintext | + +--- + +## Gaps — PHP Backend + +These four items lack isolated test coverage. They are exercised indirectly by `TwoFactorLoginFlowTest` but would be invisible to a unit test runner. + +### 1. `cancelLogin` Controller Endpoint (Critical) + +**File:** `app/Http/Controllers/UserController.php` — `cancelLogin()` action +**What it does:** Tears down the pending-MFA session state when the user cancels mid-challenge. If this is broken, users can get stuck in an unrecoverable MFA state or, worse, a session may retain stale auth context. +**Gap:** No unit or dedicated integration test for the `POST /auth/cancel-login` route. The flow test exercises the happy-path continuation but not cancellation edge cases (double-cancel, cancel with no pending session, cancel with concurrent session). + +### 2. `TwoFactorRateLimitMiddleware` Isolation (High) + +**File:** `app/Http/Middleware/TwoFactorRateLimitMiddleware.php` +**What it does:** Cache-backed, fixed-window rate limiting for verify/recovery/resend. Counters survive session cleanup. Verify/recovery increment only on failure; resend increments always. +**Gap:** The middleware is tested indirectly through `TwoFactorLoginFlowTest` (`testVerifyRateLimitBlocksAfterThreshold` etc.), but there are no isolated middleware unit tests covering: window expiry after TTL, per-action counter separation, resend counting regardless of response status, and behavior when no pending session key exists. + +### 3. `MFACookieManager` Trait Isolation (Medium) + +**File:** `app/Http/Controllers/Traits/MFACookieManager.php` +**What it does:** Reads the raw device-trust cookie from the request and queues the `Set-Cookie` header. Cookie name, lifetime, and security flags (Secure, HttpOnly, SameSite=lax) are configuration-driven. +**Gap:** No unit test verifies that `queueDeviceTrustCookie` passes the correct flags to `Cookie::queue`, that the lifetime calculation (`days × 24 × 60`) is right, or that `getCookieToken` returns `null` when no cookie is present. A misconfigured `$secure = true` hardcode already exists in the code and warrants explicit assertion. + +### 4. `EncryptCookies` Exclusion (Medium) + +**File:** `app/Http/Middleware/EncryptCookies.php` +**What it does:** Excludes the device-trust token from Laravel's cookie encryption layer so the raw token survives the round-trip. +**Gap:** No test asserts that `config('two_factor.cookie_name')` is in `$except`, so a future refactor that drops the constructor injection would silently encrypt the cookie and break device trust comparison in `DeviceTrustService` with no test failure. + +--- + +## Gaps — JavaScript Frontend + +All 15 frontend files introduced or substantially modified by this PR have no test coverage of any kind. + +### File Coverage Table + +| File | Lines | Category | Risk | Notes | +|---|---|---|---|---| +| `resources/js/login/login.js` | 1,000 | State machine / orchestrator | **Critical** | Core MFA flow controller: `handleAuthenticatePasswordOk` dispatches to `FLOW.MFA`; `handleMfaError` maps 401/412/429/0 to UI states; `resetToPasswordFlow`; `onVerify2FA`; `onVerifyRecovery`; `onResend2FA` | +| `resources/js/login/components/two_factor_form.js` | 149 | UI Component | **Critical** | Countdown timer with dual `useEffect` (expiry + cooldown), resend cooldown guard, expired-code state, trust-device checkbox | +| `resources/js/login/components/otp_input_form.js` | 117 | UI Component | **High** | OTP entry for email-verification flow; error display, submit guard | +| `resources/js/login/components/password_input_form.js` | 193 | UI Component | **High** | Password entry + show/hide; attempt-count error states; `data-testid` error label | +| `resources/js/login/components/recovery_code_form.js` | 84 | UI Component | **High** | Recovery code entry, empty-submit guard | +| `resources/js/login/actions.js` | 66 | API Layer | **High** | `verify2FA`, `resend2FA`, `verifyRecoveryCode`, `cancelLogin`, `authenticateWithPassword` — all XHR wrappers; URL sourced from `window.*_ENDPOINT` | +| `resources/js/base_actions.js` | 248 | API Layer | **High** | `postRawRequest` / `postRawRequestFull` — XHR transport, redirect-following, `responseURL` extraction; used by every action | +| `resources/js/login/components/email_input_form.js` | 61 | UI Component | **Medium** | Email entry step; `data-testid="error-label"` | +| `resources/js/login/components/email_error_actions.js` | 60 | UI Component | **Medium** | Unknown-email CTA display | +| `resources/js/login/components/existing_account_actions.js` | 47 | UI Component | **Medium** | Account-exists action set | +| `resources/js/login/components/help_links.js` | 78 | UI Component | **Medium** | Context-sensitive help links | +| `resources/js/login/constants.js` | 32 | Constants | **Low** | `FLOW`, `HTTP_CODES`, `MFA_ERROR_CODE` enum values | +| `resources/js/login/components/otp_help_links.js` | 20 | UI Component | **Low** | OTP-specific help link | +| `resources/js/login/components/third_party_identity_providers.js` | 36 | UI Component | **Low** | SSO provider list display | +| `resources/js/shared/HTMLRender.jsx` | 29 | Shared Utility | **Low** | DOMPurify wrapper; `...rest` prop forwarding | + +--- + +## Priority Recommendations + +| Priority | Item | Rationale | +|---|---|---| +| **Critical** | Unit tests for `login.js` state machine | `handleAuthenticatePasswordOk`, `handleMfaError`, `handleAuthenticateValidation`, and `resetToPasswordFlow` are pure state logic that can be tested without a DOM. These are the highest-value, lowest-effort tests — each branch covers a real user failure mode. | +| **Critical** | Jest component tests for `TwoFactorForm` | The countdown + cooldown dual-timer is the most complex UI logic in the PR. Timer behavior, expired-code state, and resend-button disabling are invisible in E2E tests but trivially verifiable with `@testing-library/react` + `jest.useFakeTimers`. | +| **Critical** | Dedicated integration test for `cancelLogin` | Covers the session-cleanup contract that is otherwise only exercised by the happy path. | +| **High** | Jest tests for `actions.js` and `base_actions.js` | Mock `window.*_ENDPOINT` and `superagent`; assert that `postRawRequestFull` extracts `responseURL` as `finalUrl`. These are the only XHR-level contracts between React and the PHP backend. | +| **High** | Playwright E2E: full MFA flow | `goes to 2FA step after password → enters code → logs in` and the expired-session regression. The scaffold (`tests/e2e/`) already exists. | +| **High** | `TwoFactorRateLimitMiddleware` unit tests | Isolated cache-mock tests for window expiry and per-action counter separation. | +| **Medium** | `MFACookieManager` unit tests | Assert cookie flag values. | +| **Medium** | Jest component tests: `RecoveryCodeForm`, `PasswordInputForm`, `OTPInputForm` | Error-display and empty-submit guard branches. | +| **Medium** | `EncryptCookies` exclusion assertion | One-line test: `assertContains(config('two_factor.cookie_name'), (new EncryptCookies(...))->getExcept())`. | +| **Low** | `constants.js` smoke test | Not worth dedicated tests; covered by any consumer test that imports the file. | + +--- + +## How to Run What Exists Today + +```bash +# PHP — all suites +./vendor/bin/phpunit + +# PHP — MFA suite only +./vendor/bin/phpunit --testsuite "Two Factor Authentication Test Suite" + +# PHP — integration suite only +./vendor/bin/phpunit tests/TwoFactorLoginFlowTest.php + +# JS — unit tests (Jest) +yarn test:unit:ci + +# E2E (requires Docker stack) +docker compose --profile e2e run --rm playwright npx playwright test tests/e2e/tests/auth/ +``` diff --git a/docker-compose.yml b/docker-compose.yml index a185686e..644780e1 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -57,6 +57,28 @@ services: networks: - idp-local-net env_file: ./.env + playwright: + build: + context: ./docker-compose/playwright + container_name: idp-playwright + working_dir: /var/www + volumes: + - ./:/var/www + - playwright_cache:/root/.cache/ms-playwright + # Lets the e2e suite `docker exec idp-app php artisan idp:get-latest-otp + # ` to read a real OTP value (see tests/e2e/utils/otp.ts) - + # grants this container full control of the host's Docker daemon, not + # just idp-app, so keep this service dev/e2e-only (profiles: [e2e]). + - /var/run/docker.sock:/var/run/docker.sock + networks: + - idp-local-net + depends_on: + - nginx + profiles: + - e2e + environment: + - APP_URL=http://nginx + nginx: image: nginx:alpine container_name: nginx-idp @@ -119,3 +141,4 @@ networks: volumes: mysql_idp: elasticsearch_data: + playwright_cache: diff --git a/docker-compose/playwright/Dockerfile b/docker-compose/playwright/Dockerfile new file mode 100644 index 00000000..14394765 --- /dev/null +++ b/docker-compose/playwright/Dockerfile @@ -0,0 +1,12 @@ +FROM mcr.microsoft.com/playwright:v1.61.1-jammy + +# Docker CLI only (no daemon) - lets the e2e suite shell out to +# `docker exec idp-app php artisan idp:get-latest-otp ` to read a +# real OTP value without adding DB/mail dependencies to the test runner. +# Requires /var/run/docker.sock to be mounted at runtime (docker-compose.yml). +ARG DOCKER_CLI_VERSION=27.3.1 +RUN apt-get update \ + && apt-get install -y --no-install-recommends ca-certificates curl \ + && curl -fsSL "https://download.docker.com/linux/static/stable/x86_64/docker-${DOCKER_CLI_VERSION}.tgz" \ + | tar -xz --strip-components=1 -C /usr/local/bin docker/docker \ + && rm -rf /var/lib/apt/lists/* diff --git a/jest.config.js b/jest.config.js new file mode 100644 index 00000000..2cb8907a --- /dev/null +++ b/jest.config.js @@ -0,0 +1,18 @@ +module.exports = { + testEnvironment: 'jsdom', + testMatch: ['/tests/js/**/*.test.js'], + // @marsidev/react-turnstile ships as pure ESM; allow Babel to transform it. + transformIgnorePatterns: ['/node_modules/(?!@marsidev/react-turnstile)'], + moduleNameMapper: { + '\\.(css|scss|sass|less)$': 'identity-obj-proxy', + '\\.(jpg|jpeg|png|gif|svg|ttf|woff|woff2|eot|otf|webp)$': + '/tests/js/__mocks__/fileMock.js', + }, + setupFilesAfterEnv: ['/tests/js/setup.js'], + transform: { + '^.+\\.[jt]sx?$': 'babel-jest', + }, + moduleDirectories: ['node_modules', 'resources/js'], + collectCoverageFrom: ['resources/js/**/*.{js,jsx}', '!resources/js/index.js'], + coverageDirectory: 'tests/js/coverage', +}; diff --git a/package.json b/package.json index 729fca9c..96e436f3 100644 --- a/package.json +++ b/package.json @@ -9,7 +9,12 @@ "build-dev": "./node_modules/.bin/webpack --config webpack.dev.js", "build": "./node_modules/.bin/webpack --config webpack.prod.js", "serve": "webpack-dev-server --open --port=8888 --server-type https --config webpack.dev.js", - "test": "jest --watch" + "test": "jest --watch", + "test:unit": "jest --testPathPattern=tests/js", + "test:unit:ci": "jest --testPathPattern=tests/js --ci --coverage", + "test:e2e": "playwright test", + "test:e2e:ui": "playwright test --ui", + "test:e2e:report": "playwright show-report tests/e2e/report" }, "devDependencies": { "@babel/core": "^7.17.8", @@ -21,6 +26,10 @@ "@babel/preset-flow": "^7.7.4", "@babel/preset-react": "^7.7.4", "@babel/runtime": "^7.20.7", + "@playwright/test": "^1.61.1", + "@testing-library/user-event": "^13", + "@testing-library/jest-dom": "^5.16.5", + "@testing-library/react": "^12.1.5", "babel-cli": "^6.26.0", "babel-jest": "^26.6.3", "babel-loader": "^8.2.4", diff --git a/phpunit.xml b/phpunit.xml index 5d09a0bb..1f73569a 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -25,11 +25,11 @@ ./tests/TwoFactorRepositoriesTest.php ./tests/unit/UserTwoFactorTest.php - ./tests/Unit/MFA/AbstractMFAChallengeStrategyTest.php - ./tests/Unit/MFA/EmailOTPMFAChallengeStrategyTest.php - ./tests/Unit/MFA/MFAChallengeStrategyFactoryTest.php - ./tests/Unit/TwoFactorAuditServiceTest.php - ./tests/Unit/MFAGateServiceTest.php + ./tests/unit/MFA/AbstractMFAChallengeStrategyTest.php + ./tests/unit/MFA/EmailOTPMFAChallengeStrategyTest.php + ./tests/unit/MFA/MFAChallengeStrategyFactoryTest.php + ./tests/unit/TwoFactorAuditServiceTest.php + ./tests/unit/MFAGateServiceTest.php ./tests/TwoFactorLoginFlowTest.php diff --git a/playwright.config.ts b/playwright.config.ts new file mode 100644 index 00000000..0df15725 --- /dev/null +++ b/playwright.config.ts @@ -0,0 +1,22 @@ +import { defineConfig, devices } from '@playwright/test'; + +export default defineConfig({ + testDir: './tests/e2e/tests', + fullyParallel: true, + forbidOnly: !!process.env.CI, + retries: process.env.CI ? 2 : 0, + workers: process.env.CI ? parseInt(process.env.PLAYWRIGHT_WORKERS ?? '1') : undefined, + reporter: [['html', { outputFolder: 'tests/e2e/report' }]], + use: { + baseURL: process.env.APP_URL || 'http://localhost:8001', + trace: 'on-first-retry', + screenshot: 'only-on-failure', + video: 'retain-on-failure', + }, + projects: [ + { + name: 'chromium', + use: { ...devices['Desktop Chrome'] }, + }, + ], +}); diff --git a/readme.md b/readme.md index 26b145d9..8df45030 100644 --- a/readme.md +++ b/readme.md @@ -79,10 +79,47 @@ nvm use # Tests +## Backend (PHPUnit) + +```bash php artisan view:clear php artisan cache:clear - ./vendor/bin/phpunit +``` + +## Frontend — Unit/Component (Jest) + +Run from inside the `idp-app` container: + +```bash +yarn test:unit # watch mode +yarn test:unit:ci # single run with coverage +``` + +## Frontend — E2E (Playwright) + +Run from the **host** (outside any container). The full stack must be running (`./start_local_server.sh`). + +```bash +# Run all E2E tests +docker compose --profile e2e run --rm playwright npx playwright test + +# Run a specific file +docker compose --profile e2e run --rm playwright npx playwright test tests/e2e/tests/auth/login.spec.ts +``` + +> E2E tests cannot be run from inside the `idp-app` container — it has no browser. +> The `playwright` service (`mcr.microsoft.com/playwright:v1.61.1-jammy`) includes Chromium and all required system dependencies. + +### Viewing the HTML report + +The report is written to `tests/e2e/report/` on the host. Serve it from the host (not from inside any container) so the browser can reach it: + +```bash +nvm use 22.2.0 +yarn test:e2e:report +# Open http://localhost:9323 +``` # install docker compose diff --git a/resources/js/base_actions.js b/resources/js/base_actions.js index 18cd3b49..e6c1ea06 100644 --- a/resources/js/base_actions.js +++ b/resources/js/base_actions.js @@ -92,6 +92,30 @@ export const postRawRequest = (endpoint) => (params, headers = {}) => { }) } +export const postRawRequestFull = (endpoint) => (params, headers = {}) => { + let url = URI(endpoint); + + let key = url.toString(); + + cancel(key); + + let req = http.post(url.toString()); + + schedule(key, req); + + return req.set(headers).send(params).timeout({ + response: 60000, + deadline: 60000, + }).then((res) => { + let json = res.body; + end(key); + return Promise.resolve({response: json}); + }).catch((error) => { + end(key); + return Promise.reject(error); + }) +} + export const putRawRequest = (endpoint) => (payload = null, params={}, headers = {}) => { let url = URI(endpoint); diff --git a/resources/js/components/recovery_code_display.js b/resources/js/components/recovery_code_display.js new file mode 100644 index 00000000..08f5192a --- /dev/null +++ b/resources/js/components/recovery_code_display.js @@ -0,0 +1,76 @@ +import React, {useState} from "react"; +import Box from "@material-ui/core/Box"; +import Button from "@material-ui/core/Button"; +import Grid from "@material-ui/core/Grid"; +import Typography from "@material-ui/core/Typography"; +import AssignmentIcon from "@material-ui/icons/Assignment"; +import CheckCircleIcon from "@material-ui/icons/CheckCircle"; +import {downloadTextFile} from "../utils"; + +import styles from "./recovery_codes.module.scss"; + +const DEFAULT_APP_NAME = "OpenStackID"; + +const buildFileContent = (codes, email, appName) => { + const date = new Date().toISOString().slice(0, 10); + return [ + `${appName} Recovery Codes`, + `Generated: ${date}`, + `Account: ${email}`, + "", + "Keep these codes somewhere safe. Each code can only be used once to sign in, and they will not be shown again.", + "", + ...codes, + ].join("\n"); +}; + +const RecoveryCodeDisplay = ({codes, email, appName = DEFAULT_APP_NAME}) => { + const [copied, setCopied] = useState(false); + + const handleCopy = () => { + navigator.clipboard.writeText(codes.join("\n")).then(() => { + setCopied(true); + setTimeout(() => setCopied(false), 2000); + }); + }; + + const handleDownload = () => { + const date = new Date().toISOString().slice(0, 10); + const appSlug = appName.toLowerCase().replace(/[^a-z0-9]+/g, "-").replace(/^-|-$/g, ""); + downloadTextFile(`${appSlug}-recovery-codes-${date}.txt`, buildFileContent(codes, email, appName)); + }; + + return ( + + + {codes.map((code, idx) => ( + + {code} + + ))} + + + +   + + + + ); +}; + +export default RecoveryCodeDisplay; diff --git a/resources/js/components/recovery_code_modal.js b/resources/js/components/recovery_code_modal.js new file mode 100644 index 00000000..f0a09c01 --- /dev/null +++ b/resources/js/components/recovery_code_modal.js @@ -0,0 +1,64 @@ +import React, {useEffect, useState} from "react"; +import Box from "@material-ui/core/Box"; +import Button from "@material-ui/core/Button"; +import Dialog from "@material-ui/core/Dialog"; +import DialogActions from "@material-ui/core/DialogActions"; +import DialogContent from "@material-ui/core/DialogContent"; +import DialogTitle from "@material-ui/core/DialogTitle"; +import Typography from "@material-ui/core/Typography"; +import WarningRoundedIcon from "@material-ui/icons/WarningRounded"; +import RecoveryCodeDisplay from "./recovery_code_display"; + +import styles from "./recovery_codes.module.scss"; + +const ACK_DELAY_SECONDS = 5; + +const RecoveryCodeModal = ({open, codes, email, appName, onAcknowledge}) => { + const [secondsLeft, setSecondsLeft] = useState(ACK_DELAY_SECONDS); + + useEffect(() => { + if (!open) return undefined; + + setSecondsLeft(ACK_DELAY_SECONDS); + const interval = setInterval(() => { + setSecondsLeft((prev) => (prev > 0 ? prev - 1 : 0)); + }, 1000); + + return () => clearInterval(interval); + }, [open]); + + return ( + + Save Your Recovery Codes + + + + + These codes will not be shown again. Copy or download them now and store them somewhere safe. + + + {codes && } + + + + + + ); +}; + +export default RecoveryCodeModal; diff --git a/resources/js/components/recovery_codes.module.scss b/resources/js/components/recovery_codes.module.scss new file mode 100644 index 00000000..c4437a1e --- /dev/null +++ b/resources/js/components/recovery_codes.module.scss @@ -0,0 +1,43 @@ +.recovery_codes_panel { + margin-top: 15px; +} + +.codes_grid { + margin: 8px 0; + padding: 12px; + background-color: #f5f5f5; + border-radius: 4px; +} + +.code { + font-family: monospace; + font-size: 1rem; + letter-spacing: 1px; +} + +.warning_banner { + display: flex; + align-items: flex-start; + margin-bottom: 16px; + padding: 10px 14px; + background-color: #fdecea; + border-left: 4px solid #f44336; + border-radius: 4px; +} + +.warning_icon { + color: #f44336; + margin-right: 10px; + margin-top: 1px; + flex-shrink: 0; +} + +.low_code_warning { + display: flex; + align-items: center; + justify-content: space-between; + margin-top: 8px; + padding: 8px 12px; + background-color: #fff3e0; + border-radius: 4px; +} diff --git a/resources/js/components/recovery_codes_panel.js b/resources/js/components/recovery_codes_panel.js new file mode 100644 index 00000000..986c70f7 --- /dev/null +++ b/resources/js/components/recovery_codes_panel.js @@ -0,0 +1,144 @@ +import React, {useState} from "react"; +import Box from "@material-ui/core/Box"; +import Button from "@material-ui/core/Button"; +import Grid from "@material-ui/core/Grid"; +import IconButton from "@material-ui/core/IconButton"; +import Link from "@material-ui/core/Link"; +import TextField from "@material-ui/core/TextField"; +import Typography from "@material-ui/core/Typography"; +import CloseIcon from "@material-ui/icons/Close"; +import {regenerateRecoveryCodes} from "../profile/actions"; +import {handleErrorResponse} from "../utils"; +import RecoveryCodeModal from "./recovery_code_modal"; +import { + RECOVERY_CODES_LOW_WARNING_DISMISSED_KEY, + DEFAULT_RECOVERY_CODES_LOW_THRESHOLD, +} from "../shared/recovery_codes"; + +import styles from "./recovery_codes.module.scss"; + +const RecoveryCodesPanel = ({ + recoveryCodesRemaining, + recoveryCodesTotal, + lowCodeThreshold = DEFAULT_RECOVERY_CODES_LOW_THRESHOLD, + email, + appName, + initialCodes = null + }) => { + const [regenerating, setRegenerating] = useState(false); + const [currentPassword, setCurrentPassword] = useState(""); + const [loading, setLoading] = useState(false); + const [remaining, setRemaining] = useState(recoveryCodesRemaining); + const [total, setTotal] = useState(recoveryCodesTotal); + const [codes, setCodes] = useState(initialCodes); + const [warningDismissed, setWarningDismissed] = useState( + sessionStorage.getItem(RECOVERY_CODES_LOW_WARNING_DISMISSED_KEY) === "1" + ); + + const handleRegenerate = () => { + setLoading(true); + regenerateRecoveryCodes(currentPassword).then(({response}) => { + setLoading(false); + setRegenerating(false); + setCurrentPassword(""); + setCodes(response.recovery_codes); + setRemaining(response.recovery_codes.length); + setTotal(response.recovery_codes.length); + }).catch((err) => { + setLoading(false); + handleErrorResponse(err); + }); + }; + + const handleAcknowledge = () => { + setCodes(null); + }; + + const dismissLowCodeWarning = () => { + sessionStorage.setItem(RECOVERY_CODES_LOW_WARNING_DISMISSED_KEY, "1"); + setWarningDismissed(true); + }; + + const showLowCodeWarning = !warningDismissed && remaining < lowCodeThreshold; + + return ( + <> + + + Recovery Codes: {remaining} of {total} remaining + + { + showLowCodeWarning && + + + You're running low on recovery codes. Regenerate them to avoid getting locked out. + + + + + + } + { + !regenerating && + { + e.preventDefault(); + setRegenerating(true); + }}> + Regenerate Codes + + } + { + regenerating && + + setCurrentPassword(e.target.value)} + onKeyDown={(e) => { + // This panel lives inside the profile page's own
; + // it must not let Enter bubble up and submit that form too. + if (e.key === "Enter") { + e.preventDefault(); + if (currentPassword && !loading) handleRegenerate(); + } + }} + // Detaches this input from the ancestor (the profile page + // wraps everything in one big form) so the browser's native + // "Enter submits the enclosing form" / password-manager-driven + // auto-submit can never fire a GET on it, regardless of the + // onKeyDown handler above. + inputProps={{form: "recovery-codes-detached-form", autoComplete: "off"}} + data-testid="recovery-codes-current-password" + /> +   + +   + { + e.preventDefault(); + setRegenerating(false); + setCurrentPassword(""); + }}> + Cancel + + + } + + + + ); +}; + +export default RecoveryCodesPanel; diff --git a/resources/js/components/two_factor_section.js b/resources/js/components/two_factor_section.js new file mode 100644 index 00000000..ece17eff --- /dev/null +++ b/resources/js/components/two_factor_section.js @@ -0,0 +1,68 @@ +import React, {useState} from "react"; +import Button from "@material-ui/core/Button"; +import Typography from "@material-ui/core/Typography"; +import {enableTwoFactor} from "../profile/actions"; +import {handleErrorResponse} from "../utils"; +import RecoveryCodesPanel from "./recovery_codes_panel"; + +const DEFAULT_METHOD = "email_otp"; + +const TwoFactorSection = ({ + twoFactorEnabled, + recoveryCodesRemaining, + recoveryCodesTotal, + recoveryCodesLowThreshold, + email, + appName + }) => { + const [enabled, setEnabled] = useState(twoFactorEnabled); + const [loading, setLoading] = useState(false); + const [remaining, setRemaining] = useState(recoveryCodesRemaining); + const [total, setTotal] = useState(recoveryCodesTotal); + const [enrollmentCodes, setEnrollmentCodes] = useState(null); + + const handleEnable = () => { + setLoading(true); + enableTwoFactor(DEFAULT_METHOD).then(({response}) => { + setLoading(false); + setRemaining(response.recovery_codes.length); + setTotal(response.recovery_codes.length); + setEnrollmentCodes(response.recovery_codes); + setEnabled(true); + }).catch((err) => { + setLoading(false); + handleErrorResponse(err); + }); + }; + + if (!enabled) { + return ( + <> + + Two-factor authentication is not enabled for your account. + + + + ); + } + + return ( + + ); +}; + +export default TwoFactorSection; diff --git a/resources/js/login/components/email_input_form.js b/resources/js/login/components/email_input_form.js index ab26ff47..d074bb23 100644 --- a/resources/js/login/components/email_input_form.js +++ b/resources/js/login/components/email_input_form.js @@ -50,7 +50,7 @@ const EmailInputForm = ({ )} {emailError != "" && ( - + {emailError} )} diff --git a/resources/js/login/components/otp_code_input.js b/resources/js/login/components/otp_code_input.js index 55c8e77b..cb5cbfc3 100644 --- a/resources/js/login/components/otp_code_input.js +++ b/resources/js/login/components/otp_code_input.js @@ -38,7 +38,7 @@ const OtpCodeInput = ({ /> {otpError && - + {otpError} } diff --git a/resources/js/login/components/password_input_form.js b/resources/js/login/components/password_input_form.js index f89786b9..96e5a1cc 100644 --- a/resources/js/login/components/password_input_form.js +++ b/resources/js/login/components/password_input_form.js @@ -56,7 +56,7 @@ const PasswordInputForm = ({ if (attempts > 0 && attempts < maxAttempts && userIsActive) { return ( -

+

Incorrect password. You have {attemptsLeft} more attempt {attemptsLeft !== 1 ? "s" : ""} before your account is locked.

@@ -65,7 +65,7 @@ const PasswordInputForm = ({ if (attempts > 0 && attempts === maxAttempts && userIsActive) { return ( -

+

Incorrect password. You have reached the maximum ({maxAttempts}) login attempts. Your account will be locked after another failed login. @@ -75,7 +75,7 @@ const PasswordInputForm = ({ if (attempts > 0 && attempts === maxAttempts && !userIsActive) { return ( -

+

Your account has been locked due to multiple failed login attempts. Please contact support to unlock it. @@ -84,7 +84,7 @@ const PasswordInputForm = ({ } return ( - + {passwordError} ); diff --git a/resources/js/login/components/recovery_code_form.js b/resources/js/login/components/recovery_code_form.js index 49ace5f6..50830e04 100644 --- a/resources/js/login/components/recovery_code_form.js +++ b/resources/js/login/components/recovery_code_form.js @@ -31,7 +31,7 @@ const RecoveryCodeForm = ({ }; return ( - +

Enter a recovery code

Enter one of the recovery codes you saved when you enabled two-step verification. @@ -52,7 +52,7 @@ const RecoveryCodeForm = ({ error={!!recoveryError} /> {recoveryError && ( - + {recoveryError} )} @@ -61,7 +61,8 @@ const RecoveryCodeForm = ({ disabled={disableInput || recoveryCode === ''} color="primary" type="submit" - target="_self"> + target="_self" + data-testid="verify-button"> VERIFY @@ -72,7 +73,7 @@ const RecoveryCodeForm = ({ Back to verification code {" · "} - + Cancel diff --git a/resources/js/login/components/two_factor_form.js b/resources/js/login/components/two_factor_form.js index 762f34c5..c06ff08e 100644 --- a/resources/js/login/components/two_factor_form.js +++ b/resources/js/login/components/two_factor_form.js @@ -64,7 +64,7 @@ const TwoFactorForm = ({ }; return ( - + + target="_self" + data-testid="verify-button"> VERIFY @@ -103,17 +104,18 @@ const TwoFactorForm = ({

Didn't receive it? Check your spam folder or{" "} 0 || disableInput) ? styles.disabled_link : ''}> + className={(cooldown > 0 || disableInput) ? styles.disabled_link : ''} + data-testid="resend-link"> {cooldown > 0 ? `resend code (${cooldown}s)` : 'resend code'} .

{/* "Use a different method" is intentionally hidden in Phase I (email_otp only). */}
- + Cancel - + Use a recovery code instead
diff --git a/resources/js/login/login.js b/resources/js/login/login.js index f04bd4e2..3aa2100c 100644 --- a/resources/js/login/login.js +++ b/resources/js/login/login.js @@ -31,8 +31,13 @@ import EmailErrorActions from "./components/email_error_actions"; import ThirdPartyIdentityProviders from "./components/third_party_identity_providers"; import TwoFactorForm from "./components/two_factor_form"; import RecoveryCodeForm from "./components/recovery_code_form"; +import { + RECOVERY_CODES_LOW_WARNING_DISMISSED_KEY, + DEFAULT_RECOVERY_CODES_LOW_THRESHOLD, +} from "../shared/recovery_codes"; import styles from "./login.module.scss"; +import recoveryCodesStyles from "../components/recovery_codes.module.scss"; import "./third_party_identity_providers.scss"; import { FLOW, @@ -93,6 +98,11 @@ class LoginPage extends React.Component { // change writes otp_lifetime atomically with flow, so props.otpLifetime // is only ever missing when there's no pending OTP to show a countdown for. passwordlessLifetime: props.otpLifetime ?? null, + // Set once a recovery-code login succeeds with a low remaining count, so + // the redirect can be held until the user acknowledges the warning - + // this is the only point where the SPA still controls the page (see + // onVerifyRecovery()/onContinueAfterLowRecoveryCodes() below). + lowRecoveryCodesWarning: null, }; if (props.authError != "" && !this.state.user_fullname) { @@ -130,6 +140,7 @@ class LoginPage extends React.Component { this.onVerify2FA = this.onVerify2FA.bind(this); this.onResend2FA = this.onResend2FA.bind(this); this.onVerifyRecovery = this.onVerifyRecovery.bind(this); + this.onContinueAfterLowRecoveryCodes = this.onContinueAfterLowRecoveryCodes.bind(this); this.onUseRecovery = this.onUseRecovery.bind(this); this.onBackToOtp = this.onBackToOtp.bind(this); this.resetToPasswordFlow = this.resetToPasswordFlow.bind(this); @@ -318,9 +329,13 @@ class LoginPage extends React.Component { onRecoveryCodeChange(ev) { let { value } = ev.target; + // Recovery codes are generated and hashed without the "-" separator; it is + // added only for on-screen readability (XXXX-XXXX). Strip any non-alphanumeric + // characters here so a code typed or pasted exactly as displayed still matches. + const normalized = value.replace(/[^A-Za-z0-9]/g, "").toUpperCase(); this.setState({ ...this.state, - recoveryCode: value, + recoveryCode: normalized, errors: { ...this.state.errors, recovery: "" }, }); } @@ -536,10 +551,26 @@ class LoginPage extends React.Component { verifyRecoveryCode(recoveryCode, this.props.token).then( (payload) => { - // See onVerify2FA() for rationale. const { response } = payload; - window.location.href = + const redirectUrl = (response && response.redirect_url) || window.location.href; + const remaining = response && response.recovery_codes_remaining; + const threshold = + (response && response.recovery_codes_low_threshold) ?? + DEFAULT_RECOVERY_CODES_LOW_THRESHOLD; + const alreadyDismissed = + sessionStorage.getItem(RECOVERY_CODES_LOW_WARNING_DISMISSED_KEY) === "1"; + + if (typeof remaining === "number" && remaining < threshold && !alreadyDismissed) { + this.setState({ + ...this.state, + lowRecoveryCodesWarning: { remaining, redirectUrl }, + }); + return; + } + + // See onVerify2FA() for rationale on using a real top-level navigation. + window.location.href = redirectUrl; }, (error) => { this.handleMfaError(error, "recovery"); @@ -547,6 +578,11 @@ class LoginPage extends React.Component { ); } + onContinueAfterLowRecoveryCodes() { + sessionStorage.setItem(RECOVERY_CODES_LOW_WARNING_DISMISSED_KEY, "1"); + window.location.href = this.state.lowRecoveryCodesWarning.redirectUrl; + } + onUseRecovery() { this.setState({ ...this.state, @@ -829,7 +865,7 @@ class LoginPage extends React.Component { onVerify={this.onVerify2FA} /> )} - {showRecoveryForm && ( + {showRecoveryForm && !this.state.lowRecoveryCodesWarning && ( )} + {showRecoveryForm && this.state.lowRecoveryCodesWarning && ( +
+ + You have {this.state.lowRecoveryCodesWarning.remaining} recovery + code{this.state.lowRecoveryCodesWarning.remaining === 1 ? "" : "s"} left. + Regenerate them from your profile after signing in to avoid getting + locked out. + + +
+ )} {isPasswordFlow && ( // proceed to ask for password ( 2nd step ) - <> +
- +
)} {isOtpFlow && ( // proceed to ask for password ( 2nd step ) @@ -1000,9 +1057,14 @@ const theme = createTheme({ }, }); -ReactDOM.render( - - - , - document.querySelector("#root"), -); +export { LoginPage }; + +const root = document.querySelector("#root"); +if (root) { + ReactDOM.render( + + + , + root, + ); +} diff --git a/resources/js/profile/actions.js b/resources/js/profile/actions.js index c1c6c2f0..0c9ea422 100644 --- a/resources/js/profile/actions.js +++ b/resources/js/profile/actions.js @@ -1,4 +1,4 @@ -import {deleteRawRequest, getRawRequest, putFile, putRawRequest} from "../base_actions"; +import {deleteRawRequest, getRawRequest, postRawRequestFull, putFile, putRawRequest} from "../base_actions"; import moment from "moment"; export const PAGE_SIZE = 10; @@ -87,6 +87,16 @@ export const revokeAllTokens = async () => { return deleteRawRequest(window.REVOKE_ALL_TOKENS_ENDPOINT)({'X-CSRF-TOKEN': window.CSFR_TOKEN}); } +export const regenerateRecoveryCodes = async (currentPassword) => { + const params = {current_password: currentPassword}; + return postRawRequestFull(window.REGENERATE_RECOVERY_CODES_ENDPOINT)(params, {'X-CSRF-TOKEN': window.CSFR_TOKEN}); +} + +export const enableTwoFactor = async (method) => { + const params = {method}; + return postRawRequestFull(window.ENABLE_TWO_FACTOR_ENDPOINT)(params, {'X-CSRF-TOKEN': window.CSFR_TOKEN}); +} + const normalizeEntity = (entity) => { entity.public_profile_show_photo = entity.public_profile_show_photo ? 1 : 0; entity.public_profile_show_fullname = entity.public_profile_show_fullname ? 1 : 0; diff --git a/resources/js/profile/profile.js b/resources/js/profile/profile.js index a530d04a..878f86a5 100644 --- a/resources/js/profile/profile.js +++ b/resources/js/profile/profile.js @@ -1,5 +1,6 @@ import React, {useState} from "react"; import ReactDOM from "react-dom"; +import Box from "@material-ui/core/Box"; import Button from "@material-ui/core/Button"; import Card from "@material-ui/core/Card"; import CardContent from "@material-ui/core/CardContent"; @@ -26,6 +27,7 @@ import Navbar from "../components/navbar/navbar"; import Divider from "@material-ui/core/Divider"; import Link from "@material-ui/core/Link"; import PasswordChangePanel from "../components/password_change_panel"; +import TwoFactorSection from "../components/two_factor_section"; import LoadingIndicator from "../components/loading_indicator"; import TopLogo from "../components/top_logo/top_logo"; import {handleErrorResponse} from "../utils"; @@ -35,13 +37,18 @@ import styles from "./profile.module.scss"; const ProfilePage = ({ appLogo, + appName, countries, csrfToken, initialValues, languages, menuConfig, passwordPolicy, - redirectUri + redirectUri, + twoFactorEnabled, + recoveryCodesRemaining, + recoveryCodesTotal, + recoveryCodesLowThreshold }) => { const [pic, setPic] = useState(null); const [loading, setLoading] = useState(false); @@ -754,11 +761,27 @@ const ProfilePage = ({ )} - - + + + + + + + Two-Factor Authentication + + + + { +const HTMLRender = ({ children, className, style, component = "div", ...rest }) => { const html = DOMPurify.sanitize(children || ""); const Component = component; @@ -12,6 +12,7 @@ const HTMLRender = ({ children, className, style, component = "div" }) => { style={style} className={className} dangerouslySetInnerHTML={{ __html: html }} + {...rest} /> ); }; diff --git a/resources/js/shared/recovery_codes.js b/resources/js/shared/recovery_codes.js new file mode 100644 index 00000000..c144f52b --- /dev/null +++ b/resources/js/shared/recovery_codes.js @@ -0,0 +1,5 @@ +// Shared between the profile page's RecoveryCodesPanel and the login page's +// post-MFA-recovery-login warning, so dismissing the low-code warning in +// either place suppresses it everywhere else for the rest of the session. +export const RECOVERY_CODES_LOW_WARNING_DISMISSED_KEY = "recovery_codes_low_warning_dismissed"; +export const DEFAULT_RECOVERY_CODES_LOW_THRESHOLD = 3; diff --git a/resources/js/signup/signup.js b/resources/js/signup/signup.js index 1d595e2a..93a98b2f 100644 --- a/resources/js/signup/signup.js +++ b/resources/js/signup/signup.js @@ -84,10 +84,12 @@ const SignUpPage = ({ return errors; }, onSubmit: (values) => { - const turnstileResponse = captcha.current?.getResponse(); - if (!turnstileResponse) { - setCaptchaConfirmation("Remember to check the captcha"); - return; + if (captchaPublicKey) { + const turnstileResponse = captcha.current?.getResponse(); + if (!turnstileResponse) { + setCaptchaConfirmation("Remember to check the captcha"); + return; + } } doHtmlFormPost(); }, diff --git a/resources/js/utils.js b/resources/js/utils.js index 7b35a202..5d5d711c 100644 --- a/resources/js/utils.js +++ b/resources/js/utils.js @@ -82,6 +82,18 @@ export const formatTime = (timeInSeconds) => { return res; } +export const downloadTextFile = (filename, content) => { + const blob = new Blob([content], {type: 'text/plain'}); + const url = URL.createObjectURL(blob); + const link = document.createElement('a'); + link.href = url; + link.download = filename; + document.body.appendChild(link); + link.click(); + document.body.removeChild(link); + URL.revokeObjectURL(url); +}; + export const decodeHtmlEntities = (text) => { const textarea = document.createElement('textarea'); textarea.innerHTML = text; diff --git a/resources/views/profile.blade.php b/resources/views/profile.blade.php index 335094e7..672081ba 100644 --- a/resources/views/profile.blade.php +++ b/resources/views/profile.blade.php @@ -103,7 +103,11 @@ initialValues: initialValues, languages: languages, menuConfig: menuConfig, - passwordPolicy: passwordPolicy + passwordPolicy: passwordPolicy, + twoFactorEnabled: {{ $two_factor_enabled ? 'true' : 'false' }}, + recoveryCodesRemaining: {{ (int) $recovery_codes_remaining }}, + recoveryCodesTotal: {{ (int) $recovery_codes_total }}, + recoveryCodesLowThreshold: {{ (int) $recovery_codes_low_threshold }} } window.GET_USER_ACTIONS_ENDPOINT = '{{URL::action("Api\UserActionApiController@getActionsByCurrentUser")}}'; @@ -112,6 +116,8 @@ window.REVOKE_ALL_TOKENS_ENDPOINT = '{{URL::action("Api\UserApiController@revokeAllMyTokens")}}'; window.SAVE_PROFILE_ENDPOINT = '{!!URL::action("Api\UserApiController@updateMe")!!}'; window.SAVE_PIC_ENDPOINT = '{!!URL::action("Api\UserApiController@updateMyPic")!!}'; + window.REGENERATE_RECOVERY_CODES_ENDPOINT = '{!!URL::action("Api\UserApiController@regenerateRecoveryCodes")!!}'; + window.ENABLE_TWO_FACTOR_ENDPOINT = '{!!URL::action("Api\UserApiController@enableTwoFactor")!!}'; window.CSFR_TOKEN = document.head.querySelector('meta[name="csrf-token"]').content; {!! script_to('assets/profile.js') !!} diff --git a/routes/web.php b/routes/web.php index ecbeda35..61d97573 100644 --- a/routes/web.php +++ b/routes/web.php @@ -197,6 +197,8 @@ Route::put('', "UserApiController@updateMe"); Route::put('pic', "UserApiController@updateMyPic"); Route::get('actions', "UserActionApiController@getActionsByCurrentUser"); + Route::post('recovery-codes/regenerate', "UserApiController@regenerateRecoveryCodes"); + Route::post('2fa/enable', "UserApiController@enableTwoFactor"); }); Route::get('access-tokens', ['middleware' => ['openstackid.currentuser.serveradmin.json'], 'uses' => 'ClientApiController@getAllAccessTokens']); diff --git a/start_local_server.sh b/start_local_server.sh index 0535f4b8..62d66591 100755 --- a/start_local_server.sh +++ b/start_local_server.sh @@ -2,11 +2,31 @@ set -e export DOCKER_SCAN_SUGGEST=false -docker compose run --rm app composer install -docker compose run --rm app php artisan doctrine:migrations:migrate --no-interaction -docker compose run --rm app php artisan db:seed --force -docker compose run --rm app php artisan idp:create-super-admin test@test.com 1Qaz2wsx! +# Install PHP deps without running post-autoload scripts (package:discover +# boots Laravel which triggers the OTEL exporter flush — hangs if the +# collector isn't up yet). +docker compose run --rm app composer install --no-scripts + +# JS deps and build don't involve artisan, safe to run before the full stack. docker compose run --rm app yarn install docker compose run --rm app yarn build + +# Bring up the full stack so the OTEL collector is reachable before any +# artisan command runs. docker compose up -d -docker compose exec app /bin/bash \ No newline at end of file + +echo "Waiting for app container to be ready..." +until docker compose exec app true 2>/dev/null; do sleep 1; done + +# Now run artisan commands with every service available. +docker compose exec app php artisan package:discover --ansi +docker compose exec app php artisan doctrine:migrations:migrate --no-interaction +docker compose exec app php artisan db:seed --force +docker compose exec app php artisan idp:create-super-admin test@test.com 1Qaz2wsx! +docker compose exec app php artisan idp:create-raw-user e2e@test.com 1Qaz2wsx! + +# Install Playwright Chromium into the named volume (skipped automatically if +# already cached from a previous run). +docker compose --profile e2e run --rm playwright npx playwright install chromium + +docker compose exec app /bin/bash diff --git a/tests/RecoveryCodeRegenerationTest.php b/tests/RecoveryCodeRegenerationTest.php new file mode 100644 index 00000000..cec67de8 --- /dev/null +++ b/tests/RecoveryCodeRegenerationTest.php @@ -0,0 +1,237 @@ +withoutMiddleware(); + $this->be($this->admin()); + Session::start(); + } + + public function testRegenerateWithCorrectPasswordInvalidatesOldCodesAndReturnsNewOnes(): void + { + $admin = $this->admin(); + $this->createRecoveryCode($admin, 'OLD-CODE-' . uniqid(), false); + + $response = $this->regenerate(self::SEED_PASSWORD); + + $this->assertResponseStatus(200); + $payload = json_decode($response->getContent(), true); + $this->assertArrayHasKey('recovery_codes', $payload); + + $expectedCount = (int)config('auth.recovery_codes.count', 10); + $this->assertCount($expectedCount, $payload['recovery_codes']); + foreach ($payload['recovery_codes'] as $code) { + $this->assertMatchesRegularExpression('/^[A-Z0-9]+-[A-Z0-9]+$/', $code); + } + + EntityManager::clear(); + $remaining = EntityManager::getRepository(UserRecoveryCode::class) + ->findBy(['user' => $admin->getId(), 'used_at' => null]); + $this->assertCount( + $expectedCount, + $remaining, + 'old codes must be invalidated and replaced by exactly the configured count' + ); + } + + public function testRegenerateWithWrongPasswordFailsAndDoesNotTouchExistingCodes(): void + { + $admin = $this->admin(); + $plain = 'KEEP-ME-' . uniqid(); + $this->createRecoveryCode($admin, $plain, false); + + $response = $this->regenerate('this-is-not-the-password'); + + $this->assertResponseStatus(412); + + EntityManager::clear(); + $remaining = EntityManager::getRepository(UserRecoveryCode::class) + ->findBy(['user' => $admin->getId(), 'used_at' => null]); + $this->assertNotEmpty($remaining, 'existing codes must not be touched when the password confirmation fails'); + } + + public function testRegenerateRequiresCurrentPassword(): void + { + $response = $this->action('POST', 'Api\\UserApiController@regenerateRecoveryCodes', [], [], [], []); + + $this->assertResponseStatus(412); + } + + public function testRegenerateLogsAuditEvent(): void + { + $admin = $this->admin(); + + $this->regenerate(self::SEED_PASSWORD); + + EntityManager::clear(); + $entries = EntityManager::getRepository(TwoFactorAuditLog::class) + ->findBy(['user' => $admin->getId(), 'event_type' => TwoFactorAuditLog::EventRecoveryCodesGenerated]); + $this->assertNotEmpty($entries, 'a recovery_codes_generated audit entry must be recorded'); + } + + public function testEnableTwoFactorGeneratesRecoveryCodes(): void + { + $admin = $this->admin(); + + $response = $this->enableTwoFactor('email_otp'); + + $this->assertResponseStatus(200); + $payload = json_decode($response->getContent(), true); + $this->assertArrayHasKey('recovery_codes', $payload); + + $expectedCount = (int)config('auth.recovery_codes.count', 10); + $this->assertCount($expectedCount, $payload['recovery_codes']); + + EntityManager::clear(); + $reloaded = EntityManager::getRepository(User::class)->find($admin->getId()); + $this->assertTrue($reloaded->isTwoFactorEnabled(), '2FA must be enabled on the user after enrollment'); + + $remaining = EntityManager::getRepository(UserRecoveryCode::class) + ->findBy(['user' => $admin->getId(), 'used_at' => null]); + $this->assertCount($expectedCount, $remaining); + } + + public function testDisplayedRecoveryCodeRedeemsThroughVerifyRecoveryCode(): void + { + $admin = $this->admin(); + + $response = $this->regenerate(self::SEED_PASSWORD); + $this->assertResponseStatus(200); + $payload = json_decode($response->getContent(), true); + $displayedCode = $payload['recovery_codes'][0]; + $this->assertMatchesRegularExpression('/^[A-Z0-9]+-[A-Z0-9]+$/', $displayedCode); + + EntityManager::clear(); + $admin = EntityManager::getRepository(User::class)->find($admin->getId()); + $unusedBefore = EntityManager::getRepository(UserRecoveryCode::class) + ->findBy(['user' => $admin->getId(), 'used_at' => null]); + + // The hash was generated over the dash-less string; redeeming the code + // exactly as it was displayed (with its "-" separator) must still work. + // Goes through IAuthService, like the real login flow, because + // verifyRecoveryCode() takes a PESSIMISTIC_WRITE row lock that requires + // an open transaction. + $strategy = MFAChallengeStrategyFactory::create(User::MFAMethod_OTP); + app(IAuthService::class)->verifyMFARecoveryCode($admin, $strategy, $displayedCode); + + EntityManager::clear(); + $unusedAfter = EntityManager::getRepository(UserRecoveryCode::class) + ->findBy(['user' => $admin->getId(), 'used_at' => null]); + + $this->assertCount( + count($unusedBefore) - 1, + $unusedAfter, + 'the code redeemed exactly as displayed must be consumed exactly once' + ); + } + + public function testEnableTwoFactorRejectsWhenAlreadyEnabled(): void + { + $this->enableTwoFactor('email_otp'); + $this->assertResponseStatus(200); + + $response = $this->enableTwoFactor('email_otp'); + + $this->assertResponseStatus(412); + + EntityManager::clear(); + $admin = $this->admin(); + $this->assertTrue($admin->isTwoFactorEnabled(), '2FA must remain enabled after the rejected second call'); + } + + public function testEnableTwoFactorRejectsUnavailableMethod(): void + { + // sms_otp is a stub in Phase I (isPhoneNumberVerified() is hardcoded false), + // so enable2FA() must reject it regardless of the requesting user. + $response = $this->enableTwoFactor('sms_otp'); + + $this->assertResponseStatus(412); + } + + public function testEnableTwoFactorRequiresMethod(): void + { + $response = $this->action('POST', 'Api\\UserApiController@enableTwoFactor', [], [], [], []); + + $this->assertResponseStatus(412); + } + + public function testEnableTwoFactorLogsEnrollmentAuditEvent(): void + { + $admin = $this->admin(); + + $this->enableTwoFactor('email_otp'); + + EntityManager::clear(); + $entries = EntityManager::getRepository(TwoFactorAuditLog::class) + ->findBy(['user' => $admin->getId(), 'event_type' => TwoFactorAuditLog::EventEnrollmentChanged]); + $this->assertNotEmpty($entries, 'an enrollment_changed audit entry must be recorded'); + } + + private function enableTwoFactor(string $method) + { + return $this->action('POST', 'Api\\UserApiController@enableTwoFactor', [ + 'method' => $method, + ], [], [], []); + } + + private function admin(): User + { + $user = EntityManager::getRepository(User::class)->findOneBy(['identifier' => self::ADMIN_IDENTIFIER]); + $this->assertInstanceOf(User::class, $user, 'seeded admin user not found'); + return $user; + } + + private function regenerate(string $password) + { + return $this->action('POST', 'Api\\UserApiController@regenerateRecoveryCodes', [ + 'current_password' => $password, + ], [], [], []); + } + + private function createRecoveryCode(User $user, string $plain, bool $used): int + { + $code = new UserRecoveryCode(); + $code->setUser($user); + $code->setCodeHash(Hash::make($plain)); + if ($used) { + $code->markUsed(); + } + EntityManager::persist($code); + EntityManager::flush(); + return $code->getId(); + } +} diff --git a/tests/TurnstileProtectedControllersTest.php b/tests/TurnstileProtectedControllersTest.php index c6076f40..dffc0134 100644 --- a/tests/TurnstileProtectedControllersTest.php +++ b/tests/TurnstileProtectedControllersTest.php @@ -17,9 +17,12 @@ /** * Class TurnstileProtectedControllersTest * - * Smoke tests verifying that cf-turnstile-response is always required on the - * five auth endpoints that gate every submission behind Turnstile (unlike - * UserController::postLogin, which only activates the rule above a threshold). + * Smoke tests verifying that cf-turnstile-response is required on the five auth + * endpoints that gate every submission behind Turnstile. + * + * Requests MUST go over HTTPS (callSecure) because .env.testing sets + * SSL_ENABLED=true, which causes SSLMiddleware to redirect plain HTTP requests + * to HTTPS before reaching any controller. */ final class TurnstileProtectedControllersTest extends BrowserKitTestCase { @@ -37,8 +40,8 @@ private function sessionHasValidationError(string $field): bool private function postWithSession(string $url, array $data = []): void { - $this->call('GET', $url); - $this->call('POST', $url, array_merge(['_token' => Session::token()], $data)); + $this->callSecure('GET', $url); + $this->callSecure('POST', $url, array_merge(['_token' => Session::token()], $data)); } // ------------------------------------------------------------------------- diff --git a/tests/TwoFactorLoginFlowTest.php b/tests/TwoFactorLoginFlowTest.php index c7c934da..cff0ff0f 100644 --- a/tests/TwoFactorLoginFlowTest.php +++ b/tests/TwoFactorLoginFlowTest.php @@ -16,6 +16,7 @@ use App\libs\Auth\Models\TwoFactorAuditLog; use App\libs\Auth\Models\UserRecoveryCode; use App\libs\Auth\Models\UserTrustedDevice; +use App\Mail\OAuth2PasswordlessOTPMail; use App\Services\Auth\IDeviceTrustService; use App\Services\Auth\ITwoFactorAuditService; use Auth\AuthHelper; @@ -26,6 +27,7 @@ use Illuminate\Support\Facades\Config; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Hash; +use Illuminate\Support\Facades\Mail; use Illuminate\Support\Facades\Session; use App\libs\OAuth2\Repositories\IOAuth2OTPRepository; use Auth\Repositories\IUserRecoveryCodeRepository; @@ -363,6 +365,33 @@ public function testCancelClearsPasswordlessOtpSessionState(): void $this->assertNull(Session::get('user_is_active')); } + // ------------------------------------------------------------------------- + // email delivery + // ------------------------------------------------------------------------- + + public function testMFAChallengeQueuesEmailWithCorrectOTPCode(): void + { + $this->postLogin(self::ADMIN_EMAIL, self::SEED_PASSWORD); + + $dbCode = $this->latestOtpCode(self::ADMIN_EMAIL); + + Mail::assertQueued( + OAuth2PasswordlessOTPMail::class, + function (OAuth2PasswordlessOTPMail $mail) use ($dbCode): bool { + return $mail->email === self::ADMIN_EMAIL + && $mail->otp === $dbCode; + } + ); + } + + public function testResendMFAChallengeQueuesAdditionalEmail(): void + { + $this->postLogin(self::ADMIN_EMAIL, self::SEED_PASSWORD); + $this->resend(); + + Mail::assertQueued(OAuth2PasswordlessOTPMail::class, 2); + } + // ------------------------------------------------------------------------- // verify2FA // ------------------------------------------------------------------------- @@ -453,7 +482,12 @@ public function testOTPCodeRejectsReuseAfterSuccessfulVerification(): void public function testRecoveryCodeRejectsReuseAfterTransactionCommit(): void { $admin = $this->user(self::ADMIN_EMAIL); - $plain = 'RECOVERY-REUSE-TX-' . uniqid(); + // No "-" and all-uppercase: verifyRecoveryCode() now strips separators + // and uppercases the submitted code before Hash::check() (real codes are + // hashed dash-less and all-uppercase; the dash/case are display-only), + // so a fixture hashed with lowercase uniqid() hex would never match its + // own (normalized) submission. + $plain = 'RECOVERYREUSETX' . strtoupper(uniqid()); $this->createRecoveryCode($admin, $plain, false); // First use — must succeed. @@ -574,7 +608,7 @@ public function testOTPRedeemRowLockBlocksConcurrentConnection(): void public function testRecoveryCodeRowLockBlocksConcurrentConnection(): void { $admin = $this->user(self::ADMIN_EMAIL); - $plain = 'RECOVERY-LOCK-' . uniqid(); + $plain = 'RECOVERYLOCK' . strtoupper(uniqid()); $codeId = $this->createRecoveryCode($admin, $plain, false); /** @var IUserRecoveryCodeRepository $recoveryRepo */ @@ -754,7 +788,7 @@ public function testRecoveryAuditFailureDoesNotBlockLogin(): void // Audit is best-effort: a failure emitting recovery_used must NOT 500 a // user whose recovery code is already burned and session established. $admin = $this->user(self::ADMIN_EMAIL); - $plain = 'RECOVERY-AUDIT-FAIL-' . uniqid(); + $plain = 'RECOVERYAUDITFAIL' . strtoupper(uniqid()); $this->createRecoveryCode($admin, $plain, false); $auditMock = \Mockery::mock(ITwoFactorAuditService::class); @@ -807,7 +841,7 @@ public function testDeviceTrustFailureDoesNotBlockLogin(): void public function testRecoveryCodeLoginSucceeds(): void { $admin = $this->user(self::ADMIN_EMAIL); - $plain = 'RECOVERY-PLAIN-123'; + $plain = 'RECOVERYPLAIN123'; $codeId = $this->createRecoveryCode($admin, $plain, false); $this->postLogin(self::ADMIN_EMAIL, self::SEED_PASSWORD); @@ -827,7 +861,7 @@ public function testRecoveryCodeLoginSucceeds(): void public function testUsedRecoveryCodeFails(): void { $admin = $this->user(self::ADMIN_EMAIL); - $plain = 'RECOVERY-USED-456'; + $plain = 'RECOVERYUSED456'; $this->createRecoveryCode($admin, $plain, true); // already used $this->postLogin(self::ADMIN_EMAIL, self::SEED_PASSWORD); diff --git a/tests/e2e/fixtures/index.ts b/tests/e2e/fixtures/index.ts new file mode 100644 index 00000000..f6d80a7c --- /dev/null +++ b/tests/e2e/fixtures/index.ts @@ -0,0 +1,87 @@ +import { test as base } from '@playwright/test'; +import { LoginPage } from '../pages/LoginPage'; +import { RegisterPage } from '../pages/RegisterPage'; + +type E2EFixtures = { + loginPage: LoginPage; + registerPage: RegisterPage; + authenticatedPage: LoginPage; +}; + +export const test = base.extend({ + page: async ({ page }, use) => { + // When running inside the Docker playwright container, APP_URL=http://nginx is + // injected by docker-compose. The PHP app bakes http://localhost:8001 into the + // page HTML (asset src attributes and window.*_ENDPOINT globals). Two problems: + // 1. Assets at http://localhost:8001/assets/** → unreachable from the container. + // 2. XHR to http://localhost:8001/** is cross-origin from http://nginx, so the + // browser omits session cookies → server returns 419 CSRF error. + // + // Fix A: route.continue rewrite for assets (scripts, images, CSS). + // Fix B: addInitScript intercepts window.*_ENDPOINT assignments before they are + // read by React, rewriting them to http://nginx so XHR is same-origin. + // + // From the host (APP_URL unset or http://localhost:*), no interception is needed. + const internalUrl = process.env.APP_URL; + if (internalUrl && !internalUrl.includes('localhost')) { + // Fix A: rewrite asset URLs. + await page.route(/^http:\/\/localhost(:\d+)?\//, (route) => { + const rewritten = route.request().url() + .replace(/^http:\/\/localhost(:\d+)?/, internalUrl); + route.continue({ url: rewritten }); + }); + + // Fix B: intercept window.*_ENDPOINT property assignments so every XHR + // made by React targets http://nginx (same origin), ensuring the session + // cookie is automatically included and CSRF validation succeeds. + const endpoints = [ + 'VERIFY_ACCOUNT_ENDPOINT', + 'EMIT_OTP_ENDPOINT', + 'RESEND_VERIFICATION_EMAIL_ENDPOINT', + 'VERIFY_2FA_ENDPOINT', + 'RESEND_2FA_ENDPOINT', + 'CANCEL_LOGIN_ENDPOINT', + 'RECOVERY_2FA_ENDPOINT', + 'FORM_ACTION_ENDPOINT', + ]; + await page.addInitScript(({ endpoints, internalUrl }) => { + for (const key of endpoints) { + let _val; + Object.defineProperty(window, key, { + configurable: true, + enumerable: true, + set(v) { + _val = typeof v === 'string' + ? v.replace(/http:\/\/localhost(:\d+)?/, internalUrl) + : v; + }, + get() { return _val; }, + }); + } + }, { endpoints, internalUrl }); + } + await use(page); + }, + + loginPage: async ({ page }, use) => { + await use(new LoginPage(page)); + }, + + registerPage: async ({ page }, use) => { + await use(new RegisterPage(page)); + }, + + // Pre-authenticated session using the raw E2E user (no group memberships, + // so MFA is never enforced and the login completes without a 2FA challenge). + // Override via TEST_USER_EMAIL / TEST_USER_PASSWORD env vars if needed. + authenticatedPage: async ({ page }, use) => { + const loginPage = new LoginPage(page); + await loginPage.login( + process.env.TEST_USER_EMAIL || 'e2e@test.com', + process.env.TEST_USER_PASSWORD || '1Qaz2wsx!' + ); + await use(loginPage); + }, +}); + +export { expect } from '@playwright/test'; diff --git a/tests/e2e/pages/LoginPage.ts b/tests/e2e/pages/LoginPage.ts new file mode 100644 index 00000000..d05b4be0 --- /dev/null +++ b/tests/e2e/pages/LoginPage.ts @@ -0,0 +1,67 @@ +import { type Page, type Locator } from '@playwright/test'; + +export class LoginPage { + readonly page: Page; + readonly emailInput: Locator; + readonly passwordInput: Locator; + // Email step: button has title="Continue" (text is ">") + readonly emailSubmitButton: Locator; + // Password step: button text is "Continue", type="button" (no title) + readonly passwordSubmitButton: Locator; + readonly rememberMeCheckbox: Locator; + readonly errorLabel: Locator; + readonly otpInput: Locator; + // Password step container + readonly passwordForm: Locator; + // Two-factor (MFA) step + readonly twoFactorForm: Locator; + readonly verifyButton: Locator; + readonly resendLink: Locator; + readonly cancelLink: Locator; + readonly useRecoveryLink: Locator; + // Recovery code step + readonly recoveryForm: Locator; + + constructor(page: Page) { + this.page = page; + this.emailInput = page.locator('#email'); + this.passwordInput = page.locator('#password'); + this.emailSubmitButton = page.locator('button[title="Continue"]'); + this.passwordSubmitButton = page.getByRole('button', { name: 'Continue' }); + this.rememberMeCheckbox = page.locator('#remember'); + this.errorLabel = page.locator('[data-testid="error-label"]'); + this.otpInput = page.locator('[data-testid="otp_code"]'); + this.passwordForm = page.locator('[data-testid="password-form"]'); + this.twoFactorForm = page.locator('[data-testid="two-factor-form"]'); + this.verifyButton = page.locator('[data-testid="verify-button"]'); + this.resendLink = page.locator('[data-testid="resend-link"]'); + this.cancelLink = page.locator('[data-testid="cancel-link"]'); + this.useRecoveryLink = page.locator('[data-testid="use-recovery-link"]'); + this.recoveryForm = page.locator('[data-testid="recovery-form"]'); + } + + async goto() { + await this.page.goto('/auth/login'); + } + + async fillEmail(email: string) { + await this.emailInput.fill(email); + await this.emailSubmitButton.click(); + } + + async fillPassword(password: string) { + await this.passwordInput.fill(password); + await this.passwordSubmitButton.click(); + } + + async login(email: string, password: string) { + await this.goto(); + await this.fillEmail(email); + await this.fillPassword(password); + } + + async fillOtp(code: string) { + await this.otpInput.fill(code); + await this.passwordSubmitButton.click(); + } +} diff --git a/tests/e2e/pages/RegisterPage.ts b/tests/e2e/pages/RegisterPage.ts new file mode 100644 index 00000000..1c03f16e --- /dev/null +++ b/tests/e2e/pages/RegisterPage.ts @@ -0,0 +1,57 @@ +import { type Page, type Locator } from '@playwright/test'; + +export class RegisterPage { + readonly page: Page; + readonly firstNameInput: Locator; + readonly lastNameInput: Locator; + readonly emailInput: Locator; + readonly passwordInput: Locator; + readonly passwordConfirmInput: Locator; + readonly codeOfConductCheckbox: Locator; + readonly submitButton: Locator; + // MUI FormHelperText error messages (not CSS-module classes, so not hashed) + readonly errorContainer: Locator; + // SweetAlert2 popup shown for server-side errors (e.g. duplicate email) + readonly swalPopup: Locator; + + constructor(page: Page) { + this.page = page; + this.firstNameInput = page.locator('[name="first_name"]'); + this.lastNameInput = page.locator('[name="last_name"]'); + this.emailInput = page.locator('[name="email"]'); + this.passwordInput = page.locator('[name="password"]'); + this.passwordConfirmInput = page.locator('[name="password_confirmation"]'); + this.codeOfConductCheckbox = page.locator('[name="agree_code_of_conduct"]'); + this.submitButton = page.locator('button[type="submit"]'); + this.errorContainer = page.locator('p.MuiFormHelperText-root.Mui-error').first(); + this.swalPopup = page.locator('.swal2-popup'); + } + + async goto() { + await this.page.goto('/auth/register'); + } + + // MUI Select does not render a native . +const getPasswordInput = () => screen.getByTestId('recovery-codes-current-password').querySelector('input'); + +describe('RecoveryCodesPanel', () => { + beforeEach(() => { + window.sessionStorage.clear(); + jest.clearAllMocks(); + }); + + it('shows the remaining/total count', () => { + render( + + ); + expect(screen.getByTestId('recovery-codes-count')).toHaveTextContent('Recovery Codes: 7 of 10 remaining'); + }); + + it('shows a dismissable low-code warning when remaining is below the threshold', () => { + render( + + ); + expect(screen.getByTestId('low-code-warning')).toBeInTheDocument(); + + fireEvent.click(screen.getByLabelText('dismiss')); + expect(screen.queryByTestId('low-code-warning')).not.toBeInTheDocument(); + }); + + it('does not show the low-code warning when there are enough codes', () => { + render( + + ); + expect(screen.queryByTestId('low-code-warning')).not.toBeInTheDocument(); + }); + + it('respects a custom lowCodeThreshold instead of the default of 3', () => { + // 4 remaining would NOT trigger the default threshold (3), but does with a custom threshold of 5. + render( + + ); + expect(screen.getByTestId('low-code-warning')).toBeInTheDocument(); + }); + + it('respects a custom lower threshold (2 remaining is not below a threshold of 2)', () => { + // With the default threshold (3) this would show the warning; a custom + // threshold of 2 must be honored instead of the hardcoded default. + render( + + ); + expect(screen.queryByTestId('low-code-warning')).not.toBeInTheDocument(); + }); + + it('opens the modal immediately when initialCodes is provided', () => { + const codes = ['AAAA-1111', 'BBBB-2222']; + render( + + ); + expect(screen.getByText('AAAA-1111')).toBeInTheDocument(); + }); + + it('regenerates codes after confirming the current password and opens the modal', async () => { + const newCodes = ['NEW1-CODE', 'NEW2-CODE']; + regenerateRecoveryCodes.mockResolvedValue({response: {recovery_codes: newCodes}}); + + render( + + ); + + fireEvent.click(screen.getByText('Regenerate Codes')); + fireEvent.change(getPasswordInput(), {target: {value: 'my-password'}}); + fireEvent.click(screen.getByTestId('confirm-regenerate-button')); + + expect(regenerateRecoveryCodes).toHaveBeenCalledWith('my-password'); + expect(await screen.findByText('NEW1-CODE')).toBeInTheDocument(); + expect(screen.getByTestId('recovery-codes-count')).toHaveTextContent('Recovery Codes: 2 of 2 remaining'); + }); + + it('shows an error and keeps the existing count when the password is wrong', async () => { + regenerateRecoveryCodes.mockRejectedValue({ + status: 412, + response: {body: {errors: ['current_password is not correct.']}}, + }); + + render( + + ); + + fireEvent.click(screen.getByText('Regenerate Codes')); + fireEvent.change(getPasswordInput(), {target: {value: 'wrong'}}); + fireEvent.click(screen.getByTestId('confirm-regenerate-button')); + + expect(regenerateRecoveryCodes).toHaveBeenCalledWith('wrong'); + await new Promise((resolve) => setImmediate(resolve)); + expect(screen.getByTestId('recovery-codes-count')).toHaveTextContent('Recovery Codes: 7 of 10 remaining'); + }); +}); diff --git a/tests/js/components/two_factor_section.test.js b/tests/js/components/two_factor_section.test.js new file mode 100644 index 00000000..e5e9dfef --- /dev/null +++ b/tests/js/components/two_factor_section.test.js @@ -0,0 +1,48 @@ +import React from 'react'; +import {render, screen, fireEvent} from '@testing-library/react'; +import TwoFactorSection from '../../../resources/js/components/two_factor_section'; +import {enableTwoFactor} from '../../../resources/js/profile/actions'; + +jest.mock('../../../resources/js/profile/actions'); +jest.mock('sweetalert2', () => jest.fn()); + +describe('TwoFactorSection', () => { + beforeEach(() => { + window.sessionStorage.clear(); + jest.clearAllMocks(); + }); + + it('shows the enable button when 2FA is not enabled', () => { + render( + + ); + expect(screen.getByTestId('enable-two-factor-button')).toBeInTheDocument(); + expect(screen.queryByTestId('recovery-codes-count')).not.toBeInTheDocument(); + }); + + it('shows the recovery codes panel when 2FA is already enabled', () => { + render( + + ); + expect(screen.queryByTestId('enable-two-factor-button')).not.toBeInTheDocument(); + expect(screen.getByTestId('recovery-codes-count')).toHaveTextContent('Recovery Codes: 7 of 10 remaining'); + }); + + it('enables 2FA and shows the enrollment codes in the modal', async () => { + const codes = ['AAAA-1111', 'BBBB-2222']; + enableTwoFactor.mockResolvedValue({response: {recovery_codes: codes}}); + + render( + + ); + + fireEvent.click(screen.getByTestId('enable-two-factor-button')); + + expect(enableTwoFactor).toHaveBeenCalledWith('email_otp'); + expect(await screen.findByText('AAAA-1111')).toBeInTheDocument(); + expect(screen.getByTestId('recovery-codes-count')).toHaveTextContent('Recovery Codes: 2 of 2 remaining'); + }); +}); diff --git a/tests/js/login/components/two-factor-form.test.js b/tests/js/login/components/two-factor-form.test.js new file mode 100644 index 00000000..d7278eec --- /dev/null +++ b/tests/js/login/components/two-factor-form.test.js @@ -0,0 +1,56 @@ +import React from 'react'; +import { render, screen } from '@testing-library/react'; +import TwoFactorForm from '../../../../resources/js/login/components/two_factor_form'; + +// Suppress the 1-second interval so tests don't bleed real timers into each other. +beforeEach(() => jest.useFakeTimers()); +afterEach(() => jest.useRealTimers()); + +const baseProps = { + otpCode: '123456', + otpError: '', + otpLength: 6, + otpLifetime: 300, + codeVersion: 0, + disableInput: false, + trustDevice: false, + onCodeChange: jest.fn(), + onVerify: jest.fn(), + onTrustDeviceChange: jest.fn(), + onResend: jest.fn(), + onUseRecovery: jest.fn(), + onCancel: jest.fn(), +}; + +describe('TwoFactorForm', () => { + + it('renders countdown when otpLifetime > 0', () => { + render(); + // formatTime(300) → "5 minutes"; the paragraph reads "Code expires in 5 minutes." + expect(screen.getByText(/Code expires in 5 minutes\./)).toBeInTheDocument(); + expect(screen.queryByText(/has expired/)).not.toBeInTheDocument(); + }); + + it('renders expired state when otpLifetime is 0', () => { + render(); + expect( + screen.getByText(/Your verification code has expired\. Please request a new one\./) + ).toBeInTheDocument(); + expect(screen.queryByText(/Code expires in/)).not.toBeInTheDocument(); + }); + + it('VERIFY button is disabled when otpCode is empty', () => { + render(); + // MUI Button spreads unknown props to its root