Refactor OmniAuth::Strategies::GroupSaml

Backports refactoring from Group SAML metadata MR

ee/lib/gitlab/auth/group_saml/group_lookup.rb

@@ -33,9 +33,11 @@ module Gitlab
           path.match(path_regex).try(:[], :group)
         end
 
-        def path_from_params
-          params = Rack::Request.new(env).params
+        def params
+          @params ||= Rack::Request.new(env).params
+        end
 
+        def path_from_params
           params['group_path']
         end
       end

ee/lib/omni_auth/strategies/group_saml.rb

@@ -5,15 +5,11 @@ module OmniAuth
       option :callback_path, ->(env) { callback?(env) }
 
       def setup_phase
+        require_saml_provider
+
         # Set devise scope for custom callback URL
         env["devise.mapping"] = Devise.mappings[:user]
 
-        group_lookup = Gitlab::Auth::GroupSaml::GroupLookup.new(env)
-
-        unless group_lookup.group_saml_enabled?
-          self.class.invalid_group!(group_lookup.path)
-        end
-
         settings = Gitlab::Auth::GroupSaml::DynamicSettings.new(group_lookup.group).to_h
         env['omniauth.strategy'].options.merge!(settings)
 
@@ -33,6 +29,18 @@ module OmniAuth
       def self.callback?(env)
         env['PATH_INFO'] =~ Gitlab::PathRegex.saml_callback_regex
       end
+
+      private
+
+      def group_lookup
+        @group_lookup ||= Gitlab::Auth::GroupSaml::GroupLookup.new(env)
+      end
+
+      def require_saml_provider
+        unless group_lookup.group_saml_enabled?
+          self.class.invalid_group!(group_lookup.path)
+        end
+      end
     end
   end
 end

ee/spec/lib/omni_auth/strategies/group_saml_spec.rb

@@ -50,6 +50,13 @@ describe OmniAuth::Strategies::GroupSaml, type: :strategy do
 
         expect(auth_hash[:info]['email']).to eq("user@example.com")
       end
+
+      it 'sets omniauth setings from configured settings' do
+        post "/groups/my-group/-/saml/callback", SAMLResponse: saml_response
+
+        options = last_request.env['omniauth.strategy'].options
+        expect(options['idp_cert_fingerprint']).to eq fingerprint
+      end
     end
 
     context 'with invalid SAMLResponse' do