Improvements on OAuth refactoring

packages/shopify-app-remix/src/server/authenticate/admin/authenticate.ts

@@ -1,20 +1,13 @@
-import {redirect} from '@remix-run/server-runtime';
-import {
-  JwtPayload,
-  Session,
-  Shopify,
-  ShopifyRestResources,
-} from '@shopify/shopify-api';
+import {Session, Shopify, ShopifyRestResources} from '@shopify/shopify-api';
 
 import type {BasicParams} from '../../types';
 import type {AppConfig, AppConfigArg} from '../../config-types';
 import {
   getSessionTokenHeader,
-  validateSessionToken,
-  rejectBotRequest as respondToBotRequest,
-  respondToOptionsRequest,
   ensureCORSHeadersFactory,
   getSessionTokenFromUrlParam,
+  respondToBotRequest,
+  respondToOptionsRequest,
 } from '../helpers';
 
 import type {BillingContext} from './billing/types';
@@ -32,18 +25,10 @@ import type {
 import {
   createAdminApiContext,
   redirectFactory,
-  redirectToShopifyOrAppRoot,
   renderAppBridge,
 } from './helpers';
 import {AuthorizationStrategy} from './strategies/types';
 
-const SESSION_TOKEN_PARAM = 'id_token';
-
-interface ShopWithSessionContext {
-  sessionContext?: SessionContext;
-  shop: string;
-}
-
 interface AuthStrategyParams extends BasicParams {
   strategy: AuthorizationStrategy;
 }
@@ -67,50 +52,40 @@ export class AuthStrategy<
   public async authenticateAdmin(
     request: Request,
   ): Promise<AdminContext<Config, Resources>> {
-    const {api, logger, config} = this;
+    const {config, logger} = this;
+    const params = {api: this.api, logger, config};
 
-    let sessionContext: SessionContext;
     try {
-      respondToBotRequest({api, logger, config}, request);
-      respondToOptionsRequest({api, logger, config}, request);
-      await this.respondToBouncePageRequest(request);
-      await this.respondToExitIframeRequest(request);
-      await this.strategy.respondToOAuthRequests(request);
-
-      logger.info('Authenticating admin request');
+      respondToBotRequest(params, request);
+      respondToOptionsRequest(params, request);
+      await this.respondToBouncePageRequest(params, request);
+      await this.respondToExitIframeRequest(params, request);
+      await this.strategy.authenticate(request);
 
-      const sessionTokenHeader = getSessionTokenHeader(request);
+      // If the request is authenticated, we must have a session token
+      const sessionToken = (getSessionTokenHeader(request) ||
+        getSessionTokenFromUrlParam(request))!;
 
-      if (!sessionTokenHeader) {
-        await this.validateUrlParams(request);
-        await this.strategy.ensureInstalledOnShop(request);
-        await this.ensureAppIsEmbeddedIfRequired(request);
-        await this.ensureSessionTokenSearchParamIfRequired(request);
-      }
-
-      const authenticatedSession = await this.loadActiveSession(request);
-
-      sessionContext = await this.strategy.acquireAccessToken({
-        sessionContext: authenticatedSession.sessionContext,
-        shop: authenticatedSession.shop,
+      const sessionContext = await this.strategy.exchangeToken(
         request,
-      });
+        sessionToken,
+      );
+
+      return this.createContext(request, sessionContext);
     } catch (errorOrResponse) {
       if (errorOrResponse instanceof Response) {
-        ensureCORSHeadersFactory(
-          {api, logger, config},
-          request,
-        )(errorOrResponse);
+        ensureCORSHeadersFactory(params, request)(errorOrResponse);
       }
 
       throw errorOrResponse;
     }
-
-    return this.createContext(request, sessionContext);
   }
 
-  private async respondToBouncePageRequest(request: Request) {
-    const {config, logger, api} = this;
+  private async respondToBouncePageRequest(
+    params: BasicParams,
+    request: Request,
+  ) {
+    const {config, logger, api} = params;
     const url = new URL(request.url);
 
     if (url.pathname === config.auth.patchSessionTokenPath) {
@@ -119,8 +94,11 @@ export class AuthStrategy<
     }
   }
 
-  private async respondToExitIframeRequest(request: Request) {
-    const {config, logger, api} = this;
+  private async respondToExitIframeRequest(
+    params: BasicParams,
+    request: Request,
+  ) {
+    const {config, logger, api} = params;
     const url = new URL(request.url);
 
     if (url.pathname === config.auth.exitIframePath) {
@@ -131,128 +109,6 @@ export class AuthStrategy<
     }
   }
 
-  private async validateUrlParams(request: Request) {
-    const {api, config, logger} = this;
-
-    if (config.isEmbeddedApp) {
-      const url = new URL(request.url);
-      const shop = api.utils.sanitizeShop(url.searchParams.get('shop')!);
-      if (!shop) {
-        logger.debug('Missing or invalid shop, redirecting to login path', {
-          shop,
-        });
-        throw redirect(config.auth.loginPath);
-      }
-
-      const host = api.utils.sanitizeHost(url.searchParams.get('host')!);
-      if (!host) {
-        logger.debug('Invalid host, redirecting to login path', {
-          host: url.searchParams.get('host'),
-        });
-        throw redirect(config.auth.loginPath);
-      }
-    }
-  }
-
-  private async ensureAppIsEmbeddedIfRequired(request: Request) {
-    const {api, logger, config} = this;
-    const url = new URL(request.url);
-
-    const shop = url.searchParams.get('shop')!;
-
-    if (api.config.isEmbeddedApp && url.searchParams.get('embedded') !== '1') {
-      logger.debug('App is not embedded, redirecting to Shopify', {shop});
-      await redirectToShopifyOrAppRoot(request, {api, logger, config});
-    }
-  }
-
-  private async ensureSessionTokenSearchParamIfRequired(request: Request) {
-    const {api, logger} = this;
-    const url = new URL(request.url);
-
-    const shop = url.searchParams.get('shop')!;
-    const searchParamSessionToken = url.searchParams.get(SESSION_TOKEN_PARAM);
-    const isEmbedded = url.searchParams.get('embedded') === '1';
-
-    if (api.config.isEmbeddedApp && isEmbedded && !searchParamSessionToken) {
-      logger.debug(
-        'Missing session token in search params, going to bounce page',
-        {shop},
-      );
-      this.redirectToBouncePage(url);
-    }
-  }
-
-  private getShopFromSessionToken(payload: JwtPayload): string {
-    const dest = new URL(payload.dest);
-    return dest.hostname;
-  }
-
-  private async loadActiveSession(
-    request: Request,
-  ): Promise<ShopWithSessionContext> {
-    const {config, logger, api} = this;
-
-    let shop: string;
-    let sessionId: string | undefined;
-    let payload: JwtPayload | undefined;
-
-    const sessionToken = (getSessionTokenHeader(request) ||
-      getSessionTokenFromUrlParam(request))!;
-
-    if (config.isEmbeddedApp) {
-      payload = await validateSessionToken({config, logger, api}, sessionToken);
-      shop = this.getShopFromSessionToken(payload);
-
-      logger.debug('Session token is present, validating session', {shop});
-      sessionId = config.useOnlineTokens
-        ? api.session.getJwtSessionId(shop, payload.sub)
-        : api.session.getOfflineId(shop);
-    } else {
-      const url = new URL(request.url);
-      shop = url.searchParams.get('shop')!;
-
-      // eslint-disable-next-line no-warning-comments
-      // TODO move this check into loadSession once we add support for it in the library
-      // https://github.com/orgs/Shopify/projects/6899/views/1?pane=issue&itemId=28378114
-      sessionId = await api.session.getCurrentId({
-        isOnline: config.useOnlineTokens,
-        rawRequest: request,
-      });
-    }
-
-    if (!sessionId) {
-      logger.debug('Session id not found in cookies, redirecting to OAuth', {
-        shop,
-      });
-      return {shop};
-    }
-
-    logger.debug('Loading session from storage', {sessionId});
-
-    const session = await config.sessionStorage.loadSession(sessionId);
-
-    logger.debug('Found session, request is valid', {shop});
-
-    return {sessionContext: session && {session, token: payload}, shop};
-  }
-
-  private redirectToBouncePage(url: URL): never {
-    const {config} = this;
-
-    // Make sure we always point to the configured app URL so it also works behind reverse proxies (that alter the Host
-    // header).
-    url.searchParams.set(
-      'shopify-reload',
-      `${config.appUrl}${url.pathname}${url.search}`,
-    );
-
-    // eslint-disable-next-line no-warning-comments
-    // TODO Make sure this works on chrome without a tunnel (weird HTTPS redirect issue)
-    // https://github.com/orgs/Shopify/projects/6899/views/1?pane=issue&itemId=28376650
-    throw redirect(`${config.auth.patchSessionTokenPath}${url.search}`);
-  }
-
   private createContext(
     request: Request,
     sessionContext: SessionContext,

packages/shopify-app-remix/src/server/authenticate/admin/helpers/ensure-app-is-embedded-if-required.ts

@@ -0,0 +1,18 @@
+import {BasicParams} from '../../../types';
+
+import {redirectToShopifyOrAppRoot} from './redirect-to-shopify-or-app-root';
+
+export const ensureAppIsEmbeddedIfRequired = async (
+  params: BasicParams,
+  request: Request,
+) => {
+  const {api, logger, config} = params;
+  const url = new URL(request.url);
+
+  const shop = url.searchParams.get('shop')!;
+
+  if (api.config.isEmbeddedApp && url.searchParams.get('embedded') !== '1') {
+    logger.debug('App is not embedded, redirecting to Shopify', {shop});
+    await redirectToShopifyOrAppRoot(request, {api, logger, config});
+  }
+};

packages/shopify-app-remix/src/server/authenticate/admin/helpers/ensure-session-token-search-param-if-required.ts

@@ -0,0 +1,25 @@
+import {BasicParams} from '../../../types';
+
+import {redirectToBouncePage} from './redirect-to-bounce-page';
+
+const SESSION_TOKEN_PARAM = 'id_token';
+
+export const ensureSessionTokenSearchParamIfRequired = async (
+  params: BasicParams,
+  request: Request,
+) => {
+  const {api, logger} = params;
+  const url = new URL(request.url);
+
+  const shop = url.searchParams.get('shop')!;
+  const searchParamSessionToken = url.searchParams.get(SESSION_TOKEN_PARAM);
+  const isEmbedded = url.searchParams.get('embedded') === '1';
+
+  if (api.config.isEmbeddedApp && isEmbedded && !searchParamSessionToken) {
+    logger.debug(
+      'Missing session token in search params, going to bounce page',
+      {shop},
+    );
+    redirectToBouncePage(params, url);
+  }
+};

packages/shopify-app-remix/src/server/authenticate/admin/helpers/index.ts

@@ -1,7 +1,10 @@
 export * from './begin-auth';
 export * from './create-admin-api-context';
+export * from './ensure-app-is-embedded-if-required';
+export * from './ensure-session-token-search-param-if-required';
 export * from './handle-client-error';
 export * from './redirect-to-auth-page';
+export * from './redirect-to-bounce-page';
 export * from './redirect-to-shopify-or-app-root';
 export * from './redirect-with-app-bridge-headers';
 export * from './redirect-with-exitiframe';

packages/shopify-app-remix/src/server/authenticate/admin/helpers/redirect-to-bounce-page.ts

@@ -0,0 +1,19 @@
+import {redirect} from '@remix-run/server-runtime';
+
+import {BasicParams} from '../../../types';
+
+export const redirectToBouncePage = (params: BasicParams, url: URL): never => {
+  const {config} = params;
+
+  // Make sure we always point to the configured app URL so it also works behind reverse proxies (that alter the Host
+  // header).
+  url.searchParams.set(
+    'shopify-reload',
+    `${config.appUrl}${url.pathname}${url.search}`,
+  );
+
+  // eslint-disable-next-line no-warning-comments
+  // TODO Make sure this works on chrome without a tunnel (weird HTTPS redirect issue)
+  // https://github.com/orgs/Shopify/projects/6899/views/1?pane=issue&itemId=28376650
+  throw redirect(`${config.auth.patchSessionTokenPath}${url.search}`);
+};