Skip to content

Commit 4f360b0

Browse files
enmaboyaenmaboya
authored
replace rest with graphql in the access scope query, add cache (#359)
1 parent cc05cb0 commit 4f360b0

File tree

4 files changed

+94
-58
lines changed

4 files changed

+94
-58
lines changed

‎src/Http/Middleware/VerifyScopes.php

+72-32
Original file line numberDiff line numberDiff line change
@@ -3,51 +3,91 @@
33
namespace Osiset\ShopifyApp\Http\Middleware;
44

55
use Closure;
6-
use Exception;
76
use Illuminate\Http\Request;
8-
use Osiset\ShopifyApp\Contracts\ShopModel as IShopModel;
7+
use Illuminate\Support\Facades\{Cache, Log, Redirect};
8+
use Osiset\ShopifyApp\Contracts\ShopModel;
99
use Osiset\ShopifyApp\Util;
1010

1111
class VerifyScopes
1212
{
13+
private const CURRENT_SCOPES_CACHE_KEY = 'currentScopes';
14+
1315
/**
14-
* Checks if a shop has all required access scopes.
15-
* If a required access scope is missing, it will redirect the app
16-
* for re-authentication
17-
*
18-
* @param Request $request The request object.
19-
* @param Closure $next The next action.
20-
*
21-
* @throws Exception
16+
* Handle an incoming request.
2217
*
23-
* @return mixed
18+
* @return \Illuminate\Http\Response|\Illuminate\Http\RedirectResponse
2419
*/
2520
public function handle(Request $request, Closure $next)
2621
{
27-
/** @var $shop IShopModel */
22+
/** @var ?ShopModel */
2823
$shop = auth()->user();
29-
$scopesResponse = $shop->api()->rest('GET', '/admin/oauth/access_scopes.json');
30-
if ($scopesResponse && $scopesResponse['errors']) {
31-
return $next($request);
32-
}
33-
$scopes = json_decode(json_encode($scopesResponse['body']['access_scopes']), false);
34-
$scopes = array_map(static function ($scope) {
35-
return $scope->handle;
36-
}, $scopes);
37-
38-
$requiredScopes = explode(',', Util::getShopifyConfig('api_scopes'));
39-
$missingScopes = array_diff($requiredScopes, $scopes);
40-
if (count($missingScopes) === 0) {
41-
return $next($request);
24+
25+
if ($shop) {
26+
$response = $this->currentScopes($shop);
27+
28+
if ($response['hasErrors']) {
29+
return $next($request);
30+
}
31+
32+
$hasMissingScopes = filled(
33+
array_diff(
34+
explode(',', config('shopify-app.api_scopes')),
35+
$response['result']
36+
)
37+
);
38+
39+
if ($hasMissingScopes) {
40+
Cache::forget($this->cacheKey($shop->getDomain()->toNative()));
41+
42+
return Redirect::route(Util::getShopifyConfig('route_names.authenticate'), [
43+
'shop' => $shop->getDomain()->toNative(),
44+
'host' => $request->get('host'),
45+
'locale' => $request->get('locale'),
46+
]);
47+
}
4248
}
4349

44-
return redirect()->route(
45-
Util::getShopifyConfig('route_names.authenticate'),
46-
[
47-
'shop' => $shop->getDomain()->toNative(),
48-
'host' => $request->get('host'),
49-
'locale' => $request->get('locale'),
50-
]
50+
return $next($request);
51+
}
52+
53+
/**
54+
* @return array{hasErrors: bool, result: string[]}
55+
*/
56+
private function currentScopes(ShopModel $shop): array
57+
{
58+
/** @var array{errors: bool, status: int, body: \Gnikyt\BasicShopifyAPI\ResponseAccess} */
59+
$response = Cache::remember(
60+
$this->cacheKey($shop->getDomain()->toNative()),
61+
now()->addDay(),
62+
fn() => $shop->api()->graph('{
63+
currentAppInstallation {
64+
accessScopes {
65+
handle
66+
}
67+
}
68+
}')
5169
);
70+
71+
if (! $response['errors'] && blank(data_get($response['body']->toArray(), 'data.currentAppInstallation.userErrors'))) {
72+
return [
73+
'hasErrors' => false,
74+
'result' => array_column(
75+
data_get($response['body'], 'data.currentAppInstallation.accessScopes')->toArray(),
76+
'handle'
77+
),
78+
];
79+
}
80+
81+
Log::error('Fetch current app installation access scopes error: ' . json_encode(data_get($response['body']->toArray(), 'data.currentAppInstallation.userErrors')));
82+
83+
return [
84+
'hasErrors' => true,
85+
'result' => [],
86+
];
87+
}
88+
89+
private function cacheKey(string $shopDomain): string
90+
{
91+
return sprintf("{$shopDomain}.%s", self::CURRENT_SCOPES_CACHE_KEY);
5292
}
5393
}

‎tests/Http/Middleware/VerifyScopesTest.php

+4-15
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,12 @@ class VerifyScopesTest extends TestCase
1818
public function setUp(): void
1919
{
2020
parent::setUp();
21+
2122
$this->auth = $this->app->make(AuthManager::class);
2223
}
2324

2425
public function testMissingScopes(): void
2526
{
26-
// Setup API stub
2727
$this->setApiStub();
2828
ApiStub::stubResponses(['access_scopes']);
2929

@@ -34,18 +34,14 @@ public function testMissingScopes(): void
3434

3535
$request = Request::create('/', 'GET', ['shop' => $shop->getDomain()->toNative()]);
3636

37-
// Run the middleware
3837
$middleware = new VerifyScopesMiddleware();
39-
$result = $middleware->handle($request, function () {
40-
});
38+
$result = $middleware->handle($request, function () {});
4139

42-
//this line needs to assert if proper redirect was made
4340
$this->assertEquals(302, $result->getStatusCode());
4441
}
4542

4643
public function testMatchingScopes(): void
4744
{
48-
// Setup API stub
4945
$this->setApiStub();
5046
ApiStub::stubResponses(['access_scopes']);
5147

@@ -56,18 +52,14 @@ public function testMatchingScopes(): void
5652

5753
$request = Request::create('/', 'GET', ['shop' => $shop->getDomain()->toNative()]);
5854

59-
// Run the middleware
6055
$middleware = new VerifyScopesMiddleware();
61-
$result = $middleware->handle($request, function () {
62-
});
56+
$result = $middleware->handle($request, function () {});
6357

64-
//this line needs to assert if proper redirect was made
6558
$this->assertEquals($result, null);
6659
}
6760

6861
public function testScopeApiFailure(): void
6962
{
70-
// Setup API stub
7163
$this->setApiStub();
7264
ApiStub::stubResponses(['access_scopes_error']);
7365

@@ -78,12 +70,9 @@ public function testScopeApiFailure(): void
7870

7971
$request = Request::create('/', 'GET', ['shop' => $shop->getDomain()->toNative()]);
8072

81-
// Run the middleware
8273
$middleware = new VerifyScopesMiddleware();
83-
$result = $middleware->handle($request, function () {
84-
});
74+
$result = $middleware->handle($request, function () {});
8575

86-
//this line needs to assert if proper redirect was made
8776
$this->assertEquals($result, null);
8877
}
8978
}

‎tests/fixtures/access_scopes.json

+11-7
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
11
{
2-
"access_scopes": [
3-
{
4-
"handle": "read_products"
5-
},
6-
{
7-
"handle": "write_products"
2+
"data": {
3+
"currentAppInstallation": {
4+
"accessScopes": [
5+
{
6+
"handle": "write_products"
7+
},
8+
{
9+
"handle": "read_products"
10+
}
11+
]
812
}
9-
]
13+
}
1014
}

‎tests/fixtures/access_scopes_error.json

+7-4
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
{
2-
"errors": [
3-
{
4-
"message": "Could not get access copes"
2+
"data": {
3+
"currentAppInstallation": {
4+
"accessScopes": [],
5+
"userErrors": {
6+
"message": "Could not get access copes"
7+
}
58
}
6-
]
9+
}
710
}

0 commit comments

Comments
 (0)