Skip to content

Commit

Permalink
Replace deprecated id with rawId (#589)
Browse files Browse the repository at this point in the history
"id" has been deprecated in the Credential class and it was replaced by the new property "$rawId". This change was also included in several functional tests where Base64UrlSafe was being used previously. Now, 'rawId' is used throughout the system to maintain consistency. A validation to ensure the presence of a 'rawId' in the constructor of the Credential class has also been added.
  • Loading branch information
Spomky authored Jun 29, 2024
1 parent 51b7a85 commit 82b5cd3
Show file tree
Hide file tree
Showing 10 changed files with 47 additions and 23 deletions.
8 changes: 8 additions & 0 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -2348,6 +2348,14 @@ parameters:
count: 1
path: src/webauthn/src/CollectedClientData.php

-
message: """
#^Access to deprecated property \\$id of class Webauthn\\\\Credential\\:
since 4\\.9\\.0\\. Please use the property rawId instead\\.$#
"""
count: 1
path: src/webauthn/src/Credential.php

-
message: "#^Cannot access offset 'authData' on mixed\\.$#"
count: 1
Expand Down
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 82b5cd3

Please sign in to comment.