Authentication backend get_userinfo incorrectly assumes the endpoint responds with application/json

mozilla / mozilla-django-oidc

A django OpenID Connect library

https://mozilla-django-oidc.readthedocs.io

Mozilla Public License 2.0

444 stars 166 forks source link

Authentication backend get_userinfo incorrectly assumes the endpoint responds with application/json #517

Open sergei-maertens opened 8 months ago

sergei-maertens commented 8 months ago

https://github.com/mozilla/mozilla-django-oidc/blob/bf0d1439da144daa061128a5c857af162b904cc7/mozilla_django_oidc/auth.py#L282

This line tries to decode the response content as JSON, which is one of the ways this endpoint may be implemented. The other way is that it responds with a JWT, having a Content-Type header of application/jwt;charset=utf-8 and that fails to decode as JSON, as the JWT itself needs to be processed.

Environment details:

OS: Arch Linux
Python: 3.10
Django: 3.2 LTS
mozilla-django-oidc: 3.0.0

How to reproduce:

Use an OIDC_OP_USER_ENDPOINT that responds with a JWT
Authenticate using OIDC
Observe JSONDecodeError crash

ttiikeri commented 8 months ago

From an OIDC specification point of view, this a good point from @sergei-maertens. Under the 5.3.2 Successful UserInfo Response heading, here's what said about the application/jwt header:

If the UserInfo Response is signed and/or encrypted, then the Claims are returned in a JWT and the content-type MUST be application/jwt.

sergei-maertens commented 7 months ago

@akatsoulas are you working on this? I've got an implementation in our downstream package that I'd prefer to see contributed back upstream, so I can draft a PR.

ivanov17 commented 7 months ago

I'm having the same problem now.

There also seem to be a few more reports about this:

The first of them was published almost four years ago.

@akatsoulas, can we hope that this will be fixed soon?

sergei-maertens commented 7 months ago

Awesome that you found these cross-references, I did search in the issue list but couldn't find anything.

Since I haven't heard back from @akatsoulas, I will just draft a PR with our implementation that can serve as a starting point, or at the least a way so that downstream packages can override the method in the backend to solve their own needs.

sergei-maertens commented 7 months ago

I have a draft PR #521 for this

lebaudantoine commented 2 months ago

Hey, @sergei-maertens are you still working on this? I have an implementation and would be super happy to offer it by the end of the week through a PR.

sergei-maertens commented 2 months ago

Well I have the attached PR asking for some review/input, so I'd say this is blocked by maintainer availability rather than by me