Skip to content

Commit d496f6e

Browse files
paulomargpaulomarg
authored
Merge pull request #345 from Shopify/use_state_as_oauth_cookie
Stop storing session when beginning OAuth
2 parents 746d931 + 588ecf4 commit d496f6e

4 files changed

Lines changed: 183 additions & 233 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ and adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
77

88
## Unreleased
99

10+
- [#345](https://github.com/Shopify/shopify-api-php/pull/345) [Patch] Stop storing a session in the database when beginning OAuth, only when completing it
11+
1012
## v5.5.0 - 2024-04-18
1113

1214
- [#332](https://github.com/Shopify/shopify-api-php/pull/332) [Patch] Avoid writing to system temporary directory when an API deprecation is encountered

‎src/Auth/OAuth.php‎

Lines changed: 110 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,14 @@
2828
*/
2929
class OAuth
3030
{
31+
public const STATE_COOKIE_NAME = 'shopify_app_state';
32+
public const STATE_SIG_COOKIE_NAME = 'shopify_app_state_sig';
3133
public const SESSION_ID_COOKIE_NAME = 'shopify_session_id';
3234
public const SESSION_ID_SIG_COOKIE_NAME = 'shopify_session_id_sig';
3335
public const ACCESS_TOKEN_POST_PATH = '/admin/oauth/access_token';
3436

3537
/**
36-
* Initializes a session and cookie for the OAuth process, and returns the authorization url
38+
* Begins the OAuth process by setting the appropriate cookies, and returns the authorization url
3739
*
3840
* @param string $shop A Shopify domain name or hostname
3941
* @param string $redirectPath Redirect path for callback
@@ -64,37 +66,26 @@ public static function begin(
6466
$redirectPath = trim(strtolower($redirectPath));
6567
$redirectPath = ($redirectPath[0] == '/') ? $redirectPath : '/' . $redirectPath;
6668

67-
$mySessionId = $isOnline ? Uuid::uuid4()->toString() : self::getOfflineSessionId($sanitizedShop);
69+
$state = Uuid::uuid4()->toString();
6870

69-
$cookieSet = self::setCookieSessionId($setCookieFunction, $mySessionId, strtotime('+1 minute'));
71+
$cookieSet = self::setStateCookie($setCookieFunction, $state, strtotime('+1 minute'));
7072
if (!$cookieSet) {
7173
throw new CookieSetException(
7274
'OAuth Cookie could not be saved.'
7375
);
7476
}
7577

76-
$session = new Session($mySessionId, $sanitizedShop, $isOnline, Uuid::uuid4()->toString());
77-
7878
if ($isOnline) {
79-
$session->setExpires(strtotime('+1 minute'));
8079
$grantOptions = 'per-user';
8180
} else {
8281
$grantOptions = '';
8382
}
8483

85-
$sessionStored = Context::$SESSION_STORAGE->storeSession($session);
86-
87-
if (!$sessionStored) {
88-
throw new SessionStorageException(
89-
'OAuth Session could not be saved. Please check your session storage functionality.'
90-
);
91-
}
92-
9384
$query = [
9485
'client_id' => Context::$API_KEY,
9586
'scope' => Context::$SCOPES->toString(),
9687
'redirect_uri' => Context::$HOST_SCHEME . '://' . Context::$HOST_NAME . $redirectPath,
97-
'state' => $session->getState(),
88+
'state' => $state,
9889
'grant_options[]' => $grantOptions,
9990
];
10091

@@ -125,19 +116,18 @@ public static function callback(array $cookies, array $query, ?callable $setCook
125116
Context::throwIfUninitialized();
126117
Context::throwIfPrivateApp('OAuth is not allowed for private apps');
127118

128-
$cookieSessionId = self::getCookieSessionId($cookies);
129-
$session = Context::$SESSION_STORAGE->loadSession($cookieSessionId);
130-
if (!$session) {
131-
throw new OAuthSessionNotFoundException(
132-
'You may have taken more than 60 seconds to complete the OAuth process and the session cannot be found'
133-
);
134-
}
135-
136-
if (!self::isCallbackQueryValid($query, $session)) {
119+
$cookieState = self::getStateCookie($cookies);
120+
if (!self::isCallbackQueryValid($query, $cookieState)) {
137121
throw new InvalidOAuthException('Invalid OAuth callback.');
138122
}
139123

140-
$response = self::fetchAccessToken($query, $session);
124+
$sanitizedShop = Utils::sanitizeShopDomain($query['shop'] ?? '');
125+
$response = self::fetchAccessToken($query, $sanitizedShop);
126+
127+
$isOnline = $response instanceof AccessTokenOnlineResponse;
128+
129+
$sessionId = $isOnline ? Uuid::uuid4()->toString() : self::getOfflineSessionId($sanitizedShop);
130+
$session = new Session($sessionId, $sanitizedShop, $isOnline, '');
141131

142132
$session->setAccessToken($response->getAccessToken());
143133
$session->setScope($response->getScope());
@@ -151,15 +141,7 @@ public static function callback(array $cookies, array $query, ?callable $setCook
151141
// JWT.
152142
if (Context::$IS_EMBEDDED_APP) {
153143
$jwtSessionId = self::getJwtSessionId($session->getShop(), $session->getOnlineAccessInfo()->getId());
154-
$jwtSession = $session->clone($jwtSessionId);
155-
156-
$sessionDeleted = Context::$SESSION_STORAGE->deleteSession($session->getId());
157-
if (!$sessionDeleted) {
158-
throw new SessionStorageException(
159-
'OAuth Session could not be deleted. Please check your session storage functionality.',
160-
);
161-
}
162-
$session = $jwtSession;
144+
$session = $session->clone($jwtSessionId);
163145
}
164146
}
165147

@@ -171,15 +153,15 @@ public static function callback(array $cookies, array $query, ?callable $setCook
171153
}
172154

173155
$sessionExpiration = ($session->getExpires() ? (int)$session->getExpires()->format('U') : null);
174-
$cookieSet = self::setCookieSessionId(
156+
$cookieSet = self::setSessionIdCookie(
175157
$setCookieFunction,
176-
$cookieSessionId,
158+
$session->getId(),
177159
Context::$IS_EMBEDDED_APP ? time() : $sessionExpiration
178160
);
161+
$cookieSet = $cookieSet && self::setStateCookie($setCookieFunction, $cookieState, time());
162+
179163
if (!$cookieSet) {
180-
throw new CookieSetException(
181-
'OAuth Cookie could not be saved.'
182-
);
164+
throw new CookieSetException('OAuth Cookie could not be saved.');
183165
}
184166

185167
return $session;
@@ -249,34 +231,61 @@ public static function getCurrentSessionId(array $rawHeaders, array $cookies, bo
249231
if (!$cookies) {
250232
throw new CookieNotFoundException('Could not find the current session id in the cookies');
251233
}
252-
$currentSessionId = self::getCookieSessionId($cookies);
234+
$currentSessionId = self::getSessionIdCookie($cookies);
253235
}
254236

255237
return $currentSessionId;
256238
}
257239

258240
/**
259-
* Fetches the current session ID from the given cookies.
241+
* Fetches the current state from the given cookies.
260242
*
261243
* @param array $cookies The $cookies param from `callback`
262244
*
263-
* @return string The ID of the current session
245+
* @return string The state for the current OAuth process
264246
* @throws CookieNotFoundException
265247
*/
266-
private static function getCookieSessionId(array $cookies): string
248+
private static function getStateCookie(array $cookies): string
267249
{
268-
$signature = $cookies[self::SESSION_ID_SIG_COOKIE_NAME] ?? null;
269-
$cookieId = $cookies[self::SESSION_ID_COOKIE_NAME] ?? null;
250+
$value = self::getCookie($cookies, self::STATE_COOKIE_NAME, self::STATE_SIG_COOKIE_NAME);
251+
if (!$value) {
252+
throw new CookieNotFoundException(
253+
'You may have taken more than 60 seconds to complete the OAuth process and need to start over'
254+
);
255+
}
270256

271-
$sessionId = null;
272-
if ($signature && $cookieId) {
273-
$expectedSignature = hash_hmac('sha256', (string) $cookieId, Context::$API_SECRET_KEY);
257+
return (string)$value;
258+
}
274259

275-
if ($signature === $expectedSignature) {
276-
$sessionId = $cookieId;
277-
}
278-
}
260+
/**
261+
* Sets the state for OAuth in the right cookie.
262+
*
263+
* @param null|callable $setCookieFunction An optional override for setting cookie in response
264+
* @param string $state The ID of the session to save
265+
* @param int $expiration Epoch timestamp (in s) when the cookie expires
266+
*
267+
* @return bool Whether the cookie was successfully set
268+
*/
269+
private static function setStateCookie(?callable $setCookieFunction, $state, $expiration): bool
270+
{
271+
$signature = hash_hmac('sha256', $state, Context::$API_SECRET_KEY);
272+
$signatureCookie = new OAuthCookie($signature, self::STATE_SIG_COOKIE_NAME, $expiration, true, true);
273+
$cookie = new OAuthCookie($state, self::STATE_COOKIE_NAME, $expiration, true, true);
279274

275+
return self::setCookie($setCookieFunction, $cookie, $signatureCookie);
276+
}
277+
278+
/**
279+
* Fetches the current session ID from the given cookies.
280+
*
281+
* @param array $cookies The $cookies param from `callback`
282+
*
283+
* @return string The ID of the current session
284+
* @throws CookieNotFoundException
285+
*/
286+
private static function getSessionIdCookie(array $cookies): string
287+
{
288+
$sessionId = self::getCookie($cookies, self::SESSION_ID_COOKIE_NAME, self::SESSION_ID_SIG_COOKIE_NAME);
280289
if (!$sessionId) {
281290
throw new CookieNotFoundException("Could not find the current session id in the cookies");
282291
}
@@ -293,12 +302,46 @@ private static function getCookieSessionId(array $cookies): string
293302
*
294303
* @return bool Whether the cookie was successfully set
295304
*/
296-
private static function setCookieSessionId(?callable $setCookieFunction, $sessionId, $expiration): bool
305+
private static function setSessionIdCookie(?callable $setCookieFunction, $sessionId, $expiration): bool
297306
{
298307
$signature = hash_hmac('sha256', $sessionId, Context::$API_SECRET_KEY);
299308
$signatureCookie = new OAuthCookie($signature, self::SESSION_ID_SIG_COOKIE_NAME, $expiration, true, true);
300309
$cookie = new OAuthCookie($sessionId, self::SESSION_ID_COOKIE_NAME, $expiration, true, true);
301310

311+
return self::setCookie($setCookieFunction, $cookie, $signatureCookie);
312+
}
313+
314+
/**
315+
* Fetches the value of a cookie.
316+
*
317+
* @param array $cookies The cookies array
318+
* @param string $name The name of the cookie
319+
* @param string $signatureName The name of the signature cookie
320+
*
321+
* @return string|null The value of the cookie, or null if it's invalid or missing
322+
*/
323+
private static function getCookie(array $cookies, string $name, string $signatureName): string | null
324+
{
325+
$signature = $cookies[$signatureName] ?? null;
326+
$cookieId = $cookies[$name] ?? null;
327+
328+
$value = null;
329+
if ($signature && $cookieId) {
330+
$expectedSignature = hash_hmac('sha256', (string) $cookieId, Context::$API_SECRET_KEY);
331+
332+
if ($signature === $expectedSignature) {
333+
$value = $cookieId;
334+
}
335+
}
336+
337+
return $value;
338+
}
339+
340+
private static function setCookie(
341+
?callable $setCookieFunction,
342+
OAuthCookie $cookie,
343+
OAuthCookie $signatureCookie
344+
): bool {
302345
if ($setCookieFunction) {
303346
$cookieSet = $setCookieFunction($signatureCookie);
304347
$cookieSet = $cookieSet && $setCookieFunction($cookie);
@@ -333,55 +376,54 @@ private static function setCookieSessionId(?callable $setCookieFunction, $sessio
333376
/**
334377
* Checks whether the given query parameters are from a valid callback request.
335378
*
336-
* @param array $query The URL query parameters
337-
* @param Session $session The current session
379+
* @param array $query The URL query parameters
380+
* @param string|null $stateCookie The value of the cookie containing the OAuth state
338381
*
339382
* @return bool
340383
* @throws UninitializedContextException
341384
*/
342-
private static function isCallbackQueryValid(array $query, Session $session): bool
385+
private static function isCallbackQueryValid(array $query, string | null $stateCookie): bool
343386
{
344387
$sanitizedShop = Utils::sanitizeShopDomain($query['shop'] ?? '');
345388
$state = $query['state'] ?? '';
346389
$code = $query['code'] ?? '';
347390

348391
return (
349392
($code) &&
350-
($sanitizedShop && strcmp($session->getShop(), $sanitizedShop) === 0) &&
351-
($state && strcmp($session->getState(), (string) $state) === 0) &&
393+
($sanitizedShop) &&
394+
($state && $stateCookie && strcmp($stateCookie, (string) $state) === 0) &&
352395
Utils::validateHmac($query, Context::$API_SECRET_KEY)
353396
);
354397
}
355398

356399
/**
357400
* Fetches the access token for the given OAuth session, using the query parameters returned by Shopify
358401
*
359-
* @param array $query The URL query params from the OAuth callback
360-
* @param Session $session The OAuth session
402+
* @param array $query The URL query params from the OAuth callback
403+
* @param string $shop The request shop
361404
*
362405
* @return AccessTokenResponse|AccessTokenOnlineResponse The access token exchanged for the OAuth code
363406
* @throws HttpRequestException
364407
*/
365-
private static function fetchAccessToken(
366-
array $query,
367-
Session $session
368-
) {
408+
private static function fetchAccessToken(array $query, string $shop)
409+
{
369410
$post = [
370411
'client_id' => Context::$API_KEY,
371412
'client_secret' => Context::$API_SECRET_KEY,
372413
'code' => $query['code'],
373414
];
374415

375-
$client = new Http($session->getShop());
416+
$client = new Http($shop);
376417
$response = self::requestAccessToken($client, $post);
377418
if ($response->getStatusCode() !== 200) {
378419
throw new HttpRequestException("Failed to get access token: {$response->getDecodedBody()}");
379420
}
380421

381-
if ($session->isOnline()) {
382-
return self::buildAccessTokenOnlineResponse($response->getDecodedBody());
422+
$body = $response->getDecodedBody();
423+
if (array_key_exists('associated_user', $body) && $body['associated_user']) {
424+
return self::buildAccessTokenOnlineResponse($body);
383425
} else {
384-
return self::buildAccessTokenResponse($response->getDecodedBody());
426+
return self::buildAccessTokenResponse($body);
385427
}
386428
}
387429

‎tests/Auth/MockSessionStorage.php‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,16 @@ public function deleteSession(string $sessionId): bool
6464
}
6565
}
6666

67+
/**
68+
* Retrieves all sessions in this storage.
69+
*
70+
* @return array
71+
*/
72+
public function getAllSessions(): array
73+
{
74+
return $this->testSessions;
75+
}
76+
6777
/**
6878
* Retrieves the calls made to this class for assertions.
6979
*

0 commit comments

Comments
 (0)