diff --git a/.changeset/hot-pens-attend.md b/.changeset/hot-pens-attend.md new file mode 100644 index 00000000..f014b108 --- /dev/null +++ b/.changeset/hot-pens-attend.md @@ -0,0 +1,5 @@ +--- +"@saleor/app-sdk": patch +--- + +Schema version passed to manifest handler will be string, not float diff --git a/.changeset/silent-walls-heal.md b/.changeset/silent-walls-heal.md deleted file mode 100644 index c23fcfac..00000000 --- a/.changeset/silent-walls-heal.md +++ /dev/null @@ -1,92 +0,0 @@ ---- -"@saleor/app-sdk": major ---- - -`createManifestHandler` will now require `saleor-schema-version` header, sent by Saleor when fetching manifest. -`schemaVersion` parameter passed to factory method will be always defined - -### Previously: - -```ts -const handler = createManifestHandler({ - manifestFactory({ schemaVersion }) { - schemaVersion -> null or number - return { - // ... - } - } -}) -``` - -Example request: - -```http -GET /api/manifest -host: my-app.com -``` - -```http -GET /api/manifest -host: my-app.com -saleor-schema-version: 3.20 -``` - -Example response: - -```http -Content-Type: application/json - -{ - "name": "Example Saleor App" - ... -} -``` - -### Now: - -```ts -const handler = createManifestHandler({ - manifestFactory({ schemaVersion }) { - schemaVersion -> number - return { - // ... - } - } -}) -``` - -### Invalid request - -```http -GET /api/manifest -host: my-app.com -``` - -Response: - -```http -HTTP 400 -Content-Type: text/plain - -Missing schema version header -``` - -### Valid request - -```http -GET /api/manifest -host: my-app.com -saleor-schema-version: 3.20 -``` - -Response: - -```http -HTTP 200 -Content-Type: application/json - -{ - "name": "Example Saleor App" - ... -} -``` diff --git a/src/handlers/actions/manifest-action-handler.test.ts b/src/handlers/actions/manifest-action-handler.test.ts index c13fa9d2..cdee68b9 100644 --- a/src/handlers/actions/manifest-action-handler.test.ts +++ b/src/handlers/actions/manifest-action-handler.test.ts @@ -39,7 +39,7 @@ describe("ManifestActionHandler", () => { expect(manifestFactory).toHaveBeenCalledWith({ appBaseUrl: "http://example.com", request: {}, - schemaVersion: 3.20, + schemaVersion: "3.20", }); }); @@ -64,9 +64,13 @@ 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); @@ -74,8 +78,7 @@ describe("ManifestActionHandler", () => { 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(); }); }); diff --git a/src/handlers/actions/manifest-action-handler.ts b/src/handlers/actions/manifest-action-handler.ts index 78899693..1ae409b2 100644 --- a/src/handlers/actions/manifest-action-handler.ts +++ b/src/handlers/actions/manifest-action-handler.ts @@ -14,8 +14,12 @@ export type CreateManifestHandlerOptions = { 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; }; @@ -36,14 +40,6 @@ export class ManifestActionHandler implements ActionHandlerInterface { return invalidMethodResponse; } - if (!schemaVersion) { - return { - status: 400, - bodyType: "string", - body: "Missing schema version header", - }; - } - try { const manifest = await options.manifestFactory({ appBaseUrl: baseURL, diff --git a/src/handlers/platforms/aws-lambda/create-manifest-handler.test.ts b/src/handlers/platforms/aws-lambda/create-manifest-handler.test.ts index e5173d4a..7f4c2d1a 100644 --- a/src/handlers/platforms/aws-lambda/create-manifest-handler.test.ts +++ b/src/handlers/platforms/aws-lambda/create-manifest-handler.test.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({ diff --git a/src/handlers/platforms/aws-lambda/saleor-webhooks/saleor-sync-webhook.ts b/src/handlers/platforms/aws-lambda/saleor-webhooks/saleor-sync-webhook.ts index 6f4c56ea..c28d817f 100644 --- a/src/handlers/platforms/aws-lambda/saleor-webhooks/saleor-sync-webhook.ts +++ b/src/handlers/platforms/aws-lambda/saleor-webhooks/saleor-sync-webhook.ts @@ -7,19 +7,19 @@ import { AwsLambdaWebhookHandler, SaleorWebApiWebhook, WebhookConfig } from "./s export type AwsLambdaSyncWebhookHandler< TPayload, - TEvent extends SyncWebhookEventType = SyncWebhookEventType + TEvent extends SyncWebhookEventType = SyncWebhookEventType, > = AwsLambdaWebhookHandler>; export class SaleorSyncWebhook< TPayload = unknown, - TEvent extends SyncWebhookEventType = SyncWebhookEventType + TEvent extends SyncWebhookEventType = SyncWebhookEventType, > extends SaleorWebApiWebhook> { readonly event: TEvent; protected readonly eventType = "sync" as const; protected extraContext = { - buildResponse: buildSyncWebhookResponsePayload, + buildResponse: buildSyncWebhookResponsePayload, }; constructor(configuration: WebhookConfig) { diff --git a/src/handlers/platforms/fetch-api/create-manifest-handler.test.ts b/src/handlers/platforms/fetch-api/create-manifest-handler.test.ts index a3dee3d9..3ea4117e 100644 --- a/src/handlers/platforms/fetch-api/create-manifest-handler.test.ts +++ b/src/handlers/platforms/fetch-api/create-manifest-handler.test.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({ diff --git a/src/handlers/platforms/next/create-manifest-handler.test.ts b/src/handlers/platforms/next/create-manifest-handler.test.ts index 5af517c6..1d494af5 100644 --- a/src/handlers/platforms/next/create-manifest-handler.test.ts +++ b/src/handlers/platforms/next/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); diff --git a/src/handlers/shared/saleor-request-processor.test.ts b/src/handlers/shared/saleor-request-processor.test.ts index 49c35fc4..bdf433fe 100644 --- a/src/handlers/shared/saleor-request-processor.test.ts +++ b/src/handlers/shared/saleor-request-processor.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", }); }); diff --git a/src/handlers/shared/saleor-request-processor.ts b/src/handlers/shared/saleor-request-processor.ts index e945df95..68df7c38 100644 --- a/src/handlers/shared/saleor-request-processor.ts +++ b/src/handlers/shared/saleor-request-processor.ts @@ -40,18 +40,20 @@ export class SaleorRequestProcessor { 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)), }; } } diff --git a/src/handlers/shared/saleor-webhook-validator.test.ts b/src/handlers/shared/saleor-webhook-validator.test.ts index 2025fc40..45fa112a 100644 --- a/src/handlers/shared/saleor-webhook-validator.test.ts +++ b/src/handlers/shared/saleor-webhook-validator.test.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,10 +351,10 @@ describe("SaleorWebhookValidator", () => { expect(mockAPL.set).toHaveBeenCalledWith( expect.objectContaining({ jwks: "new-jwks", - }) + }), ); expect(fetchRemoteJwksModule.fetchRemoteJwks).toHaveBeenCalledWith( - authDataNoJwks.saleorApiUrl + authDataNoJwks.saleorApiUrl, ); expect(verifySignatureModule.verifySignatureWithJwks).toHaveBeenCalledTimes(2); }); @@ -362,10 +362,10 @@ describe("SaleorWebhookValidator", () => { 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, ); }); });