Test: refactor test structure and hierarchy for better readability

Author: gerardo-navarro

spec/omniauth/strategies/saml_spec.rb

@@ -287,46 +287,47 @@ def post_xml(xml = :example_response, opts = {})
         expect(last_response).to be_redirect
         expect(last_response.location).to match /https:\/\/example.com\//
       end
-    end
-
-    context "when response relay state is invalid" do
-      before do
-        saml_options[:slo_default_relay_state] = "/signed-out"
-      end
 
-      [
-        "//attacker.test",
-        "javascript:alert(1)",
-      ].each do |unsafe_relay_state|
-        it "falls back to the default when the relay state is #{unsafe_relay_state}" do
-          post "/auth/saml/slo", {
+      context "when response relay state is invalid" do
+        let(:saml_options) { super().merge(slo_default_relay_state: '/signed-out') }
+        let(:params) do
+          {
             SAMLResponse: load_xml(:example_logout_response),
-            RelayState: unsafe_relay_state,
-          }, "rack.session" => {"saml_transaction_id" => "_3fef1069-d0c6-418a-b68d-6f008a4787e9"}
+            RelayState: "https://example.com/",
+          }
+        end
 
-          expect(last_response).to be_redirect
-          expect(last_response.location).to eq("/signed-out")
+        let(:opts) do
+          { "rack.session" => { "saml_transaction_id" => "_3fef1069-d0c6-418a-b68d-6f008a4787e9" } }
         end
-      end
 
-      it "allows absolute https relay states" do
-        post "/auth/saml/slo", {
-          SAMLResponse: load_xml(:example_logout_response),
-          RelayState: "https://example.com/logout",
-        }, "rack.session" => {"saml_transaction_id" => "_3fef1069-d0c6-418a-b68d-6f008a4787e9"}
+        subject(:post_slo_response) { post "/auth/saml/slo", params, opts }
 
-        expect(last_response.location).to eq("https://example.com/logout")
-      end
+        [
+          "//attacker.test",
+          "javascript:alert(1)",
+        ].each do |unsafe_relay_state|
+          context "#{unsafe_relay_state}"  do
+            let(:params) { super().merge(RelayState: unsafe_relay_state)}
 
-      it "raises when there is no safe fallback" do
-        saml_options.delete(:slo_default_relay_state)
+            it 'falls back to the default' do
+              is_expected.to be_redirect.and have_attributes(location: "/signed-out")
+            end
+          end
+        end
 
-        expect do
-          post "/auth/saml/slo", {
-            SAMLResponse: load_xml(:example_logout_response),
-            RelayState: "javascript:alert(1)",
-          }, "rack.session" => {"saml_transaction_id" => "_3fef1069-d0c6-418a-b68d-6f008a4787e9"}
-        end.to raise_error(OmniAuth::Strategies::SAML::ValidationError, "Invalid RelayState")
+        context 'when absolute https relay state' do
+          let(:params) { super().merge(RelayState: "https://example.com/logout")}
+
+          it { is_expected.to be_redirect.and have_attributes(location: "https://example.com/logout") }
+        end
+
+        context 'when there is no safe fallback' do
+          let(:saml_options) { super().except(:slo_default_relay_state) }
+          let(:params) { super().merge(RelayState: 'javascript:alert(1)')}
+
+          it { expect { post_slo_response }.to raise_error(OmniAuth::Strategies::SAML::ValidationError, "Invalid RelayState") }
+        end
       end
     end