Skip to content

Updates _sp_authn_requests_signed_alg to work with multiple bindings #142

New issue

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

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

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from

Conversation

mobleyc
Copy link

@mobleyc mobleyc commented Feb 10, 2019

_sp_authn_requests_signed_alg allows overriding the algorithm used with SignatureMethod but currently only works with the BINDING_HTTP_REDIRECT. This change enables its use for both BINDING_HTTP_REDIRECT and BINDING_HTTP_POST, during login and its use during logout.

The motivation for this change is ADFS defaults to requiring SHA256.

This change was tested with Apache Shibboleth and ADFS on Windows Server 2012.

@peppelinux
Copy link
Member

I think that would be better enable this feature in pySAML2, made a related PR here:
IdentityPython/pysaml2#597

@peppelinux
Copy link
Member

Hi @mobleyc, have you seen the pySAML2 PR I referenced, would you like to spend some words on?
I'm going to manage the old PR and a feedback from you will be really appreciated

@peppelinux
Copy link
Member

Hi @mobleyc
here we have a default behaviour:
https://github.com/knaperek/djangosaml2/blob/8d9b692b10dae2613574baf5857513b35e5f0ce1/djangosaml2/views.py#L174

As far I tested the signed authentication requests in pySAML2 works in HTTP_POST binding and there we have this check.

Is there some tests that you would like to share here?
I disabled HTTP_REDIRECT in my implementations because of this

@peppelinux peppelinux force-pushed the master branch 6 times, most recently from 0dcbdc6 to 02515a2 Compare May 1, 2020 16:25
@mobleyc
Copy link
Author

mobleyc commented May 2, 2020

Hi the issue I encountered is not signing, it's the ability to configure the signing algorithm. Currently this line. ADFS uses sha256 by default and there is not way to override this using the BINDING_HTTP_POST binding.

I can add a comment to the pySAML ticket in support of the PR if that helps.

And thank you for your help with this!

@peppelinux
Copy link
Member

Thank you @mobleyc,

I have fixed this features, because I use SHA256 in djangosaml2 in a federation that want it, with that PR you find in pySAML2.

Regarding the general refactor you've done I think that's quite good. So if you are ready for that I would ask you to have a test with the current master branch of djangosaml2, because with the current implementation if a signing is required it alway rely on HTTP_POST, see:
https://github.com/knaperek/djangosaml2/blob/312a06529198ebf4a9e12263b9f0a10725fbb6bd/djangosaml2/views.py#L175

Do a test then we decide what to do, I would accept your PR with pleasure,
thanks for feedback

@peppelinux
Copy link
Member

I think that also this should be considered

#193

@peppelinux
Copy link
Member

Hi, please can you fix the conflicting files?
I think it's time to merge it

@peppelinux
Copy link
Member

Due to missing reply but thank you for your idea

@peppelinux peppelinux closed this Sep 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants