fix creating backupcode from false customer account

This commit is contained in:
Daniel Seifert 2019-08-16 23:17:48 +02:00
parent e1f5372f6b
commit d5fc28194d
5 changed files with 68 additions and 8 deletions

View File

@ -36,7 +36,7 @@ class d3backupcode extends BaseModel
$this->assign( $this->assign(
[ [
'oxuserid' => $sUserId, 'oxuserid' => $sUserId,
'backupcode' => $this->d3EncodeBC($sCode), 'backupcode' => $this->d3EncodeBC($sCode, $sUserId),
] ]
); );
@ -53,10 +53,12 @@ class d3backupcode extends BaseModel
* @return false|string * @return false|string
* @throws DatabaseConnectionException * @throws DatabaseConnectionException
*/ */
public function d3EncodeBC($code) public function d3EncodeBC($code, $sUserId)
{ {
$oDb = DatabaseProvider::getDb(); $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)." ) ) )"; $sSelect = "SELECT BINARY MD5( CONCAT( " . $oDb->quote($code) . ", UNHEX( ".$oDb->quote($salt)." ) ) )";
return $oDb->getOne($sSelect); return $oDb->getOne($sSelect);
@ -73,4 +75,12 @@ class d3backupcode extends BaseModel
$oUser->load($sUserId); $oUser->load($sUserId);
return $oUser; return $oUser;
} }
/**
* @return User
*/
public function d3GetUserObject()
{
return oxNew(User::class);
}
} }

View File

@ -104,7 +104,7 @@ class d3backupcodelist extends ListModel
$oDb = $this->d3GetDb(); $oDb = $this->d3GetDb();
$query = "SELECT oxid FROM ".$this->getBaseObject()->getViewName(). $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()); $oDb->quoteIdentifier("oxuserid") ." = ".$oDb->quote($this->d3GetUser()->getId());
$sVerify = $oDb->getOne($query); $sVerify = $oDb->getOne($query);

View File

@ -212,7 +212,8 @@ class d3totp extends BaseModel
public function verify($totp, $seed = null) public function verify($totp, $seed = null)
{ {
$blVerify = $this->getTotp($seed)->verify($totp, null, $this->timeWindow); $blVerify = $this->getTotp($seed)->verify($totp, null, $this->timeWindow);
if (false == $blVerify) {
if (false == $blVerify && null == $seed) {
$oBC = $this->d3GetBackupCodeListObject(); $oBC = $this->d3GetBackupCodeListObject();
$blVerify = $oBC->verify($totp); $blVerify = $oBC->verify($totp);
@ -220,6 +221,9 @@ class d3totp extends BaseModel
$oException = oxNew(d3totp_wrongOtpException::class); $oException = oxNew(d3totp_wrongOtpException::class);
throw $oException; throw $oException;
} }
} elseif (false == $blVerify && $seed) {
$oException = oxNew(d3totp_wrongOtpException::class);
throw $oException;
} }
return $blVerify; return $blVerify;

View File

@ -95,7 +95,8 @@ class d3backupcodeTest extends d3TotpUnitTestCase
public function d3EncodeBCPass() public function d3EncodeBCPass()
{ {
/** @var User|PHPUnit_Framework_MockObject_MockObject $oUserMock */ /** @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( $oUserMock->assign(
array( array(
'oxpasssalt' => 'abcdefghijk' 'oxpasssalt' => 'abcdefghijk'
@ -106,13 +107,13 @@ class d3backupcodeTest extends d3TotpUnitTestCase
$oModelMock = $this->getMock(d3backupcode::class, array( $oModelMock = $this->getMock(d3backupcode::class, array(
'd3GetUser', 'd3GetUser',
)); ));
$oModelMock->method('d3GetUser')->willReturn($oUserMock); $oModelMock->method('d3GetUserObject')->willReturn($oUserMock);
$this->_oModel = $oModelMock; $this->_oModel = $oModelMock;
$this->assertSame( $this->assertSame(
'e10adc3949ba59abbe56e057f20f883e', '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() $oUser->getId()
); );
} }
/**
* @test
* @throws ReflectionException
*/
public function d3getUserObjectReturnsRightInstance()
{
$this->assertInstanceOf(
User::class,
$this->callMethod($this->_oModel, 'd3GetUserObject')
);
}
} }

View File

@ -711,6 +711,39 @@ class d3totpTest extends d3TotpUnitTestCase
$this->callMethod($this->_oModel, 'verify', ['012345']); $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 * @test
* @throws ReflectionException * @throws ReflectionException