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

Feature/add steam passport #120

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

stephane-segning
Copy link
Contributor

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

Copy link

vercel bot commented Nov 28, 2023

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
Copy link
Owner

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
Copy link
Contributor Author

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
Copy link
Owner

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
Copy link
Contributor Author

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

@adrien2p
Copy link
Owner

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
Copy link
Contributor Author

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

@adrien2p
Copy link
Owner

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
Copy link

piereligio commented Nov 30, 2023

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
Copy link
Owner

adrien2p commented Dec 1, 2023

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
Copy link

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
Copy link
Owner

adrien2p commented Dec 1, 2023

I believe then that it is a good approach 👍

@adrien2p
Copy link
Owner

adrien2p commented Jan 4, 2024

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
Copy link

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

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

Successfully merging this pull request may close these issues.

None yet

3 participants