adrien2p / medusa-plugins

A collection of awesome plugins for medusa :rocket:
https://medusa-plugins.vercel.app
MIT License
144 stars 42 forks source link

Feature/add steam passport #120

Open stephane-segning opened 7 months ago

stephane-segning commented 7 months ago

This is a pull request to add Steam support. But some limitations are to be listed:

vercel[bot] commented 7 months ago

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

Name Status Preview Comments Updated (UTC)
medusa-plugins ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 28, 2023 6:24pm
adrien2p commented 7 months ago

in medusa, the email is mandatory and a user can't exist without email. I would maybe suggest that we store the identifier in the metadata of the user and have a custom validate callback for the steam strategy. Also, it would mean that the user needs to first register/login with the classic method to gather the email and then will be able to use steam to authenticate wdyt?

stephane-segning commented 7 months ago

I would maybe suggest that we store the identifier in the metadata of the user and have a custom validate callback for the steam strategy.

Meaning we should also make sure the metadata is written at some point? Is it in the scope of this plugin? For reading it, it's simple. But writing it should be easy. What are the solutions for writing metadata?

adrien2p commented 7 months ago

You can have a look at the core validator, we are already writing some metadata to store the provider that have been used

stephane-segning commented 7 months ago

I saw that already. But for setting it, how it going to happen?

adrien2p commented 7 months ago

I saw that already. But for setting it, how it going to happen?

The problem is that it would work only if it is when the user gets created. I am not sure how to handle this strategy 😓

stephane-segning commented 7 months ago

This is it. We can also throw an exception and require the user to implement a custom logic for this case

adrien2p commented 7 months ago

This is it. We can also throw an exception and require the user to implement a custom logic for this case

sounds like the best approach for this specific case

piereligio commented 7 months ago

Since I was finally able to run the plugin from filesystem (with working Steam login route until medusa core will refuse the account since has no email), and here I understand that the final solution is to leave the implementation to the user, I'd like to make the necessary edits myself (so that I'll have what I need on my website). But I have some questions, now that I've read this conversation. @stephane-segning , on issue 118 you were mentioning that there is a way using retrieveByApiToken, but here I read that e-mails are mandatory, in medusa. So if I use API token only (editing the plugin core), I will meet issues at some point? Even in the store? (Maybe in this case I could workaround limitations generating a fake email address, and then handle login with API token only)

In my use case, Steam login was mostly for collecting the SteamID in a mandatory way, and assign it to the user, in a way that for the user was quickest and safest way possible. Maybe I should just add a button somewhere that will collect it with a Steam login? Otherwise, maybe the opposite (meaning that the user has to login on Steam and then gets asked for the email)? I wouldn't like to give the user the hassle of getting the SteamID manually, and it also could be manipulated which I don't really like. If the user has a way to update the ID using a new user though, that wouldn't necessarily be bad.

Sorry for the long comment, but after all of this I'm not entirely sure on what's the best way to handle this. Thanks in advance.

adrien2p commented 7 months ago

Yes you can indeed auto generate a random email, but I would prefer that this part stays in the hand on the consumer to be managed.

piereligio commented 7 months ago

Yes you can indeed auto generate a random email, but I would prefer that this part stays in the hand on the consumer to be managed.

Yeah makes sense. I was thinking of autogenerating it just to make the thing work, and then prompt the user to specify their own one, maybe also from user settings to be changed. In my case emails aren't important (if authentication is managed with steam), because I already customized the front store in such a way that the user can download the content from the website itself, and it's always pretty clear. But yeah it's better if I can also contact users to notify updates and such, so I might still ensure that they insert their email somehow.

adrien2p commented 7 months ago

I believe then that it is a good approach 👍

adrien2p commented 6 months ago

What should we do with this pr guys? since it requires a custom validation anyway, it seems it cant be hold by the auth plugin itself right?

piereligio commented 6 months ago

What should we do with this pr guys? since it requires a custom validation anyway, it seems it cant be hold by the auth plugin itself right?

Yeah about steam like I mentioned on the other issue, I had to make it pretty hacky to make it work. It can be useful as a starting point, but can't be in the plugin in this way, imo. Because of the email thing of course