diff --git a/.changeset/odd-cycles-destroy.md b/.changeset/odd-cycles-destroy.md new file mode 100644 index 0000000000..16a5e6a190 --- /dev/null +++ b/.changeset/odd-cycles-destroy.md @@ -0,0 +1,5 @@ +--- +"@medusajs/framework": patch +--- + +Fix(framework): http cors middleware order and options diff --git a/packages/core/framework/src/http/__fixtures__/routers/store/custom/route.ts b/packages/core/framework/src/http/__fixtures__/routers/store/custom/route.ts new file mode 100644 index 0000000000..bf62cefe1c --- /dev/null +++ b/packages/core/framework/src/http/__fixtures__/routers/store/custom/route.ts @@ -0,0 +1,3 @@ +export function GET(req, res) { + res.send("Hello from store custom route") +} diff --git a/packages/core/framework/src/http/__fixtures__/server/index.ts b/packages/core/framework/src/http/__fixtures__/server/index.ts index dba58cb86c..7180c3bd2f 100644 --- a/packages/core/framework/src/http/__fixtures__/server/index.ts +++ b/packages/core/framework/src/http/__fixtures__/server/index.ts @@ -110,6 +110,7 @@ export const createServer = async (rootDir) => { `${url}${queryParams ? "?" + queryParams : ""}` ) headers.Cookie = headers.Cookie || "" + if (opts.adminSession) { const token = generateJwtToken( { diff --git a/packages/core/framework/src/http/__tests__/index.spec.ts b/packages/core/framework/src/http/__tests__/index.spec.ts index 4a8ea7dc65..ec7c835e7b 100644 --- a/packages/core/framework/src/http/__tests__/index.spec.ts +++ b/packages/core/framework/src/http/__tests__/index.spec.ts @@ -46,12 +46,83 @@ describe("RoutesLoader", function () { }) expect(res.status).toBe(500) - console.log(res) expect(res.text).toBe( '{"code":"unknown_error","type":"unknown_error","message":"An unknown error occurred."}' ) }) + it("should not succeed on cors preflight admin request failing", async function () { + const res = await request("OPTIONS", "/admin/orders", { + headers: { + origin: "http://localhost:3000", + "access-control-request-method": "GET", + }, + adminSession: { + jwt: { + userId: "admin_user", + }, + }, + }) + + expect(res.status).toBe(204) + expect(res.headers["access-control-allow-origin"]).not.toBeTruthy() + }) + + it("should not succeed on cors preflight store request failing", async function () { + const res = await request("OPTIONS", "/store/custom", { + headers: { + origin: "http://localhost:3000", + "access-control-request-method": "GET", + }, + adminSession: { + jwt: { + userId: "admin_user", + }, + }, + }) + + expect(res.status).toBe(204) + expect(res.headers["access-control-allow-origin"]).not.toBeTruthy() + }) + + it("should succeed on cors preflight admin request", async function () { + const res = await request("OPTIONS", "/admin/orders", { + headers: { + origin: "http://localhost:7001", + "access-control-request-method": "GET", + }, + adminSession: { + jwt: { + userId: "admin_user", + }, + }, + }) + + expect(res.status).toBe(204) + expect(res.headers["access-control-allow-origin"]).toBe( + "http://localhost:7001" + ) + }) + + it("should succeed on cors preflight store request", async function () { + const res = await request("OPTIONS", "/store/custom", { + headers: { + origin: "http://localhost:8000", + "access-control-request-method": "GET", + }, + adminSession: { + jwt: { + userId: "admin_user", + }, + }, + }) + + expect(res.status).toBe(204) + expect(res.headers["access-control-allow-origin"]).toBe( + "http://localhost:8000" + ) + }) + it("should return a status 200 on GET admin/order/:id", async function () { const res = await request("GET", "/admin/orders/1000", { adminSession: { diff --git a/packages/core/framework/src/http/__tests__/routes-loader.spec.ts b/packages/core/framework/src/http/__tests__/routes-loader.spec.ts index 8ae0f86c9d..bc1df429e5 100644 --- a/packages/core/framework/src/http/__tests__/routes-loader.spec.ts +++ b/packages/core/framework/src/http/__tests__/routes-loader.spec.ts @@ -201,6 +201,18 @@ describe("Routes loader", () => { "shouldAppendAuthCors": false, "shouldAppendStoreCors": false, }, + { + "absolutePath": "${BASE_DIR}/store/custom/route.ts", + "handler": [Function], + "isRoute": true, + "matcher": "/store/custom", + "method": "GET", + "optedOutOfAuth": false, + "relativePath": "/store/custom/route.ts", + "shouldAppendAdminCors": false, + "shouldAppendAuthCors": false, + "shouldAppendStoreCors": true, + }, ] `) }) @@ -409,6 +421,18 @@ describe("Routes loader", () => { "shouldAppendAuthCors": false, "shouldAppendStoreCors": false, }, + { + "absolutePath": "${BASE_DIR}/store/custom/route.ts", + "handler": [Function], + "isRoute": true, + "matcher": "/store/custom", + "method": "GET", + "optedOutOfAuth": false, + "relativePath": "/store/custom/route.ts", + "shouldAppendAdminCors": false, + "shouldAppendAuthCors": false, + "shouldAppendStoreCors": true, + }, { "absolutePath": "${BASE_DIR_2}/store/[customer_id]/orders/[order_id]/route.ts", "handler": [Function], diff --git a/packages/core/framework/src/http/middlewares/ensure-publishable-api-key.ts b/packages/core/framework/src/http/middlewares/ensure-publishable-api-key.ts index e4ab7ebcb5..3ce2918f9a 100644 --- a/packages/core/framework/src/http/middlewares/ensure-publishable-api-key.ts +++ b/packages/core/framework/src/http/middlewares/ensure-publishable-api-key.ts @@ -19,14 +19,11 @@ export async function ensurePublishableApiKeyMiddleware( const publishableApiKey = req.get(PUBLISHABLE_KEY_HEADER) if (!isPresent(publishableApiKey)) { - try { - throw new MedusaError( - MedusaError.Types.NOT_ALLOWED, - `Publishable API key required in the request header: ${PUBLISHABLE_KEY_HEADER}. You can manage your keys in settings in the dashboard.` - ) - } catch (e) { - return next(e) - } + const error = new MedusaError( + MedusaError.Types.NOT_ALLOWED, + `Publishable API key required in the request header: ${PUBLISHABLE_KEY_HEADER}. You can manage your keys in settings in the dashboard.` + ) + return next(error) } let apiKey diff --git a/packages/core/framework/src/http/router.ts b/packages/core/framework/src/http/router.ts index acb4273b1c..02b57ff1b8 100644 --- a/packages/core/framework/src/http/router.ts +++ b/packages/core/framework/src/http/router.ts @@ -173,6 +173,7 @@ export class ApiLoader { return { origin: parseCorsOrigins(origin), credentials: true, + preflightContinue: false, } } @@ -194,11 +195,13 @@ export class ApiLoader { res, next ) { + let method: string = req.method + if (req.method === "OPTIONS") { + method = req.headers["access-control-request-method"] ?? req.method + } + const path = `${namespace}${req.path}` - const matchingRoute = routesFinder.find( - path, - req.method as MiddlewareVerb - ) + const matchingRoute = routesFinder.find(path, method as MiddlewareVerb) if (matchingRoute && matchingRoute[toggleKey] === true) { return corsFn(req, res, next) } @@ -213,7 +216,7 @@ export class ApiLoader { ? (ApiLoader.traceMiddleware(corsMiddleware, { route: namespace, }) as RequestHandler) - : cors(corsOptions) + : corsMiddleware ) } @@ -334,16 +337,17 @@ export class ApiLoader { "api-key", ]) - /** - * Publishable key check, CORS and auth setup for store routes. - */ - this.#applyStorePublishableKeyMiddleware("/store") this.#applyCorsMiddleware( routesFinder, "/store", "shouldAppendStoreCors", this.#createCorsOptions(configManager.config.projectConfig.http.storeCors) ) + /** + * Publishable key check, CORS and auth setup for store routes. + */ + this.#applyStorePublishableKeyMiddleware("/store") + this.#applyAuthMiddleware( routesFinder, "/store", diff --git a/packages/core/framework/src/http/routes-loader.ts b/packages/core/framework/src/http/routes-loader.ts index bcf4fc58bf..c9a05bd3c6 100644 --- a/packages/core/framework/src/http/routes-loader.ts +++ b/packages/core/framework/src/http/routes-loader.ts @@ -1,7 +1,7 @@ -import { join, parse, sep } from "path" import { dynamicImport, readDirRecursive } from "@medusajs/utils" +import { join, parse, sep } from "path" import { logger } from "../logger" -import { type RouteVerb, HTTP_METHODS, type RouteDescriptor } from "./types" +import { HTTP_METHODS, type RouteDescriptor, type RouteVerb } from "./types" /** * File name that is used to indicate that the file is a route file