Merge pull request #160 from onelogin/attrs

Extended solution for Attributes method [] can raise NoMethodError

Author: Lordnibbler

Gemfile

@@ -6,8 +6,10 @@ group :test do
   if RUBY_VERSION < "1.9"
     gem "nokogiri",   "~> 1.5.0"
     gem "ruby-debug", "~> 0.10.4"
+  elsif RUBY_VERSION < "2.0"
+    gem "debugger", "~> 1.1"
   else
-    gem "debugger",   "~> 1.1"
+    gem "byebug",   "~> 2.1.1"
   end
   gem "shoulda",    "~> 2.11"
   gem "rake",       "~> 10"

README.md

@@ -154,8 +154,7 @@ The following attributes are set:
   * idp_slo_target_url
   * id_cert_fingerpint
 
-If are using saml:AttributeStatement to transfer metadata, like the user name, you can access all the attributes through response.attributes. It
-contains all the saml:AttributeStatement with its 'Name' as a indifferent key and the one saml:AttributeValue as value.
+If are using saml:AttributeStatement to transfer metadata, like the user name, you can access all the attributes through response.attributes. It contains all the saml:AttributeStatement with its 'Name' as a indifferent key and the one saml:AttributeValue as value.
 
 ```ruby
 response          = OneLogin::RubySaml::Response.new(params[:SAMLResponse])
@@ -164,6 +163,106 @@ response.settings = saml_settings
 response.attributes[:username]
 ```
 
+Imagine this saml:AttributeStatement
+
+```xml
+  <saml:AttributeStatement>
+    <saml:Attribute Name="uid">
+      <saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">demo</saml:AttributeValue>
+    </saml:Attribute>
+    <saml:Attribute Name="another_value">
+      <saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">value1</saml:AttributeValue>
+      <saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">value2</saml:AttributeValue>
+    </saml:Attribute>
+    <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">role1</saml:AttributeValue>
+    </saml:Attribute>
+    <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:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">role3</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>
+```
+
+```ruby
+pp(response.attributes)   # is an OneLogin::RubySaml::Attributes object
+# => @attributes=
+  {"uid"=>["demo"],
+   "another_value"=>["value1", "value2"],
+   "role"=>["role1", "role2", "role3"],
+   "attribute_with_nil_value"=>[nil],
+   "attribute_with_nils_and_empty_strings"=>["", "valuePresent", nil, nil]}>
+
+# Active single_value_compatibility
+OneLogin::RubySaml::Attributes.single_value_compatibility = true
+
+pp(response.attributes[:uid])
+# => "demo"
+
+pp(response.attributes[:role])
+# => "role1"
+
+pp(response.attributes.single(:role))
+# => "role1"
+
+pp(response.attributes.multi(:role))
+# => ["role1", "role2", "role3"]
+
+pp(response.attributes[:attribute_with_nil_value])
+# => nil
+
+pp(response.attributes[:attribute_with_nils_and_empty_strings])
+# => ""
+
+pp(response.attributes[:not_exists])
+# => nil
+
+pp(response.attributes.single(:not_exists))
+# => nil
+
+pp(response.attributes.multi(:not_exists))
+# => nil
+
+# Deactive single_value_compatibility
+OneLogin::RubySaml::Attributes.single_value_compatibility = false
+
+pp(response.attributes[:uid])
+# => ["demo"]
+
+pp(response.attributes[:role])
+# => ["role1", "role2", "role3"]
+
+pp(response.attributes.single(:role))
+# => "role1"
+
+pp(response.attributes.multi(:role))
+# => ["role1", "role2", "role3"]
+
+pp(response.attributes[:attribute_with_nil_value])
+# => [nil]
+
+pp(response.attributes[:attribute_with_nils_and_empty_strings])
+# => ["", "valuePresent", nil, nil]
+
+pp(response.attributes[:not_exists])
+# => nil
+
+pp(response.attributes.single(:not_exists))
+# => nil
+
+pp(response.attributes.multi(:not_exists))
+# => nil
+```
+
 ## Service Provider Metadata
 
 To form a trusted pair relationship with the IdP, the SP (you) need to provide metadata XML

lib/onelogin/ruby-saml/attributes.rb

@@ -47,7 +47,7 @@ def include?(name)
 
       # Return first value for an attribute
       def single(name)
-        attributes[canonize_name(name)].first
+        attributes[canonize_name(name)].first if include?(name)
       end
 
       # Return all values for an attribute
@@ -106,4 +106,4 @@ def attributes
       end
     end
   end
-end
+end

test/response_test.rb

@@ -243,43 +243,99 @@ class RubySamlTest < Test::Unit::TestCase
           assert_equal "demo", response.attributes[:uid]
         end
 
+        should "extract single value as string in compatibility mode off" do
+          response = OneLogin::RubySaml::Response.new(fixture(:response_with_multiple_attribute_values))
+          OneLogin::RubySaml::Attributes.single_value_compatibility = false
+          assert_equal ["demo"], response.attributes[:uid]
+          # classes are not reloaded between tests so restore default
+          OneLogin::RubySaml::Attributes.single_value_compatibility = true
+        end
+
         should "extract first of multiple values as string for b/w compatibility" do
           response = OneLogin::RubySaml::Response.new(fixture(:response_with_multiple_attribute_values))
           assert_equal 'value1', response.attributes[:another_value]
         end
 
+        should "extract first of multiple values as string for b/w compatibility in compatibility mode off" do
+          response = OneLogin::RubySaml::Response.new(fixture(:response_with_multiple_attribute_values))
+          OneLogin::RubySaml::Attributes.single_value_compatibility = false
+          assert_equal ['value1', 'value2'], response.attributes[:another_value]
+          OneLogin::RubySaml::Attributes.single_value_compatibility = true
+        end
+
         should "return array with all attributes when asked in XML order" do
           response = OneLogin::RubySaml::Response.new(fixture(:response_with_multiple_attribute_values))
           assert_equal ['value1', 'value2'], response.attributes.multi(:another_value)
         end
 
+        should "return array with all attributes when asked in XML order in compatibility mode off" do
+          response = OneLogin::RubySaml::Response.new(fixture(:response_with_multiple_attribute_values))
+          OneLogin::RubySaml::Attributes.single_value_compatibility = false
+          assert_equal ['value1', 'value2'], response.attributes.multi(:another_value)
+          OneLogin::RubySaml::Attributes.single_value_compatibility = true
+        end
+
         should "return first of multiple values when multiple Attribute tags in XML" do
           response = OneLogin::RubySaml::Response.new(fixture(:response_with_multiple_attribute_values))
           assert_equal 'role1', response.attributes[:role]
         end
 
+        should "return first of multiple values when multiple Attribute tags in XML in compatibility mode off" do
+          response = OneLogin::RubySaml::Response.new(fixture(:response_with_multiple_attribute_values))
+          OneLogin::RubySaml::Attributes.single_value_compatibility = false
+          assert_equal ['role1', 'role2', 'role3'], response.attributes[:role]
+          OneLogin::RubySaml::Attributes.single_value_compatibility = true
+        end
+
         should "return all of multiple values in reverse order when multiple Attribute tags in XML" do
           response = OneLogin::RubySaml::Response.new(fixture(:response_with_multiple_attribute_values))
           assert_equal ['role1', 'role2', 'role3'], response.attributes.multi(:role)
         end
 
+        should "return all of multiple values in reverse order when multiple Attribute tags in XML in compatibility mode off" do
+          response = OneLogin::RubySaml::Response.new(fixture(:response_with_multiple_attribute_values))
+          OneLogin::RubySaml::Attributes.single_value_compatibility = false
+          assert_equal ['role1', 'role2', 'role3'], response.attributes.multi(:role)
+          OneLogin::RubySaml::Attributes.single_value_compatibility = true
+        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 nil value correctly when not in compatibility mode off" do
+          response = OneLogin::RubySaml::Response.new(fixture(:response_with_multiple_attribute_values))
+          OneLogin::RubySaml::Attributes.single_value_compatibility = false
+          assert_equal [nil], response.attributes[:attribute_with_nil_value]
+          OneLogin::RubySaml::Attributes.single_value_compatibility = true
+        end
+
         should "return multiple values including nil and empty string" do
           response = OneLogin::RubySaml::Response.new(fixture(:response_with_multiple_attribute_values))
           assert_equal ["", "valuePresent", nil, nil], response.attributes.multi(:attribute_with_nils_and_empty_strings)
         end
 
-        should "return multiple values from [] when not in compatibility mode" do
+        should "return multiple values from [] when not in compatibility mode off" do
           response = OneLogin::RubySaml::Response.new(fixture(:response_with_multiple_attribute_values))
           OneLogin::RubySaml::Attributes.single_value_compatibility = false
           assert_equal ["", "valuePresent", nil, nil], response.attributes[:attribute_with_nils_and_empty_strings]
-          # classes are not reloaded between tests so restore default
           OneLogin::RubySaml::Attributes.single_value_compatibility = true
         end
+
+        should "check what happens when trying retrieve attribute that does not exists" do
+          response = OneLogin::RubySaml::Response.new(fixture(:response_with_multiple_attribute_values))
+          assert_equal nil, response.attributes[:attribute_not_exists]
+          assert_equal nil, response.attributes.single(:attribute_not_exists)
+          assert_equal nil, response.attributes.multi(:attribute_not_exists)
+
+          OneLogin::RubySaml::Attributes.single_value_compatibility = false
+          assert_equal nil, response.attributes[:attribute_not_exists]
+          assert_equal nil, response.attributes.single(:attribute_not_exists)
+          assert_equal nil, response.attributes.multi(:attribute_not_exists)          
+          OneLogin::RubySaml::Attributes.single_value_compatibility = true
+        end
+
       end
     end