Various amends #20
Various amends #20
Conversation
Pass in the SP entity_id BaseProcessor was not being instantiated Pass extra config into the processor.create_identity method - this allows for providing additional params required by AWS IAM Saml2 This enables the staff-sso app to use as model based processor which can reference saml2 data config based on entity_id
Improve processor logic
Fix login view parses the authn request using correct binding
There was a problem hiding this comment.
Looks all good to me.
I've added a comment about a little bugfix which I just pushed which isn't in your fork, and another about a small possible optimization.
Do you find the separate is_enabled() and has_access methods on the Processor classes are okay, or could they be combined into a single method taking the request as param?
| identity=identity, userid=request.user.username, | ||
| name_id=NameID(format=resp_args['name_id_policy'].format, sp_name_qualifier=resp_args['sp_entity_id'], text=request.user.username), | ||
| identity=identity, userid=user_id, | ||
| name_id=NameID(format=resp_args['name_id_policy'].format, sp_name_qualifier=resp_args['sp_entity_id'], text=user_id), |
There was a problem hiding this comment.
As an fyi: this was changed from sp_name_qualifier=resp_args['destination'] to sp_name_qualifier=resp_args['sp_entity_id'] according to the issue filed here #18
There was a problem hiding this comment.
OK, makes sense - I'll test this in our UAT env tomorrow.
|
@mhindery I haven't had time to do what I wanted today - so maybe let's get this in a mergeable state and I'll raise another PR with some tests. |
|
I'm fine with merging this as it is. I'll then add some documentation about the changes as there are some backwards-incompatible changes, and publish a new version. Does that sound ok to you? |
|
Yes, that sounds good!
…On Fri, 11 Jan 2019 at 19:25, Mathieu Hinderyckx ***@***.***> wrote:
I'm fine with merging this as it is. I'll then add some documentation
about the changes as there are some backwards-incompatible changes, and
publish a new version. Does that sound ok to you?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#20 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAOhY4iR2HicjUgXMkNsO7EfwhdJgLbIks5vCOUpgaJpZM4Z6Y1z>
.
|
|
The docs have been updated, I've released a new version: https://pypi.org/project/djangosaml2idp/0.5.0/ Thank you for your contribution! |
As discussed, here's a PR with the changes that I've made to satisfy requirements for this project: https://www.github.com/uktrade/staff-sso. We have integrated our sso solution with Google and AWS via saml2, using this module.
Here's a summary of the changes: