Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: OP prompts for logout when no OP session #1517

Merged
merged 1 commit into from
Oct 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
n2ygk marked this conversation as resolved.
Show resolved Hide resolved
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
Loading