medusa icon indicating copy to clipboard operation
medusa copied to clipboard

Third-Party Auth - move session verification

Open pevey opened this issue 1 year ago • 3 comments

This PR is in relation to previous discussion about implementing custom auth, and specifically this conversation: https://github.com/medusajs/medusa/discussions/5251

It moves session verification to auth service methods so that they can be overridden. The authenticate and authenticateCustomer middleware handlers no longer invoke passport directly. Instead, they invoke verifySession and verifyCustomerSession, two new methods added to the authService.

I think moving this functionality make a lot of sense. New users will be looking in the auth service to try to find out how to implement custom auth. It is less intuitive to look for the middlewares. And making them service methods makes them possible to override, making it much simpler to replace the express-session/passport auth entirely and use third-party auth instead.

This should be a non-breaking change. This will make customizing auth much less intimidating since users will not have to figure out how to make it play nice with passport if they don't even want/need to use passport. They can replace it entirely.

pevey avatar Sep 30 '23 16:09 pevey

⚠️ No Changeset found

Latest commit: 6fbff01c6d6bc294e5888933bd6b49bec18c9200

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

changeset-bot[bot] avatar Sep 30 '23 16:09 changeset-bot[bot]

@pevey is attempting to deploy a commit to the medusajs Team on Vercel.

A member of the Team first needs to authorize it.

vercel[bot] avatar Sep 30 '23 16:09 vercel[bot]

@pevey @SonnyFishback Where are we at with this? Would love to implement Auth0 or something similar to abstract away auth but seems like waiting for this makes a lot of sense.

joeldodich avatar Apr 30 '24 15:04 joeldodich

This would be a great feature for our use case - I am building an app with an existing authentication service, and we would like to display parts of Medusa in a unified frontend. I need a way to log users into both our existing app and Medusa, but this isn't currently possible without major changes to the way Medusa handles sessions.

This would fit our use case perfectly, as we would be able to bypass Passport and reuse our existing authentication/sessions.

0xliam avatar May 15 '24 00:05 0xliam

Hey, thanks for the PR! Since v2 brought a lot of architectural changes, we will be closing this PR since it no longer applies to our new setup.

If you would still like to open this PR, please rebase with the v1 branch and re-request a review & we'll get back on it quickly.

riqwan avatar Jul 05 '24 12:07 riqwan

@pevey can you resubmit as this is a feature that we would like to implement with our setup as well.

thomasdefilippis avatar Jul 11 '24 20:07 thomasdefilippis