Added support to display child status codes if they exist in SAML response #12818
Conversation
spring-projects-issues
added
the
status: waiting-for-triage
|
Hi @jzheaux, could you take a look at this fix? |
There was a problem hiding this comment.
Thanks, @Anubhav-2000! Sorry for the delay in responding. I've left some inline feedback.
Also, will you please add a test that confirms the error message includes a child status code?
When you are ready, please squash your commits so that there is only one.
| String message = String.format("Invalid status [%s] for SAML response [%s]", statusCode, | ||
| response.getID()); | ||
| result = result.concat(new Saml2Error(Saml2ErrorCodes.INVALID_RESPONSE, message)); | ||
| List<String> statusCodes = getStatusCodes(response); |
There was a problem hiding this comment.
Instead of getting all the status codes ad infinitum, please just get the top-level one.
If it is of type REQUESTER, RESPONDER, or VERSION_MISMATCH, then add a single error message that includes this status code and any single child status code.
Otherwise, if it isn't SUCCESS, then add a single error message like it already does.
This is based on the logic outlined in: https://docs.oasis-open.org/security/saml/v2.0/saml-core-2.0-os.pdf#page=39
jzheaux
added
type: enhancement
|
Sure, let me take a look once |
|
Hi, @Anubhav-2000! Are you able to make the requested changes? |
jzheaux
added
the
status: ideal-for-contribution
|
Hi @jzheaux ! May I follow up on this according to your feedback ? |
|
Filed #14743, wonder if you guys could help take a look |
jzheaux
added
status: duplicate
|
Thanks, @Anubhav-2000 for getting things started! @youngkih was able to finish in a separate PR. I'm closing this in favor of #14743 |
Fixes #11725
Please let me know if i have implemented something incorrectly. First time submitting a PR for bug fix :)