diff --git a/src/ImsStorage/ImsCache.php b/src/ImsStorage/ImsCache.php index fd2797ed..96b44bd1 100644 --- a/src/ImsStorage/ImsCache.php +++ b/src/ImsStorage/ImsCache.php @@ -56,6 +56,15 @@ public function getAccessToken($key) return $this->cache[$key]; } + public function clearAccessToken($key) + { + $this->loadCache(); + unset($this->cache[$key]); + $this->saveCache(); + + return $this->cache; + } + private function loadCache() { $cache = file_get_contents(sys_get_temp_dir().'/lti_cache.txt'); diff --git a/src/Interfaces/ICache.php b/src/Interfaces/ICache.php index 5089913c..0f42a0c7 100644 --- a/src/Interfaces/ICache.php +++ b/src/Interfaces/ICache.php @@ -15,4 +15,6 @@ public function checkNonce($nonce); public function cacheAccessToken($key, $accessToken); public function getAccessToken($key); + + public function clearAccessToken($key); } diff --git a/src/LtiServiceConnector.php b/src/LtiServiceConnector.php index aa252573..ef90e41a 100644 --- a/src/LtiServiceConnector.php +++ b/src/LtiServiceConnector.php @@ -4,6 +4,7 @@ use Firebase\JWT\JWT; use GuzzleHttp\Client; +use GuzzleHttp\Exception\ClientException; use Packback\Lti1p3\Interfaces\ICache; use Packback\Lti1p3\Interfaces\ILtiRegistration; use Packback\Lti1p3\Interfaces\ILtiServiceConnector; @@ -75,28 +76,49 @@ public function getAccessToken(array $scopes) return $tokenData['access_token']; } - public function makeServiceRequest(array $scopes, string $method, string $url, string $body = null, $contentType = 'application/json', $accept = 'application/json') - { + public function makeServiceRequest( + array $scopes, + string $method, + string $url, + string $body = null, + $contentType = 'application/json', + $accept = 'application/json', + bool $shouldRetry = true + ) { $headers = [ 'Authorization' => 'Bearer '.$this->getAccessToken($scopes), 'Accept' => $accept, ]; - switch (strtoupper($method)) { - case 'POST': - $headers = array_merge($headers, ['Content-Type' => $contentType]); - $response = $this->client->request($method, $url, [ - 'headers' => $headers, - 'body' => $body, - ]); - break; - default: - $response = $this->client->request($method, $url, [ - 'headers' => $headers, - ]); - break; + try { + switch (strtoupper($method)) { + case 'POST': + $headers = array_merge($headers, ['Content-Type' => $contentType]); + $response = $this->client->request($method, $url, [ + 'headers' => $headers, + 'body' => $body, + ]); + break; + default: + $response = $this->client->request($method, $url, [ + 'headers' => $headers, + ]); + break; + } + } catch (ClientException $e) { + $status = $e->getResponse()->getStatusCode(); + + // If the error was due to invalid authentication and the request + // should be retried, clear the access token and retry it. + if ($status === 401 && $shouldRetry) { + $key = $this->getAccessTokenCacheKey($scopes); + $this->cache->clearAccessToken($key); + + return $this->makeServiceRequest($scopes, $method, $url, $body, $contentType, $accept, false); + } + + throw $e; } - $respHeaders = $response->getHeaders(); array_walk($respHeaders, function (&$value) { $value = $value[0]; @@ -104,9 +126,9 @@ public function makeServiceRequest(array $scopes, string $method, string $url, s $respBody = $response->getBody(); return [ - 'headers' => $respHeaders, - 'body' => json_decode($respBody, true), - ]; + 'headers' => $respHeaders, + 'body' => json_decode($respBody, true), + ]; } private function getAccessTokenCacheKey(array $scopes) diff --git a/tests/Certification/Lti13CertificationTest.php b/tests/Certification/Lti13CertificationTest.php index e139be0e..5ad22a61 100644 --- a/tests/Certification/Lti13CertificationTest.php +++ b/tests/Certification/Lti13CertificationTest.php @@ -54,6 +54,13 @@ public function getAccessToken($key) { return $this->launchData[$key] ?? null; } + + public function clearAccessToken($key) + { + $this->launchData[$key] = null; + + return $this->launchData; + } } class TestCookie implements ICookie diff --git a/tests/LtiServiceConnectorTest.php b/tests/LtiServiceConnectorTest.php index db6c4dc0..1f6d2119 100644 --- a/tests/LtiServiceConnectorTest.php +++ b/tests/LtiServiceConnectorTest.php @@ -3,6 +3,7 @@ namespace Tests; use GuzzleHttp\Client; +use GuzzleHttp\Exception\ClientException; use Mockery; use Packback\Lti1p3\Interfaces\ICache; use Packback\Lti1p3\Interfaces\ILtiRegistration; @@ -166,6 +167,110 @@ public function testItMakesDefaultServiceRequest() $this->assertEquals($expected, $result); } + public function testItRetriesServiceRequestOn401Error() + { + $scopes = ['scopeKey']; + $method = 'post'; + $url = 'https://example.com'; + $body = json_encode(['post' => 'body']); + $requestHeaders = [ + 'Authorization' => 'Bearer '.$this->token, + 'Accept' => 'application/json', + 'Content-Type' => 'application/json', + ]; + $responseHeaders = [ + 'Content-Type' => ['application/json'], + 'Server' => ['nginx'], + ]; + $responseBody = ['some' => 'response']; + $expected = [ + 'headers' => [ + 'Content-Type' => 'application/json', + 'Server' => 'nginx', + ], + 'body' => $responseBody, + ]; + + $this->mockCacheHasAccessToken(); + + // Mock the response failing on the first request + $mockError = Mockery::mock(ClientException::class); + $mockResponse = Mockery::mock(ResponseInterface::class); + $this->client->shouldReceive('request') + ->with($method, $url, [ + 'headers' => $requestHeaders, + 'body' => $body, + ])->once() + ->andThrow($mockError); + $mockError->shouldReceive('getResponse') + ->once()->andReturn($mockResponse); + $mockResponse->shouldReceive('getStatusCode') + ->once()->andReturn(401); + $this->cache->shouldReceive('clearAccessToken')->once(); + + // Mock the response succeeding on the retry + $this->client->shouldReceive('request') + ->with($method, $url, [ + 'headers' => $requestHeaders, + 'body' => $body, + ])->once()->andReturn($this->response); + $this->response->shouldReceive('getHeaders') + ->once()->andReturn($responseHeaders); + $this->response->shouldReceive('getBody') + ->once()->andReturn(json_encode($responseBody)); + + $result = $this->connector->makeServiceRequest($scopes, $method, $url, $body); + + $this->assertEquals($expected, $result); + } + + public function testItThrowsOnRepeated401Errors() + { + $scopes = ['scopeKey']; + $method = 'post'; + $url = 'https://example.com'; + $body = json_encode(['post' => 'body']); + $requestHeaders = [ + 'Authorization' => 'Bearer '.$this->token, + 'Accept' => 'application/json', + 'Content-Type' => 'application/json', + ]; + $responseHeaders = [ + 'Content-Type' => ['application/json'], + 'Server' => ['nginx'], + ]; + $responseBody = ['some' => 'response']; + $expected = [ + 'headers' => [ + 'Content-Type' => 'application/json', + 'Server' => 'nginx', + ], + 'body' => $responseBody, + ]; + + $this->mockCacheHasAccessToken(); + + // Mock the response failing twice + $mockError = Mockery::mock(ClientException::class); + $mockResponse = Mockery::mock(ResponseInterface::class); + $this->client->shouldReceive('request') + ->with($method, $url, [ + 'headers' => $requestHeaders, + 'body' => $body, + ])->twice() + ->andThrow($mockError); + $mockError->shouldReceive('getResponse') + ->twice()->andReturn($mockResponse); + $mockResponse->shouldReceive('getStatusCode') + ->twice()->andReturn(401); + + $this->cache->shouldReceive('clearAccessToken')->once(); + + $this->expectException(ClientException::class); + + $this->connector->makeServiceRequest($scopes, $method, $url, $body); + } + private function mockCacheHasAccessToken() { $this->registration->shouldReceive('getClientId')