fix(medusa): Clean response data usage for admin and store fields/expand (#3323)

* fix(medusa): Clean response data usage for admin and store fields/expand

* cleanup

* Create mighty-ads-fold.md

* fix integration

* fix integration

* refactor transform query and cleanup

* fix missing re naming

* Update packages/medusa/src/api/middlewares/transform-query.ts

---------

Co-authored-by: Oliver Windall Juhl <59018053+olivermrbl@users.noreply.github.com>

.changeset/mighty-ads-fold.md

@@ -0,0 +1,5 @@
+---
+"@medusajs/medusa": patch
+---
+
+fix(medusa): Clean response data usage for admin and store fields/expand

integration-tests/api/__tests__/admin/order/order.js

@@ -1615,6 +1615,16 @@ describe("/admin/orders", () => {
               id: "test-billing-address",
               first_name: "lebron",
             }),
+            shipping_total: expect.any(Number),
+            discount_total: expect.any(Number),
+            tax_total: expect.any(Number),
+            refunded_total: expect.any(Number),
+            total: expect.any(Number),
+            subtotal: expect.any(Number),
+            paid_total: expect.any(Number),
+            refundable_amount: expect.any(Number),
+            gift_card_total: expect.any(Number),
+            gift_card_tax_total: expect.any(Number),
           },
         ])
       )

integration-tests/api/__tests__/store/orders.js

@@ -177,6 +177,17 @@ describe("/store/carts", () => {
         "customer",
         "payments",
         "region",
+        // default
+        "shipping_total",
+        "discount_total",
+        "tax_total",
+        "refunded_total",
+        "total",
+        "subtotal",
+        "paid_total",
+        "refundable_amount",
+        "gift_card_total",
+        "gift_card_tax_total",
       ])
     })
 
@@ -197,6 +208,17 @@ describe("/store/carts", () => {
         "customer",
         "payments",
         "region",
+        // default
+        "shipping_total",
+        "discount_total",
+        "tax_total",
+        "refunded_total",
+        "total",
+        "subtotal",
+        "paid_total",
+        "refundable_amount",
+        "gift_card_total",
+        "gift_card_tax_total",
       ])
     })
 
@@ -212,6 +234,17 @@ describe("/store/carts", () => {
         "status",
         // selected relations
         "billing_address",
+        // default
+        "shipping_total",
+        "discount_total",
+        "tax_total",
+        "refunded_total",
+        "total",
+        "subtotal",
+        "paid_total",
+        "refundable_amount",
+        "gift_card_total",
+        "gift_card_tax_total",
       ])
     })
 

packages/medusa/src/api/middlewares/index.ts

@@ -9,7 +9,7 @@ export { getRequestedBatchJob } from "./batch-job/get-requested-batch-job"
 export { doesConditionBelongToDiscount } from "./discount/does-condition-belong-to-discount"
 export { transformIncludesOptions } from "./transform-includes-options"
 export { transformBody } from "./transform-body"
-export { transformQuery } from "./transform-query"
+export { transformQuery, transformStoreQuery } from "./transform-query"
 
 export default {
   authenticate,

packages/medusa/src/api/middlewares/transform-includes-options.ts

@@ -13,11 +13,11 @@ export function transformIncludesOptions(
   expectedIncludesFields: string[] = []
 ) {
   return (req: Request, res: Response, next: NextFunction): void => {
-    if (!allowedIncludesFields.length || !req.query["fields"]) {
+    if (!allowedIncludesFields.length || !req.query.fields) {
       return next()
     }
 
-    const fields = (req.query["fields"] as string).split(",") ?? []
+    const fields = (req.query.fields as string).split(",") ?? []
 
     for (const includesField of allowedIncludesFields) {
       const fieldIndex = fields.indexOf(includesField as keyof Order) ?? -1
@@ -40,16 +40,16 @@ export function transformIncludesOptions(
           )
         }
 
-        req["includes"] = req["includes"] ?? {}
+        req.includes = req.includes ?? {}
-        req["includes"][includesField] = true
+        req.includes[includesField] = true
       }
     }
 
-    if (req.query["fields"]) {
+    if (req.query.fields) {
       if (fields.length) {
-        req.query["fields"] = fields.join(",")
+        req.query.fields = fields.join(",")
       } else {
-        delete req.query["fields"]
+        delete req.query.fields
       }
     }
 

packages/medusa/src/api/middlewares/transform-query.ts

@@ -12,9 +12,56 @@ import { FindConfig, QueryConfig, RequestQueryFields } from "../../types/common"
 import { omit } from "lodash"
 import { removeUndefinedProperties } from "../../utils"
 
+/**
+ * Middleware that transform the query input for the admin end points
+ * @param plainToClass
+ * @param queryConfig
+ * @param config
+ */
 export function transformQuery<
   T extends RequestQueryFields,
   TEntity extends BaseEntity
+>(
+  plainToClass: ClassConstructor<T>,
+  queryConfig?: Omit<
+    QueryConfig<TEntity>,
+    "allowedRelations" | "allowedFields"
+  >,
+  config: ValidatorOptions = {}
+): (req: Request, res: Response, next: NextFunction) => Promise<void> {
+  return async (req: Request, res: Response, next: NextFunction) => {
+    try {
+      normalizeQuery()(req, res, () => void 0)
+      const validated: T = await validator<T, Record<string, unknown>>(
+        plainToClass,
+        req.query,
+        config
+      )
+      req.validatedQuery = validated
+      req.filterableFields = getFilterableFields(validated)
+      req.allowedProperties = getAllowedProperties(
+        validated,
+        req.includes ?? {},
+        queryConfig
+      )
+      attachListOrRetrieveConfig<TEntity>(req, queryConfig)
+
+      next()
+    } catch (e) {
+      next(e)
+    }
+  }
+}
+
+/**
+ * Middleware that transform the query input for the store endpoints
+ * @param plainToClass
+ * @param queryConfig
+ * @param config
+ */
+export function transformStoreQuery<
+  T extends RequestQueryFields,
+  TEntity extends BaseEntity
 >(
   plainToClass: ClassConstructor<T>,
   queryConfig?: QueryConfig<TEntity>,
@@ -29,47 +76,13 @@ export function transformQuery<
         config
       )
       req.validatedQuery = validated
-
+      req.filterableFields = getFilterableFields(validated)
-      req.filterableFields = omit(validated, [
+      req.allowedProperties = getStoreAllowedProperties(
-        "limit",
+        validated,
-        "offset",
+        req.includes ?? {},
-        "expand",
+        queryConfig
-        "fields",
+      )
-        "order",
+      attachListOrRetrieveConfig<TEntity>(req, queryConfig)
-      ])
-      req.filterableFields = removeUndefinedProperties(req.filterableFields)
-
-      if (
-        (queryConfig?.defaultFields || validated.fields) &&
-        (queryConfig?.defaultRelations || validated.expand)
-      ) {
-        req.allowedProperties = [
-          ...(validated.fields
-            ? validated.fields.split(",")
-            : queryConfig?.allowedFields || [])!,
-          ...(validated.expand
-            ? validated.expand.split(",")
-            : queryConfig?.allowedRelations || [])!,
-        ] as unknown as string[]
-      }
-
-      const includesFields = Object.keys(req["includes"] ?? {})
-      if (includesFields.length) {
-        req.allowedProperties = req.allowedProperties ?? []
-        req.allowedProperties.push(...includesFields)
-      }
-
-      if (queryConfig?.isList) {
-        req.listConfig = prepareListQuery(
-          validated,
-          queryConfig
-        ) as FindConfig<unknown>
-      } else {
-        req.retrieveConfig = prepareRetrieveQuery(
-          validated,
-          queryConfig
-        ) as FindConfig<unknown>
-      }
 
       next()
     } catch (e) {
@@ -77,3 +90,97 @@ export function transformQuery<
     }
   }
 }
+
+/**
+ * Omit the non filterable config from the validated object
+ * @param obj
+ */
+function getFilterableFields<T extends RequestQueryFields>(obj: T): T {
+  const result = omit(obj, [
+    "limit",
+    "offset",
+    "expand",
+    "fields",
+    "order",
+  ]) as T
+  return removeUndefinedProperties(result)
+}
+
+/**
+ * build and attach the `retrieveConfig` or `listConfig`
+ * @param req
+ * @param queryConfig
+ */
+function attachListOrRetrieveConfig<TEntity extends BaseEntity>(
+  req: Request,
+  queryConfig?: QueryConfig<TEntity>
+) {
+  const validated = req.validatedQuery
+  if (queryConfig?.isList) {
+    req.listConfig = prepareListQuery(
+      validated,
+      queryConfig
+    ) as FindConfig<unknown>
+  } else {
+    req.retrieveConfig = prepareRetrieveQuery(
+      validated,
+      queryConfig
+    ) as FindConfig<unknown>
+  }
+}
+/**
+ * Build the store allowed props based on the custom fields query params, the defaults and the includes options.
+ * This can be used later with `cleanResponseData` in order to clean up the returned objects to the client.
+ * @param queryConfig
+ * @param validated
+ * @param includesOptions
+ */
+function getStoreAllowedProperties<TEntity extends BaseEntity>(
+  validated: RequestQueryFields,
+  includesOptions: Record<string, boolean>,
+  queryConfig?: QueryConfig<TEntity>
+): string[] {
+  const allowed: string[] = []
+  allowed.push(
+    ...(validated.fields
+      ? validated.fields.split(",")
+      : queryConfig?.allowedFields || []),
+    ...(validated.expand
+      ? validated.expand.split(",")
+      : queryConfig?.allowedRelations || [])
+  )
+
+  const includeKeys = Object.keys(includesOptions)
+  if (includeKeys) {
+    allowed.push(...includeKeys)
+  }
+
+  return allowed
+}
+
+/**
+ * Build the admin allowed props based on the custom fields query params, the defaults and the includes options.
+ * Since admin can access everything, it is only in order to return what the user asked for through fields and expand query params.
+ * This can be used later with `cleanResponseData` in order to clean up the returned objects to the client.
+ * @param queryConfig
+ * @param validated
+ * @param includesOptions
+ */
+function getAllowedProperties<TEntity extends BaseEntity>(
+  validated: RequestQueryFields,
+  includesOptions: Record<string, boolean>,
+  queryConfig?: QueryConfig<TEntity>
+): string[] {
+  const allowed: string[] = []
+  allowed.push(
+    ...(validated.fields?.split(",") ?? []),
+    ...(validated.expand?.split(",") ?? [])
+  )
+
+  const includeKeys = Object.keys(includesOptions)
+  if (includeKeys) {
+    allowed.push(...includeKeys)
+  }
+
+  return allowed
+}

packages/medusa/src/api/routes/admin/orders/get-order.ts

@@ -1,5 +1,7 @@
 import { OrderService } from "../../../../services"
 import { FindParams } from "../../../../types/common"
+import { cleanResponseData } from "../../../../utils/clean-response-data"
+import { Order } from "../../../../models"
 
 /**
  * @oas [get] /admin/orders/{id}
@@ -60,9 +62,15 @@ export default async (req, res) => {
 
   const orderService: OrderService = req.scope.resolve("orderService")
 
-  const order = await orderService.retrieveWithTotals(id, req.retrieveConfig, {
+  let order: Partial<Order> = await orderService.retrieveWithTotals(
-    includes: req.includes,
+    id,
-  })
+    req.retrieveConfig,
+    {
+      includes: req.includes,
+    }
+  )
+
+  order = cleanResponseData(order, req.allowedProperties)
 
   res.json({ order: order })
 }

packages/medusa/src/api/routes/admin/orders/list-orders.ts

@@ -1,10 +1,9 @@
 import { IsNumber, IsOptional, IsString } from "class-validator"
 
 import { AdminListOrdersSelector } from "../../../../types/orders"
-import { Order } from "../../../../models"
 import { OrderService } from "../../../../services"
 import { Type } from "class-transformer"
-import { pick } from "lodash"
+import { cleanResponseData } from "../../../../utils/clean-response-data"
 
 /**
  * @oas [get] /admin/orders
@@ -200,19 +199,14 @@ import { pick } from "lodash"
 export default async (req, res) => {
   const orderService: OrderService = req.scope.resolve("orderService")
 
-  const { skip, take, select, relations } = req.listConfig
+  const { skip, take } = req.listConfig
 
   const [orders, count] = await orderService.listAndCount(
     req.filterableFields,
     req.listConfig
   )
 
-  let data: Partial<Order>[] = orders
+  const data = cleanResponseData(orders, req.allowedProperties)
-
-  const fields = [...select, ...relations]
-  if (fields.length) {
-    data = orders.map((o) => pick(o, fields))
-  }
 
   res.json({ orders: data, count, offset: skip, limit: take })
 }

packages/medusa/src/api/routes/store/carts/index.ts

@@ -1,11 +1,11 @@
 import "reflect-metadata"
-import { RequestHandler, Router } from "express"
+import { Router } from "express"
 
 import { Cart, Order, Swap } from "../../../../"
 import { FindParams } from "../../../../types/common"
 import middlewares, {
   transformBody,
-  transformQuery,
+  transformStoreQuery,
 } from "../../../middlewares"
 import { StorePostCartsCartReq } from "./update-cart"
 import { StorePostCartReq } from "./create-cart"
@@ -33,7 +33,7 @@ export default (app, container) => {
 
   route.get(
     "/:id",
-    transformQuery(FindParams, {
+    transformStoreQuery(FindParams, {
       defaultRelations: defaultStoreCartRelations,
       defaultFields: defaultStoreCartFields,
       isList: false,

packages/medusa/src/api/routes/store/collections/index.ts

@@ -1,7 +1,7 @@
 import { Router } from "express"
 import { PaginatedResponse } from "../../../../types/common"
 import { ProductCollection } from "../../../../"
-import middlewares, { transformQuery } from "../../../middlewares"
+import middlewares, { transformStoreQuery } from "../../../middlewares"
 import { StoreGetCollectionsParams } from "./list-collections"
 
 const route = Router()
@@ -11,7 +11,7 @@ export default (app) => {
 
   route.get(
     "/",
-    transformQuery(StoreGetCollectionsParams, {
+    transformStoreQuery(StoreGetCollectionsParams, {
       allowedFields,
       isList: true,
     }),

packages/medusa/src/api/routes/store/customers/index.ts

@@ -1,7 +1,7 @@
 import { Router } from "express"
 import { Customer, Order } from "../../../.."
 import { PaginatedResponse } from "../../../../types/common"
-import middlewares, { transformQuery } from "../../../middlewares"
+import middlewares, { transformStoreQuery } from "../../../middlewares"
 import {
   defaultStoreOrdersFields,
   defaultStoreOrdersRelations,
@@ -41,7 +41,7 @@ export default (app, container) => {
 
   route.get(
     "/me/orders",
-    transformQuery(StoreGetCustomersCustomerOrdersParams, {
+    transformStoreQuery(StoreGetCustomersCustomerOrdersParams, {
       defaultFields: defaultStoreOrdersFields,
       defaultRelations: defaultStoreOrdersRelations,
       isList: true,

packages/medusa/src/api/routes/store/order-edits/index.ts

@@ -7,7 +7,7 @@ import {
 } from "../../../../types/order-edit"
 import middlewares, {
   transformBody,
-  transformQuery,
+  transformStoreQuery,
 } from "../../../middlewares"
 import { StorePostOrderEditsOrderEditDecline } from "./decline-order-edit"
 
@@ -18,7 +18,7 @@ export default (app) => {
 
   route.get(
     "/:id",
-    transformQuery(FindParams, {
+    transformStoreQuery(FindParams, {
       defaultRelations: defaultStoreOrderEditRelations,
       defaultFields: defaultStoreOrderEditFields,
       allowedFields: defaultStoreOrderEditFields,

packages/medusa/src/api/routes/store/orders/index.ts

@@ -3,7 +3,7 @@ import "reflect-metadata"
 import { Order } from "../../../.."
 import middlewares, {
   transformBody,
-  transformQuery,
+  transformStoreQuery,
 } from "../../../middlewares"
 import requireCustomerAuthentication from "../../../middlewares/require-customer-authentication"
 import { StorePostCustomersCustomerOrderClaimReq } from "./request-order"
@@ -21,7 +21,7 @@ export default (app) => {
    */
   route.get(
     "/",
-    transformQuery(StoreGetOrdersParams, {
+    transformStoreQuery(StoreGetOrdersParams, {
       defaultFields: defaultStoreOrdersFields,
       defaultRelations: defaultStoreOrdersRelations,
       allowedFields: allowedStoreOrdersFields,
@@ -36,7 +36,7 @@ export default (app) => {
    */
   route.get(
     "/:id",
-    transformQuery(StoreGetOrderParams, {
+    transformStoreQuery(StoreGetOrderParams, {
       defaultFields: defaultStoreOrdersFields,
       defaultRelations: defaultStoreOrdersRelations,
       allowedFields: allowedStoreOrdersFields,

packages/medusa/src/api/routes/store/orders/lookup-order.ts

@@ -100,7 +100,9 @@ export default async (req, res) => {
 
   const order = orders[0]
 
-  res.json({ order: cleanResponseData(order, req.allowedProperties || []) })
+  res.json({
+    order: cleanResponseData(order, req.allowedProperties || []),
+  })
 }
 
 export class ShippingAddressPayload {

packages/medusa/src/api/routes/store/payment-collections/index.ts

@@ -2,7 +2,7 @@ import { Router } from "express"
 import "reflect-metadata"
 import middlewares, {
   transformBody,
-  transformQuery,
+  transformStoreQuery,
 } from "../../../middlewares"
 
 import { PaymentCollection, PaymentSession } from "../../../../models"
@@ -18,7 +18,7 @@ export default (app, container) => {
 
   route.get(
     "/:id",
-    transformQuery(StoreGetPaymentCollectionsParams, {
+    transformStoreQuery(StoreGetPaymentCollectionsParams, {
       defaultFields: defaultPaymentCollectionFields,
       defaultRelations: defaultPaymentCollectionRelations,
       isList: false,

packages/medusa/src/api/routes/store/product-categories/index.ts

@@ -1,5 +1,5 @@
 import { Router } from "express"
-import middlewares, { transformQuery } from "../../../middlewares"
+import middlewares, { transformStoreQuery } from "../../../middlewares"
 import { ProductCategory } from "../../../../models"
 import { PaginatedResponse } from "../../../../types/common"
 
@@ -18,7 +18,7 @@ export default (app) => {
 
   route.get(
     "/",
-    transformQuery(StoreGetProductCategoriesParams, {
+    transformStoreQuery(StoreGetProductCategoriesParams, {
       defaultFields: defaultStoreProductCategoryFields,
       allowedFields: allowedStoreProductCategoryFields,
       defaultRelations: defaultStoreProductCategoryRelations,
@@ -29,7 +29,7 @@ export default (app) => {
 
   route.get(
     "/:id",
-    transformQuery(StoreGetProductCategoryParams, {
+    transformStoreQuery(StoreGetProductCategoryParams, {
       defaultFields: defaultStoreProductCategoryFields,
       allowedFields: allowedStoreProductCategoryFields,
       defaultRelations: defaultStoreProductCategoryRelations,

packages/medusa/src/api/routes/store/product-tags/index.ts

@@ -1,7 +1,7 @@
 import { Router } from "express"
 import { ProductTag } from "../../../../models"
 import { PaginatedResponse } from "../../../../types/common"
-import middlewares, { transformQuery } from "../../../middlewares"
+import middlewares, { transformStoreQuery } from "../../../middlewares"
 import { StoreGetProductTagsParams } from "./list-product-tags"
 
 const route = Router()
@@ -11,7 +11,7 @@ export default (app: Router) => {
 
   route.get(
     "/",
-    transformQuery(StoreGetProductTagsParams, {
+    transformStoreQuery(StoreGetProductTagsParams, {
       defaultFields: defaultStoreProductTagFields,
       defaultRelations: defaultStoreProductTagRelations,
       allowedFields: allowedStoreProductTagFields,

packages/medusa/src/api/routes/store/product-types/index.ts

@@ -1,7 +1,7 @@
 import { Router } from "express"
 import { ProductType } from "../../../.."
 import { PaginatedResponse } from "../../../../types/common"
-import middlewares, { transformQuery } from "../../../middlewares"
+import middlewares, { transformStoreQuery } from "../../../middlewares"
 import "reflect-metadata"
 import { StoreGetProductTypesParams } from "./list-product-types"
 
@@ -12,7 +12,7 @@ export default (app) => {
 
   route.get(
     "/",
-    transformQuery(StoreGetProductTypesParams, {
+    transformStoreQuery(StoreGetProductTypesParams, {
       defaultFields: defaultStoreProductTypeFields,
       defaultRelations: defaultStoreProductTypeRelations,
       allowedFields: allowedStoreProductTypeFields,

packages/medusa/src/api/routes/store/products/index.ts

@@ -1,8 +1,8 @@
-import { RequestHandler, Router } from "express"
+import { Router } from "express"
 import "reflect-metadata"
 
 import { Product } from "../../../.."
-import middlewares, { transformQuery } from "../../../middlewares"
+import middlewares, { transformStoreQuery } from "../../../middlewares"
 import { PaginatedResponse } from "../../../../types/common"
 import { extendRequestParams } from "../../../middlewares/publishable-api-key/extend-request-params"
 import { validateProductSalesChannelAssociation } from "../../../middlewares/publishable-api-key/validate-product-sales-channel-association"
@@ -19,7 +19,7 @@ export default (app) => {
 
   route.get(
     "/",
-    transformQuery(StoreGetProductsParams, {
+    transformStoreQuery(StoreGetProductsParams, {
       defaultRelations: defaultStoreProductsRelations,
       defaultFields: defaultStoreProductsFields,
       allowedFields: allowedStoreProductsFields,
@@ -31,7 +31,7 @@ export default (app) => {
 
   route.get(
     "/:id",
-    transformQuery(StoreGetProductsProductParams, {
+    transformStoreQuery(StoreGetProductsProductParams, {
       defaultRelations: defaultStoreProductsRelations,
       defaultFields: defaultStoreProductsFields,
       allowedFields: allowedStoreProductsFields,

packages/medusa/src/types/global.ts

@@ -21,6 +21,7 @@ declare global {
       retrieveConfig: FindConfig<unknown>
       filterableFields: Record<string, unknown>
       allowedProperties: string[]
+      includes?: Record<string, boolean>
       errors: string[]
     }
   }

packages/medusa/src/utils/clean-response-data.ts

@@ -1,21 +1,53 @@
 import { pick } from "lodash"
 
+// TODO: once the legacy totals decoration will be removed.
+// We will be able to only compute the totals if one of the total fields is present
+// and therefore avoid totals computation if the user don't want them to appear in the response
+// and therefore the below const will be removed
+const EXCLUDED_FIELDS = [
+  "shipping_total",
+  "discount_total",
+  "tax_total",
+  "refunded_total",
+  "total",
+  "subtotal",
+  "paid_total",
+  "refundable_amount",
+  "gift_card_total",
+  "gift_card_tax_total",
+  "item_tax_total",
+  "shipping_tax_total",
+  "refundable",
+  "original_total",
+  "original_tax_total",
+]
+
 /**
- * Filter response data to contain props specified in the fields array.
+ * Filter response data to contain props specified in the `allowedProperties`.
+ * You can read more in the transformQuery middleware utility methods.
  *
  * @param data - record or an array of records in the response
  * @param fields - record props allowed in the response
  */
-function cleanResponseData<T>(data: T, fields: string[]) {
+function cleanResponseData<T extends unknown | unknown[]>(
+  data: T,
+  fields: string[]
+): T extends [] ? Partial<T>[] : Partial<T> {
   if (!fields.length) {
-    return data
+    return data as T extends [] ? Partial<T>[] : Partial<T>
   }
 
-  if (Array.isArray(data)) {
+  const isDataArray = Array.isArray(data)
-    return data.map((record) => pick(record, fields))
+  const fieldsSet = new Set([...fields, ...EXCLUDED_FIELDS])
-  }
 
-  return pick(data, fields)
+  fields = [...fieldsSet]
+
+  let arrayData: Partial<T>[] = isDataArray ? data : [data]
+  arrayData = arrayData.map((record) => pick(record, fields))
+
+  return (isDataArray ? arrayData : arrayData[0]) as T extends []
+    ? Partial<T>[]
+    : Partial<T>
 }
 
 export { cleanResponseData }

packages/modules-sdk/package.json

@@ -32,6 +32,5 @@
     "build": "tsc --build",
     "test": "jest",
     "test:unit": "jest"
-  },
+  }
-  "peerDependencies": {}
 }