USe a factory function rather than a class for authenticate. The base…

… functionality for authenticate is not complex enough for a Class, and just adds more ceremony
Shopify · Nov 23, 2023 · 126ca2d · 126ca2d
1 parent 5ba059c
commit 126ca2d
Show file tree

Hide file tree

Showing 2 changed files with 64 additions and 96 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
@@ -1,7 +1,7 @@
-import {Session, Shopify, ShopifyRestResources} from '@shopify/shopify-api';
+import {ShopifyRestResources} from '@shopify/shopify-api';
 
 import type {BasicParams} from '../../types';
-import type {AppConfig, AppConfigArg} from '../../config-types';
+import type {AppConfigArg} from '../../config-types';
 import {
   getSessionTokenHeader,
   ensureCORSHeadersFactory,
@@ -10,14 +10,14 @@ import {
   respondToOptionsRequest,
 } from '../helpers';
 
-import type {BillingContext} from './billing/types';
 import {
   cancelBillingFactory,
   requestBillingFactory,
   requireBillingFactory,
 } from './billing';
 import type {
   AdminContext,
+  AuthenticateAdmin,
   EmbeddedAdminContext,
   NonEmbeddedAdminContext,
   SessionContext,
@@ -33,73 +33,16 @@ interface AuthStrategyParams extends BasicParams {
   strategy: AuthorizationStrategy;
 }
 
-export class AuthStrategy<
-  Config extends AppConfigArg,
+export function authStrategyFactory<
+  ConfigArg extends AppConfigArg,
   Resources extends ShopifyRestResources = ShopifyRestResources,
-> {
-  protected strategy: AuthorizationStrategy;
-  protected api: Shopify;
-  protected config: AppConfig;
-  protected logger: Shopify['logger'];
-
-  public constructor({strategy, api, config, logger}: AuthStrategyParams) {
-    this.api = api;
-    this.config = config;
-    this.logger = logger;
-    this.strategy = strategy;
-  }
-
-  public async authenticateAdmin(
-    request: Request,
-  ): Promise<AdminContext<Config, Resources>> {
-    const {config, logger, api} = this;
-    const params = {api, logger, config};
-
-    try {
-      respondToBotRequest(params, request);
-      respondToOptionsRequest(params, request);
-      await this.respondToBouncePageRequest(params, request);
-      await this.respondToExitIframeRequest(params, request);
-      await this.strategy.respondToOAuthRequests(request);
-
-      logger.info('Authenticating admin request');
-
-      const headerSessionToken = getSessionTokenHeader(request);
-      const searchParamSessionToken = getSessionTokenFromUrlParam(request);
-      const sessionToken = (headerSessionToken || searchParamSessionToken)!;
-
-      logger.debug('Attempting to authenticate session token', {
-        sessionToken: {
-          header: headerSessionToken,
-          search: searchParamSessionToken,
-        },
-      });
-
-      const sessionContext = await this.strategy.authenticate(
-        request,
-        sessionToken,
-      );
-
-      logger.debug('Request is valid, loaded session from session token', {
-        shop: sessionContext.session.shop,
-        isOnline: sessionContext.session.isOnline,
-      });
-
-      return this.createContext(request, sessionContext);
-    } catch (errorOrResponse) {
-      if (errorOrResponse instanceof Response) {
-        ensureCORSHeadersFactory(params, request)(errorOrResponse);
-      }
-
-      throw errorOrResponse;
-    }
-  }
+>({
+  strategy,
+  ...params
+}: AuthStrategyParams): AuthenticateAdmin<ConfigArg, Resources> {
+  const {api, logger, config} = params;
 
-  private async respondToBouncePageRequest(
-    params: BasicParams,
-    request: Request,
-  ) {
-    const {config, logger, api} = params;
+  async function respondToBouncePageRequest(request: Request) {
     const url = new URL(request.url);
 
     if (url.pathname === config.auth.patchSessionTokenPath) {
@@ -108,11 +51,7 @@ export class AuthStrategy<
     }
   }
 
-  private async respondToExitIframeRequest(
-    params: BasicParams,
-    request: Request,
-  ) {
-    const {config, logger, api} = params;
+  async function respondToExitIframeRequest(request: Request) {
     const url = new URL(request.url);
 
     if (url.pathname === config.auth.exitIframePath) {
@@ -123,21 +62,25 @@ export class AuthStrategy<
     }
   }
 
-  private createContext(
+  function createContext(
     request: Request,
     sessionContext: SessionContext,
-  ): AdminContext<Config, Resources> {
-    const {api, logger, config} = this;
+  ): AdminContext<ConfigArg, Resources> {
+    const {session} = sessionContext;
 
     const context:
-      | EmbeddedAdminContext<Config, Resources>
-      | NonEmbeddedAdminContext<Config, Resources> = {
+      | EmbeddedAdminContext<ConfigArg, Resources>
+      | NonEmbeddedAdminContext<ConfigArg, Resources> = {
       admin: createAdminApiContext<Resources>(request, sessionContext.session, {
         api,
         logger,
         config,
       }),
-      billing: this.createBillingContext(request, sessionContext.session),
+      billing: {
+        require: requireBillingFactory({api, logger, config}, request, session),
+        request: requestBillingFactory({api, logger, config}, request, session),
+        cancel: cancelBillingFactory({api, logger, config}, request, session),
+      },
       session: sessionContext.session,
       cors: ensureCORSHeadersFactory({api, logger, config}, request),
     };
@@ -147,22 +90,47 @@ export class AuthStrategy<
         ...context,
         sessionToken: sessionContext!.token!,
         redirect: redirectFactory({api, config, logger}, request),
-      } as AdminContext<Config, Resources>;
+      } as AdminContext<ConfigArg, Resources>;
     } else {
-      return context as AdminContext<Config, Resources>;
+      return context as AdminContext<ConfigArg, Resources>;
     }
   }
 
-  private createBillingContext(
-    request: Request,
-    session: Session,
-  ): BillingContext<Config> {
-    const {api, logger, config} = this;
-
-    return {
-      require: requireBillingFactory({api, logger, config}, request, session),
-      request: requestBillingFactory({api, logger, config}, request, session),
-      cancel: cancelBillingFactory({api, logger, config}, request, session),
-    };
-  }
+  return async function authenticateAdmin(request: Request) {
+    try {
+      respondToBotRequest(params, request);
+      respondToOptionsRequest(params, request);
+      await respondToBouncePageRequest(request);
+      await respondToExitIframeRequest(request);
+      await strategy.respondToOAuthRequests(request);
+
+      logger.info('Authenticating admin request');
+
+      const headerSessionToken = getSessionTokenHeader(request);
+      const searchParamSessionToken = getSessionTokenFromUrlParam(request);
+      const sessionToken = (headerSessionToken || searchParamSessionToken)!;
+
+      logger.debug('Attempting to authenticate session token', {
+        sessionToken: {
+          header: headerSessionToken,
+          search: searchParamSessionToken,
+        },
+      });
+
+      const sessionContext = await strategy.authenticate(request, sessionToken);
+
+      logger.debug('Request is valid, loaded session from session token', {
+        shop: sessionContext.session.shop,
+        isOnline: sessionContext.session.isOnline,
+      });
+
+      return createContext(request, sessionContext);
+    } catch (errorOrResponse) {
+      if (errorOrResponse instanceof Response) {
+        ensureCORSHeadersFactory(params, request)(errorOrResponse);
+      }
+
+      throw errorOrResponse;
+    }
+  };
 }
diff --git a/packages/shopify-app-remix/src/server/shopify-app.ts b/packages/shopify-app-remix/src/server/shopify-app.ts
@@ -21,7 +21,7 @@ import {
 } from './types';
 import {SHOPIFY_REMIX_LIBRARY_VERSION} from './version';
 import {registerWebhooksFactory} from './authenticate/webhooks';
-import {AuthStrategy} from './authenticate/admin/authenticate';
+import {authStrategyFactory} from './authenticate/admin/authenticate';
 import {authenticateWebhookFactory} from './authenticate/webhooks/authenticate';
 import {overrideLogger} from './override-logger';
 import {addDocumentResponseHeadersFactory} from './authenticate/helpers';
@@ -66,7 +66,7 @@ export function shopifyApp<
 
   const params: BasicParams = {api, config, logger};
   const oauth = new AuthCodeFlowStrategy(params);
-  const authStrategy = new AuthStrategy<Config, Resources>({
+  const authStrategy = authStrategyFactory<Config, Resources>({
     ...params,
     strategy: oauth,
   });
@@ -79,7 +79,7 @@ export function shopifyApp<
     addDocumentResponseHeaders: addDocumentResponseHeadersFactory(params),
     registerWebhooks: registerWebhooksFactory(params),
     authenticate: {
-      admin: authStrategy.authenticateAdmin.bind(authStrategy),
+      admin: authStrategy,
       public: authenticatePublicFactory<Config['future'], Resources>(params),
       webhook: authenticateWebhookFactory<
         Config['future'],