Email change despite not confirming it


(Leo McArdle) #1

We’ve found a bit of a bizarre bug in our authentication provider, which seems to affect (at least one) core authentication provider, and I expect more.

Here's the crux of the problem:
  1. Create a new account
  2. Change the email address on that account
  3. Don’t click on the link in the email
  4. Log out, log in with the old email
  5. Observe the email address on that account is now the new email, despite you having not verified it

This change of email address despite the user not confirming it will then result in additional bugs depending on the authentication provider in question:

With the one we use in Mozilla, after the steps shown in the video above, a user will no longer be able to log in with their old email, and will have to start logging in with their new one.

With Google, the user will still be able to log in with their old email (because a GoogleUserInfo row still exists for it) and will never be able to log in with their new one because a duplicate user_id error will be thrown for the new GoogleUserInfo row that Discourse tries to create.

That problem can be seen in a spec I wrote, which passes when it really shouldn’t:

updater = EmailUpdater.new(user.guardian, user)
updater.change_to(new_email)

user.reload
expect(user.email).to eq(old_email)

response = login(old_identity)
expect(response['authenticated']).to eq(true)

user.reload
expect(user.email).to eq(new_email)

expect { login(new_identity) }.to raise_error(ActiveRecord::RecordNotUnique)

I’m starting to attempt to find a fix, but I thought I’d post now that I’ve finally nailed down the actual bug, in case anyone else had any ideas.


(Leo McArdle) #3

@zogstrip whoops, I assume from your unlisting that this has security implications. I assumed since this requires access to the account in the first place, there were none - but I guess I was wrong.

I’ve been eating lunch so I’m afraid that this point I’m no closer to finding a solution.


(Régis Hanol) #4

Not sure yet. Was merely a precaution.

EDIT: @eviltrout just fixed a related bug. Note the "should_validate_email = true")


(Leo McArdle) #5

This is the problem:

EDIT: and here’s the fix: FIX: don’t activate un-confirmed email on omniauth authentication by LeoMcA · Pull Request #5176 · discourse/discourse · GitHub


(Jeff Atwood) #7

This topic was automatically closed after 4 days. New replies are no longer allowed.