Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Undefined User in middlewares #5964

Closed
lukebui opened this issue Dec 25, 2023 · 14 comments
Closed

Undefined User in middlewares #5964

lukebui opened this issue Dec 25, 2023 · 14 comments

Comments

@lukebui
Copy link
Contributor

lukebui commented Dec 25, 2023

Bug report

Describe the bug

According to the example on how to create a middleware, the req.user property should show the currently logged in user. However, when applying a middleware to existing routes, the req.user is undefined. Applying to custom routes will correctly attach the user record to the MedusaRequest.

System information

Medusa version (including plugins): v1.19.0
Node.js version: v18 or v20
Database: Postgres (duh)
Operating system: Linux
Browser (if relevant):

Steps to reproduce the behavior

Step 1: Create a new app using the medusa new command and the default starter template.

Step 2: Create a middleware to console.log the current logged in user at path src/api/middlewares.ts:

import {
  MedusaNextFunction,
  MedusaRequest,
  MedusaResponse,
  MiddlewaresConfig,
} from "@medusajs/medusa";

export const config: MiddlewaresConfig = {
  routes: [
    {
      matcher: "*",
      middlewares: [
        (
          req: MedusaRequest,
          _res: MedusaResponse,
          next: MedusaNextFunction
        ) => {
          console.log(req.user);
          next();
        },
      ],
    },
  ],
};

Step 3: Create a custom API route at src/api/admin/hello/route.ts:

import { MedusaRequest, MedusaResponse } from "@medusajs/medusa";

export async function GET(_req: MedusaRequest, res: MedusaResponse) {
  res.send("Hello there.");
}

Step 4: Run the dev server using npm run dev.

Expected behavior

When accessing an existing route, e.g. /admin/products using Postman, the console log for req.user would be undefined. But for the custom route /admin/hello, the app correctly log the current user (or should I say {userId: '<string>'}).

Update 1:

I tried:

import {
  MedusaNextFunction,
  MedusaRequest,
  MedusaResponse,
  MiddlewaresConfig,
  authenticate
} from "@medusajs/medusa";

export const config: MiddlewaresConfig = {
  routes: [
    {
      matcher: "*",
      middlewares: [
        authenticate(),
        (
          req: MedusaRequest,
          _res: MedusaResponse,
          next: MedusaNextFunction
        ) => {
          console.log(req.user);
          next();
        },
      ],
    },
  ],
};

but the program gets caught in an infinite loop.

@lukebui
Copy link
Contributor Author

lukebui commented Dec 25, 2023

@adrien2p has helped me to find the solution.

First, the authenticate middleware must be called before custom middleware for protected routes. Second, one must remember to whitelist unprotected routes such as auth.

This should be in the documentation.

@lukebui lukebui closed this as completed Dec 25, 2023
@lukebui
Copy link
Contributor Author

lukebui commented Dec 25, 2023

Does applying custom middlewares override all default ones?

@lukebui lukebui reopened this Dec 25, 2023
@adrien2p
Copy link
Member

No it doesn't, but applying them on core route will apply the custom ones first

@helkaroui
Copy link

I'm trying to follow your blog on extending medusa to Marketplace https://medusajs.com/blog/extending-medusa-usecase-marketplace/

And I have the same issue with the middleware :
image
when using the custom middleware requests are not authenticated :
image

What is the recommended way to apply authentication middleware before the custom ones ?

@adrien2p
Copy link
Member

adrien2p commented Jan 8, 2024

you need to apply the authenticate middleware first

@pmltechpile
Copy link

you need to apply the authenticate middleware first

I am also experiencing a similar issue. I extended three services: Product, Store and User. For both the Product and Store I am able to pull the loggedInUser just fine however for the User service, I am getting a null value when I pull the loggedInUser from the container. This is what I have for the MiddlewaresConfig where I apply the authenticate middleware first as you have advised to do:

MiddlewaresConfig

@pmltechpile
Copy link

pmltechpile commented Jan 26, 2024

you need to apply the authenticate middleware first

I am also experiencing a similar issue. I extended three services: Product, Store and User. For both the Product and Store I am able to pull the loggedInUser just fine however for the User service, I am getting a null value when I pull the loggedInUser from the container. This is what I have for the MiddlewaresConfig where I apply the authenticate middleware first as you have advised to do:

MiddlewaresConfig

Ok I got it to work with the help of other members in the discord. Ultimately one had a solution to make the service TRANSIENT and not SCOPED. This was the only service that differs from the other services I have extended (PRODUCT, ORDERS and STORE) whicha are all made to be SCOPED. Extremely confused as to why this is the case but at least it's working now and I hope this helps others when running across this issue. IMO this issue should not be marked as closed as per the docs (https://docs.medusajs.com/development/services/extend-service) we are encouraged to make the extended service SCOPED.

@vholik
Copy link

vholik commented Feb 6, 2024

you need to apply the authenticate middleware first

I am also experiencing a similar issue. I extended three services: Product, Store and User. For both the Product and Store I am able to pull the loggedInUser just fine however for the User service, I am getting a null value when I pull the loggedInUser from the container. This is what I have for the MiddlewaresConfig where I apply the authenticate middleware first as you have advised to do:
MiddlewaresConfig

Ok I got it to work with the help of other members in the discord. Ultimately one had a solution to make the service TRANSIENT and not SCOPED. This was the only service that differs from the other services I have extended (PRODUCT, ORDERS and STORE) whicha are all made to be SCOPED. Extremely confused as to why this is the case but at least it's working now and I hope this helps others when running across this issue. IMO this issue should not be marked as closed as per the docs (https://docs.medusajs.com/development/services/extend-service) we are encouraged to make the extended service SCOPED.

Can confirm that marking user service as SCOPED will result in not registering dependency in awilix (in my case loggedInUser). Marking service as TRANSIENT solved this issue.

@marcopadillo
Copy link

Anyone else encountering a CORS issue when adding a product with the following middleware config:

export const config: MiddlewaresConfig = {
  routes: [
    {
      matcher: /\/admin\/[^(auth)].*/,
      middlewares: [authenticate(), registerLoggedInUser],
    },
  ],
}

image

Adding cors() inside the middlewares array would solve this problem, but loggedInUser will revert to being undefined again. It seems the sequence of middlewares inside the array does not matter.

@niioo
Copy link

niioo commented Apr 25, 2024

facing the same issue

@niioo
Copy link

niioo commented Apr 25, 2024

has anyone fixed this??

@kim-hanho
Copy link

kim-hanho commented May 8, 2024

This is not a bug from Medusa, but from the multi-stage dependencies.
The issue is that your service(e.g ProductService()) is constructed before the loggedInUser data is injected.

LoggedInUserMiddleware uses UserService() as dependency, and (in my case) my UserService uses ShippingProfileService as dependency.
Because Medusa core's ShippingProfileService uses ProductService as dependency, so it means that... when I try to call UserService from LoggedInUserMiddleware, UserService -> ShippingProfileService -> ProductService has been constructed in a sequence.

So the ProductService which does not has LoggedInUser data inside, is being reused for the whole request.

So please check your UserService()'s dependencies.

My solution is create a custom service which does not has any dependent services and use it inside of LoggedInUserMiddleware, instead of using UserService.

class IndependentUserService extends TransactionBaseService {
  // Service that does not have any dependency in other services

  protected userRepository_: typeof UserRepository

  constructor(container) {
    super(container)

    this.userRepository_ = container.userRepository;
  }

  async retrieve(userId: string, config: FindConfig<User> = {}): Promise<User> {
    if (!isDefined(userId)) {
      throw new MedusaError(
        MedusaError.Types.NOT_FOUND,
        `"userId" must be defined`
      )
    }

    const userRepo = this.activeManager_.withRepository(this.userRepository_)
    const query = buildQuery({ id: userId }, config)

    const users = await userRepo.find(query)

    if (!users.length) {
      throw new MedusaError(
        MedusaError.Types.NOT_FOUND,
        `User with id: ${userId} was not found`
      )
    }

    return users[0]
  }
}

export default IndependentUserService

and

in loggedInUser middleware

    ...
    if (req.user && req.user.userId) {
      const independentUserService =
        req.scope.resolve("independentUserService") as IndependentUserService
      loggedInUser = await independentUserService.retrieve(req.user.userId)
    }

    req.scope.register({
      loggedInUser: {
        resolve: () => loggedInUser,
      },
    })
    ...

@EdgarSilvaAlluxi
Copy link

Anyone else encountering a CORS issue when adding a product with the following middleware config:

export const config: MiddlewaresConfig = {
  routes: [
    {
      matcher: /\/admin\/[^(auth)].*/,
      middlewares: [authenticate(), registerLoggedInUser],
    },
  ],
}

image

Adding cors() inside the middlewares array would solve this problem, but loggedInUser will revert to being undefined again. It seems the sequence of middlewares inside the array does not matter.

Did you fixed the issue? im facing the same problem...

@u11d-bartlomiej-galezowski

#6585 (comment)

Here is workaround for that issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests