assert that crendentialId and credential fits into database fields

both values have no defined length, possibly the database fields are still too short

composer.json

@@ -46,7 +46,8 @@
     "nyholm/psr7-server": "^1.0.2",
     "ext-json": "*",
     "d3/testingtools": "^1.1",
-    "d3/oxid-dic-handler": "^1.0"
+    "d3/oxid-dic-handler": "^1.0",
+    "beberlei/assert": "^3.2"
   },
   "require-dev": {
     "friendsofphp/php-cs-fixer": "^2.19",

src/Application/Controller/Admin/d3user_webauthn.php

@@ -93,7 +93,6 @@ class d3user_webauthn extends AdminDetailsController
 
     /**
      * @return void
-     * @throws AssertionFailedException
      * @throws Throwable
      */
     public function saveAuthn(): void
@@ -116,7 +115,7 @@ class d3user_webauthn extends AdminDetailsController
             d3GetOxidDIC()->get('d3ox.webauthn.'.LoggerInterface::class)->error($e->getDetailedErrorMessage(), ['UserId' => $this->getEditObjectId()]);
             d3GetOxidDIC()->get('d3ox.webauthn.'.LoggerInterface::class)->debug($e->getTraceAsString());
             d3GetOxidDIC()->get('d3ox.webauthn.'.UtilsView::class)->addErrorToDisplay($e);
-        } catch (Exception|NotFoundExceptionInterface|ContainerExceptionInterface|DoctrineDriverException $e) {
+        } catch (Exception|NotFoundExceptionInterface|ContainerExceptionInterface|DoctrineDriverException|AssertionFailedException $e) {
             d3GetOxidDIC()->get('d3ox.webauthn.'.LoggerInterface::class)->error($e->getMessage(), ['UserId' => $this->getEditObjectId()]);
             d3GetOxidDIC()->get('d3ox.webauthn.'.LoggerInterface::class)->debug($e->getTraceAsString());
             d3GetOxidDIC()->get('d3ox.webauthn.'.UtilsView::class)->addErrorToDisplay($e->getMessage());

src/Application/Controller/d3_account_webauthn.php

@@ -26,6 +26,7 @@ use D3\Webauthn\Application\Model\Webauthn;
 use Doctrine\DBAL\Driver\Exception as DoctrineDriverException;
 use Doctrine\DBAL\Exception as DoctrineException;
 use OxidEsales\Eshop\Application\Controller\AccountController;
+use OxidEsales\Eshop\Core\Language;
 use OxidEsales\Eshop\Core\Registry;
 use OxidEsales\Eshop\Core\Request;
 use OxidEsales\Eshop\Core\SeoEncoder;
@@ -118,7 +119,6 @@ class d3_account_webauthn extends AccountController
      * @return void
      * @throws DoctrineDriverException
      * @throws DoctrineException
-     * @throws AssertionFailedException
      * @throws Throwable
      */
     public function saveAuthn(): void
@@ -140,7 +140,23 @@ class d3_account_webauthn extends AccountController
                 $webauthn->saveAuthn($credential, d3GetOxidDIC()->get('d3ox.webauthn.'.Request::class)->getRequestEscapedParameter('keyname'));
             }
         } catch (WebauthnException $e) {
+            d3GetOxidDIC()->get('d3ox.webauthn.'.LoggerInterface::class)->error(
+                $e->getDetailedErrorMessage(),
+                ['UserId' => $this->getUser()->getId()]
+            );
+            d3GetOxidDIC()->get('d3ox.webauthn.'.LoggerInterface::class)->debug($e->getTraceAsString());
             d3GetOxidDIC()->get('d3ox.webauthn.'.UtilsView::class)->addErrorToDisplay($e);
+        } catch (AssertionFailedException $e) {
+            /** @var Language $language */
+            $language = d3GetOxidDIC()->get('d3ox.webauthn.'.Language::class);
+            d3GetOxidDIC()->get('d3ox.webauthn.'.LoggerInterface::class)->error(
+                $e->getMessage(),
+                ['UserId' => $this->getUser()->getId()]
+            );
+            d3GetOxidDIC()->get('d3ox.webauthn.'.LoggerInterface::class)->debug($e->getTraceAsString());
+            d3GetOxidDIC()->get('d3ox.webauthn.'.UtilsView::class)->addErrorToDisplay(
+                $language->translateString('D3_WEBAUTHN_ERR_NOTCREDENTIALNOTSAVEABLE')
+            );
         }
     }
 

src/Application/Model/Credential/PublicKeyCredential.php

@@ -15,7 +15,10 @@ declare(strict_types=1);
 
 namespace D3\Webauthn\Application\Model\Credential;
 
+use Assert\Assert;
+use Assert\AssertionFailedException;
 use D3\TestingTools\Production\IsMockable;
+use D3\Webauthn\Setup\Actions;
 use DateTime;
 use Doctrine\DBAL\Driver\Exception as DoctrineDriverException;
 use Doctrine\DBAL\Exception as DoctrineException;
@@ -63,11 +66,20 @@ class PublicKeyCredential extends BaseModel
 
     /**
      * @param string $credentialId
+     * @throws AssertionFailedException
      */
     public function setCredentialId(string $credentialId): void
     {
+        $encodedCID = base64_encode($credentialId);
+
+        Assert::that($encodedCID)
+            ->maxLength(
+                Actions::FIELDLENGTH_CREDID,
+                'the credentialId (%3$d) does not fit into the database field (%2$d)'
+            );
+
         $this->assign([
-            'credentialid' => base64_encode($credentialId),
+            'credentialid' => $encodedCID,
         ]);
     }
 
@@ -99,11 +111,20 @@ class PublicKeyCredential extends BaseModel
 
     /**
      * @param PublicKeyCredentialSource $credential
+     * @throws AssertionFailedException
      */
     public function setCredential(PublicKeyCredentialSource $credential): void
     {
+        $encodedCredential = base64_encode(serialize($credential));
+
+        Assert::that($encodedCredential)
+            ->maxLength(
+                Actions::FIELDLENGTH_CREDENTIAL,
+                'the credential source (%3$d) does not fit into the database field (%2$d)',
+            );
+
         $this->assign([
-            'credential' => base64_encode(serialize($credential)),
+            'credential' => $encodedCredential,
         ]);
     }
 
@@ -131,6 +152,7 @@ class PublicKeyCredential extends BaseModel
      * @throws DoctrineException
      * @throws NotFoundExceptionInterface
      * @throws Exception
+     * @throws AssertionFailedException
      */
     public function saveCredentialSource(PublicKeyCredentialSource $publicKeyCredentialSource, string $keyName = null): void
     {

src/Application/Model/Credential/PublicKeyCredentialList.php

@@ -15,6 +15,7 @@ declare(strict_types=1);
 
 namespace D3\Webauthn\Application\Model\Credential;
 
+use Assert\AssertionFailedException;
 use D3\TestingTools\Production\IsMockable;
 use Doctrine\DBAL\Driver\Exception as DoctrineDriverException;
 use Doctrine\DBAL\Exception as DoctrineException;
@@ -158,8 +159,10 @@ class PublicKeyCredentialList extends ListModel implements PublicKeyCredentialSo
     }
 
     /**
+     * unused, but must fulfil the interface
      * @param PublicKeyCredentialSource $publicKeyCredentialSource
      * @return void
+     * @throws AssertionFailedException
      */
     public function saveCredentialSource(PublicKeyCredentialSource $publicKeyCredentialSource): void
     {

src/Application/translations/de/d3webauthn_lang.php

@@ -47,4 +47,5 @@ $aLang = [
     'D3_WEBAUTHN_ERR_UNSECURECONNECTION'   => 'Die Verwendung von Sicherheitsschlüsseln ist nur bei lokalen oder gesicherten Verbindungen (https) möglich.',
     'D3_WEBAUTHN_ERR_LOGINPROHIBITED'      => 'Die Anmeldung mit Sicherheitsschlüssel ist aus technischen Gründen derzeit leider nicht möglich. Bitte verwenden Sie statt dessen Ihr Passwort.',
     'D3_WEBAUTHN_ERR_NOTLOADEDUSER'        => "Kann keine Anmeldedaten von nicht geladenem Kundenkonto beziehen.",
+    'D3_WEBAUTHN_ERR_NOTCREDENTIALNOTSAVEABLE' => "Der Schlüssel kann aus technischen Gründen nicht registriert werden. Bitte wenden Sie sich an den Shopbetreiber.",
 ];

src/Setup/Actions.php

@@ -39,6 +39,9 @@ use Psr\Log\LoggerInterface;
 
 class Actions
 {
+    public const FIELDLENGTH_CREDID = 512;
+    public const FIELDLENGTH_CREDENTIAL = 2000;
+
     use IsMockable;
 
     public $seo_de = 'sicherheitsschluessel';
@@ -56,8 +59,8 @@ class Actions
             `OXUSERID` char(32) NOT NULL,
             `OXSHOPID` int(11) NOT NULL,
             `NAME` varchar(100) NOT NULL,
-            `CREDENTIALID` varchar(512) NOT NULL,
+            `CREDENTIALID` varchar(".self::FIELDLENGTH_CREDID.") NOT NULL,
-            `CREDENTIAL` varchar(2000) NOT NULL,
+            `CREDENTIAL` varchar(".self::FIELDLENGTH_CREDENTIAL.") NOT NULL,
             `OXTIMESTAMP` timestamp NOT NULL DEFAULT current_timestamp() ON UPDATE current_timestamp(),
             PRIMARY KEY (`OXID`),
             KEY `CREDENTIALID_IDX` (`CREDENTIALID`),

src/tests/unit/Application/Controller/d3_account_webauthnTest.php

@@ -15,6 +15,7 @@ declare(strict_types=1);
 
 namespace D3\Webauthn\tests\unit\Application\Controller;
 
+use Assert\InvalidArgumentException;
 use D3\TestingTools\Development\CanAccessRestricted;
 use D3\Webauthn\Application\Controller\d3_account_webauthn;
 use D3\Webauthn\Application\Model\Credential\PublicKeyCredential;
@@ -22,6 +23,7 @@ use D3\Webauthn\Application\Model\Credential\PublicKeyCredentialList;
 use D3\Webauthn\Application\Model\Exceptions\WebauthnException;
 use D3\Webauthn\Application\Model\Webauthn;
 use D3\Webauthn\tests\unit\WAUnitTestCase;
+use Generator;
 use OxidEsales\Eshop\Application\Model\User;
 use OxidEsales\Eshop\Core\Registry;
 use OxidEsales\Eshop\Core\Request;
@@ -287,14 +289,12 @@ class d3_account_webauthnTest extends WAUnitTestCase
     }
 
     /**
-     * @return array
+     * @return Generator
      */
-    public function canSetAuthnRegisterDataProvider(): array
+    public function canSetAuthnRegisterDataProvider(): Generator
     {
-        return [
+        yield 'dont throw exception'  => [false];
-            'dont throw exception'  => [false],
+        yield 'throw exception'  => [true];
-            'throw exception'  => [true],
-        ];
     }
 
     /**
@@ -333,6 +333,19 @@ class d3_account_webauthnTest extends WAUnitTestCase
     {
         $_POST['error'] = 'msg';
 
+        /** @var LoggerInterface|MockObject $loggerMock */
+        $loggerMock = $this->getMockForAbstractClass(LoggerInterface::class, [], '', true, true, true, ['error', 'debug']);
+        $loggerMock->expects($this->once())->method('error')->willReturn(true);
+        $loggerMock->expects($this->once())->method('debug')->willReturn(true);
+        d3GetOxidDIC()->set('d3ox.webauthn.'.LoggerInterface::class, $loggerMock);
+
+        /** @var User|MockObject $userMock */
+        $userMock = $this->getMockBuilder(User::class)
+            ->disableOriginalConstructor()
+            ->onlyMethods(['getId'])
+            ->getMock();
+        $userMock->method('getId')->willReturn('userId');
+
         /** @var UtilsView|MockObject $utilsViewMock */
         $utilsViewMock = $this->getMockBuilder(UtilsView::class)
             ->onlyMethods(['addErrorToDisplay'])
@@ -340,12 +353,6 @@ class d3_account_webauthnTest extends WAUnitTestCase
         $utilsViewMock->expects($this->atLeastOnce())->method('addErrorToDisplay');
         d3GetOxidDIC()->set('d3ox.webauthn.'.UtilsView::class, $utilsViewMock);
 
-        /** @var LoggerInterface|MockObject $loggerMock */
-        $loggerMock = $this->getMockForAbstractClass(LoggerInterface::class, [], '', true, true, true, ['error', 'debug']);
-        $loggerMock->expects($this->never())->method('error')->willReturn(true);
-        $loggerMock->expects($this->never())->method('debug')->willReturn(true);
-        d3GetOxidDIC()->set('d3ox.webauthn.'.LoggerInterface::class, $loggerMock);
-
         /** @var Request|MockObject $requestMock */
         $requestMock = $this->getMockBuilder(Request::class)
             ->onlyMethods(['getRequestEscapedParameter'])
@@ -355,8 +362,11 @@ class d3_account_webauthnTest extends WAUnitTestCase
         )->willReturn('errorMsg');
         d3GetOxidDIC()->set('d3ox.webauthn.'.Request::class, $requestMock);
 
-        /** @var d3_account_webauthn $oControllerMock */
+        /** @var d3_account_webauthn|MockObject $oControllerMock */
-        $oControllerMock = oxNew(d3_account_webauthn::class);
+        $oControllerMock = $this->getMockBuilder(d3_account_webauthn::class)
+            ->onlyMethods(['getUser'])
+            ->getMock();
+        $oControllerMock->method('getUser')->willReturn($userMock);
 
         $this->_oController = $oControllerMock;
 
@@ -406,19 +416,33 @@ class d3_account_webauthnTest extends WAUnitTestCase
      * @test
      * @return void
      * @throws ReflectionException
+     * @dataProvider canSaveAuthnFailedDataProvider
      * @covers \D3\Webauthn\Application\Controller\d3_account_webauthn::saveAuthn
      */
-    public function canSaveAuthnFailed()
+    public function canSaveAuthnFailed($exception)
     {
         $_POST['credential'] = 'msg';
         $_POST['keyname'] = 'key_name';
 
+        /** @var LoggerInterface|MockObject $loggerMock */
+        $loggerMock = $this->getMockForAbstractClass(LoggerInterface::class, [], '', true, true, true, ['error', 'debug']);
+        $loggerMock->expects($this->once())->method('error')->willReturn(true);
+        $loggerMock->expects($this->once())->method('debug')->willReturn(true);
+        d3GetOxidDIC()->set('d3ox.webauthn.'.LoggerInterface::class, $loggerMock);
+
+        /** @var User|MockObject $userMock */
+        $userMock = $this->getMockBuilder(User::class)
+            ->disableOriginalConstructor()
+            ->onlyMethods(['getId'])
+            ->getMock();
+        $userMock->method('getId')->willReturn('userId');
+
         /** @var Webauthn|MockObject $webauthnMock */
         $webauthnMock = $this->getMockBuilder(Webauthn::class)
             ->onlyMethods(['saveAuthn'])
             ->getMock();
         $webauthnMock->expects($this->once())->method('saveAuthn')
-            ->willThrowException(oxNew(WebauthnException::class));
+            ->willThrowException($exception);
         d3GetOxidDIC()->set(Webauthn::class, $webauthnMock);
 
         /** @var UtilsView|MockObject $utilsViewMock */
@@ -428,8 +452,11 @@ class d3_account_webauthnTest extends WAUnitTestCase
         $utilsViewMock->expects($this->atLeastOnce())->method('addErrorToDisplay');
         d3GetOxidDIC()->set('d3ox.webauthn.'.UtilsView::class, $utilsViewMock);
 
-        /** @var d3_account_webauthn $oControllerMock */
+        /** @var d3_account_webauthn|MockObject $oControllerMock */
-        $oControllerMock = oxNew(d3_account_webauthn::class);
+        $oControllerMock = $this->getMockBuilder(d3_account_webauthn::class)
+            ->onlyMethods(['getUser'])
+            ->getMock();
+        $oControllerMock->method('getUser')->willReturn($userMock);
 
         $this->_oController = $oControllerMock;
 
@@ -439,6 +466,15 @@ class d3_account_webauthnTest extends WAUnitTestCase
         );
     }
 
+    /**
+     * @return Generator
+     */
+    public function canSaveAuthnFailedDataProvider(): Generator
+    {
+        yield 'WebauthnException'   => [oxNew(WebauthnException::class)];
+        yield 'AssertionException'  => [oxNew(InvalidArgumentException::class, 'msg', 200)];
+    }
+
     /**
      * @test
      * @throws ReflectionException
@@ -467,14 +503,12 @@ class d3_account_webauthnTest extends WAUnitTestCase
     }
 
     /**
-     * @return array[]
+     * @return Generator
      */
-    public function canDeleteDataProvider(): array
+    public function canDeleteDataProvider(): Generator
     {
-        return [
+        yield 'has delete id' => ['deleteId', $this->once()];
-            'has delete id' => ['deleteId', $this->once()],
+        yield 'has no delete id' => [null, $this->never()];
-            'has no delete id' => [null, $this->never()],
-        ];
     }
 
     /**

src/tests/unit/Application/Model/Credential/PublicKeyCredentialTest.php

@@ -240,12 +240,14 @@ class PublicKeyCredentialTest extends WAUnitTestCase
 
         /** @var PublicKeyCredential|MockObject $sut */
         $sut = $this->getMockBuilder(PublicKeyCredential::class)
-            ->onlyMethods(['exists', 'getIdByCredentialId', 'load', 'save'])
+            ->onlyMethods(['exists', 'getIdByCredentialId', 'load', 'save', 'setCredential', 'setCredentialId'])
             ->getMock();
         $sut->method('exists')->willReturn($credIdExist);
         $sut->expects($this->exactly((int) $doSave))->method('getIdByCredentialId');
         $sut->expects($this->exactly((int) ($doSave && $credIdExist)))->method('load');
         $sut->expects($this->exactly((int) $doSave))->method('save');
+        $sut->expects($this->exactly((int) $doSave))->method('setCredential');
+        $sut->expects($this->exactly((int) $doSave))->method('setCredentialId');
 
         $this->callMethod(
             $sut,