mozilla / discourse

Issues repo for Discourse roadmap within Mozilla
https://github.com/mozilla/discourse/projects/1
5 stars 4 forks source link

Create new account window is displayed when changing secondary email as primary in Github #176

Open rleitan opened 5 years ago

rleitan commented 5 years ago

[Precondition]: Autologin is disabled Github account with 2 emails ( A is primary email) Discourse account created before account linking feature ( log in using Github method - with email "A")

[STR]: 1.Deratcheting rule is enabled

  1. Change secondary email from Github as primary (e.g email B becomes primary email)
  2. Login in discourse using Github username

[Expected result]: Successful login in discourse account already created

[Actual result]: Create new account window is displayed

[Note]: The issue is not reproducible in mozillians staging.

┆Issue is synchronized with this Jira Task

LeoMcA commented 5 years ago

I imagine this is working in mozillians because I expect it's matching on user_id, whereas Discourse matches on email.

We should probably take a hybrid approach where we attempt to match by user_id first, and then if a Discourse user can't be found we match by email. This would require us to keep track of all user_ids a user has though, and would still fail if we've never seen that user_id before in Discourse.

I think attempting user_id and then email is a solution once/if we have auth0 level account linking in place, so the user_id is stable. @gdestuynder is that still happening?

In the meantime, we could make this situation better in some cases by iterating over all secondary emails in a user's IAM profile on login, until we find a Discourse account with one of these, and then logging a user into that account. @gdestuynder how do you feel about any security implications of that?

gdestuynder commented 5 years ago

@LeoMcA yes its still happening note that you should generally try to rely on user_id regardless

the implications of using email are that the IdPs can always change their user_ids and expect that you will respect that. we do a best effort to compensate for when this happens of course (by disabling the user account for example) but your mileage will vary.

ex:

hmitsch commented 5 years ago

@LeoMcA and @gdestuynder here's my opinion: In an ideal world, (all) RPs should use user_id as "user name" on their end.

Most ParSys Properties (Discourse, Reps Portal, etc), currently use email as "user name".

Over time we should change this behaviour and migrate to user_id. If this can be done "easy" in Discourse, let's do it!

Thoughts?

-Henrik

gdestuynder commented 5 years ago

"yes" :)

hmitsch commented 5 years ago

Cool! Let's wait for @LeoMcA's Discourse impact analysis.

hmitsch commented 5 years ago

I just spoke to @LeoMcA, this is what we landed on:

Because there are currently other priorities, we put this scope item into the Icebox.

LeoMcA commented 5 years ago

A note to my future self: we'd also need to remove IAM user_ids when users are anonymised when implementing this to ensure users don't get logged into their anonymised account if they decide to return to Discourse.

gdestuynder commented 5 years ago

note, starting from cis v2 you can also use uuid which is a unique user_id as well, but opaque

rugk commented 5 years ago

BTW the title of this issue can be simplified: If your change your mail on GitHub, you cannot login again to Discourse with that account. (I think the GitHub's notification mail is considered the primary one.) It does not matter in which way you change your mail…

Also possibly consider to use the public mail instead of the primary one. (because the primary one is hidden, while the public mail in your profile may be okay to use)


Anyway, far worse is that the "other [opposite] way" also works: When you change your Discourse mail, you cannot login via GitHub to Discourse anymore. Reported at https://github.com/mozilla/discourse/issues/191