pysaml2 icon indicating copy to clipboard operation
pysaml2 copied to clipboard

base_client's create_authn_request method returns different types depending on parameters

Open tadumtada opened this issue 7 years ago • 1 comments

Using version 4.6.3, the base_client's create_authn_request method returns different types depending on parameters sign_prepare and sign. With sign_prepare==True I get an instance of AuthnRequest, with sign_prepare==False and sign==True, it returns an unicode string which is created in entity.sign with the method signed_instance_factory.

This causes my code to brake if I change the settings/parameters.

I suggest to either change the comment in the method create_authn_request such that others don't run into the problem and not to brake any existing code or to return a consistent type or to remove the sign parameter and let everybody sign the return AuthnRequest after the create_authn_request method has been called.

The latter is what I implemented, meaning to set sign==False and sign afterwards if I want. This works but it would have been nice to have it commented.

tadumtada avatar Nov 22 '18 15:11 tadumtada

In general I agree that the creation of the authentication request is separate action/concern than the signing of a request. Blending the two results in such inconsistencies. This function does many more things. I don't really think that it is easy to just remove parameters. Moving forward, the most probable way that this will be handled is to leave it as it is, and rebuild the functionality in a new module, then deprecate the old and push people to use the new module, providing instructions on how to switch.

For now, I have amended the docstring to include the alternative result.

c00kiemon5ter avatar Jan 14 '19 19:01 c00kiemon5ter