Add JWT token auth #3109

fdobrovolny · 2017-07-27T18:37:11Z

closes #2359
https://pulp.plan.io/issues/2359

pep8speaks · 2017-07-27T18:37:14Z

Hello @BrnoPCmaniak! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on September 25, 2017 at 16:40 Hours UTC

bmbouter · 2017-07-27T19:54:25Z

docs/integration_guide/rest_api/authentication.rst

+   from datetime import datetime, timedelta
+
+
+   def generate_token_offline(username, jwt_secret, exp_delta=timedelta(days=14)):


The requirement we heard from users is that they want to generate tokens without having Pulp installed, so I think this function should go away and we need docs describing from a high level about how users can generate the JWT using non-pulp tools. Does that make any sense? I could also not be understanding this.

ohhhhh this is in a Python codeblock and isn't actual code. This is perfect. Thank you!

mikeadamz · 2017-08-03T04:57:21Z

Let me know if you need help with this, I've recently spent a bunch of time implementing JWT elsewhere.

bmbouter · 2017-08-05T13:42:16Z

@mikeadamz Some help testing would be ideal. Overall we're looking for this PR to fulfill the use cases of the authentication portion of the MVP. Getting a Pulp3 vagrant environment going is what I recommend to test out pulp. Also here are some Pulp3 dev notes we maintain on the wiki.

This contains the changes to the default HTTP adapter for the requests library proposed in GitHub pull request pulp#3109. Note that at this time, that pull request has not been accepted and is subject to change. In addition to the modified adapter, the Pulp streamer (and only the Pulp streamer) has been changed to use this adapter. Note this fix only works if urllib-1.16+ is used in conjunction with requests. closes pulp#1788

fdobrovolny · 2017-09-11T15:45:29Z

The doc failure will be solved when pulp/pulp-ci#428 is merged.

daviddavis · 2017-09-11T21:53:43Z

docs/integration_guide/rest_api/authentication.rst

+
+For using JWT tokens you have to set ``Authorization`` header as follows:
+::
+  Authorization: JWT eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJ1c2VybmFtZSI6ImFkbWluIiwiZXhwIjoxNTAyMzgzMDExfQ.3ZpcclxV6hN8ui2HUbwXLJsHl2lhesiCPeDVV2GIbJg


Why are you using JWT here? Typically I see Bearer being used like on the jwt.io site.

@daviddavis It's the default value from the library they decided to use JWT instead of Bearer, which they originally used, to maintain compatibility with django-oauth2-provider which also uses it. Source: jpadilla/django-rest-framework-jwt#4

daviddavis · 2017-09-11T22:13:26Z

A couple observations from testing. First, I assumed I could get a jwt token by using basic auth. However, it looks like I have to POST username and password. I think it would be handy to be able to use basic auth to get a jwt (but maybe not?). Maybe we can open a plan.io issue and worry about it later.

Also, I get a 401 response when using an outdated jwt token (which I expect) but also a weird "Error decoding signature" error in the response body:

$ http -v --auth-type jwt get :8000/api/v3/repositories/                                                                                                                              
GET /api/v3/repositories/ HTTP/1.1
Accept: */*
Accept-Encoding: gzip, deflate
Authorization: JWT eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJ1c2VybmFtZSI6ImFkbWluIiwiZXhwIjoxNTA2Mzc1NTE3fQ.JoS2KFN7rj22PSZJAjf7F1tIKF4RroJWhFLf7zvL7ro
Connection: keep-alive
Host: localhost:8000
User-Agent: HTTPie/0.9.9



HTTP/1.0 401 Unauthorized
Allow: GET, POST, HEAD, OPTIONS
Content-Length: 38
Content-Type: application/json
Date: Mon, 11 Sep 2017 21:56:45 GMT
Server: WSGIServer/0.2 CPython/3.5.4
Vary: Accept
WWW-Authenticate: JWT realm="api"
X-Frame-Options: SAMEORIGIN

{
    "detail": "Error decoding signature."
}

For basic auth when I use a bad username/password, I get something more expected:

{
    "detail": "Invalid username/password."
}

Other than that, everything seems to work well. 👍

fdobrovolny · 2017-09-15T15:42:10Z

@daviddavis It was in the original MVP, but then we decided not to include it to maximize the usage of the library vs rewriting it. I wasn't present the decision so I'm not exactly sure. You can read more on it here: https://pulp.plan.io/issues/2359#note-11

fdobrovolny · 2017-09-15T16:14:41Z

ok test

closes pulp#2359 https://pulp.plan.io/issues/2359

fdobrovolny · 2017-09-25T17:25:12Z

@daviddavis @bmbouter I changed the error message and even fixed a bug which I didn't know about (when a user that no longer exists try to login), But I have to add few things so if you could re-review it would be great.

daviddavis · 2017-10-02T20:30:56Z

Retested and it works 👍

Any clue on the docs failure?

bmbouter · 2017-10-03T15:25:59Z

The docs builder error is that the jwt library is not installed in the docs building environment. That environment is bootstrapped on each Jenkins slave here.

It looks like python3 is being used to build all docs environments. Adding jwt in as a dependency to install in that environment is probably what we need to do. The best thing to do is to have the setup.py build the environment so that we can stay DRY on these deps, but there are several issues preventing that from being done including:

That job runs both pulp2 and pulp3 jobs and only pulp3 has the setup.py that names its required Python deps
We would need the docs builder that builds nightly to build from source, but we would need the docs builder for PRs to build from the branch of the PR. Only that branch would have the updated setup.py change needed to have the docs builder pass.

For all of ^ reasons, I recommend:

Add and merge jwt as a dependency for all docs build environments here.
rebuild these docs by force pushing a meaningless change.
Merge this after docs builders are happy.

bmbouter · 2017-10-03T18:47:02Z

I posted a PR that should fix the doc builders here: pulp/pulp-ci#434

bmbouter · 2017-10-03T18:54:25Z

ok test

bmbouter · 2017-10-03T19:08:40Z

docs/contributing/platform_api/app/auth.rst

@@ -0,0 +1,9 @@
+pulp.app.auth


I think these need to be s/pulp/pulpcore/ right? Also the indention needs to line up too (iirc).

bmbouter · 2017-10-03T19:08:50Z

docs/contributing/platform_api/app/auth.rst

+
+.. automodule:: pulpcore.app.auth
+
+pulp.app.auth.jwt


Same comments here as above.

bmbouter · 2017-10-03T19:20:23Z

ok test

bmbouter · 2017-10-03T19:51:18Z

ok test

bmbouter · 2017-10-03T20:07:39Z

ok test

bmbouter · 2017-10-03T20:21:47Z

ok test

bmbouter · 2017-10-03T20:41:48Z

ok test

bmbouter · 2017-10-03T20:52:28Z

ok test

bmbouter · 2017-10-03T21:45:27Z

ok test

bmbouter · 2017-10-03T22:02:19Z

ok test

bmbouter · 2017-10-03T23:37:35Z

ok test

bmbouter · 2017-10-04T21:32:06Z

I applied the changes to pulp_packaging and pushed them to Jenkins with JJB. One more test will show if it all works for realz.

bmbouter · 2017-10-04T21:32:11Z

ok test

bmbouter · 2017-10-04T22:37:23Z

The latest test passed so this is good to merge. @BrnoPCmaniak I think this is good to merge. It's pretty sweet so consider identifying that it's merged via a short note to pulp-dev and linking to the nightly docs whenever they get built.

fdobrovolny added the 3.0 label Jul 27, 2017

fdobrovolny requested a review from bmbouter July 27, 2017 18:37

fdobrovolny force-pushed the JWT_token branch 5 times, most recently from 50110fe to e47ccc8 Compare July 27, 2017 19:53

bmbouter reviewed Jul 27, 2017

View reviewed changes

fdobrovolny force-pushed the JWT_token branch 4 times, most recently from 2e7f8a9 to b4339ca Compare July 27, 2017 20:51

fdobrovolny force-pushed the JWT_token branch from b4339ca to db6b2d6 Compare September 11, 2017 15:19

daviddavis reviewed Sep 11, 2017

View reviewed changes

Add JWT token auth

a7fe25d

closes pulp#2359 https://pulp.plan.io/issues/2359

fdobrovolny force-pushed the JWT_token branch from db6b2d6 to a7fe25d Compare September 25, 2017 16:40

bmbouter reviewed Oct 3, 2017

View reviewed changes

docs/contributing/platform_api/app/auth.rst

.. automodule:: pulpcore.app.auth

pulp.app.auth.jwt

Copy link

Member

bmbouter Oct 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comments here as above.

daviddavis approved these changes Oct 4, 2017

View reviewed changes

fdobrovolny merged commit 6f37fb6 into pulp:3.0-dev Oct 11, 2017

fdobrovolny deleted the JWT_token branch October 11, 2017 12:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add JWT token auth #3109

Add JWT token auth #3109

fdobrovolny commented Jul 27, 2017

pep8speaks commented Jul 27, 2017 •

edited

Loading

bmbouter Jul 27, 2017

bmbouter Jul 27, 2017

mikeadamz commented Aug 3, 2017

bmbouter commented Aug 5, 2017

fdobrovolny commented Sep 11, 2017

daviddavis Sep 11, 2017 •

edited

Loading

fdobrovolny Sep 15, 2017 •

edited

Loading

daviddavis commented Sep 11, 2017

fdobrovolny commented Sep 15, 2017

fdobrovolny commented Sep 15, 2017

fdobrovolny commented Sep 25, 2017

daviddavis commented Oct 2, 2017

bmbouter commented Oct 3, 2017

bmbouter commented Oct 3, 2017

bmbouter commented Oct 3, 2017

bmbouter Oct 3, 2017

bmbouter Oct 3, 2017

bmbouter commented Oct 3, 2017

bmbouter commented Oct 3, 2017

bmbouter commented Oct 3, 2017

bmbouter commented Oct 3, 2017

bmbouter commented Oct 3, 2017

bmbouter commented Oct 3, 2017

bmbouter commented Oct 3, 2017

bmbouter commented Oct 3, 2017

bmbouter commented Oct 3, 2017

bmbouter commented Oct 4, 2017

bmbouter commented Oct 4, 2017

bmbouter commented Oct 4, 2017

		from datetime import datetime, timedelta


		def generate_token_offline(username, jwt_secret, exp_delta=timedelta(days=14)):

Add JWT token auth #3109

Add JWT token auth #3109

Conversation

fdobrovolny commented Jul 27, 2017

pep8speaks commented Jul 27, 2017 • edited Loading

Comment last updated on September 25, 2017 at 16:40 Hours UTC

bmbouter Jul 27, 2017

Choose a reason for hiding this comment

bmbouter Jul 27, 2017

Choose a reason for hiding this comment

mikeadamz commented Aug 3, 2017

bmbouter commented Aug 5, 2017

fdobrovolny commented Sep 11, 2017

daviddavis Sep 11, 2017 • edited Loading

Choose a reason for hiding this comment

fdobrovolny Sep 15, 2017 • edited Loading

Choose a reason for hiding this comment

daviddavis commented Sep 11, 2017

fdobrovolny commented Sep 15, 2017

fdobrovolny commented Sep 15, 2017

fdobrovolny commented Sep 25, 2017

daviddavis commented Oct 2, 2017

bmbouter commented Oct 3, 2017

bmbouter commented Oct 3, 2017

bmbouter commented Oct 3, 2017

bmbouter Oct 3, 2017

Choose a reason for hiding this comment

bmbouter Oct 3, 2017

Choose a reason for hiding this comment

bmbouter commented Oct 3, 2017

bmbouter commented Oct 3, 2017

bmbouter commented Oct 3, 2017

bmbouter commented Oct 3, 2017

bmbouter commented Oct 3, 2017

bmbouter commented Oct 3, 2017

bmbouter commented Oct 3, 2017

bmbouter commented Oct 3, 2017

bmbouter commented Oct 3, 2017

bmbouter commented Oct 4, 2017

bmbouter commented Oct 4, 2017

bmbouter commented Oct 4, 2017

pep8speaks commented Jul 27, 2017 •

edited

Loading

daviddavis Sep 11, 2017 •

edited

Loading

fdobrovolny Sep 15, 2017 •

edited

Loading