1395356 - Use auth0 for TH login, and get TC credentials from there

Status: RESOLVED FIXED

(Tree Management :: Treeherder, enhancement, P2)

Description

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

This is the culmination of work to stop using Okta, and should also fix some of the issues around user identification and credentials expiration in Treeherder.

I'll try to make the patch for this, but I'll likely need some help.

Okta, and with it the current federated login support, is going away in October, so there's a time limit here.

Comment 1

[Ed Morley [:emorley]]

If it helps, the Treeherder UI parts are here:
https://github.com/mozilla/treeherder/blob/edba0fb429d0a96d2f76cad9be1c65c83dfdb4f6/ui/partials/main/thGlobalTopNavPanel.html#L97
https://github.com/mozilla/treeherder/blob/edba0fb429d0a96d2f76cad9be1c65c83dfdb4f6/ui/partials/main/login.html
https://github.com/mozilla/treeherder/blob/edba0fb429d0a96d2f76cad9be1c65c83dfdb4f6/ui/js/treeherder_app.js#L43-L45
https://github.com/mozilla/treeherder/blob/edba0fb429d0a96d2f76cad9be1c65c83dfdb4f6/ui/js/components/auth.js

And then the UI callback handler hits the API defined here:
https://github.com/mozilla/treeherder/blob/edba0fb429d0a96d2f76cad9be1c65c83dfdb4f6/treeherder/webapp/api/auth.py

Which uses a custom Django authentication backend:
https://github.com/mozilla/treeherder/blob/edba0fb429d0a96d2f76cad9be1c65c83dfdb4f6/treeherder/auth/backends.py

For more on the Django parts, see:
https://docs.djangoproject.com/en/1.11/topics/auth/customizing/#writing-an-authentication-backend

PS: Was this bug meant to be in a Treeherder component?

Comment 2

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

I thought dumping it in your component would be a little rude, since time is short.  I know we talked about this months ago, but I've struggled to get the auth0/taskcluster bits in place and only now looped back to TH support.

Thanks for the pointers!!

Comment 3

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

Ed, could you request an Auth0 client for this?  That way it won't be tied to me :)

https://mozilla.service-now.com/sp?id=sc_cat_item&sys_id=1e9746c20f76aa0087591d2be1050ecb

The technology you want is OIDC.  Callback URL can be something currently unused like /oidc-login (let me know what you choose).  I think you'll only need to allow LDAP logins (but it's up to you -- passwordless would let anyone login, although they will have few TC scopes).

That may take a few days, but it can happen in parallel with the work to implement it.  It's probably easiest to just put the resulting clientId and clientSecret directly into the TH config somewhere.

Comment 4

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

https://mozilla-django-oidc.readthedocs.io/en/stable/installation.html#quick-start recommends /oidc/callback as the callback URL.

Comment 5

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

I've made some PRs to mozilla-django-oidc to add minor funtionality that will be necessary:
 https://github.com/mozilla/mozilla-django-oidc/pull/166
 https://github.com/mozilla/mozilla-django-oidc/pull/167

I think, roughly, that the implementation should look like this:

 - use mozilla-django-oidc to support logins with Auth0 (you shouldn't need any auth0-specific code). This is totally standard Auth0 / OIDC stuff.

 - follow https://docs.taskcluster.net/reference/integrations/taskcluster-login/docs/getting-user-creds:
   - set OIDC_RP_AUDIENCE to `login.taskcluster.net`
   - set OIDC_RP_SCOPES to `openid email full-user-credentials`

 - communicate the access_token to the frontend.  I'm a little vague on how to do this, but OIDC_STORE_ACCESS_TOKEN will cause mozilla-django-oidc to put the value in the Django session, so that's a start.  I think if you're careful with CORS, you could make it available via an API (`get_user_access_token`?) call from the frontend.

 - in the frontend, when credentials are required for a Taskcluster API call, follow the instructions in the doc linked above:
---
To get credentials, call the oidcCredentials endpoint with provider mozilla-auth0. Pass the access_token from Auth0 in the Authorization header as described in the API documentation.

Note that the returned credentials may or may not contain a certificate field. Be sure that any code handling credentials is compatible with either result. As always, callers should not interpret the resulting credentials in any way, although displaying the clientId to the user is acceptable.
---

We may soon have a Taskcluster client library that can support this automatically, if you give it the access_token.

Comment 6

[Ed Morley [:emorley]]

(In reply to Dustin J. Mitchell [:dustin] from comment #3)
> Ed, could you request an Auth0 client for this?  That way it won't be tied to me :)

Good idea - I'm happy to do this once I have a better idea of which options to pick :-)

Some questions/thoughts:

1) Is both Okta and email sign-in going away, or just the former? I'm presuming both (and login.tc.net will disappear too)

2) If both, I'm presuming email login will still be possible via Auth0, like happens for pulseguardian etc? (That's the "passwordless" option in comment 3 right?)

3) Currently Treeherder uses the Taskcluster clientID as for the username field in Django. From the mozilla-django-oidc docs [1] I see it defaults to a hash of the email but we can override this. Can the email from Auth0 be trusted (unlike the client ID)?

4) Since the backend requires an Auth0 client, what workflow can contributors use to work on Treeherder locally? Disabling auth entirely / ...?

5) From a brief inspection of the mozilla-django-oidc source, I believe it's only doing client side refresh redirects, rather than supplementing them with server side re-verifications like happens in lua-resty-openidc [2] (used by OrangeFactor). For improved UX it seems like it would be better to do both? (The feature would only be able to be enabled when OIDC_STORE_ACCESS_TOKEN is True of course)

6) For capturing/storing the access_token client-side, currently Treeherder does this on the initial callback which hits the static UI rather than a Django view, avoiding the need for a custom API endpoint to pass the token back to the client. To do something similar for mozilla-django-oidc, we'd either need them to add a JSON view or else perform an additional redirect from the static UI to the existing mozilla-django-oidc HTML Django view.

7) The current mozilla-django-oidc refresh implementation looks problematic given our decoupled UI and API (we don't use Django views for most of our UI), since the WhiteNoise middleware that serves the static assets is meant to be as high up in the django middleware order as possible, to reduce time to serve, however their RefreshIDToken middleware would need to be above this (which isn't great from a performance point of view). And if it's above it, it looks like it will trigger for even say JS assets, since they are not caught by is_ajax() [3].

8) Given (6) and (7) it seems like we really need them to fix this: https://github.com/mozilla/mozilla-django-oidc/issues/133




[1] https://mozilla-django-oidc.readthedocs.io/en/stable/installation.html#changing-how-django-users-are-created
[2] https://github.com/pingidentity/lua-resty-openidc
[3] https://github.com/mozilla/mozilla-django-oidc/blob/29e0c8e10891949270e9b12e8aa95aaf1a68c59b/mozilla_django_oidc/middleware.py#L63-L83

Comment 7

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

We chatted by voice.  Roughly:

1/2) yes, passwordless is the option there -- no change in behavior for users.

3) yes, the email can be trusted if profile says it was validated.  For the login methods we use, this will always be true. The `user_id` field from the profile is the actual unique identifier (stable over email changes in github, for example).

4) development workflow is TBD - we are working on that for taskcluster-tools, too.

5/7) we'll need to work with the IAM team to figure out how refreshes should work (I'll get going on that)

6) either of those solutions sound reasonable.

In fact, you could implement the auth0 client as an SPA and do all of the work on the frontend.  Then just pass the id_token to the backend in an API call to identify the user.  That would mean no mozilla-django-oidc at all (although you would probably need a few Python libraries to validate that id_token).

Comment 8

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

From https://wiki.mozilla.org/Security/Guidelines/OpenID_connect:
---
Refresh token:

Avoid using or storing refresh tokens. This is especially important for relying parties (RP) which are websites (as opposed to mobile apps for example, which may not always have network access). Refresh tokens never expire and thus are very powerful. These are usually not needed for an authentication flow, though they may be needed for specific authorization flows. 
---

so, there's that!  No need to worry about refresh tokens :)

The lifetimes of both id_tokens (for the treeherder client) and access_tokens (for the TC API) can be controlled in Auth0, so we can set them to whatever the IAM team and Treeherder team agree on (in fact, I think we can set the access_token expiration to something very large, but I'll work that out on a separate bug).

The more I think about it, the more I like the idea in the last paragraph of comment 7: write this as a SPA, and pass the id_token off to the backend whenever it changes as a way to set the Django user (so all the Django side has to do is validate the id_token jwt and get the corresponding user's email address).  The Django user session expiration should be set to match the id_token's exp claim.

For expiration, then, the frontend can monitor the expiration of both tokens (whichever expires sooner) and change the UI to indicate a logout has occurred when it expires.  A more advanced option would be to load the login page in a hidden iframe with prompt=none periodically, only "logging out" if that fails -- but maybe that's overkill.

Comment 9

[Ed Morley [:emorley]]

In bug 1395357 comment 3 it says that Okta is being disabled on Oct 1st, which is much sooner than we were aware. As such this bug might be quite tight timing for us given existing commitments and the uncertainty over exact approach / having to break new ground on libraries/tools/documentation (especially given some of the pieces we need, eg the JS taskcluster library function to handle auto-refreshing the tasckluster token doesn't yet exist afaik). 

However it looks like bug 1395574 means that login.taskcluster.net will have the ability to support Auth0 logins after Okta is switched off, which would give us a bit more time to get the UX right here (given we're changing quite a few pieces).

Therefore I think we might be best to finish up this quarter's existing work and target this for Q4 instead - at which point there will be more prior art (eg bug 1395358) for inspiration too.

Comment 10

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

Yeah, I'm bummed to find out that "sometime in October" turned out to be "October 1", too, and I'm sorry I couldn't get things to move fast enough before mid-August to give you more lead-time.  It wasn't until late August that I knew this approach would even work :(

I suspect bug 1395574 will have some UI papercuts, and we will be replacing that URL with a redirect sometime in October to support migration of other users, so this is a way to limp a few weeks into October (principally since Rok is on leave until then), but nothing more.

Comment 11

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

Hassan is going to work on this, based on the pointers in comment 1.

In subsequent conversations, I think we've discovered that the plan in comment 5 is not a good one -- instead, we would build a "single-page app" client where the auth0 work is all handled in the browser, and only send the id_token to the Django backend, to an API similar to treeherder/webapp/api/auth.py.

(In reply to Dustin J. Mitchell [:dustin] from comment #3)
> Ed, could you request an Auth0 client for this?  That way it won't be tied
> to me :)

Ed, did you request this?

Comment 12

[Ed Morley [:emorley]]

I haven't requested one yet. Looking at the form I see lots of questions to which I don't know the answer (eg do we need different credentials for dev/stage/prod? how does this work for a client side app? or are these just for the server side callback?) etc.

Comment 13

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

https://docs.taskcluster.net/manual/using/integration/frontend#creating-a-simple-login-integration has some details.  Yes for both stage and prod.

Comment 14

[Hassan Ali (:hassan)]

:emorley, any additional questions from the request form which you're not totally sure about?

Comment 15

[Ed Morley [:emorley]]

Ah I was a bit confused but now I've found https://auth0.com/docs/clients/client-types#confidential-vs-public-clients and https://auth0.com/docs/api-auth/grant/implicit and re-read comment 8 it all makes a bit more sense. Sadly this area is one where I only ever just touch the surface enough to have a basic understanding, but not to fully remember it the next time it comes up. 

Questions about the client request form (these aren't mentioned on the TC docs page as far as I can see?):
* For "What is your redirect/callback URL?" -- How strongly are the callback URLs tied to the client? eg can I use the dev instance client ID for localhost too?
  -> Ah seems that we have to explicitly whitelist localhost: https://mana.mozilla.org/wiki/display/SECURITY/SSO+Request+Form#SSORequestForm-Iwouldliketotestlocallyonmyworkstation,howdoIdothat?
* For "Do you need specific restrictions on whom can login?" -- I presume we want to pick "Anyone can login (I'll take care of access control myself if needed)" ?
* For "What is your logout URL?" -- shall I leave that blank for now? (it's optional on the form)
* For "Which login options would you like to support?" -- the options vary between "LDAP only" all the way through to "LDAP + Github + Google + Passwordless". I'm presuming we definitely don't want passwordless, but I don't know about Google/GitHub?
* Given that for the SPA client type there is no secret to go with the client ID, do we still need separate dev/stage/prod client IDs? (multiple callback URLs can be whitelisted per client ID) It's just that otherwise it will get a bit messy for dev vs stage vs prod webpack builds having to hardcode different client IDs - and will break certain workflows (eg building the UI locally and pointing at prod API).

Comment 16

[:Eli Perelman]

(In reply to Ed Morley [:emorley] from comment #15)
> Ah I was a bit confused but now I've found
> https://auth0.com/docs/clients/client-types#confidential-vs-public-clients
> and https://auth0.com/docs/api-auth/grant/implicit and re-read comment 8 it
> all makes a bit more sense. Sadly this area is one where I only ever just
> touch the surface enough to have a basic understanding, but not to fully
> remember it the next time it comes up. 
> 
> Questions about the client request form (these aren't mentioned on the TC
> docs page as far as I can see?):
> * For "What is your redirect/callback URL?" -- How strongly are the callback
> URLs tied to the client? eg can I use the dev instance client ID for
> localhost too?
>   -> Ah seems that we have to explicitly whitelist localhost:
> https://mana.mozilla.org/wiki/display/SECURITY/
> SSO+Request+Form#SSORequestForm-Iwouldliketotestlocallyonmyworkstation,
> howdoIdothat?

For bz-dev-stats we set the localhost values for the client during development, and override them in Heroku env for production:

https://github.com/eliperelman/bz-dev-stats/blob/master/.neutrinorc.js#L2-L9

You'll want to request 2 OIDC clients, one for development which includes localhost and a path for the callback, and one for production which includes any deployment URLs with the same path for callback.

> * For "Do you need specific restrictions on whom can login?" -- I presume we
> want to pick "Anyone can login (I'll take care of access control myself if
> needed)" ?

This is probably correct.

> * For "What is your logout URL?" -- shall I leave that blank for now? (it's
> optional on the form)

Yeah, you probably don't need this either.

> * For "Which login options would you like to support?" -- the options vary
> between "LDAP only" all the way through to "LDAP + Github + Google +
> Passwordless". I'm presuming we definitely don't want passwordless, but I
> don't know about Google/GitHub?

Dustin, can you advise here?

> * Given that for the SPA client type there is no secret to go with the
> client ID, do we still need separate dev/stage/prod client IDs? (multiple
> callback URLs can be whitelisted per client ID) It's just that otherwise it
> will get a bit messy for dev vs stage vs prod webpack builds having to
> hardcode different client IDs - and will break certain workflows (eg
> building the UI locally and pointing at prod API).

This is a good point. I still think you should separate development and production at a minimum, but this may be more up to IAM.

Comment 17

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

Eli's right about multiple clients - you'll want separate value there.  IIRC TH had some way to provide credentials on the command line when running locally?  That might work here, too.  Typically Taskcluster client tools take TASKCLUSTER_CLIENT_ID, TASKCLUSTER_ACCESS_TOKEN, and optionally TASKCLUSTER_CERTIFICATE, and `taskcluster signin` sets those variables.

Regarding which login options to support -- we allow all methods, since we want to mirror what Mozillians does.  The way we make that "safe" is that we also look at the provider, so for example logging in as [e-mail] with passwordless doesn't get me any LDAP-associated roles.  So as far as Taskcluster is concerned, all methods is OK.

My understanding is that we now force users to login with the "most reliable" option for a given email.  So for example if I try to login with passwordless `[e-mail]` it sees that there is also an LDAP account with that email, and indicates I should use LDAP.  So I *think* it's safe to allow all options, and then key off of the `primaryEmail` field as a user identifier to which permissions (is_staff) are tied.

Andrew, can you confirm or deny my understanding?

Comment 18

[Ed Morley [:emorley]]

(In reply to :Eli Perelman from comment #16)
> You'll want to request 2 OIDC clients, one for development which includes
> localhost and a path for the callback, and one for production which includes
> any deployment URLs with the same path for callback.

Ah so just two clients - one for treeherder-{prototype,stage,prod} live deployed instances, and one for local development. My main concern is that I'd prefer to not have to have 4 clients (one for each + local), so this would be a definite improvement.

(In reply to Dustin J. Mitchell [:dustin] from comment #17)
> Eli's right about multiple clients - you'll want separate value there.  IIRC
> TH had some way to provide credentials on the command line when running
> locally?  That might work here, too.  Typically Taskcluster client tools
> take TASKCLUSTER_CLIENT_ID, TASKCLUSTER_ACCESS_TOKEN, and optionally
> TASKCLUSTER_CERTIFICATE, and `taskcluster signin` sets those variables.

The mechanics of providing different values via environment variables are easy. The problem is hardcoding them in the built "dist" bundle to a particular environment, since we often mix and match between them. (eg if I download a production bundle from Heroku to debug a minification problem, I ideally don't want to have to unpick the client ID to get it to work). This would also break usage of Heroku release promotion (since environment specific variables shouldn't be baked in at build-time, only run-time, under the 12-factor app methodology).

If there's a particular security reason (eg a particular attack vector if localhost is on the allowed callbacks list) why we need separate clients (*given they don't have credentials, and the client ID is written in cleartext in client side source*), then I was thinking fewer client IDs would simplify things. 

Hence this part of Eli's reply:

(In reply to :Eli Perelman from comment #16)
> > * Given that for the SPA client type there is no secret to go with the
> > client ID, do we still need separate dev/stage/prod client IDs? (multiple
> > callback URLs can be whitelisted per client ID) It's just that otherwise it
> > will get a bit messy for dev vs stage vs prod webpack builds having to
> > hardcode different client IDs - and will break certain workflows (eg
> > building the UI locally and pointing at prod API).
> 
> This is a good point. I still think you should separate development and
> production at a minimum, but this may be more up to IAM.

Perhaps the IAM team are the best ones to ask about this - Andrew, are you the right person to ask? :-)

Comment 19

[Ed Morley [:emorley]]

(In reply to Ed Morley [:emorley] from comment #18)
> If there's a particular security reason (eg a particular attack vector if
> localhost is on the allowed callbacks list) why we need separate clients
> (*given they don't have credentials, and the client ID is written in
> cleartext in client side source*), then I was thinking fewer client IDs
> would simplify things. 

Sorry this should have read:

If there's a particular security reason (eg a particular attack vector if localhost is on the allowed callbacks list) why we need separate clients (*given they don't have credentials, and the client ID is written in cleartext in client side source*), then fair enough - but I was thinking fewer client IDs would simplify things, so good to check first in case we can get away with that.

Comment 20

[Ed Morley [:emorley]]

Reading the Auth0 client settings page (https://auth0.com/docs/clients/client-settings/single-page-app#settings), it says:

"""
Client Secret: A string used to sign and validate id_tokens for authentication flows and to gain access to select Auth0 API endpoints. By default, the value is hidden, so check the Reveal Client Secret box to see this value.


While the Client ID is considered public information, the Client Secret must be kept confidential. If anyone can access your Client Secret they can issue tokens and access resources they shouldn't.
"""

What's not clear is whether a client ID+secret combo can validate a different client's id_token.

For example, treeherder's `yarn start` runs a local frontend instance that proxies localhost/api/... requests to the production Treeherder instance. With separate client IDs, this local instance would be configured with CLIENT_DEV, which is set up to allow callbacks to localhost (since the callback would need to hit the local webpack-dev-server instance), but would redirect the request to the Django auth backend to the prod Treeherder instance. This would then try to validate CLIENT_DEV's id_token payload, using it's own CLIENT_PROD+CLIENT_PROD_SECRET combo.

(This is separate to the "download bundle from prod" scenario in comment 18)

Comment 21

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

That's why I was suggesting using some command-line mechanism for simulating logins -- I don't know of an easy way to support logins for arbitrary development environments.

Comment 22

[Hassan Ali (:hassan)]

Andrew, any thoughts on Comment 17 and Comment 18? Thanks.

Comment 23

[Andrew Krug [:andrew]]

(In reply to Dustin J. Mitchell [:dustin] from comment #17)
> Eli's right about multiple clients - you'll want separate value there.  IIRC
> TH had some way to provide credentials on the command line when running
> locally?  That might work here, too.  Typically Taskcluster client tools
> take TASKCLUSTER_CLIENT_ID, TASKCLUSTER_ACCESS_TOKEN, and optionally
> TASKCLUSTER_CERTIFICATE, and `taskcluster signin` sets those variables.
> 
> Regarding which login options to support -- we allow all methods, since we
> want to mirror what Mozillians does.  The way we make that "safe" is that we
> also look at the provider, so for example logging in as
> [e-mail] with passwordless doesn't get me any LDAP-associated
> roles.  So as far as Taskcluster is concerned, all methods is OK.
> 
> My understanding is that we now force users to login with the "most
> reliable" option for a given email.  So for example if I try to login with
> passwordless `[e-mail]` it sees that there is also an LDAP
> account with that email, and indicates I should use LDAP.  So I *think* it's
> safe to allow all options, and then key off of the `primaryEmail` field as a
> user identifier to which permissions (is_staff) are tied.
> 
> Andrew, can you confirm or deny my understanding?

This is correct.  We require users to sign in with the most "secure" form of a login that they are capable of.  In future we will likely not even ask these questions on the RP form.  The next generation of the auth0 lock will be smart enough to only show options a user can use.

Comment 24

[Andrew Krug [:andrew]]

(In reply to Ed Morley [:emorley] from comment #18)
> (In reply to :Eli Perelman from comment #16)
> > You'll want to request 2 OIDC clients, one for development which includes
> > localhost and a path for the callback, and one for production which includes
> > any deployment URLs with the same path for callback.
> 
> Ah so just two clients - one for treeherder-{prototype,stage,prod} live
> deployed instances, and one for local development. My main concern is that
> I'd prefer to not have to have 4 clients (one for each + local), so this
> would be a definite improvement.
> 
> (In reply to Dustin J. Mitchell [:dustin] from comment #17)
> > Eli's right about multiple clients - you'll want separate value there.  IIRC
> > TH had some way to provide credentials on the command line when running
> > locally?  That might work here, too.  Typically Taskcluster client tools
> > take TASKCLUSTER_CLIENT_ID, TASKCLUSTER_ACCESS_TOKEN, and optionally
> > TASKCLUSTER_CERTIFICATE, and `taskcluster signin` sets those variables.
> 
> The mechanics of providing different values via environment variables are
> easy. The problem is hardcoding them in the built "dist" bundle to a
> particular environment, since we often mix and match between them. (eg if I
> download a production bundle from Heroku to debug a minification problem, I
> ideally don't want to have to unpick the client ID to get it to work). This
> would also break usage of Heroku release promotion (since environment
> specific variables shouldn't be baked in at build-time, only run-time, under
> the 12-factor app methodology).
> 
> If there's a particular security reason (eg a particular attack vector if
> localhost is on the allowed callbacks list) why we need separate clients
> (*given they don't have credentials, and the client ID is written in
> cleartext in client side source*), then I was thinking fewer client IDs
> would simplify things. 
> 
> Hence this part of Eli's reply:
> 
> (In reply to :Eli Perelman from comment #16)
> > > * Given that for the SPA client type there is no secret to go with the
> > > client ID, do we still need separate dev/stage/prod client IDs? (multiple
> > > callback URLs can be whitelisted per client ID) It's just that otherwise it
> > > will get a bit messy for dev vs stage vs prod webpack builds having to
> > > hardcode different client IDs - and will break certain workflows (eg
> > > building the UI locally and pointing at prod API).
> > 
> > This is a good point. I still think you should separate development and
> > production at a minimum, but this may be more up to IAM.
> 
> Perhaps the IAM team are the best ones to ask about this - Andrew, are you
> the right person to ask? :-)

We would strongly reccomend you use a different auth0 client_id + client_secret combo for each environment.  In our environments we use parameter store in AWS, CredStash, .env, and other methods to abstract these in a secure way.  Even if you're using discovery in OIDC flows there are some tangible benefits from a logging / instrumentation perspective in use knowing what traffic is coming from what application.

Comment 25

[Hassan Ali (:hassan)]

I guess we'll need to create a client for each environment. Ed, is there anything else that needs to be addressed? Otherwise, we can proceed with submitting the request form.

Comment 26

[Ed Morley [:emorley]]

I still don't feel we have consensus on this. Comment 24 suggests we'll need 4 clients, and comment 16 says 2 clients are being used for other apps.

Perhaps let's just create a test client so you can be unblocked (that has just the dev and localhost callback URLs), and once we figure out how we're going to maintain the various development workflows, we can expand from there?

Comment 27

[Hassan Ali (:hassan)]

That works. A test client will definitely help for now.

Comment 28

[Ed Morley [:emorley]]

I've submitted the service now request with...

"""

# What is the name and purpose of the service to use Mozilla SSO?
Treeherder (https://wiki.mozilla.org/EngineeringProductivity/Projects/Treeherder)

# What is the URL of the service to use Mozilla SSO?
https://treeherder-prototype.herokuapp.com

# Who should the gpg-encrypted credentials be e-mailed to?
Hassan Ali

# How many users do you expect to use the service per month? 
500

# What kind of environment are you setting up?
DEV - An environment in good condition to test with a real federated login

# Do you need this environment hosted by our Development SSO Infrastructure?
No, use production SSO infrastructure

# Do you need specific restrictions on whom can login? 
Anyone can login (I'll take care of access control myself if needed)

# Which technology would you like to use? 
OIDC

# What is your redirect/callback URL? 
https://treeherder-prototype.herokuapp.com/#/login
http://localhost:5000/#/login
http://localhost:8000/#/login

# What is your logout URL?
<blank>

# Do you have CORS requirements (URLs)?
<blank>

# Which login options would you like to support? 
LDAP + GitHub + Google + Passwordless

# Do you have any other requirement? (settings, etc.)
Please can this client:
* be of type "SPA (single-page application)"
* use "RS256 algorithms"
(I'm taking this from: https://docs.taskcluster.net/manual/using/integration/frontend#creating-a-simple-login-integration)

This client will be used for the dev instance and localhost testing. Later we'll likely need additional clients for prod etc, but I'll open a new ticket then. 

"""

Hassan, I think I've managed to add you to the CC list of the request (Service Now grumble gumble) - let me know if you didn't get a notification.

Comment 29

[:Eli Perelman]

Note, with this being a SPA, there won't actually be any gpg credentials to mail.

Comment 30

[Ed Morley [:emorley]]

I thought that at first, but the Auth0 docs imply that there is still a secret that's required if verifying the id_token server side? (comment 20)

The SS0 request Mana page and the service now form seem to be tailored for the private client case - perhaps after this bug we can help them make it clearer for other SPA apps in the future?

Comment 31

[Hassan Ali (:hassan)]

Thanks for submitting Ed. I think we will need to change the callback URL. Auth0 appends the credentials in the hash. Having something like http://localhost:8000/#/login will not work since it will lose the path parameter when redirecting. Auth0 strips everything after the first hash and replaces with the relevant response information. The redirect URL will end up like http://localhost:8000/#accessToken=...

For SPA, Auth0 doesn't provide the option to have the credentials as query parameters so the option I see is to have the callback URL set to something like http://localhost:8000/login or http://localhost:8000/login.html.

Comment 32

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

(In reply to Ed Morley [:emorley] from comment #30)
> I thought that at first, but the Auth0 docs imply that there is still a
> secret that's required if verifying the id_token server side? (comment 20)

This may be the case, and SPA's do *have* secret tokens.  I haven't implemented this particular corner of things (validating id_tokens for a SPA) so I'm not sure -- we'll have to see!

Comment 33

[Ed Morley [:emorley]]

Ah ok. I copied the existing callback URLs used by the login.tc.net implementation, since production is served by gunicorn+whitenoise on Heroku, and currently doesn't have full re-write support line one would with nginx/apache. We could hardcode something in our custom WhiteNoise subclass as a workaround:
https://github.com/mozilla/treeherder/blob/36ebe5cedfd022672c601b7b922178690e8cf816/treeherder/middleware.py#L7-L51

...but a separate entrypoint with template login.html might be easier (and quicker to load) for now. (The current Treeherder auth implementation opens the login.tc.net page in a new tab, to avoid disrupting the users current state, so having that new tab hit the index page is unnecessary, since the tab won't be persisting so wouldn't have needed an additional redirect anyway)

Comment 34

[Hassan Ali (:hassan)]

Right. the redirection to login.html would happen on the new tab, which will then close the window if everything is successful. It's definitely unnecessary for the redirection to hit the index page. Instead of the workaround, I think it's better to send a request to have index.html added to the list of allowed callbacks.

Comment 35

[Hassan Ali (:hassan)]

s/index.html/login.html/

Comment 36

[Ed Morley [:emorley]]

I updated the service now ticket earlier, and the callback URLs have now been updated. An email with the client ID (only) was sent to Hassan, who also forwarded it to me.

Comment 37

[Hassan Ali (:hassan)]

From, https://docs.taskcluster.net/reference/integrations/taskcluster-login/docs/getting-user-creds#sign-in-flow:

- the OIDC audience must include login.taskcluster.net
- the OIDC scopes must include full-user-credentials and openid

I believe these are missing from the client I received. I will try to get these added.

Comment 38

[Ed Morley [:emorley]]

Ah sorry! I hadn't read beyond the things to request on this page:
https://docs.taskcluster.net/manual/using/integration/frontend#creating-a-simple-login-integration

Comment 39

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/treeherder/pull/3144

Comment 40

[Ed Morley [:emorley]]

Comment on attachment 8943312 [details] [review]
Link to GitHub pull-request: https://github.com/mozilla/treeherder/pull/3144

Cameron, I don't suppose you could take a glance at this - particularly the changes to the parts you wrote? :-)
(I have a review in progress but have to stop for food now; will try and finish it up over the weekend)

Comment 41

[Ed Morley [:emorley]]

I've submitted an SSO client Service Now change request to update the callback URLs to:

https://treeherder.mozilla.org/login.html,
https://treeherder.allizom.org/login.html,
https://treeherder-prototype.herokuapp.com/login.html,
http://localhost:5000/login.html,
http://localhost:8000/login.html

I believe Hassan has also previously requested that the audience value be changed from https://treeherder-prototype.herokuapp.com to login.taskcluster.net ?

Comment 42

[Hassan Ali (:hassan)]

Thanks Ed. The audience value was not changed. I simply changed the audience entry to use login.taskcluster.net and it worked. In other words, I don't believe it required any extra changes from the IAM team.

Comment 43

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

Clients don't have audiences -- APIs do.  And you're not adding a new API.

The `aud` claim for the id_token you receive should be equal to the SPA's Client ID.

Audience basically means 'is this meant for me?'.  So the only thing validating an audience of `login.taskcluster.net` should be login.taskcluster.net.

Comment 44

[Ed Morley [:emorley]]

Ah that makes sense.

I was just confused by the email from Justin after initial setup which said the audience was "https://treeherder-prototype.herokuapp.com" and I'd thought this had been updated later, given comment 37.

Comment 45

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

No lie, that's pretty confusing.  I don't see an "audience" option anywhere in the UI to set up a Client in Auth0 (admittedly, in my personal Auth0 account, not in the Mozilla account to which I do not have access).

Comment 46

[Cameron Dawson [:camd]]

Comment on attachment 8943312 [details] [review]
Link to GitHub pull-request: https://github.com/mozilla/treeherder/pull/3144

This looks good to me.  I don't claim to grok everything here, but based on my context, it looks great.  I tested it on prototype and it appears to work quite well.  Thanks!!

Comment 48

[Treeherder GitHub Bugbot]

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

https://github.com/mozilla/treeherder/commit/e1b0168127c10aaed70780c82430160cc7edcc16
Bug 1395356 - Use auth0 instead of login.taskcluster.net for SSO (#3144)

## Rough summary of the changes

### Front end
The auth callback is written in React and lives under the /login.html endpoint. It communicates with Treeherder using the localStorage.

### Credential expiration
The Django user session expiration is set to expire when the client access token or the id token expires (whichever one expires first). These values are controlled by the IAM team. Presently, the access token expires after 1 day and the id token expires after a week. That being said, the session will therefore expire after 1 day. If you want this value change, we simply need to send a request to the IAM team.

### Credential renewal
Renewals are set to happen every 15 minutes or so. The renewal is skewed slightly so that different open tabs don't renew at the same time. Once renewal happens, both tokens are renewed and the Django session is updated.

### Migration
If the userSession localStorage key is not set, then the user will be logged out including logging out from the Django session. In other words, all users will be automatically logged out when the merge to production happens.

Comment 49

[Ed Morley [:emorley]]

Comment on attachment 8943312 [details] [review]
Link to GitHub pull-request: https://github.com/mozilla/treeherder/pull/3144

(Set on GitHub, forgot to set here)

This is working great for me on stage - should we deploy to prod now?

Comment 50

[Hassan Ali (:hassan)]

How about we deploy to production tomorrow morning. Not a big fan of deploying late afternoon :)

Comment 51

[Ed Morley [:emorley]]

With Heroku rollbacks are quick/easy/deterministic, and any issues with the changes here would likely manifest within <1 hour of deploy (given everyone will get logged out on deploy and the TC creds are renewed every 15 minutes) - so this possibly doesn't need as much planning over deploy time as a data ingestion/schema change issue might.

However I'm happy to wait until tomorrow -- my changes in bug 1363722 were blocked on this landing on master rather than the prod deploy itself :-)

Comment 53

[Ed Morley [:emorley]]

I don't suppose you could post to dev.platform about the new auth - and mention that:
* the one-time permissions prompt (http://web.mit.edu/jwalden/www/new-auth-request.png) isn't something to be worried about (Waldo was asking in #developers)
* the "Your credentials are expired. They must expire every 3 days (Bug 1328434). Log out and back in again..." behaviour has now been fixed (yey!)
* their session stays alive, so long as they access the site once every 24 hours
* they must now use the most secure login method available to them (eg LDAP > ...)

Comment 54

[Ionuț Goldan [:igoldan]]

Guys, I'm getting an import error when running |celery -A treeherder worker -B --concurrency 5| locally.

This is the originating error:
File "/home/vagrant/treeherder/treeherder/auth/backends.py", line 7, in <module>
    from jose import jwt
ImportError: cannot import name jwt

Comment 55

[Ed Morley [:emorley]]

Answered on IRC, but for posterity - your environment needs the latest packages installed - run `vagrant provision` to keep the environment up to date. (Another reason for switching to Docker/Docker Compose; bug 1169263).

Comment 56

[Hassan Ali (:hassan)]

(In reply to Ed Morley [:emorley] from comment #53)
> I don't suppose you could post to dev.platform about the new auth - and
> mention that:
> * the one-time permissions prompt
> (http://web.mit.edu/jwalden/www/new-auth-request.png) isn't something to be
> worried about (Waldo was asking in #developers)
> * the "Your credentials are expired. They must expire every 3 days (Bug
> 1328434). Log out and back in again..." behaviour has now been fixed (yey!)
> * their session stays alive, so long as they access the site once every 24
> hours
> * they must now use the most secure login method available to them (eg LDAP
> > ...)

Good idea. I'll send something shortly.

Comment 57

[Hassan Ali (:hassan)]

Sent https://groups.google.com/forum/#!topic/mozilla.dev.platform/PK_obIp20v8

Comment 58

[Ed Morley [:emorley]]

Many thanks for posting that :-)

I was just looking at the taskcluster-tools Auth0 implementation and I see there is a fair amount of overlap between it and Treeherder's. Do you think it may be useful in the future if there was a `taskcluster-login-client` (or other suitable name) library that consolidates the controller logic and even perhaps the react components (if only for the callback views)?

Such a library would make the transitions for bug 1437116 easier too - since it would be a case of just updating to a newer version.

# Treeherder implementation:
https://github.com/mozilla/treeherder/blob/a74003f41b38d8d5198db7dfab056fbe19133d75/ui/js/auth/AuthService.js
https://github.com/mozilla/treeherder/blob/a74003f41b38d8d5198db7dfab056fbe19133d75/ui/js/auth/auth-utils.js
https://github.com/mozilla/treeherder/blob/a74003f41b38d8d5198db7dfab056fbe19133d75/ui/js/services/taskcluster.js
--
https://github.com/mozilla/treeherder/blob/a74003f41b38d8d5198db7dfab056fbe19133d75/ui/js/components/auth.js
--
https://github.com/mozilla/treeherder/blob/a74003f41b38d8d5198db7dfab056fbe19133d75/ui/entry-login.jsx
https://github.com/mozilla/treeherder/blob/a74003f41b38d8d5198db7dfab056fbe19133d75/ui/js/auth/LoginCallback.jsx

# taskcluster-tools implementation:
https://github.com/taskcluster/taskcluster-tools/blob/fe19156c4987d4db2e16b052b3b5889e4138f4eb/src/auth/AuthController.js
https://github.com/taskcluster/taskcluster-tools/blob/fe19156c4987d4db2e16b052b3b5889e4138f4eb/src/auth/auth0.js
https://github.com/taskcluster/taskcluster-tools/blob/fe19156c4987d4db2e16b052b3b5889e4138f4eb/src/auth/UserSession.js
-
https://github.com/taskcluster/taskcluster-tools/blob/fe19156c4987d4db2e16b052b3b5889e4138f4eb/src/components/CredentialsMenu/index.js
https://github.com/taskcluster/taskcluster-tools/blob/fe19156c4987d4db2e16b052b3b5889e4138f4eb/src/auth/Auth0LoginMenuItem.js
https://github.com/taskcluster/taskcluster-tools/blob/fe19156c4987d4db2e16b052b3b5889e4138f4eb/src/auth/DevelopmentLoginMenuItem.js
https://github.com/taskcluster/taskcluster-tools/blob/fe19156c4987d4db2e16b052b3b5889e4138f4eb/src/auth/ManualLoginMenuItem.js
-
https://github.com/taskcluster/taskcluster-tools/blob/fe19156c4987d4db2e16b052b3b5889e4138f4eb/src/views/Auth0Login/index.js
https://github.com/taskcluster/taskcluster-tools/blob/fe19156c4987d4db2e16b052b3b5889e4138f4eb/src/views/DevelopmentLogin/index.js
https://github.com/taskcluster/taskcluster-tools/blob/fe19156c4987d4db2e16b052b3b5889e4138f4eb/src/views/ManualLogin/index.js
https://github.com/taskcluster/taskcluster-tools/blob/fe19156c4987d4db2e16b052b3b5889e4138f4eb/src/components/ManualModal/index.js

Comment 59

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

That sounds like a great idea.  Now that there are two implementations, the commonalities and differences are a little clearer.  Maybe in another bug :)

Comment 60

[Ed Morley [:emorley]]

Agree on all counts - filed bug 1437321 :-)