Merge pull request #15 from gerardo-navarro/gerardo-navarro-remove-saml-relay-state-validation

Restore SLO logout response redirect handling

Author: gerardo-navarro

lib/omniauth/strategies/saml.rb

@@ -170,17 +170,9 @@ def slo_relay_state
           relay_state = request.params["RelayState"]
 
           return relay_state if valid_slo_relay_state?(relay_state)
-
-          default_relay_state = default_slo_relay_state
-
-          if default_relay_state.nil?
-            raise OmniAuth::Strategies::SAML::ValidationError.new("Invalid RelayState")
-          end
-
-          default_relay_state
-        else
-          default_slo_relay_state
         end
+
+        default_slo_relay_state
       end
 
       def valid_slo_relay_state?(relay_state)

spec/omniauth/strategies/saml_spec.rb

@@ -303,9 +303,11 @@ def post_xml(xml = :example_response, opts = {})
       context "when relay state is an absolute https URL" do
         let(:relay_state) { "https://example.com/" }
 
-        it "raises an invalid relay state error" do
-          expect { post_slo_response }.
-            to raise_error(OmniAuth::Strategies::SAML::ValidationError, "Invalid RelayState")
+        it "redirects without a location header" do
+          post_slo_response
+
+          expect(last_response).to be_redirect
+          expect(last_response.headers.fetch("Location")).to be_nil
         end
       end
 
@@ -335,7 +337,12 @@ def post_xml(xml = :example_response, opts = {})
         context "when response relay state is invalid" do
           let(:relay_state) { "javascript:alert(1)" }
 
-          it { expect { post_slo_response }.to raise_error(OmniAuth::Strategies::SAML::ValidationError, "Invalid RelayState") }
+          it "redirects without a location header" do
+            post_slo_response
+
+            expect(last_response).to be_redirect
+            expect(last_response.headers.fetch("Location")).to be_nil
+          end
         end
       end
     end
@@ -452,9 +459,12 @@ def post_xml(xml = :example_response, opts = {})
             }
           end
 
-          it "raises an error" do
-            expect { subject }.
-              to raise_error(OmniAuth::Strategies::SAML::ValidationError, "Invalid RelayState")
+          it "redirects without including a RelayState parameter" do
+            subject
+
+            expect(last_response).to be_redirect
+            expect(last_response.location).to match %r{https://idp\.sso\.example\.com/signoff/29490}
+            expect(last_response.location).not_to match(/RelayState=/)
           end
         end
       end
@@ -634,7 +644,13 @@ def test_default_relay_state(static_default_relay_state = nil, &block_default_re
       let(:params) { { RelayState: "//example.com" } }
       subject { post "/auth/saml/spslo", params }
 
-      it { expect { subject }.to raise_error(OmniAuth::Strategies::SAML::ValidationError, "Invalid RelayState") }
+      it "redirects without including a RelayState parameter" do
+        subject
+
+        expect(last_response).to be_redirect
+        expect(last_response.location).to match %r{https://idp\.sso\.example\.com/signoff/29490}
+        expect(last_response.location).not_to match(/RelayState=/)
+      end
     end
 
     it "should give not implemented without an idp_slo_service_url" do