Merge pull request #38 from packbackbooks/no-file-get-contents

PODB-266 Get the public keyset via a client instead of file_get_contents
packbackbooks · Nov 23, 2021 · c7b45e1 · c7b45e1
2 parents c6b32db + cac5891
commit c7b45e1
Show file tree

Hide file tree

Showing 9 changed files with 121 additions and 35 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,8 @@
+## 4.1.1
+
+* Updated `LtiMessageLaunch` to fetch JWKs via an HTTP client instead of `file_get_contents()`. ([#38](https://github.com/packbackbooks/lti-1-3-php-library/pull/38))
+* Added new methods to `ILtiServiceConnector`: `makeRequest()`, `getRequestBody()`. ([#38](https://github.com/packbackbooks/lti-1-3-php-library/pull/38))
+
 ## 4.1.0
 
 * Allowed `getGrades()` to be called without a line item. ([#34](https://github.com/packbackbooks/lti-1-3-php-library/pull/34))
@@ -10,7 +15,7 @@
 
 ## 3.0.3
 
-* Added response/request logging to LtiServiceConnector. ([#31](https://github.com/packbackbooks/lti-1-3-php-library/pull/31))
+* Added response/request logging to `LtiServiceConnector`. ([#31](https://github.com/packbackbooks/lti-1-3-php-library/pull/31))
 
 Note: Upgrade to 4.0.0 for this logging to be disabled by default.
 

diff --git a/UPGRADES.md b/UPGRADES.md
@@ -1,20 +1,24 @@
 ## 3.0 to 4.0
 
-### New method implemented on the `ILtiServiceConnector`
+### New methods implemented on the `ILtiServiceConnector`
 
-Version 4.0 introduced changes to the `Packback\Lti1p3\Interfaces\ILtiServiceConnector` interface, adding one method: `setDebuggingMode()`.
+Version 4.0 introduced changes to the `Packback\Lti1p3\Interfaces\ILtiServiceConnector` interface, adding the following methods:
+
+- `setDebuggingMode()`
+- `makeRequest()`
+- `getRequestBody()`
 
 ## 2.0 to 3.0
 
 ### New method implemented on the `ICache`
 
 Version 3.0 introduced changes to the `Packback\Lti1p3\Interfaces\ICache` interface, adding one method: `clearAccessToken()`. This method must be implemented to any custom implementations of the interface. The [Laravel Implementation Guide](https://github.com/packbackbooks/lti-1-3-php-library/wiki/Laravel-Implementation-Guide#cache) contains an example.
 
-### Using GuzzleHttp\Client instead of curl
+### Using `GuzzleHttp\Client` instead of curl
 
 The `Packback\Lti1p3\LtiServiceConnector` now uses Guzzle instead of curl to make requests. This puts control of this client and its configuration in the hands of the developer. The section below contains information on implementing this change.
 
-### Changes to the LtiServiceConnector and LTI services
+### Changes to the `LtiServiceConnector` and LTI services
 
 The implementation of the `Packback\Lti1p3\LtiServiceConnector` changed to act as a general API Client for the various LTI service (Assignment Grades, Names Roles Provisioning, etc.) Specifically, the constructor for the following classes now accept different arguments:
 

diff --git a/src/Interfaces/ILtiServiceConnector.php b/src/Interfaces/ILtiServiceConnector.php
@@ -2,10 +2,16 @@
 
 namespace Packback\Lti1p3\Interfaces;
 
+use GuzzleHttp\Psr7\Response;
+
 interface ILtiServiceConnector
 {
     public function getAccessToken(ILtiRegistration $registration, array $scopes);
 
+    public function makeRequest(IServiceRequest $request);
+
+    public function getResponseBody(Response $request): array;
+
     public function makeServiceRequest(
         ILtiRegistration $registration,
         array $scopes,

diff --git a/src/LtiMessageLaunch.php b/src/LtiMessageLaunch.php
@@ -6,6 +6,7 @@
 use Firebase\JWT\JWK;
 use Firebase\JWT\JWT;
 use GuzzleHttp\Client;
+use GuzzleHttp\Exception\TransferException;
 use Packback\Lti1p3\Interfaces\ICache;
 use Packback\Lti1p3\Interfaces\ICookie;
 use Packback\Lti1p3\Interfaces\IDatabase;
@@ -265,18 +266,24 @@ public function getLaunchId()
 
     private function getPublicKey()
     {
-        $key_set_url = $this->registration->getKeySetUrl();
+        $keySetUrl = $this->registration->getKeySetUrl();
+        $request = new ServiceRequest(LtiServiceConnector::METHOD_GET, $keySetUrl);
 
         // Download key set
-        $public_key_set = json_decode(file_get_contents($key_set_url), true);
+        try {
+            $response = $this->serviceConnector->makeRequest($request);
+        } catch (TransferException $e) {
+            throw new LtiException(static::ERR_NO_PUBLIC_KEY);
+        }
+        $publicKeySet = $this->serviceConnector->getResponseBody($response);
 
-        if (empty($public_key_set)) {
+        if (empty($publicKeySet)) {
             // Failed to fetch public keyset from URL.
             throw new LtiException(static::ERR_FETCH_PUBLIC_KEY);
         }
 
         // Find key used to sign the JWT (matches the KID in the header)
-        foreach ($public_key_set['keys'] as $key) {
+        foreach ($publicKeySet['keys'] as $key) {
             if ($key['kid'] == $this->jwt['header']['kid']) {
                 try {
                     return openssl_pkey_get_details(

diff --git a/src/LtiServiceConnector.php b/src/LtiServiceConnector.php
@@ -5,6 +5,7 @@
 use Firebase\JWT\JWT;
 use GuzzleHttp\Client;
 use GuzzleHttp\Exception\ClientException;
+use GuzzleHttp\Psr7\Response;
 use Packback\Lti1p3\Interfaces\ICache;
 use Packback\Lti1p3\Interfaces\ILtiRegistration;
 use Packback\Lti1p3\Interfaces\ILtiServiceConnector;
@@ -71,15 +72,30 @@ public function getAccessToken(ILtiRegistration $registration, array $scopes)
             'form_params' => $authRequest,
         ]);
 
-        $body = (string) $response->getBody();
-        $tokenData = json_decode($body, true);
+        $tokenData = $this->getResponseBody($response);
 
         // Cache access token
         $this->cache->cacheAccessToken($accessTokenKey, $tokenData['access_token']);
 
         return $tokenData['access_token'];
     }
 
+    public function makeRequest(IServiceRequest $request)
+    {
+        return $this->client->request(
+            $request->getMethod(),
+            $request->getUrl(),
+            $request->getPayload()
+        );
+    }
+
+    public function getResponseBody(Response $response): array
+    {
+        $responseBody = (string) $response->getBody();
+
+        return json_decode($responseBody, true);
+    }
+
     public function makeServiceRequest(
         ILtiRegistration $registration,
         array $scopes,
@@ -89,11 +105,7 @@ public function makeServiceRequest(
         $request->setAccessToken($this->getAccessToken($registration, $scopes));
 
         try {
-            $response = $this->client->request(
-                $request->getMethod(),
-                $request->getUrl(),
-                $request->getPayload()
-            );
+            $response = $this->makeRequest($request);
         } catch (ClientException $e) {
             $status = $e->getResponse()->getStatusCode();
 
@@ -108,26 +120,26 @@ public function makeServiceRequest(
 
             throw $e;
         }
-        $respHeaders = $response->getHeaders();
-        array_walk($respHeaders, function (&$value) {
+        $responseHeaders = $response->getHeaders();
+        array_walk($responseHeaders, function (&$value) {
             $value = $value[0];
         });
-        $respBody = $response->getBody();
+        $responseBody = $this->getResponseBody($response);
 
         if ($this->debuggingMode) {
             error_log('Syncing grade for this lti_user_id: '.
                 json_decode($request->getPayload()['body'])->userId.' '.print_r([
                     'request_method' => $request->getMethod(),
                     'request_url' => $request->getUrl(),
                     'request_body' => $request->getPayload()['body'],
-                    'response_headers' => $respHeaders,
-                    'response_body' => (string) $respBody,
+                    'response_headers' => $responseHeaders,
+                    'response_body' => json_encode($responseBody),
                 ], true));
         }
 
         return [
-            'headers' => $respHeaders,
-            'body' => json_decode($respBody, true),
+            'headers' => $responseHeaders,
+            'body' => $responseBody,
             'status' => $response->getStatusCode(),
         ];
     }

diff --git a/src/ServiceRequest.php b/src/ServiceRequest.php
@@ -6,11 +6,12 @@
 
 class ServiceRequest implements IServiceRequest
 {
-    public $method;
-    public $url;
-    public $body;
-    public $contentType = 'application/json';
-    public $accept = 'application/json';
+    private $method;
+    private $url;
+    private $body;
+    private $accessToken;
+    private $contentType = 'application/json';
+    private $accept = 'application/json';
 
     public function __construct(string $method, string $url)
     {
@@ -80,10 +81,13 @@ public function setContentType(string $contentType): IServiceRequest
     private function getHeaders(): array
     {
         $headers = [
-            'Authorization' => $this->accessToken,
             'Accept' => $this->accept,
         ];
 
+        if (isset($this->accessToken)) {
+            $headers['Authorization'] = $this->accessToken;
+        }
+
         if ($this->getMethod() === LtiServiceConnector::METHOD_POST) {
             $headers['Content-Type'] = $this->contentType;
         }

diff --git a/tests/Certification/Lti13CertificationTest.php b/tests/Certification/Lti13CertificationTest.php
@@ -4,9 +4,12 @@
 
 use Carbon\Carbon;
 use Firebase\JWT\JWT;
+use GuzzleHttp\Psr7\Response;
+use Mockery;
 use Packback\Lti1p3\Interfaces\ICache;
 use Packback\Lti1p3\Interfaces\ICookie;
 use Packback\Lti1p3\Interfaces\IDatabase;
+use Packback\Lti1p3\Interfaces\ILtiServiceConnector;
 use Packback\Lti1p3\JwksEndpoint;
 use Packback\Lti1p3\LtiConstants;
 use Packback\Lti1p3\LtiDeployment;
@@ -217,6 +220,7 @@ public function setUp(): void
             LtiOidcLogin::COOKIE_PREFIX.static::STATE,
             static::STATE
         );
+        $this->serviceConnector = Mockery::mock(ILtiServiceConnector::class);
     }
 
     public function buildJWT($data, $header)
@@ -345,6 +349,15 @@ public function testInvalidCertificationCases()
         $casesCount = count($testCases);
         $testedCases = 0;
 
+        $request = Mockery::mock(Response::class);
+        $this->serviceConnector->shouldReceive('makeRequest')
+            // All but one invalid cert case get the JWK
+            ->times($casesCount - 1)
+            ->andReturn($request);
+        $this->serviceConnector->shouldReceive('getResponseBody')
+            ->times($casesCount - 1)
+            ->andReturn(json_decode(file_get_contents(static::JWKS_FILE), true));
+
         foreach ($testCases as $testCase) {
             $testCaseDir = $testCasesDir.DIRECTORY_SEPARATOR.$testCase.DIRECTORY_SEPARATOR;
 
@@ -391,7 +404,7 @@ public function testInvalidCertificationCases()
             ];
 
             try {
-                LtiMessageLaunch::new($this->db, $this->cache, $this->cookie)
+                LtiMessageLaunch::new($this->db, $this->cache, $this->cookie, $this->serviceConnector)
                     ->validate($params);
             } catch (\Exception $e) {
                 $this->assertInstanceOf(LtiException::class, $e);
@@ -440,7 +453,13 @@ public function testValidCertificationCases()
                 'state' => static::STATE,
             ];
 
-            $result = LtiMessageLaunch::new($this->db, $this->cache, $this->cookie)
+            $request = Mockery::mock(Response::class);
+            $this->serviceConnector->shouldReceive('makeRequest')
+                ->once()->andReturn($request);
+            $this->serviceConnector->shouldReceive('getResponseBody')
+                ->once()->andReturn(json_decode(file_get_contents(static::JWKS_FILE), true));
+
+            $result = LtiMessageLaunch::new($this->db, $this->cache, $this->cookie, $this->serviceConnector)
                 ->validate($params);
 
             // Assertions
@@ -474,7 +493,13 @@ private function launch($payload)
             'state' => static::STATE,
         ];
 
-        return LtiMessageLaunch::new($this->db, $this->cache, $this->cookie)
+        $request = Mockery::mock(Response::class);
+        $this->serviceConnector->shouldReceive('makeRequest')
+            ->once()->andReturn($request);
+        $this->serviceConnector->shouldReceive('getResponseBody')
+            ->once()->andReturn(json_decode(file_get_contents(static::JWKS_FILE), true));
+
+        return LtiMessageLaunch::new($this->db, $this->cache, $this->cookie, $this->serviceConnector)
             ->validate($params);
     }
 }
diff --git a/tests/LtiMessageLaunchTest.php b/tests/LtiMessageLaunchTest.php
@@ -4,6 +4,7 @@
 
 use Carbon\Carbon;
 use Firebase\JWT\JWT;
+use GuzzleHttp\Psr7\Response;
 use Mockery;
 use Packback\Lti1p3\Interfaces\ICache;
 use Packback\Lti1p3\Interfaces\ICookie;
@@ -179,6 +180,10 @@ public function testItValidatesALaunch()
             ->once()->andReturn($this->issuer['client_id']);
         $this->registration->shouldReceive('getKeySetUrl')
             ->once()->andReturn($this->issuer['key_set_url']);
+        $this->serviceConnector->shouldReceive('makeRequest')
+            ->once()->andReturn(Mockery::mock(Response::class));
+        $this->serviceConnector->shouldReceive('getResponseBody')
+            ->once()->andReturn(json_decode(file_get_contents(static::JWKS_FILE), true));
         $this->database->shouldReceive('findDeployment')
             ->once()->andReturn(['a deployment']);
         $this->cache->shouldReceive('cacheLaunchData')
@@ -359,6 +364,10 @@ public function testALaunchFailsIfDeploymentIdIsMissing()
             ->once()->andReturn($this->payload['aud']);
         $this->registration->shouldReceive('getKeySetUrl')
             ->once()->andReturn($this->issuer['key_set_url']);
+        $this->serviceConnector->shouldReceive('makeRequest')
+            ->once()->andReturn(Mockery::mock(Response::class));
+        $this->serviceConnector->shouldReceive('getResponseBody')
+            ->once()->andReturn(json_decode(file_get_contents(static::JWKS_FILE), true));
 
         $this->expectException(LtiException::class);
         $this->expectExceptionMessage(LtiMessageLaunch::ERR_MISSING_DEPLOYEMENT_ID);
@@ -384,6 +393,10 @@ public function testALaunchFailsIfNoDeployment()
             ->once()->andReturn($this->payload['aud']);
         $this->registration->shouldReceive('getKeySetUrl')
             ->once()->andReturn($this->issuer['key_set_url']);
+        $this->serviceConnector->shouldReceive('makeRequest')
+            ->once()->andReturn(Mockery::mock(Response::class));
+        $this->serviceConnector->shouldReceive('getResponseBody')
+            ->once()->andReturn(json_decode(file_get_contents(static::JWKS_FILE), true));
         $this->database->shouldReceive('findDeployment')
             ->once()->andReturn();