Minor Fixes and Updated Tests

3 files changed, 31 insertions, 20 deletions

diff --git a/src/Otp.php b/src/Otp.php
index 0a2b599..1df9782 100644
--- a/src/Otp.php
+++ b/src/Otp.php
@@ -62,8 +62,8 @@ class Otp implements OtpInterface
     */
     public function hotp($secret, $counter)
     {
-        if (!is_numeric($counter)) {
-            throw new \InvalidArgumentException('Counter must be integer');
+        if (!is_numeric($counter) || $counter < 0) {
+            throw new \InvalidArgumentException('Invalid counter supplied');
         }
         
         $hash = hash_hmac(
@@ -96,10 +96,14 @@ class Otp implements OtpInterface
         return $this->safeCompare($this->hotp($secret, $counter), $key);
     }
 
+
+    /* (non-PHPdoc)
+     * @see Otp.OtpInterface::checkHotpResync()
+    */
     public function checkHotpResync($secret, $counter, $key, $counterwindow = 2)
     {
-        if (!is_numeric($counter)) {
-            throw new \InvalidArgumentException('Counter must be integer');
+        if (!is_numeric($counter) || $counter < 0) {
+            throw new \InvalidArgumentException('Invalid counter supplied');
         }
 
         if(!is_numeric($counterwindow) || $counterwindow < 0){
diff --git a/src/OtpInterface.php b/src/OtpInterface.php
index b91b5f1..e241d5d 100644
--- a/src/OtpInterface.php
+++ b/src/OtpInterface.php
@@ -48,7 +48,7 @@ interface OtpInterface
      * @param integer $counter Counter
      * @param string  $key     User supplied key
      *
-     * @return boolean the counter if key is correct else false
+     * @return boolean True if key is correct
      */
     function checkHotp($secret, $counter, $key);
 
diff --git a/tests/OtpTest.php b/tests/OtpTest.php
index 3b59e2c..e9fcc56 100644
--- a/tests/OtpTest.php
+++ b/tests/OtpTest.php
@@ -110,14 +110,19 @@ class OtpTest extends \PHPUnit_Framework_TestCase
 		$this->assertEquals('47863826', $this->Otp->hotp($secret, floor(20000000000/30)), 'sha512 with time 20000000000');
 		*/
 	}
-	
+	public function invalidCounterValues()
+	{
+		return [['a'], [-1]];
+	}
+
 	/**
+	 * @dataProvider invalidCounterValues
 	 * @expectedException InvalidArgumentException
-	 * @expectedExceptionMessage Counter must be integer
+	 * @expectedExceptionMessage Invalid counter supplied
 	 */
-	public function testHotpInvalidCounter()
+	public function testHotpInvalidCounter($counter)
 	{
-		$this->Otp->hotp($this->secret, 'a');
+		$this->Otp->hotp($this->secret, $counter);
 	}
 
 	/**
@@ -128,14 +133,14 @@ class OtpTest extends \PHPUnit_Framework_TestCase
 		$secret = $this->secret;
 
 		// test default counter window
-		$this->assertEquals(0, $this->Otp->checkHotpResync($secret, 0, '755224'));
-		$this->assertEquals(1, $this->Otp->checkHotpResync($secret, 0, '287082'));
-		$this->assertEquals(2, $this->Otp->checkHotpResync($secret, 0, '359152'));
+		$this->assertSame(0, $this->Otp->checkHotpResync($secret, 0, '755224'));
+		$this->assertSame(1, $this->Otp->checkHotpResync($secret, 0, '287082'));
+		$this->assertSame(2, $this->Otp->checkHotpResync($secret, 0, '359152'));
 
 		// test provided counter window
-		$this->assertEquals(3, $this->Otp->checkHotpResync($secret, 0, '969429', 3));
-		$this->assertEquals(4, $this->Otp->checkHotpResync($secret, 0, '338314', 4));
-		$this->assertEquals(5, $this->Otp->checkHotpResync($secret, 0, '254676', 5));
+		$this->assertSame(3, $this->Otp->checkHotpResync($secret, 0, '969429', 3));
+		$this->assertSame(4, $this->Otp->checkHotpResync($secret, 0, '338314', 4));
+		$this->assertSame(5, $this->Otp->checkHotpResync($secret, 0, '254676', 5));
 
 		// test failures
 		$this->assertFalse($this->Otp->checkHotpResync($secret, 7, '287922'));
@@ -144,21 +149,23 @@ class OtpTest extends \PHPUnit_Framework_TestCase
 	}
 
 	/**
+	 * @dataProvider invalidCounterValues
 	 * @expectedException InvalidArgumentException
-	 * @expectedExceptionMessage Counter must be integer
+	 * @expectedExceptionMessage Invalid counter supplied
 	 */
-	public function testHotpResyncInvalidCounter()
+	public function testHotpResyncInvalidCounter($counter)
 	{
-		$this->Otp->checkHotpResync($this->secret, 'a', '755224');
+		$this->Otp->checkHotpResync($this->secret, $counter, '755224');
 	}
 
 	/**
+	 * @dataProvider invalidCounterValues
 	 * @expectedException InvalidArgumentException
 	 * @expectedExceptionMessage Invalid counterwindow supplied
 	 */
-	public function testHotpResyncInvalidCounterWindow()
+	public function testHotpResyncInvalidCounterWindow($counterwindow)
 	{
-		$this->Otp->checkHotpResync($this->secret, 0, '755224', 'a');
+		$this->Otp->checkHotpResync($this->secret, 0, '755224', $counterwindow);
 	}
 
 }