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

[Security] SessionRefresh is not handling POST requests #504

Open maartendekeizer opened 1 year ago

maartendekeizer commented 1 year ago

We have a situation with a page that contains a form. The page is secured by authentication via the Mozilla Django OIDC backend. When the form is submitted (POST) the data is saved and the user is redirected to another page (GET). We have the SessionRefresh middleware active.

We find out that when the user have the page open for a long time (longer than the expiry data of the JWT token) the user is still able to do the form submission (POST request), the request is processed as normal and the user is NOT forced to refresh the JWT token. When the user is making a GET request (for example when redirected after the POST request) the middleware will correctly forced the user to reauthenticate against the OIDC service.

I found out that this behaviour is explicitly made here: https://github.com/mozilla/mozilla-django-oidc/blob/71e4af8283a10aa51234de705d34cd298e927f97/mozilla_django_oidc/middleware.py#L111

I can understand why they made it only works on GET request (loosing POST data after redirect to OIDC service). But this exception is also a big security risk. The most dangerous calls (data modification) can be made via a POST request and the user is still able to do this even the JWT token is expired. This is also not limited to one POST request.

In the documentation it is mentioned why it is important to renew the JWT token. This strange and insecure behaviour is NOT documented. https://mozilla-django-oidc.readthedocs.io/en/stable/installation.html#validate-id-tokens-by-renewing-them

I my opinion this exception for POST request should be removed/disabled.

I can imagine that we made this configurable for legacy support, so this behaviour can be activated. This should be done together with clear documentation about the consequences.

Addition: Also other HTTP-verbs like PUT, PATCH, HEAD and DELETE are vulnerable to this exception