medusajs / medusa

The world's most flexible commerce platform.
https://medusajs.com
MIT License
25.93k stars 2.6k forks source link

Custom middleware Auth and CORS #6585

Closed andrei-s-making-science closed 3 days ago

andrei-s-making-science commented 8 months ago

Bug report

Describe the bug

Playing around with authorization middleware, bit similar to https://docs.medusajs.com/modules/users/backend/rbac

When adding a custom middleware with matcher for /admin/* routes: For the core /admin routes e.g. GET /admin/users:

  1. in case of bearer token request - req.user.userId is not defined ❌
  2. in case of cookies request - req.session.user_id is defined ✅
  3. in case of api_token request - req.user is not defined ❌
  4. in case of non authorized request - req.user is not defined, req.session.user_id is not defined ✅

For /admin/custom routes e.g. GET /admin/custom/roles

  1. in case of bearer token request - req.user.userId is defined ✅
  2. in case of cookies request - req.session.user_id is defined ✅
  3. in case of api_token request - req.user is defined as User object ✅
  4. in case of non authorized request - req.user is not defined, req.session.user_id is not defined ✅

Expected behaviour: For the core /admin routes e.g. GET /admin/users - 1. and 3. should define req.user.user_id and req.user accordingly.

Notes

Tried some workarounds described in: https://github.com/medusajs/medusa/issues/5964, which have other issues. See other sections below for more information.

Would really appreciate your help.

System information

Medusa version (including plugins): v1.20.2, (see the package.json below) Node.js version: v18 Database: Postgres Operating system: MacOS 12.5.1, Apple M1 Pro Browser (if relevant): Chrome

Steps to reproduce the behavior

Predecessor

  1. Create middleware function
  2. Apply the middleware in middlewares.ts import { MiddlewaresConfig, authenticate } from '@medusajs/medusa'; import authorize from './middlewares/authorize'; import log from './middlewares/log';

Example:

import { MiddlewaresConfig } from '@medusajs/medusa';
import authorize from './middlewares/authorize';

export const config: MiddlewaresConfig = {
  routes: [
    {
      matcher: /^\/admin\/(?!auth|analytics-config|users\/reset-password|users\/password-token|invites\/accept).*/,
      middlewares: [authorize],
    },
  ],
};

Case 1. Middlewares

  1. Follow Predecessor steps
  2. Request GET /admin/users

Actual result: req.user, req.session.user_id are undefined Expected result: req.user, req.session.user_id are defined

Case 2. Middlewares with authenticate() middleware

  1. Follow Predecessor steps
  2. Add authenticate() middleware before the custom middleware.
  3. Request GET /admin/users

Example:

import { MiddlewaresConfig, authenticate } from '@medusajs/medusa';
import authorize from './middlewares/authorize';

export const config: MiddlewaresConfig = {
  routes: [
    {
      matcher: /^\/admin\/(?!auth|analytics-config|users\/reset-password|users\/password-token|invites\/accept).*/,
      middlewares: [authenticate(), authorize],
    },
  ],
};

Actual result: req.user, req.session.user_id are defined, CORS error during request from admin UI for /admin/users (exceptions: some endpoints e.g. /admin/products) Expected result: req.user, req.session.user_id are defined, CORS errors and not thrown

Case 3. Middlewares with authenticate() and cors() middlewares

  1. Follow Predecessor steps
  2. Add authenticate() middleware before the custom middleware.
  3. Add core() middleware before the authenticate() middleware.
  4. Request GET /admin/users

Example:

import { MiddlewaresConfig, authenticate } from '@medusajs/medusa';
import authorize from './middlewares/authorize';
import cors from 'cors'

export const config: MiddlewaresConfig = {
  routes: [
    {
      matcher: /^\/admin\/(?!auth|analytics-config|users\/reset-password|users\/password-token|invites\/accept).*/,
      middlewares: [cors(), authenticate(), authorize],
    },
  ],
};

Actual result: req.user, req.session.user_id are defined, CORS error during request from admin UI for /admin/users (exceptions: some endpoints e.g. /admin/products) Expected result: req.user, req.session.user_id are defined, CORS errors and not thrown

Expected behavior

No additional middlewares required or provide a solution of how to avoid the CORS issue.

Screenshots

CORS if authenticate() middleware applied:

Screenshot 2024-03-05 at 13 09 14

No CORS for some admin endpoints:

Screenshot 2024-03-05 at 13 09 26

Code snippets

Configuration

package.json
"dependencies": {
    "@medusajs/cache-inmemory": "^1.8.9",
    "@medusajs/cache-redis": "^1.8.9",
    "@medusajs/event-bus-local": "^1.9.7",
    "@medusajs/event-bus-redis": "^1.8.10",
    "@medusajs/file-local": "^1.0.2",
    "@medusajs/inventory": "^1.11.6",
    "@medusajs/medusa": "^1.20.2",
    "@medusajs/stock-location": "^1.11.5",
    "awilix": "^8.0.1",
    "body-parser": "^1.19.0",
    "class-validator": "^0.14.1",
    "cors": "^2.8.5",
    "dotenv": "16.3.1",
    "express": "^4.17.2",
    "medusa-core-utils": "^1.2.1",
    "medusa-fulfillment-manual": "^1.1.39",
    "medusa-interfaces": "^1.3.7",
    "medusa-payment-manual": "^1.0.24",
    "medusa-plugin-strapi": "*",
    "typeorm": "^0.3.16"
  },
  "devDependencies": {
    "@babel/cli": "^7.14.3",
    "@babel/core": "^7.14.3",
    "@babel/preset-typescript": "^7.21.4",
    "@medusajs/medusa-cli": "^1.3.21",
    "@types/express": "^4.17.13",
    "@types/jest": "^27.4.0",
    "@types/jsonwebtoken": "^9.0.5",
    "@types/node": "^18.19.3",
    "@typescript-eslint/eslint-plugin": "^6.16.0",
    "@typescript-eslint/parser": "^6.16.0",
    "babel-preset-medusa-package": "^1.1.19",
    "cross-env": "^7.0.3",
    "eslint": "^8.56.0",
    "eslint-config-airbnb-base": "^15.0.0",
    "eslint-config-airbnb-typescript": "^17.1.0",
    "eslint-config-prettier": "^9.1.0",
    "eslint-plugin-import": "^2.29.1",
    "eslint-plugin-prettier": "^5.1.2",
    "husky": "^8.0.3",
    "jest": "^27.3.1",
    "lint-staged": "^15.2.0",
    "prettier": "^3.1.1",
    "ts-jest": "^27.0.7",
    "ts-loader": "^9.2.6",
    "typescript": "^4.5.2"
  },

.env

# Admin
## CORS
ADMIN_CORS=http://localhost:7001

admin ui is being accessed via http://localhost:7001. not sure if this information is required, as described in Summary section: CORS is being thrown in case authenticate() middleware applied and not every core /admin endpoint throws CORS.

Code

/src/api/middlewares.ts
import { MiddlewaresConfig, authenticate } from '@medusajs/medusa';
import authorize from './middlewares/authorize';
import log from './middlewares/log';

export const config: MiddlewaresConfig = {
  routes: [
    {
      // TODO: verify if analytics-config requires auth. Throwing CORS at the moment.
      matcher: /^\/admin\/(?!auth|analytics-config|users\/reset-password|users\/password-token|invites\/accept).*/,
      // TODO: verify why authenticate throws cors and why req.user doesn't exist in case no authenticate()
      middlewares: [authorize],
    },
  ],
};
/src/api/middlewares/authorize.ts

Not sharing a code as of redundancy, tried an empty log middleware - same result as described in Summary section e.g.

import type { MedusaNextFunction, MedusaRequest, MedusaResponse } from '@medusajs/medusa';

async function log(req: MedusaRequest, res: MedusaResponse, next: MedusaNextFunction): Promise<void> {
  console.warn(`:::req.app._router.stack`, req.app._router.stack);
  console.warn(
    `:::req.app._router.stack`,
    req.app._router.stack.map((element: { handle: () => unknown }) => element.handle?.toString()),
  );
  console.warn(`:::req.user`, req.user);
  console.warn(`:::req.session`, req.session);

  next();
}

export default log;

Additional context

Things i've tried:

  1. If i add authenticate() middleware - req.user will be defined for the core /admin routes, but CORS errors will appear on admin UI for some of the endpoints: GET /admin/users, GET /admin/store, GET /invites. Some of the endpoints do not throw CORS e.g. GET /admin/products
    1. If i additionally add cors() middleware - same issue as described in 1.
  2. req.app._router.stack in custom middleware contains the authenticate middleware, but looks like it's not applied.
    1. documentation mentions that there is no need of adding authenticate() or cors() middlewares, as they are in place, which seems to be right, but not sure exactly how medusa code operates, cannot even understand how v1 grabs the middlewares, found the code only for v2 (looks like i need more time for this).
  3. cookies, tokens exist in req
  4. if i specify middleware methods without OPTIONS - CORS preflifght will return 200 instead of 204 and relevant request will fail. So i have to check for method in the middleware code and skip OPTIONS methods.
  5. reinstalled node_modules
  6. when i've placed a bunch of console.log() in the node_modules/@medusajs/medusa/dist/ to debug - the issue disappeared. Can be a race condition of i'm missing something. But when i've tried to access without auth and then put it back on - it began again.
  7. looks like CORS error is also happening when session runs out.
olivermrbl commented 8 months ago

@andrei-s-making-science, thanks for sharing. Exemplary issue description.

Any chance you can provide a reproduction in a repository? That would make the time to resolution shorter.

andrei-s-making-science commented 8 months ago

@olivermrbl medusa.zip

You can remove the usage of medusa-plugin-strapi (in medusa-config.js, package.json), it should not affect the result.

Thanks for your help!

codeustad commented 8 months ago

Try with import * as cors from 'cors' in your middleware.

andrei-s-making-science commented 8 months ago

@codeustad will do that and will get back to you with the results.

What about necessity of the authenticate() middleware? as i can see it is configured and stored in req.app._router.stack, but bearer token and api_token are not providing req.user for core /admin resources?

UPDATE:

import { MiddlewaresConfig, authenticate } from '@medusajs/medusa';
import { parseCorsOrigins } from 'medusa-core-utils';
import authorize from './middlewares/authorize';
import * as cors from 'cors';

export const config: MiddlewaresConfig = {
  routes: [
    {
      // TODO: verify if analytics-config requires auth. Throwing CORS at the moment.
      matcher: /^\/admin\/(?!auth|analytics-config|users\/reset-password|users\/password-token|invites\/accept).*/,
      // TODO: verify why authenticate throws cors and why req.user doesn't exist in case no authenticate()
      middlewares: [
        cors.default({ credentials: true, origin: parseCorsOrigins(process.env.ADMIN_CORS ?? '') }),
        authenticate(),
        authorize,
      ],
    },
  ],
};

Looks like this helped. I believe this can be used as a temporary solution for us, huge thanks @codeustad, looks like i've messed up with import during my tests. Still having a question, which is described above 👀

github-actions[bot] commented 2 weeks ago

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 3 days.

github-actions[bot] commented 3 days ago

This issue was closed because it has been stalled for 3 days with no activity.