4 changed files with 98 additions and 134 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@ -4,11 +4,7 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

-## [Unreleased](https://git.d3data.de/D3Public/oxtotp/compare/2.0.0.1...rel_2.x)
-
-## [2.0.0.1](https://git.d3data.de/D3Public/oxtotp/compare/2.0.0.0...2.0.0.1) - 2022-11-09
-### Fixed
- Further protection of the login
+## [Unreleased](https://git.d3data.de/D3Public/oxtotp/compare/2.0.0.0...rel_2.x)

 ## [2.0.0.0](https://git.d3data.de/D3Public/oxtotp/compare/1.1.0.0...2.0.0.0) - 2022-09-30
 ### Added
--- a/src/Modules/Application/Component/d3_totp_UserComponent.php
+++ b/src/Modules/Application/Component/d3_totp_UserComponent.php
@ -22,18 +22,18 @@ use OxidEsales\Eshop\Application\Model\User;
 use OxidEsales\Eshop\Core\Exception\DatabaseConnectionException;
 use OxidEsales\Eshop\Core\Registry;
 use OxidEsales\Eshop\Core\Session;
-use OxidEsales\Eshop\Core\Utils;
 use OxidEsales\Eshop\Core\UtilsView;

 class d3_totp_UserComponent extends d3_totp_UserComponent_parent
 {
    /**
-     * @return void
+     * @return string|void
+     * @throws DBALException
     * @throws DatabaseConnectionException
     */
-    public function login()
+    public function login_noredirect()
    {
-        parent::login();
+        parent::login_noredirect();

        $oUser = $this->getUser();

@ -42,23 +42,21 @@ class d3_totp_UserComponent extends d3_totp_UserComponent_parent
            $totp->loadByUserId($oUser->getId());

            if ($totp->isActive()
-                && !$this->d3GetSession()->getVariable(d3totp::TOTP_SESSION_VARNAME)
+                && !Registry::getSession()->getVariable(d3totp::TOTP_SESSION_VARNAME)
            ) {
-                $this->d3GetSession()->setVariable(
+                Registry::getSession()->setVariable(
                    d3totp::TOTP_SESSION_CURRENTCLASS,
                    $this->getParent()->getClassKey() != 'd3totplogin' ? $this->getParent()->getClassKey() : 'start'
                );
-
-                $this->d3GetSession()->setVariable(d3totp::TOTP_SESSION_CURRENTUSER, $oUser->getId());
-                $this->d3GetSession()->setVariable(
+                Registry::getSession()->setVariable(d3totp::TOTP_SESSION_CURRENTUSER, $oUser->getId());
+                Registry::getSession()->setVariable(
                    d3totp::TOTP_SESSION_NAVFORMPARAMS,
                    $this->getParent()->getViewConfig()->getNavFormParams()
                );

                $oUser->logout();

-                $sUrl = Registry::getConfig()->getShopHomeUrl() . 'cl=d3totplogin';
-                $this->d3GetUtils()->redirect($sUrl, false);
+                return "d3totplogin";
            }
        }
    }
@ -66,7 +64,7 @@ class d3_totp_UserComponent extends d3_totp_UserComponent_parent
    /**
     * @return d3totp
     */
-    public function d3GetTotpObject(): d3totp
+    public function d3GetTotpObject()
    {
        return oxNew(d3totp::class);
    }
@ -88,13 +86,7 @@ class d3_totp_UserComponent extends d3_totp_UserComponent_parent

        try {
            if (!$this->isNoTotpOrNoLogin($totp) && $this->hasValidTotp($sTotp, $totp)) {
-                // relogin, don't extract from this try block
-                $this->d3GetSession()->setVariable(d3totp::TOTP_SESSION_VARNAME, $sTotp);
-                $this->d3GetSession()->setVariable('usr', $oUser->getId());
-                $this->setUser(null);
-                $this->setLoginStatus(USER_LOGIN_SUCCESS);
-                $this->_afterLogin($oUser);
-
+                $this->d3TotpRelogin($oUser, $sTotp);
                $this->d3TotpClearSessionVariables();

                return false;
@ -114,14 +106,6 @@ class d3_totp_UserComponent extends d3_totp_UserComponent_parent
        return Registry::getUtilsView();
    }

-    /**
-     * @return Utils
-     */
-    public function d3GetUtils()
-    {
-        return Registry::getUtils();
-    }
-
    public function cancelTotpLogin()
    {
        $this->d3TotpClearSessionVariables();
@ -154,6 +138,19 @@ class d3_totp_UserComponent extends d3_totp_UserComponent_parent
            );
    }

+    /**
+     * @param User $oUser
+     * @param $sTotp
+     */
+    public function d3TotpRelogin(User $oUser, $sTotp)
+    {
+        $this->d3GetSession()->setVariable(d3totp::TOTP_SESSION_VARNAME, $sTotp);
+        $this->d3GetSession()->setVariable('usr', $oUser->getId());
+        $this->setUser(null);
+        $this->setLoginStatus(USER_LOGIN_SUCCESS);
+        $this->_afterLogin($oUser);
+    }
+
    public function d3TotpClearSessionVariables()
    {
        $this->d3GetSession()->deleteVariable(d3totp::TOTP_SESSION_CURRENTCLASS);
--- a/src/metadata.php
+++ b/src/metadata.php
@ -54,7 +54,7 @@ $aModule = [
        'de'        => 'Zwei-Faktor-Authentisierung (TOTP) f&uuml;r OXID eSales Shop',
        'en'        => 'Two-factor authentication (TOTP) for OXID eSales shop',
    ],
-    'version'       => '2.0.0.1',
+    'version'       => '2.0.0.0',
    'author'        => 'D&sup3; Data Development (Inh.: Thomas Dartsch)',
    'email'         => 'support@shopmodule.com',
    'url'           => 'https://www.oxidmodule.com/',
--- a/src/tests/unit/Modules/Application/Component/d3_totp_UserComponentTest.php
+++ b/src/tests/unit/Modules/Application/Component/d3_totp_UserComponentTest.php
@ -22,7 +22,6 @@ use OxidEsales\Eshop\Application\Model\User;
 use OxidEsales\Eshop\Core\Controller\BaseController;
 use OxidEsales\Eshop\Core\Registry;
 use OxidEsales\Eshop\Core\Session;
-use OxidEsales\Eshop\Core\Utils;
 use OxidEsales\Eshop\Core\UtilsView;
 use PHPUnit\Framework\MockObject\MockObject;
 use ReflectionException;
@ -54,23 +53,17 @@ class d3_totp_UserComponentTest extends d3TotpUnitTestCase
    /**
     * @test
     * @throws ReflectionException
-     * @covers \D3\Totp\Modules\Application\Component\d3_totp_UserComponent::login
+     * @covers \D3\Totp\Modules\Application\Component\d3_totp_UserComponent::login_noredirect
     */
-    public function loginFailsIfNoUserLoggedIn()
+    public function login_noredirectFailsIfNoUserLoggedIn()
    {
        $oUser = false;

-        /** @var Utils|MockObject $oUtilsMock */
-        $oUtilsMock = $this->getMockBuilder(Utils::class)
-                           ->onlyMethods(['redirect'])
-                           ->getMock();
-        $oUtilsMock->expects($this->never())->method('redirect')->willReturn(true);
-
-        /** @var Session|MockObject $oSessionMock */
-        $oSessionMock = $this->getMockBuilder(Session::class)
-            ->onlyMethods(['setVariable'])
+        /** @var BaseController|MockObject $oParentMock */
+        $oParentMock = $this->getMockBuilder(BaseController::class)
+            ->addMethods(['isEnabledPrivateSales'])
            ->getMock();
-        $oSessionMock->expects($this->never())->method('setVariable');
+        $oParentMock->method('isEnabledPrivateSales')->willReturn(false);

        /** @var d3totp|MockObject $oTotpMock */
        $oTotpMock = $this->getMockBuilder(d3totp::class)
@ -84,29 +77,26 @@ class d3_totp_UserComponentTest extends d3TotpUnitTestCase
            ->onlyMethods([
                'getUser',
                'd3GetTotpObject',
-                'd3GetSession',
-                'd3GetUtils',
+                'getParent',
            ])
            ->getMock();
        $oControllerMock->method('getUser')->willReturn($oUser);
        $oControllerMock->method('d3GetTotpObject')->willReturn($oTotpMock);
-        $oControllerMock->method('d3GetSession')->willReturn($oSessionMock);
-        $oControllerMock->method('d3GetUtils')->willReturn($oUtilsMock);
+        $oControllerMock->method('getParent')->willReturn($oParentMock);

        $_GET['lgn_usr'] = 'username';

        $this->_oController = $oControllerMock;

-        $this->callMethod($this->_oController, 'login');
+        $this->callMethod($this->_oController, 'login_noredirect');
    }

    /**
     * @test
     * @throws ReflectionException
-     * @covers \D3\Totp\Modules\Application\Component\d3_totp_UserComponent::login
-     * @dataProvider loginFailTotpNotActiveOrAlreadyCheckedDataProvider
+     * @covers \D3\Totp\Modules\Application\Component\d3_totp_UserComponent::login_noredirect
     */
-    public function loginFailTotpNotActiveOrAlreadyChecked($isActive, $checked)
+    public function login_noredirectFailTotpNotActive()
    {
        /** @var User|MockObject $oUserMock */
        $oUserMock = $this->getMockBuilder(User::class)
@ -118,18 +108,11 @@ class d3_totp_UserComponentTest extends d3TotpUnitTestCase
        $oUserMock->expects($this->never())->method('logout')->willReturn(false);
        $oUserMock->method('getId')->willReturn('foo');

-        /** @var Utils|MockObject $oUtilsMock */
-        $oUtilsMock = $this->getMockBuilder(Utils::class)
-                           ->onlyMethods(['redirect'])
-                           ->getMock();
-        $oUtilsMock->expects($this->never())->method('redirect')->willReturn(true);
-
-        /** @var Session|MockObject $oSessionMock */
-        $oSessionMock = $this->getMockBuilder(Session::class)
-            ->onlyMethods(['setVariable', 'getVariable'])
+        /** @var BaseController|MockObject $oParentMock */
+        $oParentMock = $this->getMockBuilder(BaseController::class)
+            ->addMethods(['isEnabledPrivateSales'])
            ->getMock();
-        $oSessionMock->expects($this->never())->method('setVariable');
-        $oSessionMock->method('getVariable')->willReturn($checked);
+        $oParentMock->method('isEnabledPrivateSales')->willReturn(false);

        /** @var d3totp|MockObject $oTotpMock */
        $oTotpMock = $this->getMockBuilder(d3totp::class)
@ -139,7 +122,7 @@ class d3_totp_UserComponentTest extends d3TotpUnitTestCase
            ])
            ->disableOriginalConstructor()
            ->getMock();
-        $oTotpMock->expects($this->once())->method('isActive')->willReturn($isActive);
+        $oTotpMock->expects($this->once())->method('isActive')->willReturn(false);
        $oTotpMock->method('loadByUserId')->willReturn(true);

        /** @var UserComponent|MockObject $oControllerMock */
@ -147,40 +130,26 @@ class d3_totp_UserComponentTest extends d3TotpUnitTestCase
            ->onlyMethods([
                'getUser',
                'd3GetTotpObject',
-                'd3GetSession',
-                'd3GetUtils',
+                'getParent',
            ])
            ->getMock();
        $oControllerMock->method('getUser')->willReturn($oUserMock);
        $oControllerMock->method('d3GetTotpObject')->willReturn($oTotpMock);
-        $oControllerMock->method('d3GetSession')->willReturn($oSessionMock);
-        $oControllerMock->method('d3GetUtils')->willReturn($oUtilsMock);
+        $oControllerMock->method('getParent')->willReturn($oParentMock);

        $_GET['lgn_usr'] = 'username';

        $this->_oController = $oControllerMock;

-        $this->callMethod($this->_oController, 'login');
-    }
-
-    /**
-     * @return array
-     */
-    public function loginFailTotpNotActiveOrAlreadyCheckedDataProvider(): array
-    {
-        return [
-            'TOTP not active, not checked'   => [false, null],
-            'TOTP not active, checked'   => [false, true],
-            'TOTP active, checked'   => [true, true],
-        ];
+        $this->callMethod($this->_oController, 'login_noredirect');
    }

    /**
     * @test
     * @throws ReflectionException
-     * @covers \D3\Totp\Modules\Application\Component\d3_totp_UserComponent::login
+     * @covers \D3\Totp\Modules\Application\Component\d3_totp_UserComponent::login_noredirect
     */
-    public function loginPass()
+    public function login_noredirectPass()
    {
        /** @var User|MockObject $oUserMock */
        $oUserMock = $this->getMockBuilder(User::class)
@ -192,24 +161,11 @@ class d3_totp_UserComponentTest extends d3TotpUnitTestCase
        $oUserMock->expects($this->once())->method('logout')->willReturn(false);
        $oUserMock->method('getId')->willReturn('foo');

-        /** @var Utils|MockObject $oUtilsMock */
-        $oUtilsMock = $this->getMockBuilder(Utils::class)
-            ->onlyMethods(['redirect'])
-            ->getMock();
-        $oUtilsMock->expects($this->once())->method('redirect')->willReturn(true);
-
-        /** @var Session|MockObject $oSessionMock */
-        $oSessionMock = $this->getMockBuilder(Session::class)
-            ->onlyMethods(['setVariable', 'getVariable'])
-            ->getMock();
-        $oSessionMock->expects($this->atLeast(3))->method('setVariable');
-        $oSessionMock->method('getVariable')->willReturn(null);
-
        /** @var BaseController|MockObject $oParentMock */
        $oParentMock = $this->getMockBuilder(BaseController::class)
-            ->onlyMethods(['getClassKey'])
+            ->addMethods(['isEnabledPrivateSales'])
            ->getMock();
-        $oParentMock->method('getClassKey')->willReturn('foo');
+        $oParentMock->method('isEnabledPrivateSales')->willReturn(false);

        /** @var d3totp|MockObject $oTotpMock */
        $oTotpMock = $this->getMockBuilder(d3totp::class)
@ -227,22 +183,21 @@ class d3_totp_UserComponentTest extends d3TotpUnitTestCase
            ->onlyMethods([
                'getUser',
                'd3GetTotpObject',
-                'd3GetSession',
-                'd3GetUtils',
-                'getParent'
+                'getParent',
            ])
            ->getMock();
        $oControllerMock->method('getUser')->willReturn($oUserMock);
        $oControllerMock->method('d3GetTotpObject')->willReturn($oTotpMock);
        $oControllerMock->method('getParent')->willReturn($oParentMock);
-        $oControllerMock->method('d3GetSession')->willReturn($oSessionMock);
-        $oControllerMock->method('d3GetUtils')->willReturn($oUtilsMock);

        $_GET['lgn_usr'] = 'username';

        $this->_oController = $oControllerMock;

-        $this->callMethod($this->_oController, 'login');
+        $this->assertSame(
+            'd3totplogin',
+            $this->callMethod($this->_oController, 'login_noredirect')
+        );
    }

    /**
@ -272,25 +227,19 @@ class d3_totp_UserComponentTest extends d3TotpUnitTestCase
            ->getMock();
        $oTotpMock->method('loadByUserId')->willReturn(true);

-        /** @var Session|MockObject $oSessionMock */
-        $oSessionMock = $this->getMockBuilder(Session::class)
-            ->onlyMethods(['setVariable'])
-            ->getMock();
-        $oSessionMock->expects($this->never())->method('setVariable');
-
        /** @var UserComponent|MockObject $oControllerMock */
        $oControllerMock = $this->getMockBuilder(UserComponent::class)
            ->onlyMethods([
                'isNoTotpOrNoLogin',
                'hasValidTotp',
+                'd3TotpRelogin',
                'd3GetTotpObject',
-                'd3GetSession',
            ])
            ->getMock();
        $oControllerMock->method('isNoTotpOrNoLogin')->willReturn(true);
        $oControllerMock->expects($this->never())->method('hasValidTotp')->willReturn(false);
+        $oControllerMock->expects($this->never())->method('d3TotpRelogin')->willReturn(false);
        $oControllerMock->method('d3GetTotpObject')->willReturn($oTotpMock);
-        $oControllerMock->method('d3GetSession')->willReturn($oSessionMock);

        $this->_oController = $oControllerMock;

@ -314,12 +263,6 @@ class d3_totp_UserComponentTest extends d3TotpUnitTestCase
            ->getMock();
        $oTotpMock->method('loadByUserId')->willReturn(true);

-        /** @var Session|MockObject $oSessionMock */
-        $oSessionMock = $this->getMockBuilder(Session::class)
-            ->onlyMethods(['setVariable'])
-            ->getMock();
-        $oSessionMock->expects($this->never())->method('setVariable');
-
        /** @var d3totp_wrongOtpException|MockObject $oUtilsViewMock */
        $oTotpExceptionMock = $this->getMockBuilder(d3totp_wrongOtpException::class)
            ->disableOriginalConstructor()
@ -336,16 +279,16 @@ class d3_totp_UserComponentTest extends d3TotpUnitTestCase
            ->onlyMethods([
                'isNoTotpOrNoLogin',
                'hasValidTotp',
+                'd3TotpRelogin',
                'd3GetUtilsView',
                'd3GetTotpObject',
-                'd3GetSession',
            ])
            ->getMock();
        $oControllerMock->method('isNoTotpOrNoLogin')->willReturn(false);
        $oControllerMock->expects($this->once())->method('hasValidTotp')->willThrowException($oTotpExceptionMock);
+        $oControllerMock->expects($this->never())->method('d3TotpRelogin')->willReturn(false);
        $oControllerMock->method('d3GetUtilsView')->willReturn($oUtilsViewMock);
        $oControllerMock->method('d3GetTotpObject')->willReturn($oTotpMock);
-        $oControllerMock->method('d3GetSession')->willReturn($oSessionMock);

        $this->_oController = $oControllerMock;

@ -369,12 +312,6 @@ class d3_totp_UserComponentTest extends d3TotpUnitTestCase
            ->getMock();
        $oTotpMock->method('loadByUserId')->willReturn(true);

-        /** @var Session|MockObject $oSessionMock */
-        $oSessionMock = $this->getMockBuilder(Session::class)
-            ->onlyMethods(['setVariable'])
-            ->getMock();
-        $oSessionMock->expects($this->atLeast(2))->method('setVariable');
-
        /** @var UtilsView|MockObject $oUtilsViewMock */
        $oUtilsViewMock = $this->getMockBuilder(UtilsView::class)
            ->onlyMethods(['addErrorToDisplay'])
@ -386,20 +323,16 @@ class d3_totp_UserComponentTest extends d3TotpUnitTestCase
            ->onlyMethods([
                'isNoTotpOrNoLogin',
                'hasValidTotp',
+                'd3TotpRelogin',
                'd3GetUtilsView',
                'd3GetTotpObject',
-                'd3GetSession',
-                'setLoginStatus'
            ])
            ->getMock();
        $oControllerMock->method('isNoTotpOrNoLogin')->willReturn(false);
        $oControllerMock->expects($this->once())->method('hasValidTotp')->willReturn(true);
+        $oControllerMock->expects($this->once())->method('d3TotpRelogin')->willReturn(true);
        $oControllerMock->method('d3GetUtilsView')->willReturn($oUtilsViewMock);
        $oControllerMock->method('d3GetTotpObject')->willReturn($oTotpMock);
-        $oControllerMock->method('d3GetSession')->willReturn($oSessionMock);
-        $oControllerMock->expects($this->once())->method('setLoginStatus')->with(
-            $this->identicalTo(USER_LOGIN_SUCCESS)
-        );

        $this->_oController = $oControllerMock;

@ -586,6 +519,44 @@ class d3_totp_UserComponentTest extends d3TotpUnitTestCase
        );
    }

+    /**
+     * @test
+     * @throws ReflectionException
+     * @covers \D3\Totp\Modules\Application\Component\d3_totp_UserComponent::d3TotpRelogin
+     */
+    public function d3TotpReloginPass()
+    {
+        /** @var Session|MockObject $oSessionMock */
+        $oSessionMock = $this->getMockBuilder(Session::class)
+            ->onlyMethods(['setVariable'])
+            ->getMock();
+        $oSessionMock->expects($this->atLeast(2))->method('setVariable')->willReturn(false);
+
+        /** @var User|MockObject $oUserMock */
+        $oUserMock = $this->getMockBuilder(User::class)
+            ->onlyMethods(['getId'])
+            ->getMock();
+        $oUserMock->method('getId')->willReturn('foo');
+
+        /** @var UserComponent|MockObject $oControllerMock */
+        $oControllerMock = $this->getMockBuilder(UserComponent::class)
+            ->onlyMethods([
+                'd3GetSession',
+                'setUser',
+                'setLoginStatus',
+                '_afterLogin',
+            ])
+            ->getMock();
+        $oControllerMock->method('d3GetSession')->willReturn($oSessionMock);
+        $oControllerMock->expects($this->once())->method('setUser')->willReturn(false);
+        $oControllerMock->expects($this->once())->method('setLoginStatus')->willReturn(false);
+        $oControllerMock->expects($this->once())->method('_afterLogin')->willReturn(false);
+
+        $this->_oController = $oControllerMock;
+
+        $this->callMethod($this->_oController, 'd3TotpRelogin', [$oUserMock, '123456']);
+    }
+
    /**
     * @test
     * @throws ReflectionException