Parse transports and pass them through to the credential (#55)

Pulled out of #53 - it's desirable to store the transports alongside the credential in order to present future UI hints (note: it's not trusted data). Since that PR was getting pretty large, that aspect is being split out for readability. Note: this maintains complete backwards compatibility with the original wire format, but does add a new recommendation to start passing the transports through.
Firehed · Nov 19, 2023 · 982a848 · 982a848
1 parent 66d9dfc
commit 982a848
Show file tree

Hide file tree

Showing 8 changed files with 91 additions and 27 deletions.
diff --git a/README.md b/README.md
@@ -133,6 +133,7 @@ const dataForResponseParser = {
     type: credential.type,
     attestationObject: Array.from(new Uint8Array(credential.response.attestationObject)),
     clientDataJSON: Array.from(new Uint8Array(credential.response.clientDataJSON)),
+    transports: credential.response.getTransports(),
 }
 
 // Send this to your endpoint - adjust to your needs.

diff --git a/examples/readmeRegisterStep2.js b/examples/readmeRegisterStep2.js
@@ -58,6 +58,7 @@ const startRegister = async (e) => {
         type: credential.type,
         attestationObject: Array.from(new Uint8Array(credential.response.attestationObject)),
         clientDataJSON: Array.from(new Uint8Array(credential.response.clientDataJSON)),
+        transports: credential.response.getTransports(),
     }
 
     // Send this to your endpoint - adjust to your needs.

diff --git a/src/ArrayBufferResponseParser.php b/src/ArrayBufferResponseParser.php
@@ -4,6 +4,11 @@
 
 namespace Firehed\WebAuthn;
 
+use function array_filter;
+use function array_key_exists;
+use function array_map;
+use function is_array;
+
 /**
  * Translates the library-official over-the-wire data formats into the
  * necessary data structures for subsequent authentication procedures.
@@ -32,6 +37,7 @@ class ArrayBufferResponseParser implements ResponseParserInterface
      *  type: credential.type,
      *  attestationObject: new Uint8Array(credential.response.attestationObject),
      *  clientDataJSON: new Uint8Array(credential.response.clientDataJSON),
+     *  transports: credential.response.getTransports(),
      * }
      * ```
      *
@@ -42,8 +48,13 @@ class ArrayBufferResponseParser implements ResponseParserInterface
      *   type: string,
      *   attestationObject: int[],
      *   clientDataJSON: int[],
+     *   transports?: string[],
      * }
      *
+     * Note that `transports` has been added after the original data format.
+     * It's RECOMMENDED to be provided in all requests, but this should avoid
+     * disrupting applictions where it is not.
+     *
      * $response is left untyped since it performs additional checking from
      * untrusted user data.
      *
@@ -63,10 +74,17 @@ public function parseCreateResponse(array $response): Responses\AttestationInter
         if (!array_key_exists('clientDataJSON', $response) || !is_array($response['clientDataJSON'])) {
             throw new Errors\ParseError('7.1.2', 'response.clientDataJSON');
         }
+
+        $transports = array_filter(array_map(
+            Enums\AuthenticatorTransport::tryFrom(...),
+            $response['transports'] ?? []
+        ));
+
         return new CreateResponse(
             id: BinaryString::fromBytes($response['rawId']),
             ao: Attestations\AttestationObject::fromCbor(BinaryString::fromBytes($response['attestationObject'])),
             clientDataJson: BinaryString::fromBytes($response['clientDataJSON']),
+            transports: $transports,
         );
     }
 

diff --git a/src/CreateResponse.php b/src/CreateResponse.php
@@ -15,10 +15,17 @@
  */
 class CreateResponse implements Responses\AttestationInterface
 {
+    /**
+     * Note: transports are made public to simplify testing, and are not
+     * considered part of any sort of public API.
+     *
+     * @param Enums\AuthenticatorTransport[] $transports
+     */
     public function __construct(
         private BinaryString $id,
         private Attestations\AttestationObjectInterface $ao,
         private BinaryString $clientDataJson,
+        public readonly array $transports,
     ) {
     }
 
@@ -160,7 +167,6 @@ public function verify(
             coseKey: $data->coseKey,
         );
 
-
         // This is not part of the official procedure, but serves as a general
         // sanity-check around data handling.
         assert($this->id->equals($data->credentialId));

diff --git a/src/JsonResponseParser.php b/src/JsonResponseParser.php
@@ -68,12 +68,19 @@ public function parseCreateResponse(array $data): Responses\AttestationInterface
         if (!array_key_exists('clientDataJSON', $response) || !is_string($response['clientDataJSON'])) {
             throw new Errors\ParseError('7.1.2', 'response.clientDataJSON');
         }
+        if (!array_key_exists('transports', $response) || !is_array($response['transports'])) {
+            throw new Errors\ParseError('7.1.2', 'response.transports');
+        }
+        // "client platforms MUST ignore unknown values" -> tryFrom+filter
+        $transports = array_filter(array_map(Enums\AuthenticatorTransport::tryFrom(...), $response['transports']));
+
         return new CreateResponse(
             id: self::parse($data['rawId'], '7.1.2', 'rawId'),
             ao: Attestations\AttestationObject::fromCbor(
                 self::parse($response['attestationObject'], '7.1.2', 'response.attestationObject'),
             ),
             clientDataJson: self::parse($response['clientDataJSON'], '7.1.2', 'response.clientDataJSON'),
+            transports: $transports,
         );
     }
 

diff --git a/tests/ArrayBufferResponseParserTest.php b/tests/ArrayBufferResponseParserTest.php
@@ -23,6 +23,31 @@ public function testParseCreateResponse(string $directory): void
         self::assertInstanceOf(CreateResponse::class, $attestation);
     }
 
+    public function testParseCreateResponseWithTransports(): void
+    {
+        $response = $this->readFixture('touchid/register.json');
+        $response['transports'] = ['internal', 'hybrid'];
+
+        $parser = new ArrayBufferResponseParser();
+        $parsed = $parser->parseCreateResponse($response);
+        self::assertEqualsCanonicalizing([
+            Enums\AuthenticatorTransport::Hybrid,
+            Enums\AuthenticatorTransport::Internal,
+        ], $parsed->transports); // @phpstan-ignore-line (interface/impl cheat)
+    }
+
+    public function testParseCreateResponseWithInvalidTransports(): void
+    {
+        $response = $this->readFixture('touchid/register.json');
+        $response['transports'] = ['invalid', 'usb'];
+
+        $parser = new ArrayBufferResponseParser();
+        $parsed = $parser->parseCreateResponse($response);
+        self::assertEqualsCanonicalizing([
+            Enums\AuthenticatorTransport::Usb,
+        ], $parsed->transports); // @phpstan-ignore-line (interface/impl cheat)
+    }
+
     /**
      * @dataProvider badCreateResponses
      * @param mixed[] $response

diff --git a/tests/CreateResponseTest.php b/tests/CreateResponseTest.php
@@ -175,11 +175,7 @@ public function setUp(): void
 
     public function testIsUserVerified(): void
     {
-        $response = new CreateResponse(
-            id: $this->id,
-            ao: $this->attestationObject,
-            clientDataJson: $this->clientDataJson,
-        );
+        $response = $this->getDefaultResponse();
         self::assertFalse($response->isUserVerified(), 'Fixture is not verified');
     }
 
@@ -195,6 +191,7 @@ public function testCDJTypeMismatchIsError(): void
             id: $this->id,
             ao: $this->attestationObject,
             clientDataJson: $newCdj,
+            transports: [],
         );
 
         $this->expectRegistrationError('7.1.7');
@@ -203,11 +200,7 @@ public function testCDJTypeMismatchIsError(): void
 
     public function testUsedChallengeIsError(): void
     {
-        $response = new CreateResponse(
-            id: $this->id,
-            ao: $this->attestationObject,
-            clientDataJson: $this->clientDataJson,
-        );
+        $response = $this->getDefaultResponse();
 
         $cred = $response->verify($this->cm, $this->rp);
 
@@ -228,6 +221,7 @@ public function testCDJChallengeMismatchIsError(): void
             id: $this->id,
             ao: $this->attestationObject,
             clientDataJson: $newCdj,
+            transports: [],
         );
 
         $this->expectRegistrationError('7.1.8');
@@ -246,6 +240,7 @@ public function testCDJOriginMismatchIsError(): void
             id: $this->id,
             ao: $this->attestationObject,
             clientDataJson: $newCdj,
+            transports: [],
         );
 
         $this->expectRegistrationError('7.1.0');
@@ -256,11 +251,7 @@ public function testCDJOriginMismatchIsError(): void
     public function testRelyingPartyIdMismatchIsError(): void
     {
         $rp = new SingleOriginRelyingParty('https://some-other-site.example.com');
-        $response = new CreateResponse(
-            id: $this->id,
-            ao: $this->attestationObject,
-            clientDataJson: $this->clientDataJson,
-        );
+        $response = $this->getDefaultResponse();
 
         $this->expectRegistrationError('7.1.13');
         $response->verify($this->cm, $rp);
@@ -276,11 +267,7 @@ public function testUserNotPresentIsError(): void
     // 7.1.15
     public function testUserVerifiedNotPresentWhenRequiredIsError(): void
     {
-        $response = new CreateResponse(
-            id: $this->id,
-            ao: $this->attestationObject,
-            clientDataJson: $this->clientDataJson,
-        );
+        $response = $this->getDefaultResponse();
 
         $this->expectRegistrationError('7.1.15');
         $response->verify($this->cm, $this->rp, Enums\UserVerificationRequirement::Required);
@@ -322,18 +309,14 @@ public function testFormatSpecificVerificationOccurs(): void
             id: $this->id,
             ao: $ao,
             clientDataJson: $this->clientDataJson,
+            transports: [],
         );
         $response->verify($this->cm, $this->rp);
     }
 
     public function testSuccess(): void
     {
-        $response = new CreateResponse(
-            id: $this->id,
-            ao: $this->attestationObject,
-            clientDataJson: $this->clientDataJson,
-        );
-
+        $response = $this->getDefaultResponse();
         $cred = $response->verify($this->cm, $this->rp);
 
         self::assertSame(0, $cred->getSignCount());
@@ -345,4 +328,14 @@ private function expectRegistrationError(string $section): void
         $this->expectException(Errors\RegistrationError::class);
         // TODO: how to assert on $section
     }
+
+    private function getDefaultResponse(): CreateResponse
+    {
+        return new CreateResponse(
+            id: $this->id,
+            ao: $this->attestationObject,
+            clientDataJson: $this->clientDataJson,
+            transports: [],
+        );
+    }
 }
diff --git a/tests/JsonResponseParserTest.php b/tests/JsonResponseParserTest.php
@@ -34,6 +34,18 @@ public function testParseCreateResponseInputValidation(array $response): void
         $parser->parseCreateResponse($response);
     }
 
+    public function testParseCreateResponseWithInvalidTransports(): void
+    {
+        $response = $this->readFixture('safari-passkey-polyfill/register.json');
+        $response['response']['transports'] = ['invalid', 'usb']; // @phpstan-ignore-line
+
+        $parser = new JsonResponseParser();
+        $parsed = $parser->parseCreateResponse($response);
+        self::assertEqualsCanonicalizing([
+            Enums\AuthenticatorTransport::Usb,
+        ], $parsed->transports); // @phpstan-ignore-line (interface/impl cheat)
+    }
+
     /**
      * @dataProvider badGetResponses
      * @param mixed[] $response
@@ -109,6 +121,7 @@ public function badCreateResponses(): array
             'invalid attestationObject' => $makeVector(['response' => ['attestationObject' => 'not base 64']]),
             'no clientDataJSON' => $makeVector(['response' => ['clientDataJSON' => null]]),
             'invalid clientDataJSON' => $makeVector(['response' => ['clientDataJSON' => 'not base 64']]),
+            'no transports' => $makeVector(['response' => ['transports' => null]]),
         ];
     }