1017085 - We need a new signin workflow for GitHub logins

Status: RESOLVED FIXED

(developer.mozilla.org Graveyard :: Sign-in, enhancement)

Description

[Māris Fogels [:mars] (please needinfo)]

Bootstrapped from an email discussion about changes to our signin workflow to support github logins.


Take a look at page 4, item D. If we are not capturing email address via github signin, the check-box for opting into newsletter won't be enough. We'll need to ask for email address in the sign-in flow.

http://cl.ly/2b3H0F3a3x2S

Comment 1

[Māris Fogels [:mars] (please needinfo)]

Hey Holly!

I owe you the list of questions and gotchas I'm seeing with this setup.  I'm also going to CC Janis since he's working on the back-end and may be able to dispel my concerns or raise new ones. Yay concerns!

1.  Account sync'ing.  So someone already has a persona ID, signs up with GitHub as well, wants to sync.  What can we do here, Jannis?  Do we draw a hard line and say that users absolutely cannot do that?  I recall Les having concerns over trusting email addresses and info from GitHub.

2.  Jannis:  can we allow email changes if they sign up via GitHub?

3.  Can we remove the "Before signing in with Github, do you already have an MDN account using Persona?" step?  Or only use it during the first month or so?  I feel like it's one more prompt/barrier to quickly signing in.

Everything else looks good to me, but I think we need to iron out the issues above before we move to far forward.

Comment 2

[Māris Fogels [:mars] (please needinfo)]

On 14-05-28 10:07 AM, Holly Habstritt Gaal wrote:
> Hey guys, 
>
> When walking through the flows with David last week, I noticed something. We were going to ask the user if they had an MDN account *before* they sign in with Github. This would be annoying to ask somebody every time, but we do need this information at some point, so I recommend adding this step to after they authenticate with Github. This way (barring we can capture the Github email) we will only have to prompt them the first time authenticating rather than twice. 
>
> In David's #3 about removing the step below... we should not remove it, since we need to know if the user has an existing account to avoid duplicates, but *move* this step and only ask the first time the user signs in with Github. (see flow where it is moved to *after* authenticating with Github.
>
>
> Can we find a time to meet in person next week? I'm on PTO this week. The doc at this link is best talked through together. 
>
>
> http://cl.ly/2b3H0F3a3x2S

Comment 3

[Luke Crouch [:groovecoder]]

+1 putting the "Have MDN Account?" step in just the first-time path. The "Have MDN Account?" step is optional if/when the OAuth response from GitHub contains a verified email address that matches an existing MDN account. In that case, we can automatically attach their GitHub to their existing MDN account.

:jezdez - do we get verified email addresses back from GitHub via django-allauth?

If the user's MDN account email doesn't match their GitHub email, we have to ask them if they have an existing account. But in that case, we need to kick them *out* of the entire "Sign in with GitHub" flow, and into a "Sign in with Persona to connect your GitHub account" flow. The UX and technical challenge of putting a "Sign in with Persona" experience *inside* a "Sign in with GitHub" experience seems really big. :(

Comment 4

[Jannis Leidel [:jezdez]]

:groovecoder With an extension of the included Github OAuth2 provider in allauth (https://github.com/pennersr/django-allauth/blob/master/allauth/socialaccount/providers/github/provider.py) we can query the Github email API (https://developer.github.com/v3/users/emails/) additionally to the automatically returned profile data which only contains the publicly available profile email address, not the optionally verified account email address.

Comment 5

[Luke Crouch [:groovecoder]]

+shobson to bring into the "Have MDN Account?" UX discussion.

Comment 6

[Holly Habstritt Gaal [:Habber]]

Attachment: 19June2014_MDN_signin.pdf

I updated the Github sign-in flow per our conversation on Monday. I removed the Persona sign-in step when a verified email is a match on MDN and added the Unverified email use case.


Please check the Unverified flow to confirm if I'm correct. I'm also unsure exactly where in MDN's UI we should show the following type of notification: “You signed in with an unverified email. Please verify on Github.” (with a link that sends the user back to Github to verify after which we send them through the appropriate flow on MDN)

We need a standard way of displaying notifications like this on MDN. Thoughts? Inline, modal, white horizontal banner above tabzilla?

Comment 7

[Luke Crouch [:groovecoder]]

After "Verify on GitHub" the user will have to manually go back to the "Sign in with GitHub" beginning - we have no way to automatically redirect them back into the flow.

If their verified email is *not* a match and they have no MDN account, we should make sure to send them back to the MDN page from which they first clicked "Sign in with GitHub" after they create their MDN profile.

And just to be clear again, the "Create Profile Page" should be a really simple experience. (See https://bugzilla.mozilla.org/show_bug.cgi?id=962556#c7)

Comment 8

[Jannis Leidel [:jezdez]]

So looking at the actual prod data we have a few problems:

1. 5995 of 73597 users don't have any email address assigned to them, they have all signed up before 2011-08-02

This implies that we basically never ever going to allow those users to pick up their username again, which is a problem since we're basically allowing a big chunk of our user base to be zombies. There is little to do, except to let them be. I wanted to raise this issue though anyways to clarify that we have to substract those people when talking about our user base.

2. 362 email addresses are used by 2 user accounts, which means we currently don't make sure that email addresses are unique

This problem is more serious as it implies that we don't check for duplicates currently when someone signs in with Persona. I'm trying to understand how this could have happened, any feedback from you would be appreciated.

Could it be that the duplicate users signed up with the pre-Persona system and then again with Persona using a different email address, and later changing the email address to the one of the pre-Persona account?

How should we handle those users when migrating over to allauth? allauth by default assumes unqiue email addresses. This feature can be disabled in plain allauth, but with the currently discussed workflow to check if the email address returned from Github matches one of the email address in our databases we'd have to decide for those 262 email address which user account to hook it up.

Any help would be appreciated..

Comment 9

[Luke Crouch [:groovecoder]]

1. Yup, they are users who never signed in during our 8-month MindTouch-Django user migration window. About once a week we get a bug like this one asking us to set an email address for an account: https://bugzilla.mozilla.org/show_bug.cgi?id=1028441

So, we'll have to continue helping those users manually.

2. MindTouch allowed multiple user accounts with the same email address. We have another bug about this one: https://bugzilla.mozilla.org/show_bug.cgi?id=945279

Ali, Janet, should we send a pro-active email to those 262 users with duplicate accounts asking them which one they want to keep?

Comment 10

[Jannis Leidel [:jezdez]]

1. Okay, this is important information as it implies a new workflow once we have allauth as it tracks email addresses outside the user table in an own table.

2. An alternative would be to automatically bless one of the multiple email accounts which has been logged into last?

Comment 11

[Janet Swisher]

Yes, an email to those addresses seems like a good idea. Should we give them the option of setting a different email address for their secondary account? I'm not sure what the user flow would be for that.

We should also decide on a default policy for those affected users who don't reply.

Comment 12

[Jannis Leidel [:jezdez]]

Thanks Luke for the quick feedback, BTW. Ali, Janet, it would really be useful to get feedback for this rather sooner than later as it currently blocks me in my work for integrating allauth/Github login.

I'm not so convinced that emailing users would result it a dramatically high response rate, with the sad side effect of us having to decide for them which account to use and/or drop them anyway. Depending on whether those affected users have previously used their account to edit wiki pages or upload demos they may actually lose access to their data because of that.

So to continue the alternative I mentioned in the previous message, I could also see those profiles to be marked for special treatment when they login again next. We could basically ask them to decide which of the user accounts using the same email address (after they've proven it's their email address with Persona) should be dropped or merged.

Comment 13

[Jannis Leidel [:jezdez]]

Oh, thanks Janet, that was quick!

I'm not sure if allowing to set a different email address to their secondary account is enough as we don't know their motivation for having two accounts with the same address or whether it's just a technical issue in the past that caused it.

I'm totally on board with regard to having a default policy, from a technical perspective I'd rather not have to maintain a subset of accounts that are dead by default.

Comment 14

[Luke Crouch [:groovecoder]]

Okay, so how about this?

For missing emails, we update our manual workflow to fix accounts in the allauth_* tables

For duplicate emails, we email all affected users with a notice:

  * Their 'primary' account, as defined by most recent sign-in
  * Their 'secondary' account
  * A link to file a bug in the "Login" component of MDN if they want to change the email address of their (secondary) account(s)

For code that detects duplicate accounts by email address, we always use the account with the most recent sign-in.

Comment 15

[Jannis Leidel [:jezdez]]

(In reply to Luke Crouch [:groovecoder] from comment #14)
> Okay, so how about this?
> 
> For missing emails, we update our manual workflow to fix accounts in the
> allauth_* tables

Makes sense, I can show you how to do that under the new system or even document the steps required to do it.

> For duplicate emails, we email all affected users with a notice:
> 
>   * Their 'primary' account, as defined by most recent sign-in
>   * Their 'secondary' account
>   * A link to file a bug in the "Login" component of MDN if they want to
> change the email address of their (secondary) account(s)
> 
> For code that detects duplicate accounts by email address, we always use the
> account with the most recent sign-in.

Works for me, I'll work on this as part of the next step to move to allauth.

Comment 16

[Luke Crouch [:groovecoder]]

FWIW, I created a mzl.la short link for filing the bug:

http://mzl.la/mdn_change_email_bug

Comment 17

[Stephanie Hobson [:shobson]]

Am I right to assume that we don't have a way for users to combine accounts? Is that why no one has suggested letting users merge their duplicate accounts?

Comment 18

[Jannis Leidel [:jezdez]]

:shobson As far as I know we don't have that ability right now. It would indeed be the best option for users and depending on the number of domain objects (e.g. demos, wiki documents, wiki revisions etc) this could be not that hard™ I would need to check for sure how many things a user can "own" to be certain.

Comment 19

[Justin Crawford [:hoosteeno] [:jcrawford]]

In today's dev meeting and an earlier email thread, :davidwalsh asked for a sign-off on the wireframes in comment 6. I'll review and update this bug.

Comment 20

[Justin Crawford [:hoosteeno] [:jcrawford]]

There was some chatter in IRC about :jezdez's comments above (starting around comment 8), and this one was clarifying:

--
[10:43:30]  <@jezdez>	 hoosteeno: groovecoder: we have a new situation where we have some invalid data in our user tables that needs to be figured out. users have used their email adresses multiple times. I was trying to suggest that this isn't a blocker for github to launch if we provide the ability to do this manually via a bug form. later we can even work on a feature that would allow users to chose which user account to make their primary, or even eventually
--

Comment 21

[Justin Crawford [:hoosteeno] [:jcrawford]]

(In reply to Holly Habstritt Gaal [:Habber] from comment #6)
> We need a standard way of displaying notifications like this on MDN.
> Thoughts? Inline, modal, white horizontal banner above tabzilla?

:habber, can you file a bug for that one?

(In reply to Luke Crouch [:groovecoder] from comment #16)
> FWIW, I created a mzl.la short link for filing the bug:
> 
> http://mzl.la/mdn_change_email_bug

I think this is satisfactory for the near term in order to unblock us to launch this feature. To be clear, responding to these bugs will require a backend engineer. We need to keep a close eye on the scale of demand for this.

(In reply to Justin Crawford [:hoosteeno] from comment #19)
> In today's dev meeting and an earlier email thread, :davidwalsh asked for a
> sign-off on the wireframes in comment 6. I'll review and update this bug.

Holly and I walked through the wires and flows today and I think they're good. Thanks :habber!

:davidwalsh, there are a few unanswered questions that I think we need to tease out with real interactions. In particular...
* How dramatic should the messaging be on the "verified email, no match" case in order to minimize the number of people who create duplicate accounts?
* How can we best lead people through the "unverified email" scenario?

I don't think we can answer these in flowcharts. I think we have to answer them with interactions. Could we demo an implementation of this that leaves room for us to adjust around those two questions?

Other questions:
* What assets still need to be created for this? Final copy? Any visual designs?
* How can we explore the effectiveness of the flows before final launch?

Comment 22

[Holly Habstritt Gaal [:Habber]]

(In reply to Justin Crawford [:hoosteeno] from comment #21)
> (In reply to Holly Habstritt Gaal [:Habber] from comment #6)
> > We need a standard way of displaying notifications like this on MDN.
> > Thoughts? Inline, modal, white horizontal banner above tabzilla?
> 
> :habber, can you file a bug for that one?
> 
> (In reply to Luke Crouch [:groovecoder] from comment #16)
> > FWIW, I created a mzl.la short link for filing the bug:
> > 
> > http://mzl.la/mdn_change_email_bug
> 
> I think this is satisfactory for the near term in order to unblock us to
> launch this feature. To be clear, responding to these bugs will require a
> backend engineer. We need to keep a close eye on the scale of demand for
> this.
> 
> (In reply to Justin Crawford [:hoosteeno] from comment #19)
> > In today's dev meeting and an earlier email thread, :davidwalsh asked for a
> > sign-off on the wireframes in comment 6. I'll review and update this bug.
> 
> Holly and I walked through the wires and flows today and I think they're
> good. Thanks :habber!
> 
> :davidwalsh, there are a few unanswered questions that I think we need to
> tease out with real interactions. In particular...
> * How dramatic should the messaging be on the "verified email, no match"
> case in order to minimize the number of people who create duplicate accounts?
> * How can we best lead people through the "unverified email" scenario?
> 
> I don't think we can answer these in flowcharts. I think we have to answer
> them with interactions. Could we demo an implementation of this that leaves
> room for us to adjust around those two questions?
> 
> Other questions:
> * What assets still need to be created for this? Final copy? Any visual
> designs?
> * How can we explore the effectiveness of the flows before final launch?

Bug for notification style:
https://bugzilla.mozilla.org/show_bug.cgi?id=1031547

I'm open to help with any testing once the flow exists on a demo server or behind a waffle flag.

Comment 23

[Stephanie Hobson [:shobson]]

Attachment: mergeprofiles_140703.jpg

If we are able to offer the ability to merge accounts I think the link should be on the edit profile page. I've attached a mockup of how it could go underneath the save changes button.

Comment 24

[Stephanie Hobson [:shobson]]

Attachment: profilelinks_140703.jpg

I've attached mockups of changes we'd need to make to the Edit Profile page considering the use cases of:
- user has not provided any information for other site
- user has provided link a has not connected site for sign in
- user has connected site to sign in
- how this would look on mobile
- the addition of Persona as a listed profile if the user has connected multiple services for sign in

Comment 25

[Luke Crouch [:groovecoder]]

The initial implementation is done and we are iterating under https://bugzilla.mozilla.org/showdependencytree.cgi?id=1046775 and filing new bugs for un-implemented features.