Merge pull request #244 from gerardo-navarro/gerardo-navarro-refactor-saml-spec-small

Refactor helper method #post_xml from the global scope into the relevant test context

Author: fh1ch

spec/omniauth/strategies/saml_spec.rb

@@ -6,10 +6,6 @@
   end
 end
 
-def post_xml(xml = :example_response, opts = {})
-  post "/auth/saml/callback", opts.merge({'SAMLResponse' => load_xml(xml)})
-end
-
 describe OmniAuth::Strategies::SAML, :type => :strategy do
   include OmniAuth::Test::StrategyTestCase
 
@@ -118,24 +114,27 @@ def post_xml(xml = :example_response, opts = {})
   end
 
   describe 'POST /auth/saml/callback' do
-    subject { last_response }
-
     let(:xml) { :example_response }
+    let(:params) { { 'SAMLResponse' => load_xml(xml) } }
+
+    subject(:post_callback_response) do
+      post "/auth/saml/callback", params
+    end
 
     before :each do
       allow(Time).to receive(:now).and_return(Time.utc(2012, 11, 8, 20, 40, 00))
     end
 
     context "when the response is valid" do
-      before :each do
-        post_xml
-      end
-
       it "should set the uid to the nameID in the SAML response" do
+        post_callback_response
+
         expect(auth_hash['uid']).to eq '_1f6fcf6be5e13b08b1e3610e7ff59f205fbd814f23'
       end
 
       it "should set the raw info to all attributes" do
+        post_callback_response
+
         expect(auth_hash['extra']['raw_info'].all.to_hash).to eq(
           'first_name'   => ['Rajiv'],
           'last_name'    => ['Manglani'],
@@ -146,6 +145,8 @@ def post_xml(xml = :example_response, opts = {})
       end
 
       it "should set the response_object to the response object from ruby_saml response" do
+        post_callback_response
+
         expect(auth_hash['extra']['response_object']).to be_kind_of(OneLogin::RubySaml::Response)
       end
     end
@@ -154,24 +155,22 @@ def post_xml(xml = :example_response, opts = {})
       before :each do
         saml_options.delete(:assertion_consumer_service_url)
         OmniAuth.config.full_host = 'http://localhost:9080'
-        post_xml
       end
 
       it { is_expected.not_to fail_with(:invalid_ticket) }
     end
 
     context "when there is no SAMLResponse parameter" do
-      before :each do
-        post '/auth/saml/callback'
-      end
+      let(:params) { {} }
 
       it { is_expected.to fail_with(:invalid_ticket) }
     end
 
     context "when there is no name id in the XML" do
+      let(:xml) { :no_name_id }
+
       before :each do
         allow(Time).to receive(:now).and_return(Time.utc(2012, 11, 8, 23, 55, 00))
-        post_xml :no_name_id
       end
 
       it { is_expected.to fail_with(:invalid_ticket) }
@@ -180,58 +179,55 @@ def post_xml(xml = :example_response, opts = {})
     context "when the fingerprint is invalid" do
       before :each do
         saml_options[:idp_cert_fingerprint] = "00:00:00:00:00:0C:6C:A9:41:0F:6E:83:F6:D1:52:25:45:58:89:FB"
-        post_xml
       end
 
       it { is_expected.to fail_with(:invalid_ticket) }
     end
 
     context "when the digest is invalid" do
-      before :each do
-        post_xml :digest_mismatch
-      end
+      let(:xml) { :digest_mismatch }
 
       it { is_expected.to fail_with(:invalid_ticket) }
     end
 
     context "when the signature is invalid" do
-      before :each do
-        post_xml :invalid_signature
-      end
+      let(:xml) { :invalid_signature }
 
       it { is_expected.to fail_with(:invalid_ticket) }
     end
 
     context "when the response is stale" do
+      let(:xml) { :example_response }
+
       before :each do
         allow(Time).to receive(:now).and_return(Time.utc(2012, 11, 8, 20, 45, 00))
       end
 
       context "without :allowed_clock_drift option" do
-        before { post_xml :example_response }
-
         it { is_expected.to fail_with(:invalid_ticket) }
       end
 
       context "with :allowed_clock_drift option" do
         before :each do
           saml_options[:allowed_clock_drift] = 60
-          post_xml :example_response
         end
 
         it { is_expected.to_not fail_with(:invalid_ticket) }
       end
     end
 
     context "when response has custom attributes" do
+      let(:xml) { :custom_attributes }
+
       before :each do
         saml_options[:idp_cert_fingerprint] = "3B:82:F1:F5:54:FC:A8:FF:12:B8:4B:B8:16:61:1D:E4:8E:9B:E2:3C"
         saml_options[:attribute_statements] = {
           email: ["http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress"],
           first_name: ["http://schemas.xmlsoap.org/ws/2005/05/identity/claims/givenname"],
           last_name: ["http://schemas.xmlsoap.org/ws/2005/05/identity/claims/surname"]
         }
-        post_xml :custom_attributes
+
+        post_callback_response
       end
 
       it "should obey attribute statements mapping" do
@@ -245,10 +241,13 @@ def post_xml(xml = :example_response, opts = {})
     end
 
     context "when using custom user id attribute" do
+      let(:xml) { :custom_attributes }
+
       before :each do
         saml_options[:idp_cert_fingerprint] = "3B:82:F1:F5:54:FC:A8:FF:12:B8:4B:B8:16:61:1D:E4:8E:9B:E2:3C"
         saml_options[:uid_attribute] = "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress"
-        post_xml :custom_attributes
+
+        post_callback_response
       end
 
       it "should return user id attribute" do
@@ -259,11 +258,10 @@ def post_xml(xml = :example_response, opts = {})
     context "when using custom user id attribute, but it is missing" do
       before :each do
         saml_options[:uid_attribute] = "missing_attribute"
-        post_xml
       end
 
       it "should fail to authenticate" do
-        should fail_with(:invalid_ticket)
+        expect(post_callback_response).to fail_with(:invalid_ticket)
         expect(last_request.env['omniauth.error']).to be_instance_of(OmniAuth::Strategies::SAML::ValidationError)
         expect(last_request.env['omniauth.error'].message).to eq("SAML response missing 'missing_attribute' attribute")
       end