Skip to content

Commit 54e9d14

Browse files
committed
fix: OP prompts for logout when no OP session
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.
1 parent 28b512a commit 54e9d14

File tree

3 files changed

+91
-17
lines changed

3 files changed

+91
-17
lines changed

CHANGELOG.md

+5-1
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [unreleased]
99
### Added
10-
* Support for Wildcard Origin and Redirect URIs, https://github.com/jazzband/django-oauth-toolkit/issues/1506
10+
* #1506 Support for Wildcard Origin and Redirect URIs
1111
<!--
1212
### Changed
1313
### Deprecated
1414
### Removed
15+
-->
1516
### Fixed
17+
* #1517 OP prompts for logout when no OP session
18+
* #1512 client_secret not marked sensitive
19+
<!--
1620
### Security
1721
-->
1822

oauth2_provider/views/oidc.py

+58-9
Original file line numberDiff line numberDiff line change
@@ -367,17 +367,66 @@ def validate_logout_request(self, id_token_hint, client_id, post_logout_redirect
367367
return application, id_token.user if id_token else None
368368

369369
def must_prompt(self, token_user):
370-
"""Indicate whether the logout has to be confirmed by the user. This happens if the
371-
specifications force a confirmation, or it is enabled by `OIDC_RP_INITIATED_LOGOUT_ALWAYS_PROMPT`.
370+
"""
371+
per: https://openid.net/specs/openid-connect-rpinitiated-1_0.html
372+
373+
> At the Logout Endpoint, the OP SHOULD ask the End-User whether to log
374+
> out of the OP as well. Furthermore, the OP MUST ask the End-User this
375+
> question if an id_token_hint was not provided or if the supplied ID
376+
> Token does not belong to the current OP session with the RP and/or
377+
> currently logged in End-User.
372378
373-
A logout without user interaction (i.e. no prompt) is only allowed
374-
if an ID Token is provided that matches the current user.
375379
"""
376-
return (
377-
oauth2_settings.OIDC_RP_INITIATED_LOGOUT_ALWAYS_PROMPT
378-
or token_user is None
379-
or token_user != self.request.user
380-
)
380+
381+
if not self.request.user.is_authenticated:
382+
"""
383+
> the OP MUST ask ask the End-User whether to log out of the OP as
384+
385+
If the user does not have an active session with the OP, they cannot
386+
end their OP session, so there is nothing to prompt for. This occurs
387+
in cases where the user has logged out of the OP via another channel
388+
such as the OP's own logout page, session timeout or another RP's
389+
logout page.
390+
"""
391+
return False
392+
393+
if oauth2_settings.OIDC_RP_INITIATED_LOGOUT_ALWAYS_PROMPT:
394+
"""
395+
> At the Logout Endpoint, the OP SHOULD ask the End-User whether to
396+
> log out of the OP as well
397+
398+
The admin has configured the OP to always prompt the userfor logout
399+
per the SHOULD recommendation.
400+
"""
401+
return True
402+
403+
if token_user is None:
404+
"""
405+
> the OP MUST ask ask the End-User whether to log out of the OP as
406+
> well if the supplied ID Token does not belong to the current OP
407+
> session with the RP.
408+
409+
token_user will only be populated if an ID token was found for the
410+
RP (Application) that is requesting the logout. If token_user is not
411+
then we must prompt the user.
412+
"""
413+
return True
414+
415+
if token_user != self.request.user:
416+
"""
417+
> the OP MUST ask ask the End-User whether to log out of the OP as
418+
> well if the supplied ID Token does not belong to the logged in
419+
> End-User.
420+
421+
is_authenticated indicates that there is a logged in user and was
422+
tested in the first condition.
423+
token_user != self.request.user indicates that the token does not
424+
belong to the logged in user, Therefore we need to prompt the user.
425+
"""
426+
return True
427+
428+
""" We didn't find a reason to prompt the user """
429+
return False
381430

382431
def do_logout(self, application=None, post_logout_redirect_uri=None, state=None, token_user=None):
383432
user = token_user or self.request.user

tests/test_oidc_views.py

+28-7
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,10 @@ def test_must_prompt(oidc_tokens, other_user, rp_settings, ALWAYS_PROMPT):
311311
== ALWAYS_PROMPT
312312
)
313313
assert RPInitiatedLogoutView(request=mock_request_for(other_user)).must_prompt(oidc_tokens.user) is True
314+
assert (
315+
RPInitiatedLogoutView(request=mock_request_for(AnonymousUser())).must_prompt(oidc_tokens.user)
316+
is False
317+
)
314318

315319

316320
def test__load_id_token():
@@ -577,29 +581,46 @@ def test_token_deletion_on_logout(oidc_tokens, logged_in_client, rp_settings):
577581

578582

579583
@pytest.mark.django_db(databases=retrieve_current_databases())
580-
def test_token_deletion_on_logout_expired_session(oidc_tokens, client, rp_settings):
584+
def test_token_deletion_on_logout_without_op_session_get(oidc_tokens, client, rp_settings):
581585
AccessToken = get_access_token_model()
582586
IDToken = get_id_token_model()
583587
RefreshToken = get_refresh_token_model()
584588
assert AccessToken.objects.count() == 1
585589
assert IDToken.objects.count() == 1
586590
assert RefreshToken.objects.count() == 1
591+
587592
rsp = client.get(
588593
reverse("oauth2_provider:rp-initiated-logout"),
589594
data={
590595
"id_token_hint": oidc_tokens.id_token,
591596
"client_id": oidc_tokens.application.client_id,
592597
},
593598
)
594-
assert rsp.status_code == 200
599+
assert rsp.status_code == 302
595600
assert not is_logged_in(client)
596601
# Check that all tokens are active.
597-
access_token = AccessToken.objects.get()
598-
assert not access_token.is_expired()
599-
id_token = IDToken.objects.get()
600-
assert not id_token.is_expired()
602+
assert AccessToken.objects.count() == 0
603+
assert IDToken.objects.count() == 0
604+
assert RefreshToken.objects.count() == 1
605+
606+
with pytest.raises(AccessToken.DoesNotExist):
607+
AccessToken.objects.get()
608+
609+
with pytest.raises(IDToken.DoesNotExist):
610+
IDToken.objects.get()
611+
601612
refresh_token = RefreshToken.objects.get()
602-
assert refresh_token.revoked is None
613+
assert refresh_token.revoked is not None
614+
615+
616+
@pytest.mark.django_db(databases=retrieve_current_databases())
617+
def test_token_deletion_on_logout_without_op_session_post(oidc_tokens, client, rp_settings):
618+
AccessToken = get_access_token_model()
619+
IDToken = get_id_token_model()
620+
RefreshToken = get_refresh_token_model()
621+
assert AccessToken.objects.count() == 1
622+
assert IDToken.objects.count() == 1
623+
assert RefreshToken.objects.count() == 1
603624

604625
rsp = client.post(
605626
reverse("oauth2_provider:rp-initiated-logout"),

0 commit comments

Comments
 (0)