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

auth.signIn() with magic link creates users in Auth.users table #1176

Closed
suilillo opened this issue Apr 16, 2021 · 43 comments · Fixed by supabase/auth-js#199 or supabase/auth#448
Closed
Labels
documentation Improvements or additions to documentation help wanted Extra attention is needed

Comments

@suilillo
Copy link

Bug report

Describe the bug

Using magic link feature to login creates the user if the email provided doesn't exist on Auth.users table.

To Reproduce

  1. Use this snippet to login with magic lik

const { user, session, error } = await supabase.auth.signIn({ email: 'example@email.com' })

  1. Use an email that doesn't exist on Auth.users table
  2. Check Auth.users table

Expected behavior

If the email provided is not in Auth.users table it shouldn't be created and an error should be thrown.

@suilillo suilillo added the bug Something isn't working label Apr 16, 2021
@edgarasben
Copy link

I think this could be an option. Some apps might want a magic sign-up/log-in while others might only want a magic log-in.

@kiwicopple
Copy link
Member

If the email provided is not in Auth.users table it shouldn't be created and an error should be thrown.

Yeah, as @edgarasben points out, there are plenty of apps that use "magic signin" - i.e. the user never creates a password. So this is expected behaviour. I'll keep this open for a few days if there are comments or suggested improvements

@suilillo
Copy link
Author

suilillo commented Apr 20, 2021

I would suggest to explain this in documentation to avoid confusion.

Thanks!

@kiwicopple kiwicopple added documentation Improvements or additions to documentation help wanted Extra attention is needed and removed bug Something isn't working labels Apr 20, 2021
@edgarasben
Copy link

I think there could be an option here.

If you see the way magic.link works, they have both - log in and sign up pages.

image

If you input email that does not exist yet, you get an error:
image

@pichichi91
Copy link

Just a spontanous thought - but how about defining the behaviour of the magic link based on, if signup is enabled/disabled in the backend?

  • if signup is disabled: The admin probably wants to create/invite user on their own, so when an unknown email is entered the request could be ignored
  • if signup is enabled: The admin probably wants to let users create their account on their own so when an unknown email is entered it could be created in the backend

I think there could be an option here.
If you input email that does not exist yet, you get an error:
image

This looks nice, but maybe on a security perspective, you don't want to show the message that the account couldn't be found - so attackers could somehow pinpoint which emails have already been registered?. Personally, I'd just display a message like "An email has been sent to your inbox"

@edgarasben
Copy link

edgarasben commented May 17, 2021

Disabling the registration and inviting people manually as you say could be a nice option too. (Log in only).

I was talking more about having some way to separate sign up and log in, while still relying on magic link tech technique. I expect some people might be confused about what's happening with signin only. Am I creating an account? Am I logging in?

But good point about the security. You could add "If your account exists, an email will be sent to your inbox".

@nicholaschiang
Copy link
Contributor

The current behavior mimics the behavior of Firebase Authentication which has both seamless sign-up (when a user doesn't exist) and login via email links. From the Firebase Authentication docs (see also this magic link login/signup guide):

Unlike password reset and email verification, the email used does not necessarily need to belong to an existing user, as this operation can be used to sign up new users into your app via email link.

I think that Supabase should continue to mimic that ☝️ behavior to make it easier and more intuitive for users (like me) who are migrating from Firebase to Supabase.

I do agree with @pichichi91 that manual invites might be a common enough use-case. But I think a better solution (instead of changing the current signup/login behavior of the magic link) would be a "test mode" similar to Google OAuth2 where you have to manually specify "test user email addresses" that are allowed to login or signup at all:

Screenshot 2021-05-26 9 57 01 AM

@rsdavis
Copy link

rsdavis commented Jun 21, 2021

Since this is left open for opinions, I'll add mine.

I think it's awesome that magic links has streamlined the signup/login paths into a single flow. It is also in-line with how the OAuth providers work. It's much cleaner and simpler.

I can see how some users might be a bit perplexed, but if there is only one "sign in" page, seems like they will get it soon enough :)

Also, I noticed that using a magic link for a new user signup will send the "confirmation" email template, while using a magic link for an existing user will send the "magic link" template. So, you can customize these email templates to help the user understand whether they have created an account or just logged in.

@CR1AT0RS
Copy link

I think we can just add an option called "Disable SignUps" with magic link. I personally like the automatic magic link signup.

@oliviermtl
Copy link

Automatic signup with magic link is nice, but it should only be available via supabase.auth.signUp()
If there is a typo in the email entered by the user when signin in, he still receives a confirmation message, but will not receive the magic link since it has been sent to an invalid email.

@richcorbs
Copy link

Just a spontanous thought - but how about defining the behaviour of the magic link based on, if signup is enabled/disabled in the backend?

I like the idea but in practice how would we ever get new users in the system (auth.users) if there wasn't a way to sign them up?

@richcorbs
Copy link

richcorbs commented Sep 2, 2021

What about an argument like createMissingUser passed to the magic link flow like:

supabase.auth.signIn({ email: 'me@email.com', createMissingUser: false })

That's still clumsy. Maybe a setting would be best.

@i-pip
Copy link
Contributor

i-pip commented Sep 3, 2021

What about an argument like createMissingUser passed to the magic link flow like:

supabase.auth.signIn({ email: 'me@email.com', createMissingUser: false })

That's still clumsy. Maybe a setting would be best.

building on @richcorbs' suggestion, this could also go like

supabase.auth.signIn({email: 'me@email.com', signUp: false }) 

I can create a pull request for this in gotrue and gotrue-js

@ulisses-cruz
Copy link

I'm having the same issue.

But in my case, I want users to only signUp with email and password. No magic link.
But if a user leaves the password empty when signing In it receives a magic link in his email.

An option to disable magic link signIn would solve my problem.

@silentworks
Copy link
Contributor

silentworks commented Oct 20, 2021

Maybe magic link sign up/sign in should be explicitly stated like how we do with social auth.

supabase.auth.signIn({ email: 'me@example.com', provider: 'magiclink' })

Of course this would be a breaking change, but I guess creating a new major version of the libraries could solve this.

@i-pip
Copy link
Contributor

i-pip commented Oct 24, 2021

since the magic link issue applies to both signIn and signUp, how about maybe adding a magicLink boolean to both methods (signIn & signUp)

for signIn:

supabase.auth.signIn({email: 'me@example.com', magicLink: false})

for signUp:

supabase.auth.signUp({email: 'me@example.com', password: 'my-super-secret-pwd', magicLink: false})

This has the advantage of not breaking current behaviour but still allows you to turn off magic links

@ulisses-cruz
Copy link

since the magic link issue applies to both signIn and signUp,

I believe the problem is only in signIn. More specifically, in this code:

https://github.com/supabase/gotrue-js/blob/82f4bca4843f62e02da61d8217adb5b4350a1856/src/GoTrueClient.ts#L195-L200

@kristjanmar
Copy link

I also think this is strange. My website has a paid subscription offering and there is no free tier, so it makes no sense for people who are not customers to be able to sign up and log in.

However, now people are able to create accounts by entering their email into my magic link login form. That doesn't make sense for my use case, which I believe is pretty common.

The signIn function should just sign users in, not create accounts. For that we have signUp already.

Adding a setting in the dashboard or in the signIn function to modify this functionality would be great. There are some good suggestions above.

@bard
Copy link
Contributor

bard commented Nov 16, 2021

The way I dealt with that was adding an "authorized" field to the profiles table, and modifying security policies accordingly. Initially, it felt like a hack, but after some thinking, I believe it's conceptually correct.

A local user/password table is a direct means of authentication; a Google or GitHub account is a delegated (via OAuth) means of authentication; an email account is another delegated (via magiclink) means of authentication. In principle, all of them only answer the question, "who's this user?", and not the question "what is this user allowed to do?"

Yes, there's often the implied rule that, if the user can authenticate against the local user/password table, it also means "this user is authorized". But the moment you add social login to the mix, you run into a problem similar to what we're talking about here: users can authenticate regardless of what's in your user/password table (because you've delegated identity), and you can't block them from doing so (because you don't know the user's id the identity provider tells you). So "authenticated" no longer implies "has an account" no longer implies "is authorized".

Similar cases could be made when you need to deactivate accounts but cannot delete them (because of data consistency, compliance, etc): existence of account is distinct from authorization of the account owner.

Now, if we could replace the overloaded signIn() term with authenticate(), and signUp() with something like createLocalAccount()...

@kiwicopple
Copy link
Member

I'd need to double check the workflow, but I'm pretty sure no user will be created if you disable signups in the Dashboard under Auth > Settings > Disable signups

image

After that, you can manually create new users using the createUser() method. Created users should be able to login, and if they haven't been created they will be blocked

@kristjanmar
Copy link

I'd need to double check the workflow, but I'm pretty sure no user will be created if you disable signups in the Dashboard under Auth > Settings > Disable signups

Would this also disable the auth.signUp()?

@kiwicopple
Copy link
Member

Would this also disable the auth.signUp()?

yes it would, which should be expected right?

the reason why we create users on magic link signIn() is because otherwise a developer would need to implement one of these:

  1. initial signup with email + password (to create the account), subsequent signups with magic link. This probably defeats the point of magic link
  2. initial signIn() with magic link returns an error, then the developer catches the error and calls signUp() with magic link. In this case the end result is the same as calling signIn()

To address some of the comments:

If there is a typo in the email entered by the user when signin in, he still receives a confirmation message, but will not receive the magic link since it has been sent to an invalid email.

That's the same as the password functionality - if you call signUp with username and password you'll also receive a success confirmation. Even if we switched the magic link to signUp() only it would have this issue.

building on @richcorbs' suggestion, this could also go like

supabase.auth.signIn({email: 'me@email.com', signUp: false }) 

This seems like a viable option since there is backwards compatibility. I'd probably rename the param to be a bit clearer which side-effect it's preventing. eg: disableSignUp: true

I'll keep this open a bit for feedback so that we can decide what the community feels is best

@kristjanmar
Copy link

This seems like a viable option since there is backwards compatibility. I'd probably rename the param to be a bit clearer which side-effect it's preventing. eg: disableSignUp: true

I'll keep this open a bit for feedback so that we can decide what the community feels is best

Thanks for clarifying, and +1 for "disableSignUp: true"

@devdev-dev
Copy link

This seems like a viable option since there is backwards compatibility. I'd probably rename the param to be a bit clearer which side-effect it's preventing. eg: disableSignUp: true

I'll keep this open a bit for feedback so that we can decide what the community feels is best

I would appreciate this extension to the interface to better control the sign up process. Greate suggestion :)

@aantn
Copy link

aantn commented Dec 22, 2021

I would also like the disableSignUp: true option. Any ETA on this?

@songololo
Copy link

Another vote for adding a disableSignUp flag so that the sign up logic can be kept distinct from log in.

Sign up flows may require users to accept terms and conditions, privacy policies, or provide additional information such as names, companies, etc.

For me, this is pretty confusing behaviour for handling the user experience and complicates the front end logic.

@kiwicopple
Copy link
Member

Added the chore to our Auth server here: supabase/auth#311

@kangmingtay
Copy link
Member

kangmingtay commented Dec 29, 2021

Hey everyone, we have started working on adding this to gotrue and would like to get some feedback on this issue (supabase/auth#318 (comment)). There are a few possible solutions being thrown around right now:

Problem: Including a no_signup flag to decide whether or not a user should be automatically signed up can be exploited if used on the client-side.

  1. Instead of using a flag, we add it as an internal config in gotrue which can be set via the "Auth -> Settings" page on the dashboard.
  2. Create another set of /magiclink and /otp endpoints behind the admin endpoint /admin/magiclink and /admin/otp which would require using the service_role key so that these can only be invoked from the server-side. IMO this isn't ideal because JAM stack users will not be make use of this.

Currently, I'm in favour of going with (1) but I am open to other suggestions too :)

Decided to go with (2), but renaming the flag to create_user so the intention is clearer. From the security perspective, i think it should be fine because ultimately, the user still needs to have access to the email provided in order to click on the link.

cc @dthyresson

@kristjanmar
Copy link

Currently, I'm in favour of going with (1) but I am open to other suggestions too :)

Agreed, #1 seems like a good choice.

@kristjanmar
Copy link

kristjanmar commented Jan 26, 2022

I wanted to implement this but I am getting a typescript error when I try to add the "create_user" flag to my signIn function:

"Argument of type '{ email: string; create_user: boolean; }' is not assignable to parameter of type 'UserCredentials'.
Object literal may only specify known properties, and 'create_user' does not exist in type 'UserCredentials'.ts(2345)"

Here's my code:

async function signIn(email: string) {
  const { error } = await supabase.auth.signIn({ email, create_user: false })
  return { error }
}

@kangmingtay
Copy link
Member

hey @kristjanmar, we haven't added it to gotrue-js yet but will be looking to do so soon!

hope this new param helps everyone on this thread to build better magiclink login flows 💪🏻

@kristjanmar
Copy link

Thanks @kangmingtay -- I managed to get rid of the type error by adding // @ts-ignore above the line.

However, the "create_user: false" flag is not working. When I type an email that is not signed up in the login form, it creates the user in Supabase.

I already updated to the latest version of supabase-js and tested this several times. The user is always created, just like before.

@kangmingtay
Copy link
Member

yeah this is because the gotrue version that your supabase project is currently on is not updated to the latest one yet, we'll be rolling out the updates to all existing projects soon!

@kristjanmar
Copy link

That makes sense, thanks for clarifying.

@kristjanmar
Copy link

I see that supabase/gotrue-js has been updated. Does this mean that the create_user: false flag should work?

@kristjanmar
Copy link

yeah this is because the gotrue version that your supabase project is currently on is not updated to the latest one yet, we'll be rolling out the updates to all existing projects soon!

Any updates on this?

@mirkonasato
Copy link

Creating the user is fine in my case, but how can we tell if it's a new user and therefore the magic link has type=signup, or if it's an existing user and the link has type=magiclink?

The email message says to click the link or "Alternatively, enter the code", so I guess I also need to show an input box for the user to manually enter the code if they so choose, calling GET /verify that requires the right type.

@kangmingtay
Copy link
Member

hey @mirkonasato,

Creating the user is fine in my case, but how can we tell if it's a new user and therefore the magic link has type=signup, or if it's an existing user and the link has type=magiclink?

Good point, I've created a PR to address this

The email message says to click the link or "Alternatively, enter the code", so I guess I also need to show an input box for the user to manually enter the code if they so choose, calling GET /verify that requires the right type.

Hmm let me check on this. We recently merged a PR that would send both the confirmation link and token to the user's email. But you can also modify the email template to only send the email link.

@mirkonasato
Copy link

Thanks @kangmingtay. It is possible to modify the magic link template email so it's not a big deal. I could also call generateLink() and send the email myself. From what I can see generateLink returns an object that will have an email_confirmed_at field only if the user already exists.

@kristjanmar
Copy link

This is still not working, unfortunately. The user gets added when typing an email into the login form even when the flag is activated.

async function signIn(email: string) {
	const { error } = await supabase.auth.signIn({
		email,
		shouldCreateUser: false
	})
	return { error }
}

I've tried shouldCreateUser, create_user and noSignup as the flag, since it is unclear which one it's supposed to be. But none of them work.

@chrisb2244
Copy link

@kristjanmar I think you need to put the shouldCreateUser in the second parameter, that is:

const { user, error, session } = await supabase.auth.signIn({email}, {shouldCreateUser: false})

@ChronSyn
Copy link
Contributor

I've just been trying this with phone login, and unfortunately, when set to true, this returns the error 'signups not allowed for otp' even for an account which already exists (and preventing the login from proceeding):

supabase.auth.signIn(
  { phone: `+${phone number for existing account}` },
  { shouldCreateUser: false },
)

If I set it to shouldCreateUser to true and make no other changes, the login process continues as normal. Unfortunately, this also means that accounts are still created (if they don't exist) when using signIn.

This is when using the library "@supabase/supabase-js": "^1.33.3".

@kangmingtay
Copy link
Member

Hey @ChronSyn, thanks for reporting this with the repro steps. I'll take a look at this soon!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation help wanted Extra attention is needed
Projects
None yet