Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Feature refactor saml modules #125

Merged

Conversation

c00kiemon5ter
Copy link
Member

This changeset is a generic cleanup on the saml base, frontend and backend modules.
I tried to make the code simpler and more readable, while avoiding repetition.

The general idea is that configuration keys should not be magic values all over the code and thus should be defined in the modules that uses the keys. Initialisation of common keys in the front and back end are handled by the base module.

While the changes may look a lot, most are formatting changes (whitespace, max line length, etc) and there are only a couple of actual code changes on how fallbacks are handled.
These happen on backend's __init__ regarding key_file_paths and on the frontend's _handle_authn_response function regarding how policy settings are accessed to gather info about sign_assertion, sign_response, sign_alg and digest_alg.

PS: It should be easier to look at each commit separately


auth_info = {}
if self.acr_mapping:
auth_info["class_ref"] = self.acr_mapping.get(internal_response.auth_info.issuer, self.acr_mapping[""])
auth_info["class_ref"] = self.acr_mapping.get(
internal_response.auth_info.issuer, self.acr_mapping[""])
Copy link
Member Author

@c00kiemon5ter c00kiemon5ter Jul 26, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could switch to get_dict_defaults() here, instead of manually trying the "" key.
I think this happens elsewhere too, and we can agree that get_dict_defaults should dictate on how fallback keys should be checked (as a reminder there are two special/fallback keys: "" and "default").

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree.

@c00kiemon5ter c00kiemon5ter force-pushed the feature-refactor-saml-modules branch from b1491d9 to e87af90 Compare July 27, 2017 06:59
@johanlundberg
Copy link
Contributor

This PR makes the code much more readable. I will merge it if no one else has something to add.

@johanlundberg johanlundberg merged commit fde214c into IdentityPython:master Jul 28, 2017
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants