Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/Db/PublicKeyCredentialEntity.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
13 changes: 8 additions & 5 deletions lib/Listener/StateChangeRegistryUpdater.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
}
Expand Down
64 changes: 64 additions & 0 deletions lib/Model/Device.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
<?php

declare(strict_types=1);

/**
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OCA\TwoFactorWebauthn\Model;

use OCA\TwoFactorWebauthn\Db\PublicKeyCredentialEntity;

final class Device implements \JsonSerializable {
public function __construct(
private readonly int $entityId,
private readonly string $id,
private readonly string $name,
private readonly ?int $createdAt,
private readonly bool $active,
) {
}

public static function fromPublicKeyCredentialEntity(PublicKeyCredentialEntity $entity): self {
return new self(
$entity->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(),
];
}
}
8 changes: 5 additions & 3 deletions lib/Provider/WebAuthnProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

Expand Down
36 changes: 16 additions & 20 deletions lib/Service/WebAuthnManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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');
}
Expand Down Expand Up @@ -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 {
Expand Down
6 changes: 4 additions & 2 deletions tests/Unit/Controller/SettingsControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}
13 changes: 3 additions & 10 deletions tests/Unit/Listener/StateChangeRegistryUpdaterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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')
Expand Down
13 changes: 3 additions & 10 deletions tests/Unit/Provider/WebAuthnProviderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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));
Expand All @@ -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));
Expand Down
16 changes: 11 additions & 5 deletions tests/Unit/Service/WebAuthnManagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand All @@ -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,
Expand All @@ -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())
Expand Down
Loading