1339882 - Treeherder auth doesn't allow login with manual/temporary tc credentials

Status: RESOLVED INVALID

(Tree Management :: Treeherder, defect)

Description

[Brian Stack [:bstack]]

In https://github.com/mozilla/treeherder/pull/2011/files#diff-55856cb63e771da7073dd615d450fc88R18 a regex is created to pull the email out of mozilla-ldap/email clientIds.

It seems to not accept temp creds that are created from those "root" creds as can be seen at this traceback https://papertrailapp.com/systems/treeherder-prod/events?focus=768270016788463664. Pasted here for posterity


Feb 14 13:17:48 treeherder-prod app/web.1: [2017-02-14 21:17:47,992] WARNING [treeherder.webapp.api.auth:62] Email required for login. 
Feb 14 13:17:48 treeherder-prod app/web.1: Traceback (most recent call last): 
Feb 14 13:17:48 treeherder-prod app/web.1:   File "/app/treeherder/webapp/api/auth.py", line 50, in login 
Feb 14 13:17:48 treeherder-prod app/web.1:     port=int(port)) 
Feb 14 13:17:48 treeherder-prod app/web.1:   File "/app/.heroku/python/lib/python2.7/site-packages/django/contrib/auth/__init__.py", line 74, in authenticate 
Feb 14 13:17:48 treeherder-prod app/web.1:     user = backend.authenticate(**credentials) 
Feb 14 13:17:48 treeherder-prod app/web.1:   File "/app/treeherder/auth/backends.py", line 106, in authenticate 
Feb 14 13:17:48 treeherder-prod app/web.1:     email = self._extract_email_from_clientid(client_id) 
Feb 14 13:17:48 treeherder-prod app/web.1:   File "/app/treeherder/auth/backends.py", line 58, in _extract_email_from_clientid 
Feb 14 13:17:48 treeherder-prod app/web.1:     "No email found in clientId: '{}'".format(client_id)) 
Feb 14 13:17:48 treeherder-prod app/web.1: NoEmailException: No email found in clientId: 'mozilla-ldap/[e-mail]/treeherder-login-temp'

Comment 1

[Brian Stack [:bstack]]

The more I think about this, the more I think this might be semi-correct behavior. At least according to https://docs.taskcluster.net/manual/integrations/apis/3rdparty#authenticating-with-scopes, maybe we do want to be super restrictive about which clientIds to actually look at if we're going to look at all.

Does that make sense, dustin?

Comment 2

[Ed Morley [:emorley]]

What scopes did your temporary client ID have? eg we could use "assume:mozilla-user:..." to cross-reference.

Comment 3

[Brian Stack [:bstack]]

Oh yeah, that's a good point. I gave it "assume:mozilla-user:[e-mail]". I think requiring temp creds to provide that seems like a really nice compromise.

Comment 4

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

Oh, I should expand that section of the docs with the more detailed approach we've talked about for treeherder: looking for an identity in the clientId, then verifying that with the groups.

In this case, I assume there is a regex expecting `^mozilla-ldap/<email>$`; looking for `mozilla-ldap/<email>(/.*)?$` would allow this particular pattern, and follow the convention of allowing users to create clientIds with prefix `mozilla-ldap/<email>/`.  It's just a convention, but as long as you're verifying that convention by checking scopes, it's OK (if someone makes a clientId that matches that pattern with email=[e-mail], we're still OK because they don't have `assume:mozilla-user:[e-mail]`)