Skip to content

Commit

Permalink
Refactor code for deprecation of "id" property
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Spomky committed Jun 15, 2024
1 parent 684f7cb commit 03b7a7f
Show file tree
Hide file tree
Showing 9 changed files with 39 additions and 23 deletions.
28 changes: 26 additions & 2 deletions src/webauthn/src/Credential.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
8 changes: 4 additions & 4 deletions src/webauthn/src/PublicKeyCredential.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand All @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion src/webauthn/src/PublicKeyCredentialLoader.php
Original file line number Diff line number Diff line change
Expand Up @@ -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'])
Expand Down
6 changes: 1 addition & 5 deletions tests/library/Functional/AppleAttestationStatementTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(
Expand Down
3 changes: 1 addition & 2 deletions tests/library/Functional/AttestationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

namespace Webauthn\Tests\Functional;

use ParagonIE\ConstantTime\Base64UrlSafe;
use PHPUnit\Framework\Attributes\Test;
use RangeException;
use Webauthn\AttestedCredentialData;
Expand Down Expand Up @@ -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'),
Expand Down
7 changes: 3 additions & 4 deletions tests/library/Functional/PackedAttestationStatementTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

namespace Webauthn\Tests\Functional;

use ParagonIE\ConstantTime\Base64UrlSafe;
use PHPUnit\Framework\Attributes\Test;
use Webauthn\AttestedCredentialData;
use Webauthn\AuthenticatorAttestationResponse;
Expand Down Expand Up @@ -52,7 +51,7 @@ public function aPackedAttestationCanBeVerified(): void
'xYw3gEj0LVL83JXz7oKL14XQjh9W1NMFrTALWI+lqXl7ndKW+n8JFYsBCuKbZA3zRAUxAZDHG/tXHsAi6TbO0Q==',
true
),
Base64UrlSafe::decode($publicKeyCredential->id)
$publicKeyCredential->rawId
);
static::assertSame(
base64_decode(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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),
Expand Down
3 changes: 1 addition & 2 deletions tests/library/Functional/W10Test.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 03b7a7f

Please sign in to comment.