search

jazzband / django-oauth-toolkit

OAuth2 goodies for the Djangonauts!
https://django-oauth-toolkit.readthedocs.io
Other
3.06k stars 777 forks source link

Added oauth2_settings.RESOURCE_SERVER_INTROSPECTION_RESPONSE_FIELD setting to let po… #1347

Closed fijemax closed 1 month ago

fijemax commented 8 months ago

Fixes #1325

Description of the Change

Checklist

codecov[bot] commented 8 months ago

Codecov Report

Merging #1347 (bef53ce) into master (4c13679) will not change coverage. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1347   +/-   ##
=======================================
  Coverage   97.54%   97.54%           
=======================================
  Files          32       32           
  Lines        2120     2120           
=======================================
  Hits         2068     2068           
  Misses         52       52
Files Coverage Δ
oauth2_provider/oauth2_validators.py 94.09% <100.00%> (ø)
oauth2_provider/settings.py 100.00% <ø> (ø)

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

dopry commented 8 months ago

@fijemax, I think I see your need here. The user is being fetched or created here. The current implementation uses username as the key field. Username may not be set as it's an optional in the response and frankly we don't know the required fields on someones usermodel so this approach is a bit naieve.

I think this approach might work for your use case, but at the moment I don't think I have enough context to be confident this is a good fix that will work for the wider community. I'm gonna do some digging around this method so I better understand how it's used. Can you please provide more detail on your use case in your original issue to provide more context?

@n2ygk, do you have thoughts?

n2ygk commented 8 months ago

@dopry @fijemax I have not been following this issue. Isn't getting the sub or other added claims described here? Or here for userinfo.

n2ygk commented 1 month ago

Isn't this accomplished by #1325?

fijemax commented 1 month ago

Hello, What do you mean ?

n2ygk commented 1 month ago

Hello, What do you mean ?

@fijemax See #1325 which allows setting the username field value. This looks to me to address the same issue you are addressing here.

n2ygk commented 1 month ago

@fijemax please edit the PR description. I've initialized it with the PR template.

n2ygk commented 1 month ago

I believe this feature is resolved by #1325