Merge pull request #97 from creative-commoners/pulls/4/php8

DEP PHP 8 support

client/src/lib/convert.js

@@ -1,5 +1,12 @@
-export const base64ToByteArray = string =>
-  Uint8Array.from(atob(string), c => c.charCodeAt(0));
+export const base64ToByteArray = string => {
+  // String replace because server with encode with 'web safe' base64_encode
+  const b = atob(string.replace(/_/g, '/').replace(/-/g, '+'));
+  return Uint8Array.from(b, c => c.charCodeAt(0));
+};
 
-export const byteArrayToBase64 = byteArray =>
-  btoa(String.fromCharCode(...(new Uint8Array(byteArray))));
+export const byteArrayToBase64 = byteArray => {
+  // We specifically do not want to make the 'web safe' string replacements above
+  // doing so will break this functionality
+  const uarr = new Uint8Array(byteArray);
+  return btoa(String.fromCharCode(...uarr));
+};

client/src/lib/tests/convert-test.js

@@ -0,0 +1,28 @@
+/* global jest, describe, it, expect, window */
+
+import { base64ToByteArray, byteArrayToBase64 } from '../convert';
+
+describe('convert', () => {
+    it('converts base64ToByteArray', () => {
+        const string = 'abc/def+ghi';
+        const expected = new Uint8Array([105, 183, 63, 117, 231, 254, 130, 24]);
+        const actual = base64ToByteArray(string);
+        expect(expected).toEqual(actual);
+    });
+
+    it('converts base64ToByteArray when encoded web-safe', () => {
+        const string = 'abc_def-ghi';
+        const expected = new Uint8Array([105, 183, 63, 117, 231, 254, 130, 24]);
+        const actual = base64ToByteArray(string);
+        expect(expected).toEqual(actual);
+    });
+
+    it('converts byteArrayToBase64', () => {
+        const byteArray = [105, 183, 63, 117, 231, 254, 130, 24];
+        // this is supposed to be slightly different from string
+        // used in base64ToByteArray tests
+        const expected = 'abc/def+ghg=';
+        const actual = byteArrayToBase64(byteArray);
+        expect(expected).toEqual(actual);
+    });
+});

composer.json

@@ -9,8 +9,9 @@
     "license": "BSD-3-Clause",
     "require": {
         "php": "^7.3 || ^8.0",
+        "ext-bcmath": "*",
         "silverstripe/mfa": "^4.0",
-        "web-auth/webauthn-lib": "^1.0",
+        "web-auth/webauthn-lib": "^3.3",
         "guzzlehttp/psr7": "^1.6"
     },
     "require-dev": {

src/BaseHandlerTrait.php

@@ -21,6 +21,10 @@ trait BaseHandlerTrait
 {
     /**
      * @return Decoder
+     *
+     * @deprecated will be removed in 5.0 - as of v3 of webauthn-lib the Decoder is now created within both:
+     * AttestationObjectLoader::__construct()
+     * AuthenticatorAssertionResponseValidator::__construct()
      */
     protected function getDecoder(): Decoder
     {
@@ -48,7 +52,7 @@ protected function getAttestationObjectLoader(
         AttestationStatementSupportManager $attestationStatementSupportManager,
         Decoder $decoder
     ): AttestationObjectLoader {
-        return new AttestationObjectLoader($attestationStatementSupportManager, $decoder);
+        return new AttestationObjectLoader($attestationStatementSupportManager);
     }
 
     /**
@@ -60,6 +64,6 @@ protected function getPublicKeyCredentialLoader(
         AttestationObjectLoader $attestationObjectLoader,
         Decoder $decoder
     ): PublicKeyCredentialLoader {
-        return new PublicKeyCredentialLoader($attestationObjectLoader, $decoder);
+        return new PublicKeyCredentialLoader($attestationObjectLoader);
     }
 }

src/RegisterHandler.php

@@ -109,6 +109,7 @@ public function register(HTTPRequest $request, StoreInterface $store): Result
     {
         $options = $this->getCredentialCreationOptions($store);
         $data = json_decode((string) $request->getBody(), true);
+        $publicKeyCredentialSource = null;
 
         try {
             if (empty($data['credentials'])) {
@@ -134,7 +135,8 @@ public function register(HTTPRequest $request, StoreInterface $store): Result
             $psrRequest = ServerRequest::fromGlobals();
 
             // Validate the webauthn response
-            $this->getAuthenticatorAttestationResponseValidator($attestationStatementSupportManager, $store)
+            $publicKeyCredentialSource = $this
+                ->getAuthenticatorAttestationResponseValidator($attestationStatementSupportManager, $store)
                 ->check($response, $options, $psrRequest);
         } catch (Exception $e) {
             $this->logger->error($e->getMessage());
@@ -143,17 +145,12 @@ public function register(HTTPRequest $request, StoreInterface $store): Result
 
         $credentialRepository = $this->getCredentialRepository($store);
 
-        $source = PublicKeyCredentialSource::createFromPublicKeyCredential(
-            $publicKeyCredential,
-            $options->getUser()->getId()
-        );
-
         // Clear the repository so only one key is registered at a time
         // NOTE: This can be considered temporary behaviour until the UI supports managing multiple keys
         $credentialRepository->reset();
 
         // Persist the "credential source"
-        $credentialRepository->saveCredentialSource($source);
+        $credentialRepository->saveCredentialSource($publicKeyCredentialSource);
 
         return Result::create()->setContext($credentialRepository->toArray());
     }
@@ -259,12 +256,10 @@ protected function getCredentialCreationOptions(
             $this->getUserEntity($store->getMember()),
             random_bytes(32),
             [new PublicKeyCredentialParameters('public-key', Algorithms::COSE_ALGORITHM_ES256)],
-            40000,
-            [],
-            $this->getAuthenticatorSelectionCriteria(),
-            PublicKeyCredentialCreationOptions::ATTESTATION_CONVEYANCE_PREFERENCE_NONE,
-            new AuthenticationExtensionsClientInputs()
+            40000
         );
+        $credentialOptions->setAuthenticatorSelection($this->getAuthenticatorSelectionCriteria());
+        $credentialOptions->setAttestation(PublicKeyCredentialCreationOptions::ATTESTATION_CONVEYANCE_PREFERENCE_NONE);
 
         $store->setState(['credentialOptions' => $credentialOptions] + $state);
 

src/VerifyHandler.php

@@ -5,6 +5,7 @@
 namespace SilverStripe\WebAuthn;
 
 use CBOR\Decoder;
+use Cose\Algorithm\Manager;
 use Exception;
 use GuzzleHttp\Psr7\ServerRequest;
 use InvalidArgumentException;
@@ -22,6 +23,7 @@
 use Webauthn\PublicKeyCredentialRequestOptions;
 use Webauthn\PublicKeyCredentialSource;
 use Webauthn\TokenBinding\TokenBindingNotSupportedHandler;
+use Cose\Algorithm\Signature\ECDSA\ES256;
 
 class VerifyHandler implements VerifyHandlerInterface
 {
@@ -160,13 +162,9 @@ protected function getCredentialRequestOptions(
             return $source->getPublicKeyCredentialDescriptor();
         }, $validCredentials);
 
-        $options = new PublicKeyCredentialRequestOptions(
-            random_bytes(32),
-            40000,
-            null,
-            $descriptors,
-            PublicKeyCredentialRequestOptions::USER_VERIFICATION_REQUIREMENT_PREFERRED
-        );
+        $options = new PublicKeyCredentialRequestOptions(random_bytes(32), 40000);
+        $options->allowCredentials($descriptors);
+        $options->setUserVerification(PublicKeyCredentialRequestOptions::USER_VERIFICATION_REQUIREMENT_PREFERRED);
 
         // Persist the options for later
         $store->addState(['credentialOptions' => $options]);
@@ -183,11 +181,13 @@ protected function getAuthenticatorAssertionResponseValidator(
         Decoder $decoder,
         StoreInterface $store
     ): AuthenticatorAssertionResponseValidator {
+        $manager = new Manager();
+        $manager->add(new ES256());
         return new AuthenticatorAssertionResponseValidator(
             $this->getCredentialRepository($store),
-            $decoder,
             new TokenBindingNotSupportedHandler(),
-            new ExtensionOutputCheckerHandler()
+            new ExtensionOutputCheckerHandler(),
+            $manager
         );
     }
 }

tests/RegisterHandlerTest.php

@@ -180,7 +180,10 @@ public function testRegister(
             ->setMethods(['getPublicKeyCredentialLoader', 'getAuthenticatorAttestationResponseValidator'])
             ->getMock();
 
+        $publicKeyCredentialSourceMock = $this->createMock(PublicKeyCredentialSource::class);
         $responseValidatorMock = $this->createMock(AuthenticatorAttestationResponseValidator::class);
+        $responseValidatorMock->method('check')->willReturn($publicKeyCredentialSourceMock);
+
         // Allow the data provider to customise the validation check handling
         if ($responseValidatorMockCallback) {
             $responseValidatorMockCallback($responseValidatorMock);

tests/VerifyHandlerTest.php

@@ -19,6 +19,7 @@
 use Webauthn\AuthenticatorResponse;
 use Webauthn\PublicKeyCredential;
 use Webauthn\PublicKeyCredentialLoader;
+use Webauthn\PublicKeyCredentialSource;
 
 class VerifyHandlerTest extends SapphireTest
 {
@@ -132,7 +133,10 @@ public function testVerify(
             ->setMethods(['getPublicKeyCredentialLoader', 'getAuthenticatorAssertionResponseValidator'])
             ->getMock();
 
+        $publicKeyCredentialSourceMock = $this->createMock(PublicKeyCredentialSource::class);
         $responseValidatorMock = $this->createMock(AuthenticatorAssertionResponseValidator::class);
+        $responseValidatorMock->method('check')->willReturn($publicKeyCredentialSourceMock);
+
         // Allow the data provider to customise the validation check handling
         if ($responseValidatorMockCallback) {
             $responseValidatorMockCallback($responseValidatorMock);