1212936 - Migrate submitters of data from oauth to per-user Hawk credentials

Status: RESOLVED FIXED

(Tree Management :: Treeherder: API, defect, P2)

Description

[Ed Morley [:emorley]]

Bug 1160111 added support for new per-user Hawk credentials, that are to replace the current per-project (repository) oauth ones. Bug 1209555 then migrated Treeherder to use these credentials for the ETL layer.

We'll need to migrate other clients too, once we have the docs in place (bug 1212931).

This bug is for tracking that migration & eventually double-checking access logs to confirm we'd ok to remove legacy oauth support.

Comment 1

[Ed Morley [:emorley]]

Support has been added for per-user credentials for submitting to Treeherder's API. This means that:
* each submitter will have their own client_id and secret for submitting to the API (rather than shared credentials), which will make it easier to track submissions from different sources and revoke just one user in case of compromise/leak
* there are no longer different credentials per repository, which makes it easier for clients to start submitting to different repos (in the initial implementation everyone can submit to all repos; we may consider adding in controls later, but even if we do, it will no longer require code changes on the submitters side/keeping track of multiple credentials)
* the owner of an api key can see it in the web UI so we don't have to send it over PM or private bugs, and treeherder admins can reset/revoke keys from the admin UI too
* we're no longer using oauth, but instead Hawk for authentication - which uses headers rather than URI params (so reduces chances of leaks in logs) and also doesn't submit the secret itself over the wire.
* we'll eventually need everyone to switch over, but there will be a deprecation period where both authentication methods will work

We're still finalising the last few pieces of this (docs, python client support etc), but in the meantime I wanted to give a heads up, and ask if you could:
1) Request an API key for stage and/or prod. For the client id I'd suggest using values like "taskcluster", "mozmill" etc. Use these forms (the api key will be associated with whomever was signed in when it was requested, but this can be changed later if needed):
  - Stage: https://treeherder.allizom.org/credentials/create/
  - Prod: https://treeherder.mozilla.org/credentials/create/
2) File a bug in whatever component you use, blocking this one, with a summary along the lines of "Submit to Treeherder's API using Hawk credentials" - and in the description mention the client_id you've chosen in #1.

I'll then press "authorise" on each of the requested keys. Then once we have docs finished and the new python client released (we'll also need to figure out the plan for the nodejs client), I'll comment in the bugs you've filed, with next steps.

Thanks :-)

Comment 2

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

Maja and Syd, given that firefox-ui-tests and firefox-media-tests will live side-by-side also in Jenkins, I wonder if we should request a single client_id like mozauto.

Comment 3

[Syd Polk :sydpolk]

:whimboo, that sounds good to me.

Comment 4

[Ed Morley [:emorley]]

We'll also be using the client_id to track things like which submitters are using deprecated APIs etc, so as long as both of them will be using the same submission code etc that's fine, or if you think they might have different codepaths, then separate would be preferred.

Comment 5

[Syd Polk :sydpolk]

We are probably going to need a separate client_id for the WebRTC tests as well.

Comment 6

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

I filed bug 1215566 for our submission of test reports for Firefox ui tests.

Comment 7

[Nils Ohlmeier [:drno]]

Created bug 1217584 for the WebRTC test lab setup.

Comment 8

[Syd Polk :sydpolk]

Nils in handling the WebRTC case. Maja is handling firefox-media-tests.

Comment 9

[Syd Polk :sydpolk]

Actually, I need to make these changes for firefox-media-tests, per conversation with :maja_zf.

Comment 10

[Bob Clary [:bc] (inactive)]

I'll work on updating Autophone in Bug 1218222

Comment 11

[Ed Morley [:emorley]]

Taskcluster is having strange intermittent errors submitting to some endpoints, so to debug I've increased the log level on Heroku by deploying a custom branch (auto-deploy is disabled):
https://github.com/mozilla/treeherder/compare/debug-hawk

Their submission code:
https://github.com/mozilla/treeherder-node/pull/6/files

I've added credentials there for taskcluster, which are now being used to submit there.

Since the logs are so verbose, I've also disabled celery beat on heroku to stop the noise from treeherder's own tasks.

Example error we were seeing on stage:
https://emorley.pastebin.mozilla.org/8851124

And the corresponding log from the client's perspective:
https://papertrailapp.com/systems/mozilla-taskcluster-staging/events?r=597945745139871752-597945819064479752

We now need to look for equivalents in the debug output on heroku here:
https://papertrailapp.com/systems/treeherder/events

Comment 12

[Ed Morley [:emorley]]

An occurrence is here:
https://papertrailapp.com/systems/treeherder/events?q=program%3Aapp%2Fweb.1&centered_on_id=597961838482993190

Comment 13

[Ed Morley [:emorley]]

(In reply to Ed Morley [:emorley] from comment #12)
> An occurrence is here:
> https://papertrailapp.com/systems/treeherder/events?q=program%3Aapp%2Fweb.
> 1&centered_on_id=597961838482993190

Corresponding TC log:
https://papertrailapp.com/systems/mozilla-taskcluster-staging/events?r=597961826122379271-597961844371795999

Comment 14

[Greg Arndt [:garndt]]

The issue described above was related to encoding of the payload when calculating the authorization header.  mohawk automatically takes care of utf8 encoding it when sending a hawk request, and also uses utf8 when calculating the payload hash when authorizing a request as well.  The node hawk client does not have such an automatic enforcement.  It appears that utf8 is the standard encoding to use, and a code comment is in the hawk code saying it should be utf8 encoding.  emorley opened up an issue for the node hawk library to clarify this in the documentation.

Unfortunately the error returned by mohawk is not clear what the problem is other than the mac was incorrect.  Looking at the reference implementation even if the calcualted payload hash is different, it should not cause an issue during MAC validation because the payload hash sent should be used while calculating the MAC to compare.  This is not the case in mohawk as it calculates the payload hash itself (encoding the payload as utf8) and using that to construct the MAC for comparison.  This differs from the reference implementation as well as hides an obscure bug.

Comment 15

[Greg Arndt [:garndt]]

Also, an issue was opened for mohawk about this.

Comment 16

[Ed Morley [:emorley]]

(In reply to Greg Arndt [:garndt] from comment #15)
> Also, an issue was opened for mohawk about this.

Thank you :-) 
(is at https://github.com/kumar303/mohawk/issues/15)

The issue I opened against the hawk docs to make it clearer that the body should be UTF8 encoded is:
https://github.com/hueniverse/hawk/issues/152

Comment 17

[Ed Morley [:emorley]]

(To be clear for others CCed: only those using the nodejs hawk client need to manually UTF8 encode the body, the python client is fine)

Comment 18

[Syd Polk :sydpolk]

mozauto client_id created in both treeherder.mozilla.org and treeherder.allizom.org by others. This is what firefox-media-tests will use.

Comment 19

[Ed Morley [:emorley]]

(Bug 1217955 blocks the task of moving all submitters to hawk; so it blocks this bug rather than depending on it. If you'd like a bug to depend on, the credentials were created in bug 1215566.)

Comment 20

[Ed Morley [:emorley]]

There are now no submitters when I grep the prod/stage gunicorn logs and also on:
https://insights.newrelic.com/accounts/677903/explorer?eventType=Transaction&timerange=day&facet=request.parameters.oauth_consumer_key