Align the behavior of the success? method on Response and LogoutResponse

Author: pitbulk

lib/onelogin/ruby-saml/logoutresponse.rb

@@ -52,10 +52,7 @@ def initialize(response, settings = nil, options = {})
       # @raise [ValidationError] if soft == false and validation fails
       #
       def success?
-        unless status_code == "urn:oasis:names:tc:SAML:2.0:status:Success"
-          return append_error("Bad status code. Expected <urn:oasis:names:tc:SAML:2.0:status:Success>, but was: <#@status_code>")
-        end
-        true
+        return status_code == "urn:oasis:names:tc:SAML:2.0:status:Success"
       end
 
       # @return [String|nil] Gets the InResponseTo attribute from the Logout Response if exists.
@@ -65,7 +62,7 @@ def in_response_to
           node = REXML::XPath.first(
             document,
             "/p:LogoutResponse",
-            { "p" => PROTOCOL, "a" => ASSERTION }
+            { "p" => PROTOCOL }
           )
           node.nil? ? nil : node.attributes['InResponseTo']
         end
@@ -88,7 +85,7 @@ def issuer
       #
       def status_code
         @status_code ||= begin
-          node = REXML::XPath.first(document, "/p:LogoutResponse/p:Status/p:StatusCode", { "p" => PROTOCOL, "a" => ASSERTION })
+          node = REXML::XPath.first(document, "/p:LogoutResponse/p:Status/p:StatusCode", { "p" => PROTOCOL })
           node.nil? ? nil : node.attributes["Value"]
         end
       end
@@ -98,7 +95,7 @@ def status_message
           node = REXML::XPath.first(
             document,
             "/p:LogoutResponse/p:Status/p:StatusMessage",
-            { "p" => PROTOCOL, "a" => ASSERTION }
+            { "p" => PROTOCOL }
           )
           Utils.element_text(node)
         end

test/logout_responses/logoutresponse_fixtures.rb

@@ -44,6 +44,25 @@ def unsuccessful_logout_response_document(opts = {})
       </samlp:LogoutResponse>"
 end
 
+def unsuccessful_logout_response_with_message_document(opts = {})
+  opts = default_logout_response_opts.merge(opts)
+
+  "<samlp:LogoutResponse
+        xmlns:samlp=\"urn:oasis:names:tc:SAML:2.0:protocol\"
+        ID=\"#{random_id}\" Version=\"2.0\"
+        IssueInstant=\"#{opts[:issue_instant]}\"
+        Destination=\"#{opts[:settings].single_logout_service_url}\"
+        InResponseTo=\"#{opts[:uuid]}\">
+      <saml:Issuer xmlns:saml=\"urn:oasis:names:tc:SAML:2.0:assertion\">#{opts[:settings].issuer}</saml:Issuer>
+      <samlp:Status xmlns:samlp=\"urn:oasis:names:tc:SAML:2.0:protocol\">
+      <samlp:StatusCode xmlns:samlp=\"urn:oasis:names:tc:SAML:2.0:protocol\"
+          Value=\"urn:oasis:names:tc:SAML:2.0:status:Requester\">
+      </samlp:StatusCode>
+      <samlp:StatusMessage>Logoutrequest expired</samlp:StatusMessage>
+      </samlp:Status>
+      </samlp:LogoutResponse>"
+end
+
 def invalid_xml_logout_response_document
   "<samlp:SomethingAwful
         xmlns:samlp=\"urn:oasis:names:tc:SAML:2.0:protocol\"

test/logoutresponse_test.rb

@@ -107,15 +107,22 @@ class RubySamlTest < Minitest::Test
           assert_includes logoutresponse.errors, "The InResponseTo of the Logout Response: #{logoutresponse.in_response_to}, does not match the ID of the Logout Request sent by the SP: #{expected_request_id}"
         end
 
-        it "invalidate logout response with wrong request status" do
+        it "invalidate logout response with unexpected request status" do
           logoutresponse = OneLogin::RubySaml::Logoutresponse.new(unsuccessful_logout_response_document, settings)
 
           assert !logoutresponse.success?
           assert !logoutresponse.validate
-          assert_includes logoutresponse.errors, "Bad status code. Expected <urn:oasis:names:tc:SAML:2.0:status:Success>, but was: <urn:oasis:names:tc:SAML:2.0:status:Requester>"
           assert_includes logoutresponse.errors, "The status code of the Logout Response was not Success, was Requester"
         end
 
+        it "invalidate logout response with unexpected request status and status message" do
+          logoutresponse = OneLogin::RubySaml::Logoutresponse.new(unsuccessful_logout_response_with_message_document, settings)
+
+          assert !logoutresponse.success?
+          assert !logoutresponse.validate
+          assert_includes logoutresponse.errors, "The status code of the Logout Response was not Success, was Requester -> Logoutrequest expired"
+        end
+
         it "invalidate logout response when in lack of issuer setting" do
           bad_settings = settings
           bad_settings.issuer = nil
@@ -137,7 +144,7 @@ class RubySamlTest < Minitest::Test
           logoutresponse = OneLogin::RubySaml::Logoutresponse.new(unsuccessful_logout_response_document, settings)
           collect_errors = true
           assert !logoutresponse.validate(collect_errors)
-          assert_includes logoutresponse.errors, "Bad status code. Expected <urn:oasis:names:tc:SAML:2.0:status:Success>, but was: <urn:oasis:names:tc:SAML:2.0:status:Requester>"
+          assert_includes logoutresponse.errors, "The status code of the Logout Response was not Success, was Requester"
           assert_includes logoutresponse.errors, "Doesn't match the issuer, expected: <#{logoutresponse.settings.idp_entity_id}>, but was: <http://app.muda.no>"
         end
 
@@ -185,14 +192,14 @@ class RubySamlTest < Minitest::Test
           logoutresponse = OneLogin::RubySaml::Logoutresponse.new(unsuccessful_logout_response_document, settings)
 
           assert_raises(OneLogin::RubySaml::ValidationError) { logoutresponse.validate }
-          assert_includes logoutresponse.errors, "Bad status code. Expected <urn:oasis:names:tc:SAML:2.0:status:Success>, but was: <urn:oasis:names:tc:SAML:2.0:status:Requester>"
+          assert_includes logoutresponse.errors, "The status code of the Logout Response was not Success, was Requester"
         end
 
         it "raise validation error when in bad state" do
           # no settings
           logoutresponse = OneLogin::RubySaml::Logoutresponse.new(unsuccessful_logout_response_document, settings)
           assert_raises(OneLogin::RubySaml::ValidationError) { logoutresponse.validate }
-          assert_includes logoutresponse.errors, "Bad status code. Expected <urn:oasis:names:tc:SAML:2.0:status:Success>, but was: <urn:oasis:names:tc:SAML:2.0:status:Requester>"
+          assert_includes logoutresponse.errors, "The status code of the Logout Response was not Success, was Requester"
         end
 
         it "raise validation error when in lack of issuer setting" do