Improve debug

[pitbulk]

• Add option to raise response validation exceptions #37 Add option to raise response validation exceptions
• Optionally raise detailed exceptions vs. returning False
• Implement a more specific exception class for handling some validation errors
• Improve/Fix tests
• Add support for retrieving the last ID of the generated AuthNRequest / LogoutRequest
• Add hooks to retrieve last-sent and last-received requests and responses

[coveralls]

Coverage increased (+1.001%) to 96.262% when pulling 7ac2640 on improve_debug into f011fda on master.

[coveralls]

Coverage increased (+0.4%) to 95.707% when pulling 9ea32f5 on improve_debug into f011fda on master.

[coveralls]

Coverage increased (+0.4%) to 95.668% when pulling 73ee977 on improve_debug into f011fda on master.

[coveralls]

Coverage increased (+0.5%) to 95.728% when pulling a510f16 on improve_debug into f011fda on master.

[nickw444]

This will break backward compatibility

[pitbulk]

Ok I will keep the old and mark as deprecated

[nickw444]

I personally don't like the use of "codes" in exceptions, since users of the library need to understand they need to inspect a specific property in the exception, but I do see how this is keeping consistency with the implementation of the existing OneLogin_Saml2_Error.

A better solution would be to have an exception for each of these individually, that possibly inherit from a base class exception class OneLogin_Saml2_ValidationError.

[quantus]

I also don't like exception codes. I would like to have OneLogin_Saml2_Error that would be the base class for all exception raised by this library. Then we could have OneLogin_Saml2_ValidationError exception class inheriting that, and then each error would have own exception class.

Some code showing usage:

class OneLogin_Saml2_Error(Exception):
    ....

class OneLogin_Saml2_ValidationError(OneLogin_Saml2_Error):
    # For backwards compatibility
    code = 123

class OneLogin_Saml2_ExpiredResponseError(OneLogin_Saml2_ValidationError):
    # If we want to keep backwards compatibility
    code = 123


# Usage:

try:
    auth.process_sso(...)
except OneLogin_Saml2_ExpiredResponseError as e:
    # Not important, tell user to retry
except OneLogin_Saml2_Error as e:
    # Inform user about the error and log it somewhere

With that said, I really liked the changes in this PR. 👍 I think that the exception handling can be changed to the way I showed later on without breaking any existing code, so that can be implemented later on.

[pitbulk]

My idea here is to have 2 different kind of exceptions:

• Error, related to how are the toolkit environment/settings
• ValidationError, some exception that happened when validating a SAMLResponse

I got your point of view, but then we need 50+ exception methods ...

In real scenarios I don't think that you gonna need to catch many specific exception, rather than just check if there was an environment/settings error, or a validation error.

[quantus]

Yeah, you are right that in most cases there is no need to many specific exceptions, and making separate class for each error would add lots of really small exception classes. I think that in many cases it is quite irrelevant what the exact error is, so one common validation error class makes sense.

I am no longer sure if the validation error should be inherited from the OneLogin_Saml2_Error class. That way the user could always catch all exceptions easily, but the downside of this is that both errors would share the same error code range. Then one would need to make sure that the OneLogin_Saml2_Error and OneLogin_Saml2_ValidationError classes never share any error code values.

[nickw444]

NameID not found in the Logout Request instead?

[pitbulk]

Ok I will fix that message

[quantus]

I think the dsig_ctx.verify(signature_node) code should be inside try except block and then the raised exception should be converted to OneLogin_Saml2_Error exception. This way the user of this library doesn't need to consider different exception types raised by the xmlsec library, and catching OneLogin_Saml2_Error exceptions will cover invalid signature exceptions as well.

[pitbulk]

I wanted to offer a direct way to get the real cause when a signature validation fails.
instead of catch the exception here on the validate_node_sign method, but I think I can provide this info on an extra parameter. Let me fix that

[coveralls]

Coverage increased (+0.5%) to 95.786% when pulling 37788e2 on improve_debug into f011fda on master.

[coveralls]

Coverage increased (+0.5%) to 95.786% when pulling a096f36 on improve_debug into f011fda on master.

[quantus]

CHILDREN_NODE_NOT_FOIND_IN_KEYINFO -> CHILDREN_NODE_NOT_FOUND_IN_KEYINFO

[coveralls]

Coverage increased (+0.5%) to 95.786% when pulling acb88e0 on improve_debug into f011fda on master.

[coveralls]

Coverage increased (+0.5%) to 95.786% when pulling fa6d3ea on improve_debug into f011fda on master.

[coveralls]

Coverage increased (+0.5%) to 95.786% when pulling cbbe084 on improve_debug into f011fda on master.

[coveralls]

Coverage increased (+0.5%) to 95.786% when pulling c90a62d on improve_debug into f011fda on master.