Feat/validate query enhancement (#9705)

* feat(framework): Enhance query validation

* feat(framework): Enhance query validation

* feat(framework): Enhance query validation

* feat(framework): Enhance query validation

* fix

* split restriction per http domain

* fix

* fix unit tests

* fix middleware

* cleanup allowed fields

* update docs

* missing allowed

* export

* missing allowed

* missing fields

* improvements

* rm unnecessary fields

* wip

* update symbol support

* update symbol support

* update allowed

* update allowed
This commit is contained in:
Adrien de Peretti
2024-10-22 16:47:05 +02:00
committed by GitHub
parent 6e0a1e3a86
commit 29d9f90fbf
17 changed files with 372 additions and 64 deletions

View File

@@ -730,6 +730,27 @@ export type ProjectConfigOptions = {
* ```
*/
authMethodsPerActor?: Record<string, string[]>
/**
* Specifies the fields that can't be selected in the response unless specified in the allowed query config.
* This is useful to restrict sensitive fields from being exposed in the API.
*
* @example
*
* ```js title="medusa-config.js"
* module.exports = defineConfig({
* projectConfig: {
* http: {
* restrictedFields: {
* store: ["order", "orders"],
* }
* }
* ```
*/
restrictedFields?: {
store?: string[]
/*admin?: string[]*/
}
}
}

View File

@@ -2,6 +2,8 @@ import z from "zod"
import { MedusaError } from "@medusajs/utils"
import { validateAndTransformQuery } from "../utils/validate-query"
import { MedusaNextFunction, MedusaRequest, MedusaResponse } from "../types"
import { RestrictedFields } from "../utils/restricted-fields"
import { QueryConfig } from "@medusajs/types"
export const createSelectParams = () => {
return z.object({
@@ -60,6 +62,7 @@ describe("validateAndTransformQuery", () => {
it("should transform the input query", async () => {
let mockRequest = {
restrictedFields: new RestrictedFields(),
query: {},
} as MedusaRequest
const mockResponse = {} as MedusaResponse
@@ -148,6 +151,7 @@ describe("validateAndTransformQuery", () => {
})
mockRequest = {
...mockRequest,
query: {
limit: "10",
offset: "5",
@@ -167,6 +171,7 @@ describe("validateAndTransformQuery", () => {
})
mockRequest = {
...mockRequest,
query: {
limit: "10",
offset: "5",
@@ -202,6 +207,7 @@ describe("validateAndTransformQuery", () => {
it("should transform the input query taking into account the fields symbols (+,- or no symbol)", async () => {
let mockRequest = {
restrictedFields: new RestrictedFields(),
query: {
fields: "id",
},
@@ -234,6 +240,7 @@ describe("validateAndTransformQuery", () => {
)
mockRequest = {
...mockRequest,
query: {
fields: "+test_prop,-prop-test-something",
},
@@ -275,6 +282,7 @@ describe("validateAndTransformQuery", () => {
)
mockRequest = {
...mockRequest,
query: {
fields: "+test_prop,-updated_at",
},
@@ -315,11 +323,16 @@ describe("validateAndTransformQuery", () => {
})
it(`should transform the input and manage the allowed fields and relations properly without error`, async () => {
const restrictedFields = new RestrictedFields()
let mockRequest = {
restrictedFields,
query: {
fields: "*product.variants,+product.id",
fields: "product.*, *product.variants,+product.id",
},
} as unknown as MedusaRequest
restrictedFields.add(["product"])
const mockResponse = {} as MedusaResponse
const nextFunction: MedusaNextFunction = jest.fn()
@@ -385,12 +398,14 @@ describe("validateAndTransformQuery", () => {
"metadata.children.id",
"metadata.product.id",
"product.id",
"product.*",
"product.variants.*",
],
})
)
mockRequest = {
...mockRequest,
query: {
fields: "store.name",
},
@@ -441,6 +456,7 @@ describe("validateAndTransformQuery", () => {
it("should throw when attempting to transform the input if disallowed fields are requested", async () => {
let mockRequest = {
restrictedFields: new RestrictedFields(),
query: {
fields: "+test_prop",
},
@@ -459,12 +475,6 @@ describe("validateAndTransformQuery", () => {
"metadata.children.id",
"metadata.product.id",
],
defaultRelations: [
"metadata",
"metadata.parent",
"metadata.children",
"metadata.product",
],
allowed: [
"id",
"created_at",
@@ -490,6 +500,7 @@ describe("validateAndTransformQuery", () => {
)
mockRequest = {
...mockRequest,
query: {
fields: "product",
},
@@ -506,12 +517,6 @@ describe("validateAndTransformQuery", () => {
"metadata.children.id",
"metadata.product.id",
],
defaultRelations: [
"metadata",
"metadata.parent",
"metadata.children",
"metadata.product",
],
allowed: [
"id",
"created_at",
@@ -537,6 +542,7 @@ describe("validateAndTransformQuery", () => {
)
mockRequest = {
...mockRequest,
query: {
fields: "store",
},
@@ -580,6 +586,7 @@ describe("validateAndTransformQuery", () => {
)
mockRequest = {
...mockRequest,
query: {
fields: "*product",
},
@@ -621,6 +628,7 @@ describe("validateAndTransformQuery", () => {
)
mockRequest = {
...mockRequest,
query: {
fields: "*product.variants",
},
@@ -663,6 +671,7 @@ describe("validateAndTransformQuery", () => {
)
mockRequest = {
...mockRequest,
query: {
fields: "*product",
},
@@ -704,4 +713,47 @@ describe("validateAndTransformQuery", () => {
)
)
})
it("should throw when attempting to transform the input if restricted fields are requested", async () => {
const restrictedFields = new RestrictedFields()
let mockRequest = {
restrictedFields,
query: {
fields: "*product",
},
} as unknown as MedusaRequest
restrictedFields.add(["product"])
const mockResponse = {} as MedusaResponse
const nextFunction: MedusaNextFunction = jest.fn()
const queryConfig: QueryConfig<any> = {
defaults: [
"id",
"created_at",
"updated_at",
"deleted_at",
"metadata.id",
"metadata.parent.id",
"metadata.children.id",
"metadata.product.id",
],
isList: true,
}
const middleware = validateAndTransformQuery(
createFindParams(),
queryConfig
)
await middleware(mockRequest, mockResponse, nextFunction)
expect(nextFunction).toHaveBeenLastCalledWith(
new MedusaError(
MedusaError.Types.INVALID_DATA,
`Requested fields [metadata.product.id, product] are not valid`
)
)
})
})

View File

@@ -12,3 +12,4 @@ export * from "./utils/define-middlewares"
export * from "./utils/maybe-apply-link-filter"
export * from "./utils/refetch-entities"
export * from "./utils/unless-path"
export * from "./utils/restricted-fields"

View File

@@ -23,6 +23,7 @@ import { ensurePublishableApiKeyMiddleware } from "./middlewares/ensure-publisha
import {
GlobalMiddlewareDescriptor,
HTTP_METHODS,
MedusaNextFunction,
MedusaRequest,
MedusaResponse,
MiddlewareFunction,
@@ -35,6 +36,7 @@ import {
RouteHandler,
RouteVerb,
} from "./types"
import { RestrictedFields } from "./utils/restricted-fields"
const log = ({
activityId,
@@ -244,7 +246,7 @@ export class ApiRoutesLoader {
static traceMiddleware?: (
handler: RequestHandler | MiddlewareFunction,
route: { route: string; method?: string }
) => RequestHandler
) => RequestHandler | MiddlewareFunction
constructor({
app,
@@ -589,14 +591,15 @@ export class ApiRoutesLoader {
* Applies middleware that checks if a valid publishable key is set on store request
*/
applyStorePublishableKeyMiddleware(route: string) {
let middleware =
ensurePublishableApiKeyMiddleware as unknown as RequestHandler
let middleware = ensurePublishableApiKeyMiddleware as unknown as
| RequestHandler
| MiddlewareFunction
if (ApiRoutesLoader.traceMiddleware) {
middleware = ApiRoutesLoader.traceMiddleware(middleware, { route: route })
}
this.#router.use(route, middleware)
this.#router.use(route, middleware as RequestHandler)
}
/**
@@ -609,7 +612,9 @@ export class ApiRoutesLoader {
authType: AuthType | AuthType[],
options?: { allowUnauthenticated?: boolean; allowUnregistered?: boolean }
) {
let authenticateMiddleware = authenticate(actorType, authType, options)
let authenticateMiddleware: RequestHandler | MiddlewareFunction =
authenticate(actorType, authType, options)
if (ApiRoutesLoader.traceMiddleware) {
authenticateMiddleware = ApiRoutesLoader.traceMiddleware(
authenticateMiddleware,
@@ -617,7 +622,7 @@ export class ApiRoutesLoader {
)
}
this.#router.use(route, authenticateMiddleware)
this.#router.use(route, authenticateMiddleware as RequestHandler)
}
/**
@@ -933,14 +938,39 @@ export class RoutesLoader {
app,
activityId,
sourceDir,
baseRestrictedFields = [],
}: {
app: Express
activityId?: string
sourceDir: string | string[]
baseRestrictedFields?: string[]
}) {
this.#app = app
this.#activityId = activityId
this.#sourceDir = sourceDir
this.#assignRestrictedFields(baseRestrictedFields)
}
#assignRestrictedFields(baseRestrictedFields: string[]) {
this.#app.use("/store", ((
req: MedusaRequest,
_: MedusaResponse,
next: MedusaNextFunction
) => {
req.restrictedFields = new RestrictedFields()
req.restrictedFields.add(baseRestrictedFields)
next()
}) as unknown as RequestHandler)
this.#app.use("/admin", ((
req: MedusaRequest,
_: MedusaResponse,
next: MedusaNextFunction
) => {
req.restrictedFields = new RestrictedFields()
next()
}) as unknown as RequestHandler)
}
async load() {

View File

@@ -7,6 +7,7 @@ import {
RequestQueryFields,
} from "@medusajs/types"
import { MedusaContainer } from "../container"
import { RestrictedFields } from "./utils/restricted-fields"
/**
* List of all the supported HTTP methods
@@ -135,6 +136,9 @@ export interface MedusaRequest<Body = unknown>
session?: any
rawBody?: any
requestId?: string
restrictedFields?: RestrictedFields
/**
* An object that carries the context that is used to calculate prices for variants
*/

View File

@@ -1,5 +1,5 @@
import { pick } from "lodash"
import { RequestQueryFields, FindConfig, QueryConfig } from "@medusajs/types"
import { FindConfig, QueryConfig, RequestQueryFields } from "@medusajs/types"
import {
isDefined,
isPresent,
@@ -23,11 +23,83 @@ export function pickByConfig<TModel>(
return obj
}
function checkRestrictedFields({
fields,
restricted,
}: {
fields: string[]
restricted: string[]
}): string[] {
const notAllowedFields: string[] = []
fields.forEach((field) => {
const fieldSegments = field.split(".")
const hasRestrictedField = restricted.some((restrictedField) =>
fieldSegments.includes(restrictedField)
)
if (hasRestrictedField) {
notAllowedFields.push(field)
return
}
return
})
return notAllowedFields
}
function checkAllowedFields({
fields,
allowed,
starFields,
}: {
fields: string[]
starFields: Set<string>
allowed: string[]
}): string[] {
const notAllowedFields: string[] = []
fields.forEach((field) => {
const hasAllowedField = allowed.includes(field)
if (hasAllowedField) {
return
}
// Select full relation in that case it must match an allowed field fully
// e.g product.variants in that case we must have a product.variants in the allowedFields
if (starFields.has(field)) {
if (hasAllowedField) {
return
}
notAllowedFields.push(field)
return
}
const fieldStartsWithAllowedField = allowed.some((allowedField) =>
field.startsWith(allowedField)
)
if (!fieldStartsWithAllowedField) {
notAllowedFields.push(field)
return
}
})
return notAllowedFields
}
export function prepareListQuery<T extends RequestQueryFields, TEntity>(
validated: T,
queryConfig: QueryConfig<TEntity> = {}
queryConfig: QueryConfig<TEntity> & { restricted?: string[] } = {}
) {
let { allowed = [], defaults = [], defaultLimit = 50, isList } = queryConfig
let {
allowed = [],
restricted = [],
defaults = [],
defaultLimit = 50,
isList,
} = queryConfig
const { order, fields, limit = defaultLimit, offset = 0 } = validated
// e.g *product.variants meaning that we want all fields from the product.variants
@@ -46,7 +118,8 @@ export function prepareListQuery<T extends RequestQueryFields, TEntity>(
field.startsWith("-") ||
field.startsWith("+") ||
field.startsWith(" ") ||
field.startsWith("*")
field.startsWith("*") ||
field.endsWith(".*")
)
})
@@ -68,44 +141,32 @@ export function prepareListQuery<T extends RequestQueryFields, TEntity>(
}
allFields.forEach((field) => {
if (field.startsWith("*")) {
starFields.add(field.replace(/^\*/, ""))
if (field.startsWith("*") || field.endsWith(".*")) {
starFields.add(field.replace(/(^\*|\.\*$)/, ""))
allFields.delete(field)
}
})
const notAllowedFields: string[] = []
let notAllowedFields: string[] = []
if (allowed.length) {
;[...allFields, ...Array.from(starFields)].forEach((field) => {
const hasAllowedField = allowed.includes(field)
if (allowed.length || restricted.length) {
const fieldsToCheck = [...allFields, ...Array.from(starFields)]
if (hasAllowedField) {
return
}
// Select full relation in that case it must match an allowed field fully
// e.g product.variants in that case we must have a product.variants in the allowedFields
if (starFields.has(field)) {
if (hasAllowedField) {
return
}
notAllowedFields.push(field)
return
}
const fieldStartsWithAllowedField = allowed.some((allowedField) =>
field.startsWith(allowedField)
)
if (!fieldStartsWithAllowedField) {
notAllowedFields.push(field)
return
}
})
if (allowed.length) {
notAllowedFields = checkAllowedFields({
fields: fieldsToCheck,
starFields,
allowed,
})
} else if (restricted.length) {
notAllowedFields = checkRestrictedFields({
fields: fieldsToCheck,
restricted,
})
}
}
if (allFields.size && notAllowedFields.length) {
if (notAllowedFields.length) {
throw new MedusaError(
MedusaError.Types.INVALID_DATA,
`Requested fields [${Array.from(notAllowedFields).join(
@@ -170,7 +231,7 @@ export function prepareListQuery<T extends RequestQueryFields, TEntity>(
export function prepareRetrieveQuery<T extends RequestQueryFields, TEntity>(
validated: T,
queryConfig?: QueryConfig<TEntity>
queryConfig?: QueryConfig<TEntity> & { restricted?: string[] }
) {
const { listConfig, remoteQueryConfig } = prepareListQuery(
validated,

View File

@@ -0,0 +1,18 @@
export class RestrictedFields {
/**
* Fields that are restricted from being selected in the response.
* Those fields can be allowed if specified in the allowed configuration of the query config of an end point.
*
* @type {string[]}
* @private
*/
#restrictedFields: Set<string> = new Set()
list() {
return Array.from(this.#restrictedFields)
}
add(fields: string[]) {
fields.map((field) => this.#restrictedFields.add(field))
}
}

View File

@@ -68,14 +68,25 @@ export function validateAndTransformQuery<TEntity extends BaseEntity>(
next: NextFunction
) {
try {
const allowed = (req.allowed ?? queryConfig.allowed ?? []) as string[]
const restricted = req.restrictedFields?.list()
const allowed = queryConfig.allowed ?? []
// If any custom allowed fields are set, we add them to the allowed list along side the one configured in the query config if any
if (req.allowed?.length) {
allowed.push(...req.allowed)
}
delete req.allowed
const query = normalizeQuery(req)
const validated = await zodValidator(zodSchema, query)
const cnf = queryConfig.isList
? prepareListQuery(validated, { ...queryConfig, allowed })
: prepareRetrieveQuery(validated, { ...queryConfig, allowed })
? prepareListQuery(validated, { ...queryConfig, allowed, restricted })
: prepareRetrieveQuery(validated, {
...queryConfig,
allowed,
restricted,
})
req.validatedQuery = validated
req.filterableFields = getFilterableFields(req.validatedQuery)

View File

@@ -300,14 +300,22 @@ export type RawRounding = {
*/
export type QueryConfig<TEntity> = {
/**
* Default fields and relations to return
* Default fields and relations to return.
* use `*` or `.*` to select all fields from a relations (e.g '*products' or 'products.*' will select all products properties)
*/
defaults?: (keyof TEntity | string)[]
/**
* Fields and relations that are allowed to be requested
* Fields and relations that are allowed to be requested.
* Symbol such as `*`, `+` and `-` should be removed as they dont make sense for
* the authorization search.
*/
allowed?: string[]
defaultLimit?: number
/**
* If the route that will use that configuration is supposed to return a list of entities. This
* will change the configuration that will be created on req.listConfig and req.remoteQueryConfig (among
* other things it will include pagination and sorting)
*/
isList?: boolean
}

View File

@@ -724,6 +724,27 @@ export type ProjectConfigOptions = {
* ```
*/
authMethodsPerActor?: Record<string, string[]>
/**
* Specifies the fields that can't be selected in the response unless specified in the allowed query config.
* This is useful to restrict sensitive fields from being exposed in the API.
*
* @example
*
* ```js title="medusa-config.js"
* module.exports = defineConfig({
* projectConfig: {
* http: {
* restrictedFields: {
* store: ["order", "orders"],
* }
* }
* ```
*/
restrictedFields?: {
store?: string[]
/*admin?: string[]*/
}
}
}

View File

@@ -1,5 +1,5 @@
import { Modules } from "../../modules-sdk"
import { defineConfig } from "../define-config"
import { DEFAULT_STORE_RESTRICTED_FIELDS, defineConfig } from "../define-config"
describe("defineConfig", function () {
it("should merge empty config with the defaults", function () {
@@ -133,6 +133,13 @@ describe("defineConfig", function () {
"authCors": "http://localhost:7000,http://localhost:7001,http://localhost:5173",
"cookieSecret": "supersecret",
"jwtSecret": "supersecret",
"restrictedFields": {
"store": [
${DEFAULT_STORE_RESTRICTED_FIELDS.map((v) => `"${v}"`).join(
",\n "
)},
],
},
"storeCors": "http://localhost:8000",
},
},
@@ -282,6 +289,13 @@ describe("defineConfig", function () {
"authCors": "http://localhost:7000,http://localhost:7001,http://localhost:5173",
"cookieSecret": "supersecret",
"jwtSecret": "supersecret",
"restrictedFields": {
"store": [
${DEFAULT_STORE_RESTRICTED_FIELDS.map((v) => `"${v}"`).join(
",\n "
)},
],
},
"storeCors": "http://localhost:8000",
},
},
@@ -439,6 +453,13 @@ describe("defineConfig", function () {
"authCors": "http://localhost:7000,http://localhost:7001,http://localhost:5173",
"cookieSecret": "supersecret",
"jwtSecret": "supersecret",
"restrictedFields": {
"store": [
${DEFAULT_STORE_RESTRICTED_FIELDS.map((v) => `"${v}"`).join(
",\n "
)},
],
},
"storeCors": "http://localhost:8000",
},
},
@@ -597,6 +618,13 @@ describe("defineConfig", function () {
"authCors": "http://localhost:7000,http://localhost:7001,http://localhost:5173",
"cookieSecret": "supersecret",
"jwtSecret": "supersecret",
"restrictedFields": {
"store": [
${DEFAULT_STORE_RESTRICTED_FIELDS.map((v) => `"${v}"`).join(
",\n "
)},
],
},
"storeCors": "http://localhost:8000",
},
},
@@ -743,6 +771,13 @@ describe("defineConfig", function () {
"authCors": "http://localhost:7000,http://localhost:7001,http://localhost:5173",
"cookieSecret": "supersecret",
"jwtSecret": "supersecret",
"restrictedFields": {
"store": [
${DEFAULT_STORE_RESTRICTED_FIELDS.map((v) => `"${v}"`).join(
",\n "
)},
],
},
"storeCors": "http://localhost:8000",
},
},
@@ -889,6 +924,13 @@ describe("defineConfig", function () {
"authCors": "http://localhost:7000,http://localhost:7001,http://localhost:5173",
"cookieSecret": "supersecret",
"jwtSecret": "supersecret",
"restrictedFields": {
"store": [
${DEFAULT_STORE_RESTRICTED_FIELDS.map((v) => `"${v}"`).join(
",\n "
)},
],
},
"storeCors": "http://localhost:8000",
},
},

View File

@@ -20,6 +20,15 @@ const DEFAULT_DATABASE_URL = "postgres://localhost/medusa-starter-default"
const DEFAULT_ADMIN_CORS =
"http://localhost:7000,http://localhost:7001,http://localhost:5173"
export const DEFAULT_STORE_RESTRICTED_FIELDS = [
"order",
"orders",
/*"customer",
"customers",
"payment_collection",
"payment_collections"*/
]
type InternalModuleDeclarationOverride = InternalModuleDeclaration & {
/**
* Optional key to be used to identify the module, if not provided, it will be inferred from the module joiner config service name.
@@ -79,6 +88,9 @@ export function defineConfig(config: Config = {}): ConfigModule {
authCors: process.env.AUTH_CORS || DEFAULT_ADMIN_CORS,
jwtSecret: process.env.JWT_SECRET || DEFAULT_SECRET,
cookieSecret: process.env.COOKIE_SECRET || DEFAULT_SECRET,
restrictedFields: {
store: DEFAULT_STORE_RESTRICTED_FIELDS,
},
...http,
},
...restOfProjectConfig,

View File

@@ -1,3 +1,4 @@
// TODO: Global todo, review all default fields to prevent over fetching by default
export const defaultStoreCartFields = [
"id",
"currency_code",
@@ -33,8 +34,13 @@ export const defaultStoreCartFields = [
"promotions.application_method.value",
"promotions.application_method.type",
"promotions.application_method.currency_code",
"items",
"items.thumbnail",
"region",
"items.id",
"items.product",
"items.product.id",
"items.variant",
"items.variant_id",
"items.product_id",
"items.product.categories.id",

View File

@@ -15,6 +15,10 @@ const defaultStoreCustomersFields = [
export const retrieveTransformQueryConfig = {
defaults: defaultStoreCustomersFields,
allowed: [
...defaultStoreCustomersFields.map((f) => f.replace("*", "")),
"orders",
],
isList: false,
}

View File

@@ -7,8 +7,8 @@ export const defaultStoreRetrieveReturnReasonFields = [
"created_at",
"updated_at",
"deleted_at",
"*.parent_return_reason",
"*.return_reason_children",
"*parent_return_reason",
"*return_reason_children",
]
export const retrieveTransformQueryConfig = {

View File

@@ -1,5 +1,10 @@
import { snakeCase } from "lodash"
import { Query } from "@medusajs/framework"
import {
MedusaNextFunction,
MedusaRequest,
MedusaResponse,
Query,
} from "@medusajs/framework"
import { ApiRoutesLoader } from "@medusajs/framework/http"
import { Tracer } from "@medusajs/framework/telemetry"
import type { SpanExporter } from "@opentelemetry/sdk-trace-node"
@@ -84,7 +89,11 @@ export function instrumentHttpLayer() {
* OpenTelemetry
*/
ApiRoutesLoader.traceMiddleware = (handler) => {
return async (req, res, next) => {
return async (
req: MedusaRequest<any>,
res: MedusaResponse,
next: MedusaNextFunction
) => {
if (shouldExcludeResource(req.originalUrl)) {
return handler(req, res, next)
}

View File

@@ -3,6 +3,7 @@ import { join } from "path"
import qs from "qs"
import { RoutesLoader } from "@medusajs/framework/http"
import { MedusaContainer, PluginDetails } from "@medusajs/framework/types"
import { ConfigModule } from "@medusajs/framework/config"
type Options = {
app: Express
@@ -47,6 +48,12 @@ export default async ({ app, container, plugins }: Options) => {
join(__dirname, "../api")
)
const {
projectConfig: {
http: { restrictedFields },
},
} = container.resolve<ConfigModule>("configModule")
// TODO: Figure out why this is causing issues with test when placed inside ./api.ts
// Adding this here temporarily
// Test: (packages/medusa/src/api/routes/admin/currencies/update-currency.ts)
@@ -54,6 +61,7 @@ export default async ({ app, container, plugins }: Options) => {
await new RoutesLoader({
app: app,
sourceDir: sourcePaths,
baseRestrictedFields: restrictedFields?.store,
}).load()
} catch (err) {
throw Error(