From cc13da9c17714732fb87669a39f4aade30d35a6d Mon Sep 17 00:00:00 2001 From: Michel Le Bihan Date: Sat, 25 Jul 2026 19:34:26 +0200 Subject: [PATCH] feat(webauthn): Skip 2FA for user verified logins A WebAuthn login with user verification is already a second factor, so do not ask for an additional 2FA challenge Signed-off-by: Michel Le Bihan --- core/Controller/WebAuthnController.php | 7 ++-- .../Authentication/Login/LoginData.php | 15 ++++++++ .../Authentication/Login/TwoFactorCommand.php | 5 ++- .../Authentication/WebAuthn/Manager.php | 5 +-- .../Login/TwoFactorCommandTest.php | 35 +++++++++++++++++++ 5 files changed, 61 insertions(+), 6 deletions(-) diff --git a/core/Controller/WebAuthnController.php b/core/Controller/WebAuthnController.php index 2518ab274aac5..b3e923daf4d78 100644 --- a/core/Controller/WebAuthnController.php +++ b/core/Controller/WebAuthnController.php @@ -77,7 +77,7 @@ public function finishAuthentication(string $data): JSONResponse { // Obtain the publicKeyCredentialOptions from when we started the registration $publicKeyCredentialRequestOptions = PublicKeyCredentialRequestOptions::createFromString($this->session->get(self::WEBAUTHN_LOGIN)); $uid = $this->session->get(self::WEBAUTHN_LOGIN_UID); - $this->webAuthnManger->finishAuthentication($publicKeyCredentialRequestOptions, $data, $uid); + $authenticatorData = $this->webAuthnManger->finishAuthentication($publicKeyCredentialRequestOptions, $data, $uid); //TODO: add other parameters $loginData = new LoginData( @@ -85,10 +85,11 @@ public function finishAuthentication(string $data): JSONResponse { $uid, '' ); - $this->webAuthnChain->process($loginData); + $loginData->setWebAuthnUserVerified($authenticatorData->isUserVerified()); + $result = $this->webAuthnChain->process($loginData); return new JSONResponse([ - 'defaultRedirectUrl' => $this->urlGenerator->linkToDefaultPageUrl(), + 'defaultRedirectUrl' => $result->getRedirectUrl() ?? $this->urlGenerator->linkToDefaultPageUrl(), ]); } } diff --git a/lib/private/Authentication/Login/LoginData.php b/lib/private/Authentication/Login/LoginData.php index 8791b00413bfe..ab4d464625d1f 100644 --- a/lib/private/Authentication/Login/LoginData.php +++ b/lib/private/Authentication/Login/LoginData.php @@ -16,6 +16,13 @@ class LoginData { /** @var IUser|false|null */ private $user = null; + /** + * True when this login was performed via WebAuthn *and* the authenticator + * verified the user (PIN, biometrics, …), which makes the login itself a + * multi-factor authentication. + */ + private bool $webAuthnUserVerified = false; + public function __construct( private IRequest $request, private string $username, @@ -76,4 +83,12 @@ public function setRememberLogin(bool $rememberLogin): void { public function isRememberLogin(): bool { return $this->rememberLogin; } + + public function setWebAuthnUserVerified(bool $webAuthnUserVerified): void { + $this->webAuthnUserVerified = $webAuthnUserVerified; + } + + public function isWebAuthnUserVerified(): bool { + return $this->webAuthnUserVerified; + } } diff --git a/lib/private/Authentication/Login/TwoFactorCommand.php b/lib/private/Authentication/Login/TwoFactorCommand.php index c8805386b745d..4f269fe04dd3f 100644 --- a/lib/private/Authentication/Login/TwoFactorCommand.php +++ b/lib/private/Authentication/Login/TwoFactorCommand.php @@ -25,7 +25,10 @@ public function __construct( #[\Override] public function process(LoginData $loginData): LoginResult { - if (!$this->twoFactorManager->isTwoFactorAuthenticated($loginData->getUser())) { + // A WebAuthn login with user verification combines possession of the + // authenticator with a PIN or a biometric, so it is a second factor already + if ($loginData->isWebAuthnUserVerified() + || !$this->twoFactorManager->isTwoFactorAuthenticated($loginData->getUser())) { return $this->processNextOrFinishSuccessfully($loginData); } diff --git a/lib/private/Authentication/WebAuthn/Manager.php b/lib/private/Authentication/WebAuthn/Manager.php index 932631f2aeaae..b32184add015c 100644 --- a/lib/private/Authentication/WebAuthn/Manager.php +++ b/lib/private/Authentication/WebAuthn/Manager.php @@ -27,6 +27,7 @@ use Webauthn\AuthenticatorAssertionResponseValidator; use Webauthn\AuthenticatorAttestationResponse; use Webauthn\AuthenticatorAttestationResponseValidator; +use Webauthn\AuthenticatorData; use Webauthn\AuthenticatorSelectionCriteria; use Webauthn\PublicKeyCredentialCreationOptions; use Webauthn\PublicKeyCredentialDescriptor; @@ -168,7 +169,7 @@ public function startAuthentication(string $uid, string $serverHost): PublicKeyC ); } - public function finishAuthentication(PublicKeyCredentialRequestOptions $publicKeyCredentialRequestOptions, string $data, string $uid) { + public function finishAuthentication(PublicKeyCredentialRequestOptions $publicKeyCredentialRequestOptions, string $data, string $uid): AuthenticatorData { $attestationStatementSupportManager = new AttestationStatementSupportManager(); $attestationStatementSupportManager->add(new NoneAttestationStatementSupport()); @@ -216,7 +217,7 @@ public function finishAuthentication(PublicKeyCredentialRequestOptions $publicKe throw $e; } - return true; + return $response->authenticatorData; } public function deleteRegistration(IUser $user, int $id): void { diff --git a/tests/lib/Authentication/Login/TwoFactorCommandTest.php b/tests/lib/Authentication/Login/TwoFactorCommandTest.php index 68b161790ede5..feb139c880de4 100644 --- a/tests/lib/Authentication/Login/TwoFactorCommandTest.php +++ b/tests/lib/Authentication/Login/TwoFactorCommandTest.php @@ -56,6 +56,41 @@ public function testNotTwoFactorAuthenticated(): void { $this->assertTrue($result->isSuccess()); } + public function testSkippedForVerifiedWebAuthnLogin(): void { + $data = $this->getLoggedInLoginData(); + $data->setWebAuthnUserVerified(true); + $this->twoFactorManager->expects($this->never()) + ->method('prepareTwoFactorLogin'); + + $result = $this->cmd->process($data); + + $this->assertTrue($result->isSuccess()); + $this->assertNull($result->getRedirectUrl()); + } + + public function testNotSkippedForWebAuthnLoginWithoutUserVerification(): void { + $data = $this->getLoggedInLoginData(); + $data->setWebAuthnUserVerified(false); + $this->twoFactorManager->expects($this->once()) + ->method('isTwoFactorAuthenticated') + ->willReturn(true); + $this->twoFactorManager->expects($this->once()) + ->method('prepareTwoFactorLogin'); + $this->twoFactorManager->expects($this->once()) + ->method('getProviderSet') + ->willReturn(new ProviderSet([], false)); + $this->twoFactorManager->expects($this->once()) + ->method('getLoginSetupProviders') + ->willReturn([]); + $this->urlGenerator->expects($this->once()) + ->method('linkToRoute') + ->willReturn('two/factor/url'); + + $result = $this->cmd->process($data); + + $this->assertEquals('two/factor/url', $result->getRedirectUrl()); + } + public function testProcessOneActiveProvider(): void { $data = $this->getLoggedInLoginData(); $this->twoFactorManager->expects($this->once())