Merge pull request 1EdTech#20 from packbackbooks/invalid-token-retry

ONCALL-216: Implement invalid token retry.
snake · Sep 2, 2021 · 9e67c49 · 9e67c49
2 parents 5e0f652 + c4899ee
commit 9e67c49
Show file tree

Hide file tree

Showing 5 changed files with 164 additions and 19 deletions.
diff --git 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
@@ -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
@@ -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,38 +76,59 @@ 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];
         });
         $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
@@ -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
@@ -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')