Added additional checking for otp counter

2 files changed, 106 insertions, 91 deletions

diff --git a/src/Otp/Otp.php b/src/Otp/Otp.php
index b88e889..59aee18 100644
--- a/src/Otp/Otp.php
+++ b/src/Otp/Otp.php
@@ -67,73 +67,77 @@ class Otp implements OtpInterface
 	 */
 	protected $algorithm = 'sha1';
 	
-	/* (non-PHPdoc)
-	 * @see Otp.OtpInterface::hotp()
-	*/
-	public function hotp($secret, $counter)
-	{
-		$hash = hash_hmac(
-				$this->algorithm,
-				$this->getBinaryCounter($counter),
-				$secret,
-				true
-		);
-	
-		return str_pad($this->truncate($hash), $this->digits, '0', STR_PAD_LEFT);
-	}
-	
-	/* (non-PHPdoc)
-	 * @see Otp.OtpInterface::totp()
-	*/
-	public function totp($secret, $timecounter = null)
-	{
-		if (is_null($timecounter)) {
-			$timecounter = $this->getTimecounter();
-		}
-	
-		return $this->hotp($secret, $timecounter);
-	}
-	
-	/* (non-PHPdoc)
-	 * @see Otp.OtpInterface::checkHotp()
-	*/
-	public function checkHotp($secret, $counter, $key)
-	{
-		return $this->safeCompare($this->hotp($secret, $counter), $key);
-	}
-	
-	/* (non-PHPdoc)
-	 * @see Otp.OtpInterface::checkTotp()
-	*/
-	public function checkTotp($secret, $key)
-	{
-		// Counter comes from time now
-		// Also we check the current timestamp as well as previous and future ones
-		// according to $timerange
-		$timecounter = $this->getTimecounter();
-	
-		$start = $timecounter - ($this->timerange);
-		$end = $timecounter + ($this->timerange);
-	
-		// We first try the current, as it is the most likely to work
-		if ($this->safeCompare($this->totp($secret, $timecounter), $key)) {
-			return true;
-		}
-	
-		// Well, that didn't work, so try the others
-		for ($t = $start; $t <= $end; $t = $t + 1) {
-			if ($t == $timecounter) {
-				// Already tried that one
-				continue;
-			}
-				
-			if ($this->safeCompare($this->totp($secret, $t), $key)) {
-				return true;
-			}
+	/* (non-PHPdoc)

+	 * @see Otp.OtpInterface::hotp()

+	*/

+	public function hotp($secret, $counter)

+	{

+		if (!is_numeric($counter)) {
+			throw new \InvalidArgumentException('Counter must be integer');
 		}
-	
-		// if none worked, then return false
-		return false;
+		
+		$hash = hash_hmac(

+				$this->algorithm,

+				$this->getBinaryCounter($counter),

+				$secret,

+				true

+		);

+	

+		return str_pad($this->truncate($hash), $this->digits, '0', STR_PAD_LEFT);

+	}

+	

+	/* (non-PHPdoc)

+	 * @see Otp.OtpInterface::totp()

+	*/

+	public function totp($secret, $timecounter = null)

+	{

+		if (is_null($timecounter)) {

+			$timecounter = $this->getTimecounter();

+		}

+	

+		return $this->hotp($secret, $timecounter);

+	}

+	

+	/* (non-PHPdoc)

+	 * @see Otp.OtpInterface::checkHotp()

+	*/

+	public function checkHotp($secret, $counter, $key)

+	{

+		return $this->safeCompare($this->hotp($secret, $counter), $key);

+	}

+	

+	/* (non-PHPdoc)

+	 * @see Otp.OtpInterface::checkTotp()

+	*/

+	public function checkTotp($secret, $key)

+	{

+		// Counter comes from time now

+		// Also we check the current timestamp as well as previous and future ones

+		// according to $timerange

+		$timecounter = $this->getTimecounter();

+	

+		$start = $timecounter - ($this->timerange);

+		$end = $timecounter + ($this->timerange);

+	

+		// We first try the current, as it is the most likely to work

+		if ($this->safeCompare($this->totp($secret, $timecounter), $key)) {

+			return true;

+		}

+	

+		// Well, that didn't work, so try the others

+		for ($t = $start; $t <= $end; $t = $t + 1) {

+			if ($t == $timecounter) {

+				// Already tried that one

+				continue;

+			}

+				

+			if ($this->safeCompare($this->totp($secret, $t), $key)) {

+				return true;

+			}

+		}

+	

+		// if none worked, then return false

+		return false;

 	}
 	
 	/**
@@ -146,10 +150,10 @@ class Otp implements OtpInterface
 	 * @return \Otp\Otp
 	 */
 	
-	/*
-	 * This has been disabled since it does not bring the expected results
-	 * according to the RFC test vectors for sha256 or sha512.
-	 * Until that is fixed, the algorithm simply stays at sha1.
+	/*

+	 * This has been disabled since it does not bring the expected results

+	 * according to the RFC test vectors for sha256 or sha512.

+	 * Until that is fixed, the algorithm simply stays at sha1.

 	 * Google Authenticator does not support sha256 and sha512 at the moment.
 	 *
 	
@@ -198,10 +202,10 @@ class Otp implements OtpInterface
 	 *
 	 * @return integer
 	 */
-	public function getPeriod()
-	{
-		return $this->period;
-	}
+	public function getPeriod()

+	{

+		return $this->period;

+	}

 	
 	/**
 	 * Setting number of otp digits
@@ -226,9 +230,9 @@ class Otp implements OtpInterface
 	 *
 	 * @return integer
 	 */
-	public function getDigits()
-	{
-		return $this->digits;
+	public function getDigits()

+	{

+		return $this->digits;

 	}
 	
 	/**
@@ -252,8 +256,8 @@ class Otp implements OtpInterface
 	 * @return integer Time counter
 	 */
 	protected function getTimecounter()
-	{
-		return floor(time() / $this->period);
+	{

+		return floor(time() / $this->period);

 	}
 	
 	/**
@@ -267,13 +271,13 @@ class Otp implements OtpInterface
 	 */
 	protected function truncate($hash)
 	{
-		$offset = ord($hash[19]) & 0xf;
-		
+		$offset = ord($hash[19]) & 0xf;

+		

 		return (
-		    ((ord($hash[$offset+0]) & 0x7f) << 24 ) |
-			((ord($hash[$offset+1]) & 0xff) << 16 ) |
-			((ord($hash[$offset+2]) & 0xff) << 8 ) |
-			(ord($hash[$offset+3]) & 0xff)
+		    ((ord($hash[$offset+0]) & 0x7f) << 24 ) |

+			((ord($hash[$offset+1]) & 0xff) << 16 ) |

+			((ord($hash[$offset+2]) & 0xff) << 8 ) |

+			(ord($hash[$offset+3]) & 0xff)

 		    ) % pow(10, $this->digits);
 	}
 	
@@ -298,11 +302,11 @@ class Otp implements OtpInterface
 		// time differences in sha1 creation, all you know is that a longer
 		// input takes longer to hash, not how long the actual compared value is
 		$result = 0;
-		
-		for ($i = 0; $i < 40; $i++) {
-			$result |= ord($sha1a[$i]) ^ ord($sha1b[$i]);
+		

+		for ($i = 0; $i < 40; $i++) {

+			$result |= ord($sha1a[$i]) ^ ord($sha1b[$i]);

 		}
-		
+		

 		return $result == 0;
 	}
 
diff --git a/tests/Otp/OtpTest.php b/tests/Otp/OtpTest.php
index 81ac3cf..109ced8 100644
--- a/tests/Otp/OtpTest.php
+++ b/tests/Otp/OtpTest.php
@@ -18,6 +18,8 @@ class OtpTest extends \PHPUnit_Framework_TestCase
 	 */
 	private $Otp;
 	
+	private $secret = "12345678901234567890";
+	
 	/**
 	 * Prepares the environment before running a test.
 	 */
@@ -47,7 +49,7 @@ class OtpTest extends \PHPUnit_Framework_TestCase
 	 */
 	public function testHotpRfc()
 	{
-		$secret = "12345678901234567890";
+		$secret = $this->secret;
 	
 		$this->assertEquals('755224', $this->Otp->hotp($secret, 0));
 		$this->assertEquals('287082', $this->Otp->hotp($secret, 1));
@@ -71,7 +73,7 @@ class OtpTest extends \PHPUnit_Framework_TestCase
 	 */
 	public function testTotpRfc()
 	{
-		$secret = "12345678901234567890";
+		$secret = $this->secret;
 		
 		// Test vectors are in 8 digits
 		$this->Otp->setDigits(8);
@@ -111,5 +113,14 @@ class OtpTest extends \PHPUnit_Framework_TestCase
 		$this->assertEquals('47863826', $this->Otp->hotp($secret, floor(20000000000/30)), 'sha512 with time 20000000000');
 		*/
 	}
+	
+	/**

+	 * @expectedException InvalidArgumentException
+	 * @expectedExceptionMessage Counter must be integer

+	 */
+	public function testHotpInvalidCounter()
+	{
+		$this->Otp->hotp($this->secret, 'a');
+	}
 
 }