Err code 501 504 #725
Err code 501 504 #725
Conversation
Codecov Report
@@ Coverage Diff @@
## master #725 +/- ##
=========================================
- Coverage 63.83% 63.8% -0.03%
=========================================
Files 64 64
Lines 11675 11677 +2
Branches 2059 2060 +1
=========================================
- Hits 7453 7451 -2
- Misses 3649 3653 +4
Partials 573 573
Continue to review full report at Codecov.
|
| @@ -2298,6 +2298,11 @@ def do_verified_logout( | |||
|
|
|||
| if res.status_code < 300: | |||
| logger.info("Logged out from {}".format(_cid)) | |||
| elif res.status_code in [501, 504]: | |||
| logger.info( | |||
There was a problem hiding this comment.
I am not sure how to properly handle this. I do agree that 501 and 504 shouldn't be treated as a complete failure, but it should be at least a partial failure.
501 means that the logout on that provider failed completely
504 means that some other mechanism after the logout failed as well
The log should be higher than info, at least a warning if not error.
If I understand it correctly this shouldn't be logged to the events as a fault?
I am also not sure about not appending some of these to the failed...
There was a problem hiding this comment.
Sorry I missed this comment.
I agree it should be regarded as an error, the question is what kind of error.
I also agree that it should be logged as a warning.
It should not be logged to events as a failure as it is not a OP <-> RP communication error. The communication between the OP and the RP was OK it was just that on the RP side the procedure failed.
There was a problem hiding this comment.
I would think that logging as an error should be fine. Warning the user would be a responsibility of the RP, I guess.
|
@rohe Any chance on finishing this? |
CHANGELOG.md.According to the backchannel draft a backchannel logout request should accept error codes 501 and 504 beside the normally accepted 200.
https://openid.net/specs/openid-connect-backchannel-1_0.html#BCResponse