Skip to content
Open
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
7 changes: 4 additions & 3 deletions core/Controller/WebAuthnController.php
Original file line number Diff line number Diff line change
Expand Up @@ -77,18 +77,19 @@ 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(
$this->request,
$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(),
]);
}
}
15 changes: 15 additions & 0 deletions lib/private/Authentication/Login/LoginData.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
}
}
5 changes: 4 additions & 1 deletion lib/private/Authentication/Login/TwoFactorCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
5 changes: 3 additions & 2 deletions lib/private/Authentication/WebAuthn/Manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());

Expand Down Expand Up @@ -216,7 +217,7 @@ public function finishAuthentication(PublicKeyCredentialRequestOptions $publicKe
throw $e;
}

return true;
return $response->authenticatorData;
}

public function deleteRegistration(IUser $user, int $id): void {
Expand Down
35 changes: 35 additions & 0 deletions tests/lib/Authentication/Login/TwoFactorCommandTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down