dlindahl / omniauth-cas

A CAS OmniAuth Strategy
MIT License
88 stars 79 forks source link

Remove nokogiri dependency and use REXML for XML parsing #40

Closed GUI closed 3 years ago

GUI commented 8 years ago

This removes nokogiri as a dependency in favor of Ruby's built-in REXML parser. Since nokogiri can be a somewhat large dependency that needs to be compiled, this removes that as a hard dependency for using omniauth-cas.

I'm not sure if you'd be interested in this pull request, but one thing that led me down this path was some of the recent security updates in libxml requiring nokogiri to be updated: https://github.com/sparklemotion/nokogiri/blob/v1.6.6.4/CHANGELOG.rdoc#1664--2015-11-19 I have nothing against Nokogiri (I use it myself on some projects), and security issues are probably going to crop up regardless of the library, but nokogiri is a bit of an extra hassle since system-wide libxml updates don't fix nokogiri's bundled version (although it looks like they're looking to change that in Nokogiri 1.7). In any case, since omniauth-cas doesn't seem to be doing a ton of XML parsing, I was just looking to simplify my dependencies in several apps that don't need Nokogiri for anything else.

This changes the documented response of user_info being nil for invalid responses, but the tests seem to contradict this, since it was always returning an empty hash for failure responses. This stems from the fact that I don't think find_authentication_success would ever return nil because Nokogiri::XML::XPath::SyntaxError would never be thrown (that would only be thrown if the xpath expression was invalid). So even if the document didn't match the XPath expression, Nokogiri always returned an empty result, rather than nil. So functionally, I don't think anything's changed, it just fixes the documentation of user_info.

dlindahl commented 8 years ago

I no longer have access to a CAS server and therefore am no longer fit to maintain this project.

If you would like to volunteer to be a maintainer of this project, please let me know by opening an Issue.

vjt commented 3 years ago

Nokogiri works well and there is no need as of now to change documented responses.

Thanks anyway