do not require schema version on manifest handler (#412)

* do not require schema version on manifest handler

* fix case where saleor schema version was converted to float

* fix case where saleor schema version was converted to float

* changeset

* fix buildResponse

Author: lkostrowski

.changeset/hot-pens-attend.md

@@ -0,0 +1,5 @@
+---
+"@saleor/app-sdk": patch
+---
+
+Schema version passed to manifest handler will be string, not float

.changeset/silent-walls-heal.md

@@ -39,7 +39,7 @@ describe("ManifestActionHandler", () => {
     expect(manifestFactory).toHaveBeenCalledWith({
       appBaseUrl: "http://example.com",
       request: {},
-      schemaVersion: 3.20,
+      schemaVersion: "3.20",
     });
   });
 
@@ -64,18 +64,21 @@ describe("ManifestActionHandler", () => {
     expect(result.status).toBe(405);
     expect(result.body).toBe("Method not allowed");
     expect(manifestFactory).not.toHaveBeenCalled();
-  })
+  });
 
-  it("should return 400 when receives null schema version header from unsupported legacy Saleor version", async () => {
+  /**
+   * api/manifest endpoint is GET and header should be optional. It can be used to install the app eventually,
+   * but also to preview the manifest from the plain GET request
+   */
+  it("should NOT return 400 when receives null schema version header from unsupported legacy Saleor version", async () => {
     adapter.getHeader = vi.fn().mockReturnValue(null);
     const handler = new ManifestActionHandler(adapter);
 
     const manifestFactory = vi.fn().mockResolvedValue(mockManifest);
 
     const result = await handler.handleAction({ manifestFactory });
 
-    expect(result.status).toBe(400);
-    expect(result.body).toBe("Missing schema version header");
-    expect(manifestFactory).not.toHaveBeenCalled();
+    expect(result.status).toBe(200);
+    expect(manifestFactory).toHaveBeenCalled();
   });
 });

src/handlers/actions/manifest-action-handler.test.ts

@@ -14,8 +14,12 @@ export type CreateManifestHandlerOptions<T> = {
   manifestFactory(context: {
     appBaseUrl: string;
     request: T;
-    /** Added in Saleor 3.15 */
-    schemaVersion: number;
+    /**
+     * Schema version is optional. During installation, Saleor will send it,
+     * so manifest can be generated according to the version. But it may
+     * be also requested from plain GET from the browser, so it may not be available
+     */
+    schemaVersion?: string;
   }): AppManifest | Promise<AppManifest>;
 };
 
@@ -36,14 +40,6 @@ export class ManifestActionHandler<I> implements ActionHandlerInterface {
       return invalidMethodResponse;
     }
 
-    if (!schemaVersion) {
-      return {
-        status: 400,
-        bodyType: "string",
-        body: "Missing schema version header",
-      };
-    }
-
     try {
       const manifest = await options.manifestFactory({
         appBaseUrl: baseURL,

src/handlers/actions/manifest-action-handler.ts

@@ -43,8 +43,8 @@ describe("AWS Lambda createManifestHandler", () => {
       expect.objectContaining({
         appBaseUrl: expectedBaseUrl,
         request: event,
-        schemaVersion: 3.2,
-      })
+        schemaVersion: "3.20",
+      }),
     );
     expect(response.statusCode).toBe(200);
     expect(JSON.parse(response.body!)).toStrictEqual({
@@ -98,8 +98,8 @@ describe("AWS Lambda createManifestHandler", () => {
       expect.objectContaining({
         appBaseUrl: expectedBaseUrl,
         request: event,
-        schemaVersion: 3.2,
-      })
+        schemaVersion: "3.20",
+      }),
     );
     expect(response.statusCode).toBe(200);
     expect(JSON.parse(response.body!)).toStrictEqual({

src/handlers/platforms/aws-lambda/create-manifest-handler.test.ts

@@ -7,19 +7,19 @@ import { AwsLambdaWebhookHandler, SaleorWebApiWebhook, WebhookConfig } from "./s
 
 export type AwsLambdaSyncWebhookHandler<
   TPayload,
-  TEvent extends SyncWebhookEventType = SyncWebhookEventType
+  TEvent extends SyncWebhookEventType = SyncWebhookEventType,
 > = AwsLambdaWebhookHandler<TPayload, SyncWebhookInjectedContext<TEvent>>;
 
 export class SaleorSyncWebhook<
   TPayload = unknown,
-  TEvent extends SyncWebhookEventType = SyncWebhookEventType
+  TEvent extends SyncWebhookEventType = SyncWebhookEventType,
 > extends SaleorWebApiWebhook<TPayload, SyncWebhookInjectedContext<TEvent>> {
   readonly event: TEvent;
 
   protected readonly eventType = "sync" as const;
 
   protected extraContext = {
-    buildResponse: buildSyncWebhookResponsePayload,
+    buildResponse: buildSyncWebhookResponsePayload<TEvent>,
   };
 
   constructor(configuration: WebhookConfig<TEvent>) {

src/handlers/platforms/aws-lambda/saleor-webhooks/saleor-sync-webhook.ts

@@ -36,7 +36,7 @@ describe("Fetch API createManifestHandler", () => {
     expect(mockManifestFactory).toHaveBeenCalledWith({
       appBaseUrl: baseUrl,
       request,
-      schemaVersion: 3.2,
+      schemaVersion: "3.20",
     });
     expect(response.status).toBe(200);
     await expect(response.json()).resolves.toStrictEqual({

src/handlers/platforms/fetch-api/create-manifest-handler.test.ts

@@ -38,7 +38,7 @@ describe("Next.js createManifestHandler", () => {
     expect(mockManifestFactory).toHaveBeenCalledWith({
       appBaseUrl: baseUrl,
       request: req,
-      schemaVersion: 3.2,
+      schemaVersion: "3.20",
     });
 
     expect(res.statusCode).toBe(200);

src/handlers/platforms/next/create-manifest-handler.test.ts

@@ -89,7 +89,7 @@ describe("SaleorRequestProcessor", () => {
         signature: "signature-value",
         event: "event-name",
         saleorApiUrl: "https://api.saleor.io",
-        schemaVersion: 3.2,
+        schemaVersion: "3.20",
       });
     });
 

src/handlers/shared/saleor-request-processor.test.ts

@@ -40,18 +40,20 @@ export class SaleorRequestProcessor<T> {
   private toStringOrUndefined = (value: string | string[] | undefined | null) =>
     value ? value.toString() : undefined;
 
-  private toFloatOrNull = (value: string | string[] | undefined | null) =>
-    value ? parseFloat(value.toString()) : undefined;
-
   getSaleorHeaders() {
     return {
       authorizationBearer: this.toStringOrUndefined(
-        this.adapter.getHeader(SALEOR_AUTHORIZATION_BEARER_HEADER)
+        this.adapter.getHeader(SALEOR_AUTHORIZATION_BEARER_HEADER),
       ),
       signature: this.toStringOrUndefined(this.adapter.getHeader(SALEOR_SIGNATURE_HEADER)),
       event: this.toStringOrUndefined(this.adapter.getHeader(SALEOR_EVENT_HEADER)),
       saleorApiUrl: this.toStringOrUndefined(this.adapter.getHeader(SALEOR_API_URL_HEADER)),
-      schemaVersion: this.toFloatOrNull(this.adapter.getHeader(SALEOR_SCHEMA_VERSION)),
+      /**
+       * Schema version must remain string. Since format is "x.x" like "3.20" for javascript it's a floating numer - so it's 3.2
+       * Saleor version 3.20 != 3.2.
+       * Semver must be compared as strings
+       */
+      schemaVersion: this.toStringOrUndefined(this.adapter.getHeader(SALEOR_SCHEMA_VERSION)),
     };
   }
 }

src/handlers/shared/saleor-request-processor.ts

@@ -14,14 +14,14 @@ vi.spyOn(verifySignatureModule, "verifySignatureFromApiUrl").mockImplementation(
     if (signature !== "mocked_signature") {
       throw new Error("Wrong signature");
     }
-  }
+  },
 );
 vi.spyOn(verifySignatureModule, "verifySignatureWithJwks").mockImplementation(
   async (domain, signature) => {
     if (signature !== "mocked_signature") {
       throw new Error("Wrong signature");
     }
-  }
+  },
 );
 
 describe("SaleorWebhookValidator", () => {
@@ -33,7 +33,7 @@ describe("SaleorWebhookValidator", () => {
   const validHeaders = {
     saleorApiUrl: mockAPL.workingSaleorApiUrl,
     event: "product_updated",
-    schemaVersion: 3.2,
+    schemaVersion: "3.20",
     signature: "mocked_signature",
     authorizationBearer: "mocked_bearer",
     domain: "example.com",
@@ -316,10 +316,10 @@ describe("SaleorWebhookValidator", () => {
       expect(mockAPL.set).toHaveBeenCalledWith(
         expect.objectContaining({
           jwks: "new-jwks",
-        })
+        }),
       );
       expect(fetchRemoteJwksModule.fetchRemoteJwks).toHaveBeenCalledWith(
-        authDataNoJwks.saleorApiUrl
+        authDataNoJwks.saleorApiUrl,
       );
       // it's called only once because jwks was missing initially, so we skipped first validation
       expect(verifySignatureModule.verifySignatureWithJwks).toHaveBeenCalledTimes(1);
@@ -351,21 +351,21 @@ describe("SaleorWebhookValidator", () => {
       expect(mockAPL.set).toHaveBeenCalledWith(
         expect.objectContaining({
           jwks: "new-jwks",
-        })
+        }),
       );
       expect(fetchRemoteJwksModule.fetchRemoteJwks).toHaveBeenCalledWith(
-        authDataNoJwks.saleorApiUrl
+        authDataNoJwks.saleorApiUrl,
       );
       expect(verifySignatureModule.verifySignatureWithJwks).toHaveBeenCalledTimes(2);
     });
 
     it("Returns an error when new JWKS cannot be fetched", async () => {
       vi.spyOn(mockAPL, "get").mockResolvedValue(authDataNoJwks);
       vi.spyOn(verifySignatureModule, "verifySignatureWithJwks").mockRejectedValue(
-        new Error("Initial verification failed")
+        new Error("Initial verification failed"),
       );
       vi.spyOn(fetchRemoteJwksModule, "fetchRemoteJwks").mockRejectedValue(
-        new Error("JWKS fetch failed")
+        new Error("JWKS fetch failed"),
       );
 
       const result = await validator.validateRequest({
@@ -408,7 +408,7 @@ describe("SaleorWebhookValidator", () => {
 
       expect(verifySignatureModule.verifySignatureWithJwks).toHaveBeenCalledTimes(2);
       expect(fetchRemoteJwksModule.fetchRemoteJwks).toHaveBeenCalledWith(
-        authDataNoJwks.saleorApiUrl
+        authDataNoJwks.saleorApiUrl,
       );
     });
   });