The former returns false even if the SAML response is valid (i.e. it's well formed and correctly compiled) but the status code is different from SUCCESS. The latter does not check the status code, and returns true in case of a well-formed but unsuccessful logout response: apart from the obvious urn:oasis:names:tc:SAML:2.0:status:PartialLogout status code, looking at the list of possible status codes I think there are some that may also apply to a SLO scenario (being urn:oasis:names:tc:SAML:2.0:status:VersionMismatch the first one that comes ahead my eyes after a quick look).
Actually, I personally think that having isValid(String) return false in case of a non-successful response is not the best choice, because it makes harder for the consuming code to discriminate between a valid (but unsuccessful) response and a really invalid one (i.e.: not respecting SAML requirements for the given profile, invalid signatures, etc.). However, perhaps having both methods behave in the same way would be better in any case.
This issue is just to take note about the fact that there's a lack of consistency between:
com.onelogin.saml2.authn.SamlResponse.isValid(String)andcom.onelogin.saml2.logout.LogoutResponse.isValid(String)The former returns
falseeven if the SAML response is valid (i.e. it's well formed and correctly compiled) but the status code is different from SUCCESS. The latter does not check the status code, and returnstruein case of a well-formed but unsuccessful logout response: apart from the obviousurn:oasis:names:tc:SAML:2.0:status:PartialLogoutstatus code, looking at the list of possible status codes I think there are some that may also apply to a SLO scenario (beingurn:oasis:names:tc:SAML:2.0:status:VersionMismatchthe first one that comes ahead my eyes after a quick look).Actually, I personally think that having
isValid(String)returnfalsein case of a non-successful response is not the best choice, because it makes harder for the consuming code to discriminate between a valid (but unsuccessful) response and a really invalid one (i.e.: not respecting SAML requirements for the given profile, invalid signatures, etc.). However, perhaps having both methods behave in the same way would be better in any case.What do you think?