From d5fc28194ddade9c83666949ca4a8f6568020f20 Mon Sep 17 00:00:00 2001 From: Daniel Seifert Date: Fri, 16 Aug 2019 23:17:48 +0200 Subject: [PATCH] fix creating backupcode from false customer account --- src/Application/Model/d3backupcode.php | 16 +++++++-- src/Application/Model/d3backupcodelist.php | 2 +- src/Application/Model/d3totp.php | 6 +++- .../Application/Model/d3backupcodeTest.php | 19 +++++++++-- .../unit/Application/Model/d3totpTest.php | 33 +++++++++++++++++++ 5 files changed, 68 insertions(+), 8 deletions(-) diff --git a/src/Application/Model/d3backupcode.php b/src/Application/Model/d3backupcode.php index 38a318b..304cb56 100644 --- a/src/Application/Model/d3backupcode.php +++ b/src/Application/Model/d3backupcode.php @@ -36,7 +36,7 @@ class d3backupcode extends BaseModel $this->assign( [ 'oxuserid' => $sUserId, - 'backupcode' => $this->d3EncodeBC($sCode), + 'backupcode' => $this->d3EncodeBC($sCode, $sUserId), ] ); @@ -53,10 +53,12 @@ class d3backupcode extends BaseModel * @return false|string * @throws DatabaseConnectionException */ - public function d3EncodeBC($code) + public function d3EncodeBC($code, $sUserId) { $oDb = DatabaseProvider::getDb(); - $salt = $this->d3GetUser()->getFieldData('oxpasssalt'); + $oUser = $this->d3GetUserObject(); + $oUser->load($sUserId); + $salt = $oUser->getFieldData('oxpasssalt'); $sSelect = "SELECT BINARY MD5( CONCAT( " . $oDb->quote($code) . ", UNHEX( ".$oDb->quote($salt)." ) ) )"; return $oDb->getOne($sSelect); @@ -73,4 +75,12 @@ class d3backupcode extends BaseModel $oUser->load($sUserId); return $oUser; } + + /** + * @return User + */ + public function d3GetUserObject() + { + return oxNew(User::class); + } } \ No newline at end of file diff --git a/src/Application/Model/d3backupcodelist.php b/src/Application/Model/d3backupcodelist.php index 6d9739b..21b5be6 100644 --- a/src/Application/Model/d3backupcodelist.php +++ b/src/Application/Model/d3backupcodelist.php @@ -104,7 +104,7 @@ class d3backupcodelist extends ListModel $oDb = $this->d3GetDb(); $query = "SELECT oxid FROM ".$this->getBaseObject()->getViewName(). - " WHERE ".$oDb->quoteIdentifier('backupcode')." = ".$oDb->quote($this->getBaseObject()->d3EncodeBC($totp))." AND ". + " WHERE ".$oDb->quoteIdentifier('backupcode')." = ".$oDb->quote($this->getBaseObject()->d3EncodeBC($totp, $this->d3GetUser()->getId()))." AND ". $oDb->quoteIdentifier("oxuserid") ." = ".$oDb->quote($this->d3GetUser()->getId()); $sVerify = $oDb->getOne($query); diff --git a/src/Application/Model/d3totp.php b/src/Application/Model/d3totp.php index 905dc80..e0233d8 100644 --- a/src/Application/Model/d3totp.php +++ b/src/Application/Model/d3totp.php @@ -212,7 +212,8 @@ class d3totp extends BaseModel public function verify($totp, $seed = null) { $blVerify = $this->getTotp($seed)->verify($totp, null, $this->timeWindow); - if (false == $blVerify) { + + if (false == $blVerify && null == $seed) { $oBC = $this->d3GetBackupCodeListObject(); $blVerify = $oBC->verify($totp); @@ -220,6 +221,9 @@ class d3totp extends BaseModel $oException = oxNew(d3totp_wrongOtpException::class); throw $oException; } + } elseif (false == $blVerify && $seed) { + $oException = oxNew(d3totp_wrongOtpException::class); + throw $oException; } return $blVerify; diff --git a/src/tests/unit/Application/Model/d3backupcodeTest.php b/src/tests/unit/Application/Model/d3backupcodeTest.php index 4d2fe2e..f2d169a 100644 --- a/src/tests/unit/Application/Model/d3backupcodeTest.php +++ b/src/tests/unit/Application/Model/d3backupcodeTest.php @@ -95,7 +95,8 @@ class d3backupcodeTest extends d3TotpUnitTestCase public function d3EncodeBCPass() { /** @var User|PHPUnit_Framework_MockObject_MockObject $oUserMock */ - $oUserMock = $this->getMock(User::class, array(), array(), '', false); + $oUserMock = $this->getMock(User::class, array('load'), array(), '', false); + $oUserMock->method('load')->willReturn(true); $oUserMock->assign( array( 'oxpasssalt' => 'abcdefghijk' @@ -106,13 +107,13 @@ class d3backupcodeTest extends d3TotpUnitTestCase $oModelMock = $this->getMock(d3backupcode::class, array( 'd3GetUser', )); - $oModelMock->method('d3GetUser')->willReturn($oUserMock); + $oModelMock->method('d3GetUserObject')->willReturn($oUserMock); $this->_oModel = $oModelMock; $this->assertSame( 'e10adc3949ba59abbe56e057f20f883e', - $this->callMethod($this->_oModel, 'd3EncodeBC', array('123456')) + $this->callMethod($this->_oModel, 'd3EncodeBC', array('123456', 'userId')) ); } @@ -156,4 +157,16 @@ class d3backupcodeTest extends d3TotpUnitTestCase $oUser->getId() ); } + + /** + * @test + * @throws ReflectionException + */ + public function d3getUserObjectReturnsRightInstance() + { + $this->assertInstanceOf( + User::class, + $this->callMethod($this->_oModel, 'd3GetUserObject') + ); + } } \ No newline at end of file diff --git a/src/tests/unit/Application/Model/d3totpTest.php b/src/tests/unit/Application/Model/d3totpTest.php index 5c157e1..fecacbc 100644 --- a/src/tests/unit/Application/Model/d3totpTest.php +++ b/src/tests/unit/Application/Model/d3totpTest.php @@ -711,6 +711,39 @@ class d3totpTest extends d3TotpUnitTestCase $this->callMethod($this->_oModel, 'verify', ['012345']); } + /** + * @test + * @throws ReflectionException + */ + public function verifyWithSeedFailed() + { + $this->setExpectedException(d3totp_wrongOtpException::class); + + /** @var d3backupcodelist|PHPUnit_Framework_MockObject_MockObject $oBackupCodeListMock */ + $oBackupCodeListMock = $this->getMock(d3backupcodelist::class, array( + 'verify' + )); + $oBackupCodeListMock->expects($this->never())->method('verify')->willReturn(false); + + /** @var d3totp|PHPUnit_Framework_MockObject_MockObject $oTotpMock */ + $oTotpMock = $this->getMock(d3totp::class, array( + 'verify' + )); + $oTotpMock->expects($this->once())->method('verify')->willReturn(false); + + /** @var d3totp|PHPUnit_Framework_MockObject_MockObject $oModelMock */ + $oModelMock = $this->getMock(d3totp::class, array( + 'getTotp', + 'd3GetBackupCodeListObject' + )); + $oModelMock->method('getTotp')->willReturn($oTotpMock); + $oModelMock->method('d3GetBackupCodeListObject')->willReturn($oBackupCodeListMock); + + $this->_oModel = $oModelMock; + + $this->callMethod($this->_oModel, 'verify', ['012345', 'abcdef']); + } + /** * @test * @throws ReflectionException