Fix extraction of the response issuer#333
Merged
pitbulk merged 2 commits intoSAML-Toolkits:masterfrom Jun 24, 2021
Merged
Conversation
A ">1" in place of a ">0" was causing the inability to extract the Response issuer(s) correctly. Added a test case to check this. Fixed another test case that should (IMHO) return an error regarding the issuer, rather than the signature.
The SamlResponse.getIssuers() contract is quite controversial. For a valid response, it will always return just one element. For an invalid response, depending on the cause it may: - fail if no Assertion is present: this means, in particular, that if the status code is not a success one, it's impossible to retrieve the Response issuer with this method (although it may be a reasonable requirement, for logging purposes for instance) - fail if multiple Assertions are present: again, the Response Issuer cannot be retrieved in this case either - fail in the unlikely event that multiple Response Issuers were found - return up to 2 issuers at most, if different issuers were set on the Response and on the Assertion (which will make isValid() fail), with the inability to determine which is the Response Issuer and which is the Assertion Issuer (indeed: the former will be the first one in the list, the latter will be the second, but this contract is a bit weak) For these reasons, two different methods were provided to retrieve the Response and the Assertion Issuers, with the former that will succeed even when the status code is not a successful one. Also, because of the above reasons, the getIssuers() method was deprecated in favour of the two new ones.
Contributor
Author
|
I added a second commit which will split |
pitbulk
force-pushed
the
master
branch
3 times, most recently
from
ab7e4d7 to
3c79c8c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A ">1" in place of a ">0" was causing the inability to extract the
Response issuer(s) correctly.
Added a test case to check this.
Fixed another test case that should (IMHO) return an error regarding
the issuer, rather than the signature.