Skip to content

Commit

Permalink
fix: OP prompts for logout when no OP session (#1517)
Browse files Browse the repository at this point in the history
The OAuth provider is prompting users who no longer have an user session
with the OAuth Provider to logout of the OP. This happens in scenarios
given the user has logged out of the OP directly or via another client.
In cases where the user does not have a session on the OP we should not
prompt them to log out of the OP as there is no session, but we should
still clear out their tokens to terminate the session for the Application.
  • Loading branch information
dopry authored Oct 9, 2024
1 parent 28b512a commit 59bd7af
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 17 deletions.
6 changes: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
<!--
### Changed
### Deprecated
### Removed
-->
### Fixed
* #1517 OP prompts for logout when no OP session
* #1512 client_secret not marked sensitive
<!--
### Security
-->

Expand Down
67 changes: 58 additions & 9 deletions oauth2_provider/views/oidc.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
35 changes: 28 additions & 7 deletions tests/test_oidc_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand Down Expand Up @@ -577,29 +581,46 @@ 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={
"id_token_hint": oidc_tokens.id_token,
"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"),
Expand Down

0 comments on commit 59bd7af

Please sign in to comment.