Restored support for NIL as well as empty AttributeValues

dividedmind · Jun 6, 2014 · ec7d7ba · ec7d7ba
1 parent 9eab546
commit ec7d7ba
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 7 deletions.
diff --git a/lib/onelogin/ruby-saml/attribute_value.rb b/lib/onelogin/ruby-saml/attribute_value.rb
@@ -4,11 +4,13 @@ module RubySaml
     # Wrapper for AttributeValue with multiple values
     # It is subclass of String to be backwards compatible
     # Use AttributeValue#values to get all values as an array
-    class AttributeValue < String
-      attr_accessor :values
-      def initialize(str="", values=[])
+    module AttributeValue
+      def values
+        @values ||= []
+        @values
+      end
+      def values=(values)
         @values = values
-        super(str)
       end
     end
   end

diff --git a/lib/onelogin/ruby-saml/response.rb b/lib/onelogin/ruby-saml/response.rb
@@ -60,10 +60,17 @@ def attributes
 
           stmt_element.elements.each do |attr_element|
             name  = attr_element.attributes["Name"]
-            values = attr_element.elements.collect(&:text)
+            values = attr_element.elements.collect{|e|
+              # SAMLCore requires that nil AttributeValues MUST contain xsi:nil XML attribute set to "true" or "1"
+              # otherwise the value is to be regarded as empty.
+              ["true", "1"].include?(e.attributes['xsi:nil']) ? nil : e.text.to_s
+            }
+
+            # Monkey-patch first value to contain all values (so that the type is retained)
+            attr_value = values.first
+            attr_value.extend AttributeValue
+            attr_value.values = values.reverse # retain XML order
 
-            # Set up a string-like wrapper for the values array
-            attr_value = AttributeValue.new(values.first, values.reverse)
             # Merge values if the Attribute has already been seen
             if result[name]
               attr_value.values += result[name].values

diff --git a/test/response_test.rb b/test/response_test.rb
@@ -249,6 +249,16 @@ class RubySamlTest < Test::Unit::TestCase
           response = OneLogin::RubySaml::Response.new(fixture(:response_with_multiple_attribute_values))
           assert_equal ['role2', 'role1'], response.attributes[:role].values
         end
+
+        should "return nil value correctly" do
+          response = OneLogin::RubySaml::Response.new(fixture(:response_with_multiple_attribute_values))
+          assert_nil response.attributes[:attribute_with_nil_value]
+        end
+
+        should "return multiple values including nil and empty string" do
+          response = OneLogin::RubySaml::Response.new(fixture(:response_with_multiple_attribute_values))
+          assert_equal [nil, nil, "valuePresent", ""], response.attributes[:attribute_with_nils_and_empty_strings].values
+        end
       end
     end
 

diff --git a/test/responses/response_with_multiple_attribute_values.xml b/test/responses/response_with_multiple_attribute_values.xml
@@ -52,6 +52,15 @@
       <saml:Attribute Name="role">
         <saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">role2</saml:AttributeValue>
       </saml:Attribute>
+      <saml:Attribute Name="attribute_with_nil_value">
+        <saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:nil="true"/>
+      </saml:Attribute>
+      <saml:Attribute Name="attribute_with_nils_and_empty_strings">
+        <saml:AttributeValue/>
+        <saml:AttributeValue>valuePresent</saml:AttributeValue>
+        <saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:nil="true"/>
+        <saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:nil="1"/>
+      </saml:Attribute>
     </saml:AttributeStatement>
   </saml:Assertion>
 </samlp:Response>