From f1e7522ba15e058f76b9adeea43af8b3dda70555 Mon Sep 17 00:00:00 2001 From: Florent Morselli Date: Tue, 9 Apr 2024 10:02:40 +0200 Subject: [PATCH] Refactor code for deprecation of "id" property The "id" property in the PublicKeyCredential is deprecated. This commit refactors the relevant code, specifically in the "PublicKeyCredential" class and several test classes, to replace the use of "Base64UrlSafe::decode($publicKeyCredential->id)" with "$publicKeyCredential->rawId". The changes are made such that the functionality is maintained but future compatibility is ensured. --- src/webauthn/src/Credential.php | 6 +++++- .../PublicKeyCredentialDenormalizer.php | 2 +- src/webauthn/src/PublicKeyCredential.php | 17 +++++++++++++---- src/webauthn/src/PublicKeyCredentialLoader.php | 2 +- .../AppleAttestationStatementTest.php | 6 +----- ...AttestationStatementWithTokenBindingTest.php | 3 +-- tests/library/Functional/AttestationTest.php | 3 +-- .../PackedAttestationStatementTest.php | 7 +++---- tests/library/Functional/W10Test.php | 3 +-- 9 files changed, 27 insertions(+), 22 deletions(-) diff --git a/src/webauthn/src/Credential.php b/src/webauthn/src/Credential.php index cbad84cc..ff99cc79 100644 --- a/src/webauthn/src/Credential.php +++ b/src/webauthn/src/Credential.php @@ -9,10 +9,14 @@ */ abstract class Credential { + public readonly string $rawId; + public function __construct( public readonly string $id, - public readonly string $type + public readonly string $type, + null|string $rawId = null, ) { + $this->rawId = $rawId ?? base64_encode($this->id); } /** diff --git a/src/webauthn/src/Denormalizer/PublicKeyCredentialDenormalizer.php b/src/webauthn/src/Denormalizer/PublicKeyCredentialDenormalizer.php index 74b230cb..33899b70 100644 --- a/src/webauthn/src/Denormalizer/PublicKeyCredentialDenormalizer.php +++ b/src/webauthn/src/Denormalizer/PublicKeyCredentialDenormalizer.php @@ -29,7 +29,7 @@ public function denormalize(mixed $data, string $type, string $format = null, ar $data['rawId'] = $rawId; return PublicKeyCredential::create( - $data['id'], + null, $data['type'], $data['rawId'], $this->denormalizer->denormalize($data['response'], AuthenticatorResponse::class, $format, $context), diff --git a/src/webauthn/src/PublicKeyCredential.php b/src/webauthn/src/PublicKeyCredential.php index b4cbd48f..ce438f76 100644 --- a/src/webauthn/src/PublicKeyCredential.php +++ b/src/webauthn/src/PublicKeyCredential.php @@ -14,12 +14,21 @@ class PublicKeyCredential extends Credential implements Stringable { public function __construct( - string $id, + null|string $id, string $type, - public readonly string $rawId, + string $rawId, public readonly AuthenticatorResponse $response ) { - parent::__construct($id, $type); + if ($id !== null) { + trigger_deprecation( + 'web-auth/webauthn-lib', + '4.9.0', + 'The property "$id" is deprecated and will be removed in 5.0.0. Please set null use "rawId" instead.' + ); + } else { + $id = base64_encode($rawId); + } + parent::__construct($id, $type, $rawId); } /** @@ -31,7 +40,7 @@ public function __toString(): string return json_encode($this->getPublicKeyCredentialDescriptor(), JSON_THROW_ON_ERROR); } - public static function create(string $id, string $type, string $rawId, AuthenticatorResponse $response): self + public static function create(null|string $id, string $type, string $rawId, AuthenticatorResponse $response): self { return new self($id, $type, $rawId, $response); } diff --git a/src/webauthn/src/PublicKeyCredentialLoader.php b/src/webauthn/src/PublicKeyCredentialLoader.php index 19861c1a..88b2a9c1 100644 --- a/src/webauthn/src/PublicKeyCredentialLoader.php +++ b/src/webauthn/src/PublicKeyCredentialLoader.php @@ -86,7 +86,7 @@ public function loadArray(array $json): PublicKeyCredential hash_equals($id, $rawId) || throw InvalidDataException::create($json, 'Invalid ID'); $publicKeyCredential = PublicKeyCredential::create( - $json['id'], + null, $json['type'], $rawId, $this->createResponse($json['response']) diff --git a/tests/library/Functional/AppleAttestationStatementTest.php b/tests/library/Functional/AppleAttestationStatementTest.php index 97bbba97..fb4da4b0 100644 --- a/tests/library/Functional/AppleAttestationStatementTest.php +++ b/tests/library/Functional/AppleAttestationStatementTest.php @@ -5,7 +5,6 @@ namespace Webauthn\Tests\Functional; use DateTimeImmutable; -use ParagonIE\ConstantTime\Base64UrlSafe; use PHPUnit\Framework\Attributes\Test; use Webauthn\AttestationStatement\AttestationStatement; use Webauthn\AttestedCredentialData; @@ -55,10 +54,7 @@ public function anAppleAttestationCanBeVerified(): void $this->getAuthenticatorAttestationResponseValidator() ->check($publicKeyCredential->response, $publicKeyCredentialCreationOptions, 'dev.dontneeda.pw'); $publicKeyCredentialDescriptor = $publicKeyCredential->getPublicKeyCredentialDescriptor(); - static::assertSame( - base64_decode('J4lAqPXhefDrUD7oh5LQMbBH5TE', true), - Base64UrlSafe::decode($publicKeyCredential->id) - ); + static::assertSame(base64_decode('J4lAqPXhefDrUD7oh5LQMbBH5TE', true), $publicKeyCredential->rawId); static::assertSame(base64_decode('J4lAqPXhefDrUD7oh5LQMbBH5TE', true), $publicKeyCredentialDescriptor->id); static::assertSame( PublicKeyCredentialDescriptor::CREDENTIAL_TYPE_PUBLIC_KEY, diff --git a/tests/library/Functional/AttestationStatementWithTokenBindingTest.php b/tests/library/Functional/AttestationStatementWithTokenBindingTest.php index c99a3f9f..d4e74c77 100644 --- a/tests/library/Functional/AttestationStatementWithTokenBindingTest.php +++ b/tests/library/Functional/AttestationStatementWithTokenBindingTest.php @@ -5,7 +5,6 @@ namespace Webauthn\Tests\Functional; use Cose\Algorithms; -use ParagonIE\ConstantTime\Base64UrlSafe; use PHPUnit\Framework\Attributes\Test; use Webauthn\AttestedCredentialData; use Webauthn\AuthenticatorAttestationResponse; @@ -55,7 +54,7 @@ public function anAttestationWithTokenBindingCanBeVerified(): void '+uZVS9+4JgjAYI49YhdzTgHmbn638+ZNSvC0UtHkWTVS+CtTjnaSbqtzdzijByOAvEAsh+TaQJAr43FRj+dYag==', true ), - Base64UrlSafe::decode($publicKeyCredential->id) + $publicKeyCredential->rawId ); static::assertSame( base64_decode( diff --git a/tests/library/Functional/AttestationTest.php b/tests/library/Functional/AttestationTest.php index 22366cfe..d44e0946 100644 --- a/tests/library/Functional/AttestationTest.php +++ b/tests/library/Functional/AttestationTest.php @@ -4,7 +4,6 @@ namespace Webauthn\Tests\Functional; -use ParagonIE\ConstantTime\Base64UrlSafe; use PHPUnit\Framework\Attributes\Test; use RangeException; use Webauthn\AttestedCredentialData; @@ -56,7 +55,7 @@ public function anAttestationSignedWithEcDSA521ShouldBeVerified(): void $publicKeyCredentialDescriptor = $publicKeyCredential->getPublicKeyCredentialDescriptor(); static::assertSame( hex2bin('4787c0563f68b2055564bef21dfb4f7953a68e89b7c70e192caec3b7ff26cce3'), - Base64UrlSafe::decode($publicKeyCredential->id) + $publicKeyCredential->rawId ); static::assertSame( hex2bin('4787c0563f68b2055564bef21dfb4f7953a68e89b7c70e192caec3b7ff26cce3'), diff --git a/tests/library/Functional/PackedAttestationStatementTest.php b/tests/library/Functional/PackedAttestationStatementTest.php index 62cc68f8..01b1b052 100644 --- a/tests/library/Functional/PackedAttestationStatementTest.php +++ b/tests/library/Functional/PackedAttestationStatementTest.php @@ -4,7 +4,6 @@ namespace Webauthn\Tests\Functional; -use ParagonIE\ConstantTime\Base64UrlSafe; use PHPUnit\Framework\Attributes\Test; use Webauthn\AttestedCredentialData; use Webauthn\AuthenticatorAttestationResponse; @@ -52,7 +51,7 @@ public function aPackedAttestationCanBeVerified(): void 'xYw3gEj0LVL83JXz7oKL14XQjh9W1NMFrTALWI+lqXl7ndKW+n8JFYsBCuKbZA3zRAUxAZDHG/tXHsAi6TbO0Q==', true ), - Base64UrlSafe::decode($publicKeyCredential->id) + $publicKeyCredential->rawId ); static::assertSame( base64_decode( @@ -114,7 +113,7 @@ public function aPackedAttestationWithSelfStatementCanBeVerified(): void 'AFkzwaxVuCUz4qFPaNAgnYgoZKKTtvGIAaIASAbnlHGy8UktdI/jN0CetpIkiw9++R0AF9a6OJnHD+G4aIWur+Pxj+sI9xDE+AVeQKve', true ), - Base64UrlSafe::decode($publicKeyCredential->id) + $publicKeyCredential->rawId ); static::assertSame( base64_decode( @@ -171,7 +170,7 @@ public function p2(): void $publicKeyCredentialDescriptor = $publicKeyCredential->getPublicKeyCredentialDescriptor(); static::assertSame( base64_decode('RSRHHrZblfX23SKbu09qBzVp8Y1W1c9GI1EtHZ9gDzY=', true), - Base64UrlSafe::decode($publicKeyCredential->id) + $publicKeyCredential->rawId ); static::assertSame( base64_decode('RSRHHrZblfX23SKbu09qBzVp8Y1W1c9GI1EtHZ9gDzY=', true), diff --git a/tests/library/Functional/W10Test.php b/tests/library/Functional/W10Test.php index 3268b9ae..68a866e5 100644 --- a/tests/library/Functional/W10Test.php +++ b/tests/library/Functional/W10Test.php @@ -4,7 +4,6 @@ namespace Webauthn\Tests\Functional; -use ParagonIE\ConstantTime\Base64UrlSafe; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; use Symfony\Component\Uid\Uuid; @@ -46,7 +45,7 @@ public function anAttestationCanBeVerified( $publicKeyCredentialSource = $this->getAuthenticatorAttestationResponseValidator() ->check($publicKeyCredential->response, $publicKeyCredentialCreationOptions, $host); $publicKeyCredentialDescriptor = $publicKeyCredential->getPublicKeyCredentialDescriptor(); - static::assertSame($credentialId, Base64UrlSafe::decode($publicKeyCredential->id)); + static::assertSame($credentialId, $publicKeyCredential->rawId); static::assertSame($credentialId, $publicKeyCredentialDescriptor->id); static::assertSame( PublicKeyCredentialDescriptor::CREDENTIAL_TYPE_PUBLIC_KEY,