Use better exceptions

SAM-IT · Oct 8, 2018 · 51a529d · 51a529d
1 parent 60bec7e
commit 51a529d
Show file tree

Hide file tree

Showing 8 changed files with 96 additions and 38 deletions.
diff --git a/src/ExpiredLinkException.php b/src/ExpiredLinkException.php
@@ -0,0 +1,14 @@
+<?php
+declare(strict_types=1);
+
+
+namespace SamIT\Yii2\UrlSigner;
+
+
+class ExpiredLinkException extends UrlSignerException
+{
+    public function __construct(int $code = 0, \Exception $previous = null)
+    {
+        parent::__construct('This URL has expired', $code, $previous);
+    }
+}
diff --git a/src/HmacFilter.php b/src/HmacFilter.php
@@ -43,9 +43,7 @@ public function beforeAction($action)
          * @var Request $request
          */
         $request = $action->controller->module->get('request');
-        if (!$this->signer->verify($request->queryParams, $action->controller->route)) {
-            throw new ForbiddenHttpException("No or invalid HMAC");
-        }
+        $this->signer->verify($request->queryParams, $action->controller->route);
         return true;
     }
 

diff --git a/src/InvalidHmacException.php b/src/InvalidHmacException.php
@@ -0,0 +1,14 @@
+<?php
+declare(strict_types=1);
+
+
+namespace SamIT\Yii2\UrlSigner;
+
+
+class InvalidHmacException extends UrlSignerException
+{
+    public function __construct(int $code = 0, \Exception $previous = null)
+    {
+        parent::__construct('This security code in this URL invalid', $code, $previous);
+    }
+}
diff --git a/src/MissingHmacException.php b/src/MissingHmacException.php
@@ -0,0 +1,14 @@
+<?php
+declare(strict_types=1);
+
+
+namespace SamIT\Yii2\UrlSigner;
+
+
+class MissingHmacException extends UrlSignerException
+{
+    public function __construct(int $code = 0, \Exception $previous = null)
+    {
+        parent::__construct('This security code for this URL is missing', $code, $previous);
+    }
+}
diff --git a/src/UrlSigner.php b/src/UrlSigner.php
@@ -140,10 +140,15 @@ private function time(): int
     {
         return $this->_currentTimestamp ?? \time();
     }
-    private function checkExpiration(array $params): bool
+
+    private function checkExpiration(array $params): void
     {
         // Check expiration date.
-        return !isset($params[$this->expirationParam]) || $params[$this->expirationParam] > $this->time();
+        if (isset($params[$this->expirationParam])
+            && $params[$this->expirationParam] <= $this->time()
+        ) {
+            throw new ExpiredLinkException();
+        }
     }
 
     /**
@@ -190,21 +195,21 @@ private function getSignedParams(array $params): array
      * @throws \Exception
      * @return bool
      */
-    public function verify(array $params, string $route):bool
+    public function verify(array $params, string $route):void
     {
         if (!isset($params[$this->hmacParam])) {
-           return false;
+            throw new MissingHmacException();
         }
         $hmac = $params[$this->hmacParam];
 
         $signedParams = $this->getSignedParams($params);
 
         $calculated = $this->calculateHMAC($signedParams, $route);
         if (!\hash_equals($calculated, $hmac)) {
-            return false;
+            throw new InvalidHmacException();
         }
 
-        return $this->checkExpiration($params);
+        $this->checkExpiration($params);
     }
 
     private function urlEncode(string $bytes): string

diff --git a/src/UrlSignerException.php b/src/UrlSignerException.php
@@ -0,0 +1,12 @@
+<?php
+declare(strict_types=1);
+
+
+namespace SamIT\Yii2\UrlSigner;
+
+
+use yii\web\ForbiddenHttpException;
+
+abstract class UrlSignerException extends ForbiddenHttpException
+{
+}
diff --git a/tests/unit/HmacFilterTest.php b/tests/unit/HmacFilterTest.php
@@ -33,14 +33,8 @@ public function testValidConfig(): void
 
     public function testVerifyFalse(): void
     {
-        $mock = $this->make(SamIT\Yii2\UrlSigner\UrlSigner::class, [
-            'verify' => \Codeception\Stub\Expected::once(function() {
-                return false;
-            })
-        ]);
-
         $filter = new \SamIT\Yii2\UrlSigner\HmacFilter([
-            'signer' => $mock
+            'signer' => new SamIT\Yii2\UrlSigner\UrlSigner(['secret' => 'test123'])
         ]);
 
 

diff --git a/tests/unit/UrlSignerTest.php b/tests/unit/UrlSignerTest.php
@@ -1,17 +1,20 @@
 <?php
 declare(strict_types=1);
 
+use SamIT\Yii2\UrlSigner\InvalidHmacException;
+use SamIT\Yii2\UrlSigner\MissingHmacException;
+use SamIT\Yii2\UrlSigner\UrlSigner;
 class UrlSignerTest extends \Codeception\Test\Unit
 {
     public function testInvalidConfig(): void
     {
         $this->expectException(\yii\base\InvalidConfigException::class);
-        $signer = new SamIT\Yii2\UrlSigner\UrlSigner();
+        $signer = new UrlSigner();
 
     }
     public function testSimple(): void
     {
-        $signer = new SamIT\Yii2\UrlSigner\UrlSigner([
+        $signer = new UrlSigner([
             'secret' => 'test123'
         ]);
 
@@ -26,12 +29,12 @@ public function testSimple(): void
         $this->assertArrayNotHasKey($signer->paramsParam, $params);
         $route = $params[0];
         unset($params[0]);
-        $this->assertTrue($signer->verify($params, $route));
+        $signer->verify($params, $route);
     }
 
     public function testAdditions(): void
     {
-        $signer = new SamIT\Yii2\UrlSigner\UrlSigner([
+        $signer = new UrlSigner([
             'secret' => 'test123'
         ]);
 
@@ -46,14 +49,14 @@ public function testAdditions(): void
         $this->assertArrayHasKey($signer->paramsParam, $params);
         $route = $params[0];
         unset($params[0]);
-        $this->assertTrue($signer->verify($params, $route));
+        $signer->verify($params, $route);
         $params['extra'] = 'cool';
-        $this->assertTrue($signer->verify($params, $route));
+        $signer->verify($params, $route);
     }
 
     public function testDoubleSign(): void
     {
-        $signer = new SamIT\Yii2\UrlSigner\UrlSigner([
+        $signer = new UrlSigner([
             'secret' => 'test123'
         ]);
 
@@ -69,7 +72,7 @@ public function testDoubleSign(): void
 
     public function testMissingHmac(): void
     {
-        $signer = new SamIT\Yii2\UrlSigner\UrlSigner([
+        $signer = new UrlSigner([
             'secret' => 'test123'
         ]);
         $params = [
@@ -83,14 +86,15 @@ public function testMissingHmac(): void
         $this->assertArrayNotHasKey($signer->paramsParam, $params);
         $route = $params[0];
         unset($params[0]);
-        $this->assertTrue($signer->verify($params, $route));
+        $signer->verify($params, $route);
         unset($params[$signer->hmacParam]);
-        $this->assertFalse($signer->verify($params, $route));
+        $this->expectException(MissingHmacException::class);
+        $signer->verify($params, $route);
     }
 
     public function testModification(): void
     {
-        $signer = new SamIT\Yii2\UrlSigner\UrlSigner([
+        $signer = new UrlSigner([
             'secret' => 'test123'
         ]);
         $params = [
@@ -106,12 +110,13 @@ public function testModification(): void
         unset($params[0]);
 
         $params['test'] = 'abd';
-        $this->assertFalse($signer->verify($params, $route));
+        $this->expectException(InvalidHmacException::class);
+        $signer->verify($params, $route);
     }
 
     public function testRelativeRoute(): void
     {
-        $signer = new SamIT\Yii2\UrlSigner\UrlSigner([
+        $signer = new UrlSigner([
             'secret' => 'test123'
         ]);
 
@@ -125,7 +130,7 @@ public function testRelativeRoute(): void
 
     public function testMissingExpiration(): void
     {
-        $signer = new SamIT\Yii2\UrlSigner\UrlSigner([
+        $signer = new UrlSigner([
             'secret' => 'test123'
         ]);
 
@@ -136,12 +141,13 @@ public function testMissingExpiration(): void
         $signer->signParams($params);
 
         unset($params[$signer->expirationParam]);
-        $this->assertFalse($signer->verify($params, '/controller/action'));
+        $this->expectException(InvalidHmacException::class);
+        $signer->verify($params, '/controller/action');
     }
 
     public function testNoExpiration(): void
     {
-        $signer = new SamIT\Yii2\UrlSigner\UrlSigner([
+        $signer = new UrlSigner([
             'secret' => 'test123'
         ]);
         $expirationParam = $signer->expirationParam;
@@ -154,14 +160,14 @@ public function testNoExpiration(): void
         $signer->signParams($params);
         $this->assertNotContains($expirationParam, $params);
 
-        $this->assertTrue($signer->verify($params, '/controller/action'));
+        $signer->verify($params, '/controller/action');
         $signer->expirationParam = $expirationParam;
-        $this->assertTrue($signer->verify($params, '/controller/action'));
+        $signer->verify($params, '/controller/action');
     }
 
     public function testDefaultExpiration(): void
     {
-        $signer = new SamIT\Yii2\UrlSigner\UrlSigner([
+        $signer = new UrlSigner([
             'secret' => 'test123',
             'defaultExpirationInterval' => 'P14D'
         ]);
@@ -175,7 +181,7 @@ public function testDefaultExpiration(): void
 
     public function testTimeMocking(): void
     {
-        $signer = new SamIT\Yii2\UrlSigner\UrlSigner([
+        $signer = new UrlSigner([
             'secret' => 'test123',
             'defaultExpirationInterval' => 'PT01S'
         ]);
@@ -186,10 +192,11 @@ public function testTimeMocking(): void
         $this->assertContains('expires', $params);
         $this->assertSame(\time() + 1, $params['expires']);
         $signer->setCurrentTimestamp(\time() - 10);
-        $this->assertTrue($signer->verify($params, '/controller/action'));
+        $signer->verify($params, '/controller/action');
         $signer->setCurrentTimestamp(null);
-        $this->assertTrue($signer->verify($params, '/controller/action'));
+        $signer->verify($params, '/controller/action');
         $signer->setCurrentTimestamp(\time() + 5);
+        $this->expectException(\SamIT\Yii2\UrlSigner\ExpiredLinkException::class);
         $this->assertFalse($signer->verify($params, '/controller/action'));
     }
 }