More robust namespace handling. Fixes issue with ADFS.

lib/onelogin/saml/response.rb

@@ -3,17 +3,15 @@
 
 module Onelogin::Saml
   class Response
-    attr_accessor :response, :document, :logger, :settings, :namespace
+    ASSERTION = "urn:oasis:names:tc:SAML:2.0:assertion"
+    PROTOCOL  = "urn:oasis:names:tc:SAML:2.0:protocol"
+
+    attr_accessor :response, :document, :logger, :settings
 
     def initialize(response)
       raise ArgumentError.new("Response cannot be nil") if response.nil?
-      self.response  = response
-      self.document  = XMLSecurity::SignedDocument.new(Base64.decode64(response))
-      self.namespace = "saml"
-
-      if document.elements["/#{namespace}p:Response/"].nil?
-        self.namespace = "saml2"
-      end
+      self.response = response
+      self.document = XMLSecurity::SignedDocument.new(Base64.decode64(response))
     end
 
     def is_valid?
@@ -26,36 +24,40 @@ def is_valid?
 
     # The value of the user identifier as designated by the initialization request response
     def name_id
-      @name_id ||= document.elements["/#{namespace}p:Response/#{namespace}:Assertion/#{namespace}:Subject/#{namespace}:NameID"].text
+      @name_id ||= begin
+        node = REXML::XPath.first(document, "/p:Response/a:Assertion/a:Subject/a:NameID", { "p" => PROTOCOL, "a" => ASSERTION })
+        node.text
+      end
     end
 
-    # A hash of alle the attributes with the response. Assuming there is onlye one value for each key
+    # A hash of alle the attributes with the response. Assuming there is only one value for each key
     def attributes
-      saml_attribute_statements = document.elements["/#{namespace}p:Response/#{namespace}:Assertion/#{namespace}:AttributeStatement"].elements
-      statements = saml_attribute_statements.map do |child|
-        child.attributes.map do |key, attribute|
-          [attribute, child.elements.first.text]
+      @attr_statements ||= begin
+        result = {}
+
+        stmt_element = REXML::XPath.first(document, "/p:Response/a:Assertion/a:AttributeStatement", { "p" => PROTOCOL, "a" => ASSERTION })
+        stmt_element.elements.each do |attr_element|
+          name  = attr_element.attributes["Name"]
+          value = attr_element.elements.first.text
+
+          result[name] = value
         end
-      end
 
-      hash = Hash[statements.flatten(1)]
-      @attributes ||= make_hash_access_indiferent(hash)
+        result.keys.each do |key|
+          result[key.intern] = result[key]
+        end
+
+        result
+      end
     end
 
     # When this user session should expire at latest
     def session_expires_at
-      @expires_at ||= Time.parse(document.elements["/#{namespace}p:Response/#{namespace}:Assertion/#{namespace}:AuthnStatement"].attributes["SessionNotOnOrAfter"])
-    end
-
-  private
-
-    def make_hash_access_indiferent(hash)
-      sym_hash = {}
-      hash.each  do |key, value|
-        sym_hash[key.intern] = value
+      @expires_at ||= begin
+        node = REXML::XPath.first(document, "/p:Response/a:Assertion/a:AuthnStatement", { "p" => PROTOCOL, "a" => ASSERTION })
+        Time.parse(node.attributes["SessionNotOnOrAfter"]) if node && node.attributes["SessionNotOnOrAfter"]
       end
-
-      sym_hash.merge(hash)
     end
+
   end
 end

test/responses/response3.xml.base64

@@ -0,0 +1,66 @@
+PD94bWwgdmVyc2lvbj0iMS4wIj8+CjxzYW1scDpSZXNwb25zZSB4bWxuczpz
+YW1scD0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOnByb3RvY29sIiBJ
+RD0iXzZiODVkMGRkLWJmYTgtNGRlZi04MmMyLTg2MjFlMDQ1MjQ3NyIgVmVy
+c2lvbj0iMi4wIiBJc3N1ZUluc3RhbnQ9IjIwMTEtMDUtMDJUMTk6NDM6NTQu
+NjkyWiIgRGVzdGluYXRpb249Imh0dHBzOi8vZXhhbXBsZS5jb20vYWNjZXNz
+L3NhbWwiIENvbnNlbnQ9InVybjpvYXNpczpuYW1lczp0YzpTQU1MOjIuMDpj
+b25zZW50OnVuc3BlY2lmaWVkIiBJblJlc3BvbnNlVG89Il9mYjg0MThkMC01
+NzFlLTAxMmUtZWVlMC0wMDUwNTY5MjAwZDAiPgogIDxJc3N1ZXIgeG1sbnM9
+InVybjpvYXNpczpuYW1lczp0YzpTQU1MOjIuMDphc3NlcnRpb24iPmh0dHA6
+Ly9leGFtcGxlLmNvbS9zZXJ2aWNlcy90cnVzdDwvSXNzdWVyPgogIDxzYW1s
+cDpTdGF0dXM+CiAgICA8c2FtbHA6U3RhdHVzQ29kZSBWYWx1ZT0idXJuOm9h
+c2lzOm5hbWVzOnRjOlNBTUw6Mi4wOnN0YXR1czpTdWNjZXNzIi8+CiAgPC9z
+YW1scDpTdGF0dXM+CiAgPEFzc2VydGlvbiB4bWxucz0idXJuOm9hc2lzOm5h
+bWVzOnRjOlNBTUw6Mi4wOmFzc2VydGlvbiIgSUQ9Il9kYmU2YTM2NS05NTgy
+LTQ2MGYtYjRiMS0xZjc5YmY3MGY3NmIiIElzc3VlSW5zdGFudD0iMjAxMS0w
+NS0wMlQxOTo0Mzo1NC42NDVaIiBWZXJzaW9uPSIyLjAiPgogICAgPElzc3Vl
+cj5odHRwOi8vZXhhbXBsZS5jb20vc2VydmljZXMvdHJ1c3Q8L0lzc3Vlcj4K
+ICAgIDxkczpTaWduYXR1cmUgeG1sbnM6ZHM9Imh0dHA6Ly93d3cudzMub3Jn
+LzIwMDAvMDkveG1sZHNpZyMiPgogICAgICA8ZHM6U2lnbmVkSW5mbz4KICAg
+ICAgICA8ZHM6Q2Fub25pY2FsaXphdGlvbk1ldGhvZCBBbGdvcml0aG09Imh0
+dHA6Ly93d3cudzMub3JnLzIwMDEvMTAveG1sLWV4Yy1jMTRuIyIvPgogICAg
+ICAgIDxkczpTaWduYXR1cmVNZXRob2QgQWxnb3JpdGhtPSJodHRwOi8vd3d3
+LnczLm9yZy8yMDAwLzA5L3htbGRzaWcjcnNhLXNoYTEiLz4KICAgICAgICA8
+ZHM6UmVmZXJlbmNlIFVSST0iI19kYmU2YTM2NS05NTgyLTQ2MGYtYjRiMS0x
+Zjc5YmY3MGY3NmIiPgogICAgICAgICAgPGRzOlRyYW5zZm9ybXM+CiAgICAg
+ICAgICAgIDxkczpUcmFuc2Zvcm0gQWxnb3JpdGhtPSJodHRwOi8vd3d3Lncz
+Lm9yZy8yMDAwLzA5L3htbGRzaWcjZW52ZWxvcGVkLXNpZ25hdHVyZSIvPgog
+ICAgICAgICAgICA8ZHM6VHJhbnNmb3JtIEFsZ29yaXRobT0iaHR0cDovL3d3
+dy53My5vcmcvMjAwMS8xMC94bWwtZXhjLWMxNG4jIi8+CiAgICAgICAgICA8
+L2RzOlRyYW5zZm9ybXM+CiAgICAgICAgICA8ZHM6RGlnZXN0TWV0aG9kIEFs
+Z29yaXRobT0iaHR0cDovL3d3dy53My5vcmcvMjAwMC8wOS94bWxkc2lnI3No
+YTEiLz4KICAgICAgICAgIDxkczpEaWdlc3RWYWx1ZT5EaWdlc3Q8L2RzOkRp
+Z2VzdFZhbHVlPgogICAgICAgIDwvZHM6UmVmZXJlbmNlPgogICAgICA8L2Rz
+OlNpZ25lZEluZm8+CiAgICAgIDxkczpTaWduYXR1cmVWYWx1ZT5TaWduYXR1
+cmU8L2RzOlNpZ25hdHVyZVZhbHVlPgogICAgICA8S2V5SW5mbyB4bWxucz0i
+aHR0cDovL3d3dy53My5vcmcvMjAwMC8wOS94bWxkc2lnIyI+CiAgICAgICAg
+PGRzOlg1MDlEYXRhPgogICAgICAgICAgPGRzOlg1MDlDZXJ0aWZpY2F0ZT5T
+dHVmZjwvZHM6WDUwOUNlcnRpZmljYXRlPgogICAgICAgIDwvZHM6WDUwOURh
+dGE+CiAgICAgIDwvS2V5SW5mbz4KICAgIDwvZHM6U2lnbmF0dXJlPgogICAg
+PFN1YmplY3Q+CiAgICAgIDxOYW1lSUQgRm9ybWF0PSJ1cm46b2FzaXM6bmFt
+ZXM6dGM6U0FNTDoxLjE6bmFtZWlkLWZvcm1hdDplbWFpbEFkZHJlc3MiPnNv
+bWVvbmVAZXhhbXBsZS5jb208L05hbWVJRD4KICAgICAgPFN1YmplY3RDb25m
+aXJtYXRpb24gTWV0aG9kPSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6
+Y206YmVhcmVyIj4KICAgICAgICA8U3ViamVjdENvbmZpcm1hdGlvbkRhdGEg
+SW5SZXNwb25zZVRvPSJfZmI4NDE4ZDAtNTcxZS0wMTJlLWVlZTAtMDA1MDU2
+OTIwMGQwIiBOb3RPbk9yQWZ0ZXI9IjIwMTEtMDUtMDJUMTk6NDg6NTQuNzA3
+WiIgUmVjaXBpZW50PSJodHRwczovL2V4YW1wbGUuY29tL2FjY2Vzcy9zYW1s
+Ii8+CiAgICAgIDwvU3ViamVjdENvbmZpcm1hdGlvbj4KICAgIDwvU3ViamVj
+dD4KICAgIDxDb25kaXRpb25zIE5vdEJlZm9yZT0iMjAxMS0wNS0wMlQxOTo0
+Mzo1NC42NDVaIiBOb3RPbk9yQWZ0ZXI9IjIwMTEtMDUtMDJUMjA6NDM6NTQu
+NjQ1WiI+CiAgICAgIDxBdWRpZW5jZVJlc3RyaWN0aW9uPgogICAgICAgIDxB
+dWRpZW5jZT5jb25zdW1lci5leGFtcGxlLmNvbTwvQXVkaWVuY2U+CiAgICAg
+IDwvQXVkaWVuY2VSZXN0cmljdGlvbj4KICAgIDwvQ29uZGl0aW9ucz4KICAg
+IDxBdHRyaWJ1dGVTdGF0ZW1lbnQ+CiAgICAgIDxBdHRyaWJ1dGUgTmFtZT0i
+aHR0cDovL3NjaGVtYXMueG1sc29hcC5vcmcvd3MvMjAwNS8wNS9pZGVudGl0
+eS9jbGFpbXMvZW1haWxhZGRyZXNzIj4KICAgICAgICA8QXR0cmlidXRlVmFs
+dWU+c29tZW9uZUBleGFtcGxlLmNvbTwvQXR0cmlidXRlVmFsdWU+CiAgICAg
+IDwvQXR0cmlidXRlPgogICAgPC9BdHRyaWJ1dGVTdGF0ZW1lbnQ+CiAgICA8
+QXV0aG5TdGF0ZW1lbnQgQXV0aG5JbnN0YW50PSIyMDExLTA1LTAyVDE5OjQz
+OjU0LjI4NVoiIFNlc3Npb25JbmRleD0iX2RiZTZhMzY1LTk1ODItNDYwZi1i
+NGIxLTFmNzliZjcwZjc2YiI+CiAgICAgIDxBdXRobkNvbnRleHQ+CiAgICAg
+ICAgPEF1dGhuQ29udGV4dENsYXNzUmVmPnVybjpvYXNpczpuYW1lczp0YzpT
+QU1MOjIuMDphYzpjbGFzc2VzOlBhc3N3b3JkUHJvdGVjdGVkVHJhbnNwb3J0
+PC9BdXRobkNvbnRleHRDbGFzc1JlZj4KICAgICAgPC9BdXRobkNvbnRleHQ+
+CiAgICA8L0F1dGhuU3RhdGVtZW50PgogIDwvQXNzZXJ0aW9uPgo8L3NhbWxw
+OlJlc3BvbnNlPgo=

test/ruby-saml_test.rb

@@ -35,6 +35,8 @@ class RubySamlTest < Test::Unit::TestCase
       assert !response.name_id.nil?
       response = Onelogin::Saml::Response.new(response_document_2)
       assert !response.name_id.nil?
+      response = Onelogin::Saml::Response.new(response_document_3)
+      assert !response.name_id.nil?
     end
 
     context "#is_valid?" do
@@ -65,6 +67,9 @@ class RubySamlTest < Test::Unit::TestCase
       should "extract the value of the name id element" do
         response = Onelogin::Saml::Response.new(response_document)
         assert_equal "[email protected]", response.name_id
+
+        response = Onelogin::Saml::Response.new(response_document_3)
+        assert_equal "[email protected]", response.name_id
       end
     end
 
@@ -84,12 +89,20 @@ class RubySamlTest < Test::Unit::TestCase
         assert_equal "demo", response.attributes[:uid]
         assert_equal "value", response.attributes[:another_value]
       end
+
+      should "work for implicit namespaces" do
+        response = Onelogin::Saml::Response.new(response_document_3)
+        assert_equal "[email protected]", response.attributes["http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress"]
+      end
     end
 
     context "#session_expires_at" do
       should "extract the value of the SessionNotOnOrAfter attribute" do
         response = Onelogin::Saml::Response.new(response_document)
         assert response.session_expires_at.is_a?(Time)
+
+        response = Onelogin::Saml::Response.new(response_document_2)
+        assert response.session_expires_at.nil?
       end
     end
   end

test/test_helper.rb

@@ -15,4 +15,9 @@ def response_document
   def response_document_2
     @response_document2 ||= File.read(File.join(File.dirname(__FILE__), 'responses', 'response2.xml.base64'))
   end
+
+  def response_document_3
+    @response_document3 ||= File.read(File.join(File.dirname(__FILE__), 'responses', 'response3.xml.base64'))
+  end
+
 end