🐞 fix(oidc-logout): 修复一个引发500 error的问题

[Polaris-d]

Fixes #1280

Description of the Change

Returns a 400 error when no id_token_hint is provided and it is impossible to confirm who the request came from

Checklist

• [x] PR only contains one change (considered splitting up PR)
• [x] unit-test added
• [ ] documentation updated
• [ ] CHANGELOG.md updated (only for user relevant changes)
• [x] author name in AUTHORS

[codecov[bot]]

Codecov Report

Merging #1281 (d5e1908) into master (13a6143) will decrease coverage by 0.05%. The diff coverage is 80.00%.

:exclamation: Current head d5e1908 differs from pull request most recent head 54eec08. Consider uploading reports for the commit 54eec08 to get more accurate results

@@            Coverage Diff             @@
##           master    #1281      +/-   ##
==========================================
- Coverage   97.29%   97.25%   -0.05%     
==========================================
  Files          31       31              
  Lines        1996     2001       +5     
==========================================
+ Hits         1942     1946       +4     
- Misses         54       55       +1     

Impacted Files	Coverage Δ
oauth2_provider/views/oidc.py	99.49% <66.66%> (-0.51%)	:arrow_down:
oauth2_provider/exceptions.py	100.00% <100.00%> (ø)

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[tonial]

Hi,

The OIDC RP-initiated logout specifications (https://openid.net/specs/openid-connect-rpinitiated-1_0.html#RPLogout) states that id_token_hint is only recommended.

This means the view should allow a RP to send a user without id_token_hint, and the code should not raise an error if the user session already expired.

I think a better way to fix the issue would be to improve again do_logout() by skipping the if oauth2_settings.OIDC_RP_INITIATED_LOGOUT_DELETE_TOKENS: part if no user was found (token_user == None and self.request.user is anonymous)

[Polaris-d]

Hi,

The OIDC RP-initiated logout specifications (https://openid.net/specs/openid-connect-rpinitiated-1_0.html#RPLogout) states that id_token_hint is only recommended.

This means the view should allow a RP to send a user without id_token_hint, and the code should not raise an error if the user session already expired.

I think a better way to fix the issue would be to improve again do_logout() by skipping the if oauth2_settings.OIDC_RP_INITIATED_LOGOUT_DELETE_TOKENS: part if no user was found (token_user == None and self.request.user is anonymous)

I read it again carefully and found some very interesting places

1. It has never been mentioned that the OP is not logged in, does this mean that the author may not be aware of this situation, or does it mean that the OP must be logged in when using the logout endpoint
2. At the Logout Endpoint, the OP SHOULD ask the End-User whether to log out of the OP as well. It is specially emphasized here that when logging out, the user should be prompted whether to log out the OP at the same time, which should mean that RP and OP should be processed separately according to the user's choice, instead of logging out at the same time

There is no doubt that it is precisely because of the ambiguity of the documentation that we cannot clarify the processing flow of this situation. I also tried to find some sites that already support logout and refer to their practices, but so far I have nothing reward

[tonial]

Hi,

I agree on the ambiguity of the specifications, but in this case I think it's pretty clear that we should not raise an error :

Note that because RP-Initiated Logout Requests are intended to be idempotent, it is explicitly not an error for an RP to request that a logout be performed when the OP does not consider that the End-User is logged in with the OP at the requesting RP.

If think we can either mock the logout (prompt for confirmation, but do nothing afterward), or display to the user that he's already logged out (less idempotent).

[Polaris-d]

Hi,

I agree on the ambiguity of the specifications, but in this case I think it's pretty clear that we should not raise an error :

Note that because RP-Initiated Logout Requests are intended to be idempotent, it is explicitly not an error for an RP to request that a logout be performed when the OP does not consider that the End-User is logged in with the OP at the requesting RP.

If think we can either mock the logout (prompt for confirmation, but do nothing afterward), or display to the user that he's already logged out (less idempotent).

Yes, you are right. This can be treated as the case described by the OP does not consider that the End-User is logged in with the OP at the requesting RP. I think I will add judgment in do_logout. In this case, avoid deleting any token and performing operations, and only keep the processing of redirection But the work is too busy recently, the code may be later

[tonial]

Thanks @Polaris-d

Since I've got some time and also had to mitigate the issue on my project, I've opened # 1284 to fix the issue