diff --git a/.env.example b/.env.example index d65f1e0b6..373c1e246 100644 --- a/.env.example +++ b/.env.example @@ -168,3 +168,5 @@ L5_SWAGGER_OPEN_API_SPEC_VERSION=3.1.2 TWO_FACTOR_RECOVERY_ENROLLMENT_WINDOW=600 TWO_FACTOR_MAX_ENROLL_REQUESTS=5 TWO_FACTOR_ENROLL_WINDOW_MINUTES=15 +TWO_FACTOR_MAX_PASSWORD_CONFIRM_ATTEMPTS=5 +TWO_FACTOR_PASSWORD_CONFIRM_WINDOW_MINUTES=15 diff --git a/app/Http/Controllers/AdminController.php b/app/Http/Controllers/AdminController.php index c885633f2..bd9e690f0 100644 --- a/app/Http/Controllers/AdminController.php +++ b/app/Http/Controllers/AdminController.php @@ -660,8 +660,11 @@ public function editUser($user_id){ return View::make("admin.edit-user", [ 'user_id' => $user_id, + // a server admin manages their own 2FA from the profile (it asks for the + // password); UserService::update ignores the key when editing oneself + 'is_own_account' => $this->auth_service->getCurrentUser()?->getId() == $user->getId(), 'user' => json_encode(SerializerRegistry::getInstance() - ->getSerializer($user, SerializerRegistry::SerializerType_Private)->serialize('groups')), + ->getSerializer($user, SerializerRegistry::SerializerType_Admin)->serialize('groups')), 'countries' => CountryList::getCountries(), 'languages' => $lang2Code, ] diff --git a/app/Http/Controllers/Api/OAuth2/OAuth2UserApiController.php b/app/Http/Controllers/Api/OAuth2/OAuth2UserApiController.php index 3f4ac9cde..1880fca76 100644 --- a/app/Http/Controllers/Api/OAuth2/OAuth2UserApiController.php +++ b/app/Http/Controllers/Api/OAuth2/OAuth2UserApiController.php @@ -258,6 +258,11 @@ protected function curateUpdatePayload(array $payload): array if (isset($payload['active'])) unset($payload['active']); + // 2FA is managed only from the admin user CRUD (server admin session) or the + // owner's profile, never by an OAuth2 client acting with the token owner's rights + if (array_key_exists('two_factor_enabled', $payload)) + unset($payload['two_factor_enabled']); + return HTMLCleaner::cleanData($payload, [ 'bio', 'statement_of_interest' diff --git a/app/Http/Controllers/Api/UserApiController.php b/app/Http/Controllers/Api/UserApiController.php index fa2274680..f2ca8e78a 100644 --- a/app/Http/Controllers/Api/UserApiController.php +++ b/app/Http/Controllers/Api/UserApiController.php @@ -23,6 +23,8 @@ use Exception; use Illuminate\Http\Request as LaravelRequest; use Illuminate\Support\Facades\Auth; +use Illuminate\Support\Facades\Config; +use Illuminate\Support\Facades\Cookie; use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Request; use Illuminate\Support\Facades\Validator; @@ -158,7 +160,7 @@ public function lock($id) protected function getAllSerializerType(): string { - return SerializerRegistry::SerializerType_Private; + return SerializerRegistry::SerializerType_Admin; } /** @@ -298,6 +300,54 @@ public function enableTwoFactor() }); } + /** + * Disables 2FA for the current user (SDS idp-mfa.md §5.6). Deletes their recovery + * codes and revokes every trusted device, so the device trust cookie is expired too. + * Requires the current password, or for a user without one a session inside the + * recovery enrollment window (SDS rev 2.6 §4.10.3). Refused for group-enforced users. + * + * @return \Illuminate\Http\JsonResponse|mixed + */ + public function disableTwoFactor() + { + if (!Auth::check()) + return $this->error403(); + + return $this->processRequest(function () { + $data = Request::all(); + $validator = Validator::make($data, [ + 'current_password' => 'sometimes|nullable|string', + ]); + + if (!$validator->passes()) { + return $this->error412($validator->getMessageBag()->getMessages()); + } + + $user = Auth::user(); + + if ($user->isTwoFactorEnforcedByGroup()) { + return $this->error412(['two_factor' => ['Two-factor authentication is required for members of an administrator group and cannot be turned off.']]); + } + + if (!$user->isTwoFactorEnabled()) { + return $this->error412(['two_factor' => ['Two-factor authentication is not enabled for this account.']]); + } + + // the service enforces current_password / the enrollment window for the + // self-service actor and throws ValidationException (-> 412) + $this->recovery_code_service->disableTwoFactor($user, $data['current_password'] ?? null, $user); + + // every trusted device was just revoked + Cookie::queue(Cookie::forget( + Config::get('two_factor.cookie_name', 'device_trust_token'), + Config::get('session.path'), + Config::get('session.domain') + )); + + return $this->ok(['two_factor_enabled' => false]); + }); + } + /** * Invalidates the current user's recovery codes and generates a fresh batch. * Plaintext codes are returned once in the response and never persisted. @@ -438,6 +488,6 @@ public function updatePic($id) protected function serializerType(): string { - return SerializerRegistry::SerializerType_Private; + return SerializerRegistry::SerializerType_Admin; } } \ No newline at end of file diff --git a/app/Http/Controllers/Factories/UserValidationRulesFactory.php b/app/Http/Controllers/Factories/UserValidationRulesFactory.php index 9e34ca7a3..663cb5cdd 100644 --- a/app/Http/Controllers/Factories/UserValidationRulesFactory.php +++ b/app/Http/Controllers/Factories/UserValidationRulesFactory.php @@ -58,6 +58,8 @@ public static function build(array $data, $update = false, ?User $currentUser = // admin fields 'email_verified' => 'nullable|boolean', 'active' => 'nullable|boolean', + // honoured only for a server admin editing ANOTHER user (UserService::update) + 'two_factor_enabled' => 'nullable|boolean', 'groups' => 'sometimes|int_array', 'public_profile_show_photo' => 'sometimes|boolean', 'public_profile_show_fullname' => 'sometimes|boolean', diff --git a/app/Http/Controllers/UserController.php b/app/Http/Controllers/UserController.php index f765b5c6d..969a4cb67 100644 --- a/app/Http/Controllers/UserController.php +++ b/app/Http/Controllers/UserController.php @@ -1208,13 +1208,16 @@ public function getProfile() return View::make("profile", array_merge([ 'user' => json_encode(SerializerRegistry::getInstance()->getSerializer( - $user, SerializerRegistry::SerializerType_Private)->serialize()), + $user, SerializerRegistry::SerializerType_Admin)->serialize()), "openid_url" => $this->server_configuration_service->getUserIdentityEndpointURL($user->getIdentifier()), "sites" => $sites, 'actions' => $actions, 'countries' => CountryList::getCountries(), 'languages' => $lang2Code, - 'two_factor_enabled' => $user->shouldRequire2FA(), + // the stored flag, not the effective requirement: the profile needs to tell a + // self-enrolled user (can turn it off) from one enforced by group (cannot) + 'two_factor_enabled' => $user->isTwoFactorEnabled(), + 'two_factor_enforced' => $user->isTwoFactorEnforcedByGroup(), // The session completed a 2FA challenge within the enrollment window: // a batch can be generated without the password. 'recovery_codes_window_open' => $window_open, diff --git a/app/Http/Middleware/TwoFactorRateLimitMiddleware.php b/app/Http/Middleware/TwoFactorRateLimitMiddleware.php index 48282f6fd..06cbb4161 100644 --- a/app/Http/Middleware/TwoFactorRateLimitMiddleware.php +++ b/app/Http/Middleware/TwoFactorRateLimitMiddleware.php @@ -33,7 +33,9 @@ * * - every request reserves an attempt atomically before the controller runs. * - verify / recovery: the attempt is refunded unless the response is a failure. - * - resend / otp: every request counts. + * - password_confirm: the attempt is refunded when the request succeeds (2xx), + * so only rejected password confirmations count. + * - resend / otp / enroll: every request counts. * * @package App\Http\Middleware */ @@ -54,7 +56,7 @@ public function __construct(private readonly ITwoFactorRateLimitService $rate_li /** * @param \Illuminate\Http\Request $request * @param Closure $next - * @param string $action one of verify|recovery|resend|otp + * @param string $action one of verify|recovery|resend|otp|enroll|password_confirm * @return mixed */ public function handle($request, Closure $next, string $action = ITwoFactorRateLimitService::ActionVerify) @@ -99,6 +101,13 @@ public function handle($request, Closure $next, string $action = ITwoFactorRateL $this->rate_limit_service->refund($action, $subject); } + // password confirmation: the controller answers a wrong password with a plain + // 412 (no error_code), so any non-2xx response counts as a failed attempt + if ($action === ITwoFactorRateLimitService::ActionPasswordConfirm + && method_exists($response, 'isSuccessful') && $response->isSuccessful()) { + $this->rate_limit_service->refund($action, $subject); + } + return $response; } diff --git a/app/Mail/TwoFactorEnabledByAdminMail.php b/app/Mail/TwoFactorEnabledByAdminMail.php new file mode 100644 index 000000000..444640f71 --- /dev/null +++ b/app/Mail/TwoFactorEnabledByAdminMail.php @@ -0,0 +1,80 @@ +user_email = $user->getEmail(); + $this->user_fullname = $user->getFullName(); + $this->profile_url = URL::action('UserController@getProfile'); + } + + /** + * @return $this + */ + public function build() + { + $this->subject = sprintf('[%s] Two-factor authentication enabled', Config::get('app.app_name')); + Log::debug(sprintf("TwoFactorEnabledByAdminMail::build to %s", $this->user_email)); + return $this->from(Config::get("mail.from")) + ->to($this->user_email) + ->subject($this->subject) + ->view('emails.auth.two_factor_enabled_by_admin'); + } +} diff --git a/app/ModelSerializers/Auth/UserSerializer.php b/app/ModelSerializers/Auth/UserSerializer.php index dd4ef9354..bcf26ab48 100644 --- a/app/ModelSerializers/Auth/UserSerializer.php +++ b/app/ModelSerializers/Auth/UserSerializer.php @@ -43,7 +43,7 @@ final class PublicUserSerializer extends BaseUserSerializer * Class PrivateUserSerializer * @package App\ModelSerializers\Auth */ -final class PrivateUserSerializer extends BaseUserSerializer +class PrivateUserSerializer extends BaseUserSerializer { protected static $allowed_relations = [ @@ -130,4 +130,42 @@ public function serialize($expand = null, array $fields = [], array $relations = return $values; } -} \ No newline at end of file +} + +/** + * Class AdminUserSerializer + * The private user plus the 2FA state. For the user admin CRUD, the admin edit page and the + * user's own profile. Never for OAuth2 clients: they get the private serializer, whose + * shape does not change. + * @package App\ModelSerializers\Auth + */ +final class AdminUserSerializer extends PrivateUserSerializer +{ + protected static $array_mappings = [ + 'TwoFactorEnabled' => 'two_factor_enabled:json_boolean', + 'TwoFactorMethod' => 'two_factor_method:json_string', + 'TwoFactorEnforcedAt' => 'two_factor_enforced_at:datetime_epoch', + ]; + + /** + * @param null $expand + * @param array $fields + * @param array $relations + * @param array $params + * @return array + */ + public function serialize($expand = null, array $fields = [], array $relations = [], array $params = []) + { + $user = $this->object; + if (!$user instanceof User) return []; + + $values = parent::serialize($expand, $fields, $relations, $params); + + // group-derived, read only: not a stored column + if (empty($fields) || in_array('two_factor_enforced', $fields)) { + $values['two_factor_enforced'] = $user->isTwoFactorEnforcedByGroup(); + } + + return $values; + } +} diff --git a/app/ModelSerializers/SerializerRegistry.php b/app/ModelSerializers/SerializerRegistry.php index 12250286e..4ca269439 100644 --- a/app/ModelSerializers/SerializerRegistry.php +++ b/app/ModelSerializers/SerializerRegistry.php @@ -11,6 +11,7 @@ * See the License for the specific language governing permissions and * limitations under the License. **/ +use App\ModelSerializers\Auth\AdminUserSerializer; use App\ModelSerializers\Auth\PrivateUserSerializer; use App\ModelSerializers\Auth\PublicGroupSerializer; use App\ModelSerializers\Auth\PublicUserSerializer; @@ -43,6 +44,7 @@ final class SerializerRegistry const SerializerType_Public = 'PUBLIC'; const SerializerType_Private = 'PRIVATE'; + const SerializerType_Admin = 'ADMIN'; private function __clone(){} @@ -79,6 +81,7 @@ private function __construct() $this->registry["User"] = [ self::SerializerType_Public => PublicUserSerializer::class, self::SerializerType_Private => PrivateUserSerializer::class, + self::SerializerType_Admin => AdminUserSerializer::class, ]; $this->registry["UserAction"] = UserActionSerializer::class; diff --git a/app/Services/Auth/IRecoveryCodeService.php b/app/Services/Auth/IRecoveryCodeService.php index 53bcaaf11..929ce603c 100644 --- a/app/Services/Auth/IRecoveryCodeService.php +++ b/app/Services/Auth/IRecoveryCodeService.php @@ -135,4 +135,19 @@ public function getStatus(User $user): RecoveryCodesStatus; * and the session is outside the enrollment window */ public function disableTwoFactor(User $user, ?string $currentPassword, ?User $actor): void; + + /** + * Enables 2FA for a user on behalf of an administrator (support path). Unlike + * enableTwoFactorAndGenerateCodes() no recovery codes are generated: codes are shown + * once and only to their owner, who is told by email to generate them from their + * profile. Audited as enrollment_changed with {enabled: true, actor_id, reason: admin}. + * The email and the audit row are best-effort once 2FA is enabled. + * + * @param User $user + * @param string $method one of User::ValidMFAMethods + * @param User $actor the administrator performing the change + * @return void + * @throws ValidationException if $method is not valid or not available + */ + public function enableTwoFactorForUser(User $user, string $method, User $actor): void; } diff --git a/app/Services/Auth/ITwoFactorRateLimitService.php b/app/Services/Auth/ITwoFactorRateLimitService.php index e8e66312f..1576b2cde 100644 --- a/app/Services/Auth/ITwoFactorRateLimitService.php +++ b/app/Services/Auth/ITwoFactorRateLimitService.php @@ -40,6 +40,9 @@ interface ITwoFactorRateLimitService public const ActionResend = 'resend'; public const ActionOtp = 'otp'; public const ActionEnroll = 'enroll'; + // password confirmation of the authenticated user's 2FA changes (disable 2FA, + // regenerate recovery codes): one budget shared by both routes + public const ActionPasswordConfirm = 'password_confirm'; public const RATE_LIMIT_ERROR_CODE = MFAConstants::ERROR_CODE_RATE_LIMIT; public const RATE_LIMIT_MESSAGE = 'Too many attempts. Please try again later.'; diff --git a/app/Services/Auth/RecoveryCodeService.php b/app/Services/Auth/RecoveryCodeService.php index e9d9168b4..badd8cb96 100644 --- a/app/Services/Auth/RecoveryCodeService.php +++ b/app/Services/Auth/RecoveryCodeService.php @@ -14,6 +14,7 @@ **/ use App\libs\Auth\Models\TwoFactorAuditLog; +use App\Mail\TwoFactorEnabledByAdminMail; use App\libs\Auth\Models\UserRecoveryCode; use Auth\MFAConstants; use Auth\Repositories\IUserRecoveryCodeRepository; @@ -22,6 +23,7 @@ use Auth\User; use Illuminate\Support\Facades\Hash; use Illuminate\Support\Facades\Log; +use Illuminate\Support\Facades\Mail; use Illuminate\Support\Facades\Session; use Laminas\Math\Rand; use models\exceptions\ValidationException; @@ -294,6 +296,41 @@ public function disableTwoFactor(User $user, ?string $currentPassword, ?User $ac } } + /** + * @inheritDoc + */ + public function enableTwoFactorForUser(User $user, string $method, User $actor): void + { + $this->tx_service->transaction(function () use ($user, $method) { + // the lock re-hydrates $user, so it must precede enable2FA() + $this->repository->lockUserRow($user); + $user->enable2FA($method); + $this->user_repository->add($user, false); + }); + + try { + $this->audit_service->log( + $user, + TwoFactorAuditLog::EventEnrollmentChanged, + $method, + IPHelper::getUserIp(), + [ + 'enabled' => true, + 'actor_id' => $actor->getId(), + 'reason' => 'admin', + ] + ); + } catch (\Throwable $ex) { + Log::warning($ex); + } + + try { + Mail::queue(new TwoFactorEnabledByAdminMail($user)); + } catch (\Throwable $ex) { + Log::warning($ex); + } + } + /** * Invalidates every existing recovery code for the user and generates a * fresh batch, within the caller's already-open transaction. The caller diff --git a/app/Services/Auth/TwoFactorRateLimitService.php b/app/Services/Auth/TwoFactorRateLimitService.php index 8026b2739..0a0a42ce4 100644 --- a/app/Services/Auth/TwoFactorRateLimitService.php +++ b/app/Services/Auth/TwoFactorRateLimitService.php @@ -103,6 +103,13 @@ private function limitsFor(string $action): array ]; } + if ($action === self::ActionPasswordConfirm) { + return [ + (int) Config::get('two_factor.rate_limit.max_password_confirm_attempts', 5), + (int) Config::get('two_factor.rate_limit.password_confirm_window_minutes', 15) * 60, + ]; + } + return [ (int) Config::get('two_factor.rate_limit.max_attempts', 3), (int) Config::get('two_factor.rate_limit.window_seconds', 900), diff --git a/app/Services/Auth/TwoFactorServiceProvider.php b/app/Services/Auth/TwoFactorServiceProvider.php index 4e0ac29bc..7e3cc17d3 100644 --- a/app/Services/Auth/TwoFactorServiceProvider.php +++ b/app/Services/Auth/TwoFactorServiceProvider.php @@ -96,7 +96,7 @@ private function registerRateLimiters(): void // state: a pending-user subject would resolve to nothing and the // middleware would let every request through unthrottled. The subject // is the authenticated user instead. - RateLimiter::for($limiterName(ITwoFactorRateLimitService::ActionEnroll), function () use ($rateLimitService, $respondRateLimited) { + $byAuthenticatedUser = function (string $action) use ($rateLimitService, $respondRateLimited) { $userId = Auth::id(); if (is_null($userId)) { @@ -105,10 +105,17 @@ private function registerRateLimiters(): void return (new Limit( (int) $userId, - $rateLimitService->getLimit(ITwoFactorRateLimitService::ActionEnroll), - $rateLimitService->getWindowSeconds(ITwoFactorRateLimitService::ActionEnroll) + $rateLimitService->getLimit($action), + $rateLimitService->getWindowSeconds($action) ))->response($respondRateLimited); - }); + }; + + RateLimiter::for($limiterName(ITwoFactorRateLimitService::ActionEnroll), fn () => $byAuthenticatedUser(ITwoFactorRateLimitService::ActionEnroll)); + + // Password confirmation of 2FA changes (disable, regenerate recovery codes): + // keyed by the authenticated user, one counter for both routes so password + // guesses cannot move from one to the other. + RateLimiter::for($limiterName(ITwoFactorRateLimitService::ActionPasswordConfirm), fn () => $byAuthenticatedUser(ITwoFactorRateLimitService::ActionPasswordConfirm)); RateLimiter::for($limiterName(ITwoFactorRateLimitService::ActionOtp), function (Request $request) use ($rateLimitService, $respondRateLimited) { $username = strtolower(trim($request->input('username', ''))); diff --git a/app/Services/OpenId/UserService.php b/app/Services/OpenId/UserService.php index cb5f56545..67a1dc834 100644 --- a/app/Services/OpenId/UserService.php +++ b/app/Services/OpenId/UserService.php @@ -23,6 +23,7 @@ use App\libs\Utils\FileSystem\FileNameSanitizer; use App\Mail\MonitoredSecurityGroupNotificationEmail; use App\Services\AbstractService; +use App\Services\Auth\IRecoveryCodeService; use App\Services\Auth\IUserIdentifierGeneratorService; use Auth\Group; use Auth\IUserNameGeneratorService; @@ -95,6 +96,11 @@ final class UserService extends AbstractService implements IUserService */ private $identifier_service; + /** + * @var IRecoveryCodeService + */ + private $recovery_code_service; + /** * UserService constructor. * @param IUserRepository $repository @@ -105,6 +111,8 @@ final class UserService extends AbstractService implements IUserService * @param ILogService $log_service * @param IResourceServerContext $server_ctx * @param IUserIdentifierGeneratorService $identifier_service + * @param IClientRepository $client_repository + * @param IRecoveryCodeService $recovery_code_service */ public function __construct ( @@ -116,10 +124,12 @@ public function __construct ILogService $log_service, IResourceServerContext $server_ctx, IUserIdentifierGeneratorService $identifier_service, - IClientRepository $client_repository + IClientRepository $client_repository, + IRecoveryCodeService $recovery_code_service ) { parent::__construct($tx_service); + $this->recovery_code_service = $recovery_code_service; $this->repository = $repository; $this->group_repository = $group_repository; $this->user_name_generator = $user_name_generator; @@ -295,8 +305,10 @@ public function create(array $payload): IEntity public function update(int $id, array $payload): IEntity { $password_changed = false; + $two_factor_change = null; + $acting_admin = null; - $user = $this->tx_service->transaction(function () use ($id, $payload, &$password_changed) { + $user = $this->tx_service->transaction(function () use ($id, $payload, &$password_changed, &$two_factor_change, &$acting_admin) { $user = $this->repository->getById($id); @@ -368,6 +380,28 @@ public function update(int $id, array $payload): IEntity } } + // 2FA is managed here and not in UserFactory::populate: it needs the acting user. + // Only a server admin editing ANOTHER user may change it, and only when the value + // differs from the stored one (the admin form sends the key only when the admin + // changed the box; other API clients may still send it on every save). Anyone + // else, including PUT /users/me, has the key ignored: turning 2FA off needs the + // password and goes through POST /users/me/2fa/disable. + if (isset($payload['two_factor_enabled']) + && !is_null($current_user) + && $current_user->getId() != $user->getId() + && ($current_user->isAdmin() || $current_user->isSuperAdmin())) { + $requested = boolval($payload['two_factor_enabled']); + if ($requested !== $user->isTwoFactorEnabled()) { + // checked against the groups of this same update (the admin form sends + // them on every save) and inside the transaction: a refused change rolls + // the whole update back + if (!$requested && $user->isTwoFactorEnforcedByGroup()) + throw new ValidationException("Two-factor authentication is enforced by group membership and cannot be disabled."); + $two_factor_change = $requested; + $acting_admin = $current_user; + } + } + if ($former_email != $user->getEmail()) { Log::warning(sprintf("UserService::update use id %s - email changed old %s - email new %s", $id, $former_email, $user->getEmail())); $user->clearEmailVerification(); @@ -379,6 +413,12 @@ public function update(int $id, array $payload): IEntity $password_changed = true; } + // enable2FA() runs after this transaction commits, so check its precondition here + // (after populate and the email change, which may set or clear email verification): + // a refused enable must roll back the whole update, like the group check above. + if ($two_factor_change === true && !$user->isTwoFactorMethodEnabled(User::MFAMethod_OTP)) + throw new ValidationException("Two-factor authentication requires a verified email address."); + return $user; }); @@ -387,14 +427,28 @@ public function update(int $id, array $payload): IEntity Event::dispatch(new UserPasswordResetSuccessful($user->getId())); } + // after the update transaction: the recovery code service runs its own (it locks the + // users row and re-hydrates $user, and DoctrineTransactionService does not nest). + // Every business rule that could refuse the change was checked inside the update + // transaction above, so only an infrastructure failure gets here; the profile + // changes are committed by then, so they are still published and audited. try { - if (Config::get("queue.enable_message_broker", false) == true) - PublishUserUpdated::dispatch($user)->onConnection('message_broker'); - } catch (\Exception $ex) { - Log::warning($ex); - } + if (!is_null($two_factor_change)) { + if ($two_factor_change) + $this->recovery_code_service->enableTwoFactorForUser($user, User::MFAMethod_OTP, $acting_admin); + else + $this->recovery_code_service->disableTwoFactor($user, null, $acting_admin); + } + } finally { + try { + if (Config::get("queue.enable_message_broker", false) == true) + PublishUserUpdated::dispatch($user)->onConnection('message_broker'); + } catch (\Exception $ex) { + Log::warning($ex); + } - $this->addUserCRUDAction($user, $payload, "UPDATE"); + $this->addUserCRUDAction($user, $payload, "UPDATE"); + } return $user; } diff --git a/config/two_factor.php b/config/two_factor.php index 954056f88..caf06ae76 100644 --- a/config/two_factor.php +++ b/config/two_factor.php @@ -120,5 +120,11 @@ // authenticated user; every request counts (SDS idp-mfa.md §4.12). 'max_enroll_requests' => env('TWO_FACTOR_MAX_ENROLL_REQUESTS', 5), 'enroll_window_minutes' => env('TWO_FACTOR_ENROLL_WINDOW_MINUTES', 15), + + // Password confirmation of the authenticated user's 2FA changes (POST + // 2fa/disable and recovery-codes/regenerate), keyed by the authenticated + // user and shared by both routes; only failed requests count. + 'max_password_confirm_attempts' => env('TWO_FACTOR_MAX_PASSWORD_CONFIRM_ATTEMPTS', 5), + 'password_confirm_window_minutes' => env('TWO_FACTOR_PASSWORD_CONFIRM_WINDOW_MINUTES', 15), ], ]; diff --git a/resources/js/admin/edit_user/actions.js b/resources/js/admin/edit_user/actions.js index ca278dcd5..1e22f30fc 100644 --- a/resources/js/admin/edit_user/actions.js +++ b/resources/js/admin/edit_user/actions.js @@ -72,8 +72,12 @@ export const fetchGroups = async (url, query) => { return response.data; } -export const save = async (entity, pic, token) => { - return putRawRequest(window.SAVE_PROFILE_ENDPOINT)(normalizeEntity(entity), {} ,{'X-CSRF-TOKEN': token}) +/** + * @param twoFactorBaseline the 2FA flag the form was last loaded/saved with: the flag is sent + * only when the admin changed it (see normalizeEntity) + */ +export const save = async (entity, pic, token, twoFactorBaseline) => { + return putRawRequest(window.SAVE_PROFILE_ENDPOINT)(normalizeEntity(entity, twoFactorBaseline), {} ,{'X-CSRF-TOKEN': token}) .then((res) => { if (pic) { return putFile(window.SAVE_PIC_ENDPOINT)(pic, 'pic', {'X-CSRF-TOKEN': token}); @@ -83,7 +87,7 @@ export const save = async (entity, pic, token) => { ); } -const normalizeEntity = (entity) => { +export const normalizeEntity = (entity, twoFactorBaseline) => { entity.public_profile_show_photo = entity.public_profile_show_photo ? 1 : 0; entity.public_profile_show_fullname = entity.public_profile_show_fullname ? 1 : 0; @@ -91,6 +95,17 @@ const normalizeEntity = (entity) => { entity.public_profile_allow_chat_with_me = entity.public_profile_allow_chat_with_me ? 1 : 0; entity.active = entity.active ? 1 : 0; entity.email_verified = entity.email_verified ? 1 : 0; + // 1/0 like the other checkboxes, but only when the admin changed the box during this + // form session: a stale form (2FA turned on by the user after the page loaded) must not + // send the old value on an unrelated save, the server would disable 2FA. The rest of + // the 2FA state is read only. + if (!!entity.two_factor_enabled === !!twoFactorBaseline) + delete entity.two_factor_enabled; + else + entity.two_factor_enabled = entity.two_factor_enabled ? 1 : 0; + delete entity.two_factor_enforced; + delete entity.two_factor_enforced_at; + delete entity.two_factor_method; entity.post_code = entity.post_code.toString(); if (entity.birthday) { diff --git a/resources/js/admin/edit_user/components/two_factor_checkbox.js b/resources/js/admin/edit_user/components/two_factor_checkbox.js new file mode 100644 index 000000000..4e256d08e --- /dev/null +++ b/resources/js/admin/edit_user/components/two_factor_checkbox.js @@ -0,0 +1,49 @@ +import React from "react"; +import moment from "moment"; +import Checkbox from "@material-ui/core/Checkbox"; +import FormControlLabel from "@material-ui/core/FormControlLabel"; +import FormHelperText from "@material-ui/core/FormHelperText"; + +export const ENFORCED_BY_GROUP_TEXT = "Enforced by group membership"; +export const OWN_ACCOUNT_TEXT = "Manage your own two-factor authentication from your profile"; + +/** + * 2FA checkbox of the admin edit user form. A user enforced by group membership is + * required to complete 2FA whatever the stored flag says, so the box shows checked and + * disabled: nobody can switch it off. The stored flag is what the form submits. + * On the admin's own account it is read only too: the user CRUD ignores the key when an + * admin edits themselves, so turning 2FA off must go through the profile (password). + */ +const TwoFactorCheckbox = ({enabled, enforced = false, enforcedAt = null, ownAccount = false, onChange}) => { + const readOnly = !!enforced || !!ownAccount; + return ( + <> + { if (!readOnly && onChange) onChange(e); }} + inputProps={{"data-testid": "two-factor-checkbox"}} + color="primary"/>} + label="Two-Factor Authentication (Email OTP)" + /> + { + enforced && + {ENFORCED_BY_GROUP_TEXT} + } + { + !enforced && ownAccount && + {OWN_ACCOUNT_TEXT} + } + { + !!enforcedAt && + + Enforced since {moment.unix(Number(enforcedAt)).utc().format("YYYY-MM-DD HH:mm")} UTC + + } + + ); +}; + +export default TwoFactorCheckbox; diff --git a/resources/js/admin/edit_user/edit_user.js b/resources/js/admin/edit_user/edit_user.js index ca6adf34a..93e1f88eb 100644 --- a/resources/js/admin/edit_user/edit_user.js +++ b/resources/js/admin/edit_user/edit_user.js @@ -1,4 +1,4 @@ -import React, {useState} from "react"; +import React, {useRef, useState} from "react"; import ReactDOM from "react-dom"; import ArrowBack from '@material-ui/icons/ArrowBack'; import Button from "@material-ui/core/Button"; @@ -9,6 +9,7 @@ import CssBaseline from "@material-ui/core/CssBaseline"; import Checkbox from "@material-ui/core/Checkbox"; import Grid from "@material-ui/core/Grid"; import GroupsInput from "./components/groups_input"; +import TwoFactorCheckbox from "./components/two_factor_checkbox"; import MenuItem from "@material-ui/core/MenuItem"; import TextField from "@material-ui/core/TextField"; import Typography from "@material-ui/core/Typography"; @@ -41,6 +42,7 @@ const EditUserPage = ({ csrfToken, fetchGroupsURL, initialValues, + isOwnAccount = false, languages, menuConfig, passwordPolicy, @@ -73,12 +75,17 @@ const EditUserPage = ({ }); } + // 2FA flag as last loaded/saved: save() sends two_factor_enabled only when it differs + const twoFactorBaseline = useRef(!!initialValues?.two_factor_enabled); + const formik = useFormik({ initialValues: initialValues, validationSchema: buildValidationSchema(), onSubmit: (values) => { setLoading(true); - save({...values, groups: selectedGroups}, pic , csrfToken).then((res) => { + const twoFactorSubmitted = !!values.two_factor_enabled; + save({...values, groups: selectedGroups}, pic , csrfToken, twoFactorBaseline.current).then((res) => { + twoFactorBaseline.current = twoFactorSubmitted; setLoading(false); Swal("User saved", "The user has been saved successfully", "success"); }).catch((err) => { @@ -733,6 +740,12 @@ const EditUserPage = ({ color="primary"/>} label="Email Verified?" /> + { - const [enabled, setEnabled] = useState(twoFactorEnabled); + const [enabled, setEnabled] = useState(twoFactorEnabled || twoFactorEnforced); const [loading, setLoading] = useState(false); // Standing and first batch returned by "Enable": the panel is mounted with them and // asks the user to confirm the batch before it counts as usable. const [enrollment, setEnrollment] = useState(null); + const [confirmingDisable, setConfirmingDisable] = useState(false); + const [currentPassword, setCurrentPassword] = useState(""); + // Shown inside the dialog: a SweetAlert opens below a MUI Dialog (z-index 1060 vs 1300), + // so the usual error popup would be hidden behind it. + const [disableError, setDisableError] = useState(null); + const handleEnable = () => { setLoading(true); enableTwoFactor(DEFAULT_METHOD).then(({response}) => { @@ -38,27 +56,139 @@ const TwoFactorSection = ({ }); }; - if (!enabled) { - return ( - <> - - Two-factor authentication is not enabled for your account. + const closeDisableDialog = () => { + setConfirmingDisable(false); + setCurrentPassword(""); + setDisableError(null); + }; + + // A user without a password sends none: the server accepts the request only inside + // the recovery enrollment window. + const handleDisable = () => { + setLoading(true); + setDisableError(null); + disableTwoFactor(hasPassword ? currentPassword : null).then(() => { + setLoading(false); + closeDisableDialog(); + setEnrollment(null); + setEnabled(false); + }).catch((err) => { + setLoading(false); + // the dialog stays open so a mistyped password can be retried + handleErrorResponse(err, (title, message) => { + setDisableError(Array.isArray(message) ? message.join(" ") : (message || title)); + }); + }); + }; + + const handleToggle = (e) => { + if (twoFactorEnforced || loading) return; + if (e.target.checked) { + handleEnable(); + } else { + setConfirmingDisable(true); + } + }; + + const toggle = ( + + + } + label="Two-Factor Authentication (Email OTP)" + /> + { + twoFactorEnforced && + + {ENFORCED_MESSAGE} + + } + + ); + + // Rendered in a portal and driven by onClick only: the profile page wraps everything + // in one
, so this dialog must never submit it. + const disableDialog = ( + + Turn off two-factor authentication? + + + Your recovery codes will be deleted and every trusted device will be removed. + {hasPassword ? " Enter your current password to confirm." : ""} + { + hasPassword && + { + setCurrentPassword(e.target.value); + setDisableError(null); + }} + onKeyDown={(e) => { + if (e.key === "Enter") { + e.preventDefault(); + if (currentPassword && !loading) handleDisable(); + } + }} + inputProps={{form: "disable-two-factor-detached-form", autoComplete: "off"}} + data-testid="disable-two-factor-current-password" + /> + } + { + disableError && + + {disableError} + + } + + + + + + ); + + if (!enabled) { + return ( + <> + {toggle} + + Two-factor authentication is not enabled for your account. + ); } if (enrollment) { return ( + <> + {toggle} + {disableDialog} + ); } return ( + <> + {toggle} + {disableDialog} + ); }; diff --git a/resources/js/profile/actions.js b/resources/js/profile/actions.js index e853e7d8d..ccf458373 100644 --- a/resources/js/profile/actions.js +++ b/resources/js/profile/actions.js @@ -113,6 +113,13 @@ export const enableTwoFactor = async (method) => { return postRawRequestFull(window.ENABLE_TWO_FACTOR_ENDPOINT)(params, {'X-CSRF-TOKEN': window.CSFR_TOKEN}); } +// currentPassword is optional: a user without a password (social-only account) disables +// inside the recovery enrollment window instead of confirming one. +export const disableTwoFactor = async (currentPassword = null) => { + const params = currentPassword ? {current_password: currentPassword} : {}; + return postRawRequestFull(window.DISABLE_TWO_FACTOR_ENDPOINT)(params, jsonHeaders()); +} + 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 600bfbe43..4a46a4757 100644 --- a/resources/js/profile/profile.js +++ b/resources/js/profile/profile.js @@ -47,6 +47,7 @@ const ProfilePage = ({ passwordPolicy, redirectUri, twoFactorEnabled, + twoFactorEnforced, recoveryCodesRemaining, recoveryCodesTotal, recoveryCodesLowThreshold, @@ -792,6 +793,7 @@ const ProfilePage = ({ { } return showMessage("Validation error", messageLines, "warning"); } + if (err.status === 429) { + // rate limited (e.g. too many failed password confirmations) + const message = err.response?.body?.error_message || "Too many attempts. Please try again later."; + return showMessage("Too many attempts", message, "warning"); + } return showMessage("Something went wrong!", null, "error"); }; }; diff --git a/resources/views/admin/edit-user.blade.php b/resources/views/admin/edit-user.blade.php index cdb7b24c7..771b2c76d 100644 --- a/resources/views/admin/edit-user.blade.php +++ b/resources/views/admin/edit-user.blade.php @@ -84,6 +84,7 @@ csrfToken: document.head.querySelector('meta[name="csrf-token"]').content, fetchGroupsURL: '{!!URL::action("Api\GroupApiController@getAll")!!}', initialValues: initialValues, + isOwnAccount: {{ $is_own_account ? 'true' : 'false' }}, languages: languages, menuConfig: menuConfig, passwordPolicy: passwordPolicy, diff --git a/resources/views/emails/auth/two_factor_enabled_by_admin.blade.php b/resources/views/emails/auth/two_factor_enabled_by_admin.blade.php new file mode 100644 index 000000000..fce10b102 --- /dev/null +++ b/resources/views/emails/auth/two_factor_enabled_by_admin.blade.php @@ -0,0 +1,28 @@ +@extends('emails.email_layout') + +@section('content') + + + + + + + + + + + + + + + +
+
Dear {{ $user_fullname }},
+
+
Two-factor authentication was enabled on your {{ Config::get('app.app_name') }} account by an administrator. From now on you will be asked for a verification code sent to {{ $user_email }} when you sign in.
+
+
Please generate your recovery codes from your profile page and keep them in a safe place: they let you sign in if you cannot receive the verification code.
+
+
If you did not expect this change please email {{ Config::get('app.help_email') }}.
+
+@stop diff --git a/resources/views/profile.blade.php b/resources/views/profile.blade.php index 294dcedcc..919eeb860 100644 --- a/resources/views/profile.blade.php +++ b/resources/views/profile.blade.php @@ -105,6 +105,7 @@ menuConfig: menuConfig, passwordPolicy: passwordPolicy, twoFactorEnabled: {{ $two_factor_enabled ? 'true' : 'false' }}, + twoFactorEnforced: {{ $two_factor_enforced ? 'true' : 'false' }}, recoveryCodesRemaining: {{ (int) $recovery_codes_remaining }}, recoveryCodesTotal: {{ (int) $recovery_codes_total }}, recoveryCodesLowThreshold: {{ (int) $recovery_codes_low_threshold }}, @@ -122,6 +123,7 @@ 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.DISABLE_TWO_FACTOR_ENDPOINT = '{!!URL::action("Api\UserApiController@disableTwoFactor")!!}'; window.ENROLL_RECOVERY_CODES_ENDPOINT = '{!!URL::action("Api\UserApiController@enrollRecoveryCodes")!!}'; window.ACKNOWLEDGE_RECOVERY_CODES_ENDPOINT = '{!!URL::action("Api\UserApiController@acknowledgeRecoveryCodes")!!}'; window.CSFR_TOKEN = document.head.querySelector('meta[name="csrf-token"]').content; diff --git a/routes/web.php b/routes/web.php index 281351c29..bbe4c50b5 100644 --- a/routes/web.php +++ b/routes/web.php @@ -198,10 +198,11 @@ Route::put('', "UserApiController@updateMe"); Route::put('pic', "UserApiController@updateMyPic"); Route::get('actions', "UserActionApiController@getActionsByCurrentUser"); - Route::post('recovery-codes/regenerate', ['middleware' => ['csrf'], 'uses' => "UserApiController@regenerateRecoveryCodes"]); + Route::post('recovery-codes/regenerate', ['middleware' => ['csrf', '2fa.rate:password_confirm'], 'uses' => "UserApiController@regenerateRecoveryCodes"]); Route::post('recovery-codes/enroll', ['middleware' => ['csrf', '2fa.rate:enroll'], 'uses' => "UserApiController@enrollRecoveryCodes"]); Route::post('recovery-codes/acknowledge', ['middleware' => ['csrf'], 'uses' => "UserApiController@acknowledgeRecoveryCodes"]); Route::post('2fa/enable', ['middleware' => ['csrf'], 'uses' => "UserApiController@enableTwoFactor"]); + Route::post('2fa/disable', ['middleware' => ['csrf', '2fa.rate:password_confirm'], 'uses' => "UserApiController@disableTwoFactor"]); }); Route::get('access-tokens', ['middleware' => ['openstackid.currentuser.serveradmin.json'], 'uses' => 'ClientApiController@getAllAccessTokens']); diff --git a/tests/AdminUserTwoFactorUpdateTest.php b/tests/AdminUserTwoFactorUpdateTest.php new file mode 100644 index 000000000..749f83c82 --- /dev/null +++ b/tests/AdminUserTwoFactorUpdateTest.php @@ -0,0 +1,258 @@ +withoutMiddleware(); + $this->admin = EntityManager::getRepository(User::class)->findOneBy(['identifier' => 'sebastian.marcet']); + $this->be($this->admin); + Session::start(); + } + + private function plainUser(bool $enrolled, bool $email_verified = true): User + { + $user = UserFactory::build([ + 'first_name' => 'Plain', + 'last_name' => 'Target', + 'email' => 'target.' . uniqid() . '@test.invalid', + 'active' => true, + 'email_verified' => $email_verified, + 'identifier' => 'target.' . uniqid(), + 'password' => '1Qaz2wsx!', + 'password_enc' => AuthHelper::AlgSHA1_V2_4, + ]); + EntityManager::persist($user); + EntityManager::flush(); + if ($enrolled) { + $this->app->make(IRecoveryCodeService::class)->enableTwoFactorAndGenerateCodes($user, User::MFAMethod_OTP); + $this->app->make(IDeviceTrustService::class)->trustDevice($user, 'agent-a', '127.0.0.1'); + } + return $user; + } + + private function updateUser(User $target, array $payload) + { + return $this->action('PUT', 'Api\\UserApiController@update', ['id' => $target->getId()], $payload, [], []); + } + + private function row(User $user): object + { + return DB::table('users')->where('id', $user->getId())->first(); + } + + private function lastAudit(User $user): ?array + { + $row = DB::table('two_factor_audit_log')->where('user_id', $user->getId()) + ->where('event_type', 'enrollment_changed')->orderByDesc('id')->first(); + return $row ? json_decode($row->metadata, true) : null; + } + + public function testAdminEnablesTwoFactorWithoutRecoveryCodesAndNotifiesTheUser(): void + { + $user = $this->plainUser(false); + + $this->updateUser($user, ['two_factor_enabled' => 1]); + + $this->assertResponseStatus(201); + $row = $this->row($user); + $this->assertSame(1, (int)$row->two_factor_enabled); + $this->assertNotNull($row->two_factor_enforced_at); + $this->assertSame(0, DB::table('user_recovery_codes')->where('user_id', $user->getId())->count()); + $audit = $this->lastAudit($user); + $this->assertTrue($audit['enabled']); + $this->assertSame($this->admin->getId(), $audit['actor_id']); + $this->assertSame('admin', $audit['reason']); + Mail::assertQueued(TwoFactorEnabledByAdminMail::class, 1); + } + + public function testAdminDisablesTwoFactorWithTheSameSideEffectsAsSelfService(): void + { + $user = $this->plainUser(true); + + $this->updateUser($user, ['two_factor_enabled' => 0]); + + $this->assertResponseStatus(201); + $row = $this->row($user); + $this->assertSame(0, (int)$row->two_factor_enabled); + $this->assertNull($row->two_factor_enforced_at); + $this->assertSame(0, DB::table('user_recovery_codes')->where('user_id', $user->getId())->count()); + $this->assertSame(0, DB::table('user_trusted_devices') + ->where('user_id', $user->getId())->where('is_revoked', 0)->count()); + $audit = $this->lastAudit($user); + $this->assertFalse($audit['enabled']); + $this->assertSame($this->admin->getId(), $audit['actor_id']); + $this->assertSame('admin', $audit['reason']); + Mail::assertNotQueued(TwoFactorEnabledByAdminMail::class); + } + + public function testDisablingAGroupEnforcedUserReturns412AndSavesNothing(): void + { + $user = $this->plainUser(true); + $group = $this->app->make(IGroupRepository::class)->getOneBySlug(IGroupSlugs::SponsorUsersGroup); + $user->addToGroup($group); + EntityManager::flush(); + Config::set('two_factor.enforced_groups', [IGroupSlugs::SponsorUsersGroup]); + + $this->updateUser($user, ['two_factor_enabled' => 0, 'first_name' => 'Renamed']); + + $this->assertResponseStatus(412); + EntityManager::clear(); + $this->assertSame('Plain', $this->row($user)->first_name, 'the rest of the update must be rolled back'); + $this->assertSame(1, (int)$this->row($user)->two_factor_enabled); + $this->assertSame(10, DB::table('user_recovery_codes')->where('user_id', $user->getId())->count()); + } + + public function testDisablingWhileAddingAnEnforcedGroupInTheSameSaveReturns412AndSavesNothing(): void + { + $user = $this->plainUser(true); + $group = $this->app->make(IGroupRepository::class)->getOneBySlug(IGroupSlugs::SponsorUsersGroup); + Config::set('two_factor.enforced_groups', [IGroupSlugs::SponsorUsersGroup]); + + $this->updateUser($user, [ + 'two_factor_enabled' => 0, + 'first_name' => 'Renamed', + 'groups' => [$group->getId()], + ]); + + $this->assertResponseStatus(412); + EntityManager::clear(); + $this->assertSame('Plain', $this->row($user)->first_name, 'the rest of the update must be rolled back'); + $this->assertSame(0, DB::table('user_groups') + ->where('user_id', $user->getId())->where('group_id', $group->getId())->count(), + 'the group change must be rolled back'); + $this->assertSame(1, (int)$this->row($user)->two_factor_enabled); + $this->assertSame(10, DB::table('user_recovery_codes')->where('user_id', $user->getId())->count()); + } + + public function testDisablingWhileRemovingTheEnforcedGroupInTheSameSaveSucceeds(): void + { + $user = $this->plainUser(true); + $group = $this->app->make(IGroupRepository::class)->getOneBySlug(IGroupSlugs::SponsorUsersGroup); + $user->addToGroup($group); + EntityManager::flush(); + Config::set('two_factor.enforced_groups', [IGroupSlugs::SponsorUsersGroup]); + + // the admin form always sends the full list: the enforced group is no longer in it + $raw = $this->app->make(IGroupRepository::class)->getOneBySlug(IGroupSlugs::RawUsersGroup); + $this->updateUser($user, ['two_factor_enabled' => 0, 'groups' => [$raw->getId()]]); + + $this->assertResponseStatus(201); + EntityManager::clear(); + $this->assertSame(0, DB::table('user_groups') + ->where('user_id', $user->getId())->where('group_id', $group->getId())->count()); + $this->assertSame(0, (int)$this->row($user)->two_factor_enabled); + $this->assertSame(0, DB::table('user_recovery_codes')->where('user_id', $user->getId())->count()); + } + + public function testEnablingForAnUnverifiedEmailReturns412AndSavesNothing(): void + { + $user = $this->plainUser(false, false); + + $this->updateUser($user, ['two_factor_enabled' => 1, 'first_name' => 'Renamed']); + + $this->assertResponseStatus(412); + EntityManager::clear(); + $this->assertSame('Plain', $this->row($user)->first_name, 'the rest of the update must be rolled back'); + $this->assertSame(0, (int)$this->row($user)->two_factor_enabled); + Mail::assertNotQueued(TwoFactorEnabledByAdminMail::class); + } + + public function testEnablingTogetherWithEmailVerifiedInTheSameSaveSucceeds(): void + { + $user = $this->plainUser(false, false); + + $this->updateUser($user, ['two_factor_enabled' => 1, 'email_verified' => 1]); + + $this->assertResponseStatus(201); + $this->assertSame(1, (int)$this->row($user)->two_factor_enabled); + Mail::assertQueued(TwoFactorEnabledByAdminMail::class, 1); + } + + public function testAPayloadWithoutTheKeyLeavesTwoFactorUntouched(): void + { + $user = $this->plainUser(true); + + $this->updateUser($user, ['first_name' => 'Renamed']); + + $this->assertResponseStatus(201); + $this->assertSame(1, (int)$this->row($user)->two_factor_enabled); + $this->assertSame(10, DB::table('user_recovery_codes')->where('user_id', $user->getId())->count()); + $this->assertSame(0, DB::table('user_trusted_devices') + ->where('user_id', $user->getId())->where('is_revoked', 1)->count()); + Mail::assertNotQueued(TwoFactorEnabledByAdminMail::class); + } + + public function testAnUnchangedValueIsIgnored(): void + { + $user = $this->plainUser(true); + + $this->updateUser($user, ['first_name' => 'Renamed', 'two_factor_enabled' => 1]); + + $this->assertResponseStatus(201); + $this->assertSame(10, DB::table('user_recovery_codes')->where('user_id', $user->getId())->count()); + $this->assertSame(1, DB::table('two_factor_audit_log') + ->where('user_id', $user->getId())->where('event_type', 'enrollment_changed')->count(), + 'only the enrollment of the fixture, no second row'); + Mail::assertNotQueued(TwoFactorEnabledByAdminMail::class); + } + + public function testAUserCannotTurnOffTheirOwnTwoFactorThroughTheProfileUpdate(): void + { + $user = $this->plainUser(true); + $this->be($user); + + $this->action('PUT', 'Api\\UserApiController@updateMe', [], ['two_factor_enabled' => 0], [], []); + + $this->assertSame(1, (int)$this->row($user)->two_factor_enabled); + $this->assertSame(10, DB::table('user_recovery_codes')->where('user_id', $user->getId())->count()); + } + + public function testAnAdminCannotChangeTheirOwnTwoFactorThroughTheUserCrud(): void + { + $admin = $this->plainUser(true); + $group = $this->app->make(IGroupRepository::class)->getOneBySlug(IGroupSlugs::SuperAdminGroup); + $admin->addToGroup($group); + EntityManager::flush(); + $this->be($admin); + + $this->updateUser($admin, ['two_factor_enabled' => 0]); + + $this->assertSame(1, (int)$this->row($admin)->two_factor_enabled); + } +} diff --git a/tests/DisableTwoFactorEndpointTest.php b/tests/DisableTwoFactorEndpointTest.php new file mode 100644 index 000000000..36de5807e --- /dev/null +++ b/tests/DisableTwoFactorEndpointTest.php @@ -0,0 +1,241 @@ +withoutMiddleware(); + Session::start(); + } + + private function enrolledUser(bool $withPassword = true): User + { + $payload = [ + 'first_name' => 'Disable', + 'last_name' => 'Endpoint', + 'email' => 'disable.' . uniqid() . '@test.invalid', + 'active' => true, + 'email_verified' => true, + 'identifier' => 'disable.' . uniqid(), + ]; + if ($withPassword) { + $payload['password'] = self::SEED_PASSWORD; + $payload['password_enc'] = AuthHelper::AlgSHA1_V2_4; + } + $user = UserFactory::build($payload); + EntityManager::persist($user); + EntityManager::flush(); + + $this->app->make(IRecoveryCodeService::class) + ->enableTwoFactorAndGenerateCodes($user, User::MFAMethod_OTP); + $this->app->make(IDeviceTrustService::class)->trustDevice($user, 'agent-a', '127.0.0.1'); + $this->be($user); + return $user; + } + + private function disable(?string $password) + { + return $this->action('POST', 'Api\\UserApiController@disableTwoFactor', + is_null($password) ? [] : ['current_password' => $password], [], [], []); + } + + private function row(User $user): object + { + return DB::table('users')->where('id', $user->getId())->first(); + } + + private function assertStillEnrolled(User $user): void + { + $this->assertSame(1, (int)$this->row($user)->two_factor_enabled); + $this->assertSame(10, DB::table('user_recovery_codes')->where('user_id', $user->getId())->count()); + $this->assertSame(0, DB::table('user_trusted_devices') + ->where('user_id', $user->getId())->where('is_revoked', 1)->count()); + } + + public function testDisableWithCorrectPasswordClearsEverything(): void + { + $user = $this->enrolledUser(); + + $response = $this->disable(self::SEED_PASSWORD); + + $this->assertResponseStatus(200); + $this->assertSame(['two_factor_enabled' => false], json_decode($response->getContent(), true)); + + $row = $this->row($user); + $this->assertSame(0, (int)$row->two_factor_enabled); + $this->assertNull($row->two_factor_enforced_at); + $this->assertSame(0, DB::table('user_recovery_codes')->where('user_id', $user->getId())->count()); + $devices = DB::table('user_trusted_devices')->where('user_id', $user->getId())->get(); + $this->assertNotEmpty($devices); + foreach ($devices as $device) { + $this->assertSame(1, (int)$device->is_revoked); + } + + $audit = DB::table('two_factor_audit_log') + ->where('user_id', $user->getId()) + ->where('event_type', TwoFactorAuditLog::EventEnrollmentChanged) + ->orderByDesc('id')->first(); + $metadata = json_decode($audit->metadata, true); + $this->assertFalse($metadata['enabled']); + $this->assertSame($user->getId(), $metadata['actor_id']); + $this->assertSame('self', $metadata['reason']); + } + + public function testDisableExpiresTheDeviceTrustCookie(): void + { + $this->enrolledUser(); + + $this->disable(self::SEED_PASSWORD); + + $this->assertResponseStatus(200); + $name = config('two_factor.cookie_name', 'device_trust_token'); + $queued = array_filter(Cookie::getQueuedCookies(), fn($c) => $c->getName() === $name); + $this->assertCount(1, $queued); + $this->assertLessThan(time(), array_values($queued)[0]->getExpiresTime()); + } + + public function testDisableWithWrongPasswordChangesNothing(): void + { + $user = $this->enrolledUser(); + + $this->disable('not-the-password'); + + $this->assertResponseStatus(412); + $this->assertStillEnrolled($user); + } + + public function testDisableWithoutPasswordIsRejectedForUserWithPassword(): void + { + $user = $this->enrolledUser(); + + $this->disable(null); + + $this->assertResponseStatus(412); + $this->assertStillEnrolled($user); + } + + public function testDisableUnauthenticatedReturns403(): void + { + $this->enrolledUser(); + Auth::logout(); + + $this->disable(self::SEED_PASSWORD); + + $this->assertResponseStatus(403); + } + + public function testDisableWhenTwoFactorIsNotEnabledReturns412(): void + { + $user = $this->enrolledUser(); + $this->disable(self::SEED_PASSWORD); + $this->assertResponseStatus(200); + + $this->disable(self::SEED_PASSWORD); + + $this->assertResponseStatus(412); + } + + public function testDisableForGroupEnforcedUserReturns412AndChangesNothing(): void + { + $admin = EntityManager::getRepository(User::class)->findOneBy(['identifier' => 'sebastian.marcet']); + $this->assertTrue($admin->isTwoFactorEnforcedByGroup(), 'seeded admin must be in an enforced group'); + $this->app->make(IRecoveryCodeService::class)->generateRecoveryCodes($admin); + $before = DB::table('user_recovery_codes')->where('user_id', $admin->getId())->count(); + $this->be($admin); + + $this->disable(self::SEED_PASSWORD); + + $this->assertResponseStatus(412); + $this->assertSame($before, DB::table('user_recovery_codes')->where('user_id', $admin->getId())->count()); + EntityManager::clear(); + $this->assertTrue(EntityManager::getRepository(User::class)->find($admin->getId())->shouldRequire2FA()); + } + + public function testDisableForUserWithoutPasswordInsideTheWindow(): void + { + $user = $this->enrolledUser(false); + Session::put(MFAConstants::SESSION_KEY_COMPLETED_AT, ['user_id' => $user->getId(), 'at' => time() - 30]); + + $this->disable(null); + + $this->assertResponseStatus(200); + $this->assertSame(0, (int)$this->row($user)->two_factor_enabled); + } + + public function testDisableForUserWithoutPasswordOutsideTheWindow(): void + { + $user = $this->enrolledUser(false); + $window = (int)config('two_factor.recovery_enrollment_window', 600); + Session::put(MFAConstants::SESSION_KEY_COMPLETED_AT, ['user_id' => $user->getId(), 'at' => time() - $window - 5]); + + $this->disable(null); + + $this->assertResponseStatus(412); + $this->assertStillEnrolled($user); + } + + public function testAdminSerializerExposesTheTwoFactorFieldsAndPrivateAndPublicDoNot(): void + { + $user = $this->enrolledUser(); + $keys = ['two_factor_enabled', 'two_factor_method', 'two_factor_enforced', 'two_factor_enforced_at']; + $registry = SerializerRegistry::getInstance(); + + $admin = $registry->getSerializer($user, SerializerRegistry::SerializerType_Admin)->serialize(); + foreach ($keys as $key) { + $this->assertArrayHasKey($key, $admin); + } + $this->assertTrue($admin['two_factor_enabled']); + $this->assertSame(User::MFAMethod_OTP, $admin['two_factor_method']); + $this->assertFalse($admin['two_factor_enforced']); + $this->assertNotNull($admin['two_factor_enforced_at']); + // the admin serializer is the private one plus the 2FA state + $this->assertSame([], array_diff( + array_keys($registry->getSerializer($user, SerializerRegistry::SerializerType_Private)->serialize()), + array_keys($admin) + )); + + // OAuth2 clients get the private serializer, public is public: neither has the 2FA state + foreach ([SerializerRegistry::SerializerType_Private, SerializerRegistry::SerializerType_Public] as $type) { + $payload = $registry->getSerializer($user, $type)->serialize(); + foreach ($keys as $key) { + $this->assertArrayNotHasKey($key, $payload, "$type must not expose $key"); + } + } + } +} diff --git a/tests/OAuth2UserUpdateApiTest.php b/tests/OAuth2UserUpdateApiTest.php index 24e3094c1..d9b2b0e3f 100644 --- a/tests/OAuth2UserUpdateApiTest.php +++ b/tests/OAuth2UserUpdateApiTest.php @@ -13,9 +13,13 @@ **/ use App\libs\Auth\Models\IGroupSlugs; +use App\libs\Auth\Factories\UserFactory; use App\libs\OAuth2\IUserScopes; +use App\Services\Auth\IRecoveryCodeService; +use Auth\AuthHelper; use Auth\Group; use Auth\User; +use Illuminate\Support\Facades\DB; use LaravelDoctrine\ORM\Facades\EntityManager; /** @@ -107,6 +111,50 @@ public function testUserUpdate() $this->assertTrue($response->first_name == $new_first_name); } + /** + * The token owner is a super admin, but 2FA must not be manageable by an OAuth2 client: + * the key is dropped from the payload and the rest of the update goes through. + */ + public function testUserUpdateIgnoresTwoFactorEnabled() + { + $target = UserFactory::build([ + 'first_name' => 'Plain', + 'last_name' => 'Target', + 'email' => 'target.' . uniqid() . '@test.invalid', + 'active' => true, + 'email_verified' => true, + 'identifier' => 'target.' . uniqid(), + 'password' => '1Qaz2wsx!', + 'password_enc' => AuthHelper::AlgSHA1_V2_4, + ]); + EntityManager::persist($target); + EntityManager::flush(); + $this->app->make(IRecoveryCodeService::class)->enableTwoFactorAndGenerateCodes($target, User::MFAMethod_OTP); + + $headers = [ + "HTTP_Authorization" => " Bearer " . $this->access_token, + "CONTENT_TYPE" => "application/json" + ]; + + $this->action + ( + "PUT", + "Api\\OAuth2\\OAuth2UserApiController@update", + ['id' => $target->getId()], + [], + [], + [], + $headers, + json_encode(['first_name' => 'Renamed', 'two_factor_enabled' => false]) + ); + + $this->assertResponseStatus(201); + $row = DB::table('users')->where('id', $target->getId())->first(); + $this->assertSame('Renamed', $row->first_name); + $this->assertSame(1, (int)$row->two_factor_enabled); + $this->assertSame(10, DB::table('user_recovery_codes')->where('user_id', $target->getId())->count()); + } + protected function getScopes() { return [ diff --git a/tests/TwoFactorLoginFlowTest.php b/tests/TwoFactorLoginFlowTest.php index 54294be05..a3c31cb7a 100644 --- a/tests/TwoFactorLoginFlowTest.php +++ b/tests/TwoFactorLoginFlowTest.php @@ -80,7 +80,7 @@ private function flushRateLimitCounters(): void if ($admin) { $userId = $admin->getId(); $prefix = ITwoFactorRateLimitService::RATE_LIMIT_CACHE_KEY_PREFIX; - foreach (['verify', 'recovery', 'resend', 'enroll'] as $action) { + foreach (['verify', 'recovery', 'resend', 'enroll', 'password_confirm'] as $action) { Cache::forget("{$prefix}{$action}:{$userId}"); // RateLimiter::hit() also writes a companion ":timer" key holding // the window's reset timestamp - must be cleared too, or a stale @@ -191,6 +191,44 @@ public function testNonAdminWithoutMFALogsInNormally(): void $this->assertTrue(Auth::check(), 'a non-MFA user must get an authenticated session'); } + public function testPasswordLoginNoLongerHitsTheChallengeAfterTheUserDisabledTwoFactor(): void + { + $email = $this->createPlainUser(); + $user = $this->user($email); + $recovery = $this->app->make(IRecoveryCodeService::class); + $recovery->enableTwoFactorAndGenerateCodes($user, User::MFAMethod_OTP); + + // control: enrolled, the password alone does not log in + $this->postLogin($email, self::SEED_PASSWORD); + $this->assertFalse(Auth::check()); + $this->assertSame('2fa', Session::get('flow')); + $this->resetLogin(); + + $recovery->disableTwoFactor($user, self::SEED_PASSWORD, $user); + + $this->postLogin($email, self::SEED_PASSWORD); + + $this->assertResponseStatus(302); + $this->assertTrue(Auth::check(), 'with 2FA off the password alone must log the user in'); + $this->assertNotSame('2fa', Session::get('flow'), 'no 2FA screen must be pending'); + } + + public function testGroupEnforcedUserIsStillChallengedAfterARefusedDisable(): void + { + $admin = $this->user(self::ADMIN_EMAIL); + + try { + $this->app->make(IRecoveryCodeService::class)->disableTwoFactor($admin, self::SEED_PASSWORD, $admin); + $this->fail('a group-enforced user must not be able to disable 2FA'); + } catch (\models\exceptions\ValidationException $ex) { + } + + $this->postLogin(self::ADMIN_EMAIL, self::SEED_PASSWORD); + + $this->assertFalse(Auth::check(), 'enforcement does not depend on the stored flag'); + $this->assertSame('2fa', Session::get('flow')); + } + // ------------------------------------------------------------------------- // passwordless (flow=otp) login must not bypass the MFA gate // ------------------------------------------------------------------------- @@ -1224,6 +1262,31 @@ public function testEnrollRateLimitIsKeyedByTheAuthenticatedUserAndAppliesRightA $this->assertSame((string)$max, $response->headers->get('X-RateLimit-Limit')); } + public function testPasswordConfirmationRateLimitIsSharedByDisableAndRegenerateAndCountsOnlyFailures(): void + { + $admin = $this->user(self::ADMIN_EMAIL); + $this->be($admin); + $max = (int)config('two_factor.rate_limit.max_password_confirm_attempts', 5); + + // a successful confirmation gives its attempt back + $this->assertEquals(200, $this->regenerate(self::SEED_PASSWORD)->getStatusCode()); + $this->assertSame(0, (int)Cache::get(ITwoFactorRateLimitService::RATE_LIMIT_CACHE_KEY_PREFIX . 'password_confirm:' . $admin->getId(), 0)); + + // failures on either route draw from the same budget + for ($i = 0; $i < $max; $i++) { + $response = $i % 2 === 0 + ? $this->regenerate('not-the-password') + : $this->disableTwoFactor('not-the-password'); + $this->assertEquals(412, $response->getStatusCode(), "attempt #" . ($i + 1) . " must reach the controller"); + } + + // the right password no longer gets through, on either route + $response = $this->regenerate(self::SEED_PASSWORD); + $this->assertEquals(429, $response->getStatusCode()); + $this->assertSame((string)$max, $response->headers->get('X-RateLimit-Limit')); + $this->assertEquals(429, $this->disableTwoFactor(self::SEED_PASSWORD)->getStatusCode()); + } + public function testAcknowledgeWithAnEarlierBatchIdIsRejectedAndLeavesTheColumnUnchanged(): void { $admin = $this->user(self::ADMIN_EMAIL); @@ -1325,6 +1388,22 @@ private function enroll() return $this->action('POST', 'Api\\UserApiController@enrollRecoveryCodes', [], ['_token' => Session::token()]); } + private function regenerate(string $password) + { + return $this->action('POST', 'Api\\UserApiController@regenerateRecoveryCodes', [], [ + 'current_password' => $password, + '_token' => Session::token(), + ]); + } + + private function disableTwoFactor(string $password) + { + return $this->action('POST', 'Api\\UserApiController@disableTwoFactor', [], [ + 'current_password' => $password, + '_token' => Session::token(), + ]); + } + private function acknowledge(int $batchId) { return $this->action('POST', 'Api\\UserApiController@acknowledgeRecoveryCodes', [], [ diff --git a/tests/TwoFactorProfileRoutesCsrfTest.php b/tests/TwoFactorProfileRoutesCsrfTest.php index a11cd04a9..984365e01 100644 --- a/tests/TwoFactorProfileRoutesCsrfTest.php +++ b/tests/TwoFactorProfileRoutesCsrfTest.php @@ -43,6 +43,7 @@ public static function stateChangingRoutes(): array { return [ 'enable 2fa' => ['/admin/api/v1/users/me/2fa/enable'], + 'disable 2fa' => ['/admin/api/v1/users/me/2fa/disable'], 'regenerate recovery codes' => ['/admin/api/v1/users/me/recovery-codes/regenerate'], 'enroll recovery codes' => ['/admin/api/v1/users/me/recovery-codes/enroll'], 'acknowledge recovery codes' => ['/admin/api/v1/users/me/recovery-codes/acknowledge'], diff --git a/tests/js/admin/edit_user_actions.test.js b/tests/js/admin/edit_user_actions.test.js new file mode 100644 index 000000000..91fc7169c --- /dev/null +++ b/tests/js/admin/edit_user_actions.test.js @@ -0,0 +1,36 @@ +import {normalizeEntity} from '../../../resources/js/admin/edit_user/actions'; + +const entity = (overrides = {}) => ({ + public_profile_show_photo: true, + public_profile_show_fullname: true, + public_profile_show_email: false, + public_profile_allow_chat_with_me: false, + active: true, + email_verified: true, + post_code: 12345, + groups: ['1', '2'], + two_factor_enabled: false, + two_factor_enforced: false, + two_factor_enforced_at: null, + two_factor_method: null, + ...overrides, +}); + +describe('admin edit user normalizeEntity', () => { + it('omits two_factor_enabled when the admin did not change the box', () => { + expect(normalizeEntity(entity({two_factor_enabled: false}), false)).not.toHaveProperty('two_factor_enabled'); + expect(normalizeEntity(entity({two_factor_enabled: true}), true)).not.toHaveProperty('two_factor_enabled'); + }); + + it('sends the new value as 1/0 when the admin changed the box', () => { + expect(normalizeEntity(entity({two_factor_enabled: true}), false).two_factor_enabled).toBe(1); + expect(normalizeEntity(entity({two_factor_enabled: false}), true).two_factor_enabled).toBe(0); + }); + + it('never sends the read only 2FA state', () => { + const out = normalizeEntity(entity({two_factor_enforced: true, two_factor_enforced_at: 1, two_factor_method: 'otp'}), false); + expect(out).not.toHaveProperty('two_factor_enforced'); + expect(out).not.toHaveProperty('two_factor_enforced_at'); + expect(out).not.toHaveProperty('two_factor_method'); + }); +}); diff --git a/tests/js/admin/two_factor_checkbox.test.js b/tests/js/admin/two_factor_checkbox.test.js new file mode 100644 index 000000000..e901b0d57 --- /dev/null +++ b/tests/js/admin/two_factor_checkbox.test.js @@ -0,0 +1,58 @@ +import React from 'react'; +import {render, screen, fireEvent} from '@testing-library/react'; +import TwoFactorCheckbox, {ENFORCED_BY_GROUP_TEXT, OWN_ACCOUNT_TEXT} from '../../../resources/js/admin/edit_user/components/two_factor_checkbox'; + +describe('admin edit user TwoFactorCheckbox', () => { + it('reflects the stored flag and can be toggled', () => { + const onChange = jest.fn(); + const {rerender} = render(); + const box = screen.getByTestId('two-factor-checkbox'); + expect(box).not.toBeChecked(); + expect(box).not.toBeDisabled(); + expect(screen.getByText('Two-Factor Authentication (Email OTP)')).toBeInTheDocument(); + + fireEvent.click(box); + expect(onChange).toHaveBeenCalledTimes(1); + + rerender(); + expect(screen.getByTestId('two-factor-checkbox')).toBeChecked(); + expect(screen.queryByTestId('two-factor-enforced-text')).not.toBeInTheDocument(); + }); + + it('is checked and disabled with the explanation for a group-enforced user, even with the stored flag off', () => { + const onChange = jest.fn(); + render(); + + const box = screen.getByTestId('two-factor-checkbox'); + expect(box).toBeChecked(); + expect(box).toBeDisabled(); + expect(screen.getByTestId('two-factor-enforced-text')).toHaveTextContent(ENFORCED_BY_GROUP_TEXT); + expect(ENFORCED_BY_GROUP_TEXT).toBe('Enforced by group membership'); + + fireEvent.click(box); + expect(onChange).not.toHaveBeenCalled(); + }); + + it('is read only on the admin own account, keeping the stored value', () => { + const onChange = jest.fn(); + render(); + + const box = screen.getByTestId('two-factor-checkbox'); + expect(box).toBeChecked(); + expect(box).toBeDisabled(); + expect(screen.getByTestId('two-factor-own-account-text')).toHaveTextContent(OWN_ACCOUNT_TEXT); + expect(screen.queryByTestId('two-factor-enforced-text')).not.toBeInTheDocument(); + + fireEvent.click(box); + expect(onChange).not.toHaveBeenCalled(); + }); + + it('shows when 2FA was enforced as read only text, only when set', () => { + const {rerender} = render(); + expect(screen.queryByTestId('two-factor-enforced-at')).not.toBeInTheDocument(); + + // 2026-07-23 10:30:00 UTC + rerender(); + expect(screen.getByTestId('two-factor-enforced-at')).toHaveTextContent('Enforced since 2026-07-23 10:30 UTC'); + }); +}); diff --git a/tests/js/components/two_factor_section.test.js b/tests/js/components/two_factor_section.test.js index c47708784..4a25b76bf 100644 --- a/tests/js/components/two_factor_section.test.js +++ b/tests/js/components/two_factor_section.test.js @@ -1,10 +1,15 @@ import React from 'react'; -import {render, screen, fireEvent} from '@testing-library/react'; +import {render, screen, fireEvent, waitFor} from '@testing-library/react'; import TwoFactorSection from '../../../resources/js/components/two_factor_section'; -import {enableTwoFactor} from '../../../resources/js/profile/actions'; +import {enableTwoFactor, disableTwoFactor} from '../../../resources/js/profile/actions'; +import {handleErrorResponse} from '../../../resources/js/utils'; jest.mock('../../../resources/js/profile/actions'); jest.mock('sweetalert2', () => jest.fn()); +// the real handler resolves the message and hands it to the custom handler when one is given +jest.mock('../../../resources/js/utils', () => ({ + handleErrorResponse: jest.fn((err, handler) => handler && handler('Validation error', ['current_password: is not correct'], 'warning')), +})); describe('TwoFactorSection', () => { beforeEach(() => { @@ -12,12 +17,13 @@ describe('TwoFactorSection', () => { jest.clearAllMocks(); }); - it('shows the enable button when 2FA is not enabled', () => { + it('shows the switch off when 2FA is not enabled', () => { render( ); - expect(screen.getByTestId('enable-two-factor-button')).toBeInTheDocument(); + expect(screen.getByTestId('two-factor-switch')).not.toBeChecked(); + expect(screen.getByTestId('two-factor-switch')).not.toBeDisabled(); expect(screen.queryByTestId('recovery-codes-count')).not.toBeInTheDocument(); }); @@ -26,10 +32,20 @@ describe('TwoFactorSection', () => { ); - expect(screen.queryByTestId('enable-two-factor-button')).not.toBeInTheDocument(); + expect(screen.getByTestId('two-factor-switch')).toBeChecked(); expect(screen.getByTestId('recovery-codes-count')).toHaveTextContent('Recovery Codes: 7 of 10 remaining'); }); + it('treats a group-enforced user as enabled even when the stored flag is off', () => { + render( + + ); + expect(screen.getByTestId('two-factor-switch')).toBeChecked(); + expect(screen.getByTestId('two-factor-switch')).toBeDisabled(); + expect(screen.getByTestId('recovery-codes-count')).toBeInTheDocument(); + }); + it('shows "not set up yet" instead of a count when the user requires enrollment', () => { render( { email="user@test.com"/> ); - fireEvent.click(screen.getByTestId('enable-two-factor-button')); + fireEvent.click(screen.getByTestId('two-factor-switch')); expect(enableTwoFactor).toHaveBeenCalledWith('email_otp'); expect(await screen.findByText('AAAA-1111')).toBeInTheDocument(); // Not acknowledged yet: no count behind the modal. expect(screen.queryByTestId('recovery-codes-count')).not.toBeInTheDocument(); }); + + it('renders the switch checked and disabled with the explanation for a group-enforced user', () => { + render( + + ); + const toggle = screen.getByTestId('two-factor-switch'); + expect(toggle).toBeChecked(); + expect(toggle).toBeDisabled(); + expect(screen.getByTestId('two-factor-enforced-message')).toHaveTextContent( + 'Two-factor authentication is required for members of an administrator group and cannot be turned off.' + ); + fireEvent.click(toggle); + expect(screen.queryByTestId('disable-two-factor-dialog')).not.toBeInTheDocument(); + }); + + it('does not show the enforced explanation to a self-enrolled user', () => { + render( + + ); + expect(screen.queryByTestId('two-factor-enforced-message')).not.toBeInTheDocument(); + expect(screen.getByTestId('two-factor-switch')).not.toBeDisabled(); + }); + + it('turning the switch off asks for the password, disables 2FA and hides the recovery codes', async () => { + disableTwoFactor.mockResolvedValue({response: {two_factor_enabled: false}}); + render( + + ); + + fireEvent.click(screen.getByTestId('two-factor-switch')); + + expect(await screen.findByTestId('disable-two-factor-dialog')).toBeInTheDocument(); + expect(disableTwoFactor).not.toHaveBeenCalled(); + expect(screen.getByTestId('confirm-disable-two-factor-button')).toBeDisabled(); + + fireEvent.change(screen.getByTestId('disable-two-factor-current-password').querySelector('input'), + {target: {value: 'secret'}}); + fireEvent.click(screen.getByTestId('confirm-disable-two-factor-button')); + + await waitFor(() => expect(disableTwoFactor).toHaveBeenCalledWith('secret')); + await waitFor(() => expect(screen.queryByTestId('recovery-codes-count')).not.toBeInTheDocument()); + expect(screen.getByTestId('two-factor-switch')).not.toBeChecked(); + expect(screen.getByTestId('two-factor-disabled-message')).toBeInTheDocument(); + }); + + it('keeps the dialog open, shows the error inside it and keeps 2FA on when the password is rejected', async () => { + disableTwoFactor.mockRejectedValue({status: 412}); + render( + + ); + + fireEvent.click(screen.getByTestId('two-factor-switch')); + fireEvent.change((await screen.findByTestId('disable-two-factor-current-password')).querySelector('input'), + {target: {value: 'wrong'}}); + fireEvent.click(screen.getByTestId('confirm-disable-two-factor-button')); + + await waitFor(() => expect(disableTwoFactor).toHaveBeenCalledWith('wrong')); + expect(handleErrorResponse).toHaveBeenCalled(); + // shown inside the dialog: a SweetAlert would open below it + expect(await screen.findByTestId('disable-two-factor-error')).toHaveTextContent('current_password: is not correct'); + expect(screen.getByTestId('disable-two-factor-dialog')).toBeInTheDocument(); + expect(screen.getByTestId('two-factor-switch')).toBeChecked(); + expect(screen.getByTestId('recovery-codes-count')).toBeInTheDocument(); + }); + + it('cancelling the dialog leaves 2FA on and sends nothing', async () => { + render( + + ); + + fireEvent.click(screen.getByTestId('two-factor-switch')); + fireEvent.click(await screen.findByTestId('cancel-disable-two-factor-button')); + + expect(disableTwoFactor).not.toHaveBeenCalled(); + expect(screen.getByTestId('two-factor-switch')).toBeChecked(); + }); + + it('a user without a password confirms without typing one', async () => { + disableTwoFactor.mockResolvedValue({response: {two_factor_enabled: false}}); + render( + + ); + + fireEvent.click(screen.getByTestId('two-factor-switch')); + + expect(await screen.findByTestId('disable-two-factor-dialog')).toBeInTheDocument(); + expect(screen.queryByTestId('disable-two-factor-current-password')).not.toBeInTheDocument(); + fireEvent.click(screen.getByTestId('confirm-disable-two-factor-button')); + + await waitFor(() => expect(disableTwoFactor).toHaveBeenCalledWith(null)); + }); }); diff --git a/tests/js/profile/actions.test.js b/tests/js/profile/actions.test.js index 72de63bc1..ca378b442 100644 --- a/tests/js/profile/actions.test.js +++ b/tests/js/profile/actions.test.js @@ -11,6 +11,7 @@ jest.mock("superagent", () => ({ import request from "superagent"; import { enableTwoFactor, + disableTwoFactor, regenerateRecoveryCodes, enrollRecoveryCodes, acknowledgeRecoveryCodes, @@ -31,6 +32,7 @@ describe("profile/actions (unmocked request layer)", () => { beforeEach(() => { request.post.mockReset(); window.ENABLE_TWO_FACTOR_ENDPOINT = "https://idp.test/api/v2/users/me/2fa/enable"; + window.DISABLE_TWO_FACTOR_ENDPOINT = "https://idp.test/admin/api/v1/users/me/2fa/disable"; window.REGENERATE_RECOVERY_CODES_ENDPOINT = "https://idp.test/api/v2/users/me/2fa/recovery-codes"; window.ENROLL_RECOVERY_CODES_ENDPOINT = "https://idp.test/admin/api/v1/users/me/recovery-codes/enroll"; window.ACKNOWLEDGE_RECOVERY_CODES_ENDPOINT = "https://idp.test/admin/api/v1/users/me/recovery-codes/acknowledge"; @@ -49,6 +51,29 @@ describe("profile/actions (unmocked request layer)", () => { expect(response.recovery_codes).toEqual(["ABCD-1234"]); }); + it("disableTwoFactor sends current_password only in the request body, asking for JSON", async () => { + const req = makeChainableRequest({ body: { two_factor_enabled: false } }); + request.post.mockReturnValue(req); + + const { response } = await disableTwoFactor("super-secret-password"); + + const calledUrl = request.post.mock.calls[0][0]; + expect(calledUrl).toBe(window.DISABLE_TWO_FACTOR_ENDPOINT); + expect(calledUrl).not.toContain("super-secret-password"); + expect(req.send).toHaveBeenCalledWith({ current_password: "super-secret-password" }); + expect(req.set).toHaveBeenCalledWith({ "X-CSRF-TOKEN": "test-csrf-token", Accept: "application/json" }); + expect(response.two_factor_enabled).toBe(false); + }); + + it("disableTwoFactor without a password sends an empty body (passwordless disable inside the window)", async () => { + const req = makeChainableRequest({ body: { two_factor_enabled: false } }); + request.post.mockReturnValue(req); + + await disableTwoFactor(); + + expect(req.send).toHaveBeenCalledWith({}); + }); + it("regenerateRecoveryCodes sends current_password only in the request body, never in the URL", async () => { const req = makeChainableRequest({ body: { recovery_codes: ["WXYZ-5678"] } }); request.post.mockReturnValue(req);