Skip to content
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

Inline slo signature #133

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

waynewoodfield
Copy link

This week, I encountered an IdP that sent me a LogoutResponse containing a signature embedded within the SAMLResponse, instead of split out into separate Signature/SigAlg query parameters. With much stealing from SamlResponse.java object, I added support for this type of signature.

@coveralls
Copy link

coveralls commented Nov 10, 2017

Coverage Status

Coverage decreased (-1.9%) to 93.265% when pulling 5dfa413 on waynewoodfield:inline-slo-signature into e5e25f0 on onelogin:master.

@pitbulk
Copy link
Contributor

pitbulk commented Nov 10, 2017

The HTTP-Redirect binding uses the SAML Messages, RelayState and Signature and SigAlg as GET parameters.

The HTTP-POST binding uses SAML Mesage and RelayState as POST parameters, and Signature is embedded inside the SAML Message

We support HTTP-POST binding on ACS URL, but rest of the toolkits uses HTTP-Redirect binding and we don't plan to support a different protocol. Read #116

@waynewoodfield
Copy link
Author

waynewoodfield commented Nov 10, 2017 via email

@pitbulk
Copy link
Contributor

pitbulk commented Nov 10, 2017

The goal of the toolkit is to offer an SP with the minimun implementation and the easiest settings able to communicate with most of the IdPs.

Im sure that you selected for your customer that toolkit because you found it easy..otherwise..you could use other open source alternative that supports all the bindings, but is complex with complex settings.

Instead ask the SP to support all bindings..why not ask IdP software to support the minimun bindings defined on the SAML standard as required?

If I now accept your PR for LogoutResponse, I need to do the same for LogoutRequest...and then why not implement the ability to send SLO messages using POST...and after that.. why not support Artifact binding...why not support AttributeQuery....

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants