diff --git a/CHANGELOG.md b/CHANGELOG.md index 38b1d8b7..f1631726 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,12 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [unreleased] ### Added -* Support for Wildcard Origin and Redirect URIs, https://github.com/jazzband/django-oauth-toolkit/issues/1506 +* #1506 Support for Wildcard Origin and Redirect URIs ### Fixed +* #1517 OP prompts for logout when no OP session +* #1512 client_secret not marked sensitive + diff --git a/oauth2_provider/views/oidc.py b/oauth2_provider/views/oidc.py index c746c30c..a252f1be 100644 --- a/oauth2_provider/views/oidc.py +++ b/oauth2_provider/views/oidc.py @@ -367,17 +367,66 @@ def validate_logout_request(self, id_token_hint, client_id, post_logout_redirect return application, id_token.user if id_token else None def must_prompt(self, token_user): - """Indicate 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`. + """ + per: https://openid.net/specs/openid-connect-rpinitiated-1_0.html + + > At the Logout Endpoint, the OP SHOULD ask the End-User whether to log + > out of the OP as well. Furthermore, the OP MUST ask the End-User this + > question if an id_token_hint was not provided or if the supplied ID + > Token does not belong to the current OP session with the RP and/or + > currently logged in End-User. - A logout without user interaction (i.e. no prompt) is only allowed - if an ID Token is provided that matches the current user. """ - return ( - oauth2_settings.OIDC_RP_INITIATED_LOGOUT_ALWAYS_PROMPT - or token_user is None - or token_user != self.request.user - ) + + if not self.request.user.is_authenticated: + """ + > the OP MUST ask ask the End-User whether to log out of the OP as + + If the user does not have an active session with the OP, they cannot + end their OP session, so there is nothing to prompt for. This occurs + in cases where the user has logged out of the OP via another channel + such as the OP's own logout page, session timeout or another RP's + logout page. + """ + return False + + if oauth2_settings.OIDC_RP_INITIATED_LOGOUT_ALWAYS_PROMPT: + """ + > At the Logout Endpoint, the OP SHOULD ask the End-User whether to + > log out of the OP as well + + The admin has configured the OP to always prompt the userfor logout + per the SHOULD recommendation. + """ + return True + + if token_user is None: + """ + > the OP MUST ask ask the End-User whether to log out of the OP as + > well if the supplied ID Token does not belong to the current OP + > session with the RP. + + token_user will only be populated if an ID token was found for the + RP (Application) that is requesting the logout. If token_user is not + then we must prompt the user. + """ + return True + + if token_user != self.request.user: + """ + > the OP MUST ask ask the End-User whether to log out of the OP as + > well if the supplied ID Token does not belong to the logged in + > End-User. + + is_authenticated indicates that there is a logged in user and was + tested in the first condition. + token_user != self.request.user indicates that the token does not + belong to the logged in user, Therefore we need to prompt the user. + """ + return True + + """ We didn't find a reason to prompt the user """ + return False def do_logout(self, application=None, post_logout_redirect_uri=None, state=None, token_user=None): user = token_user or self.request.user diff --git a/tests/test_oidc_views.py b/tests/test_oidc_views.py index 8bdf1836..65197cbd 100644 --- a/tests/test_oidc_views.py +++ b/tests/test_oidc_views.py @@ -311,6 +311,10 @@ def test_must_prompt(oidc_tokens, other_user, rp_settings, ALWAYS_PROMPT): == ALWAYS_PROMPT ) assert RPInitiatedLogoutView(request=mock_request_for(other_user)).must_prompt(oidc_tokens.user) is True + assert ( + RPInitiatedLogoutView(request=mock_request_for(AnonymousUser())).must_prompt(oidc_tokens.user) + is False + ) def test__load_id_token(): @@ -577,13 +581,14 @@ def test_token_deletion_on_logout(oidc_tokens, logged_in_client, rp_settings): @pytest.mark.django_db(databases=retrieve_current_databases()) -def test_token_deletion_on_logout_expired_session(oidc_tokens, client, rp_settings): +def test_token_deletion_on_logout_without_op_session_get(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={ @@ -591,15 +596,31 @@ def test_token_deletion_on_logout_expired_session(oidc_tokens, client, rp_settin "client_id": oidc_tokens.application.client_id, }, ) - assert rsp.status_code == 200 + assert rsp.status_code == 302 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() + assert AccessToken.objects.count() == 0 + assert IDToken.objects.count() == 0 + assert RefreshToken.objects.count() == 1 + + with pytest.raises(AccessToken.DoesNotExist): + AccessToken.objects.get() + + with pytest.raises(IDToken.DoesNotExist): + IDToken.objects.get() + refresh_token = RefreshToken.objects.get() - assert refresh_token.revoked is None + assert refresh_token.revoked is not None + + +@pytest.mark.django_db(databases=retrieve_current_databases()) +def test_token_deletion_on_logout_without_op_session_post(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.post( reverse("oauth2_provider:rp-initiated-logout"),