From 7a0c742c012a61b92af5ad68075a5dc65bacad8e Mon Sep 17 00:00:00 2001 From: Cameron Steel Date: Thu, 15 Aug 2024 00:01:28 +1000 Subject: [PATCH] [3.x] FIX: userHandle compatibility between webauthn.js and Webpass (#94) --- .../Validator/Pipes/CheckCredentialIsForUser.php | 13 ++++++++++--- src/Attestation/Creator/Pipes/AddUserDescriptor.php | 3 ++- tests/Assertion/ValidationTest.php | 13 +++++++++++++ tests/Attestation/CreatorTest.php | 2 +- 4 files changed, 26 insertions(+), 5 deletions(-) diff --git a/src/Assertion/Validator/Pipes/CheckCredentialIsForUser.php b/src/Assertion/Validator/Pipes/CheckCredentialIsForUser.php index 1941db7..72b2a7d 100644 --- a/src/Assertion/Validator/Pipes/CheckCredentialIsForUser.php +++ b/src/Assertion/Validator/Pipes/CheckCredentialIsForUser.php @@ -4,6 +4,7 @@ use Closure; use Laragear\WebAuthn\Assertion\Validator\AssertionValidation; +use Laragear\WebAuthn\ByteBuffer; use Laragear\WebAuthn\Exceptions\AssertionException; use Ramsey\Uuid\Exception\InvalidUuidStringException; use Ramsey\Uuid\Uuid; @@ -67,12 +68,18 @@ protected function validateId(AssertionValidation $validation): void // of the authenticator, which is pushed from the application to be saved. If // the userHandle cannot be decoded and normalized, then surely is invalid. try { - $handle = Uuid::fromString($validation->json->get('response.userHandle'))->getHex()->toString(); + $handle = Uuid::fromString($validation->json->get('response.userHandle')); } catch (InvalidUuidStringException) { - throw AssertionException::make('The userHandle is not a valid hexadecimal UUID (32/36 characters).'); + try { + // This is required for compatibility with credentials created by versions + // of Webpass that used SimpleWebAuthn/browser < v10.0.0 + $handle = Uuid::fromString(ByteBuffer::decodeBase64Url($validation->json->get('response.userHandle'))); + } catch (InvalidUuidStringException) { + throw AssertionException::make('The userHandle is not a valid hexadecimal UUID (32/36 characters).'); + } } - if (! hash_equals(Uuid::fromString($validation->credential->user_id)->getHex()->toString(), $handle)) { + if (! hash_equals(Uuid::fromString($validation->credential->user_id)->getHex()->toString(), $handle->getHex()->toString())) { throw AssertionException::make('User ID is not owner of the stored credential.'); } } diff --git a/src/Attestation/Creator/Pipes/AddUserDescriptor.php b/src/Attestation/Creator/Pipes/AddUserDescriptor.php index 708a50f..d8fd93d 100644 --- a/src/Attestation/Creator/Pipes/AddUserDescriptor.php +++ b/src/Attestation/Creator/Pipes/AddUserDescriptor.php @@ -4,6 +4,7 @@ use Closure; use Laragear\WebAuthn\Attestation\Creator\AttestationCreation; +use Ramsey\Uuid\Uuid; /** * @internal @@ -19,7 +20,7 @@ public function handle(AttestationCreation $attestable, Closure $next): mixed $existingId = $attestable->user->webAuthnCredentials()->getQuery()->value('user_id'); $attestable->json->set('user', [ - 'id' => $existingId ?: $attestable->user->webAuthnId()->getHex()->toString(), + 'id' => ($existingId ? Uuid::fromString($existingId) : $attestable->user->webAuthnId())->getHex()->toString(), ...$attestable->user->webAuthnData(), ]); diff --git a/tests/Assertion/ValidationTest.php b/tests/Assertion/ValidationTest.php index d5ea39a..c620660 100644 --- a/tests/Assertion/ValidationTest.php +++ b/tests/Assertion/ValidationTest.php @@ -277,6 +277,19 @@ public function test_credential_check_is_malformed_user_handle(): void $this->validator->send($this->validation)->thenReturn(); } + public function test_credential_check_base64_user_handle(): void + { + $assertionResponse = FakeAuthenticator::assertionResponse(); + + $assertionResponse['response']['userHandle'] = base64_encode($assertionResponse['response']['userHandle']); + + $this->validation->json = new JsonTransport($assertionResponse); + + $this->validation->user = WebAuthnAuthenticatableUser::query()->first(); + + static::assertInstanceOf(AssertionValidation::class, $this->validator->send($this->validation)->thenReturn()); + } + public function test_credential_check_is_not_for_user_id(): void { DB::table('webauthn_credentials')->where('id', FakeAuthenticator::CREDENTIAL_ID)->update([ diff --git a/tests/Attestation/CreatorTest.php b/tests/Attestation/CreatorTest.php index 6e46054..56b245a 100644 --- a/tests/Attestation/CreatorTest.php +++ b/tests/Attestation/CreatorTest.php @@ -170,7 +170,7 @@ public function test_user_reuses_uuid_from_other_credential(): void ])->save(); $this->response() - ->assertJsonPath('user.id', $uuid->toString()); + ->assertJsonPath('user.id', $uuid->getHex()->toString()); } public function test_adds_existing_credentials_if_unique_by_default(): void