supabase / auth

A JWT based API for managing users and issuing JWT tokens
https://supabase.com/docs/guides/auth
MIT License
1.41k stars 346 forks source link

auth.signUp() doesn't error for existing accounts - security vulnerability #1517

Open CalebLovell opened 2 years ago

CalebLovell commented 2 years ago

Bug report

Describe the bug

supabase.auth.signUp() is not erroring for existing accounts. Right now, you can submit an existing email with any incorrect password, and supabase will return you the account metadata (without a jwt).

To Reproduce

Go to this example app Sign-up with an email and a password Log out Try to sign up again with the same email using any password you want. Try asdfasdfasdf if you want! You will get an alert saying you logged in, but you won't get a working access token. Just the email you submitted. You can also view the request in the Network tab of the Dev Tools and see metadata about the account, like when it was created and what provider it uses.

Expected behavior

Attempting to sign up with an existing email should throw an error.

System information

awalias commented 2 years ago

hey @CalebLovell this is actually a fix for a previous security issue.

Previously the interface was leaking information by allowing an attacker to see whether a given email had an account or not. Now the endpoint returns a "success" response regardless of whether an account exists already or not.

The metadata you see in the response is actually faux info - the user ID is random and the datetimes are set to the time that the request was made.

note: this is only the case for supabase instances where AUTOCONFIRM is disabled (as per the default) for accounts who have enabled AUTOCONFIRM (where accounts don't require email confirmation) the behavior is the same as before (error on duplicate accounts)

hope this helps!

CalebLovell commented 2 years ago

Ahh, very interesting. Would be great to maybe add that to the docs somewhere, because I suspect I won't be the only one confused by it. Firebase returns an error in this situation so that is what I was expecting, but I like the faux info return as an even better solution! Thanks for the response and enjoy your weekend.

naegelin commented 2 years ago

This 'security fix' seems like 'security through obscurity' . IMHO it doesn't make sense for supabase to be opinionated about how a signup process should be handled by developers. There are many use cases where a back-end service may need to know if a user already exists and having to store an additional user profile table just to be able to figure this out seems to be an unnecessary extra step. Perhaps the 'service key' responses can be accurate while the 'app key' responds with a generic 'invalid credentials'?

brunobely commented 2 years ago

In the case where a user forgets they are already signed up with their email address, most websites will show some variation of “there’s already an account for this email address”.

If a user tries to sign up with an email that already has an account associated with it, how can I tell so I can let the user know?

silentworks commented 2 years ago

I also agree that security through obscurity is not a good way to fix this as most services online do tell you if a user already exists, what they do have however is a rate limit on how many logins you can try within a certain time period to prevent brute force attacks. This fix should probably be reverted as the behaviour is unexpected and it seems to be confusing users more than anything.

serranoarevalo commented 2 years ago

@awalias so if the user is trying to create an account, when they already have one, is there a way to let me know?

ChronSyn commented 2 years ago

I also agree that security through obscurity is not a good way to fix this as most services online do tell you if a user already exists, what they do have however is a rate limit on how many logins you can try within a certain time period to prevent brute force attacks. This fix should probably be reverted as the behaviour is unexpected and it seems to be confusing users more than anything.

Correct. Rate limited logins would be the correct process for this, but it's also difficult to defend against some forms of attack unless they are targeting one specific account, in which case locking that account from logging in for a duration or until an account owner performs some verification process.

If an attacker is aiming to test as many accounts as possible, then the options are either cookies (easily defeated) or IP blocking (also easily defeated). There's some hybrid options out there such as behaviour analysis, but they have their own set of limitations and potential for false positives.

Rate-limiting registration isn't feasible to introduce as an attacker could easily bombard the system with fake registrations and block legitimate users from registering. For ecommerce sites, this would potentially make it a haven for scalpers. At the same time, it becomes difficult for developers when they're unsure if an account exists during registration.

What most online services have is a generic truthy response during the account recovery process. For example, instead of saying "An email was sent to the account we found", they'll instead say "If an account with this email exists, we have sent an email to it". I believe rate limiting per account already exists for this in Supabase.

It's a difficult problem to solve because there's potential for hackers to use the sign up process to ascertain if an account exists (as opposed to testing login), but developers want to know if an account exists when a user is registering.

naegelin commented 2 years ago

Every security control can be defeated. Supabase shouldn't be responsible for solving every edge case of security for app developers and many companies have regulatory guidance on how to implement authentication flows. There should be a straight forward api available for server to server calls so that developers can decide what the proper pattern would be for themselves based on their own needs. The current Supabase position is to create an associated "user profile" table inside supabase in order to recreate missing functionality (such as basic checks to see if a user exists). The more of these types of work arounds a developer needs to create, the less compelling supabase becomes over say a vanilla postgres + hasura instance. I would also argue that for B2B applications where access is not available to the general public many of the concerns around account harvesting, fake registration etc are much less of an issue.

brunobely commented 2 years ago

@awalias so if the user is trying to create an account, when they already have one, is there a way to let me know?

Building off of this, I think the crux of this issue and what we want to know is what should an average auth flow look like, implementation-wise, when built with Supabase? The only guide available in the official documentation uses magic links "for simplicity" but doesn't really touch on how to handle any error or edge-case scenarios, which is extremely important in production.

I'd just like to know how to do it properly, whichever the security measures taken are.

awalias commented 2 years ago

Hey everyone thanks for your comments!

To be clear this is not an example of security through obscurity, the red herring here might be the presence of faux info or the fact that we haven't done a good job of documenting this behavior. As long as we don't leak any info about whether a user account exists or not on any public endpoint, then the information is secure (at least as far as this specific issue is concerned).

The correct solution here is to take this one step further: If a user tries to sign up who already has an existing account, we still return a "check your inbox for confirmation email" message, but we instead send them an account recovery email (which is actually just a magic link) then the developer can decide whether to direct them to a password reset page or just allow them to go about their session as normal.

This solves for:

An important reason for why we decided to be "secure by default" on issues like this, is partly because of the number of users adopting our auth service. I could fairly easily take your email address, make a sign-up request to every known supabase endpoint on the internet, building up a map of all the various services that you are/are-not subscribed to. Which is quite a large privacy concern.

I propose that we do several things:

As always any help on these ones would be hugely appreciated. And thanks again for the active discussion and helping improve the product 🙏.

awalias commented 2 years ago

@naegelin I also agree with your idea that the admin endpoints should return truthful responses, if you are creating an account from the backend. We recently added auth.api.createUser I'm not sure what the behavior of this method is for existing accounts.

edit: it looks like this already works as expected - again the issue here is lack of documentation

{
  data: null,
  error: {
    message: 'Email address already registered by another user',
    status: 422
  }
}
akkie commented 2 years ago

@awalias Is it still true that the signUp function will not throw an error if the user was already registered? I ask because I get currently the error 'User already registered' if I register with an already existing email address.

My config says: enable_signup = true double_confirm_changes = true enable_confirmations = true

kevinmlong commented 2 years ago

@awalias - There still seems to be a mismatch here that I'm trying to work through. My account is set-up to require email confirmations.

Based on what you mentioned above, it would seem that a confirmation/recovery email should be sent in both case and in the UI, one can simply direct the user to check their email. That said, I'm not sure how to handle the case where a user already signed up using that email without them getting confused on the what the password is. In the case of a confirmed user getting an account recovery email, their mental model would be the password they just entered, which would be wrong if it's different than the one actually associated with the account.

irreal commented 1 year ago

We are hitting the exact same issue. Registered users with verified email addresses can forget they already signed up, they go through the sign up process, we show the message to check their emails, but no email arrives.

Any solutions?

malewis5 commented 1 year ago

Is there any update to this? We need a clear solution for letting users know if they've already signed up, like how @awalias mentioned. I can see many users getting stuck and waiting for a sign up email that will never come.

awalias commented 1 year ago

hey! Was just discussing this issue with @hf in depth - his opinion is that since it's still possible for people to determine the existence of accounts with side-channel attacks (e.g. time how long it takes to get a response, if an email is sent it will take longer etc.) that by default we should return an error similar to how firebase does it. e.g. "This email already exists".

Then the idea is to make this functionality optional to people who require more privacy controls. There is some draft documentation on the issue here: https://github.com/supabase/supabase/blob/f8682bbb095d728ee19a581170a5d972a39543e4/apps/reference/docs/guides/auth/auth-security.mdx#user-information

In the meantime I will check if we can revert the default behavior to always give an error for existing accounts

renardeinside commented 1 year ago

Adding a bit more color to this issue. The message Check your email for the confirmation link. pops up if a signUp email exists in the users table, but is added from a different provider, e.g. GitHub. This is kind of misleading (from one side), but also secure from another side.

I believe this behavior should be documented to avoid others losing a couple of hours trying to debug it.

farhanhaider1 commented 1 year ago

Is there any update to this? We need a clear solution for letting users know if they've already signed up, like how @awalias mentioned. I can see many users getting stuck and waiting for a sign up email that will never come.

exactly this issue!

user72356 commented 1 year ago

@awalias Any updates on this issue? The UX is very poor without a way of letting them know that they already have an account. And if I have to pre-vet the user, then that's a poor DX.

From https://github.com/supabase/supabase/blob/f8682bbb095d728ee19a581170a5d972a39543e4/apps/reference/docs/guides/auth/auth-security.mdx#user-information

"information about whether an account exists in the system can leak even if the application returns an ambigouous message such as "If you have an account an email has been sent." This is because the final part of the request handling will need to send out an email message or SMS using a third-party system. This makes requests naturally last longer when an account exists."

I don't understand this- couldn't this email or SMS process happen asynchronously so that you can return the response immediately in all cases?

Alternatively, your proposed fix of sending it whether there is an account or not means it will always take the same (long) time.

hf commented 1 year ago

@awalias Any updates on this issue? The UX is very poor without a way of letting them know that they already have an account. And if I have to pre-vet the user, then that's a poor DX.

Hey no updates on this yet. I understand the pain the community and customers are feeling with this, but we do require some more time to figure out how to properly approach this. We can't keep flipping between "more secure" and "less secure" messages.

From https://github.com/supabase/supabase/blob/f8682bbb095d728ee19a581170a5d972a39543e4/apps/reference/docs/guides/auth/auth-security.mdx#user-information

"information about whether an account exists in the system can leak even if the application returns an ambigouous message such as "If you have an account an email has been sent." This is because the final part of the request handling will need to send out an email message or SMS using a third-party system. This makes requests naturally last longer when an account exists."

I don't understand this- couldn't this email or SMS process happen asynchronously so that you can return the response immediately in all cases?

Adding the "send message" instruction to the asynchronous queue is a branch that can fairly easily be detected too. (This is because a non-existing user will never reach that branch making slight timing differences apparent with not-so-sophisticated statistical methods.)

Masking this sort of stuff is never really going to work well and consistently, and there will always be some edge case that can't be masked for maintainability, performance or cost reasons. This is why our thinking is moving in the direction of having a message that explains what the user's doing wrong instead of trying to mask it.

But as I mentioned, we need a bit more time to figure out how to support both types of use cases. Expect some progress for this in the next couple of months.

Alternatively, your proposed fix of sending it whether there is an account or not means it will always take the same (long) time.

This is true, but as explained above it's practically impossible to mask this.

hf commented 1 year ago

Adding some docs here to make it more apparent: https://github.com/supabase/gotrue-js/pull/567

hf commented 1 year ago

Some alternatives to consider while we work out a good solution, which could reduce user friction.

  1. Consider adding a "remember me" feature. If a user has logged out, to prevent them from being confused on the same browser store some information in local storage that you can use to give them a "welcome back" screen indicating the correct sign-in method they can use.
  2. If masking is less important for your application you can create an edge function which you can use to indicate to your users what type of account they have and what may be going on. You can use the information in the auth.users and auth.identities tables to derive some conclusions in those cases.
user72356 commented 1 year ago

@hf thank you for taking the time to answer. I trust that you will find the best solution. I figured I would share my opinion/preference, while taking the "attacker" angle into account.

Ideally, if the e-mail address is already registered, there are 2 cases:

  1. The user has already confirmed their email address. We show the "Check your email for the confirmation link." message for security purpose (so an attacker can't tell that there is an account under this email address), but we actually send the user an e-mail to tell him that he's already got an account.

  2. The user has not yet confirmed their email address. We show the generic check your email message again and we send the user the confirmation email again.

Presently, if a user is already registered, we show the check your email message but the user receives no email whatsoever.

salemshah commented 1 year ago

You can use this method until Supabase solves this problem

        const {error, data} = await supabase.auth.signUp({email, password})
        if(data?.user?.identities?.length === 0){
              alert("This user already exists")
         }
arshamg commented 1 year ago

@salemshah Thanks for the temp solution. Is there any way to also grab the provider using this method? It seems as if it doesn't correctly return the provider for the existing user.

The reason I would like to grab the provider is if the user tries to sign up with an email address that has already been previously used with e.g. Google Provider/OAuth. Ideally, I'd like to tell the user to use the Google login link or even redirect them directly.

ruan-brius commented 1 year ago

You can use this method until Supabase solves this problem

        const {error, data} = await supabase.auth.signUp({email, password})
        if(data?.user?.identities?.length === 0){
              alert("This user already exists")
         }

This method only works for accounts which have been registered with OAuth. Accounts that have been registered with password and email still returns identity for invalid users. (At least on Flutter)

user72356 commented 1 year ago

You can use this method until Supabase solves this problem

        const {error, data} = await supabase.auth.signUp({email, password})
        if(data?.user?.identities?.length === 0){
              alert("This user already exists")
         }

This method only works for accounts which have been registered with OAuth. Accounts that have been registered with password and email still returns identity for invalid users. (At least on Flutter)

Must be something with the Flutter version of the lib, or something else wrong on your end. Works great for me in JS (email/password auth). I'm guessing we're relying on an undocumented, implementation detail for it to work that way, but what can ya do...

ruan-brius commented 1 year ago

You can use this method until Supabase solves this problem

        const {error, data} = await supabase.auth.signUp({email, password})
        if(data?.user?.identities?.length === 0){
              alert("This user already exists")
         }

This method only works for accounts which have been registered with OAuth. Accounts that have been registered with password and email still returns identity for invalid users. (At least on Flutter)

Must be something with the Flutter version of the lib, or something else wrong on your end. Works great for me in JS (email/password auth). I'm guessing we're relying on an undocumented, implementation detail for it to work that way, but what can ya do...

Yeah. Disabling email verification solved my problem, anyways.

anarkrypto commented 1 year ago

It makes sense to maintain privacy, but it doesn't make sense not to email existing users.

Let's assume that my user registered with Github.

If he loses access to github or wants to create a password, he will be waiting for the email forever and will think that not receiving it is an error.

This is very confusing. User should be able to confirm their new form of authentication via email

hugonteifeh commented 1 year ago

Complete protection against user enumeration seems hard to achieve with email-based logins. This is why I consider username-based logins to be an easy and practical solution to this problem. It'd be much simpler to ask users to choose a username/login that doesn't identify them, than to ask them to create an anonymous email/phone number and come back and use this "anonymous" email.

Users would still need to provide their emails/SMS upon registration(to support OTP, magic links, etc...) but it'd only ask for the username upon signing in. Yes, this means that supabase would still need to have some mechanism to prevent leakage of the existence of an email if the system is to have a functionality like "Forget username? send it to email..." but I imagine that protection against such a leakage would be easy to implement by always responding in a constant time.

I noticed that someone asked for this feature https://github.com/supabase/gotrue/issues/903 a while ago but it was closed. I think the supabase team should reconsider this feature.

Privacy is a superhigh priority and should always be approached with an opt-out model rather than an opt-in model.

kvetoslavnovak commented 1 year ago

I have just discovered this behavior today and found this issue, really surprised and agree that This 'security fix' seems like 'security through obscurity'.

ghost commented 1 year ago

I've just opened this issue https://github.com/supabase/gotrue-js/issues/769, which is related to the discussion going on here.

devhandler commented 1 year ago

it would be really helpful if you can let developers decide how to handle this instead of forcing a solution. it's always a balance between security and convenience and nothing is absolutely secure. each service has different needs and developers can choose what works the best for their use cases

arshamg commented 1 year ago

In case this helps anyone, we don't verify emails in our dev/staging environment but we do verify emails in production. Due to this, we have a signUp function that looks like:

export const signUp = async (email: string, password: string) => {
  const { data, error } = await supabase.auth.signUp({
    email,
    password,
  });

  // in staging, we don't verify primary emails
  // Supabase returns a nice error
  if (error?.message === "User already registered")
    return { data, error: "Please sign in with your existing account" };

  if (error) {
    logger(error.message, "error");
    throw new Error(error.message);
  }

  // in production, we verify primary emails
  // supabase returns a user object with no identities if the user exists
  if (data?.user?.identities?.length === 0) {
    return { data, error: "Please sign in with your existing account" };
  }

  return { data, error };
};
wmonecke commented 1 year ago

How is this hack:

const {error, data} = await supabase.auth.signUp({email, password})
if(data?.user?.identities?.length === 0){
    alert("This user already exists")
}

still the acceptable solution? A hacker could also read this thread and exploit the hack to see if the user exists. This means that the "security" argument is not valid.

It's almost over half a year and this issue seems stale.

devhandler commented 1 year ago

@awalias @kiwicopple hope supabase leave the freedom to developers to decide how to handle existing accounts instead of dictating a solution that is really not helpful for a lot of use cases

Saranoja commented 11 months ago

Hey guys, here's an interesting finding → it appears that if you're running signUp from the server side / from a serverless function (using the service key), the returned error actually contains the message "User already registered" as opposed to running is anonymously - case in which, yes, a fake user is returned. So a reliable alternative would be to just create a custom Edge function and have that invoked. Then the function can handle the scenario however you want (send an email, forward the error etc.).

nmerchant commented 11 months ago

@Saranoja thank you!! That's super helpful to know.

aftermidnightsolutions commented 8 months ago

Keep it simple...check for an error coming back from signUp.

If there was an error, something went wrong (password not right length, etc.) so display that.

if there was no error, 1 of 2 things happened.

1) user already existed 2) user was created

If you to check the length of the return to figure out which, ok that's fine.

I chose to just return a generic message either way...something like:

"New users require authentication. Please check your email. If you are already a user and forgot your password, please reset your password form the login form"

With that approach, you don't tip your hat as to if the user existed or not, and you tip an existing user to try reseting their password if they forgot.

I suppose on condition 1 above, where the user already exists, if you get a zero length result, you could also programmatically initiate an email indicating to the user someone tried signing up using their email address. While supabase may not do this for you, there is no stoping you from doing it in the code based on the return.

TomasSestak commented 7 months ago

It's been more than two years 👀. It happens all the time that users forget that they registered before...

danieldietzel commented 7 months ago

Same problem, this is an incredibly goofy decision to not return a basic error. At least make it configurable to turn the error on and off.

aftermidnightsolutions commented 7 months ago

Same problem, this is an incredibly goofy decision to not return a basic error. At least make it configurable to turn the error on and off.

I'm seeing it does return an error. When using the following:

const { error } = await supabase.auth.signUp({ email, password })

error.message will contain "user already registered" as the error message, if the user already exists.

842u commented 7 months ago

Adding to the complexity, when email confirmation is enabled, using a service won't throw an error for an existing confirmed email. However, if you self-host supabase manually or via CLI, it will throw an error stating 'User already registered".

SEI-John commented 5 months ago

@awalias, would it be possible to get an update on where supabase stands on this issue? The behaviour is a little strange to say the least, and it seems like there isn't a firm stance ~3 years after this has been first reported. I love the package so far, but there's the odd bug/strange behaviour here and there that would be great to get resolved. Hacking around this by checking the identities array isn't optimal.

kiptoomm commented 3 months ago

As mentioned here: https://github.com/supabase/auth-js/issues/513#issuecomment-1826233042, it seems to me that the latest docs have clarified the approach:

If signUp() is called for an existing confirmed user: When both Confirm email and Confirm phone (even when phone provider is disabled) are enabled in your project, an obfuscated/fake user object is returned. When either Confirm email or Confirm phone (even when phone provider is disabled) is disabled, the error message, User already registered is returned.

It's still not an optimal solution, but at least we can get an explicit error message by disabling the email confirmation. The error response looks like:

{"code":422,"error_code":"user_already_exists","msg":"User already registered"}

RowinVanAmsterdam commented 3 months ago

I do not get the magic link in the mailbox when I try to sign up with an already existing user? I only get the fake user data object in return.

Email service is working, signup emails and sending magic link manually works without a problem.

appurist commented 3 months ago

I propose that we do several things:

As always any help on these ones would be hugely appreciated. And thanks again for the active discussion and helping improve the product 🙏.

2.5 years later it looks like:

I have yet to find any guidance from Supabase on how to handle the user experience and UI flow for when a user attempts a duplicate signup. The example code just tells them to check for the confirmation email (which is an incorrect UI that misleads the user).

So the official answer here is to create an edge function that uses the admin API to find out if a user exists and then expose that to the anon client for improved UX handling? If we're going to expose that anyway, what's the point of hiding it in an edge function and using the admin API?

I've been huge on Supabase for a long time now but the handling of this mess is almost enough to make me switch to Firebase where they actually return an error for an error.

hf commented 2 months ago

I've been huge on Supabase for a long time now but the handling of this mess is almost enough to make me switch to Firebase where they actually return an error for an error.

Sorry to hear that. It's hard for us to decide anything more concretely on this point since some Supabase users expect this behavior, while others like you expect the other one.

We keep discussing this, but it's such a hard choice to make for people. Until we are able to devote more time to figure out how to make this "anonymization" configurable, the behavior remains the same.

appurist commented 2 months ago

@hf I understand the strong desire to avoid providing any means for hackers to identify valid email addresses but the resulting UX is horrible and this is a policy that should be determined by each organizations using Supabase, not Supabase itself (globally). The result is a nasty and problematic onboarding experience. I was in your shoes arguing for tighter control until I became responsible for user creation and management at coindesk.com where I had to be more pragmatic because of this was the #1 reason for new users to abandon the onboarding process (and resulting Support nightmare).

We keep discussing this, but it's such a hard choice to make for people.

What I don't understand is why this cannot simply be an option on the email or general Auth settings, that organizations can choose the behavior of.

(I want to use Supabase, but I am now actively working on moving my projects to Firebase due to the onboarding user experience forced on us by Supabase.)

hf commented 2 months ago

What I don't understand is why this cannot simply be an option on the email or general Auth settings, that organizations can choose the behavior of.

It can be an option, but it needs to be built securely and for us to be able to maintain it properly. I understand your predicament and past experience, but in this case it's not just one web property that we're securing but tons. Some want this, some want the other thing.

If we commit to adding an option now, we have to commit to maintain the option until end-of-time. This is not something we're prepared to do at this stage, but is definitely at the forefront of the team's mind. Until we identify a way to do this properly we are not going to jump into quick fixes.