1643117 - Login improvements

Status: NEW

(Tree Management :: Treeherder: Frontend, enhancement, P1)

Description

[Sarah Clements [:sclements]]

During our UX research project we received feedback that the log in is confusing and annoying to have to login frequently.

Currently whenever a user logs in, we retrieve two different credentials:

1. User data from our IAM/auth0 service that's used for session management and for any actions that write to our database (classifications or adding notes). The access token is stored as user and userSession in local storage.

2. Taskcluster credentials are retrieved via its third party login service. These are required for any actions performed on jobs (like retriggering jobs) and the access tokens are stored as userCredentials in local storage, indexed by root url (firefox-ci and community).

The changes to be made are (which could be broken out into separate bugs or pull requests):

1. Currently if a user retriggers a job, we'll first check if the userCredentials access token for that root url is still valid. If it isn't, a new token will be retrieved then the retriggering action will be performed. However, it doesn't actually log the user in to Treeherder or retrieve the auth0 token - that's why users are confused.

2. I think we are currently using the OAuth implicit flow method for authentication, which means that a user is logged out after 2 hours of no activity. This should be switched to authorization code flow if possible, which means a user will be logged out after 24 hours.

Comment 1

[Joel Maher ( :jmaher ) (UTC -8)]

Suyash, this would be a good bug for you to look into after bug 1643114.

Comment 2

[Suyash Salampuria]

:sclements
I have made some reasearch in the oauth0
I have to make request to url
https://auth.mozilla.auth0.com/authorize?response_type=id_token%20token&client_id=q8fZZFfGEmSB2c5uSI8hOkKdDGXnlo5z&scope=openid%20profile%20email&redirect_uri=`${window.location.protocol}//${window.location.host}${loginCallbackUrl}

This will give me the HTTP 302 response with url
https://YOUR_APP/callback?code=AUTHORIZATION_CODE&state=xyzABC123

Now, make the request to https://auth.mozilla.auth0.com/oauth/token with headers
content-type: application/x-www-form-urlencoded
and data
grant_type: 'authorization_code',
client_id: 'q8fZZFfGEmSB2c5uSI8hOkKdDGXnlo5z',
client_secret: 'YOUR_CLIENT_SECRET',
code: 'YOUR_AUTHORIZATION_CODE',
redirect_uri: '${window.location.protocol}//${window.location.host}${loginCallbackUrl}'
I don't have only the client secret

This will give a HTTP 200 response and a data
{
"access_token": "",
"refresh_token": "",
"id_token": "",
"token_type": "Bearer"
}
We can decode the ID token to get all the details

Could you please check if my approach is right

Comment 3

[Sarah Clements [:sclements]]

Yes, you are on the right path - we need an authorization code instead of the access token we currently receive. So you'll need to look at where login happens: treeherder/ui/shared/auth/Login.jsx

And what happens in the callback: treeherder/ui/login-callback/LoginCallback.jsx and determine what needs to change in the logic: treeherder/ui/helpers/auth.js and what we pass to our server.

The taskcluster login logic (which is completely separate and happens after a user logins in and we retrieve the Mozilla auth0 credentials) is using the authorization flow so you might find it helpful to look at how that's implemented: treeherder/ui/helpers/taskcluster.js
You can reuse the generateNonce helper in that file for the state property. Anything that can be reused in that file should be.

Once you figure out what changes you need to make, you can open a WIP pr if you'd like feedback on the changes or additional guidance. :)

Comment 4

[GitHub Bugzilla PR Linker]

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

Comment 5

[Sarah Clements [:sclements]]

This bug is a bit more complex than I originally thought since there are standards around session management and the authorization code flow provides optional refresh tokens.

There also seems to be a new issue with the login and classifying failures per bug 1646809 so I'll prioritize this next week.

Suyash, we have some other work lined up for you with Push Health instead of this bug :)

Comment 6

[Sarah Clements [:sclements]]

I've done some reading up on this and I think we do want to go with the authorization code flow with PKCE, which is an authentication and authorization protocol designed for single page apps. We can utilize the auth0 react library which will give us a provider to wrap around the App (entry point).

However, we don't currently have a top level app; we're treating each app as a separate entry point and using the HashRouter. Other benefits to serving a top level app and letting react router handle the routing between apps is being able to share commonly used information between apps (such as repositories) and user information (used for session management) instead of re-retrieving it upon navigating to a different part of the app.

So part 1 will be to do this work in bug 1623951 and then part 2 will be the authentication updates in this bug.

Comment 7

[Sarah Clements [:sclements]]

Blocked until we know whether IAM team will be moving away from auth0.