From 7cfc07017783d425ad7f9a72e834e10c8e9949b7 Mon Sep 17 00:00:00 2001 From: Davo Date: Tue, 9 Nov 2021 15:54:02 -0600 Subject: [PATCH 01/13] PODB-266 Get JWK via http client --- src/Interfaces/ILtiServiceConnector.php | 4 ++++ src/LtiMessageLaunch.php | 9 +++++---- src/LtiServiceConnector.php | 15 +++++++++----- .../Certification/Lti13CertificationTest.php | 20 ++++++++++++++++--- tests/LtiMessageLaunchTest.php | 6 ++++++ 5 files changed, 42 insertions(+), 12 deletions(-) diff --git a/src/Interfaces/ILtiServiceConnector.php b/src/Interfaces/ILtiServiceConnector.php index 97ccca64..cbb3bae7 100644 --- a/src/Interfaces/ILtiServiceConnector.php +++ b/src/Interfaces/ILtiServiceConnector.php @@ -6,6 +6,10 @@ interface ILtiServiceConnector { public function getAccessToken(ILtiRegistration $registration, array $scopes); + public function makeRequest( + IServiceRequest $request + ); + public function makeServiceRequest( ILtiRegistration $registration, array $scopes, diff --git a/src/LtiMessageLaunch.php b/src/LtiMessageLaunch.php index 07161bc0..654bed85 100644 --- a/src/LtiMessageLaunch.php +++ b/src/LtiMessageLaunch.php @@ -265,18 +265,19 @@ 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); + $publicKeySet = $this->serviceConnector->makeRequest($request); - 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 index 719aefb3..3e335aa0 100644 --- a/src/LtiServiceConnector.php +++ b/src/LtiServiceConnector.php @@ -80,6 +80,15 @@ public function getAccessToken(ILtiRegistration $registration, array $scopes) return $tokenData['access_token']; } + public function makeRequest(IServiceRequest $request) + { + return $this->client->request( + $request->getMethod(), + $request->getUrl(), + $request->getPayload() + ); + } + public function makeServiceRequest( ILtiRegistration $registration, array $scopes, @@ -89,11 +98,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(); diff --git a/tests/Certification/Lti13CertificationTest.php b/tests/Certification/Lti13CertificationTest.php index 727cd03f..3d831d7d 100644 --- a/tests/Certification/Lti13CertificationTest.php +++ b/tests/Certification/Lti13CertificationTest.php @@ -4,9 +4,11 @@ use Carbon\Carbon; use Firebase\JWT\JWT; +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 +219,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 +348,11 @@ public function testInvalidCertificationCases() $casesCount = count($testCases); $testedCases = 0; + $this->serviceConnector->shouldReceive('makeRequest') + // All but one invalid cert case get the JWK + ->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 +399,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 +448,10 @@ public function testValidCertificationCases() 'state' => static::STATE, ]; - $result = LtiMessageLaunch::new($this->db, $this->cache, $this->cookie) + $this->serviceConnector->shouldReceive('makeRequest') + ->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 +485,10 @@ private function launch($payload) 'state' => static::STATE, ]; - return LtiMessageLaunch::new($this->db, $this->cache, $this->cookie) + $this->serviceConnector->shouldReceive('makeRequest') + ->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 index f45dcea7..f7cc69b7 100644 --- a/tests/LtiMessageLaunchTest.php +++ b/tests/LtiMessageLaunchTest.php @@ -179,6 +179,8 @@ 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(json_decode(file_get_contents(static::JWKS_FILE), true)); $this->database->shouldReceive('findDeployment') ->once()->andReturn(['a deployment']); $this->cache->shouldReceive('cacheLaunchData') @@ -359,6 +361,8 @@ 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(json_decode(file_get_contents(static::JWKS_FILE), true)); $this->expectException(LtiException::class); $this->expectExceptionMessage(LtiMessageLaunch::ERR_MISSING_DEPLOYEMENT_ID); @@ -384,6 +388,8 @@ 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(json_decode(file_get_contents(static::JWKS_FILE), true)); $this->database->shouldReceive('findDeployment') ->once()->andReturn(); From f5c5df346b255b9b9040e36d7ae1966c8af6a9f3 Mon Sep 17 00:00:00 2001 From: Davo Date: Wed, 10 Nov 2021 10:26:25 -0600 Subject: [PATCH 02/13] PODB-266 Update changelog and upgrades --- CHANGELOG.md | 7 ++++++- UPGRADES.md | 5 ++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d8e0f5ad..0325763e 100644 --- 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 a new method to `ILtiServiceConnector`: `makeRequest()`. ([#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 index 8cfa080c..1a377b5a 100644 --- a/UPGRADES.md +++ b/UPGRADES.md @@ -2,7 +2,10 @@ ### New method 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, the following methods: + +- `setDebuggingMode()` +- `makeRequest()` ## 2.0 to 3.0 From 53682c390d2e9c464b6edfe223b4ff3bd71bd19e Mon Sep 17 00:00:00 2001 From: Davo Date: Wed, 10 Nov 2021 10:35:01 -0600 Subject: [PATCH 03/13] PODB-266 typo --- UPGRADES.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/UPGRADES.md b/UPGRADES.md index 1a377b5a..72231f6c 100644 --- a/UPGRADES.md +++ b/UPGRADES.md @@ -1,8 +1,8 @@ ## 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, the following methods: +Version 4.0 introduced changes to the `Packback\Lti1p3\Interfaces\ILtiServiceConnector` interface, adding the following methods: - `setDebuggingMode()` - `makeRequest()` From 9988691f8531b4f3bc05d2e4550d36167900bf9a Mon Sep 17 00:00:00 2001 From: Davo Date: Wed, 10 Nov 2021 10:35:26 -0600 Subject: [PATCH 04/13] PODB-266 prettify --- UPGRADES.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/UPGRADES.md b/UPGRADES.md index 72231f6c..dbd88c09 100644 --- a/UPGRADES.md +++ b/UPGRADES.md @@ -13,11 +13,11 @@ Version 4.0 introduced changes to the `Packback\Lti1p3\Interfaces\ILtiServiceCon 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: From 3cab9e05e27891022556c6438e94f953509c44a7 Mon Sep 17 00:00:00 2001 From: Davo Date: Wed, 10 Nov 2021 10:48:47 -0600 Subject: [PATCH 05/13] PODB-266 fix missing property --- src/ServiceRequest.php | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/ServiceRequest.php b/src/ServiceRequest.php index eae5c5d7..fbb38021 100644 --- 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,9 +81,12 @@ 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; From b807c619189b36d1f8413adf30a2fc5652380150 Mon Sep 17 00:00:00 2001 From: Davo Date: Wed, 10 Nov 2021 10:50:05 -0600 Subject: [PATCH 06/13] PODB-266 lint --- src/ServiceRequest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ServiceRequest.php b/src/ServiceRequest.php index fbb38021..57d6ff3a 100644 --- a/src/ServiceRequest.php +++ b/src/ServiceRequest.php @@ -83,7 +83,7 @@ private function getHeaders(): array $headers = [ 'Accept' => $this->accept, ]; - + if (isset($this->accessToken)) { $headers['Authorization'] = $this->accessToken; } From 950962f908628fe98e7a152513308f9ea0883c7b Mon Sep 17 00:00:00 2001 From: Davo Date: Tue, 16 Nov 2021 17:07:28 -0600 Subject: [PATCH 07/13] PODB-266 convert response to json --- src/LtiMessageLaunch.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/LtiMessageLaunch.php b/src/LtiMessageLaunch.php index 654bed85..384d8e77 100644 --- a/src/LtiMessageLaunch.php +++ b/src/LtiMessageLaunch.php @@ -269,7 +269,8 @@ private function getPublicKey() $request = new ServiceRequest(LtiServiceConnector::METHOD_GET, $keySetUrl); // Download key set - $publicKeySet = $this->serviceConnector->makeRequest($request); + $response = $this->serviceConnector->makeRequest($request); + $publicKeySet = $response->getBody(); if (empty($publicKeySet)) { // Failed to fetch public keyset from URL. From 03b9385a993c38a7a8a50d4ee90107554c707f5d Mon Sep 17 00:00:00 2001 From: Davo Date: Tue, 16 Nov 2021 17:14:26 -0600 Subject: [PATCH 08/13] PODB-266 fix tests --- tests/Certification/Lti13CertificationTest.php | 10 ++++++++++ tests/LtiMessageLaunchTest.php | 9 +++++++++ 2 files changed, 19 insertions(+) diff --git a/tests/Certification/Lti13CertificationTest.php b/tests/Certification/Lti13CertificationTest.php index 3d831d7d..bfc8419d 100644 --- a/tests/Certification/Lti13CertificationTest.php +++ b/tests/Certification/Lti13CertificationTest.php @@ -348,8 +348,12 @@ public function testInvalidCertificationCases() $casesCount = count($testCases); $testedCases = 0; + $request = Mockery::mock(); $this->serviceConnector->shouldReceive('makeRequest') // All but one invalid cert case get the JWK + ->times($casesCount - 1) + ->andReturn($request); + $request->shouldReceive('getBody') ->times($casesCount - 1) ->andReturn(json_decode(file_get_contents(static::JWKS_FILE), true)); @@ -448,7 +452,10 @@ public function testValidCertificationCases() 'state' => static::STATE, ]; + $request = Mockery::mock(); $this->serviceConnector->shouldReceive('makeRequest') + ->once()->andReturn($request); + $request->shouldReceive('getBody') ->once()->andReturn(json_decode(file_get_contents(static::JWKS_FILE), true)); $result = LtiMessageLaunch::new($this->db, $this->cache, $this->cookie, $this->serviceConnector) @@ -485,7 +492,10 @@ private function launch($payload) 'state' => static::STATE, ]; + $request = Mockery::mock(); $this->serviceConnector->shouldReceive('makeRequest') + ->once()->andReturn($request); + $request->shouldReceive('getBody') ->once()->andReturn(json_decode(file_get_contents(static::JWKS_FILE), true)); return LtiMessageLaunch::new($this->db, $this->cache, $this->cookie, $this->serviceConnector) diff --git a/tests/LtiMessageLaunchTest.php b/tests/LtiMessageLaunchTest.php index f7cc69b7..ac227e25 100644 --- a/tests/LtiMessageLaunchTest.php +++ b/tests/LtiMessageLaunchTest.php @@ -179,7 +179,10 @@ public function testItValidatesALaunch() ->once()->andReturn($this->issuer['client_id']); $this->registration->shouldReceive('getKeySetUrl') ->once()->andReturn($this->issuer['key_set_url']); + $request = Mockery::mock(); $this->serviceConnector->shouldReceive('makeRequest') + ->once()->andReturn($request); + $request->shouldReceive('getBody') ->once()->andReturn(json_decode(file_get_contents(static::JWKS_FILE), true)); $this->database->shouldReceive('findDeployment') ->once()->andReturn(['a deployment']); @@ -361,7 +364,10 @@ public function testALaunchFailsIfDeploymentIdIsMissing() ->once()->andReturn($this->payload['aud']); $this->registration->shouldReceive('getKeySetUrl') ->once()->andReturn($this->issuer['key_set_url']); + $request = Mockery::mock(); $this->serviceConnector->shouldReceive('makeRequest') + ->once()->andReturn($request); + $request->shouldReceive('getBody') ->once()->andReturn(json_decode(file_get_contents(static::JWKS_FILE), true)); $this->expectException(LtiException::class); @@ -388,7 +394,10 @@ public function testALaunchFailsIfNoDeployment() ->once()->andReturn($this->payload['aud']); $this->registration->shouldReceive('getKeySetUrl') ->once()->andReturn($this->issuer['key_set_url']); + $request = Mockery::mock(); $this->serviceConnector->shouldReceive('makeRequest') + ->once()->andReturn($request); + $request->shouldReceive('getBody') ->once()->andReturn(json_decode(file_get_contents(static::JWKS_FILE), true)); $this->database->shouldReceive('findDeployment') ->once()->andReturn(); From 1ab0bea13b5339fb401024aa7ae2087b53a4db8e Mon Sep 17 00:00:00 2001 From: Davo Date: Wed, 17 Nov 2021 10:14:17 -0600 Subject: [PATCH 09/13] PODB-266 make a static function to handle decoding the body --- src/LtiMessageLaunch.php | 2 +- src/LtiServiceConnector.php | 24 +++++++++++++++--------- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/LtiMessageLaunch.php b/src/LtiMessageLaunch.php index 384d8e77..a7e3d2d0 100644 --- a/src/LtiMessageLaunch.php +++ b/src/LtiMessageLaunch.php @@ -270,7 +270,7 @@ private function getPublicKey() // Download key set $response = $this->serviceConnector->makeRequest($request); - $publicKeySet = $response->getBody(); + $publicKeySet = LtiServiceConnector::getResponseBody($response); if (empty($publicKeySet)) { // Failed to fetch public keyset from URL. diff --git a/src/LtiServiceConnector.php b/src/LtiServiceConnector.php index 3e335aa0..98af0e05 100644 --- 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,8 +72,7 @@ public function getAccessToken(ILtiRegistration $registration, array $scopes) 'form_params' => $authRequest, ]); - $body = (string) $response->getBody(); - $tokenData = json_decode($body, true); + $tokenData = static::getResponseBody($response); // Cache access token $this->cache->cacheAccessToken($accessTokenKey, $tokenData['access_token']); @@ -89,6 +89,12 @@ public function makeRequest(IServiceRequest $request) ); } + public static function getResponseBody(Response $response): array + { + $responseBody = (string) $response->getBody(); + return json_decode($responseBody, true); + } + public function makeServiceRequest( ILtiRegistration $registration, array $scopes, @@ -113,11 +119,11 @@ 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 = static::getResponseBody($response); if ($this->debuggingMode) { error_log('Syncing grade for this lti_user_id: '. @@ -125,14 +131,14 @@ public function makeServiceRequest( '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(), ]; } From 4442a8e122f2b11e13cbd605ff4e47114959c0ee Mon Sep 17 00:00:00 2001 From: Davo Date: Wed, 17 Nov 2021 11:36:50 -0600 Subject: [PATCH 10/13] PODB-266 wow that was obnoxious. fixing stuff --- src/Interfaces/ILtiServiceConnector.php | 8 +++++--- src/LtiMessageLaunch.php | 9 +++++++-- src/LtiServiceConnector.php | 7 ++++--- tests/Certification/Lti13CertificationTest.php | 13 +++++++------ tests/LtiMessageLaunchTest.php | 16 +++++++--------- tests/LtiServiceConnectorTest.php | 16 +++++++++++++--- 6 files changed, 43 insertions(+), 26 deletions(-) diff --git a/src/Interfaces/ILtiServiceConnector.php b/src/Interfaces/ILtiServiceConnector.php index cbb3bae7..5418d8a9 100644 --- a/src/Interfaces/ILtiServiceConnector.php +++ b/src/Interfaces/ILtiServiceConnector.php @@ -2,13 +2,15 @@ namespace Packback\Lti1p3\Interfaces; +use GuzzleHttp\Psr7\Response; + interface ILtiServiceConnector { public function getAccessToken(ILtiRegistration $registration, array $scopes); - public function makeRequest( - IServiceRequest $request - ); + public function makeRequest(IServiceRequest $request); + + public function getResponseBody(Response $request): array; public function makeServiceRequest( ILtiRegistration $registration, diff --git a/src/LtiMessageLaunch.php b/src/LtiMessageLaunch.php index a7e3d2d0..5ec91e3f 100644 --- 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\ClientException; use Packback\Lti1p3\Interfaces\ICache; use Packback\Lti1p3\Interfaces\ICookie; use Packback\Lti1p3\Interfaces\IDatabase; @@ -269,8 +270,12 @@ private function getPublicKey() $request = new ServiceRequest(LtiServiceConnector::METHOD_GET, $keySetUrl); // Download key set - $response = $this->serviceConnector->makeRequest($request); - $publicKeySet = LtiServiceConnector::getResponseBody($response); + try { + $response = $this->serviceConnector->makeRequest($request); + } catch (ClientException $e) { + throw new LtiException(static::ERR_NO_PUBLIC_KEY); + } + $publicKeySet = $this->serviceConnector->getResponseBody($response); if (empty($publicKeySet)) { // Failed to fetch public keyset from URL. diff --git a/src/LtiServiceConnector.php b/src/LtiServiceConnector.php index 98af0e05..99e0a90d 100644 --- a/src/LtiServiceConnector.php +++ b/src/LtiServiceConnector.php @@ -72,7 +72,7 @@ public function getAccessToken(ILtiRegistration $registration, array $scopes) 'form_params' => $authRequest, ]); - $tokenData = static::getResponseBody($response); + $tokenData = $this->getResponseBody($response); // Cache access token $this->cache->cacheAccessToken($accessTokenKey, $tokenData['access_token']); @@ -89,9 +89,10 @@ public function makeRequest(IServiceRequest $request) ); } - public static function getResponseBody(Response $response): array + public function getResponseBody(Response $response): array { $responseBody = (string) $response->getBody(); + return json_decode($responseBody, true); } @@ -123,7 +124,7 @@ public function makeServiceRequest( array_walk($responseHeaders, function (&$value) { $value = $value[0]; }); - $responseBody = static::getResponseBody($response); + $responseBody = $this->getResponseBody($response); if ($this->debuggingMode) { error_log('Syncing grade for this lti_user_id: '. diff --git a/tests/Certification/Lti13CertificationTest.php b/tests/Certification/Lti13CertificationTest.php index bfc8419d..a3191936 100644 --- a/tests/Certification/Lti13CertificationTest.php +++ b/tests/Certification/Lti13CertificationTest.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; @@ -348,12 +349,12 @@ public function testInvalidCertificationCases() $casesCount = count($testCases); $testedCases = 0; - $request = Mockery::mock(); + $request = Mockery::mock(Response::class); $this->serviceConnector->shouldReceive('makeRequest') // All but one invalid cert case get the JWK ->times($casesCount - 1) ->andReturn($request); - $request->shouldReceive('getBody') + $this->serviceConnector->shouldReceive('getResponseBody') ->times($casesCount - 1) ->andReturn(json_decode(file_get_contents(static::JWKS_FILE), true)); @@ -452,10 +453,10 @@ public function testValidCertificationCases() 'state' => static::STATE, ]; - $request = Mockery::mock(); + $request = Mockery::mock(Response::class); $this->serviceConnector->shouldReceive('makeRequest') ->once()->andReturn($request); - $request->shouldReceive('getBody') + $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) @@ -492,10 +493,10 @@ private function launch($payload) 'state' => static::STATE, ]; - $request = Mockery::mock(); + $request = Mockery::mock(Response::class); $this->serviceConnector->shouldReceive('makeRequest') ->once()->andReturn($request); - $request->shouldReceive('getBody') + $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) diff --git a/tests/LtiMessageLaunchTest.php b/tests/LtiMessageLaunchTest.php index ac227e25..728c663f 100644 --- 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,10 +180,9 @@ public function testItValidatesALaunch() ->once()->andReturn($this->issuer['client_id']); $this->registration->shouldReceive('getKeySetUrl') ->once()->andReturn($this->issuer['key_set_url']); - $request = Mockery::mock(); $this->serviceConnector->shouldReceive('makeRequest') - ->once()->andReturn($request); - $request->shouldReceive('getBody') + ->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']); @@ -364,10 +364,9 @@ public function testALaunchFailsIfDeploymentIdIsMissing() ->once()->andReturn($this->payload['aud']); $this->registration->shouldReceive('getKeySetUrl') ->once()->andReturn($this->issuer['key_set_url']); - $request = Mockery::mock(); $this->serviceConnector->shouldReceive('makeRequest') - ->once()->andReturn($request); - $request->shouldReceive('getBody') + ->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); @@ -394,10 +393,9 @@ public function testALaunchFailsIfNoDeployment() ->once()->andReturn($this->payload['aud']); $this->registration->shouldReceive('getKeySetUrl') ->once()->andReturn($this->issuer['key_set_url']); - $request = Mockery::mock(); $this->serviceConnector->shouldReceive('makeRequest') - ->once()->andReturn($request); - $request->shouldReceive('getBody') + ->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(); diff --git a/tests/LtiServiceConnectorTest.php b/tests/LtiServiceConnectorTest.php index 3383e8ab..bab58c2c 100644 --- a/tests/LtiServiceConnectorTest.php +++ b/tests/LtiServiceConnectorTest.php @@ -4,13 +4,14 @@ use GuzzleHttp\Client; use GuzzleHttp\Exception\ClientException; +use GuzzleHttp\Psr7\Response; use Mockery; use Packback\Lti1p3\Interfaces\ICache; use Packback\Lti1p3\Interfaces\ILtiRegistration; use Packback\Lti1p3\Interfaces\IServiceRequest; use Packback\Lti1p3\LtiRegistration; use Packback\Lti1p3\LtiServiceConnector; -use Psr\Http\Message\ResponseInterface; +use Psr\Http\Message\StreamInterface; class LtiServiceConnectorTest extends TestCase { @@ -41,7 +42,8 @@ public function setUp(): void $this->request = Mockery::mock(IServiceRequest::class); $this->cache = Mockery::mock(ICache::class); $this->client = Mockery::mock(Client::class); - $this->response = Mockery::mock(ResponseInterface::class); + $this->response = Mockery::mock(Response::class); + $this->streamInterface = Mockery::mock(StreamInterface::class); $this->scopes = ['scopeKey']; $this->token = 'TokenOfAccess'; @@ -98,6 +100,8 @@ public function testItGetsNewAccessToken() $this->client->shouldReceive('post') ->once()->andReturn($this->response); $this->response->shouldReceive('getBody') + ->once()->andReturn($this->streamInterface); + $this->streamInterface->shouldReceive('__toString') ->once()->andReturn(json_encode(['access_token' => $this->token])); $this->cache->shouldReceive('cacheAccessToken')->once(); @@ -128,6 +132,8 @@ public function testItMakesAServiceRequest() $this->response->shouldReceive('getHeaders') ->once()->andReturn($this->responseHeaders); $this->response->shouldReceive('getBody') + ->once()->andReturn($this->streamInterface); + $this->streamInterface->shouldReceive('__toString') ->once()->andReturn(json_encode($this->responseBody)); $this->response->shouldReceive('getStatusCode') ->once()->andReturn($this->responseStatus); @@ -197,6 +203,8 @@ public function testItRetriesServiceRequestOn401Error() $this->response->shouldReceive('getHeaders') ->once()->andReturn($this->responseHeaders); $this->response->shouldReceive('getBody') + ->once()->andReturn($this->streamInterface); + $this->streamInterface->shouldReceive('__toString') ->once()->andReturn(json_encode($this->responseBody)); $this->response->shouldReceive('getStatusCode') ->once()->andReturn($this->responseStatus); @@ -301,6 +309,8 @@ public function testItGetsAll() ->with($method, $this->url, $this->requestPayload) ->twice()->andReturn($this->response); $this->response->shouldReceive('getBody') + ->twice()->andReturn($this->streamInterface); + $this->streamInterface->shouldReceive('__toString') ->twice()->andReturn($responseBody); $this->response->shouldReceive('getStatusCode') ->twice()->andReturn($this->responseStatus); @@ -330,7 +340,7 @@ private function mockMakeRequest() private function mockRequestReturnsA401() { $mockError = Mockery::mock(ClientException::class); - $mockResponse = Mockery::mock(ResponseInterface::class); + $mockResponse = Mockery::mock(Response::class); $mockError->shouldReceive('getResponse') ->once()->andReturn($mockResponse); $mockResponse->shouldReceive('getStatusCode') From 9e70eda6011bf11b2d7144edf0520c4a8f2270d0 Mon Sep 17 00:00:00 2001 From: Davo Date: Wed, 17 Nov 2021 14:47:05 -0600 Subject: [PATCH 11/13] PODB-266 update docs --- CHANGELOG.md | 2 +- UPGRADES.md | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0325763e..489e435d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,7 @@ ## 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 a new method to `ILtiServiceConnector`: `makeRequest()`. ([#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 diff --git a/UPGRADES.md b/UPGRADES.md index dbd88c09..04062e2d 100644 --- a/UPGRADES.md +++ b/UPGRADES.md @@ -6,6 +6,7 @@ Version 4.0 introduced changes to the `Packback\Lti1p3\Interfaces\ILtiServiceCon - `setDebuggingMode()` - `makeRequest()` +- `getRequestBody()` ## 2.0 to 3.0 From 7cf52d36e20767d092a756efb0211700d12aceae Mon Sep 17 00:00:00 2001 From: Davo Date: Tue, 23 Nov 2021 13:05:24 -0600 Subject: [PATCH 12/13] PODB-266 Change exception type --- src/LtiMessageLaunch.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/LtiMessageLaunch.php b/src/LtiMessageLaunch.php index 5ec91e3f..25b6e03d 100644 --- a/src/LtiMessageLaunch.php +++ b/src/LtiMessageLaunch.php @@ -6,7 +6,7 @@ use Firebase\JWT\JWK; use Firebase\JWT\JWT; use GuzzleHttp\Client; -use GuzzleHttp\Exception\ClientException; +use GuzzleHttp\Exception\TransferException; use Packback\Lti1p3\Interfaces\ICache; use Packback\Lti1p3\Interfaces\ICookie; use Packback\Lti1p3\Interfaces\IDatabase; From cac58911683559cc27454cddc51e9233a67072bf Mon Sep 17 00:00:00 2001 From: Davo Date: Tue, 23 Nov 2021 13:05:52 -0600 Subject: [PATCH 13/13] PODB-266 Change exception type --- src/LtiMessageLaunch.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/LtiMessageLaunch.php b/src/LtiMessageLaunch.php index 25b6e03d..c532d8c4 100644 --- a/src/LtiMessageLaunch.php +++ b/src/LtiMessageLaunch.php @@ -272,7 +272,7 @@ private function getPublicKey() // Download key set try { $response = $this->serviceConnector->makeRequest($request); - } catch (ClientException $e) { + } catch (TransferException $e) { throw new LtiException(static::ERR_NO_PUBLIC_KEY); } $publicKeySet = $this->serviceConnector->getResponseBody($response);