diff --git a/app/Services/Auth/IRecoveryCodeService.php b/app/Services/Auth/IRecoveryCodeService.php index ab6de16e..dd9977be 100644 --- a/app/Services/Auth/IRecoveryCodeService.php +++ b/app/Services/Auth/IRecoveryCodeService.php @@ -67,4 +67,29 @@ public function countUnusedRecoveryCodes(User $user): int; * @return RecoveryCodesStatus remaining/total/low-threshold standing for the user */ public function getStatus(User $user): RecoveryCodesStatus; + + /** + * Disables 2FA for the user: clears two_factor_enabled and two_factor_enforced_at, + * deletes every recovery code and revokes every trusted device, all in a single + * transaction. A device_revoked audit event is recorded best-effort afterwards. + * + * $actor identifies who performs the operation: + * - null: a system/console operation (e.g. idp:reset-2fa), no password is required; + * - the same user: self-service, $currentPassword must match; + * - another user: an administrator acting on the account, no password is required. + * Authorizing the administrator is the caller's responsibility. + * + * The caller is responsible for emitting the settings_changed audit event, as only + * it knows the context (reason, actor, operator). + * + * Group-enforced users are still required to complete 2FA at login afterwards, + * as enforcement is derived from group membership (User::shouldRequire2FA()). + * + * @param User $user + * @param string|null $currentPassword + * @param User|null $actor + * @return void + * @throws ValidationException if the actor is the user and $currentPassword does not match + */ + public function disableTwoFactor(User $user, ?string $currentPassword, ?User $actor): void; } diff --git a/app/Services/Auth/RecoveryCodeService.php b/app/Services/Auth/RecoveryCodeService.php index a626dd82..93a7a086 100644 --- a/app/Services/Auth/RecoveryCodeService.php +++ b/app/Services/Auth/RecoveryCodeService.php @@ -16,6 +16,7 @@ use App\libs\Auth\Models\TwoFactorAuditLog; use App\libs\Auth\Models\UserRecoveryCode; use Auth\Repositories\IUserRecoveryCodeRepository; +use Auth\Repositories\IUserTrustedDeviceRepository; use Auth\Repositories\IUserRepository; use Auth\User; use Illuminate\Support\Facades\Hash; @@ -36,6 +37,7 @@ final class RecoveryCodeService implements IRecoveryCodeService public function __construct( private readonly IUserRecoveryCodeRepository $repository, private readonly IUserRepository $user_repository, + private readonly IUserTrustedDeviceRepository $trusted_device_repository, private readonly ITransactionService $tx_service, private readonly ITwoFactorAuditService $audit_service, ) { @@ -121,6 +123,42 @@ public function enableTwoFactorAndGenerateCodes(User $user, string $method): arr return $codes; } + /** + * @inheritDoc + */ + public function disableTwoFactor(User $user, ?string $currentPassword, ?User $actor): void + { + // self-service requires the current password; console and admin + // operations do not (see IRecoveryCodeService::disableTwoFactor) + if (!is_null($actor) && $actor->getId() === $user->getId() + && !$user->checkPassword(trim((string)$currentPassword))) { + throw new ValidationException('current_password is not correct.'); + } + + // a single transaction() call: nesting another one (e.g. through the audit + // service) inside it would let an inner failure close the entity manager + // under this still-running outer transaction + $this->tx_service->transaction(function () use ($user) { + $user->disable2FA(); + $this->user_repository->add($user, false); + $this->repository->deleteAllForUser($user); + $this->trusted_device_repository->revokeAllForUser($user); + }); + + // Best-effort: 2FA is already disabled and the codes and devices are gone, + // an audit-logging failure must not make the caller believe it did not happen. + try { + $this->audit_service->log( + $user, + TwoFactorAuditLog::EventDeviceRevoked, + $user->getTwoFactorMethod(), + IPHelper::getUserIp() + ); + } 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. diff --git a/tests/DisableTwoFactorIntegrationTest.php b/tests/DisableTwoFactorIntegrationTest.php new file mode 100644 index 00000000..ccaff2d8 --- /dev/null +++ b/tests/DisableTwoFactorIntegrationTest.php @@ -0,0 +1,102 @@ +service = $this->app->make(IRecoveryCodeService::class); + + $user = new User(); + $user->setEmail('disable-2fa@nomail.com'); + $user->setFirstName('Disable'); + $user->setLastName('TwoFactor'); + $user->setIdentifier('disable-2fa@nomail.com'); + $user->setPassword('P@sswordS3cret'); + $user->verifyEmail(false); + EntityManager::persist($user); + EntityManager::flush(); + $this->user = $user; + + // enrolled, with a code batch and two trusted devices + $this->service->enableTwoFactorAndGenerateCodes($user, User::MFAMethod_OTP); + $devices = $this->app->make(IDeviceTrustService::class); + $devices->trustDevice($user, 'agent-a', '127.0.0.1'); + $devices->trustDevice($user, 'agent-b', '127.0.0.1'); + } + + private function userRow(): object + { + return DB::table('users')->where('id', $this->user->getId())->first(); + } + + public function testDisableClearsEverything(): void + { + $this->assertSame(1, (int)$this->userRow()->two_factor_enabled); + $this->assertSame(10, DB::table('user_recovery_codes')->where('user_id', $this->user->getId())->count()); + + $this->service->disableTwoFactor($this->user, null, null); + + $row = $this->userRow(); + $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', $this->user->getId())->count()); + + $devices = DB::table('user_trusted_devices')->where('user_id', $this->user->getId())->get(); + $this->assertCount(2, $devices); + foreach ($devices as $device) { + $this->assertSame(1, (int)$device->is_revoked); + } + + $this->assertSame(1, DB::table('two_factor_audit_log') + ->where('user_id', $this->user->getId()) + ->where('event_type', TwoFactorAuditLog::EventDeviceRevoked) + ->count()); + // settings_changed is the caller's responsibility + $this->assertSame(0, DB::table('two_factor_audit_log') + ->where('user_id', $this->user->getId()) + ->where('event_type', TwoFactorAuditLog::EventSettingsChanged) + ->count()); + } + + public function testSelfServiceWithWrongPasswordLeavesStateUntouched(): void + { + try { + $this->service->disableTwoFactor($this->user, 'wrong', $this->user); + $this->fail('ValidationException expected'); + } catch (\models\exceptions\ValidationException $ex) { + } + + $this->assertSame(1, (int)$this->userRow()->two_factor_enabled); + $this->assertSame(10, DB::table('user_recovery_codes')->where('user_id', $this->user->getId())->count()); + $this->assertSame(0, DB::table('user_trusted_devices') + ->where('user_id', $this->user->getId())->where('is_revoked', 1)->count()); + } +} diff --git a/tests/unit/RecoveryCodeServiceDisableTwoFactorTest.php b/tests/unit/RecoveryCodeServiceDisableTwoFactorTest.php new file mode 100644 index 00000000..f65463e9 --- /dev/null +++ b/tests/unit/RecoveryCodeServiceDisableTwoFactorTest.php @@ -0,0 +1,159 @@ +code_repo = Mockery::mock(IUserRecoveryCodeRepository::class); + $this->user_repo = Mockery::mock(IUserRepository::class); + $this->device_repo = Mockery::mock(IUserTrustedDeviceRepository::class); + $this->audit_service = Mockery::mock(ITwoFactorAuditService::class); + $this->tx_service = Mockery::mock(ITransactionService::class); + $this->tx_service->shouldReceive('transaction')->andReturnUsing(fn($cb) => $cb())->byDefault(); + + $this->service = new RecoveryCodeService( + $this->code_repo, + $this->user_repo, + $this->device_repo, + $this->tx_service, + $this->audit_service + ); + } + + protected function tearDown(): void + { + // Mockery expectations are verified on close(): count them as assertions + $this->addToAssertionCount(Mockery::getContainer()->mockery_getExpectationCount()); + Mockery::close(); + parent::tearDown(); + } + + private function buildUser(int $id): \Mockery\MockInterface + { + $user = Mockery::mock(User::class); + $user->shouldReceive('getId')->andReturn($id); + $user->shouldReceive('getTwoFactorMethod')->andReturn(User::MFAMethod_OTP); + return $user; + } + + private function expectFullDisable(\Mockery\MockInterface $user): void + { + $user->shouldReceive('disable2FA')->once(); + $this->user_repo->shouldReceive('add')->once()->with($user, false); + $this->code_repo->shouldReceive('deleteAllForUser')->once()->with($user)->andReturn(10); + $this->device_repo->shouldReceive('revokeAllForUser')->once()->with($user); + $this->audit_service->shouldReceive('log') + ->once() + ->with($user, TwoFactorAuditLog::EventDeviceRevoked, User::MFAMethod_OTP, Mockery::type('string')); + } + + public function testConsoleActorNeedsNoPassword(): void + { + $user = $this->buildUser(1); + $user->shouldNotReceive('checkPassword'); + $this->expectFullDisable($user); + + $this->service->disableTwoFactor($user, null, null); + } + + public function testAdminActorNeedsNoPassword(): void + { + $user = $this->buildUser(1); + $user->shouldNotReceive('checkPassword'); + $admin = $this->buildUser(2); + $this->expectFullDisable($user); + + $this->service->disableTwoFactor($user, null, $admin); + } + + public function testSelfServiceWithCorrectPassword(): void + { + $user = $this->buildUser(1); + $user->shouldReceive('checkPassword')->once()->with('secret')->andReturn(true); + $this->expectFullDisable($user); + + $this->service->disableTwoFactor($user, ' secret ', $user); + } + + public function testSelfServiceWithWrongPasswordChangesNothing(): void + { + $user = $this->buildUser(1); + $user->shouldReceive('checkPassword')->once()->andReturn(false); + $user->shouldNotReceive('disable2FA'); + $this->tx_service->shouldNotReceive('transaction'); + $this->code_repo->shouldNotReceive('deleteAllForUser'); + $this->device_repo->shouldNotReceive('revokeAllForUser'); + $this->audit_service->shouldNotReceive('log'); + + $this->expectException(ValidationException::class); + $this->service->disableTwoFactor($user, 'wrong', $user); + } + + public function testSelfServiceWithoutPasswordIsRejected(): void + { + $user = $this->buildUser(1); + $user->shouldReceive('checkPassword')->once()->with('')->andReturn(false); + $user->shouldNotReceive('disable2FA'); + + $this->expectException(ValidationException::class); + $this->service->disableTwoFactor($user, null, $user); + } + + public function testAuditFailureDoesNotFailTheOperation(): void + { + $user = $this->buildUser(1); + $user->shouldReceive('disable2FA')->once(); + $this->user_repo->shouldReceive('add')->once(); + $this->code_repo->shouldReceive('deleteAllForUser')->once()->andReturn(0); + $this->device_repo->shouldReceive('revokeAllForUser')->once(); + $this->audit_service->shouldReceive('log')->once()->andThrow(new \RuntimeException('audit down')); + + $this->service->disableTwoFactor($user, null, null); + } +}