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

Bug: attempting to change email to one that already exists in the database #622

Open
ivan-rivera opened this issue May 4, 2021 · 6 comments

Comments

@ivan-rivera
Copy link

I've found what appears to be a bug.

I'm using Pow in a Phoenix API and I have an endpoint to update user details such as password and email address. I'm using Pow's email confirmation extension so when a user requests to update their email, Pow first updates the unconfirmed_email fields and sends out an email to the user to confirm the change. Let's say I have 2 accounts in my DB, "account1@mail.com" and "account2@mail.com". If I log in as "account1@mail.com" and try to update my email to "account2@mail.com", Pow will successfully set "account2@mail.com" as unconfirmed_email and will send a mail there. If I use the sent token, however, that's when I hit an error that this email is already taken. I'm not sure whether this has been considered but I think it's worthwhile fixing.

@MMore
Copy link

MMore commented May 10, 2021

@ivan-rivera
Copy link
Author

Hey @ivan-rivera,

it's probably covered here: https://github.com/danschultzer/pow/blob/master/lib/extensions/email_confirmation/README.md

Thanks, there is indeed a relevant section here:

When users updates their e-mail, it won't be changed until the user has confirmed the new e-mail by clicking the e-mail confirmation link. The confirmation will fail if the :email is already in use for another account. If PowInvitation is used then the same logic applies when a user accepts an invitation changing their e-mail address in the process.

However, it still feels like a bug to me. Just like when you try to create a new account using an email that has already been registered, you'd expect to get an error "this email has already been taken", I would also expect something similar in this situation. I've implemented an easy workaround inside my API, but perhaps it might be a good idea to fix this upsteam at Pow level.

@MMore
Copy link

MMore commented May 10, 2021

I feel you about this, I was also confused when I noticed that. I believe the reason for this behaviour is a side effect of preventing user enumeration which is also stated there.

@Sija
Copy link

Sija commented Jul 5, 2021

@MMore But this approach doesn't prevent the enumeration, it just moves it into the later (confirmation) stage 😕

@Schultzer
Copy link
Contributor

@MMore But this approach doesn't prevent the enumeration, it just moves it into the later (confirmation) stage 😕

FYI I believe you would need to have access to the email accounts or somehow manage to steal the tokens, for that form of user enumeration attack, where the other wouldn't require tokens at all.

@Sija
Copy link

Sija commented Jul 9, 2021

It makes it a bit more complicated, yes, but just that. Auto generating emails is not a problem these days :)

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

No branches or pull requests

4 participants