Merge pull request #1255 from Shopify/liz/refactor-hmac-validation

Refactor the validation of HMAC to be more reusable

Author: lizkenyon

.changeset/grumpy-numbers-count.md

@@ -0,0 +1,5 @@
+---
+"@shopify/shopify-api": patch
+---
+
+Refactor HMAC validation to use a common function.

packages/shopify-api/lib/flow/types.ts

@@ -1,3 +1,8 @@
+import {
+  ValidationErrorReason,
+  ValidationInvalid,
+  ValidationValid,
+} from '../utils/types';
 import {AdapterArgs} from '../../runtime/types';
 
 export interface FlowValidateParams extends AdapterArgs {
@@ -7,26 +12,19 @@ export interface FlowValidateParams extends AdapterArgs {
   rawBody: string;
 }
 
-export enum FlowValidationErrorReason {
-  MissingBody = 'missing_body',
-  MissingHmac = 'missing_hmac',
-  InvalidHmac = 'invalid_hmac',
-}
+export const FlowValidationErrorReason = {
+  ...ValidationErrorReason,
+} as const;
 
-export interface FlowValidationInvalid {
-  /**
-   * Whether the request is a valid Flow request from Shopify.
-   */
-  valid: false;
+export type FlowValidationErrorReasonType =
+  (typeof FlowValidationErrorReason)[keyof typeof FlowValidationErrorReason];
+
+export interface FlowValidationInvalid
+  extends Omit<ValidationInvalid, 'reason'> {
   /**
    * The reason why the request is not valid.
    */
-  reason: FlowValidationErrorReason;
+  reason: FlowValidationErrorReasonType;
 }
 
-export interface FlowValidationValid {
-  /**
-   * Whether the request is a valid Flow request from Shopify.
-   */
-  valid: true;
-}
+export interface FlowValidationValid extends ValidationValid {}

packages/shopify-api/lib/flow/validate.ts

@@ -1,60 +1,22 @@
-import {abstractConvertRequest, getHeader} from '../../runtime/http';
-import {HashFormat} from '../../runtime/crypto/types';
+import {
+  HmacValidationType,
+  ValidationInvalid,
+  ValidationValid,
+} from '../utils/types';
+import {validateHmacFromRequestFactory} from '../utils/hmac-validator';
 import {ConfigInterface} from '../base-types';
-import {logger} from '../logger';
-import {ShopifyHeader} from '../types';
-import {validateHmacString} from '../utils/hmac-validator';
 
-import {
-  FlowValidateParams,
-  FlowValidationInvalid,
-  FlowValidationValid,
-  FlowValidationErrorReason,
-} from './types';
+import {FlowValidateParams} from './types';
 
 export function validateFactory(config: ConfigInterface) {
   return async function validate({
     rawBody,
     ...adapterArgs
-  }: FlowValidateParams): Promise<FlowValidationInvalid | FlowValidationValid> {
-    const request = await abstractConvertRequest(adapterArgs);
-
-    if (!rawBody.length) {
-      return fail(FlowValidationErrorReason.MissingBody, config);
-    }
-
-    const hmac = getHeader(request.headers, ShopifyHeader.Hmac);
-
-    if (!hmac) {
-      return fail(FlowValidationErrorReason.MissingHmac, config);
-    }
-
-    if (await validateHmacString(config, rawBody, hmac, HashFormat.Base64)) {
-      return succeed(config);
-    }
-
-    return fail(FlowValidationErrorReason.InvalidHmac, config);
-  };
-}
-
-async function fail(
-  reason: FlowValidationErrorReason,
-  config: ConfigInterface,
-): Promise<FlowValidationInvalid> {
-  const log = logger(config);
-  await log.debug('Flow request is not valid', {reason});
-
-  return {
-    valid: false,
-    reason,
-  };
-}
-
-async function succeed(config: ConfigInterface): Promise<FlowValidationValid> {
-  const log = logger(config);
-  await log.debug('Flow request is valid');
-
-  return {
-    valid: true,
+  }: FlowValidateParams): Promise<ValidationInvalid | ValidationValid> {
+    return validateHmacFromRequestFactory(config)({
+      type: HmacValidationType.Flow,
+      rawBody,
+      ...adapterArgs,
+    });
   };
 }

packages/shopify-api/lib/utils/hmac-validator.ts

@@ -1,3 +1,10 @@
+import {logger} from '../logger';
+import {ShopifyHeader} from '../types';
+import {
+  AdapterArgs,
+  abstractConvertRequest,
+  getHeader,
+} from '../../runtime/http';
 import {ConfigInterface} from '../base-types';
 import {createSHA256HMAC} from '../../runtime/crypto';
 import {HashFormat} from '../../runtime/crypto/types';
@@ -6,11 +13,29 @@ import * as ShopifyErrors from '../error';
 import {safeCompare} from '../auth/oauth/safe-compare';
 
 import ProcessedQuery from './processed-query';
+import {
+  ValidationErrorReason,
+  ValidationInvalid,
+  HmacValidationType,
+  ValidationValid,
+  ValidationErrorReasonType,
+} from './types';
 
 const HMAC_TIMESTAMP_PERMITTED_CLOCK_TOLERANCE_SEC = 90;
 
 export type HMACSignator = 'admin' | 'appProxy';
 
+export interface ValidateParams extends AdapterArgs {
+  /**
+   * The type of validation to perform, either 'flow' or 'webhook'.
+   */
+  type: HmacValidationType;
+  /**
+   * The raw body of the request.
+   */
+  rawBody: string;
+}
+
 function stringifyQueryForAdmin(query: AuthQuery): string {
   const processedQuery = new ProcessedQuery();
   Object.keys(query)
@@ -85,6 +110,34 @@ export function getCurrentTimeInSec() {
   return Math.trunc(Date.now() / 1000);
 }
 
+export function validateHmacFromRequestFactory(config: ConfigInterface) {
+  return async function validateHmacFromRequest({
+    type,
+    rawBody,
+    ...adapterArgs
+  }: ValidateParams): Promise<ValidationInvalid | ValidationValid> {
+    const request = await abstractConvertRequest(adapterArgs);
+    if (!rawBody.length) {
+      return fail(ValidationErrorReason.MissingBody, type, config);
+    }
+    const hmac = getHeader(request.headers, ShopifyHeader.Hmac);
+    if (!hmac) {
+      return fail(ValidationErrorReason.MissingHmac, type, config);
+    }
+    const validHmac = await validateHmacString(
+      config,
+      rawBody,
+      hmac,
+      HashFormat.Base64,
+    );
+    if (!validHmac) {
+      return fail(ValidationErrorReason.InvalidHmac, type, config);
+    }
+
+    return succeed(type, config);
+  };
+}
+
 function validateHmacTimestamp(query: AuthQuery) {
   if (
     Math.abs(getCurrentTimeInSec() - Number(query.timestamp)) >
@@ -95,3 +148,29 @@ function validateHmacTimestamp(query: AuthQuery) {
     );
   }
 }
+
+async function fail(
+  reason: ValidationErrorReasonType,
+  type: HmacValidationType,
+  config: ConfigInterface,
+): Promise<ValidationInvalid> {
+  const log = logger(config);
+  await log.debug(`${type} request is not valid`, {reason});
+
+  return {
+    valid: false,
+    reason,
+  };
+}
+
+async function succeed(
+  type: HmacValidationType,
+  config: ConfigInterface,
+): Promise<ValidationValid> {
+  const log = logger(config);
+  await log.debug(`${type} request is valid`);
+
+  return {
+    valid: true,
+  };
+}

packages/shopify-api/lib/utils/types.ts

@@ -0,0 +1,31 @@
+export enum HmacValidationType {
+  Flow = 'flow',
+  Webhook = 'webhook',
+}
+
+export const ValidationErrorReason = {
+  MissingBody: 'missing_body',
+  InvalidHmac: 'invalid_hmac',
+  MissingHmac: 'missing_hmac',
+} as const;
+
+export type ValidationErrorReasonType =
+  (typeof ValidationErrorReason)[keyof typeof ValidationErrorReason];
+
+export interface ValidationInvalid {
+  /**
+   * Whether the request is a valid Flow request from Shopify.
+   */
+  valid: false;
+  /**
+   * The reason why the request is not valid.
+   */
+  reason: ValidationErrorReasonType;
+}
+
+export interface ValidationValid {
+  /**
+   * Whether the request is a valid request from Shopify.
+   */
+  valid: true;
+}

packages/shopify-api/lib/webhooks/__tests__/process.test.ts

@@ -248,7 +248,12 @@ describe('shopify.webhooks.process', () => {
     for (const header of emptyHeaders) {
       const response = await request(app)
         .post('/webhooks')
-        .set(headers(header))
+        .set(
+          headers({
+            hmac: hmac(shopify.config.apiSecretKey, rawBody),
+            ...header,
+          }),
+        )
         .send(rawBody)
         .expect(StatusCode.BadRequest);
 

packages/shopify-api/lib/webhooks/types.ts

@@ -1,3 +1,4 @@
+import {ValidationErrorReason, ValidationInvalid} from '../utils/types';
 import {AdapterArgs} from '../../runtime/types';
 import {Session} from '../session/session';
 
@@ -121,11 +122,13 @@ export interface WebhookProcessParams extends AdapterArgs {
 
 export interface WebhookValidateParams extends WebhookProcessParams {}
 
-export enum WebhookValidationErrorReason {
-  MissingHeaders = 'missing_headers',
-  MissingBody = 'missing_body',
-  InvalidHmac = 'invalid_hmac',
-}
+export const WebhookValidationErrorReason = {
+  ...ValidationErrorReason,
+  MissingHeaders: 'missing_headers',
+} as const;
+
+export type WebhookValidationErrorReasonType =
+  (typeof WebhookValidationErrorReason)[keyof typeof WebhookValidationErrorReason];
 
 export interface WebhookFields {
   webhookId: string;
@@ -136,14 +139,15 @@ export interface WebhookFields {
   subTopic?: string;
 }
 
-export interface WebhookValidationInvalid {
+export interface WebhookValidationInvalid
+  extends Omit<ValidationInvalid, 'reason'> {
   valid: false;
-  reason: WebhookValidationErrorReason;
+  reason: WebhookValidationErrorReasonType;
 }
 
 export interface WebhookValidationMissingHeaders
   extends WebhookValidationInvalid {
-  reason: WebhookValidationErrorReason.MissingHeaders;
+  reason: typeof WebhookValidationErrorReason.MissingHeaders;
   missingHeaders: string[];
 }