From b28c9c8757107269411bc385ff4d4e178fde2fdb Mon Sep 17 00:00:00 2001 From: Daniel Seifert Date: Sun, 18 Aug 2019 13:24:49 +0200 Subject: [PATCH] remove password check in admin, prevent resave, show delete message --- .../Controller/Admin/d3user_totp.php | 18 ++--- src/Application/Model/d3totp.php | 12 ++++ .../views/admin/de/d3totp_lang.php | 5 +- .../views/admin/en/d3totp_lang.php | 5 +- .../views/admin/tpl/d3user_totp.tpl | 10 --- .../Controller/Admin/d3user_totpTest.php | 70 ------------------- 6 files changed, 23 insertions(+), 97 deletions(-) diff --git a/src/Application/Controller/Admin/d3user_totp.php b/src/Application/Controller/Admin/d3user_totp.php index 37bd028..52b6c75 100644 --- a/src/Application/Controller/Admin/d3user_totp.php +++ b/src/Application/Controller/Admin/d3user_totp.php @@ -24,6 +24,7 @@ use OxidEsales\Eshop\Application\Model\User; use OxidEsales\Eshop\Core\Exception\DatabaseConnectionException; use OxidEsales\Eshop\Core\Exception\StandardException; use OxidEsales\Eshop\Core\Registry; +use OxidEsales\Eshop\Core\UtilsView; class d3user_totp extends AdminDetailsController { @@ -94,19 +95,13 @@ class d3user_totp extends AdminDetailsController $aParams = Registry::getRequest()->getRequestEscapedParameter("editval"); try { - $pwd = Registry::getRequest()->getRequestEscapedParameter("pwd"); - - /** @var d3_totp_user $oUser */ - $oUser = $this->getUserObject(); - $oUser->load($this->getEditObjectId()); - - if (false == $oUser->isSamePassword($pwd)) { - $oException = oxNew(StandardException::class, 'D3_TOTP_ERROR_PWDONTPASS'); - throw $oException; - } - /** @var d3totp $oTotp */ $oTotp = $this->getTotpObject(); + if ($oTotp->checkIfAlreadyExist($this->getEditObjectId())) { + $oException = oxNew(StandardException::class, 'D3_TOTP_ALREADY_EXIST'); + throw $oException; + }; + $oTotpBackupCodes = $this->getBackupcodeListObject(); if ($aParams['d3totp__oxid']) { $oTotp->load($aParams['d3totp__oxid']); @@ -140,6 +135,7 @@ class d3user_totp extends AdminDetailsController if ($aParams['d3totp__oxid']) { $oTotp->load($aParams['d3totp__oxid']); $oTotp->delete(); + Registry::get(UtilsView::class)->addErrorToDisplay('D3_TOTP_REGISTERDELETED'); } } diff --git a/src/Application/Model/d3totp.php b/src/Application/Model/d3totp.php index 26d5f2d..4b35a4c 100644 --- a/src/Application/Model/d3totp.php +++ b/src/Application/Model/d3totp.php @@ -65,6 +65,18 @@ class d3totp extends BaseModel } } + /** + * @param $userId + * @return bool + * @throws DatabaseConnectionException + */ + public function checkIfAlreadyExist($userId) + { + $oDb = $this->d3GetDb(); + $query = "SELECT 1 FROM ".$this->getViewName().' WHERE oxuserid = '.$oDb->quote($userId).' LIMIT 1'; + return (bool) $oDb->getOne($query); + } + /** * @return DatabaseInterface * @throws DatabaseConnectionException diff --git a/src/Application/views/admin/de/d3totp_lang.php b/src/Application/views/admin/de/d3totp_lang.php index e73777d..b905255 100644 --- a/src/Application/views/admin/de/d3totp_lang.php +++ b/src/Application/views/admin/de/d3totp_lang.php @@ -31,14 +31,13 @@ $aLang = [ 'D3_TOTP_QRCODE_HELP' => 'Scannen Sie diesen QR-Code mit Ihrer Authentisierungs-App, um dieses Benutzerkonto dort zu hinterlegen.', 'D3_TOTP_SECRET' => 'QR-Code kann nicht gescannt werden?', 'D3_TOTP_SECRET_HELP' => 'Setzen Sie keine App ein, die den QR-Code scannen kann, können Sie diese Zeichenkette auch in Ihr Authentisierungstool kopieren. Stellen Sie bitte die Passwortlänge auf 6 Zeichen und das Zeitinterval auf 30 Sekunden ein.', - 'D3_TOTP_CURRPWD' => 'Anmeldepasswort des Benutzerkontos', - 'D3_TOTP_CURRPWD_HELP' => 'Dies stellt sicher, dass nur Berechtigte Änderungen an diesen Einstellungen vornehmen dürfen.', 'D3_TOTP_CURROTP' => 'Bestätigung mit Einmalpasswort', 'D3_TOTP_CURROTP_HELP' => 'Haben Sie dieses Kundenkonto in Ihrer Authentisierungs-App registriert, generieren Sie damit ein Einmalpasswort, tragen Sie es hier ein und senden das Formular direkt darauf hin ab.', 'D3_TOTP_REGISTEREXIST' => 'vorhandene Registrierung', 'D3_TOTP_REGISTERDELETE' => 'Registrierung löschen', 'D3_TOTP_REGISTERDELETE_DESC' => 'Um die Registrierung zu ändern, löschen Sie diese bitte vorerst. Sie können sofort im Anschluss eine neue Registrierung anlegen.
Wenn Sie die Registrierung löschen, ist das Konto nicht mehr durch die Zwei-Faktor-Authentisierung geschützt.', + 'D3_TOTP_REGISTERDELETED' => 'Die Registrierung wurde gelöscht.', 'D3_TOTP_BACKUPCODES' => 'Backupcodes', 'D3_TOTP_BACKUPCODES_DESC' => 'Mit diesen Backupcodes können Sie sich anmelden, wenn die Generierung des Einmalpasswortes nicht möglich ist (z.B. Gerät verloren oder neu installiert). Sie können dann die Einstellungen zur Verwendung der 2-Faktor-Authentisierung ändern oder einen neuen Zugang erstellen. Speichern Sie sich diese Codes bitte in diesem Moment sicher ab. Nach Verlassen dieser Seite können diese Codes nicht erneut angezeigt werden.', @@ -48,5 +47,5 @@ $aLang = [ 'D3_TOTP_SAVE' => 'Speichern', 'D3_TOTP_ERROR_UNVALID' => 'Das Einmalpasswort ist ungültig.', - 'D3_TOTP_ERROR_PWDONTPASS' => 'Das Passwort passt nicht zum gewählten Benutzerkonto.', + 'D3_TOTP_ALREADY_EXIST' => 'Die Registrierung wurde schon gespeichert.', ]; diff --git a/src/Application/views/admin/en/d3totp_lang.php b/src/Application/views/admin/en/d3totp_lang.php index 48a77e9..77c90fa 100644 --- a/src/Application/views/admin/en/d3totp_lang.php +++ b/src/Application/views/admin/en/d3totp_lang.php @@ -31,14 +31,13 @@ $aLang = [ 'D3_TOTP_QRCODE_HELP' => 'Scan this QR code with your authentication app to deposit this user account.', 'D3_TOTP_SECRET' => 'Can not scan QR code?', 'D3_TOTP_SECRET_HELP' => 'If you do not use an app that can scan the QR code, you can also copy this string into your authentication tool. Please also set the password length to 6 characters and the time interval to 30 seconds.', - 'D3_TOTP_CURRPWD' => 'Login password of the user account', - 'D3_TOTP_CURRPWD_HELP' => 'This ensures that only authorized users can make changes to these settings.', 'D3_TOTP_CURROTP' => 'Confirmation with one-time password', 'D3_TOTP_CURROTP_HELP' => 'If you have registered this customer account in your authentication app, you generate a one-time password, enter it here and send the form out immediately.', 'D3_TOTP_REGISTEREXIST' => 'existing registration', 'D3_TOTP_REGISTERDELETE' => 'Delete registration', 'D3_TOTP_REGISTERDELETE_DESC' => 'To change the registration, please delete it. You can then immediately create a new registration.
If you delete the registration, the account is no longer protected by the two-factor authentication.', + 'D3_TOTP_REGISTERDELETED' => 'The registration has been deleted.', 'D3_TOTP_BACKUPCODES' => 'backup codes', 'D3_TOTP_BACKUPCODES_DESC' => 'You can use these backup codes to log on if it is not possible to generate the one-time password (e.g. device lost or newly installed). You can then change the settings to use 2-factor authentication or create a new 2FA login. Please save these codes safely at this moment. After leaving this page, these codes cannot be displayed again.', @@ -48,5 +47,5 @@ $aLang = [ 'D3_TOTP_SAVE' => 'Save', 'D3_TOTP_ERROR_UNVALID' => 'The one-time password is invalid.', - 'D3_TOTP_ERROR_PWDONTPASS' => 'The password does not match the selected user account.', + 'D3_TOTP_ALREADY_EXIST' => 'The registration has already been saved.', ]; diff --git a/src/Application/views/admin/tpl/d3user_totp.tpl b/src/Application/views/admin/tpl/d3user_totp.tpl index 8d73e9c..96f652a 100644 --- a/src/Application/views/admin/tpl/d3user_totp.tpl +++ b/src/Application/views/admin/tpl/d3user_totp.tpl @@ -101,16 +101,6 @@ - - - - - - - [{oxinputhelp ident="D3_TOTP_CURRPWD_HELP"}] - - - diff --git a/src/tests/unit/Application/Controller/Admin/d3user_totpTest.php b/src/tests/unit/Application/Controller/Admin/d3user_totpTest.php index 0e65cdb..aed9c37 100644 --- a/src/tests/unit/Application/Controller/Admin/d3user_totpTest.php +++ b/src/tests/unit/Application/Controller/Admin/d3user_totpTest.php @@ -183,49 +183,6 @@ class d3user_totpTest extends d3TotpUnitTestCase ); } - /** - * @test - * @throws ReflectionException - */ - public function cantSaveBecauseOfWrongPassword() - { - /** @var d3backupcodelist|PHPUnit_Framework_MockObject_MockObject $oControllerMock */ - $oBackupCodeListMock = $this->getMock(d3backupcodelist::class, array( - 'save', - )); - $oBackupCodeListMock->expects($this->never())->method('save')->willReturn(true); - - /** @var d3totp|PHPUnit_Framework_MockObject_MockObject $oControllerMock */ - $oTotpMock = $this->getMock(d3totp::class, array( - 'save', - ), array(), '', false); - $oTotpMock->expects($this->never())->method('save')->willReturn(true); - - /** @var User|PHPUnit_Framework_MockObject_MockObject $oControllerMock */ - $oUserMock = $this->getMock(User::class, array( - 'load', - 'isSamePassword', - )); - $oUserMock->expects($this->atLeast(1))->method('load')->willReturn(true); - $oUserMock->expects($this->atLeast(1))->method('isSamePassword')->willReturn(false); - - /** @var d3user_totp|PHPUnit_Framework_MockObject_MockObject $oControllerMock */ - $oControllerMock = $this->getMock(d3user_totp::class, array( - 'getEditObjectId', - 'getUserObject', - 'getTotpObject', - 'getBackupcodeListObject' - )); - $oControllerMock->method('getEditObjectId')->willReturn('foobar'); - $oControllerMock->expects($this->once())->method('getUserObject')->willReturn($oUserMock); - $oControllerMock->method('getTotpObject')->willReturn($oTotpMock); - $oControllerMock->method('getBackupcodeListObject')->willReturn($oBackupCodeListMock); - - $this->_oController = $oControllerMock; - - $this->callMethod($this->_oController, 'save'); - } - /** * @test * @throws ReflectionException @@ -252,14 +209,6 @@ class d3user_totpTest extends d3TotpUnitTestCase $oTotpMock->method('saveSecret')->willReturn(true); $oTotpMock->method('assign')->willReturn(true); - /** @var User|PHPUnit_Framework_MockObject_MockObject $oControllerMock */ - $oUserMock = $this->getMock(User::class, array( - 'load', - 'isSamePassword', - )); - $oUserMock->expects($this->once())->method('load')->willReturn(true); - $oUserMock->expects($this->once())->method('isSamePassword')->willReturn(true); - /** @var d3user_totp|PHPUnit_Framework_MockObject_MockObject $oControllerMock */ $oControllerMock = $this->getMock(d3user_totp::class, array( 'getEditObjectId', @@ -268,7 +217,6 @@ class d3user_totpTest extends d3TotpUnitTestCase 'getBackupcodeListObject' )); $oControllerMock->method('getEditObjectId')->willReturn('foobar'); - $oControllerMock->expects($this->once())->method('getUserObject')->willReturn($oUserMock); $oControllerMock->method('getTotpObject')->willReturn($oTotpMock); $oControllerMock->method('getBackupcodeListObject')->willReturn($oBackupCodeListMock); @@ -305,14 +253,6 @@ class d3user_totpTest extends d3TotpUnitTestCase $oTotpMock->method('saveSecret')->willReturn(true); $oTotpMock->method('assign')->willReturn(true); - /** @var User|PHPUnit_Framework_MockObject_MockObject $oControllerMock */ - $oUserMock = $this->getMock(User::class, array( - 'load', - 'isSamePassword', - )); - $oUserMock->expects($this->atLeast(1))->method('load')->willReturn(true); - $oUserMock->expects($this->atLeast(1))->method('isSamePassword')->willReturn(true); - /** @var d3user_totp|PHPUnit_Framework_MockObject_MockObject $oControllerMock */ $oControllerMock = $this->getMock(d3user_totp::class, array( 'getEditObjectId', @@ -321,7 +261,6 @@ class d3user_totpTest extends d3TotpUnitTestCase 'getBackupcodeListObject' )); $oControllerMock->method('getEditObjectId')->willReturn('foobar'); - $oControllerMock->expects($this->once())->method('getUserObject')->willReturn($oUserMock); $oControllerMock->method('getTotpObject')->willReturn($oTotpMock); $oControllerMock->method('getBackupcodeListObject')->willReturn($oBackupCodeListMock); @@ -363,14 +302,6 @@ class d3user_totpTest extends d3TotpUnitTestCase $oTotpMock->method('saveSecret')->willReturn(true); $oTotpMock->method('assign')->willReturn(true); - /** @var User|PHPUnit_Framework_MockObject_MockObject $oControllerMock */ - $oUserMock = $this->getMock(User::class, array( - 'load', - 'isSamePassword', - )); - $oUserMock->expects($this->atLeast(1))->method('load')->willReturn(true); - $oUserMock->expects($this->atLeast(1))->method('isSamePassword')->willReturn(true); - /** @var d3user_totp|PHPUnit_Framework_MockObject_MockObject $oControllerMock */ $oControllerMock = $this->getMock(d3user_totp::class, array( 'getEditObjectId', @@ -379,7 +310,6 @@ class d3user_totpTest extends d3TotpUnitTestCase 'getBackupcodeListObject' )); $oControllerMock->method('getEditObjectId')->willReturn('foobar'); - $oControllerMock->expects($this->once())->method('getUserObject')->willReturn($oUserMock); $oControllerMock->method('getTotpObject')->willReturn($oTotpMock); $oControllerMock->method('getBackupcodeListObject')->willReturn($oBackupCodeListMock);