Add intermediate data structure for attested credential data (#52)

Rather than having the `AuthenticatorData` _build_ the Credential during registration, it's now being adjusted to instead return the underlying [components](https://www.w3.org/TR/webauthn-3/#sctn-attestation) that it will need. While this is a minor structural formality right now, it will ease the adoption of the updated credential storage recommendations described in the Level 3 version of the spec that are not captured in the current L2-based implementation.
Firehed · Nov 19, 2023 · 8e7f191 · 8e7f191
1 parent a0562be
commit 8e7f191
Show file tree

Hide file tree

Showing 6 changed files with 42 additions and 32 deletions.
diff --git a/src/Attestations/FidoU2F.php b/src/Attestations/FidoU2F.php
@@ -55,9 +55,9 @@ public function verify(AuthenticatorData $data, BinaryString $clientDataHash): V
 
         // 8.6.v.3
         $rpIdHash = $data->getRpIdHash();
-        $attestedCredential = $data->getAttestedCredential();
-        $credentialId = $attestedCredential->getId();
-        $credentialPublicKey = $attestedCredential->getPublicKey();
+        $attestedCredentialData = $data->getAttestedCredentialData();
+        $credentialId = $attestedCredentialData->credentialId;
+        $credentialPublicKey = $attestedCredentialData->coseKey->getPublicKey();
         assert($credentialPublicKey instanceof EllipticCurve);
 
         // 8.6.v.4

diff --git a/src/AttestedCredentialData.php b/src/AttestedCredentialData.php
@@ -0,0 +1,18 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Firehed\WebAuthn;
+
+/**
+ * @internal
+ */
+class AttestedCredentialData
+{
+    public function __construct(
+        public readonly BinaryString $aaguid,
+        public readonly BinaryString $credentialId,
+        public readonly COSEKey $coseKey,
+    ) {
+    }
+}
diff --git a/src/AuthenticatorData.php b/src/AuthenticatorData.php
@@ -11,12 +11,6 @@
  * @internal
  *
  * @link https://www.w3.org/TR/webauthn-3/#sctn-authenticator-data
- *
- * @phpstan-type AttestedCredentialData array{
- *   aaguid: BinaryString,
- *   credentialId: BinaryString,
- *   credentialPublicKey: BinaryString,
- * }
  */
 class AuthenticatorData
 {
@@ -30,10 +24,7 @@ class AuthenticatorData
 
     private int $signCount;
 
-    /**
-     * @var ?AttestedCredentialData Attested Credential Data
-     */
-    private ?array $ACD = null;
+    private ?AttestedCredentialData $ACD = null;
 
     /**
      * @see https://w3c.github.io/webauthn/#sec-authenticator-data
@@ -80,11 +71,11 @@ public static function parse(BinaryString $bytes): AuthenticatorData
             // FIXME: support extension data & offset handling
             $rawCredentialPublicKey = $bytes->getRemaining();
 
-            $authData->ACD = [
-                'aaguid' => new BinaryString($aaguid),
-                'credentialId' => new BinaryString($credentialId),
-                'credentialPublicKey' => new BinaryString($rawCredentialPublicKey),
-            ];
+            $authData->ACD = new AttestedCredentialData(
+                aaguid: new BinaryString($aaguid),
+                credentialId: new BinaryString($credentialId),
+                coseKey: new COSEKey(new BinaryString($rawCredentialPublicKey)),
+            );
         }
         if ($ED) {
             // @codeCoverageIgnoreStart
@@ -95,7 +86,7 @@ public static function parse(BinaryString $bytes): AuthenticatorData
         return $authData;
     }
 
-    public function getAttestedCredential(): CredentialInterface
+    public function getAttestedCredentialData(): AttestedCredentialData
     {
         if ($this->ACD === null) {
             throw new OutOfRangeException(
@@ -110,11 +101,7 @@ public function getAttestedCredential(): CredentialInterface
             );
         }
 
-        return new Credential(
-            $this->ACD['credentialId'],
-            new COSEKey($this->ACD['credentialPublicKey']),
-            $this->signCount,
-        );
+        return $this->ACD;
     }
 
     public function getRpIdHash(): BinaryString

diff --git a/src/CreateResponse.php b/src/CreateResponse.php
@@ -153,12 +153,17 @@ public function verify(
         // 7.1.27
         // associate credential with new user
         // done in client code
-        $credential = $authData->getAttestedCredential();
+        $data = $authData->getAttestedCredentialData();
+        $credential = new Credential(
+            id: $this->id,
+            signCount: $authData->getSignCount(),
+            coseKey: $data->coseKey,
+        );
+
 
         // This is not part of the official procedure, but serves as a general
-        // sanity-check around data handling. It also silences an unused
-        // variable warning in PHPStan :)
-        assert($credential->getId()->equals($this->id));
+        // sanity-check around data handling.
+        assert($this->id->equals($data->credentialId));
 
         return $credential;
 

diff --git a/tests/Attestations/AttestationObjectTest.php b/tests/Attestations/AttestationObjectTest.php
@@ -140,7 +140,7 @@ public function testParsingCBOR(): void
         );
         self::assertSame(0, $ad->getSignCount(), 'sign count');
 
-        $credential = $ad->getAttestedCredential();
+        $credential = $ad->getAttestedCredentialData();
         // TODO: check keypair?
     }
 

diff --git a/tests/AuthenticatorDataTest.php b/tests/AuthenticatorDataTest.php
@@ -29,7 +29,7 @@ public function testParseAssertion(): void
         self::assertTrue($ad->isUserVerified(), 'Flags bit 2 is set, UV=true');
         self::assertSame(0, $ad->getSignCount(), 'Sign count should be zero');
         try {
-            $_ = $ad->getAttestedCredential();
+            $_ = $ad->getAttestedCredentialData();
             self::fail('AuthData does not include an attested credential');
         } catch (\Throwable) {
         }
@@ -70,7 +70,7 @@ public function testParseAttestation(): void
         );
         self::assertTrue($ad->isUserPresent(), 'Flags bit 0 is set, UP=true');
         self::assertTrue($ad->isUserVerified(), 'Flags bit 2 is set, UV=true');
-        $_ = $ad->getAttestedCredential(); // Checking that this dones't throw.
+        $_ = $ad->getAttestedCredentialData(); // Checking that this doesn't throw.
     }
 
     public function testParseAssertionWithNoFlags(): void
@@ -93,7 +93,7 @@ public function testParseAssertionWithNoFlags(): void
         self::assertFalse($ad->isUserVerified(), 'Flags bit 2 is not set, UV=false');
         self::assertSame(258, $ad->getSignCount(), 'Sign count wrong');
         try {
-            $_ = $ad->getAttestedCredential();
+            $_ = $ad->getAttestedCredentialData();
             self::fail('AuthData does not include an attested credential');
         } catch (\Throwable) {
         }