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

Improve Key Rollover management #389

Merged
merged 15 commits into from
May 18, 2017
Merged

Improve Key Rollover management #389

merged 15 commits into from
May 18, 2017

Conversation

…tes and be able to inject the data obtained on the settings.
@skyporter
Copy link

I think parsing metadata.xml has been break when this one contain more than one SingleSignOnService meta. For example if you have those meta

<SingleSignOnService Binding="urn:mace:shibboleth:1.0:profiles:AuthnRequest" Location="https://abc.com/idp/profile/Shibboleth/SSO"/>
<SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect" Location="https://abc/idp/profile/SAML2/Redirect/SSO"/>
<SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST" Location="https://abc.com/idp/profile/SAML2/POST/SSO"/>

and you set follow setting

settings.assertion_consumer_service_binding = "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST"

the idp_sso_target_url is set with the first meta, which is wrong.

Perhaps because on this line https://github.com/onelogin/ruby-saml/pull/389/files#diff-224905527907deba9ffd52ff7ab83308R55 passing options is missing ?

@pitbulk
Copy link
Collaborator Author

pitbulk commented Apr 27, 2017

I will review and create more tests

@henrikvongunten
Copy link

I'm looking for these features in the java-saml toolkit, any plans to implement them there too in near future?

@pitbulk
Copy link
Collaborator Author

pitbulk commented Apr 28, 2017

@henrikvongunten

I plan to add that functionality to Python and Java too. But there is no estimation on when I gonna be able to start that task.

@pitbulk
Copy link
Collaborator Author

pitbulk commented May 9, 2017

@skyporter I think now should be working, thanks for reporting that. have you found any other issue?

@skyporter
Copy link

@pitbulk thank you, I had test the branch and it's work fine.

I have another issue, but I don't know if it's related to this branch or not (I can't test with the master branch since I have a multi organization metadata).
My issue occurs when I try to use the POST method with a form. I follow this comment but SAML request seem wrongly encoded.

capture d ecran 2017-05-10 a 09 23 33

Torsten Schoenebaum and others added 3 commits May 10, 2017 20:55
and IdpMetadataParser#parse_remote_to_hash.

Having the parsed metadata as Hash may be useful for configuring
omniauth-saml, for instance.
Implement IdpMetadataParser#parse_to_hash
@pitbulk
Copy link
Collaborator Author

pitbulk commented May 11, 2017

We added 2 new methods (required by omniauth). [ty @tosch]
Now I think this PR is complete.

@vincentwoo @AngelicaS @demonmind @erlingwl @davelooi @tomilaine @skyporter
if you don't have any other comment about this PR, I will merge it and prepare a new release.

@pitbulk pitbulk merged commit 884e24f into master May 18, 2017
@borourke
Copy link

borourke commented Jul 3, 2017

This code is broken. You updated the "validate_signature" method to handle the cert_multi, but you never updated the "validate(collect_errors)" method to pass validations when idp_cert_multi exists but the others do not. I'll make a PR right now to fix. But this is the code that needs updating:

https://github.com/onelogin/ruby-saml/blob/52472021725be64e25bed93cbba2259717ed573b/lib/onelogin/ruby-saml/response.rb#L415

It needs an or statement for the idp_cert_multi settings.

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.

4 participants