From c63724c0642f198e4c30aad18cdc9d7c11a183ae Mon Sep 17 00:00:00 2001 From: Daniel Seifert Date: Thu, 16 Feb 2023 23:52:20 +0100 Subject: [PATCH] improve assertions --- .../Controller/Admin/d3user_webauthn.php | 7 +++- .../Controller/Admin/d3webauthnadminlogin.php | 5 +-- .../Controller/d3_account_webauthn.php | 6 +++ .../Controller/d3webauthnlogin.php | 8 ++-- src/Application/Model/Webauthn.php | 10 ++--- src/Application/Model/WebauthnErrors.php | 1 + src/Application/Model/WebauthnLogin.php | 2 + .../Component/d3_webauthn_UserComponent.php | 42 +++++++++---------- src/Setup/Actions.php | 9 ++++ .../Controller/Admin/d3user_webauthnTest.php | 3 +- .../Admin/d3webauthnadminloginTest.php | 10 +++-- .../Controller/d3_account_webauthnTest.php | 15 ++++++- .../Controller/d3webauthnloginTest.php | 19 ++++++++- .../unit/Application/Model/WebauthnTest.php | 2 +- .../data/Version20230209212939Test.php | 2 +- 15 files changed, 95 insertions(+), 46 deletions(-) diff --git a/src/Application/Controller/Admin/d3user_webauthn.php b/src/Application/Controller/Admin/d3user_webauthn.php index 189647e..f2ac7bf 100755 --- a/src/Application/Controller/Admin/d3user_webauthn.php +++ b/src/Application/Controller/Admin/d3user_webauthn.php @@ -17,6 +17,7 @@ namespace D3\Webauthn\Application\Controller\Admin; use Assert\Assert; use Assert\AssertionFailedException; +use Assert\InvalidArgumentException; use D3\TestingTools\Production\IsMockable; use D3\Webauthn\Application\Model\Credential\PublicKeyCredential; use D3\Webauthn\Application\Model\Credential\PublicKeyCredentialList; @@ -84,7 +85,7 @@ class d3user_webauthn extends AdminDetailsController try { $this->setPageType('requestnew'); $this->setAuthnRegister(); - } catch (Exception|ContainerExceptionInterface|NotFoundExceptionInterface|DoctrineDriverException $e) { + } catch (AssertionFailedException|ContainerExceptionInterface|NotFoundExceptionInterface|DoctrineDriverException $e) { d3GetOxidDIC()->get('d3ox.webauthn.'.UtilsView::class)->addErrorToDisplay($e->getMessage()); d3GetOxidDIC()->get('d3ox.webauthn.'.LoggerInterface::class)->error($e->getMessage(), ['UserId' => $this->getEditObjectId()]); d3GetOxidDIC()->get('d3ox.webauthn.'.LoggerInterface::class)->debug($e->getTraceAsString()); @@ -140,11 +141,14 @@ class d3user_webauthn extends AdminDetailsController /** * @throws DoctrineDriverException * @throws DoctrineException + * @throws InvalidArgumentException */ public function setAuthnRegister(): void { + /** @var Webauthn $authn */ $authn = d3GetOxidDIC()->get(Webauthn::class); + /** @var User $user */ $user = d3GetOxidDIC()->get('d3ox.webauthn.'.User::class); $user->load($this->getEditObjectId()); $publicKeyCredentialCreationOptions = $authn->getCreationOptions($user); @@ -169,6 +173,7 @@ class d3user_webauthn extends AdminDetailsController */ public function getCredentialList($userId): array { + /** @var User $oUser */ $oUser = d3GetOxidDIC()->get('d3ox.webauthn.'.User::class); $oUser->load($userId); diff --git a/src/Application/Controller/Admin/d3webauthnadminlogin.php b/src/Application/Controller/Admin/d3webauthnadminlogin.php index f95133b..6e85e1a 100755 --- a/src/Application/Controller/Admin/d3webauthnadminlogin.php +++ b/src/Application/Controller/Admin/d3webauthnadminlogin.php @@ -29,7 +29,6 @@ use Doctrine\DBAL\Driver\Exception as DoctrineDriverException; use Doctrine\DBAL\Exception as DoctrineException; use OxidEsales\Eshop\Application\Controller\Admin\AdminController; use OxidEsales\Eshop\Application\Controller\FrontendController; -use OxidEsales\Eshop\Core\Registry; use OxidEsales\Eshop\Core\Request; use OxidEsales\Eshop\Core\Routing\ControllerClassNameResolver; use OxidEsales\Eshop\Core\Session; @@ -63,11 +62,11 @@ class d3webauthnadminlogin extends AdminController public function render(): string { if (d3GetOxidDIC()->get('d3ox.webauthn.'.Session::class) - ->hasVariable(WebauthnConf::WEBAUTHN_ADMIN_SESSION_AUTH) + ->hasVariable(WebauthnConf::WEBAUTHN_ADMIN_SESSION_AUTH) ) { d3GetOxidDIC()->get('d3ox.webauthn.'.Utils::class)->redirect('index.php?cl=admin_start'); } elseif (!d3GetOxidDIC()->get('d3ox.webauthn.'.Session::class) - ->hasVariable(WebauthnConf::WEBAUTHN_ADMIN_SESSION_CURRENTUSER) + ->hasVariable(WebauthnConf::WEBAUTHN_ADMIN_SESSION_CURRENTUSER) ) { d3GetOxidDIC()->get('d3ox.webauthn.'.Utils::class)->redirect('index.php?cl=login'); } diff --git a/src/Application/Controller/d3_account_webauthn.php b/src/Application/Controller/d3_account_webauthn.php index cd5a13c..7ceea22 100755 --- a/src/Application/Controller/d3_account_webauthn.php +++ b/src/Application/Controller/d3_account_webauthn.php @@ -17,6 +17,7 @@ namespace D3\Webauthn\Application\Controller; use Assert\Assert; use Assert\AssertionFailedException; +use Assert\InvalidArgumentException; use D3\TestingTools\Production\IsMockable; use D3\Webauthn\Application\Controller\Traits\accountTrait; use D3\Webauthn\Application\Model\Credential\PublicKeyCredential; @@ -87,6 +88,10 @@ class d3_account_webauthn extends AccountController 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) { + 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($e); } } @@ -104,6 +109,7 @@ class d3_account_webauthn extends AccountController * @throws DoctrineException * @throws ContainerExceptionInterface * @throws NotFoundExceptionInterface + * @throws InvalidArgumentException * @return void */ public function setAuthnRegister(): void diff --git a/src/Application/Controller/d3webauthnlogin.php b/src/Application/Controller/d3webauthnlogin.php index 0e96cbb..b1f1732 100755 --- a/src/Application/Controller/d3webauthnlogin.php +++ b/src/Application/Controller/d3webauthnlogin.php @@ -63,9 +63,9 @@ class d3webauthnlogin extends FrontendController public function render(): string { if (d3GetOxidDIC()->get('d3ox.webauthn.'.Session::class) - ->hasVariable(WebauthnConf::WEBAUTHN_SESSION_AUTH) || + ->hasVariable(WebauthnConf::WEBAUTHN_SESSION_AUTH) || !d3GetOxidDIC()->get('d3ox.webauthn.'.Session::class) - ->hasVariable(WebauthnConf::WEBAUTHN_SESSION_CURRENTUSER) + ->hasVariable(WebauthnConf::WEBAUTHN_SESSION_CURRENTUSER) ) { d3GetOxidDIC()->get('d3ox.webauthn.'.Utils::class)->redirect('index.php?cl=start'); } @@ -73,7 +73,7 @@ class d3webauthnlogin extends FrontendController $this->generateCredentialRequest(); $this->addTplParam('navFormParams', d3GetOxidDIC()->get('d3ox.webauthn.'.Session::class) - ->getVariable(WebauthnConf::WEBAUTHN_SESSION_NAVFORMPARAMS)); + ->getVariable(WebauthnConf::WEBAUTHN_SESSION_NAVFORMPARAMS)); return $this->d3CallMockableFunction([FrontendController::class, 'render']); } @@ -109,7 +109,7 @@ class d3webauthnlogin extends FrontendController ->setVariable(WebauthnConf::GLOBAL_SWITCH, true); d3GetOxidDIC()->get('d3ox.webauthn.'.LoggerInterface::class)->error($e->getMessage(), ['UserId' => $userId]); d3GetOxidDIC()->get('d3ox.webauthn.'.LoggerInterface::class)->debug($e->getTraceAsString()); - Registry::getUtilsView()->addErrorToDisplay($e); + Registry::getUtilsView()->addErrorToDisplay($e->getMessage()); d3GetOxidDIC()->get('d3ox.webauthn.'.Utils::class)->redirect('index.php?cl=start'); } } diff --git a/src/Application/Model/Webauthn.php b/src/Application/Model/Webauthn.php index e2e9b51..07bfd29 100644 --- a/src/Application/Model/Webauthn.php +++ b/src/Application/Model/Webauthn.php @@ -26,7 +26,6 @@ use D3\Webauthn\Application\Model\Exceptions\WebauthnGetException; use D3\Webauthn\Modules\Application\Model\d3_User_Webauthn; use Doctrine\DBAL\Driver\Exception as DoctrineDriverException; use Doctrine\DBAL\Exception as DoctrineException; -use Exception; use Nyholm\Psr7\Factory\Psr17Factory; use Nyholm\Psr7Server\ServerRequestCreator; use OxidEsales\Eshop\Application\Model\User; @@ -78,6 +77,7 @@ class Webauthn * @throws DoctrineDriverException * @throws DoctrineException * @throws NotFoundExceptionInterface + * @throws InvalidArgumentException */ public function getCreationOptions(User $user): string { @@ -96,9 +96,7 @@ class Webauthn $json = $this->jsonEncode($publicKeyCredentialCreationOptions); - if ($json === false) { - throw oxNew(Exception::class, "can't encode creation options"); - } + Assert::that($json)->isJsonString("can't encode request options"); return $json; } @@ -149,7 +147,7 @@ class Webauthn $existingCredentials = $this->getExistingCredentials($userEntity); d3GetOxidDIC()->get('d3ox.webauthn.'.LoggerInterface::class)->debug( - 'found user credentials: '.count($existingCredentials).' for ID '.$userId) + 'found user credentials: '.count($existingCredentials).' for ID '.$userId ); // We generate the set of options. @@ -167,7 +165,7 @@ class Webauthn 'request options: '.$json ); - Assert::that($json)->minLength(1, "can't encode request options"); + Assert::that($json)->isJsonString("can't encode request options"); return $json; } diff --git a/src/Application/Model/WebauthnErrors.php b/src/Application/Model/WebauthnErrors.php index 0df2980..9fadebc 100644 --- a/src/Application/Model/WebauthnErrors.php +++ b/src/Application/Model/WebauthnErrors.php @@ -17,6 +17,7 @@ namespace D3\Webauthn\Application\Model; use D3\TestingTools\Production\IsMockable; use OxidEsales\Eshop\Core\Language; +use Psr\Log\LoggerInterface; class WebauthnErrors { diff --git a/src/Application/Model/WebauthnLogin.php b/src/Application/Model/WebauthnLogin.php index 3b0c642..4143463 100644 --- a/src/Application/Model/WebauthnLogin.php +++ b/src/Application/Model/WebauthnLogin.php @@ -112,6 +112,7 @@ class WebauthnLogin $myUtilsView = d3GetOxidDIC()->get('d3ox.webauthn.'.UtilsView::class); /** @var d3_User_Webauthn $user */ $user = d3GetOxidDIC()->get('d3ox.webauthn.'.User::class); + $userId = null; try { $userId = $this->getUserId(); @@ -160,6 +161,7 @@ class WebauthnLogin $myUtilsView = d3GetOxidDIC()->get('d3ox.webauthn.'.UtilsView::class); /** @var d3_User_Webauthn $user */ $user = d3GetOxidDIC()->get('d3ox.webauthn.'.User::class); + $userId = null; try { $userId = $this->getUserId(); diff --git a/src/Modules/Application/Component/d3_webauthn_UserComponent.php b/src/Modules/Application/Component/d3_webauthn_UserComponent.php index a09ee4d..a87a710 100755 --- a/src/Modules/Application/Component/d3_webauthn_UserComponent.php +++ b/src/Modules/Application/Component/d3_webauthn_UserComponent.php @@ -66,29 +66,27 @@ class d3_webauthn_UserComponent extends d3_webauthn_UserComponent_parent $user = d3GetOxidDIC()->get('d3ox.webauthn.'.User::class); $userId = $user->d3GetLoginUserId($lgn_user); - if ($this->d3CanUseWebauthn($lgn_user, $userId)) { - if ($this->d3HasWebauthnButNotLoggedin($userId)) { - $session = d3GetOxidDIC()->get('d3ox.webauthn.'.Session::class); - $session->setVariable( - WebauthnConf::WEBAUTHN_SESSION_CURRENTCLASS, - $this->getClassKey() != 'd3webauthnlogin' ? $this->getClassKey() : 'start' - ); - $session->setVariable( - WebauthnConf::WEBAUTHN_SESSION_CURRENTUSER, - $userId - ); - $session->setVariable( - WebauthnConf::WEBAUTHN_SESSION_NAVPARAMS, - $this->getParent()->getNavigationParams() - ); - $session->setVariable( - WebauthnConf::WEBAUTHN_SESSION_NAVFORMPARAMS, - $this->getParent()->getViewConfig()->getNavFormParams() - ); + if ($this->d3CanUseWebauthn($lgn_user, $userId) && $this->d3HasWebauthnButNotLoggedin($userId)) { + $session = d3GetOxidDIC()->get('d3ox.webauthn.'.Session::class); + $session->setVariable( + WebauthnConf::WEBAUTHN_SESSION_CURRENTCLASS, + $this->getClassKey() != 'd3webauthnlogin' ? $this->getClassKey() : 'start' + ); + $session->setVariable( + WebauthnConf::WEBAUTHN_SESSION_CURRENTUSER, + $userId + ); + $session->setVariable( + WebauthnConf::WEBAUTHN_SESSION_NAVPARAMS, + $this->getParent()->getNavigationParams() + ); + $session->setVariable( + WebauthnConf::WEBAUTHN_SESSION_NAVFORMPARAMS, + $this->getParent()->getViewConfig()->getNavFormParams() + ); - $sUrl = d3GetOxidDIC()->get('d3ox.webauthn.'.Config::class)->getShopHomeUrl() . 'cl=d3webauthnlogin'; - d3GetOxidDIC()->get('d3ox.webauthn.'.Utils::class)->redirect($sUrl); - } + $sUrl = d3GetOxidDIC()->get('d3ox.webauthn.'.Config::class)->getShopHomeUrl() . 'cl=d3webauthnlogin'; + d3GetOxidDIC()->get('d3ox.webauthn.'.Utils::class)->redirect($sUrl); } } diff --git a/src/Setup/Actions.php b/src/Setup/Actions.php index fe0abdc..5c320fe 100644 --- a/src/Setup/Actions.php +++ b/src/Setup/Actions.php @@ -42,6 +42,9 @@ class Actions public $seo_en = 'en/key-authentication'; public $stdClassName = 'd3_account_webauthn'; + /** + * @throws Exception + */ public function runModuleMigrations() { /** @var MigrationsBuilder $migrationsBuilder */ @@ -52,6 +55,7 @@ class Actions /** * Regenerate views for changed tables + * @throws Exception */ public function regenerateViews() { @@ -61,6 +65,7 @@ class Actions /** * clear cache + * @throws Exception */ public function clearCache() { @@ -126,6 +131,7 @@ class Actions /** * @return void + * @throws Exception */ public function seoUrl() { @@ -142,9 +148,11 @@ class Actions /** * @return bool + * @throws Exception */ public function hasSeoUrl(): bool { + /** @var SeoEncoder $seoEncoder */ $seoEncoder = d3GetOxidDIC()->get('d3ox.webauthn.'.SeoEncoder::class); $seoUrl = $seoEncoder->getStaticUrl( d3GetOxidDIC()->get('d3ox.webauthn.'.FrontendController::class)->getViewConfig()->getSelfLink() . @@ -156,6 +164,7 @@ class Actions /** * @return void + * @throws Exception */ public function createSeoUrl() { diff --git a/src/tests/unit/Application/Controller/Admin/d3user_webauthnTest.php b/src/tests/unit/Application/Controller/Admin/d3user_webauthnTest.php index e61bcf5..dbbaa1c 100644 --- a/src/tests/unit/Application/Controller/Admin/d3user_webauthnTest.php +++ b/src/tests/unit/Application/Controller/Admin/d3user_webauthnTest.php @@ -13,6 +13,7 @@ namespace D3\Webauthn\tests\unit\Application\Controller\Admin; +use Assert\InvalidArgumentException; use D3\TestingTools\Development\CanAccessRestricted; use D3\TestingTools\Production\IsMockable; use D3\Webauthn\Application\Controller\Admin\d3user_webauthn; @@ -172,7 +173,7 @@ class d3user_webauthnTest extends WAUnitTestCase ]) ->getMock(); $sutMock->expects($this->atLeastOnce())->method('setPageType'); - $sutMock->expects($this->atLeastOnce())->method('setAuthnRegister')->willThrowException(oxNew(WebauthnException::class)); + $sutMock->expects($this->atLeastOnce())->method('setAuthnRegister')->willThrowException(new InvalidArgumentException('msg', 20)); $this->callMethod( $sutMock, diff --git a/src/tests/unit/Application/Controller/Admin/d3webauthnadminloginTest.php b/src/tests/unit/Application/Controller/Admin/d3webauthnadminloginTest.php index 052c15c..e773054 100644 --- a/src/tests/unit/Application/Controller/Admin/d3webauthnadminloginTest.php +++ b/src/tests/unit/Application/Controller/Admin/d3webauthnadminloginTest.php @@ -144,11 +144,15 @@ class d3webauthnadminloginTest extends d3webauthnloginTest * @test * @return void * @throws ReflectionException + * @dataProvider \D3\Webauthn\tests\unit\Application\Controller\d3webauthnloginTest::generateCredentialRequestFailedDataProvider() * @covers \D3\Webauthn\Application\Controller\Admin\d3webauthnadminlogin::generateCredentialRequest */ - public function generateCredentialRequestFailed($redirectClass = 'login', $userVarName = WebauthnConf::WEBAUTHN_ADMIN_SESSION_CURRENTUSER) - { - parent::generateCredentialRequestFailed($redirectClass, $userVarName); + public function generateCredentialRequestFailed( + $exception, + $redirectClass = 'login', + $userVarName = WebauthnConf::WEBAUTHN_ADMIN_SESSION_CURRENTUSER + ) { + parent::generateCredentialRequestFailed($exception, $redirectClass, $userVarName); } /** diff --git a/src/tests/unit/Application/Controller/d3_account_webauthnTest.php b/src/tests/unit/Application/Controller/d3_account_webauthnTest.php index 35b4c08..c5ed490 100644 --- a/src/tests/unit/Application/Controller/d3_account_webauthnTest.php +++ b/src/tests/unit/Application/Controller/d3_account_webauthnTest.php @@ -210,9 +210,10 @@ class d3_account_webauthnTest extends WAUnitTestCase * @test * @return void * @throws ReflectionException + * @dataProvider canRequestNewCredentialCantGetCreationOptionsDataProvider * @covers \D3\Webauthn\Application\Controller\d3_account_webauthn::requestNewCredential() */ - public function canRequestNewCredentialCantGetCreationOptions() + public function canRequestNewCredentialCantGetCreationOptions($exception) { $oUser = oxNew(User::class); $oUser->setId('foo'); @@ -233,7 +234,7 @@ class d3_account_webauthnTest extends WAUnitTestCase ->onlyMethods(['setAuthnRegister', 'setPageType', 'getUser']) ->getMock(); $oControllerMock->expects($this->atLeastOnce())->method('setAuthnRegister') - ->willThrowException(oxNew(WebauthnException::class)); + ->willThrowException($exception); $oControllerMock->expects($this->never())->method('setPageType'); $oControllerMock->method('getUser')->willReturn($oUser); @@ -245,6 +246,16 @@ class d3_account_webauthnTest extends WAUnitTestCase ); } + /** + * @return Generator + */ + public function canRequestNewCredentialCantGetCreationOptionsDataProvider(): Generator + { + yield 'WebauthnException' => [oxNew(WebauthnException::class)]; + yield 'InvalidArgumentException' => [oxNew(InvalidArgumentException::class, 'msg', 20)]; + } + + /** * @test * @param $throwExc diff --git a/src/tests/unit/Application/Controller/d3webauthnloginTest.php b/src/tests/unit/Application/Controller/d3webauthnloginTest.php index 611a1f1..c6a1f94 100644 --- a/src/tests/unit/Application/Controller/d3webauthnloginTest.php +++ b/src/tests/unit/Application/Controller/d3webauthnloginTest.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\d3webauthnlogin; use D3\Webauthn\Application\Model\Exceptions\WebauthnException; @@ -181,9 +182,14 @@ class d3webauthnloginTest extends WAUnitTestCase * @test * @return void * @throws ReflectionException + * @dataProvider generateCredentialRequestFailedDataProvider * @covers \D3\Webauthn\Application\Controller\d3webauthnlogin::generateCredentialRequest */ - public function generateCredentialRequestFailed($redirectClass = 'start', $userVarName = WebauthnConf::WEBAUTHN_SESSION_CURRENTUSER) + public function generateCredentialRequestFailed( + $exception, + $redirectClass = 'start', + $userVarName = WebauthnConf::WEBAUTHN_SESSION_CURRENTUSER + ) { $currUserFixture = 'currentUserFixture'; @@ -209,7 +215,7 @@ class d3webauthnloginTest extends WAUnitTestCase ->onlyMethods(['getRequestOptions']) ->getMock(); $webAuthnMock->expects($this->once())->method('getRequestOptions')->with($currUserFixture) - ->willThrowException(oxNew(WebauthnException::class, 'foobar0')); + ->willThrowException($exception); d3GetOxidDIC()->set(Webauthn::class, $webAuthnMock); /** @var Utils|MockObject $utilsMock */ @@ -233,6 +239,15 @@ class d3webauthnloginTest extends WAUnitTestCase ); } + /** + * @return Generator + */ + public function generateCredentialRequestFailedDataProvider(): Generator + { + yield 'WebauthnException' => [oxNew(WebauthnException::class, 'foobar0')]; + yield 'InvalidArgumentException' => [oxNew(InvalidArgumentException::class, 'foobar0', 20)]; + } + /** * @test * @return void diff --git a/src/tests/unit/Application/Model/WebauthnTest.php b/src/tests/unit/Application/Model/WebauthnTest.php index f1db94f..7e970eb 100644 --- a/src/tests/unit/Application/Model/WebauthnTest.php +++ b/src/tests/unit/Application/Model/WebauthnTest.php @@ -182,7 +182,7 @@ class WebauthnTest extends WAUnitTestCase */ public function canGetOptionsDataProvider(): Generator { - yield 'json encoded' => ['jsonstring']; + yield 'json encoded' => [json_encode(['jsonstring'])]; yield 'json failed' => [false]; } diff --git a/src/tests/unit/migration/data/Version20230209212939Test.php b/src/tests/unit/migration/data/Version20230209212939Test.php index 06d319a..1492d23 100644 --- a/src/tests/unit/migration/data/Version20230209212939Test.php +++ b/src/tests/unit/migration/data/Version20230209212939Test.php @@ -41,7 +41,7 @@ class Version20230209212939Test extends WAUnitTestCase public function setUp(): void { - parent::setUp(); // TODO: Change the autogenerated stub + parent::setUp(); /** @var AbstractPlatform|MockObject $databasePlatformMock */ $databasePlatformMock = $this->getMockBuilder(MySQL57Platform::class)