Openid Connect Core support #545

wiliamsouza · 2018-01-18T14:07:54Z

This PR add OpenID Connect Core http://openid.net/specs/openid-connect-core-1_0.html support.

To run the tests do:

Django OAuth Toolkit cloned repository

git remote add git@github.com:wiliamsouza/django-oauth-toolkit.git wiliamsouza
git checkout -b openid-connect wiliamsouza/openid-connect
tox

Usage examples:

Authorization-code

http://openid.net/specs/openid-connect-core-1_0.html#CodeFlowAuth

token and id_token

http://127.0.0.1:8000/o/authorize?response_type=code&client_id=HCSTniKnHNj6qP6Dkms39q6IT4pahzfXws2uwTkS&redirect_uri=http://localhost/callback&scope=openid%20read&state=af0ifjsldkj&nonce=n-0S6_WzA2Mj

Only token

http://127.0.0.1:8000/o/authorize?response_type=code&client_id=HCSTniKnHNj6qP6Dkms39q6IT4pahzfXws2uwTkS&redirect_uri=http://localhost/callback&scope=read&state=af0ifjsldkj&nonce=n-0S6_WzA2Mj

curl -X POST \
    -H "Cache-Control: no-cache" \
    -H "Content-Type: application/x-www-form-urlencoded" \
    "http://127.0.0.1:8000/o/token/" \
    -d "client_id=HCSTniKnHNj6qP6Dkms39q6IT4pahzfXws2uwTkS" \
    -d "client_secret=Ay1rH78PChsOG4mshfdp2oJnomdpu5Vgtdz6jCmDkEM8mKHzcaKo5GEYNGK42KTN8XqEWbbpn1vdHJKYcBgawONx4S1xXY7GtEP9mvsMw593DeXH0aRpWlgySuxeDfe2" \
    -d "code=89BRsh4kQqrwlNMLj4coVZbrpDm0Mh" \
    -d "redirect_uri=http://localhost/callback" \
    -d "grant_type=authorization_code"

implicit

http://openid.net/specs/openid-connect-core-1_0.html#ImplicitFlowAuth

token

http://127.0.0.1:8000/o/authorize?response_type=token&client_id=Ktn0Sh4hO2gA8PKC2aqsauY4ZCxNyIdF1wNfFfJ3&redirect_uri=http://localhost/callback&scope=openid%20read&state=af0ifjsldkj&nonce=n-0S6_WzA2Mj

id_token

http://127.0.0.1:8000/o/authorize?response_type=id_token&client_id=Ktn0Sh4hO2gA8PKC2aqsauY4ZCxNyIdF1wNfFfJ3&redirect_uri=http://localhost/callback&scope=openid%20read&state=af0ifjsldkj&nonce=n-0S6_WzA2Mj

token and id_token

http://127.0.0.1:8000/o/authorize?response_type=id_token%20token&client_id=Ktn0Sh4hO2gA8PKC2aqsauY4ZCxNyIdF1wNfFfJ3&redirect_uri=http://localhost/callback&scope=openid%20read&state=af0ifjsldkj&nonce=n-0S6_WzA2Mj

openid-hybrid

http://openid.net/specs/openid-connect-core-1_0.html#HybridFlowAuth

code, id_token and token

http://127.0.0.1:8000/o/authorize?response_type=code%20id_token%20token&client_id=fZjUvm0YABo6mX1UTjo9VVYay9zj1HpaCZaoa4Fj&redirect_uri=http://localhost/callback&scope=openid%20read&state=af0ifjsldkj&nonce=n-0S6_WzA2Mj

code and id_token

http://127.0.0.1:8000/o/authorize?response_type=code%20id_token&client_id=fZjUvm0YABo6mX1UTjo9VVYay9zj1HpaCZaoa4Fj&redirect_uri=http://localhost/callback&scope=openid%20read&state=af0ifjsldkj&nonce=n-0S6_WzA2Mj

code and token

http://127.0.0.1:8000/o/authorize?response_type=code%20token&client_id=fZjUvm0YABo6mX1UTjo9VVYay9zj1HpaCZaoa4Fj&redirect_uri=http://localhost/callback&scope=openid%20read&state=af0ifjsldkj&nonce=n-0S6_WzA2Mj

code

http://127.0.0.1:8000/o/authorize?response_type=code&client_id=fZjUvm0YABo6mX1UTjo9VVYay9zj1HpaCZaoa4Fj&redirect_uri=http://localhost/callback&scope=openid%20read&state=af0ifjsldkj&nonce=n-0S6_WzA2Mj

curl -X POST \
    -H "Cache-Control: no-cache" \
    -H "Content-Type: application/x-www-form-urlencoded" \
    "http://127.0.0.1:8000/o/token/" \
    -d "client_id=fZjUvm0YABo6mX1UTjo9VVYay9zj1HpaCZaoa4Fj" \
    -d "client_secret=eiBw6IIQ4zzWFP9gSszEHZwexjCDhJjMtRxoOYQexCJMjQ6gdyN2ME9aUzbGkVopx3NSZRPUb4SV9yKpbVwwW9NKdEpkoyGmcTni7G4KTPqtJrsI9HPubFnDDzsvAf89" \
    -d "code=VQGv7tzYGO13jrRnv6oYT7jHbDSGJZ" \
    -d "redirect_uri=http://localhost/callback" \
    -d "grant_type=authorization_code"

TODO:

Create/Update docs
Merge OAuthLib PR Openid connect jwt oauthlib/oauthlib#488

coveralls · 2018-01-18T14:13:13Z

Coverage decreased (-4.7%) to 88.696% when pulling b2847e2 on wiliamsouza:openid-connect into 231efff on evonove:master.

wiliamsouza · 2018-01-18T14:17:28Z

The CI will be broke until OAuthLib version containing this PR oauthlib/oauthlib#488

jleclanche · 2018-01-24T12:19:37Z

Hi @wiliamsouza

Thanks, this is a high quality PR. I'm not merging it until the upstream work is fully released, but once it is, feel free to ping here again and I'll do a full review pass.

oauth2_provider/views/base.py

wiliamsouza · 2018-02-03T18:29:52Z

OAuthLib upcoming release 3.0.0 oauthlib/oauthlib#512

oauth2_provider/oauth2_validators.py

rhenter · 2018-03-21T14:41:11Z

Great PR. It will be very good when It wore merged

afabiani · 2018-05-31T11:02:15Z

Hi all,
how far we are on merging this?

JonathanHuot · 2018-10-04T13:03:16Z

@jleclanche, oauthlib's PR oauthlib/oauthlib#488 has been merged into oauthlib's master branch. Would be great to validate the changes here before releasing the oauthlib@3.0.0

wiliamsouza · 2018-10-04T16:23:38Z

@afabiani As soon as oauthlib@3.0.0 get released

jleclanche · 2018-10-04T16:26:05Z

@JonathanHuot Winter is pretty busy here, I'm not sure I'll soon be in a situation where i can do an updated review. And either way, I wouldn't want to land this without testing it (and I'm not in a position to test it).

So if people want to get this PR moving ahead of oauthlib 3.0 release, bring it into your installations and try it out, report back with any issues.

wiliamsouza · 2018-10-04T16:32:09Z

@jleclanche @JonathanHuot I can manage this

afabiani · 2018-10-05T08:05:04Z

@wiliamsouza currently I had to fork this one [1] since it required few more changes in order to fully work with OIDC 1.0 (tested mainly using Keycloak). Will look forward to the new release anyway. Thanks.

[1] - https://github.com/GeoNode/geonode-oauth-toolkit

jejenone · 2018-12-03T22:12:57Z

I would love to see OIDC support in DOT. Looking forward to seeing this merged.

bpereto · 2019-01-09T09:14:30Z

@wiliamsouza oauthlib@3.0.0 is released 🎉

tribals · 2019-01-11T13:06:17Z

@wiliamsouza

I created dummy Django app with this library integrated, seems like the patch is currently doesn't work.

Here is the app: https://dummy-idp-dot.herokuapp.com. I created an OAuth app, you can view it's credentials here: https://dummy-idp-dot.herokuapp.com/oauth/applications/1. I tested it with OpenID Connect debugger: https://oidcdebugger.com. Currently there is an unsupported_response_type error. Here is the request that Debugger was generated:

https://dummy-idp-dot.herokuapp.com/oauth/authorize?client_id=P25mgGSjKjx77Dw4zaiUhhJMWqYgtIVQiogSpATz&redirect_uri=https%3A%2F%2Foidcdebugger.com%2Fdebug&scope=openid%20read&response_type=token%20id_token&response_mode=form_post&nonce=lokuqbmajz

wiliamsouza · 2019-01-15T03:33:10Z

Working on it

coveralls · 2019-01-16T14:09:39Z

Pull Request Test Coverage Report for Build 1345

193 of 222 (86.94%) changed or added relevant lines in 11 files are covered.
3 unchanged lines in 2 files lost coverage.
Overall coverage decreased (-1.05%) to 93.721%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
oauth2_provider/settings.py	5	7	71.43%
oauth2_provider/views/oidc.py	25	28	89.29%
oauth2_provider/oauth2_validators.py	103	110	93.64%
oauth2_provider/models.py	31	48	64.58%

Files with Coverage Reduction	New Missed Lines	%
oauth2_provider/settings.py	1	76.56%
oauth2_provider/oauth2_validators.py	2	91.33%

Totals
Change from base Build 1341:	-1.05%
Covered Lines:	1418
Relevant Lines:	1513

💛 - Coveralls

wiliamsouza · 2019-01-16T14:18:23Z

@afabiani Great! Can you explain the changes and how do you test it with Keycloak?

wiliamsouza · 2019-01-16T14:20:00Z

@jejenone You can help it get done reviewing and testing this branch.

wiliamsouza · 2019-01-16T14:21:17Z

@tribals Can you update the code and redo the test?

tribals · 2019-01-17T07:47:12Z

@wiliamsouza Finally I'm testing in local environment. It seems to mostly work. I tried implicit flow with response_type parameter set to both token and id_token token. Server responds correctly. There are still some misunderstandings about how to hook into this library but they disappear as the more I work with library.

auvipy

please re-base to master while updating.

auvipy

it's better to pull updates from master and change models based on latest master, and then regenerate migrations based on the migrations in masters branch.

oauth2_provider/migrations/0007_authorization_grant_type.py

oauth2_provider/migrations/0008_application_algorithm.py

oauth2_provider/migrations/0009_idtoken.py

oauth2_provider/migrations/0010_accesstoken_id_token.py

wiliamsouza · 2019-02-21T18:19:36Z

@auvipy Your merge broke the build. Why did you do that before any discussion?

wiliamsouza · 2020-03-02T03:58:07Z

@tribals @jleclanche @bpereto @auvipy @fvlima @jpbonson @JC127 @esseti @kaeruko @JonathanHuot @jejenone @afabiani Ready to merge? Review, comment! I can add access to my fork to anyone that want to help. Just ask.

auvipy · 2020-03-02T10:31:13Z

tests are failing. you could add me to your branch

n2ygk · 2020-03-02T13:57:30Z

This is a big improvement and a lot of new code. I think we should hold off merging this until after the 1.3.0 release which I'm hoping to get out today or tomorrow. Given the significant new functionality, perhaps this becomes a 1.4.0 or 2.0 release. What are your thoughts on whether this is a major or minor release per semver?

And, of course, failing tests have to be resolved. They are mostly isort/flake8 issues because I undid the disabling of those tests in #798. They should be really easy to fix:

isort <filename> which will sort the includes properly
make sure to use inline double-quotes. (I just did this doing global replaces of ' with " and a few manual fixes)
comply with line-length limit
a few other items

wiliamsouza · 2020-03-02T14:43:17Z

tests are failing.

I will fix tests.

you could add me to your branch

Done! Welcome.

wiliamsouza · 2020-03-02T14:48:32Z

This is a big improvement and a lot of new code. I think we should hold off merging this until after the 1.3.0 release which I'm hoping to get out today or tomorrow.

Great!

Given the significant new functionality, perhaps this becomes a 1.4.0 or 2.0 release. What are your thoughts on whether this is a major or minor release per semver?

1.4.0 is ok.

n2ygk · 2020-03-02T14:55:55Z

tox.ini

@@ -33,6 +34,7 @@ deps =
 	pytest-xdist
 	py27: mock
 	requests
+	jwcrypto

 [testenv:py37-docs]
 basepython = python


I believe you need to add jwcrypto to deps here as well to resolve tox -e py37-docs failure.

And, of course, failing tests have to be resolved. They are mostly isort/flake8 issues because I undid the disabling of those tests in #798. They should be really easy to fix:

Those things were fixed here wiliamsouza#8. After merging this, only documentation and examples items (#545 (comment)) will remain to merge this PR

wiliamsouza · 2020-03-02T14:57:07Z

Checklist

1) Fix flake8 test
2) Fix docs test
3) Ensure coverage is higher
4) Write docs about OpenID Connect
5) Write a example application with OpenID connect flows

I think this the job that must be done.
Will take the 5 one.

fvlima · 2020-03-03T02:29:04Z

I can fix the 1 and 2 items. And about item 3, is it really necessary to put the oauth2_provider/settings.py to be covered?

wiliamsouza · 2020-03-03T20:17:29Z

I can fix the 1 and 2 items. And about item 3, is it really necessary to put the oauth2_provider/settings.py to be covered?

It seems to have some logic.

esseti · 2020-03-23T10:26:25Z

sorry to be late at the party, what/how can I help?

wiliamsouza · 2020-03-30T18:57:00Z

sorry to be late at the party, what/how can I help?

Can you handle any item from the checklist?

auvipy · 2020-03-31T14:42:11Z

you can start with rebase

fvlima · 2020-04-26T22:16:15Z

The items 1 and 2 from the checklist were fixed here wiliamsouza#8

fvlima · 2020-04-27T00:44:40Z

And about item 3, I made this comment #545 (comment) and created this PR to do it here wiliamsouza#9. This makes sense for me, but if not, just ignore it.

Add project settings to be ignored in coverage

Fix flake8 and test doc issues

michaelhelvey · 2020-06-23T18:56:53Z

oauth2_provider/models.py

+        settings.AUTH_USER_MODEL, on_delete=models.CASCADE, blank=True, null=True,
+        related_name="%(app_label)s_%(class)s"
+    )
+    token = models.TextField(unique=True)


Testing locally using MySQL the migration created by this errors with MySQL err 1170, "BLOB/TEXT column 'token' used in key specification without a key length".

Reference: https://dev.mysql.com/doc/refman/8.0/en/server-error-reference.html#error_er_blob_key_without_length

Given that this is not the primary key and (if my understanding is correct) typically a value generated by the application, perhaps we could not use a database index to ensure uniqueness? Happy to make a PR if so.

see wiliamsouza#10

fvlima · 2020-09-08T14:52:54Z

I think that this PR can be closed because we already have this implementation merged here #859

wiliamsouza mentioned this pull request Jan 18, 2018

OpenID Connect #110

Closed

jleclanche reviewed Jan 24, 2018

View reviewed changes

oauth2_provider/views/base.py Outdated Show resolved Hide resolved

wiliamsouza mentioned this pull request Jan 29, 2018

Openid connect jwt oauthlib/oauthlib#488

Merged

rhenter reviewed Mar 21, 2018

View reviewed changes

oauth2_provider/oauth2_validators.py Outdated Show resolved Hide resolved

Kurocon mentioned this pull request Apr 18, 2018

OAuth login Wiki keeps asking for permission Inter-Actief/amelie#325

Closed

wiliamsouza force-pushed the openid-connect branch from 266ff15 to 0d3904a Compare January 16, 2019 14:05

wiliamsouza mentioned this pull request Jan 18, 2019

[WIP] adding support for OIDC, rebase to current master of django-oauth-toolkit wiliamsouza/django-oauth-toolkit#2

Closed

auvipy suggested changes Feb 21, 2019

View reviewed changes

fvlima and others added 2 commits March 2, 2020 00:17

Add missing new attributes to test migration

4e1f07b

Rebase fixing conflicts and tests

df1a154

wiliamsouza force-pushed the openid-connect branch from 2e013c3 to df1a154 Compare March 2, 2020 03:30

Remove auto generate message

5b05ce0

n2ygk added this to the 1.4 milestone Mar 2, 2020

n2ygk requested changes Mar 2, 2020

View reviewed changes

Merge branch 'master' into openid-connect

368e03a

Merge branch 'master' into openid-connect

3b2b2c1

Fix flake8 issues

ef161c0

fvlima added 2 commits April 26, 2020 19:45

Fix test doc deps

723d37a

Add project settings to be ignored in coverage

e531284

auvipy self-assigned this May 8, 2020

Asif Saif Uddin added 2 commits May 9, 2020 01:30

Merge pull request #9 from fvlima/coverage-omit-settings

5680ca4

Add project settings to be ignored in coverage

Merge pull request #8 from fvlima/fixes-flake-issues

6e71b66

Fix flake8 and test doc issues

michaelhelvey reviewed Jun 23, 2020

View reviewed changes

auvipy closed this Sep 9, 2020

Openid Connect Core support #545

Openid Connect Core support #545

Conversation

wiliamsouza commented Jan 18, 2018 • edited

Authorization-code

token and id_token

Only token

implicit

token

id_token

token and id_token

openid-hybrid

code, id_token and token

code and id_token

code and token

code

coveralls commented Jan 18, 2018 • edited

wiliamsouza commented Jan 18, 2018

jleclanche commented Jan 24, 2018

wiliamsouza commented Feb 3, 2018

rhenter commented Mar 21, 2018

afabiani commented May 31, 2018

JonathanHuot commented Oct 4, 2018

wiliamsouza commented Oct 4, 2018

jleclanche commented Oct 4, 2018

wiliamsouza commented Oct 4, 2018

afabiani commented Oct 5, 2018

jejenone commented Dec 3, 2018

bpereto commented Jan 9, 2019

tribals commented Jan 11, 2019 • edited

wiliamsouza commented Jan 15, 2019

coveralls commented Jan 16, 2019 • edited

Pull Request Test Coverage Report for Build 1345

💛 - Coveralls

wiliamsouza commented Jan 16, 2019

wiliamsouza commented Jan 16, 2019

wiliamsouza commented Jan 16, 2019

tribals commented Jan 17, 2019 • edited

auvipy left a comment

Choose a reason for hiding this comment

auvipy left a comment

Choose a reason for hiding this comment

wiliamsouza commented Feb 21, 2019

wiliamsouza commented Mar 2, 2020

auvipy commented Mar 2, 2020

n2ygk commented Mar 2, 2020

wiliamsouza commented Mar 2, 2020 • edited

wiliamsouza commented Mar 2, 2020 • edited

n2ygk Mar 2, 2020

Choose a reason for hiding this comment

fvlima Apr 27, 2020 • edited

Choose a reason for hiding this comment

wiliamsouza commented Mar 2, 2020

Checklist

fvlima commented Mar 3, 2020

wiliamsouza commented Mar 3, 2020 • edited

esseti commented Mar 23, 2020

wiliamsouza commented Mar 30, 2020

auvipy commented Mar 31, 2020

fvlima commented Apr 26, 2020 • edited

fvlima commented Apr 27, 2020 • edited

michaelhelvey Jun 23, 2020

Choose a reason for hiding this comment

auvipy Jun 23, 2020

Choose a reason for hiding this comment

michaelhelvey Jun 24, 2020

Choose a reason for hiding this comment

fvlima commented Sep 8, 2020

wiliamsouza commented Jan 18, 2018 •

edited

coveralls commented Jan 18, 2018 •

edited

tribals commented Jan 11, 2019 •

edited

coveralls commented Jan 16, 2019 •

edited

tribals commented Jan 17, 2019 •

edited

wiliamsouza commented Mar 2, 2020 •

edited

wiliamsouza commented Mar 2, 2020 •

edited

fvlima Apr 27, 2020 •

edited

wiliamsouza commented Mar 3, 2020 •

edited

fvlima commented Apr 26, 2020 •

edited

fvlima commented Apr 27, 2020 •

edited