From a839314f08a3428278126285986d5491afe6c986 Mon Sep 17 00:00:00 2001 From: Lukasz Ostrowski Date: Tue, 11 Oct 2022 09:40:08 +0200 Subject: [PATCH] Add central SaleorApp instance (#71) * Add SaleorApp class * Add middleware and tests * Move APL validation to APL * Fix test * Add prepush hook * Add better error for missing vercel envs * Add test --- .husky/pre-push | 4 ++ src/APL/apl.ts | 13 +++++ src/APL/file-apl.ts | 13 ++++- src/APL/vercel-apl.test.ts | 16 ++++++ src/APL/vercel-apl.ts | 37 +++++++++++- .../next/create-app-register-handler.test.ts | 3 + .../next/create-app-register-handler.ts | 23 ++++++-- src/index.ts | 1 + src/middleware/index.ts | 1 + ...th-registered-saleor-domain-header.test.ts | 29 +++++++++- .../with-registered-saleor-domain-header.ts | 56 +++++++++++-------- src/middleware/with-saleor-app.test.ts | 27 +++++++++ src/middleware/with-saleor-app.ts | 21 +++++++ src/saleor-app.test.ts | 22 ++++++++ src/saleor-app.ts | 25 +++++++++ 15 files changed, 258 insertions(+), 33 deletions(-) create mode 100755 .husky/pre-push create mode 100644 src/middleware/with-saleor-app.test.ts create mode 100644 src/middleware/with-saleor-app.ts create mode 100644 src/saleor-app.test.ts create mode 100644 src/saleor-app.ts diff --git a/.husky/pre-push b/.husky/pre-push new file mode 100755 index 00000000..45ac5307 --- /dev/null +++ b/.husky/pre-push @@ -0,0 +1,4 @@ +#!/usr/bin/env sh +. "$(dirname -- "$0")/_/husky.sh" + +pnpm run test:ci diff --git a/src/APL/apl.ts b/src/APL/apl.ts index a761f3a7..063aab4c 100644 --- a/src/APL/apl.ts +++ b/src/APL/apl.ts @@ -3,9 +3,22 @@ export interface AuthData { token: string; } +export type AplReadyResult = + | { + ready: true; + } + | { + ready: false; + error: Error; + }; + export interface APL { get: (domain: string) => Promise; set: (authData: AuthData) => Promise; delete: (domain: string) => Promise; getAll: () => Promise; + /** + * Inform that configuration is finished and correct + */ + isReady: () => Promise; } diff --git a/src/APL/file-apl.ts b/src/APL/file-apl.ts index 4f52b9e4..9cfa5c5b 100644 --- a/src/APL/file-apl.ts +++ b/src/APL/file-apl.ts @@ -1,6 +1,6 @@ import { promises as fsPromises } from "fs"; -import { APL, AuthData } from "./apl"; +import { APL, AplReadyResult, AuthData } from "./apl"; import { createAPLDebug } from "./apl-debug"; const debug = createAPLDebug("FileAPL"); @@ -98,4 +98,15 @@ export class FileAPL implements APL { return [authData]; } + + // eslint-disable-next-line class-methods-use-this + async isReady(): Promise { + /** + * Assume FileAPL is just ready to use. + * Consider checking if directory is writable + */ + return { + ready: true, + }; + } } diff --git a/src/APL/vercel-apl.test.ts b/src/APL/vercel-apl.test.ts index c004e09a..1082c56e 100644 --- a/src/APL/vercel-apl.test.ts +++ b/src/APL/vercel-apl.test.ts @@ -158,5 +158,21 @@ describe("APL", () => { }); }); }); + + describe("isReady", () => { + it("Returns error with message mentioning missing env variables", async () => { + const apl = new VercelAPL(aplConfig); + + const result = await apl.isReady(); + + if (!result.ready) { + expect(result.error.message).toEqual( + "Env variables: \"SALEOR_AUTH_TOKEN\", \"SALEOR_DOMAIN\", \"SALEOR_REGISTER_APP_URL\", \"SALEOR_DEPLOYMENT_TOKEN\" not found or is empty. Ensure env variables exist" + ); + } else { + throw new Error("This should not happen"); + } + }); + }); }); }); diff --git a/src/APL/vercel-apl.ts b/src/APL/vercel-apl.ts index 843735c6..3bea429f 100644 --- a/src/APL/vercel-apl.ts +++ b/src/APL/vercel-apl.ts @@ -1,7 +1,8 @@ /* eslint-disable class-methods-use-this */ +// eslint-disable-next-line max-classes-per-file import fetch, { Response } from "node-fetch"; -import { APL, AuthData } from "./apl"; +import { APL, AplReadyResult, AuthData } from "./apl"; import { createAPLDebug } from "./apl-debug"; const debug = createAPLDebug("VercelAPL"); @@ -13,6 +14,16 @@ export const VercelAPLVariables = { SALEOR_DEPLOYMENT_TOKEN: "SALEOR_DEPLOYMENT_TOKEN", }; +export class VercelAplMisconfiguredError extends Error { + constructor(public missingEnvVars: string[]) { + super( + `Env variables: ${missingEnvVars + .map((v) => `"${v}"`) + .join(", ")} not found or is empty. Ensure env variables exist` + ); + } +} + const getEnvAuth = (): AuthData | undefined => { const token = process.env[VercelAPLVariables.TOKEN_VARIABLE_NAME]; const domain = process.env[VercelAPLVariables.DOMAIN_VARIABLE_NAME]; @@ -32,9 +43,9 @@ export type VercelAPLConfig = { /** Vercel APL * * Use environment variables for auth data storage. To update data on existing deployment, - * theres Saleor microservice which update new values with the Vercel API and restarts the instance. + * there's Saleor microservice which update new values with the Vercel API and restarts the instance. * - * This APL should be used for single tenant purposes due to it's limitations: + * This APL should be used for single tenant purposes due to its limitations: * - only stores single auth data entry (setting up a new one will overwrite previous values) * - changing the environment variables require server restart * @@ -122,4 +133,24 @@ export class VercelAPL implements APL { } return [authData]; } + + // eslint-disable-next-line class-methods-use-this + async isReady(): Promise { + const invalidEnvKeys = Object.values(VercelAPLVariables).filter((key) => { + const envValue = process.env[key]; + + return !envValue || envValue.length === 0; + }); + + if (invalidEnvKeys.length > 0) { + return { + ready: false, + error: new VercelAplMisconfiguredError(invalidEnvKeys), + }; + } + + return { + ready: true, + }; + } } diff --git a/src/handlers/next/create-app-register-handler.test.ts b/src/handlers/next/create-app-register-handler.test.ts index 7d5d661f..bd45a036 100644 --- a/src/handlers/next/create-app-register-handler.test.ts +++ b/src/handlers/next/create-app-register-handler.test.ts @@ -11,6 +11,9 @@ describe("create-app-register-handler", () => { set: vi.fn(), delete: vi.fn(), getAll: vi.fn(), + isReady: vi.fn().mockImplementation(async () => ({ + ready: true, + })), }; const { res, req } = createMocks({ diff --git a/src/handlers/next/create-app-register-handler.ts b/src/handlers/next/create-app-register-handler.ts index d71fa9e0..4955dd80 100644 --- a/src/handlers/next/create-app-register-handler.ts +++ b/src/handlers/next/create-app-register-handler.ts @@ -3,13 +3,11 @@ import { toNextHandler } from "retes/adapter"; import { withMethod } from "retes/middleware"; import { Response } from "retes/response"; -import { APL } from "../../APL"; import { SALEOR_DOMAIN_HEADER } from "../../const"; import { withAuthTokenRequired, withSaleorDomainPresent } from "../../middleware"; +import { HasAPL } from "../../saleor-app"; -export type CreateAppRegisterHandlerOptions = { - apl: APL; -}; +export type CreateAppRegisterHandlerOptions = HasAPL; /** * Creates API handler for Next.js. Creates handler called by Saleor that registers app. @@ -21,6 +19,23 @@ export const createAppRegisterHandler = ({ apl }: CreateAppRegisterHandlerOption const authToken = request.params.auth_token; const saleorDomain = request.headers[SALEOR_DOMAIN_HEADER] as string; + const { ready: aplReady } = await apl.isReady(); + + if (!aplReady) { + return new Response( + { + success: false, + error: { + code: "APL_NOT_READY", + message: "App is not ready yet", + }, + }, + { + status: 503, + } + ); + } + try { await apl.set({ domain: saleorDomain, token: authToken }); } catch { diff --git a/src/index.ts b/src/index.ts index 1fdc6780..c396b5ac 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,5 +1,6 @@ export * from "./const"; export * from "./headers"; export * from "./infer-webhooks"; +export * from "./saleor-app"; export * from "./types"; export * from "./urls"; diff --git a/src/middleware/index.ts b/src/middleware/index.ts index bc378634..f09e8217 100644 --- a/src/middleware/index.ts +++ b/src/middleware/index.ts @@ -3,6 +3,7 @@ export * from "./with-auth-token-required"; export * from "./with-base-url"; export * from "./with-jwt-verified"; export * from "./with-registered-saleor-domain-header"; +export * from "./with-saleor-app"; export * from "./with-saleor-domain-present"; export * from "./with-saleor-event-match"; export * from "./with-webhook-signature-verified"; diff --git a/src/middleware/with-registered-saleor-domain-header.test.ts b/src/middleware/with-registered-saleor-domain-header.test.ts index ac319f1b..9d4b38b3 100644 --- a/src/middleware/with-registered-saleor-domain-header.test.ts +++ b/src/middleware/with-registered-saleor-domain-header.test.ts @@ -4,7 +4,9 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { APL } from "../APL"; import { SALEOR_DOMAIN_HEADER } from "../const"; +import { SaleorApp } from "../saleor-app"; import { withRegisteredSaleorDomainHeader } from "./with-registered-saleor-domain-header"; +import { withSaleorApp } from "./with-saleor-app"; const getMockSuccessResponse = async () => Response.OK({}); @@ -39,7 +41,11 @@ describe("middleware", () => { }, } as unknown as Request; - const response = await withRegisteredSaleorDomainHeader({ apl: mockAPL })(mockHandlerFn)( + const app = new SaleorApp({ + apl: mockAPL, + }); + + const response = await withSaleorApp(app)(withRegisteredSaleorDomainHeader(mockHandlerFn))( mockRequest ); @@ -57,11 +63,30 @@ describe("middleware", () => { }, } as unknown as Request; - const response = await withRegisteredSaleorDomainHeader({ apl: mockAPL })(mockHandlerFn)( + const app = new SaleorApp({ + apl: mockAPL, + }); + + const response = await withSaleorApp(app)(withRegisteredSaleorDomainHeader(mockHandlerFn))( mockRequest ); expect(response.status).eq(403); expect(mockHandlerFn).toBeCalledTimes(0); }); + + it("Throws if SaleorApp not found in context", async () => { + const mockRequest = { + context: {}, + headers: { + host: "my-saleor-env.saleor.cloud", + "x-forwarded-proto": "https", + [SALEOR_DOMAIN_HEADER]: "example.com", + }, + } as unknown as Request; + + const response = await withRegisteredSaleorDomainHeader(mockHandlerFn)(mockRequest); + + expect(response.status).eq(500); + }); }); }); diff --git a/src/middleware/with-registered-saleor-domain-header.ts b/src/middleware/with-registered-saleor-domain-header.ts index 86576bf8..84c7ba65 100644 --- a/src/middleware/with-registered-saleor-domain-header.ts +++ b/src/middleware/with-registered-saleor-domain-header.ts @@ -1,37 +1,47 @@ import { Middleware } from "retes"; import { Response } from "retes/response"; -import { APL } from "../APL"; import { getSaleorHeaders } from "../headers"; import { createMiddlewareDebug } from "./middleware-debug"; +import { getSaleorAppFromRequest } from "./with-saleor-app"; const debug = createMiddlewareDebug("withRegisteredSaleorDomainHeader"); -export const withRegisteredSaleorDomainHeader = - ({ apl }: { apl: APL }): Middleware => - (handler) => - async (request) => { - const { domain: saleorDomain } = getSaleorHeaders(request.headers); +export const withRegisteredSaleorDomainHeader: Middleware = (handler) => async (request) => { + const { domain: saleorDomain } = getSaleorHeaders(request.headers); - if (!saleorDomain) { - return Response.BadRequest({ - success: false, - message: "Domain header missing.", - }); - } + if (!saleorDomain) { + return Response.BadRequest({ + success: false, + message: "Domain header missing.", + }); + } - debug("Middleware called with domain: \"%s\"", saleorDomain); + debug("Middleware called with domain: \"%s\"", saleorDomain); - const authData = await apl.get(saleorDomain); + const saleorApp = getSaleorAppFromRequest(request); - if (!authData) { - debug("Auth was not found in APL, will respond with Forbidden status"); + if (!saleorApp) { + console.error( + "SaleorApp not found in request context. Ensure your API handler is wrapped with withSaleorApp middleware" + ); - return Response.Forbidden({ - success: false, - message: `Domain ${saleorDomain} not registered.`, - }); - } + return Response.InternalServerError({ + success: false, + message: "SaleorApp is misconfigured", + }); + } - return handler(request); - }; + const authData = await saleorApp?.apl.get(saleorDomain); + + if (!authData) { + debug("Auth was not found in APL, will respond with Forbidden status"); + + return Response.Forbidden({ + success: false, + message: `Domain ${saleorDomain} not registered.`, + }); + } + + return handler(request); +}; diff --git a/src/middleware/with-saleor-app.test.ts b/src/middleware/with-saleor-app.test.ts new file mode 100644 index 00000000..b026f6c6 --- /dev/null +++ b/src/middleware/with-saleor-app.test.ts @@ -0,0 +1,27 @@ +import { Request } from "retes"; +import { Response } from "retes/response"; +import { describe, expect, it } from "vitest"; + +import { FileAPL } from "../APL"; +import { SALEOR_DOMAIN_HEADER } from "../const"; +import { SaleorApp } from "../saleor-app"; +import { withSaleorApp } from "./with-saleor-app"; + +describe("middleware", () => { + describe("withSaleorApp", () => { + it("Adds SaleorApp instance to request context", async () => { + const mockRequest = { + context: {}, + headers: { + [SALEOR_DOMAIN_HEADER]: "example.com", + }, + } as unknown as Request; + + await withSaleorApp(new SaleorApp({ apl: new FileAPL() }))((request) => { + expect(request.context.saleorApp).toBeDefined(); + + return Response.OK(""); + })(mockRequest); + }); + }); +}); diff --git a/src/middleware/with-saleor-app.ts b/src/middleware/with-saleor-app.ts new file mode 100644 index 00000000..b7decc30 --- /dev/null +++ b/src/middleware/with-saleor-app.ts @@ -0,0 +1,21 @@ +import { Middleware, Request } from "retes"; + +import { SaleorApp } from "../saleor-app"; +import { createMiddlewareDebug } from "./middleware-debug"; + +const debug = createMiddlewareDebug("withSaleorApp"); + +export const withSaleorApp = + (saleorApp: SaleorApp): Middleware => + (handler) => + async (request) => { + debug("Middleware called"); + + request.context ??= {}; + request.context.saleorApp = saleorApp; + + return handler(request); + }; + +export const getSaleorAppFromRequest = (request: Request): SaleorApp | undefined => + request.context?.saleorApp; diff --git a/src/saleor-app.test.ts b/src/saleor-app.test.ts new file mode 100644 index 00000000..e686cf23 --- /dev/null +++ b/src/saleor-app.test.ts @@ -0,0 +1,22 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; + +import { FileAPL } from "./APL"; +import { SaleorApp } from "./saleor-app"; + +describe("SaleorApp", () => { + const initialEnv = { ...process.env }; + + afterEach(() => { + process.env = { ...initialEnv }; + vi.resetModules(); + }); + + it("Constructs", () => { + const instance = new SaleorApp({ + apl: new FileAPL(), + }); + + expect(instance).toBeDefined(); + expect(instance.apl).toBeInstanceOf(FileAPL); + }); +}); diff --git a/src/saleor-app.ts b/src/saleor-app.ts new file mode 100644 index 00000000..7c7f4a45 --- /dev/null +++ b/src/saleor-app.ts @@ -0,0 +1,25 @@ +import { APL, AplReadyResult } from "./APL"; + +export interface HasAPL { + apl: APL; +} + +export interface SaleorAppParams { + apl: APL; + requiredEnvVars?: string[]; +} + +export class SaleorApp implements HasAPL { + readonly apl: APL; + + readonly requiredEnvVars: string[]; + + constructor(options: SaleorAppParams) { + this.apl = options.apl; + this.requiredEnvVars = options.requiredEnvVars ?? []; + } + + isReady(): Promise { + return this.apl.isReady(); + } +}