From 03b7a7fa9bb847671497ad72be264c558cb9db1e Mon Sep 17 00:00:00 2001 From: Florent Morselli Date: Sat, 15 Jun 2024 12:13:23 +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 | 28 +++++++++++++++++-- .../PublicKeyCredentialDenormalizer.php | 2 +- src/webauthn/src/PublicKeyCredential.php | 8 +++--- .../src/PublicKeyCredentialLoader.php | 2 +- .../AppleAttestationStatementTest.php | 6 +--- ...testationStatementWithTokenBindingTest.php | 3 +- tests/library/Functional/AttestationTest.php | 3 +- .../PackedAttestationStatementTest.php | 7 ++--- tests/library/Functional/W10Test.php | 3 +- 9 files changed, 39 insertions(+), 23 deletions(-) diff --git a/src/webauthn/src/Credential.php b/src/webauthn/src/Credential.php index cbad84cc..c53e8585 100644 --- a/src/webauthn/src/Credential.php +++ b/src/webauthn/src/Credential.php @@ -4,15 +4,39 @@ namespace Webauthn; +use InvalidArgumentException; + /** * @see https://w3c.github.io/webappsec-credential-management/#credential */ abstract class Credential { + /** + * @deprecated since 4.9.0. Please use the property rawId instead. + */ + public readonly string $id; + + public readonly string $rawId; + public function __construct( - public readonly string $id, - public readonly string $type + null|string $id, + public readonly string $type, + null|string $rawId = null, ) { + if ($id === null && $rawId === null) { + throw new InvalidArgumentException('You must provide a valid raw ID'); + } + 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); + } + $this->id = $id; + $this->rawId = $rawId ?? base64_encode($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..778d138b 100644 --- a/src/webauthn/src/PublicKeyCredential.php +++ b/src/webauthn/src/PublicKeyCredential.php @@ -14,12 +14,12 @@ 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); + parent::__construct($id, $type, $rawId); } /** @@ -31,7 +31,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,