aliev / aioauth

Asynchronous OAuth 2.0 provider for Python 3
https://aliev.me/aioauth
MIT License
214 stars 19 forks source link

OIDC token response to authorization_code grant type should always include an id_token #76

Closed tdg5 closed 1 year ago

tdg5 commented 1 year ago

This creates an OIDC specific version of the AuthorizationCodeGrantType that always includes an id_token. The scope is narrower than I thought previously, and it seems like id_token is really only required when getting a token via authorization_code. There are some indications that the password grant type could also include an id token, but so far it seems to vary by vendor.

codecov-commenter commented 1 year ago

Codecov Report

Merging #76 (7d1180d) into master (266fc9e) will decrease coverage by 3.47%. The diff coverage is 7.14%.

:mega: This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##           master      #76      +/-   ##
==========================================
- Coverage   97.87%   94.41%   -3.47%     
==========================================
  Files          15       17       +2     
  Lines         707      734      +27     
  Branches      110      114       +4     
==========================================
+ Hits          692      693       +1     
- Misses          6       32      +26     
  Partials        9        9              
Impacted Files Coverage Δ
aioauth/oidc/core/grant_type.py 0.00% <0.00%> (ø)
aioauth/oidc/core/responses.py 0.00% <0.00%> (ø)
aioauth/response_type.py 100.00% <ø> (ø)
aioauth/storage.py 100.00% <ø> (ø)
aioauth/oidc/core/requests.py 100.00% <100.00%> (ø)

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

tdg5 commented 1 year ago

I've converted this PR to a draft because I want to update it based on what I've proposed in https://github.com/aliev/aioauth/pull/75.

tdg5 commented 1 year ago

@aliev , though you should take a look at https://github.com/aliev/aioauth/pull/75 first since this PR depends on it, I think this PR is ready for review again.

tdg5 commented 1 year ago

I lean toward handling it in another PR, but I ran into a scenario today where it seems like the password grant should also have an option of returning an id_token.

tdg5 commented 1 year ago

@aliev, sorry for the delay in getting back to your comments. I've made some updates based on your comments and think this PR should be ready for you to take another look whenever it is convenient for you.

tdg5 commented 1 year ago

@aliev thanks for merging this. Sorry I didn't address your comments; I've had a busy few weeks. I will try to find some time this week to address any comments you haven't already addressed here.

aliev commented 1 year ago

@tdg5 no worries at all. thank you!