diff --git a/appinfo/database.xml b/appinfo/database.xml
index 00fb3af..006e9c5 100644
--- a/appinfo/database.xml
+++ b/appinfo/database.xml
@@ -27,6 +27,11 @@
clob
true
+
+ last_validated_key
+ text
+ 32
+
verified
boolean
diff --git a/lib/Db/TotpSecret.php b/lib/Db/TotpSecret.php
index 6e5c782..0d33a8b 100644
--- a/lib/Db/TotpSecret.php
+++ b/lib/Db/TotpSecret.php
@@ -2,6 +2,7 @@
/**
* @author Christoph Wurst
+ * @author Semih Serhat Karakaya
*
* Two-factor TOTP
*
@@ -26,7 +27,6 @@
/**
* @method string getUserId()
* @method void setUserId(string $userId)
- * @method string getSecret()
* @method void setSecret(string $secret)
* @method boolean getVerified()
* @method void setVerified(bool $verified)
@@ -39,7 +39,31 @@ class TotpSecret extends Entity {
/** @var string */
protected $secret;
+ /** @var string */
+ protected $lastValidatedKey;
+
/** @var boolean */
protected $verified;
+ /**
+ * @return string
+ */
+ public function getSecret() {
+ return $this->secret;
+ }
+
+ /**
+ * @return string
+ */
+ public function getLastValidatedKey() {
+ return $this->lastValidatedKey;
+ }
+
+ /**
+ * @param string $key
+ */
+ public function setLastValidatedKey($key) {
+ $this->lastValidatedKey = $key;
+ }
+
}
diff --git a/lib/Db/TotpSecretMapper.php b/lib/Db/TotpSecretMapper.php
index 29feaf4..740beb3 100644
--- a/lib/Db/TotpSecretMapper.php
+++ b/lib/Db/TotpSecretMapper.php
@@ -43,7 +43,7 @@ public function getSecret(IUser $user) {
/* @var $qb IQueryBuilder */
$qb = $this->db->getQueryBuilder();
- $qb->select('id', 'user_id', 'secret', 'verified')
+ $qb->select('id', 'user_id', 'secret', 'verified', 'last_validated_key')
->from('twofactor_totp_secrets')
->where($qb->expr()->eq('user_id', $qb->createNamedParameter($user->getUID())));
$result = $qb->execute();
diff --git a/lib/Service/Totp.php b/lib/Service/Totp.php
index 252399b..b6fdaae 100644
--- a/lib/Service/Totp.php
+++ b/lib/Service/Totp.php
@@ -40,9 +40,13 @@ class Totp implements ITotp {
/** @var ICrypto */
private $crypto;
- public function __construct(TotpSecretMapper $secretMapper, ICrypto $crypto) {
+ /** @var Otp */
+ private $otp;
+
+ public function __construct(TotpSecretMapper $secretMapper, ICrypto $crypto, Otp $otp) {
$this->secretMapper = $secretMapper;
$this->crypto = $crypto;
+ $this->otp = $otp;
}
public function hasSecret(IUser $user) {
@@ -109,10 +113,18 @@ public function validateSecret(IUser $user, $key) {
throw new NoTotpSecretFoundException();
}
- $secret = $this->crypto->decrypt($dbSecret->getSecret());
-
- $otp = new Otp();
- return $otp->checkTotp(Base32::decode($secret), $key, 3);
+ /**
+ * Check last validated key to prevent consecutive usage of the same key
+ */
+ if ($dbSecret->getLastValidatedKey() !== $key) {
+ $secret = $this->crypto->decrypt($dbSecret->getSecret());
+ if ($this->otp->checkTotp(Base32::decode($secret), $key, 3) === true) {
+ $dbSecret->setLastValidatedKey($key);
+ $this->secretMapper->update($dbSecret);
+ return true;
+ }
+ }
+ return false;
}
/**
diff --git a/tests/unit/Service/TotpTest.php b/tests/unit/Service/TotpTest.php
new file mode 100644
index 0000000..4976cb8
--- /dev/null
+++ b/tests/unit/Service/TotpTest.php
@@ -0,0 +1,121 @@
+
+ *
+ * Two-factor TOTP
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License, version 3,
+ * along with this program. If not, see
+ *
+ */
+
+namespace OCA\TwoFactor_Totp\Tests\Service;
+
+use OCA\TwoFactor_Totp\Db\TotpSecret;
+use OCA\TwoFactor_Totp\Db\TotpSecretMapper;
+use OCA\TwoFactor_Totp\Exception\NoTotpSecretFoundException;
+use OCA\TwoFactor_Totp\Service\Totp;
+use OCP\AppFramework\Db\DoesNotExistException;
+use OCP\IUser;
+use OCP\Security\ICrypto;
+use Otp\Otp;
+use Test\TestCase;
+
+/**
+ * Class TotpTest
+ */
+class TotpTest extends TestCase {
+
+ /** @var TotpSecretMapper | \PHPUnit_Framework_MockObject_MockObject */
+ private $secretMapper;
+
+ /** @var ICrypto | \PHPUnit_Framework_MockObject_MockObject */
+ private $crypto;
+
+ /** @var Otp | \PHPUnit_Framework_MockObject_MockObject */
+ private $otp;
+
+ /** @var Totp */
+ private $totp;
+
+ protected function setUp() {
+ parent::setUp();
+
+ $this->secretMapper = $this->createMock(TotpSecretMapper::class);
+ $this->crypto = $this->createMock(ICrypto::class);
+ $this->otp = $this->createMock(Otp::class);
+
+ $this->totp = new Totp($this->secretMapper, $this->crypto, $this->otp);
+
+ }
+
+ /**
+ * @dataProvider validationProvider
+ * @param string $lastKey
+ * @param string $key
+ * @param boolean $validationResult
+ * @param boolean $expectedResult
+ */
+ public function testValidateSecret($lastKey, $key, $validationResult, $expectedResult) {
+ /** @var IUser | \PHPUnit_Framework_MockObject_MockObject $user */
+ $user = $this->createMock(IUser::class);
+ $dbSecret = $this->createMock(TotpSecret::class);
+
+ $dbSecret->expects($this->once())
+ ->method('getLastValidatedKey')
+ ->will($this->returnValue($lastKey));
+ $this->secretMapper->expects($this->once())
+ ->method('getSecret')
+ ->with($user)
+ ->will($this->returnValue($dbSecret));
+
+ if($lastKey !== $key) {
+ $dbSecret->expects($this->once())
+ ->method('getSecret')
+ ->will($this->returnValue('secret'));
+ $this->otp->expects($this->once())
+ ->method('checkTotp')
+ ->will($this->returnValue($validationResult));
+ }
+ if($expectedResult === true) {
+ $dbSecret->expects($this->once())
+ ->method('setLastValidatedKey')
+ ->with($key);
+ $this->secretMapper->expects($this->once())
+ ->method('update')
+ ->with($dbSecret);
+ }
+ $this->assertEquals($this->totp->validateSecret($user, $key), $expectedResult);
+ }
+
+ public function validationProvider() {
+ return [
+ [ '000000', '000000', true, false ],
+ [ '000000', '000000', false, false ],
+ [ '000000', '000001', false, false ],
+ [ '000000', '000001', true, true ]
+ ];
+ }
+
+ public function testValidateSecretNoSecret() {
+ /** @var IUser | \PHPUnit_Framework_MockObject_MockObject $user */
+ $user = $this->createMock(IUser::class);
+
+ $this->secretMapper->expects($this->once())
+ ->method('getSecret')
+ ->with($user)
+ ->will($this->throwException(new DoesNotExistException('')));
+ $this->expectException(NoTotpSecretFoundException::class);
+ $this->totp->validateSecret($user, 'testkey');
+ }
+
+}
\ No newline at end of file