Compatibility with Medusa 1.17 sessions

[dPreininger]

Update to work with 1.17 sessions.

IMPORTANT: this breaks the expiresIn argument, as sessions don't expire in Medusa. A further update to Medusa core is required to set configurable expiry time on cookies and JWT tokens. This is also planned.

Supporting Bearer Auth using current structure is non-trivial, as auth strategies return status 30x (redirect).

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
medusa-plugins	❌ Failed (Inspect)			Nov 11, 2023 5:46pm

[adrien2p]

thanks for the pr have you been able to test it? for the token on redirect, do you think it could be part of the url? what do you think about the security of this?

[dPreininger]

I have tested it with the Firebase strategy and it worked.

For the access token, I was thinking something along the lines of using a second endpoint. Basically, let say that the endpoint is /<store,admin>/auth/token. This endpoint would be guarded by regular session auth. Then you could specify this endpoint as redirect url. So the way you would obtain a token would be by calling: /<store,admin>/auth/google?redirectUrl=/<store,admin>/auth/token and then the response would be { access_token: string }.

[adrien2p]

I have tested it with the Firebase strategy and it worked.

For the access token, I was thinking something along the lines of using a second endpoint. Basically, let say that the endpoint is /<store,admin>/auth/token. This endpoint would be guarded by regular session auth. Then you could specify this endpoint as redirect url. So the way you would obtain a token would be by calling: /<store,admin>/auth/google?redirectUrl=/<store,admin>/auth/token and then the response would be { access_token: string }.

That sounds reasonable in my humble opinion

[adrien2p]

Can I get you to run a test with google as well please, just to be sure

[bacarybruno]

Regarding access token, is it a technical limitation that forces to create a different route guarded by the cookie ?
I think that it can work well with web apps maybe but for mobile apps it would be better to have a way to directly get the JWT Bearer token. In react-native for instance, there are many kind of issues with cookies based auth: facebook/react-native#23185. Wdyt ?

[dPreininger]

Yes, I am aware mobile apps (as well as some web apps such as SPAs) have a bunch of limitations and issues with session/cookie based authentication systems. Bearer auth is supposed to fix that.

The other option that might be a little bit better is to change the authCallbackMiddleware function. Right now it accepts successRedirectGetter function as the last parameter. Maybe it would be better to accept successAction instead which would change the behaviour like so:

Currently, successRedirectGetter has this signiture: () => string
successAction would have this signiture: (res: Response) => void

If currently you just return the redirect url like so: () => '/some/url';

Now, you would also be able to specify the action like so: (res) => res.redirect('/some/url'); OR (res) => res.json({ access_token: 'some token' })

This would allow us to directly return the token. Then we could add a global configuration option the same way we have the strict option but it would be if we redirect to success page or to send the token instead.

What do you think @adrien2p?

[adrien2p]

I agree with the suggestion of being able to specify the action. What I would suggest is that we rely on the query param maybe to switch the behaviour, at the moment there is a redirectTo that you can provide on the url to force the redirection to somewhere else, maybe we could pass returnAccessToken=true instead and this would prevent the redirection and return the access token. That way it can be managed more granularly in a case by case need?

[dPreininger]

I agree with the suggestion of being able to specify the action. What I would suggest is that we rely on the query param maybe to switch the behaviour, at the moment there is a redirectTo that you can provide on the url to force the redirection to somewhere else, maybe we could pass returnAccessToken=true instead and this would prevent the redirection and return the access token. That way it can be managed more granularly in a case by case need?

Sure, I'll prepare the implementation like that.

[dPreininger]

I have prepared the implementation as we discussed above. However, I don't believe this is the way to go. The problem is that when we call the callback function, we just get displayed the JSON response. We don't get back to the store page.

I actually think that the original plan you had might work best (sending access token as a URL query parameter). Should I prepare that implementation instead?

The proper way of returning an access token is slightly different. We generally complete the OAuth flow in either a new window (pop up window) or a new tab. The main tab waits for it to complete and then handles saving the token once it is returned.

I don't think we should go down this route, as a lot of the plugin would have to be overwritten. So my vote is to implement you original idea @adrien2p.

[adrien2p]

I am good with that, then the consumer can get the token from that url and store it in the token manager, correct ?

[dPreininger]

I am good with that, then the consumer can get the token from that url and store it in the token manager, correct ?

Yes, you would store either in localStorage or sessionStorage (or some other persistent storage if you are on mobile).

[bacarybruno]

Just to be sure to understand, may I ask what would be the flow for mobile clients that just need to get the token ?

[odhekar]

I find it is a bit surprising that a pure JWT auth is not a "first class" scenario being considered similar to or over session auth.
We have mobile app and spa store fronts and a pure JWT based flow is a much common scenario these days.

[adrien2p]

@odhekar it is available in medusa directly, so you don't need the plugin for that.

@bacarybruno you will use the auth route and will be able to get the access token in the url of the redirect

@dPreininger will be able to confirm ☺️

[dPreininger]

Just to be sure to understand, may I ask what would be the flow for mobile clients that just need to get the token ?

You will call something like /store/auth/<your strategy>?returnAccessToken=true

Than you will receive an JSON response with the token. This is more useful for mobile apps.

Alternatively with SPAs you would call this endpoint without the query param or with ?redirectTo=someUrl query param. The token will then be inside the response url. If your response URL is something like /dashboard, the actual return url would be /dashboard?accessToken={{accessToken}}. It is worth noting that all responses will also return an authenticated session cookie, so either auth approach would work.

[bacarybruno]

Okay that's great! Having a /store/auth/<your strategy>?returnAccessToken=true as you said would be good for mobile apps indeed 👍

[adrien2p]

@dPreininger looks good, should we re include the custom expires in when the user want to get the token, it could be a qs alongside the returnAccess token? Also, the documentation will need to be updated to explain about the new qs available

[dPreininger]

I would not recommend setting expiry time on query arguments. I would prefer expiresIn would be as it was right now, setting it in config, otherwise there is no way to prevent customers to request tokens or sessions with extremely long validity periods.

[adrien2p]

Yes you are completely right, i don't know why i did suggest it as i just wanted to support it back 😅if we could just put it back since it was removed in this pr.

[adrien2p]

@dPreininger keep me in touch once the pr is ready for review after the doc update and the lifetime options are back 👌 that way we got some users ready to test

[dPreininger]

I will, but it won't be before the weekend.

[tsvetann]

do you guys need help (other than testing) to move this forward? Thanks for your efforts

[adrien2p]

@tsvetann thanks, would be great to have more tester, also, the pr needs to re add the lifetime options in the options

[tsvetann]

@dPreininger do you need any help with the implementation? Thanks for taking care

[rasmuslian]

@adrien2p @dPreininger Hey, great work so far by taking this seriously. Just wondered how the timeline looks for getting this into production? Thanks again.

[tsvetann]

@tsvetann did you scceed?

@adrien2p I npm packed dPreininger clone as he has the new changes, however when i navigate to http://localhost:9000/store/auth/google I get {"exists":false}. Are the new changes non-breaking? do I need to adjust my medusa-config to accomodate the new changes?

[adrien2p]

Someone opened an issue as well about that. Ill investigate that asap and will come back to you. I don't understand cause before the last release i ve tested and didn't come across this issue

[tsvetann]

Someone opened an issue as well about that. Ill investigate that asap and will come back to you. I don't understand cause before the last release i ve tested and didn't come across this issue

oh ok. Let me know if I can help you somehow debug this

[adrien2p]

Could you just check if in your node modules there is two different versions of passport? If there is it could be the issue

[tsvetann]

Could you just check if in your node modules there is two different versions of passport? If there is it could be the issue

yarn list passport
yarn list v1.22.19
warning ../../../package.json: No license field
warning Filtering by arguments is deprecated. Please use the pattern option instead.
└─ [email protected]
✨  Done in 0.72s.

Are you available in discord for quicker exchange during debuggin?

[tsvetann]

@adrien2p I tried different approach. Instead of packing the new changes I tried testing by using yarn link locally and when I start medusa I get the following warning:
warn: Running loader failed: Cannot find module 'passport-google-oauth2'

[adrien2p]

@dPreininger thanks a lot man, will you be able to resolve the conflict and update the doc for the new returnAccessToken in the doc directory

[dPreininger]

@dPreininger thanks a lot man, will you be able to resolve the conflict and update the doc for the new returnAccessToken in the doc directory

Yes, sure

[dPreininger]

@adrien2p deployment failed but I haven't got the access to see the logs why it failed.

[adrien2p]

dont worry for the doc, I ll manage from the main branch 💪

[dPreininger]

@adrien2p resolved comments, you can review again

[adrien2p]

Should be good to merge after this comment ❤️🙏

[dPreininger]

Here we go

[adrien2p]

I will probably merge and publish this evening guys 👌

[adrien2p]

@dPreininger @bacarybruno @odhekar @tsvetann @rasmuslian I ve pushed a new version guys, if you want to try

[adrien2p]

Looks like the doc did not deploy still. Ill check today. Dod anyone try the latest changes?

[tsvetann]

Looks like the doc did not deploy still. Ill check today. Dod anyone try the latest changes?

I just installed 1.81. Login with google works but it seems the strict option doesn't work anymore?

I have the following config:

resolve: "medusa-plugin-auth",
    /** @type {import('medusa-plugin-auth').AuthOptions} */
    options: {
      strict: "none"

and when I login with google I get: Guest customer with email already exists. My understanding was that strict: none would allow me to login with any provider. Am I doing anything wrong? or did this feature break with the latest upgrade?

Thanks