Skip to content

Commit cc13da9

Browse files
committed
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 <michel@lebihan.pl>
1 parent a5ee804 commit cc13da9

5 files changed

Lines changed: 61 additions & 6 deletions

File tree

core/Controller/WebAuthnController.php

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,18 +77,19 @@ public function finishAuthentication(string $data): JSONResponse {
7777
// Obtain the publicKeyCredentialOptions from when we started the registration
7878
$publicKeyCredentialRequestOptions = PublicKeyCredentialRequestOptions::createFromString($this->session->get(self::WEBAUTHN_LOGIN));
7979
$uid = $this->session->get(self::WEBAUTHN_LOGIN_UID);
80-
$this->webAuthnManger->finishAuthentication($publicKeyCredentialRequestOptions, $data, $uid);
80+
$authenticatorData = $this->webAuthnManger->finishAuthentication($publicKeyCredentialRequestOptions, $data, $uid);
8181

8282
//TODO: add other parameters
8383
$loginData = new LoginData(
8484
$this->request,
8585
$uid,
8686
''
8787
);
88-
$this->webAuthnChain->process($loginData);
88+
$loginData->setWebAuthnUserVerified($authenticatorData->isUserVerified());
89+
$result = $this->webAuthnChain->process($loginData);
8990

9091
return new JSONResponse([
91-
'defaultRedirectUrl' => $this->urlGenerator->linkToDefaultPageUrl(),
92+
'defaultRedirectUrl' => $result->getRedirectUrl() ?? $this->urlGenerator->linkToDefaultPageUrl(),
9293
]);
9394
}
9495
}

lib/private/Authentication/Login/LoginData.php

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,13 @@ class LoginData {
1616
/** @var IUser|false|null */
1717
private $user = null;
1818

19+
/**
20+
* True when this login was performed via WebAuthn *and* the authenticator
21+
* verified the user (PIN, biometrics, …), which makes the login itself a
22+
* multi-factor authentication.
23+
*/
24+
private bool $webAuthnUserVerified = false;
25+
1926
public function __construct(
2027
private IRequest $request,
2128
private string $username,
@@ -76,4 +83,12 @@ public function setRememberLogin(bool $rememberLogin): void {
7683
public function isRememberLogin(): bool {
7784
return $this->rememberLogin;
7885
}
86+
87+
public function setWebAuthnUserVerified(bool $webAuthnUserVerified): void {
88+
$this->webAuthnUserVerified = $webAuthnUserVerified;
89+
}
90+
91+
public function isWebAuthnUserVerified(): bool {
92+
return $this->webAuthnUserVerified;
93+
}
7994
}

lib/private/Authentication/Login/TwoFactorCommand.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,10 @@ public function __construct(
2525

2626
#[\Override]
2727
public function process(LoginData $loginData): LoginResult {
28-
if (!$this->twoFactorManager->isTwoFactorAuthenticated($loginData->getUser())) {
28+
// A WebAuthn login with user verification combines possession of the
29+
// authenticator with a PIN or a biometric, so it is a second factor already
30+
if ($loginData->isWebAuthnUserVerified()
31+
|| !$this->twoFactorManager->isTwoFactorAuthenticated($loginData->getUser())) {
2932
return $this->processNextOrFinishSuccessfully($loginData);
3033
}
3134

lib/private/Authentication/WebAuthn/Manager.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
use Webauthn\AuthenticatorAssertionResponseValidator;
2828
use Webauthn\AuthenticatorAttestationResponse;
2929
use Webauthn\AuthenticatorAttestationResponseValidator;
30+
use Webauthn\AuthenticatorData;
3031
use Webauthn\AuthenticatorSelectionCriteria;
3132
use Webauthn\PublicKeyCredentialCreationOptions;
3233
use Webauthn\PublicKeyCredentialDescriptor;
@@ -168,7 +169,7 @@ public function startAuthentication(string $uid, string $serverHost): PublicKeyC
168169
);
169170
}
170171

171-
public function finishAuthentication(PublicKeyCredentialRequestOptions $publicKeyCredentialRequestOptions, string $data, string $uid) {
172+
public function finishAuthentication(PublicKeyCredentialRequestOptions $publicKeyCredentialRequestOptions, string $data, string $uid): AuthenticatorData {
172173
$attestationStatementSupportManager = new AttestationStatementSupportManager();
173174
$attestationStatementSupportManager->add(new NoneAttestationStatementSupport());
174175

@@ -216,7 +217,7 @@ public function finishAuthentication(PublicKeyCredentialRequestOptions $publicKe
216217
throw $e;
217218
}
218219

219-
return true;
220+
return $response->authenticatorData;
220221
}
221222

222223
public function deleteRegistration(IUser $user, int $id): void {

tests/lib/Authentication/Login/TwoFactorCommandTest.php

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,41 @@ public function testNotTwoFactorAuthenticated(): void {
5656
$this->assertTrue($result->isSuccess());
5757
}
5858

59+
public function testSkippedForVerifiedWebAuthnLogin(): void {
60+
$data = $this->getLoggedInLoginData();
61+
$data->setWebAuthnUserVerified(true);
62+
$this->twoFactorManager->expects($this->never())
63+
->method('prepareTwoFactorLogin');
64+
65+
$result = $this->cmd->process($data);
66+
67+
$this->assertTrue($result->isSuccess());
68+
$this->assertNull($result->getRedirectUrl());
69+
}
70+
71+
public function testNotSkippedForWebAuthnLoginWithoutUserVerification(): void {
72+
$data = $this->getLoggedInLoginData();
73+
$data->setWebAuthnUserVerified(false);
74+
$this->twoFactorManager->expects($this->once())
75+
->method('isTwoFactorAuthenticated')
76+
->willReturn(true);
77+
$this->twoFactorManager->expects($this->once())
78+
->method('prepareTwoFactorLogin');
79+
$this->twoFactorManager->expects($this->once())
80+
->method('getProviderSet')
81+
->willReturn(new ProviderSet([], false));
82+
$this->twoFactorManager->expects($this->once())
83+
->method('getLoginSetupProviders')
84+
->willReturn([]);
85+
$this->urlGenerator->expects($this->once())
86+
->method('linkToRoute')
87+
->willReturn('two/factor/url');
88+
89+
$result = $this->cmd->process($data);
90+
91+
$this->assertEquals('two/factor/url', $result->getRedirectUrl());
92+
}
93+
5994
public function testProcessOneActiveProvider(): void {
6095
$data = $this->getLoggedInLoginData();
6196
$this->twoFactorManager->expects($this->once())

0 commit comments

Comments
 (0)