Address comments

Shopify · Nov 28, 2023 · d90c52d · d90c52d
1 parent cd4e13d
commit d90c52d
Show file tree

Hide file tree

Showing 6 changed files with 23 additions and 23 deletions.
diff --git a/packages/shopify-app-remix/src/server/authenticate/admin/authenticate.ts b/packages/shopify-app-remix/src/server/authenticate/admin/authenticate.ts
@@ -68,7 +68,7 @@ export function authStrategyFactory<
   function createContext(
     request: Request,
     session: Session,
-    sessionToken: JwtPayload,
+    sessionToken?: JwtPayload,
   ): AdminContext<ConfigArg, Resources> {
     const context:
       | EmbeddedAdminContext<ConfigArg, Resources>
@@ -137,7 +137,7 @@ export function authStrategyFactory<
         isOnline: session.isOnline,
       });
 
-      return createContext(request, session, payload!);
+      return createContext(request, session, payload);
     } catch (errorOrResponse) {
       if (errorOrResponse instanceof Response) {
         ensureCORSHeadersFactory(params, request)(errorOrResponse);
@@ -173,7 +173,7 @@ async function getSessionTokenContext(
     const dest = new URL(payload.dest);
     const shop = dest.hostname;
 
-    logger.debug('Session token is present, validating session', {shop});
+    logger.debug('Session token is valid', {shop, payload});
     const sessionId = config.useOnlineTokens
       ? api.session.getJwtSessionId(shop, payload.sub)
       : api.session.getOfflineId(shop);

diff --git a/.../handle-routes/auth-callback-path.test.ts → ...auth-code-flow/auth-callback-path.test.ts b/.../handle-routes/auth-callback-path.test.ts → ...auth-code-flow/auth-callback-path.test.ts
diff --git a/...code-flow/handle-routes/auth-path.test.ts → ...ode-flow/auth-code-flow/auth-path.test.ts b/...code-flow/handle-routes/auth-path.test.ts → ...ode-flow/auth-code-flow/auth-path.test.ts
diff --git a/...uth-code-flow/manage-access-token.test.ts → ...-flow/auth-code-flow/authenticate.test.ts b/...uth-code-flow/manage-access-token.test.ts → ...-flow/auth-code-flow/authenticate.test.ts
@@ -1,6 +1,6 @@
 import {SESSION_COOKIE_NAME, Session} from '@shopify/shopify-api';
 
-import {shopifyApp} from '../../../../..';
+import {shopifyApp} from '../../../../../..';
 import {
   APP_URL,
   BASE64_HOST,
@@ -12,9 +12,9 @@ import {
   setUpValidSession,
   testConfig,
   signRequestCookie,
-} from '../../../../../__test-helpers';
+} from '../../../../../../__test-helpers';
 
-describe('manageAccessToken', () => {
+describe('authenticate', () => {
   describe('errors', () => {
     it('redirects to exit-iframe if app is embedded and the session is no longer valid for the id_token when embedded', async () => {
       // GIVEN

diff --git a/...ode-flow/ensure-installed-on-shop.test.ts → ...ode-flow/ensure-installed-on-shop.test.ts b/...ode-flow/ensure-installed-on-shop.test.ts → ...ode-flow/ensure-installed-on-shop.test.ts
@@ -1,6 +1,6 @@
 import {LogSeverity, SESSION_COOKIE_NAME} from '@shopify/shopify-api';
 
-import {shopifyApp} from '../../../../..';
+import {shopifyApp} from '../../../../../..';
 import {
   API_KEY,
   APP_URL,
@@ -16,7 +16,7 @@ import {
   testConfig,
   signRequestCookie,
   mockExternalRequest,
-} from '../../../../../__test-helpers';
+} from '../../../../../../__test-helpers';
 
 describe('authorize.admin doc request path', () => {
   describe('errors', () => {

diff --git a/packages/shopify-app-remix/src/server/authenticate/admin/strategies/auth-code-flow.ts b/packages/shopify-app-remix/src/server/authenticate/admin/strategies/auth-code-flow.ts
@@ -56,6 +56,9 @@ export class AuthCodeFlowStrategy<
     }
 
     if (!getSessionTokenHeader(request)) {
+      // This is a document request that doesn't contain a session token. We check if the app is installed.
+      // If the app isn't installed, we initiate the OAuth auth code flow.
+      // Requests with a header can only happen after the app is installed.
       await this.ensureInstalledOnShop(request);
     }
   }
@@ -78,6 +81,8 @@ export class AuthCodeFlowStrategy<
       await redirectToAuthPage({config, logger, api}, request, shop);
     }
 
+    logger.debug('Found a valid session', {shop});
+
     return session!;
   }
 
@@ -191,29 +196,24 @@ export class AuthCodeFlowStrategy<
   private async getOfflineSession(
     request: Request,
   ): Promise<Session | undefined> {
-    const {api, config} = this;
-    const url = new URL(request.url);
-
-    const shop = url.searchParams.get('shop');
-
-    const offlineId = shop
-      ? api.session.getOfflineId(shop)
-      : await api.session.getCurrentId({isOnline: false, rawRequest: request});
-
-    return config.sessionStorage.loadSession(offlineId!);
+    const offlineId = await this.getOfflineSessionId(request);
+    return this.config.sessionStorage.loadSession(offlineId!);
   }
 
   private async hasValidOfflineId(request: Request) {
+    return Boolean(await this.getOfflineSessionId(request));
+  }
+
+  private async getOfflineSessionId(
+    request: Request,
+  ): Promise<string | undefined> {
     const {api} = this;
     const url = new URL(request.url);
-
     const shop = url.searchParams.get('shop');
 
-    const offlineId = shop
+    return shop
       ? api.session.getOfflineId(shop)
-      : await api.session.getCurrentId({isOnline: false, rawRequest: request});
-
-    return Boolean(offlineId);
+      : api.session.getCurrentId({isOnline: false, rawRequest: request});
   }
 
   private async testSession(session: Session): Promise<void> {