Skip to content

Commit

Permalink
convert to Nokogiri
Browse files Browse the repository at this point in the history
  • Loading branch information
ccutrer committed Jul 31, 2015
1 parent b65cb55 commit 89d979d
Show file tree
Hide file tree
Showing 9 changed files with 41 additions and 52 deletions.
2 changes: 2 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
source 'https://rubygems.org/'

gemspec

gem 'byebug', platform: [:mri_20, :mri_21, :mri_22]
2 changes: 1 addition & 1 deletion lib/onelogin/saml.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
require 'zlib'
require "base64"
require "xml/libxml"
require "nokogiri"
require "xml_sec"
require "cgi"

Expand Down
6 changes: 3 additions & 3 deletions lib/onelogin/saml/base_assertion.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def in_response_to
end

def document
@document ||= LibXML::XML::Document.string(xml) if xml
@document ||= Nokogiri::XML(xml) if xml
end

def xml=(value)
Expand Down Expand Up @@ -117,11 +117,11 @@ def root_attribute_value(attribute)
end

def node_attribute_value(xpath, attribute)
document.root.find_first(xpath, Onelogin::NAMESPACES)[attribute] rescue nil
document.root.at_xpath(xpath, Onelogin::NAMESPACES)[attribute] rescue nil
end

def node_content(xpath)
document.root.find_first(xpath, Onelogin::NAMESPACES).content rescue nil
document.root.at_xpath(xpath, Onelogin::NAMESPACES).content rescue nil
end

def self.generate_unique_id(length = 42)
Expand Down
21 changes: 11 additions & 10 deletions lib/onelogin/saml/response.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,17 @@ def initialize(response, settings=nil)

begin
@xml = Base64.decode64(@response)
@document = LibXML::XML::Document.string(@xml)
@document = Nokogiri::XML(@xml)
@document.extend(XMLSecurity::SignedDocument)
rescue
# could not parse document, everything is invalid
@response = nil
return
end

@issuer = document.find_first("/samlp:Response/saml:Issuer", Onelogin::NAMESPACES).content.strip rescue nil
@issuer ||= document.find_first("/samlp:Response/saml:Assertion/saml:Issuer", Onelogin::NAMESPACES).content.strip rescue nil
@status_code = document.find_first("/samlp:Response/samlp:Status/samlp:StatusCode", Onelogin::NAMESPACES)["Value"] rescue nil
@issuer = document.at_xpath("/samlp:Response/saml:Issuer", Onelogin::NAMESPACES).content.strip rescue nil
@issuer ||= document.at_xpath("/samlp:Response/saml:Assertion/saml:Issuer", Onelogin::NAMESPACES).content.strip rescue nil
@status_code = document.at_xpath("/samlp:Response/samlp:Status/samlp:StatusCode", Onelogin::NAMESPACES)["Value"] rescue nil

process(settings) if settings
end
Expand Down Expand Up @@ -55,14 +55,15 @@ def disable_signature_validation!(settings)
end

def decrypted_document
@decrypted_document ||= LibXML::XML::Document.document(document).tap do |doc|
doc.extend(XMLSecurity::SignedDocument)
doc.decrypt!(settings)
unless @decrypted_document
document.decrypt!(settings)
@decrypted_document = document
end
@decrypted_document
end

def untrusted_find_first(xpath)
decrypted_document.find(xpath, Onelogin::NAMESPACES).first
decrypted_document.at_xpath(xpath, Onelogin::NAMESPACES)
end

def trusted_find_first(xpath)
Expand All @@ -71,7 +72,7 @@ def trusted_find_first(xpath)

def trusted_find(xpath)
trusted_roots.map do |trusted_root|
trusted_root.find("descendant-or-self::#{xpath}", Onelogin::NAMESPACES).to_a
trusted_root.xpath("descendant-or-self::#{xpath}", Onelogin::NAMESPACES).to_a
end.flatten.compact
end

Expand Down Expand Up @@ -141,7 +142,7 @@ def no_authn_context?
end

def fingerprint_from_idp
if base64_cert = decrypted_document.find_first("//ds:X509Certificate", Onelogin::NAMESPACES)
if base64_cert = decrypted_document.at_xpath("//ds:X509Certificate", Onelogin::NAMESPACES)
cert_text = Base64.decode64(base64_cert.content)
cert = OpenSSL::X509::Certificate.new(cert_text)
Digest::SHA1.hexdigest(cert.to_der)
Expand Down
32 changes: 9 additions & 23 deletions lib/xml_sec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
require 'rubygems'
require 'ffi'
require 'base64'
require "xml/libxml"
require "nokogiri"
require "openssl"
require "digest/sha1"

Expand Down Expand Up @@ -330,27 +330,27 @@ def has_signature?

def signed_roots
signatures.map do |sig|
ref = sig.find('./ds:SignedInfo/ds:Reference', Onelogin::NAMESPACES).first
ref = sig.at_xpath('./ds:SignedInfo/ds:Reference', Onelogin::NAMESPACES)
signed_element_id = ref['URI'].sub(/^#/, '')

if signed_element_id.empty?
self.root
else
xpath_id_query = %Q(ancestor::*[@ID = "#{signed_element_id}"])

ref.find(xpath_id_query, Onelogin::NAMESPACES).first
ref.at_xpath(xpath_id_query, Onelogin::NAMESPACES)
end
end.compact
end

def signatures
# we only return the first, cause our signature validation only checks the first
@signatures ||= [self.find_first("//ds:Signature", Onelogin::NAMESPACES)]
@signatures ||= [self.at_xpath("//ds:Signature", Onelogin::NAMESPACES)].compact
end

def validate(idp_cert_fingerprint, logger = nil)
# get cert from response
base64_cert = self.find_first("//ds:X509Certificate", Onelogin::NAMESPACES).content
base64_cert = self.at_xpath("//ds:X509Certificate", Onelogin::NAMESPACES).content
cert_text = Base64.decode64(base64_cert)
cert = OpenSSL::X509::Certificate.new(cert_text)

Expand All @@ -364,21 +364,8 @@ def validate(idp_cert_fingerprint, logger = nil)
end
end

# create a copy of the document with the certificate removed
doc = LibXML::XML::Document.new
# doc.encoding = self.encoding == XML::Encoding::NONE ? XML::Encoding::ISO_8859_1 : self.encoding

# for some reason xmlsec doesn't like it when its UTF-8 and has other
# characters with umlauts and the like. We will just force it to another encoding.
# This should work fine since we are just validating the signature.
doc.encoding = XML::Encoding::ISO_8859_1

doc.root = doc.import(self.root)
sigcert = doc.find_first("//ds:Signature/ds:KeyInfo", Onelogin::NAMESPACES)
sigcert.remove!

# Force encoding of the xml and the xml string for validation
xml = doc.to_s(:indent => false, :encoding => doc.encoding).encode(doc.rb_encoding)
xml = to_xml(save_with: Nokogiri::XML::Node::SaveOptions::AS_XML, encoding: "ISO-8859-1")

# validate it!
validate_doc(xml, SignedDocument.format_cert(cert))
Expand Down Expand Up @@ -432,14 +419,13 @@ def validate_doc(xml, pem)
# replaces EncryptedData nodes with decrypted copies
def decrypt!(settings)
if settings.encryption_configured?
find("//xenc:EncryptedData", Onelogin::NAMESPACES).each do |node|
xpath("//xenc:EncryptedData", Onelogin::NAMESPACES).each do |node|
decrypted_xml = decrypt_node(settings, node.to_s)
if decrypted_xml
decrypted_doc = LibXML::XML::Document.string(decrypted_xml)
decrypted_doc = Nokogiri::XML(decrypted_xml)
decrypted_node = decrypted_doc.root
decrypted_node = self.import(decrypted_node)
node.parent.next = decrypted_node
node.parent.remove!
node.parent.unlink
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion ruby-saml-mod.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ Gem::Specification.new do |s|
s.test_files = s.files.grep(%r{^(test|spec|features)/})
s.require_paths = ["lib"]

s.add_dependency('libxml-ruby', '>= 2.3.0')
s.add_dependency('nokogiri', '~> 1.6')
s.add_dependency('ffi')

s.add_development_dependency 'rake'
Expand Down
12 changes: 6 additions & 6 deletions spec/logout_request_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,18 +43,18 @@ def inflate(string)
let(:forward_url) { logout_request.forward_url }

it "includes destination in the saml:LogoutRequest attributes" do
logout_xml = LibXML::XML::Document.string(logout_request.xml)
logout_xml.find_first('/samlp:LogoutRequest', Onelogin::NAMESPACES).attributes['Destination'].should == "http://idp.example.com/saml2"
logout_xml = Nokogiri::XML(logout_request.xml)
logout_xml.at_xpath('/samlp:LogoutRequest', Onelogin::NAMESPACES)['Destination'].should == "http://idp.example.com/saml2"
end

it "properly sets the Format attribute NameID based on settings" do
logout_xml = LibXML::XML::Document.string(logout_request.xml)
logout_xml.find_first('/samlp:LogoutRequest/saml:NameID', Onelogin::NAMESPACES).attributes['Format'].should == Onelogin::Saml::NameIdentifiers::UNSPECIFIED
logout_xml = Nokogiri::XML(logout_request.xml)
logout_xml.at_xpath('/samlp:LogoutRequest/saml:NameID', Onelogin::NAMESPACES)['Format'].should == Onelogin::Saml::NameIdentifiers::UNSPECIFIED
end

it "does not include the signature in the request xml" do
logout_xml = LibXML::XML::Document.string(logout_request.xml)
logout_xml.find_first('/samlp:LogoutRequest/ds:Signature', Onelogin::NAMESPACES).should be_nil
logout_xml = Nokogiri::XML(logout_request.xml)
logout_xml.at_xpath('/samlp:LogoutRequest/ds:Signature', Onelogin::NAMESPACES).should be_nil
end

it "can sign the generated query string" do
Expand Down
14 changes: 7 additions & 7 deletions spec/logout_response_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,37 +25,37 @@
end

it "includes destination in the saml:LogoutRequest attributes" do
value = xml.find_first('/samlp:LogoutResponse', Onelogin::NAMESPACES).attributes['Destination']
value = xml.at_xpath('/samlp:LogoutResponse', Onelogin::NAMESPACES)['Destination']
expect(value).to eq "http://idp.example.com/saml2?existing=param&existing=param"
end

it "includes id in the saml:LogoutRequest attributes" do
value = xml.find_first('/samlp:LogoutResponse', Onelogin::NAMESPACES).attributes['ID']
value = xml.at_xpath('/samlp:LogoutResponse', Onelogin::NAMESPACES)['ID']
expect(value).to eq id
end

it "includes issue_instant in the saml:LogoutRequest attributes" do
value = xml.find_first('/samlp:LogoutResponse', Onelogin::NAMESPACES).attributes['IssueInstant']
value = xml.at_xpath('/samlp:LogoutResponse', Onelogin::NAMESPACES)['IssueInstant']
expect(value).to eq issue_instant
end

it "includes in_response_to in the saml:LogoutRequest attributes" do
value = xml.find_first('/samlp:LogoutResponse', Onelogin::NAMESPACES).attributes['InResponseTo']
value = xml.at_xpath('/samlp:LogoutResponse', Onelogin::NAMESPACES)['InResponseTo']
expect(value).to eq in_response_to
end

it "includes issuer tag" do
value = xml.find_first("/samlp:LogoutResponse/saml:Issuer", Onelogin::NAMESPACES).content
value = xml.at_xpath("/samlp:LogoutResponse/saml:Issuer", Onelogin::NAMESPACES).content
expect(value).to eq issuer
end

it "includes status code tag" do
value = xml.find_first("/samlp:LogoutResponse/samlp:Status/samlp:StatusCode", Onelogin::NAMESPACES).attributes['Value']
value = xml.at_xpath("/samlp:LogoutResponse/samlp:Status/samlp:StatusCode", Onelogin::NAMESPACES)['Value']
expect(value).to eq Onelogin::Saml::StatusCodes::SUCCESS_URI
end

it "includes status message tag" do
value = xml.find_first("/samlp:LogoutResponse/samlp:Status/samlp:StatusMessage", Onelogin::NAMESPACES).content
value = xml.at_xpath("/samlp:LogoutResponse/samlp:Status/samlp:StatusMessage", Onelogin::NAMESPACES).content
expect(value).to eq Onelogin::Saml::LogoutResponse::STATUS_MESSAGE
end

Expand Down
2 changes: 1 addition & 1 deletion spec/meta_data_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
)
doc = REXML::Document.new Onelogin::Saml::MetaData.create(settings)
key_descriptors = REXML::XPath.match(doc, "//KeyDescriptor")
key_descriptors.should have(2).keys
key_descriptors.length.should == 2
key_descriptors[0].attributes["use"].should == "encryption"
key_descriptors[1].attributes["use"].should == "signing"
end
Expand Down

0 comments on commit 89d979d

Please sign in to comment.