From a9e0093819dcaa7bb452d2f7ad2340563721cc9b Mon Sep 17 00:00:00 2001 From: Paulo Margarido Date: Mon, 29 Nov 2021 14:10:16 -0500 Subject: [PATCH] Save signature OAuth cookies when using setcookie --- CHANGELOG.md | 1 + src/Auth/OAuth.php | 29 +++++++- tests/Auth/OAuthTest.php | 142 ++++++++++++++++++++++++++++----------- tests/UtilsTest.php | 15 ++++- 4 files changed, 141 insertions(+), 46 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 80c05aca..bbffa08e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). - [#117](https://github.com/Shopify/shopify-php-api/pull/117) Handle float `Retry-After` headers - [#114](https://github.com/Shopify/shopify-php-api/pull/114) Update session cookie expiration after OAuth +- [#116](https://github.com/Shopify/shopify-php-api/pull/116) Save signature OAuth cookies when using the fallback function for frameworkless apps ### Added ### Fixed diff --git a/src/Auth/OAuth.php b/src/Auth/OAuth.php index 526704ff..03b9f68b 100644 --- a/src/Auth/OAuth.php +++ b/src/Auth/OAuth.php @@ -25,6 +25,7 @@ class OAuth { public const SESSION_ID_COOKIE_NAME = 'shopify_session_id'; + public const SESSION_ID_SIG_COOKIE_NAME = 'shopify_session_id_sig'; public const ACCESS_TOKEN_POST_PATH = '/admin/oauth/access_token'; /** @@ -260,7 +261,18 @@ public static function getCurrentSessionId(array $rawHeaders, array $cookies, bo */ private static function getCookieSessionId(array $cookies): string { - $sessionId = $cookies[self::SESSION_ID_COOKIE_NAME] ?? null; + $signature = $cookies[self::SESSION_ID_SIG_COOKIE_NAME] ?? null; + $cookieId = $cookies[self::SESSION_ID_COOKIE_NAME] ?? null; + + $sessionId = null; + if ($signature && $cookieId) { + $expectedSignature = hash_hmac('sha256', $cookieId, Context::$API_SECRET_KEY); + + if ($signature === $expectedSignature) { + $sessionId = $cookieId; + } + } + if (!$sessionId) { throw new CookieNotFoundException("Could not find the current session id in the cookies"); } @@ -279,14 +291,27 @@ private static function getCookieSessionId(array $cookies): string */ private static function setCookieSessionId(?callable $setCookieFunction, $sessionId, $expiration): bool { + $signature = hash_hmac('sha256', $sessionId, Context::$API_SECRET_KEY); + $signatureCookie = new OAuthCookie($signature, self::SESSION_ID_SIG_COOKIE_NAME, $expiration, true, true); $cookie = new OAuthCookie($sessionId, self::SESSION_ID_COOKIE_NAME, $expiration, true, true); if ($setCookieFunction) { - $cookieSet = $setCookieFunction($cookie); + $cookieSet = $setCookieFunction($signatureCookie); + $cookieSet = $cookieSet && $setCookieFunction($cookie); } else { // @codeCoverageIgnoreStart // cannot mock setcookie() function $cookieSet = setcookie( + $signatureCookie->getName(), + $signatureCookie->getValue(), + $signatureCookie->getExpire(), + "", + "", + $signatureCookie->isSecure(), + $signatureCookie->isHttpOnly(), + ); + + $cookieSet = $cookieSet && setcookie( $cookie->getName(), $cookie->getValue(), $cookie->getExpire(), diff --git a/tests/Auth/OAuthTest.php b/tests/Auth/OAuthTest.php index 81bbc413..3459d92d 100644 --- a/tests/Auth/OAuthTest.php +++ b/tests/Auth/OAuthTest.php @@ -62,12 +62,11 @@ final class OAuthTest extends BaseTestCase */ public function testValidBegin($isOnline) { - $wasCallbackCalled = false; - $testCookieId = ''; - $cookieCallback = function ($cookie) use (&$wasCallbackCalled, &$testCookieId) { - $wasCallbackCalled = true; - $testCookieId = $cookie->getValue(); - return isset($testCookieId); + /** @var OAuthCookie[] */ + $cookiesSet = []; + $cookieCallback = function (OAuthCookie $cookie) use (&$cookiesSet) { + $cookiesSet[$cookie->getName()] = $cookie; + return !empty($cookie->getValue()); }; $returnUrl = OAuth::begin( @@ -76,15 +75,20 @@ public function testValidBegin($isOnline) $isOnline, $cookieCallback, ); - $this->assertTrue($wasCallbackCalled); - $this->assertNotEmpty($testCookieId); + $this->assertNotEmpty($cookiesSet[OAuth::SESSION_ID_COOKIE_NAME]->getValue()); + $this->assertEquals( + hash_hmac('sha256', $cookiesSet[OAuth::SESSION_ID_COOKIE_NAME]->getValue(), Context::$API_SECRET_KEY), + $cookiesSet[OAuth::SESSION_ID_SIG_COOKIE_NAME]->getValue() + ); if ($isOnline) { $grantOptions = 'per-user'; } else { $grantOptions = ''; } - $generatedState = Context::$SESSION_STORAGE->loadSession($testCookieId)->getState(); + + $cookieSessionId = $cookiesSet[OAuth::SESSION_ID_COOKIE_NAME]->getValue(); + $generatedState = Context::$SESSION_STORAGE->loadSession($cookieSessionId)->getState(); $this->assertEquals( // phpcs:ignore "https://shopname.myshopify.com/admin/oauth/authorize?client_id=ash&scope=sleepy%2Ckitty&redirect_uri=https%3A%2F%2Fwww.my-friends-cats.com%2Fredirect&state={$generatedState}&grant_options%5B%5D=$grantOptions", @@ -107,18 +111,11 @@ public function testValidCallback($isOnline, $isEmbedded) { Context::$IS_EMBEDDED_APP = $isEmbedded; - $wasCallbackCalled = false; - $testCookieId = ''; - $testCookieExpiration = null; - $cookieCallback = function (OAuthCookie $cookie) use ( - &$wasCallbackCalled, - &$testCookieId, - &$testCookieExpiration - ) { - $wasCallbackCalled = true; - $testCookieExpiration = $cookie->getExpire(); - $testCookieId = $cookie->getValue(); - return isset($testCookieId); + /** @var OAuthCookie[] */ + $cookiesSet = []; + $cookieCallback = function (OAuthCookie $cookie) use (&$cookiesSet) { + $cookiesSet[$cookie->getName()] = $cookie; + return !empty($cookie->getValue()); }; $this->createTestSession($isOnline); @@ -134,7 +131,10 @@ public function testValidCallback($isOnline, $isEmbedded) ), ]); - $mockCookies = [OAuth::SESSION_ID_COOKIE_NAME => $this->oauthSessionId]; + $mockCookies = [ + OAuth::SESSION_ID_SIG_COOKIE_NAME => hash_hmac('sha256', $this->oauthSessionId, Context::$API_SECRET_KEY), + OAuth::SESSION_ID_COOKIE_NAME => $this->oauthSessionId, + ]; $mockQuery = [ 'shop' => $this->domain, 'state' => '1234', @@ -142,8 +142,11 @@ public function testValidCallback($isOnline, $isEmbedded) 'hmac' => '0b19b6077391191829e442a97aafd7730323041e585f738415a77894c41c0a5b', ]; $actualSession = OAuth::callback($mockCookies, $mockQuery, $cookieCallback); - $this->assertTrue($wasCallbackCalled); - $this->assertEquals($this->oauthSessionId, $testCookieId); + $this->assertEquals($this->oauthSessionId, $cookiesSet[OAuth::SESSION_ID_COOKIE_NAME]->getValue()); + $this->assertEquals( + hash_hmac('sha256', $cookiesSet[OAuth::SESSION_ID_COOKIE_NAME]->getValue(), Context::$API_SECRET_KEY), + $cookiesSet[OAuth::SESSION_ID_SIG_COOKIE_NAME]->getValue() + ); $jwtSessionId = OAuth::getJwtSessionId($this->domain, '1'); @@ -160,12 +163,13 @@ public function testValidCallback($isOnline, $isEmbedded) $expectedSession = $this->buildExpectedSession($expectedSessionId, $isOnline); $this->assertEquals($expectedSession, $actualSession); + $cookieExpiration = $cookiesSet[OAuth::SESSION_ID_COOKIE_NAME]->getExpire(); if ($isEmbedded) { - $this->assertLessThanOrEqual(1, abs(time() - $testCookieExpiration)); // 1 second grace period + $this->assertLessThanOrEqual(1, abs(time() - $cookieExpiration)); // 1 second grace period } elseif ($isOnline) { - $this->assertEquals($expectedSession->getExpires()->format('U'), $testCookieExpiration); + $this->assertEquals($expectedSession->getExpires()->format('U'), $cookieExpiration); } else { - $this->assertNull($testCookieExpiration); + $this->assertNull($cookieExpiration); } } @@ -188,11 +192,28 @@ public function testCallbackFailsWithoutCookie() OAuth::callback([], []); } + public function testCallbackFailsWithInvalidSignature() + { + $this->createTestSession(false); + + $mockCookies = [ + OAuth::SESSION_ID_SIG_COOKIE_NAME => "Not the right signature", + OAuth::SESSION_ID_COOKIE_NAME => $this->oauthSessionId, + ]; + $this->expectException(\Shopify\Exception\CookieNotFoundException::class); + $this->expectExceptionMessage('Could not find the current session id in the cookies'); + OAuth::callback($mockCookies, []); + } + public function testCallbackFailsWithoutSession() { $this->createTestSession(false); - $mockCookies = [OAuth::SESSION_ID_COOKIE_NAME => "👋 This is not the session you're looking for"]; + $sessionId = "👋 This is not the session you're looking for"; + $mockCookies = [ + OAuth::SESSION_ID_SIG_COOKIE_NAME => hash_hmac('sha256', $sessionId, Context::$API_SECRET_KEY), + OAuth::SESSION_ID_COOKIE_NAME => $sessionId, + ]; $this->expectException(OAuthSessionNotFoundException::class); $this->expectExceptionMessage( 'You may have taken more than 60 seconds to complete the OAuth process and the session cannot be found' @@ -204,7 +225,10 @@ public function testCallbackFailsWithMissingHMAC() { $this->createTestSession(false); - $mockCookies = [OAuth::SESSION_ID_COOKIE_NAME => $this->oauthSessionId]; + $mockCookies = [ + OAuth::SESSION_ID_SIG_COOKIE_NAME => hash_hmac('sha256', $this->oauthSessionId, Context::$API_SECRET_KEY), + OAuth::SESSION_ID_COOKIE_NAME => $this->oauthSessionId, + ]; $mockQuery = [ 'shop' => $this->domain, 'state' => '1234', @@ -219,7 +243,10 @@ public function testCallbackFailsWithInvalidHMAC() { $this->createTestSession(false); - $mockCookies = [OAuth::SESSION_ID_COOKIE_NAME => $this->oauthSessionId]; + $mockCookies = [ + OAuth::SESSION_ID_SIG_COOKIE_NAME => hash_hmac('sha256', $this->oauthSessionId, Context::$API_SECRET_KEY), + OAuth::SESSION_ID_COOKIE_NAME => $this->oauthSessionId, + ]; $mockQuery = [ 'shop' => $this->domain, 'state' => '1234', @@ -235,7 +262,10 @@ public function testCallbackFailsWithMissingShop() { $this->createTestSession(false); - $mockCookies = [OAuth::SESSION_ID_COOKIE_NAME => $this->oauthSessionId]; + $mockCookies = [ + OAuth::SESSION_ID_SIG_COOKIE_NAME => hash_hmac('sha256', $this->oauthSessionId, Context::$API_SECRET_KEY), + OAuth::SESSION_ID_COOKIE_NAME => $this->oauthSessionId, + ]; $mockQuery = [ 'state' => '1234', 'hmac' => '0b19b6077391191829e442a97aafd7730323041e585f738415a77894c41c0a5b', @@ -249,7 +279,10 @@ public function testCallbackFailsWithInvalidShop() { $this->createTestSession(false); - $mockCookies = [OAuth::SESSION_ID_COOKIE_NAME => $this->oauthSessionId]; + $mockCookies = [ + OAuth::SESSION_ID_SIG_COOKIE_NAME => hash_hmac('sha256', $this->oauthSessionId, Context::$API_SECRET_KEY), + OAuth::SESSION_ID_COOKIE_NAME => $this->oauthSessionId, + ]; $mockQuery = [ 'shop' => 'not-a-valid.domain', 'state' => '1234', @@ -265,7 +298,10 @@ public function testCallbackFailsWithMissingState() { $this->createTestSession(false); - $mockCookies = [OAuth::SESSION_ID_COOKIE_NAME => $this->oauthSessionId]; + $mockCookies = [ + OAuth::SESSION_ID_SIG_COOKIE_NAME => hash_hmac('sha256', $this->oauthSessionId, Context::$API_SECRET_KEY), + OAuth::SESSION_ID_COOKIE_NAME => $this->oauthSessionId, + ]; $mockQuery = [ 'shop' => $this->domain, 'code' => 'real_code', @@ -280,7 +316,10 @@ public function testCallbackFailsWithInvalidState() { $this->createTestSession(false); - $mockCookies = [OAuth::SESSION_ID_COOKIE_NAME => $this->oauthSessionId]; + $mockCookies = [ + OAuth::SESSION_ID_SIG_COOKIE_NAME => hash_hmac('sha256', $this->oauthSessionId, Context::$API_SECRET_KEY), + OAuth::SESSION_ID_COOKIE_NAME => $this->oauthSessionId, + ]; $mockQuery = [ 'shop' => $this->domain, 'state' => '4321', @@ -296,7 +335,10 @@ public function testCallbackFailsWithMissingCode() { $this->createTestSession(false); - $mockCookies = [OAuth::SESSION_ID_COOKIE_NAME => $this->oauthSessionId]; + $mockCookies = [ + OAuth::SESSION_ID_SIG_COOKIE_NAME => hash_hmac('sha256', $this->oauthSessionId, Context::$API_SECRET_KEY), + OAuth::SESSION_ID_COOKIE_NAME => $this->oauthSessionId, + ]; $mockQuery = [ 'shop' => $this->domain, 'state' => '1234', @@ -311,7 +353,10 @@ public function testCallbackFailsWithInvalidCode() { $this->createTestSession(false); - $mockCookies = [OAuth::SESSION_ID_COOKIE_NAME => $this->oauthSessionId]; + $mockCookies = [ + OAuth::SESSION_ID_SIG_COOKIE_NAME => hash_hmac('sha256', $this->oauthSessionId, Context::$API_SECRET_KEY), + OAuth::SESSION_ID_COOKIE_NAME => $this->oauthSessionId, + ]; $mockQuery = [ 'shop' => $this->domain, 'state' => '1234', @@ -329,7 +374,10 @@ public function testFailsForPrivateApp() Context::$IS_PRIVATE_APP = true; - $mockCookies = [OAuth::SESSION_ID_COOKIE_NAME => $this->oauthSessionId]; + $mockCookies = [ + OAuth::SESSION_ID_SIG_COOKIE_NAME => hash_hmac('sha256', $this->oauthSessionId, Context::$API_SECRET_KEY), + OAuth::SESSION_ID_COOKIE_NAME => $this->oauthSessionId, + ]; $mockQuery = [ 'shop' => $this->domain, 'state' => '1234', @@ -358,7 +406,10 @@ public function testThrowsIfSessionDeleteFails() ), ]); - $mockCookies = [OAuth::SESSION_ID_COOKIE_NAME => $this->oauthSessionId]; + $mockCookies = [ + OAuth::SESSION_ID_SIG_COOKIE_NAME => hash_hmac('sha256', $this->oauthSessionId, Context::$API_SECRET_KEY), + OAuth::SESSION_ID_COOKIE_NAME => $this->oauthSessionId, + ]; $mockQuery = [ 'shop' => $this->domain, 'state' => '1234', @@ -390,7 +441,10 @@ public function testThrowsIfSessionStoreFails() ), ]); - $mockCookies = [OAuth::SESSION_ID_COOKIE_NAME => $this->oauthSessionId]; + $mockCookies = [ + OAuth::SESSION_ID_SIG_COOKIE_NAME => hash_hmac('sha256', $this->oauthSessionId, Context::$API_SECRET_KEY), + OAuth::SESSION_ID_COOKIE_NAME => $this->oauthSessionId, + ]; $mockQuery = [ 'shop' => $this->domain, 'state' => '1234', @@ -419,7 +473,10 @@ public function testFailsIfTokenFetchFails() ), ]); - $mockCookies = [OAuth::SESSION_ID_COOKIE_NAME => $this->oauthSessionId]; + $mockCookies = [ + OAuth::SESSION_ID_SIG_COOKIE_NAME => hash_hmac('sha256', $this->oauthSessionId, Context::$API_SECRET_KEY), + OAuth::SESSION_ID_COOKIE_NAME => $this->oauthSessionId, + ]; $mockQuery = [ 'shop' => $this->domain, 'state' => '1234', @@ -498,7 +555,10 @@ public function testGetCurrentSessionIdRaisesCookieNotFoundException() public function testGetCurrentSessionIdNonEmbeddedApp() { Context::$IS_EMBEDDED_APP = false; - $mockCookies = [OAuth::SESSION_ID_COOKIE_NAME => $this->oauthSessionId]; + $mockCookies = [ + OAuth::SESSION_ID_SIG_COOKIE_NAME => hash_hmac('sha256', $this->oauthSessionId, Context::$API_SECRET_KEY), + OAuth::SESSION_ID_COOKIE_NAME => $this->oauthSessionId, + ]; $currentSessionId = OAuth::getCurrentSessionId([], $mockCookies, true); $this->assertEquals('test_oauth_session', $currentSessionId); diff --git a/tests/UtilsTest.php b/tests/UtilsTest.php index 307967e6..f271b34c 100644 --- a/tests/UtilsTest.php +++ b/tests/UtilsTest.php @@ -220,7 +220,10 @@ public function testGraphqlProxyFailsWithJWTForNonEmbeddedApps() $token = $this->encodeJwtPayload(); $headers = ['Authorization' => "Bearer $token"]; - $cookies = [OAuth::SESSION_ID_COOKIE_NAME => 'cookie_id']; + $cookies = [ + OAuth::SESSION_ID_SIG_COOKIE_NAME => hash_hmac('sha256', 'cookie_id', Context::$API_SECRET_KEY), + OAuth::SESSION_ID_COOKIE_NAME => 'cookie_id', + ]; // The session is valid and can be loaded from the headers Context::$IS_EMBEDDED_APP = true; @@ -241,7 +244,10 @@ public function testGraphqlProxyFailsWithCookiesForEmbeddedApps() $token = $this->encodeJwtPayload(); $headers = ['Authorization' => "Bearer $token"]; - $cookies = [OAuth::SESSION_ID_COOKIE_NAME => 'cookie_id']; + $cookies = [ + OAuth::SESSION_ID_SIG_COOKIE_NAME => hash_hmac('sha256', 'cookie_id', Context::$API_SECRET_KEY), + OAuth::SESSION_ID_COOKIE_NAME => 'cookie_id', + ]; // The session is valid and can be loaded from the cookies Context::$IS_EMBEDDED_APP = false; @@ -311,7 +317,10 @@ public function testGraphqlProxyFetchesDataWithCookies() ) ]); - $cookies = [OAuth::SESSION_ID_COOKIE_NAME => $sessionId]; + $cookies = [ + OAuth::SESSION_ID_COOKIE_NAME => $sessionId, + OAuth::SESSION_ID_SIG_COOKIE_NAME => hash_hmac('sha256', $sessionId, Context::$API_SECRET_KEY), + ]; $response = Utils::graphqlProxy([], $cookies, $this->testGraphqlQuery); $this->assertThat($response, new HttpResponseMatcher(200, [], $this->testGraphqlResponse));