Merge branch 'security-sarcila-verify-saml-request-origin-12-1' into '12-1-stable'

Check that SAML identity linking validates the origin of the request

See merge request gitlab/gitlabhq!3376

4 files changed, 97 insertions, 2 deletions

diff --git a/lib/gitlab/auth/omniauth_identity_linker_base.rb b/lib/gitlab/auth/omniauth_identity_linker_base.rb
index c620fc5d6bd..a6c247f31a7 100644
--- a/lib/gitlab/auth/omniauth_identity_linker_base.rb
+++ b/lib/gitlab/auth/omniauth_identity_linker_base.rb
@@ -3,12 +3,13 @@
 module Gitlab
   module Auth
     class OmniauthIdentityLinkerBase
-      attr_reader :current_user, :oauth
+      attr_reader :current_user, :oauth, :session
 
-      def initialize(current_user, oauth)
+      def initialize(current_user, oauth, session = {})
         @current_user = current_user
         @oauth = oauth
         @changed = false
+        @session = session
       end
 
       def link
diff --git a/lib/gitlab/auth/saml/identity_linker.rb b/lib/gitlab/auth/saml/identity_linker.rb
index ae0d6dded4e..93195c3189f 100644
--- a/lib/gitlab/auth/saml/identity_linker.rb
+++ b/lib/gitlab/auth/saml/identity_linker.rb
@@ -4,6 +4,30 @@ module Gitlab
   module Auth
     module Saml
       class IdentityLinker < OmniauthIdentityLinkerBase
+        extend ::Gitlab::Utils::Override
+
+        UnverifiedRequest = Class.new(StandardError)
+
+        override :link
+        def link
+          raise_unless_request_is_gitlab_initiated! if unlinked?
+
+          super
+        end
+
+        protected
+
+        def raise_unless_request_is_gitlab_initiated!
+          raise UnverifiedRequest unless valid_gitlab_initiated_request?
+        end
+
+        def valid_gitlab_initiated_request?
+          OriginValidator.new(session).gitlab_initiated?(saml_response)
+        end
+
+        def saml_response
+          oauth.fetch(:extra, {}).fetch(:response_object, {})
+        end
       end
     end
   end
diff --git a/lib/gitlab/auth/saml/origin_validator.rb b/lib/gitlab/auth/saml/origin_validator.rb
new file mode 100644
index 00000000000..4ecc688888f
--- /dev/null
+++ b/lib/gitlab/auth/saml/origin_validator.rb
@@ -0,0 +1,41 @@
+# frozen_string_literal: true
+
+module Gitlab
+  module Auth
+    module Saml
+      class OriginValidator
+        AUTH_REQUEST_SESSION_KEY = "last_authn_request_id".freeze
+
+        def initialize(session)
+          @session = session || {}
+        end
+
+        def store_origin(authn_request)
+          session[AUTH_REQUEST_SESSION_KEY] = authn_request.uuid
+        end
+
+        def gitlab_initiated?(saml_response)
+          return false if identity_provider_initiated?(saml_response)
+
+          matches?(saml_response)
+        end
+
+        private
+
+        attr_reader :session
+
+        def matches?(saml_response)
+          saml_response.in_response_to == expected_request_id
+        end
+
+        def identity_provider_initiated?(saml_response)
+          saml_response.in_response_to.blank?
+        end
+
+        def expected_request_id
+          session[AUTH_REQUEST_SESSION_KEY]
+        end
+      end
+    end
+  end
+end
diff --git a/lib/omni_auth/strategies/saml.rb b/lib/omni_auth/strategies/saml.rb
new file mode 100644
index 00000000000..ebe062f17e0
--- /dev/null
+++ b/lib/omni_auth/strategies/saml.rb
@@ -0,0 +1,29 @@
+# frozen_string_literal: true
+
+module OmniAuth
+  module Strategies
+    class SAML
+      extend ::Gitlab::Utils::Override
+
+      # NOTE: This method duplicates code from omniauth-saml
+      #       so that we can access authn_request to store it
+      #       See: https://github.com/omniauth/omniauth-saml/issues/172
+      override :request_phase
+      def request_phase
+        authn_request = OneLogin::RubySaml::Authrequest.new
+
+        store_authn_request_id(authn_request)
+
+        with_settings do |settings|
+          redirect(authn_request.create(settings, additional_params_for_authn_request))
+        end
+      end
+
+      private
+
+      def store_authn_request_id(authn_request)
+        Gitlab::Auth::Saml::OriginValidator.new(session).store_origin(authn_request)
+      end
+    end
+  end
+end