jazzband / django-oauth-toolkit

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

Add the possibility to map user info with user model field. #1325

Open fijemax opened 1 year ago

fijemax commented 1 year ago

Hello,

I would like to know if it would interesting to let possibility to fill the UserModel with some user information just after the authentication server. This could be a custom map.

Indeed the keycloack întrospection (and all standard oauth2 introspection in general) return many informations about the user than It would be nice to map here.

More specificly, in my case the USERNAME_FIELD is map to the uuid field of my user model and this feature will permit to me to not add a pre-save traietment to the save of my model to prevent null username field.

The line concerned: https://github.com/jazzband/django-oauth-toolkit/blob/9aa27c7528cdeda0b85bac5a8a00b39d696a43f9/oauth2_provider/oauth2_validators.py#L387C1-L387C21

n2ygk commented 1 year ago

This sounds like you want OIDC claims: https://django-oauth-toolkit.readthedocs.io/en/latest/oidc.html#customizing-the-oidc-responses

fijemax commented 1 year ago

Not really, this seems to be hardcoded behavior. The username is taken directly from the introspection response, for the moment I redirect the user uuid keycloack to the username but it would be nice if the username could be overided (line 387). image

What do you think about that ? @dopry

dopry commented 1 year ago

you want to change the response of the introspection endpoint, not the data in the claims?

fijemax commented 1 year ago

I want to change the key username used to retrive the data to fill the UserModel. In my case my UserModel.USERNAME_FIELD is an uuid field and "username" value of content is a string.

fijemax commented 1 year ago

I made a PR, what do you think about this one https://github.com/jazzband/django-oauth-toolkit/pull/1347

dopry commented 1 year ago

The specification for the [introspection endpoint](per the introspection https://www.rfc-editor.org/rfc/rfc7662.html#section-2.2)

username OPTIONAL. Human-readable identifier for the resource owner who authorized this token.

the use of a non-human readable ID in the username is not within the intent of the specification.

per

Specific implementations MAY extend this structure with their own service-specific response names as top-level members of this JSON object. Response names intended to be used across domains MUST be registered in the "OAuth Token Introspection Response" registry defined in Section 3.1.

if you need another id in your implementation it should be added as another top level field. If I am not mistaken additional claims would be

if you're just having a validation issue with a blank username, you should probably update your UserModel to allow a blank username. Although I believe username is not the proper value to check to see if a user is present. sub is probably a more apporpriate field to check, since we can't have a user without an id.

fijemax commented 1 year ago

Hello @dopry, thanks for your answer, In my case I want to use the sub field of the introspection answer instead username to get or create my django UserModel. I removed the username of my django User model and I have an uuid field instead, so my UserModel.USERNAME_FIELD correspond to this uuid field. I use a keycloack as an oauth server and I get the keycloack user's uuid in the sub field so that's why I want to map the sub introspection field into the uuid of my django UserModel. Today keycloack allows me to overwrite the scope by mapping uuid (sub) into the username but it's not really clean and I lose the real username in my scope.

fijemax commented 6 months ago

Yeah exactly. I coded it for my own needs in waiting an answer. https://github.com/jazzband/django-oauth-toolkit/issues/1325#issuecomment-1773776771

n2ygk commented 6 months ago

@fijemax D'oh! I got confused.

fijemax commented 6 months ago

Sorry I'm overwhelmed right now. But I noted it on my todo list.