diff --git a/AUTHORS b/AUTHORS index 47a2aeaf2..507ef29fd 100644 --- a/AUTHORS +++ b/AUTHORS @@ -18,6 +18,7 @@ Allisson Azevedo Andrea Greco Andrej Zbín Andrew Chen Wang +Antoine Laurent Anvesh Agarwal Aristóbulo Meneses Aryan Iyappan diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c92aa849..0a135d2a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * #1211 documentation improve on 'AUTHORIZATION_CODE_EXPIRE_SECONDS'. * #1218 Confim support for Python 3.11. * #1222 Remove expired ID tokens alongside access tokens in `cleartokens` management command +* #1270 Fix RP-initiated Logout with no available Django session ## [2.2.0] 2022-10-18 diff --git a/oauth2_provider/views/oidc.py b/oauth2_provider/views/oidc.py index f819388b9..d7310c58b 100644 --- a/oauth2_provider/views/oidc.py +++ b/oauth2_provider/views/oidc.py @@ -210,13 +210,14 @@ def _validate_claims(request, claims): def validate_logout_request(request, id_token_hint, client_id, post_logout_redirect_uri): """ Validate an OIDC RP-Initiated Logout Request. - `(prompt_logout, (post_logout_redirect_uri, application))` is returned. + `(prompt_logout, (post_logout_redirect_uri, application), token_user)` is returned. `prompt_logout` indicates whether the logout has to be confirmed by the user. This happens if the specifications force a confirmation, or it is enabled by `OIDC_RP_INITIATED_LOGOUT_ALWAYS_PROMPT`. `post_logout_redirect_uri` is the validated URI where the User should be redirected to after the logout. Can be None. None will redirect to "/" of this app. If it is set `application` will also - be set to the Application that is requesting the logout. + be set to the Application that is requesting the logout. `token_user` is the id_token user, which will + used to revoke the tokens if found. The `id_token_hint` will be validated if given. If both `client_id` and `id_token_hint` are given they will be validated against each other. @@ -224,6 +225,7 @@ def validate_logout_request(request, id_token_hint, client_id, post_logout_redir id_token = None must_prompt_logout = True + token_user = None if id_token_hint: # Only basic validation has been done on the IDToken at this point. id_token, claims = _load_id_token(id_token_hint) @@ -231,6 +233,8 @@ def validate_logout_request(request, id_token_hint, client_id, post_logout_redir if not id_token or not _validate_claims(request, claims): raise InvalidIDTokenError() + token_user = id_token.user + if id_token.user == request.user: # A logout without user interaction (i.e. no prompt) is only allowed # if an ID Token is provided that matches the current user. @@ -268,7 +272,7 @@ def validate_logout_request(request, id_token_hint, client_id, post_logout_redir if not application.post_logout_redirect_uri_allowed(post_logout_redirect_uri): raise InvalidOIDCRedirectURIError("This client does not have this redirect uri registered.") - return prompt_logout, (post_logout_redirect_uri, application) + return prompt_logout, (post_logout_redirect_uri, application), token_user class RPInitiatedLogoutView(OIDCLogoutOnlyMixin, FormView): @@ -309,7 +313,7 @@ def get(self, request, *args, **kwargs): state = request.GET.get("state") try: - prompt, (redirect_uri, application) = validate_logout_request( + prompt, (redirect_uri, application), token_user = validate_logout_request( request=request, id_token_hint=id_token_hint, client_id=client_id, @@ -319,7 +323,7 @@ def get(self, request, *args, **kwargs): return self.error_response(error) if not prompt: - return self.do_logout(application, redirect_uri, state) + return self.do_logout(application, redirect_uri, state, token_user) self.oidc_data = { "id_token_hint": id_token_hint, @@ -341,7 +345,7 @@ def form_valid(self, form): state = form.cleaned_data.get("state") try: - prompt, (redirect_uri, application) = validate_logout_request( + prompt, (redirect_uri, application), token_user = validate_logout_request( request=self.request, id_token_hint=id_token_hint, client_id=client_id, @@ -349,20 +353,20 @@ def form_valid(self, form): ) if not prompt or form.cleaned_data.get("allow"): - return self.do_logout(application, redirect_uri, state) + return self.do_logout(application, redirect_uri, state, token_user) else: raise LogoutDenied() except OIDCError as error: return self.error_response(error) - def do_logout(self, application=None, post_logout_redirect_uri=None, state=None): + def do_logout(self, application=None, post_logout_redirect_uri=None, state=None, token_user=None): # Delete Access Tokens if oauth2_settings.OIDC_RP_INITIATED_LOGOUT_DELETE_TOKENS: AccessToken = get_access_token_model() RefreshToken = get_refresh_token_model() access_tokens_to_delete = AccessToken.objects.filter( - user=self.request.user, + user=token_user or self.request.user, application__client_type__in=self.token_deletion_client_types, application__authorization_grant_type__in=self.token_deletion_grant_types, ) diff --git a/tests/test_oidc_views.py b/tests/test_oidc_views.py index 6ba100d89..d1459a939 100644 --- a/tests/test_oidc_views.py +++ b/tests/test_oidc_views.py @@ -4,6 +4,7 @@ from django.test import RequestFactory, TestCase from django.urls import reverse from django.utils import timezone +from pytest_django.asserts import assertRedirects from oauth2_provider.exceptions import ClientIdMissmatch, InvalidOIDCClientError, InvalidOIDCRedirectURIError from oauth2_provider.models import get_access_token_model, get_id_token_model, get_refresh_token_model @@ -197,37 +198,37 @@ def test_validate_logout_request(oidc_tokens, public_application, other_user, rp id_token_hint=None, client_id=None, post_logout_redirect_uri=None, - ) == (True, (None, None)) + ) == (True, (None, None), None) assert validate_logout_request( request=mock_request_for(oidc_tokens.user), id_token_hint=None, client_id=client_id, post_logout_redirect_uri=None, - ) == (True, (None, application)) + ) == (True, (None, application), None) assert validate_logout_request( request=mock_request_for(oidc_tokens.user), id_token_hint=None, client_id=client_id, post_logout_redirect_uri="http://example.org", - ) == (True, ("http://example.org", application)) + ) == (True, ("http://example.org", application), None) assert validate_logout_request( request=mock_request_for(oidc_tokens.user), id_token_hint=id_token, client_id=None, post_logout_redirect_uri="http://example.org", - ) == (ALWAYS_PROMPT, ("http://example.org", application)) + ) == (ALWAYS_PROMPT, ("http://example.org", application), oidc_tokens.user) assert validate_logout_request( request=mock_request_for(other_user), id_token_hint=id_token, client_id=None, post_logout_redirect_uri="http://example.org", - ) == (True, ("http://example.org", application)) + ) == (True, ("http://example.org", application), oidc_tokens.user) assert validate_logout_request( request=mock_request_for(oidc_tokens.user), id_token_hint=id_token, client_id=client_id, post_logout_redirect_uri="http://example.org", - ) == (ALWAYS_PROMPT, ("http://example.org", application)) + ) == (ALWAYS_PROMPT, ("http://example.org", application), oidc_tokens.user) with pytest.raises(ClientIdMissmatch): validate_logout_request( request=mock_request_for(oidc_tokens.user), @@ -519,6 +520,47 @@ def test_token_deletion_on_logout(oidc_tokens, loggend_in_client, rp_settings): assert all([token.revoked <= timezone.now() for token in RefreshToken.objects.all()]) +@pytest.mark.django_db +def test_token_deletion_on_logout_expired_session(oidc_tokens, client, rp_settings): + AccessToken = get_access_token_model() + IDToken = get_id_token_model() + RefreshToken = get_refresh_token_model() + assert AccessToken.objects.count() == 1 + assert IDToken.objects.count() == 1 + assert RefreshToken.objects.count() == 1 + rsp = client.get( + reverse("oauth2_provider:rp-initiated-logout"), + data={ + "id_token_hint": oidc_tokens.id_token, + "client_id": oidc_tokens.application.client_id, + }, + ) + assert rsp.status_code == 200 + assert not is_logged_in(client) + # Check that all tokens are active. + access_token = AccessToken.objects.get() + assert not access_token.is_expired() + id_token = IDToken.objects.get() + assert not id_token.is_expired() + refresh_token = RefreshToken.objects.get() + assert refresh_token.revoked is None + + rsp = client.post( + reverse("oauth2_provider:rp-initiated-logout"), + data={ + "id_token_hint": oidc_tokens.id_token, + "client_id": oidc_tokens.application.client_id, + "allow": True, + }, + ) + assertRedirects(rsp, "http://testserver/", fetch_redirect_response=False) + assert not is_logged_in(client) + # Check that all tokens have either been deleted or expired. + assert all(token.is_expired() for token in AccessToken.objects.all()) + assert all(token.is_expired() for token in IDToken.objects.all()) + assert all(token.revoked <= timezone.now() for token in RefreshToken.objects.all()) + + @pytest.mark.django_db @pytest.mark.oauth2_settings(presets.OIDC_SETTINGS_RP_LOGOUT_KEEP_TOKENS) def test_token_deletion_on_logout_disabled(oidc_tokens, loggend_in_client, rp_settings):