Repository navigation
Vaild OIDC redirect_uri from site root in path mode - #5555
Conversation
Journals served in path mode set a script prefix of /<code>, which
leaked into every absolutify(request, reverse("oidc_authentication_callback"))
in mozilla-django-oidc. Identity providers exact-match redirect_uri
against registered callbacks, so logins from journal pages failed
(Entra AADSTS50011) and silent session refresh looped (AADSTS50196).
- Add JanewayOIDCAuthenticationRequestView, set as OIDC_AUTHENTICATE_CLASS,
which resets the script prefix around the authorize request and, when
no next url is supplied, returns users to the site they started on.
- Add opt-in JanewaySessionRefresh middleware which resets the prefix
around the prompt=none re-auth redirect and also exempts the OIDC
endpoints when reached under a journal prefix (the stock check uses
request.path and caches exempt urls under the first request's prefix).
- Add regression tests driving both paths through real requests, and a
query_parameter_from_url / login_with_expired_oidc_session test helper.
Closes #5509
ajrbyers
changed the title
mauromsl
left a comment
mauromsl
left a comment
There was a problem hiding this comment.
Please, reconsider redirecting OIDC flow through the press, with a redirect to the user location at the end over monkey patching global request status (script prefix)
| # giving /<journal_code>/ in path mode and / at press level. | ||
| default_next = reverse("website_index") | ||
| prefix = get_script_prefix() | ||
| set_script_prefix("/") | ||
| try: | ||
| response = super().get(request) | ||
| finally: | ||
| set_script_prefix(prefix) | ||
|
|
||
| if not request.session.get("oidc_login_next"): | ||
| request.session["oidc_login_next"] = default_next | ||
|
|
||
| return response |
There was a problem hiding this comment.
This monkeypatch is far from ideal. Can't we instead always redirect to the press OIDC view, and use a querystring parameter to redirect the user to where they came from? This is basically what we are pretending to do when delegating to OIDCAuthenticationRequestView via super.get() but withot control over any side effects of the mutation done via set_script_prefix("/")
There was a problem hiding this comment.
Agreed, reworked in 0adbd38. Path-mode requests now redirect to the press-level /oidc/authenticate/ with ?next= set to the page the user started on, and the press-level view just calls super().get(). set_script_prefix is gone. One caveat: domain-mode sites keep their own session cookie, so they still start the login on their own domain.
| def process_request(self, request): | ||
| prefix = get_script_prefix() | ||
| set_script_prefix("/") | ||
| try: | ||
| return super().process_request(request) | ||
| finally: | ||
| set_script_prefix(prefix) | ||
|
|
There was a problem hiding this comment.
Same commit: JanewaySessionRefresh is removed, so nothing changes the script prefix any more. Silent session refresh under a path prefix can be a follow-up if we need it.
2 participants
Closes #5509