Skip to content

Commit 2dbbbef

Browse files
authored
Fixing a vulnerability related to access to store data. (#418)
* authorization after token verification, added a restriction preventing API routes from being accessed via a browser * added forbidden web middleware groups to the configuration
1 parent ddee18f commit 2dbbbef

File tree

4 files changed

+64
-12
lines changed

4 files changed

+64
-12
lines changed

src/Http/Middleware/VerifyShopify.php

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -101,20 +101,29 @@ public function handle(Request $request, Closure $next)
101101
return $next($request);
102102
}
103103

104-
if (!Util::useNativeAppBridge()) {
105-
$shop = $this->getShopIfAlreadyInstalled($request);
106-
$storeResult = !$this->isApiRequest($request) && $shop;
104+
$tokenSource = $this->getAccessTokenFromRequest($request);
107105

108-
if ($storeResult) {
109-
$this->loginFromShop($shop);
106+
if ($tokenSource === null) {
107+
$forbiddenMiddlewareMatches = array_intersect(
108+
Util::getShopifyConfig('forbidden_web_middleware_groups'),
109+
$request->route()?->middleware() ?? []
110+
);
110111

111-
return $next($request);
112+
if (filled($forbiddenMiddlewareMatches)) {
113+
throw new HttpException('Access denied.', Response::HTTP_FORBIDDEN);
112114
}
113-
}
114115

115-
$tokenSource = $this->getAccessTokenFromRequest($request);
116+
if (!Util::useNativeAppBridge()) {
117+
$shop = $this->getShopIfAlreadyInstalled($request);
118+
$storeResult = !$this->isApiRequest($request) && $shop;
119+
120+
if ($storeResult) {
121+
$this->loginFromShop($shop);
122+
123+
return $next($request);
124+
}
125+
}
116126

117-
if ($tokenSource === null) {
118127
//Check if there is a store record in the database
119128
return $this->checkPreviousInstallation($request)
120129
// Shop exists, token not available, we need to get one

src/resources/config/shopify-app.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -588,4 +588,16 @@
588588
'frontend_engine' => env('SHOPIFY_FRONTEND_ENGINE', 'BLADE'),
589589

590590
'iframe_ancestors' => '',
591+
592+
/*
593+
|--------------------------------------------------------------------------
594+
| Forbidden middleware groups
595+
|--------------------------------------------------------------------------
596+
|
597+
| Routes prohibited from being opened in the browser.
598+
|
599+
*/
600+
'forbidden_web_middleware_groups' => [
601+
'api',
602+
]
591603
];

tests/Http/Middleware/VerifyShopifyTest.php

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22

33
namespace Osiset\ShopifyApp\Test\Http\Middleware;
44

5+
use Illuminate\Http\Response;
56
use Illuminate\Support\Facades\Request;
7+
use Illuminate\Support\Facades\Route;
68
use Osiset\ShopifyApp\Exceptions\HttpException;
79
use Osiset\ShopifyApp\Exceptions\SignatureVerificationException;
810
use Osiset\ShopifyApp\Http\Middleware\VerifyShopify;
@@ -318,7 +320,7 @@ public function testTokenProcessingAndMissMatchingShops(): void
318320
$this->runMiddleware(VerifyShopify::class, $newRequest);
319321
}
320322

321-
public function testNotNativeAppbridgeWithTokenProcessingAndLoginShop(): void
323+
public function testNotNativeAppBridgeWithTokenProcessingAndLoginShop(): void
322324
{
323325
// Create a shop that matches the token from buildToken
324326
factory($this->model)->create(['name' => 'shop-name.myshopify.com']);
@@ -349,4 +351,33 @@ public function testNotNativeAppbridgeWithTokenProcessingAndLoginShop(): void
349351
$result = $this->runMiddleware(VerifyShopify::class, $newRequest);
350352
$this->assertTrue($result[0]);
351353
}
354+
355+
public function testAccessingForbiddenMiddlewareRouteFromBrowserReceivedAccessError(): void
356+
{
357+
$this->expectException(HttpException::class);
358+
$this->expectExceptionMessage('Access denied');
359+
$this->expectExceptionCode(Response::HTTP_FORBIDDEN);
360+
361+
// Create a shop that matches the token from buildToken
362+
factory($this->model)->create(['name' => 'shop-name.myshopify.com']);
363+
$this->app['config']->set('shopify-app.frontend_engine', 'REACT');
364+
$this->app['config']->set('shopify-app.forbidden_web_middleware_groups', ['api']);
365+
$this->app['router']->get('/api/some/endpoint', fn () => true)->middleware(['api']);
366+
367+
// Setup the request
368+
$currentRequest = Request::instance();
369+
$newRequest = $currentRequest->duplicate(
370+
query: [
371+
'shop' => 'shop-name.myshopify.com',
372+
],
373+
server: [
374+
'HTTP_Authorization' => "Bearer {$this->buildToken()}",
375+
'REQUEST_URI' => 'api/some/endpoint',
376+
]
377+
);
378+
$route = Route::getRoutes()->match($newRequest);
379+
$newRequest->setRouteResolver(fn () => $route);
380+
381+
$this->runMiddleware(VerifyShopify::class, $newRequest);
382+
}
352383
}

tests/Traits/ApiControllerTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@ public function testApiWithoutToken(): void
1212
$shop = factory($this->model)->create();
1313

1414
$response = $this->getJson('/api', ['HTTP_X-Shop-Domain' => $shop->name]);
15-
$response->assertStatus(Response::HTTP_BAD_REQUEST);
16-
$response->assertExactJson(['error' => 'Session token is invalid.']);
15+
$response->assertStatus(Response::HTTP_FORBIDDEN);
16+
$response->assertExactJson(['error' => 'Access denied.']);
1717
}
1818

1919
public function testApiWithToken(): void

0 commit comments

Comments
 (0)