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

API Routes / Express Endpoints incompatibilities #7804

Closed
sonntag-philipp opened this issue Jun 23, 2024 · 5 comments
Closed

API Routes / Express Endpoints incompatibilities #7804

sonntag-philipp opened this issue Jun 23, 2024 · 5 comments

Comments

@sonntag-philipp
Copy link

Bug report

Hello everyone, I am currently trying to implement a marketplace using MercurJS based on the Medusa backend. I'm facing issues with the implementation of the password-reset route, but I think this is likely to affect other routes as well because of the routing design.

Describe the bug

The route.ts file in the source of Mercur overrides the POST method for all ids that occur within the /admin/users route.

This does not just override the functionality for posting specific user entities, but also override routes like /admin/users/password-token. It is currently not clear based on the Documentation of Medusa how to solve such issues.

Here is the code that currently handles the password-token route in the MedusaJS Api. It does it using the Express endpoints api. It is not clear how this can be integrated in the API Routes approach.

System information

Medusa version (including plugins):
Node.js version:
Database:
Operating system:
Browser (if relevant):

Steps to reproduce the behavior

  1. Setup the MercurJS backend using the CLI
  2. Try to execute the /admin/users/password-token route

Expected behavior

The password token is generated so the user can reset the password.

Screenshots


Code snippets


Additional context


@kasperkristensen
Copy link
Contributor

kasperkristensen commented Jun 24, 2024

Hey @sonntag-philipp

This happens because the override is registered before the default endpoint, and our API Routes are simply a wrapper around express, meaning the same rules applies - the first handler that matches is used. If you don't need to also overwrite the /password-token endpoint, you can make your override to /:user_id skip that exact path, which will allow it to pass through to the existing handler:

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

const SKIP_PATHS = ["/admin/users/password-token"];

export const POST = (req: MedusaRequest, res: MedusaResponse) => {
  if (SKIP_PATHS.includes(req.path)) {
    return req.next();
  }

  // Your code here
};

@sonntag-philipp
Copy link
Author

Thank you for the fast answer!

I've created a breakpoint and also a console log in the POST API Route, but it never gets executed when I call /admin/users/password-token. I do not really understand why it is not called and I'm not quite sure how to implement the endpoint myself and redirect the api request to the usual route.

Handler with Breakpoint and console.log

image

Logged output

image

@kasperkristensen
Copy link
Contributor

Hi @sonntag-philipp, looking at the source code you have linked it seems to be because the project has some custom admin auth middleware that is also being applied to this endpoint, even though it shouldn't be. The same principle applies, they would need to skip applying the middleware to this route, as it should not require a user to be logged in.

It's this line that would need to changed to also skip the password token route: https://github.com/mercurjs/mercur-api-starter/blob/982e50f0adf87307caec79200cde26c053bc7b93/src/api/middlewares.ts#L11

Hope that answers your question. Closing this issue as it's not related to an issue in Medusa.

@NicolasGorga
Copy link

@sonntag-philipp hey, did you manage to solve this? I took the skip_paths approach, plus changing Mercur's middleware config which is wrong for these unprotected routes, but it still seems to return 401 for me.

@sonntag-philipp
Copy link
Author

Hi, I did not put any more effort into this and switched to a different NodeJS eCommerce instead.

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

No branches or pull requests

3 participants