netlify / gotrue

An SWT based API for managing users and issuing SWT tokens.
https://www.gotrueapi.org
MIT License
3.78k stars 279 forks source link

Question and potentially a proposal about signing in with phone/email when signups with those are disabled #315

Closed TomasHubelbauer closed 2 years ago

TomasHubelbauer commented 2 years ago

Hi, I have a question about a patch I am proposing to the Supabase GoTrue fork: https://github.com/supabase/gotrue/pull/324

The patch makes it possible to sign in with a phone or an email even when phone/email signups have been disabled. This means that existing users utilizing these sign in options would be able to continue to sign and new users would be prevented from signing up should these options be disabled.

My problem is that I implemented this by simply removing existing checks that prevented this in the first place. And now I am wondering whether GoTrue intentionally makes it so that one can't sign in with the phone or email when signups with such method are disabled, or whether maybe those conditions were there as a result of copy-paste and folks agree letting users sign in in this case is okay?

I am worried about introducing a security issue by missing an unintended consequence of removing these checks. I figured the best way to be sure that's not what this would do is to ask at the source.

Please let me know if the change I propose looks good to you. I will be happy to patch this upstream as well if it makes sense to everyone that the change should be the case.

- Do you want to request a feature or report a bug?

I think I am reporting a bug, but not sure if I might be missing something and this is actually an intended feature.

- What is the current behavior?

When disabling email and phone signup, users cannot sign in with an email or phone.

- If the current behavior is a bug, please provide the steps to reproduce.

Disable email and phone auth providers and attempt to sign in with either - you will receive the exception whose throw-site my patch is removing.

See here: supabase/gotrue#330.

- What is the expected behavior?

I think it should be possible to sign in with an email or a phone even if the sign ups are disabled. That's what I want for my single-user Supabase app. Supabase uses a fork of GoTrue and when I manually create a user for myself and then disable all ways to sign up for anyone else, I can't sign in myself any longer.

- Please mention your Go version, and operating system version.

N/A

TomasHubelbauer commented 2 years ago

Apologies, upon further inspection it appears as thought this code was never in this upstream and was only introduce in the fork!