Add a per-user (rather than per-repository) authentication mechanism for submitting data to Treeherder

RESOLVED FIXED

Status

P3
normal
RESOLVED FIXED
4 years ago
2 years ago

People

(Reporter: emorley, Assigned: mdoglio)

Tracking

Details

Attachments

(1 attachment)

46 bytes, text/x-github-pull-request
emorley
: review+
Details | Review | Splinter Review
(Reporter)

Description

4 years ago
(Filing bugs for things we discussed as possible future goals, but do not have a bug on file).
(Reporter)

Updated

4 years ago
Priority: -- → P3
(Reporter)

Updated

4 years ago
Blocks: 1043334
(Assignee)

Updated

3 years ago
Blocks: 1164845
(Reporter)

Updated

3 years ago
Blocks: 1170562
(Reporter)

Comment 1

3 years ago
As part of this I imagine we'll have no choice but to move away from using the oauth2 package anyway, but in case not, we should absolutely consider doing so. It's had no commits since December 2011 and there's no way to work around bug 1170562 comment 2.
(Assignee)

Comment 2

3 years ago
I'm going to base this implementation on django-oauth-toolkit, which the is recommended package to do oauth authentication with django-rest-framework http://www.django-rest-framework.org/api-guide/authentication/#django-oauth-toolkit
(Assignee)

Updated

3 years ago
Blocks: 1178641
(Reporter)

Updated

3 years ago
Depends on: 1185520
(Reporter)

Updated

3 years ago
Blocks: 1196191
(Assignee)

Comment 3

3 years ago
After some investigation I decided django-oauth-toolkit is too complicated for what we need. I'm going to create a simple table holding the list clients and their credentials. And I'm gonna use hawk for the authentication since:
- there's already a hawk backend for drf
- there's already a hawk auth backend for requests
- the credentials don't go through the wire, which would also solve bug 1170562
(Assignee)

Comment 4

3 years ago
Created attachment 8663692 [details] [review]
PR 983
Assignee: nobody → mdoglio
Status: NEW → ASSIGNED
Attachment #8663692 - Flags: review?(emorley)
(Assignee)

Comment 5

3 years ago
Once attachment 8663692 [details] [review] has landed we will
- create new credentials for our own etl service
- propagate the credentials on the staging/prod machines
- enable hawk authentication in the client
(Reporter)

Comment 6

3 years ago
Comment on attachment 8663692 [details] [review]
PR 983

Looks good so far, have left some comments - reflag for review for a final glance :-)
Attachment #8663692 - Flags: review?(emorley)
(Assignee)

Updated

3 years ago
Attachment #8663692 - Flags: review?(emorley)
(Reporter)

Comment 7

3 years ago
Comment on attachment 8663692 [details] [review]
PR 983

(Had another quick glance, but didn't have time to test locally.)
Attachment #8663692 - Flags: review?(emorley) → review+
(Assignee)

Updated

3 years ago
Blocks: 1209555

Comment 8

3 years ago
Commits pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/01fab0efe95414cce1827f98fcbe5e5af7a5ffa7
Bug 1160111 - Create login page for login_required

The login_required decorator redirects unauthenticated users to
settings.LOGIN_URL, adding a `next` querystring parameter to it.
This parameter is then passed to the browserid login button so that
the user can be sent back to the original page after a successful
authentication.

https://github.com/mozilla/treeherder/commit/789c6a25435fac2f7fcc2f4b585d53fd59ab8f48
Bug 1160111 - Add treeherder.credentials app

The new app handles treeherder client credentials.
The content of the credentials table should replace the credentials file
we currently use for authentication.
This commit includes orm models, migrations and an admin interface.

https://github.com/mozilla/treeherder/commit/2b59445ef0ed95326d27e95da8da6eb0bdf4b0e6
Bug 1160111 - Add ui to manage credentials.

An authenticated user can access the new ui and add new credentials.
When new credentials are created the system generates automatically
 a secret key which is available in the credentials details.
The credentials needs to be approved by a member of staff before they
can be used for data submission; that can be done via the admin
panel.
In the admin panel it is also possible to reset the secret key of one
or more credentials. The new secret will be available to its owner
in the application details.

https://github.com/mozilla/treeherder/commit/38a2e3a1b52a9508715c9a7275a5de79296d024d
Bug 1160111 - Add requirements for hawkrest

https://github.com/mozilla/treeherder/commit/8b669ee9cd2d02284176e17b2776436599859557
Bug 1160111 - Add hawk authentication scheme

The hawk credentials lookup function is the glue between hawk and
the `application` django app. I wrote tests to verify its logic,
everything else is mostly configuration code.

https://github.com/mozilla/treeherder/commit/a5aed772a2fe7c21a81c274a8a703129ec934851
Bug 1160111 - Add throttling for hawk clients

The new throtlling class is based on the hawk client id.
I added some tests to cover both the new throttling class and the one
based on oauth.
(Assignee)

Updated

3 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
(Reporter)

Updated

3 years ago
Depends on: 1210748
(Reporter)

Updated

3 years ago
Summary: Split out the oauth credentials from the datasource table to allow per-user not per-project keys → Add a per-user (rather than per-repository) authentication mechanism for submitting data to Treeherder
(Reporter)

Updated

3 years ago
Blocks: 1212931
(Reporter)

Updated

3 years ago
Blocks: 1212936
(Reporter)

Updated

3 years ago
Depends on: 1212951
(Reporter)

Updated

3 years ago
Depends on: 1271256
(Reporter)

Updated

2 years ago
Depends on: 1303928
You need to log in before you can comment on or make changes to this bug.