diff --git a/lib/Db/PublicKeyCredentialEntity.php b/lib/Db/PublicKeyCredentialEntity.php index 6fe857bb..3afdfb5f 100644 --- a/lib/Db/PublicKeyCredentialEntity.php +++ b/lib/Db/PublicKeyCredentialEntity.php @@ -121,6 +121,7 @@ public static function fromPublicKeyCrendentialSource(string $name, $publicKeyCredentialEntity->setUserHandle($publicKeyCredentialSource->getUserHandle()); $publicKeyCredentialEntity->setCounter($publicKeyCredentialSource->getCounter()); $publicKeyCredentialEntity->setCreatedAt($ctime); + $publicKeyCredentialEntity->setActive(true); return $publicKeyCredentialEntity; } diff --git a/lib/Listener/StateChangeRegistryUpdater.php b/lib/Listener/StateChangeRegistryUpdater.php index 803371fa..8cc8d240 100644 --- a/lib/Listener/StateChangeRegistryUpdater.php +++ b/lib/Listener/StateChangeRegistryUpdater.php @@ -10,6 +10,7 @@ namespace OCA\TwoFactorWebauthn\Listener; use OCA\TwoFactorWebauthn\Event\StateChanged; +use OCA\TwoFactorWebauthn\Model\Device; use OCA\TwoFactorWebauthn\Provider\WebAuthnProvider; use OCA\TwoFactorWebauthn\Service\WebAuthnManager; use OCP\Authentication\TwoFactorAuth\IRegistry; @@ -38,13 +39,15 @@ public function __construct(IRegistry $providerRegistry, WebAuthnManager $manage public function handle(Event $event): void { if ($event instanceof StateChanged) { - $devices = array_filter($this->manager->getDevices($event->getUser()), function ($device) { - return $device['active'] === true; - }); - if ($event->isEnabled() && count($devices) > 0) { + $devices = array_filter( + $this->manager->getDevices($event->getUser()), + static fn (Device $device) => $device->isActive(), + ); + $hasDevices = !empty($devices); + if ($hasDevices && $event->isEnabled()) { // The first device was enabled -> enable provider for this user $this->providerRegistry->enableProviderFor($this->provider, $event->getUser()); - } elseif (!$event->isEnabled() && empty($devices)) { + } elseif (!$hasDevices && !$event->isEnabled()) { // The last device was removed -> disable provider for this user $this->providerRegistry->disableProviderFor($this->provider, $event->getUser()); } diff --git a/lib/Model/Device.php b/lib/Model/Device.php new file mode 100644 index 00000000..30b1e0fa --- /dev/null +++ b/lib/Model/Device.php @@ -0,0 +1,64 @@ +getId(), + $entity->getPublicKeyCredentialId(), + $entity->getName(), + $entity->getCreatedAt(), + $entity->isActive() === true, + ); + } + + public function getEntityId(): int { + return $this->entityId; + } + + public function getId(): string { + return $this->id; + } + + public function getName(): string { + return $this->name; + } + + public function getCreatedAt(): ?int { + return $this->createdAt; + } + + public function isActive(): bool { + return $this->active; + } + + #[\ReturnTypeWillChange] + public function jsonSerialize() { + return [ + 'entityId' => $this->getEntityId(), + 'id' => $this->getId(), + 'name' => $this->getName(), + 'createdAt' => $this->getCreatedAt(), + 'active' => $this->isActive(), + ]; + } +} diff --git a/lib/Provider/WebAuthnProvider.php b/lib/Provider/WebAuthnProvider.php index a7fa343a..cc9ea068 100644 --- a/lib/Provider/WebAuthnProvider.php +++ b/lib/Provider/WebAuthnProvider.php @@ -10,6 +10,7 @@ namespace OCA\TwoFactorWebauthn\Provider; use OCA\TwoFactorWebauthn\AppInfo\Application; +use OCA\TwoFactorWebauthn\Model\Device; use OCA\TwoFactorWebauthn\Service\WebAuthnManager; use OCA\TwoFactorWebauthn\Settings\Personal; use OCP\AppFramework\Services\IInitialState; @@ -104,9 +105,10 @@ public function verifyChallenge(IUser $user, string $challenge): bool { * Decides whether 2FA is enabled for the given user */ public function isTwoFactorAuthEnabledForUser(IUser $user): bool { - $devices = array_filter($this->manager->getDevices($user), function ($device) { - return $device['active'] === true; - }); + $devices = array_filter( + $this->manager->getDevices($user), + static fn (Device $device) => $device->isActive(), + ); return count($devices) > 0; } diff --git a/lib/Service/WebAuthnManager.php b/lib/Service/WebAuthnManager.php index cf57beb6..565327ea 100644 --- a/lib/Service/WebAuthnManager.php +++ b/lib/Service/WebAuthnManager.php @@ -18,7 +18,9 @@ use OCA\TwoFactorWebauthn\Db\PublicKeyCredentialEntity; use OCA\TwoFactorWebauthn\Db\PublicKeyCredentialEntityMapper; use OCA\TwoFactorWebauthn\Event\StateChanged; +use OCA\TwoFactorWebauthn\Model\Device; use OCA\TwoFactorWebauthn\Repository\WebauthnPublicKeyCredentialSourceRepository; +use OCP\AppFramework\Utility\ITimeFactory; use OCP\EventDispatcher\IEventDispatcher; use OCP\IRequest; use OCP\ISession; @@ -80,6 +82,7 @@ public function __construct( LoggerInterface $logger, private readonly IRequest $request, private readonly ISecureRandom $random, + private readonly ITimeFactory $time, ) { $this->session = $session; $this->repository = $repository; @@ -171,7 +174,7 @@ public function buildPublicKeyCredentialLoader(AttestationStatementSupportManage return $publicKeyCredentialLoader; } - public function finishRegister(IUser $user, string $name, $data): array { + public function finishRegister(IUser $user, string $name, $data): Device { if (!$this->session->exists(self::TWOFACTORAUTH_WEBAUTHN_REGISTRATION)) { throw new Exception('Twofactor Webauthn registration process was not properly initialized'); } @@ -212,30 +215,23 @@ public function finishRegister(IUser $user, string $name, $data): array { $this->stripPort($this->request->getServerHost()), ); - $this->repository->saveCredentialSource($publicKeyCredentialSource, $name); - $entity = $this->mapper->findPublicKeyCredential(base64_encode($publicKeyCredentialSource->getPublicKeyCredentialId())); + $entity = PublicKeyCredentialEntity::fromPublicKeyCrendentialSource( + $name, + $publicKeyCredentialSource, + $this->time->getTime(), + ); + $entity = $this->mapper->insert($entity); $this->eventDispatcher->dispatch(StateChanged::class, new StateChanged($user, true)); - - return [ - 'entityId' => $entity->getId(), - 'id' => base64_encode($publicKeyCredentialSource->getPublicKeyCredentialId()), - 'name' => $name, - 'createdAt' => $entity->getCreatedAt(), - 'active' => true, - ]; + return Device::fromPublicKeyCredentialEntity($entity); } + /** + * @param IUser $user + * @return Device[] + */ public function getDevices(IUser $user): array { $credentials = $this->mapper->findPublicKeyCredentials($user->getUID()); - return array_map(function (PublicKeyCredentialEntity $credential) { - return [ - 'entityId' => $credential->getId(), - 'id' => $credential->getPublicKeyCredentialId(), - 'name' => $credential->getName(), - 'createdAt' => $credential->getCreatedAt(), - 'active' => ($credential->isActive() === true) - ]; - }, $credentials); + return array_map(Device::fromPublicKeyCredentialEntity(...), $credentials); } private function stripPort(string $serverHost): string { diff --git a/tests/Unit/Controller/SettingsControllerTest.php b/tests/Unit/Controller/SettingsControllerTest.php index e973cb7f..385040d1 100644 --- a/tests/Unit/Controller/SettingsControllerTest.php +++ b/tests/Unit/Controller/SettingsControllerTest.php @@ -10,6 +10,7 @@ use ChristophWurst\Nextcloud\Testing\TestCase; use OCA\TwoFactorWebauthn\Controller\SettingsController; +use OCA\TwoFactorWebauthn\Model\Device; use OCA\TwoFactorWebauthn\Service\WebAuthnManager; use OCP\AppFramework\Http\JSONResponse; use OCP\IRequest; @@ -71,16 +72,17 @@ public function testFinishRegister(): void { ->willReturn($user); $data = 'some data'; + $device = new Device(1, 'key-1', 'my key', null, true); $this->webauthnManager->expects(self::once()) ->method('finishRegister') ->with( self::equalTo($user), self::equalTo('my key'), self::equalTo($data)) - ->willReturn([]); + ->willReturn($device); $resp = $this->controller->finishRegister('my key', $data); - self::assertEquals(new JSONResponse([]), $resp); + self::assertEquals(new JSONResponse($device), $resp); } } diff --git a/tests/Unit/Listener/StateChangeRegistryUpdaterTest.php b/tests/Unit/Listener/StateChangeRegistryUpdaterTest.php index d05e2890..5db92b87 100644 --- a/tests/Unit/Listener/StateChangeRegistryUpdaterTest.php +++ b/tests/Unit/Listener/StateChangeRegistryUpdaterTest.php @@ -11,6 +11,7 @@ use OCA\TwoFactorWebauthn\Event\StateChanged; use OCA\TwoFactorWebauthn\Listener\StateChangeRegistryUpdater; +use OCA\TwoFactorWebauthn\Model\Device; use OCA\TwoFactorWebauthn\Provider\WebAuthnProvider; use OCA\TwoFactorWebauthn\Service\WebAuthnManager; use OCP\Authentication\TwoFactorAuth\IRegistry; @@ -59,11 +60,7 @@ public function testHandleEnableFirstDevice(): void { $this->manager->expects(self::once()) ->method('getDevices') ->willReturn([ - [ - 'id' => 1, - 'name' => 'utf1', - 'active' => true, - ], + new Device(1, 'credential-id-1', 'utf2', null, true), ]); $this->providerRegistry->expects(self::once()) ->method('enableProviderFor') @@ -97,11 +94,7 @@ public function testHandleDisableWithRemainingDevices(): void { $this->manager->expects(self::once()) ->method('getDevices') ->willReturn([ - [ - 'id' => 2, - 'name' => 'utf2', - 'active' => true, - ], + new Device(2, 'credential-id-2', 'utf2', null, true), ]); $this->providerRegistry->expects(self::never()) ->method('disableProviderFor') diff --git a/tests/Unit/Provider/WebAuthnProviderTest.php b/tests/Unit/Provider/WebAuthnProviderTest.php index 3b13b950..b0e24f3f 100644 --- a/tests/Unit/Provider/WebAuthnProviderTest.php +++ b/tests/Unit/Provider/WebAuthnProviderTest.php @@ -9,6 +9,7 @@ namespace OCA\TwoFactorWebauthn\Tests\Unit\Provider; +use OCA\TwoFactorWebauthn\Model\Device; use OCA\TwoFactorWebauthn\Provider\WebAuthnLoginProvider; use OCA\TwoFactorWebauthn\Provider\WebAuthnProvider; use OCA\TwoFactorWebauthn\Service\WebAuthnManager; @@ -129,11 +130,7 @@ public function testIsTwoFactorAuthEnabledForUser(): void { $this->manager->expects(self::once()) ->method('getDevices') ->willReturn([ - [ - 'id' => 'k1', - 'name' => 'n1', - 'active' => true, - ], + new Device(1, 'k1', 'n1', null, true), ]); self::assertTrue($this->provider->isTwoFactorAuthEnabledForUser($user)); @@ -144,11 +141,7 @@ public function testIsTwoFactorAuthDisabledForUserBecauseDisabledDevice(): void $this->manager->expects(self::once()) ->method('getDevices') ->willReturn([ - [ - 'id' => 'k1', - 'name' => 'n1', - 'active' => false, - ], + new Device(1, 'k1', 'n1', null, false), ]); self::assertFalse($this->provider->isTwoFactorAuthEnabledForUser($user)); diff --git a/tests/Unit/Service/WebAuthnManagerTest.php b/tests/Unit/Service/WebAuthnManagerTest.php index 8bee15fd..c0aac98f 100644 --- a/tests/Unit/Service/WebAuthnManagerTest.php +++ b/tests/Unit/Service/WebAuthnManagerTest.php @@ -14,6 +14,7 @@ use OCA\TwoFactorWebauthn\Event\StateChanged; use OCA\TwoFactorWebauthn\Repository\WebauthnPublicKeyCredentialSourceRepository; use OCA\TwoFactorWebauthn\Service\WebAuthnManager; +use OCP\AppFramework\Utility\ITimeFactory; use OCP\EventDispatcher\IEventDispatcher; use OCP\IRequest; use OCP\ISession; @@ -45,6 +46,7 @@ class WebAuthnManagerTest extends TestCase { private IRequest&MockObject $request; private ISecureRandom&MockObject $random; + private ITimeFactory&MockObject $time; protected function setUp(): void { parent::setUp(); @@ -56,6 +58,7 @@ protected function setUp(): void { $this->logger = $this->createMock(LoggerInterface::class); $this->request = $this->createMock(IRequest::class); $this->random = $this->createMock(ISecureRandom::class); + $this->time = $this->createMock(ITimeFactory::class); $this->manager = new WebAuthnManager( $this->session, @@ -65,17 +68,20 @@ protected function setUp(): void { $this->logger, $this->request, $this->random, + $this->time, ); } - /** - * @param IUser $user - * @param int $nr - */ - private function mockRegistrations(IUser $user, $nr): void { + private function mockRegistrations(IUser $user, int $nr): void { $regs = []; for ($i = 0; $i < $nr; $i++) { $reg = new PublicKeyCredentialEntity(); + $reg->setId($i); + $reg->setPublicKeyCredentialId("credential-id-$i"); + $reg->setName("key-$i"); + $reg->setUserHandle($user->getUID()); + $reg->setActive(true); + $reg->setCreatedAt(null); $regs[] = $reg; } $this->mapper->expects(self::once())