refactor: Add custom exception and more defensive code.

ecphp · Feb 10, 2021 · 119443a · 119443a
1 parent 98166c2
commit 119443a
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 13 deletions.
diff --git a/spec/EcPhp/ApiGwAuthenticationBundle/Service/KeyLoader/JWKSKeyLoaderSpec.php b/spec/EcPhp/ApiGwAuthenticationBundle/Service/KeyLoader/JWKSKeyLoaderSpec.php
@@ -4,13 +4,12 @@
 
 namespace spec\EcPhp\ApiGwAuthenticationBundle\Service\KeyLoader;
 
+use EcPhp\ApiGwAuthenticationBundle\Exception\ApiGwAuthenticationException;
 use EcPhp\ApiGwAuthenticationBundle\Service\KeyConverter\KeyConverterInterface;
 use EcPhp\ApiGwAuthenticationBundle\Service\KeyLoader\JWKSKeyLoader;
 use EcPhp\ApiGwAuthenticationBundle\Service\KeyLoader\KeyLoaderInterface;
 use Exception;
 use PhpSpec\ObjectBehavior;
-use Symfony\Component\HttpClient\Exception\TransportException;
-use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface;
 use Symfony\Contracts\HttpClient\HttpClientInterface;
 use Symfony\Contracts\HttpClient\ResponseInterface;
 
@@ -42,10 +41,10 @@ public function it_can_throw_when_the_request_failed(KeyLoaderInterface $keyLoad
 
         $httpClient
             ->request('GET', KeyLoaderInterface::TYPE_PUBLIC)
-            ->willThrow(new TransportException('Error'));
+            ->willThrow(new ApiGwAuthenticationException('foo'));
 
         $this
-            ->shouldThrow(TransportExceptionInterface::class)
+            ->shouldThrow(ApiGwAuthenticationException::class)
             ->during('loadKey', [KeyLoaderInterface::TYPE_PUBLIC]);
     }
 

diff --git a/src/Service/KeyLoader/ApiGwKeyLoader.php b/src/Service/KeyLoader/ApiGwKeyLoader.php
@@ -4,11 +4,14 @@
 
 namespace EcPhp\ApiGwAuthenticationBundle\Service\KeyLoader;
 
+use EcPhp\ApiGwAuthenticationBundle\Exception\ApiGwAuthenticationException;
 use EcPhp\ApiGwAuthenticationBundle\Service\KeyConverter\KeyConverterInterface;
 use Lexik\Bundle\JWTAuthenticationBundle\Services\KeyLoader\RawKeyLoader;
 use Symfony\Contracts\HttpClient\HttpClientInterface;
 use Throwable;
 
+use function array_key_exists;
+
 /**
  * Class ApiGwKeyLoader.
  *
@@ -80,7 +83,7 @@ public function getSigningKey(): string
         return $this->environment[KeyLoaderInterface::TYPE_PRIVATE] ?? '';
     }
 
-    public function loadKey($type)
+    public function loadKey($type): string
     {
         $publicKey = $this->getPublicKey();
         $signingKey = $this->getSigningKey();
@@ -154,10 +157,21 @@ private function loadFailsafeKey(string $type): string
             $this->getFailsafePublicKey() :
             $this->getFailsafePrivateKey();
 
-        $jwks = json_decode(file_get_contents($key), true);
+        // Todo: Remove duplicated code in here and JWKSKeyLoader.
+        $jwksArray = json_decode(file_get_contents($key), true);
+
+        if (false === array_key_exists('keys', $jwksArray)) {
+            throw new ApiGwAuthenticationException(
+                sprintf('Invalid JWKS format of %s key at %s.', $type, $key)
+            );
+        }
 
-        $keys = $this->keyConverter->fromJWKStoPEMS($jwks['keys']);
+        if ([] === $jwksArray['keys']) {
+            throw new ApiGwAuthenticationException(
+                sprintf('Invalid JWKS format of %s key at %s, keys array is empty.', $type, $key)
+            );
+        }
 
-        return current($keys);
+        return current($this->keyConverter->fromJWKStoPEMS($jwksArray['keys']));
     }
 }
diff --git a/src/Service/KeyLoader/JWKSKeyLoader.php b/src/Service/KeyLoader/JWKSKeyLoader.php
@@ -6,8 +6,8 @@
 
 use EcPhp\ApiGwAuthenticationBundle\Exception\ApiGwAuthenticationException;
 use EcPhp\ApiGwAuthenticationBundle\Service\KeyConverter\KeyConverterInterface;
-use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface;
 use Symfony\Contracts\HttpClient\HttpClientInterface;
+use Throwable;
 
 use function array_key_exists;
 
@@ -44,15 +44,19 @@ public function getSigningKey(): string
         return $this->keyLoader->getSigningKey();
     }
 
-    public function loadKey($type)
+    public function loadKey($type): string
     {
-        // @Todo: Implements for PRIVATE key as well.
+        // Todo: Implements for PRIVATE key as well.
         $key = $this->keyLoader->getPublicKey();
 
         try {
             $jwks = $this->httpClient->request('GET', $key);
-        } catch (TransportExceptionInterface $e) {
-            throw $e;
+        } catch (Throwable $e) {
+            throw new ApiGwAuthenticationException(
+                sprintf('Unable to request uri(%s) for %s key.', $key, $type),
+                $e->getCode(),
+                $e
+            );
         }
 
         if (200 !== $statusCode = $jwks->getStatusCode()) {