From 5b0477efa191cb8f81246e713fae43380f6f8325 Mon Sep 17 00:00:00 2001 From: enmaboya Date: Wed, 30 Oct 2024 14:32:26 +0400 Subject: [PATCH] replace rest with graphql in the access scope query, add cache --- src/Http/Middleware/VerifyScopes.php | 106 ++++++++++++++------- tests/Http/Middleware/VerifyScopesTest.php | 19 +--- tests/fixtures/access_scopes.json | 18 ++-- tests/fixtures/access_scopes_error.json | 11 ++- 4 files changed, 96 insertions(+), 58 deletions(-) diff --git a/src/Http/Middleware/VerifyScopes.php b/src/Http/Middleware/VerifyScopes.php index 9991fa40..d3245a57 100644 --- a/src/Http/Middleware/VerifyScopes.php +++ b/src/Http/Middleware/VerifyScopes.php @@ -3,51 +3,93 @@ namespace Osiset\ShopifyApp\Http\Middleware; use Closure; -use Exception; use Illuminate\Http\Request; -use Osiset\ShopifyApp\Contracts\ShopModel as IShopModel; +use Illuminate\Support\Facades\{Cache, Log, Redirect}; +use Osiset\ShopifyApp\Contracts\ShopModel; use Osiset\ShopifyApp\Util; class VerifyScopes { + private const CURRENT_SCOPES_CACHE_KEY = 'currentScopes'; + /** - * Checks if a shop has all required access scopes. - * If a required access scope is missing, it will redirect the app - * for re-authentication - * - * @param Request $request The request object. - * @param Closure $next The next action. - * - * @throws Exception + * Handle an incoming request. * - * @return mixed + * @return \Illuminate\Http\Response|\Illuminate\Http\RedirectResponse */ public function handle(Request $request, Closure $next) { - /** @var $shop IShopModel */ + /** @var ?ShopModel */ $shop = auth()->user(); - $scopesResponse = $shop->api()->rest('GET', '/admin/oauth/access_scopes.json'); - if ($scopesResponse && $scopesResponse['errors']) { - return $next($request); - } - $scopes = json_decode(json_encode($scopesResponse['body']['access_scopes']), false); - $scopes = array_map(static function ($scope) { - return $scope->handle; - }, $scopes); - - $requiredScopes = explode(',', Util::getShopifyConfig('api_scopes')); - $missingScopes = array_diff($requiredScopes, $scopes); - if (count($missingScopes) === 0) { - return $next($request); + + if ($shop) { + $response = $this->currentScopes($shop); + + if ($response['hasErrors']) { + return $next($request); + } + + $hasMissingScopes = filled( + array_diff( + explode(',', config('shopify-app.api_scopes')), + $response['result'] + ) + ); + + if ($hasMissingScopes) { + Cache::forget($this->cacheKey($shop->getDomain()->toNative())); + + return Redirect::route( + Util::getShopifyConfig('route_names.authenticate'), + [ + 'shop' => $shop->getDomain()->toNative(), + 'host' => $request->get('host'), + ] + ); + } } - return redirect()->route( - Util::getShopifyConfig('route_names.authenticate'), - [ - 'shop' => $shop->getDomain()->toNative(), - 'host' => $request->get('host'), - 'locale' => $request->get('locale'), - ] + return $next($request); + } + + /** + * @return array{hasErrors: bool, result: string[]} + */ + private function currentScopes(ShopModel $shop): array + { + /** @var array{errors: bool, status: int, body: \Gnikyt\BasicShopifyAPI\ResponseAccess} */ + $response = Cache::remember( + $this->cacheKey($shop->getDomain()->toNative()), + now()->addDay(), + fn() => $shop->api()->graph('{ + currentAppInstallation { + accessScopes { + handle + } + } + }') ); + + if (! $response['errors'] && blank(data_get($response['body']->toArray(), 'data.currentAppInstallation.userErrors'))) { + return [ + 'hasErrors' => false, + 'result' => array_column( + data_get($response['body'], 'data.currentAppInstallation.accessScopes')->toArray(), + 'handle' + ), + ]; + } + + Log::error('Fetch current app installation access scopes error: ' . json_encode(data_get($response['body']->toArray(), 'data.currentAppInstallation.userErrors'))); + + return [ + 'hasErrors' => true, + 'result' => [], + ]; + } + + private function cacheKey(string $shopDomain): string + { + return sprintf("{$shopDomain}.%s", self::CURRENT_SCOPES_CACHE_KEY); } } diff --git a/tests/Http/Middleware/VerifyScopesTest.php b/tests/Http/Middleware/VerifyScopesTest.php index 683a80d5..6e678634 100644 --- a/tests/Http/Middleware/VerifyScopesTest.php +++ b/tests/Http/Middleware/VerifyScopesTest.php @@ -18,12 +18,12 @@ class VerifyScopesTest extends TestCase public function setUp(): void { parent::setUp(); + $this->auth = $this->app->make(AuthManager::class); } public function testMissingScopes(): void { - // Setup API stub $this->setApiStub(); ApiStub::stubResponses(['access_scopes']); @@ -34,18 +34,14 @@ public function testMissingScopes(): void $request = Request::create('/', 'GET', ['shop' => $shop->getDomain()->toNative()]); - // Run the middleware $middleware = new VerifyScopesMiddleware(); - $result = $middleware->handle($request, function () { - }); + $result = $middleware->handle($request, function () {}); - //this line needs to assert if proper redirect was made $this->assertEquals(302, $result->getStatusCode()); } public function testMatchingScopes(): void { - // Setup API stub $this->setApiStub(); ApiStub::stubResponses(['access_scopes']); @@ -56,18 +52,14 @@ public function testMatchingScopes(): void $request = Request::create('/', 'GET', ['shop' => $shop->getDomain()->toNative()]); - // Run the middleware $middleware = new VerifyScopesMiddleware(); - $result = $middleware->handle($request, function () { - }); + $result = $middleware->handle($request, function () {}); - //this line needs to assert if proper redirect was made $this->assertEquals($result, null); } public function testScopeApiFailure(): void { - // Setup API stub $this->setApiStub(); ApiStub::stubResponses(['access_scopes_error']); @@ -78,12 +70,9 @@ public function testScopeApiFailure(): void $request = Request::create('/', 'GET', ['shop' => $shop->getDomain()->toNative()]); - // Run the middleware $middleware = new VerifyScopesMiddleware(); - $result = $middleware->handle($request, function () { - }); + $result = $middleware->handle($request, function () {}); - //this line needs to assert if proper redirect was made $this->assertEquals($result, null); } } diff --git a/tests/fixtures/access_scopes.json b/tests/fixtures/access_scopes.json index 2d43faee..477d9f7f 100644 --- a/tests/fixtures/access_scopes.json +++ b/tests/fixtures/access_scopes.json @@ -1,10 +1,14 @@ { - "access_scopes": [ - { - "handle": "read_products" - }, - { - "handle": "write_products" + "data": { + "currentAppInstallation": { + "accessScopes": [ + { + "handle": "write_products" + }, + { + "handle": "read_products" + } + ] } - ] + } } diff --git a/tests/fixtures/access_scopes_error.json b/tests/fixtures/access_scopes_error.json index f1073e4c..72420213 100644 --- a/tests/fixtures/access_scopes_error.json +++ b/tests/fixtures/access_scopes_error.json @@ -1,7 +1,10 @@ { - "errors": [ - { - "message": "Could not get access copes" + "data": { + "currentAppInstallation": { + "accessScopes": [], + "userErrors": { + "message": "Could not get access copes" + } } - ] + } }