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

Retrieves response available options from ruby-saml #165

Conversation

mberlanda
Copy link
Contributor

In SAML-Toolkits/ruby-saml#454 was introduced a whitelisting for allowed Response options.

In my previous pr #159 I added one more option.
According to me, this whitelist introduced in 4712e99 should not be a responsibility of omniauth-saml but it should be provided out of the box by ruby-saml. It looked like @md5 agreed on this point (#159 (comment))

Since 'ruby-saml' ~>1.8 integrated this change, I updated the dependency of this gem and retrieved response available options from it.

@coveralls
Copy link

coveralls commented May 23, 2018

Coverage Status

Coverage remained the same at 100.0% when pulling 3177cf1 on mberlanda:mberlanda/inherit-response-available-options into d530da4 on omniauth:master.

@mberlanda
Copy link
Contributor Author

hey @md5 @supernova32 , do you have any feedback on this pr?

@suprnova32
Copy link
Member

@mberlanda thanks for the ping, and thanks for the PR. I think it looks good, and I'm inclined to merge it. I'll wait until tomorrow, so that if @md5, or any one else has any objections, they can let us know.

@md5
Copy link
Contributor

md5 commented Oct 8, 2018

LGTM 👍

@suprnova32 suprnova32 merged commit a0eedd6 into omniauth:master Oct 8, 2018
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