1273034 - Transition Treeherder login from Persona to login.taskcluster.net

Status: RESOLVED FIXED

(Tree Management :: Treeherder, defect, P2)

Description

[Ryan Kelly [:rfkelly]]

Persona will be decommissioned by the end of 2016, and I'm trying to ensure that all the work we need to do between now and then is captured under the following meta-bug:

  https://bugzilla.mozilla.org/show_bug.cgi?id=1197381

I couldn't find an existing bug for migrating Treeherder away from Persona, so I'm creating one.  If there is an existing bug, please link it under the above meta-bug and close this one out.

Comment 1

[Ryan Kelly [:rfkelly]]

Also, I just realized, I should have linked to this page with more information on migrating away from Persona, which we'll continue to maintain and update over the year as various Mozilla properties make the move:

  https://mana.mozilla.org/wiki/display/Identity/Persona+migration+guide+for+internal+sites

Comment 2

[Ed Morley [:emorley]]

Up until now we've been using bug 1032163 to discuss what to use instead of Persona (since the choice of authentication solution overlaps with the bigger 'how do we sync permissions with LDAP groups' issue for which that bug was originally filed), however I was thinking of breaking out to a clean bug anyway, so this is as good as any :-)

Treeherder's requirements:
* That the login authentication system can be used by both employees and non-employees
* That we have a way to sync internal Treeherder permissions (eg "can they use sheriffing features such as adjusting hidden jobs") to LDAP groups or similar, even though we're about to move to Heroku where querying the behind-vpn Mozilla LDAP server is more problematic.
* That we have a way to interact with Taskcluster on behalf of users, and with their permissions, without having to do hacky things like whitelist *@mozilla.com email addresses for retriggers etc. This is made more complex by the fact that the individual microservices cannot map an email address to permissions/scopes, so we cannot just pass on the verified identity for the user, but must have their accessToken.

The current plan is to use login.taskcluster.net, which:
* supports Okta login (even for non-employees, so long as they've added their LDAP account to mozillians.org apparently) and also Persona login (for which I've filed bug 1273089 to switch to another solution, presumably Firefox Accounts).
* maps LDAP groups to a list of scopes, so we don't need to query LDAP ourselves
* gives us an accessToken which we can use with the taskcluster APIs solving the permissions issue for things like retriggers

This bug will solely be about using login.taskcluster.net as a SSO solution for Treeherder. Later bugs will then (a) add support for using the LDAP-group-derived scopes for Treeherder permissions, and (b) add support for capturing the generated accessToken for use with Taskcluster APIs.

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

If it helps, I have a wip branch for trying to add a taskcluster login: https://github.com/mozilla/treeherder/compare/master...KWierso:taskclusterbutton?expand=1

There's a new button in the navbar that opens up a modal dialog that contains an iframe to login.taskcluster.net when clicked.
When you sign in within the iframe/modal, it redirects back into Treeherder.

At that point, it *should* postMessage the TC credentials back into the parent window and close the modal, but it isn't working at the moment.

Comment 6

[Dustin J. Mitchell [:dustin] (he/him)]

Bug 1267249 shouldn't be necessary to complete this -- it's going to make federated logins easier, but they are already possible.  It will be a while before I finish up that project.

Comment 7

[Ed Morley [:emorley]]

Agreed, I added it more to track UX papercuts for the new login. Once the work here is complete, I may end up moving them to a polish bug instead, depending on if the cumulative papercuts block people wanting to use it instead of Persona (not that Persona is exactly pleasant from a UX point of view anyway).

Comment 8

[Cameron Dawson [:camd]]

I'll take this for now, as Ed's still focusing on follow-up from the Heroku transition.  We may tag-team on this to some degree, however.

Comment 9

[Cameron Dawson [:camd]]

Another great doc about how to implement this:
https://docs.taskcluster.net/manual/3rdparty

Comment 10

[GitHub Autolander Bot]

Attachment: [treeherder] mozilla:taskcluster-auth > mozilla:master

Comment 11

[Cameron Dawson [:camd]]

Comment on attachment 8800802 [details] [review]
[treeherder] mozilla:taskcluster-auth > mozilla:master

Thanks for taking a look, Dustin.  The files you'll likely be most interested in are:

treeherder/auth/backends.py
treeherder/webapp/api/auth.py
ui/js/components/auth.js

I'm still working on getting this to work with the Django admin, so there are more changes coming, but probably not in those files.

Thanks!!

Comment 13

[Ed Morley [:emorley]]

(In reply to Ed Morley [:emorley] from comment #2)
> This bug will solely be about using login.taskcluster.net as a SSO solution
> for Treeherder. Later bugs will then (a) add support for using the
> LDAP-group-derived scopes for Treeherder permissions, and (b) add support
> for capturing the generated accessToken for use with Taskcluster APIs.

Just to elaborate on this point slightly (since it's come up on the PR) - we're deliberately not switching the authorisation parts (bug 1273092) over to TC scopes here, since it isn't on the critical path for Persona EOL and would:
* require more bug assignee time than first meets the eye (see the rough list of tasks to complete in bug 1273092 comment 1)
* increase the complexity of code review/testing (time/mental overhead is very much worse than O(lines_changed) )
* increase the chance we see regressions once deployed, that might require backout

...all of which increase the risk of not meeting the Persona EOL deadline.

I really do feel we should be defaulting to producing MVPs / incremental features, rather than making them the exception / having to justify them.

Comment 14

[Dustin J. Mitchell [:dustin] (he/him)]

The problem with MVPs is that things that aren't products never get done, as they don't qualify as "minimal" for the products they are associated with.

Comment 15

[Ed Morley [:emorley]]

Comment on attachment 8800802 [details] [review]
[treeherder] mozilla:taskcluster-auth > mozilla:master

Looking really really good! Have left some comments and am going to take another look at the UI parts Monday :-)

Comment 16

[Cameron Dawson [:camd]]

Comment on attachment 8800802 [details] [review]
[treeherder] mozilla:taskcluster-auth > mozilla:master

Thanks for all the feedback, Ed.  I did a pass through and addressed everything you mentioned.  I made that the 3rd commit on the PR, so you can diff it, if you like.  I had one question in there you'll see about that ``login`` endpoint host, port stuff.

Thanks!

Comment 17

[Ed Morley [:emorley]]

Comment on attachment 8800802 [details] [review]
[treeherder] mozilla:taskcluster-auth > mozilla:master

Awesome work!

Have left hopefully the last round of significant comments. With those addressed/discussed and the tests fixed, we should be good for a final quick review pass and then onto landing :-)

Comment 18

[William Lachance (:wlach)]

Comment on attachment 8800802 [details] [review]
[treeherder] mozilla:taskcluster-auth > mozilla:master

No red flags I saw in the review of the UI code.

Comment 19

[Cameron Dawson [:camd]]

Comment on attachment 8800802 [details] [review]
[treeherder] mozilla:taskcluster-auth > mozilla:master

OK, Ed.  I've made all the changes per your feedback and our discussions.  I included them all in an extra commit, so they'll be easier to find, if you like.  Thanks again.  :)

I double-checked it on stage and looks to still be working well.

Comment 20

[Ed Morley [:emorley]]

Comment on attachment 8800802 [details] [review]
[treeherder] mozilla:taskcluster-auth > mozilla:master

Looks great! I've left some comments on the PR, but doesn't need further review.

Before we deploy to production, I think we'll want to check on stage that things work for contributors using non-moco email addresses, in particularly whether they correctly get linked to their existing account (that might have is_staff etc), rather than getting a new User with no permissions.

Other than that - we should be good to go :-)

Comment 21

[Treeherder GitHub Bugbot]

Commit pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/ba8165c23934f65df3bbd809f75af94e77a2a258
Bug 1273034 - Transition to Taskcluster Auth from Persona (#1922)

In this commit, Sheriff access is still maintained in the
Treeherder DB, rather than using the scopes derived from
LDAP.

For local usage with Vagrant, this requires accessing
Treeherder with localhost instead of
local.treeherder.mozilla.org

Loggin in to the Django Admin is not enabled in this
branch.  Do use the admin, you must first login through
the normal Treeherder front-end.  Then the admin will
be accessible if the user has the privileges to do so.

Persona login will still be technically possible through the
login.taskcluster.net site.  But that choice will go away
shortly.

Comment 22

[Masatoshi Kimura [:emk]]

The treeherder login link still open the Persona popup. Is this expected?
How can I login treeherder without using Persona, after all?

Comment 23

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

The code has landed, but we need to wait for the Treeherder devs to do a push to production to pick up the changes. I'd expect that sometime next week, most likely.

Comment 24

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Cameron, I've tested the TC login on staging and noticed that I'm not able to login via an external email. I can create such an account and enter the verification code, but then the following message appears:

> Error logging in: Invalid email for clientId: 'email/[e-mail]'. Invalid value for scope 'assume:mozilla-user': 'None'

Comment 25

[Ed Morley [:emorley]]

Which of the TC login options are you using? (Okta, Auth0, Persona)
Afaik the email with need be an LDAP email, but contributors can still log in.

Comment 26

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Auth0 in that case. So as it looks like it will not be possible anymore to sign-in with an email which is not registered by any service provided by Mozilla. :( It means my Gmail address which I'm using for Bugzilla won't work anymore.

Comment 27

[Cameron Dawson [:camd]]

Henrik: I talked to Dustin today and am going to meet with him and Ed tomorrow to talk about options for this.  I think it will be safe for us to use the email in the client id to log in as well.  But that email will likely not be safe for sheriff access.  It will work for retriggering, canceling, etc.  Bug 1319246

Comment 28

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to Cameron Dawson [:camd] from comment #27)
> tomorrow to talk about options for this.  I think it will be safe for us to
> use the email in the client id to log in as well.  But that email will
> likely not be safe for sheriff access.  It will work for retriggering,
> canceling, etc.  Bug 1319246

The email I have used here is a registered and verified email in my Mozillians profile. So why is it not safe?