Merge branch 'feature/oauth_generic_provider' into 'master'

Make oauth provider login generic

See merge request gitlab-org/gitlab-ce!8809

8 files changed, 86 insertions, 20 deletions

diff --git a/changelogs/unreleased/oauth_generic_provider.yml b/changelogs/unreleased/oauth_generic_provider.yml
new file mode 100644
index 00000000000..3b6f8b04529
--- /dev/null
+++ b/changelogs/unreleased/oauth_generic_provider.yml
@@ -0,0 +1,4 @@
+---
+title: Make oauth provider login generic
+merge_request: 8809
+author: Horatiu Eugen Vlad
\ No newline at end of file
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index 86393ee254d..f5ccf952cf9 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -40,8 +40,8 @@ module Gitlab
       end
 
       def find_with_user_password(login, password)
-        # Avoid resource intensive login checks if password is not provided
-        return unless password.present?
+        # Avoid resource intensive checks if login credentials are not provided
+        return unless login.present? && password.present?
 
         # Nothing to do here if internal auth is disabled and LDAP is
         # not configured
@@ -50,14 +50,26 @@ module Gitlab
         Gitlab::Auth::UniqueIpsLimiter.limit_user! do
           user = User.by_login(login)
 
-          # If no user is found, or it's an LDAP server, try LDAP.
-          #   LDAP users are only authenticated via LDAP
-          if user.nil? || user.ldap_user?
-            # Second chance - try LDAP authentication
-            Gitlab::Auth::LDAP::Authentication.login(login, password)
-          elsif Gitlab::CurrentSettings.password_authentication_enabled_for_git?
-            user if user.active? && user.valid_password?(password)
+          return if user && !user.active?
+
+          authenticators = []
+
+          if user
+            authenticators << Gitlab::Auth::OAuth::Provider.authentication(user, 'database')
+
+            # Add authenticators for all identities if user is not nil
+            user&.identities&.each do |identity|
+              authenticators << Gitlab::Auth::OAuth::Provider.authentication(user, identity.provider)
+            end
+          else
+            # If no user is provided, try LDAP.
+            #   LDAP users are only authenticated via LDAP
+            authenticators << Gitlab::Auth::LDAP::Authentication
           end
+
+          authenticators.compact!
+
+          user if authenticators.find { |auth| auth.login(login, password) }
         end
       end
 
diff --git a/lib/gitlab/auth/database/authentication.rb b/lib/gitlab/auth/database/authentication.rb
new file mode 100644
index 00000000000..260a77058a4
--- /dev/null
+++ b/lib/gitlab/auth/database/authentication.rb
@@ -0,0 +1,16 @@
+# These calls help to authenticate to OAuth provider by providing username and password
+#
+
+module Gitlab
+  module Auth
+    module Database
+      class Authentication < Gitlab::Auth::OAuth::Authentication
+        def login(login, password)
+          return false unless Gitlab::CurrentSettings.password_authentication_enabled_for_git?
+
+          user&.valid_password?(password)
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/auth/ldap/authentication.rb b/lib/gitlab/auth/ldap/authentication.rb
index cbb9cf4bb9c..e70c3ab6b46 100644
--- a/lib/gitlab/auth/ldap/authentication.rb
+++ b/lib/gitlab/auth/ldap/authentication.rb
@@ -7,7 +7,7 @@
 module Gitlab
   module Auth
     module LDAP
-      class Authentication
+      class Authentication < Gitlab::Auth::OAuth::Authentication
         def self.login(login, password)
           return unless Gitlab::Auth::LDAP::Config.enabled?
           return unless login.present? && password.present?
@@ -28,11 +28,7 @@ module Gitlab
           Gitlab::Auth::LDAP::Config.providers
         end
 
-        attr_accessor :provider, :ldap_user
-
-        def initialize(provider)
-          @provider = provider
-        end
+        attr_accessor :ldap_user
 
         def login(login, password)
           @ldap_user = adapter.bind_as(
@@ -62,7 +58,7 @@ module Gitlab
         end
 
         def user
-          return nil unless ldap_user
+          return unless ldap_user
 
           Gitlab::Auth::LDAP::User.find_by_uid_and_provider(ldap_user.dn, provider)
         end
diff --git a/lib/gitlab/auth/o_auth/authentication.rb b/lib/gitlab/auth/o_auth/authentication.rb
new file mode 100644
index 00000000000..ed03b9f8b40
--- /dev/null
+++ b/lib/gitlab/auth/o_auth/authentication.rb
@@ -0,0 +1,21 @@
+# These calls help to authenticate to OAuth provider by providing username and password
+#
+
+module Gitlab
+  module Auth
+    module OAuth
+      class Authentication
+        attr_reader :provider, :user
+
+        def initialize(provider, user = nil)
+          @provider = provider
+          @user = user
+        end
+
+        def login(login, password)
+          raise NotImplementedError
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/auth/o_auth/provider.rb b/lib/gitlab/auth/o_auth/provider.rb
index f8ab8ee1388..5fb61ffe00d 100644
--- a/lib/gitlab/auth/o_auth/provider.rb
+++ b/lib/gitlab/auth/o_auth/provider.rb
@@ -8,11 +8,28 @@ module Gitlab
           "google_oauth2"  => "Google"
         }.freeze
 
+        def self.authentication(user, provider)
+          return unless user
+          return unless enabled?(provider)
+
+          authenticator =
+            case provider
+            when /^ldap/
+              Gitlab::Auth::LDAP::Authentication
+            when 'database'
+              Gitlab::Auth::Database::Authentication
+            end
+
+          authenticator&.new(provider, user)
+        end
+
         def self.providers
           Devise.omniauth_providers
         end
 
         def self.enabled?(name)
+          return true if name == 'database'
+
           providers.include?(name.to_sym)
         end
 
diff --git a/lib/gitlab/auth/o_auth/user.rb b/lib/gitlab/auth/o_auth/user.rb
index acd785bb02d..b6a96081278 100644
--- a/lib/gitlab/auth/o_auth/user.rb
+++ b/lib/gitlab/auth/o_auth/user.rb
@@ -161,7 +161,7 @@ module Gitlab
 
         def find_by_uid_and_provider
           identity = Identity.with_extern_uid(auth_hash.provider, auth_hash.uid).take
-          identity && identity.user
+          identity&.user
         end
 
         def build_new_user
diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb
index 0467e0251b3..6dbbb1ad7bb 100644
--- a/spec/requests/git_http_spec.rb
+++ b/spec/requests/git_http_spec.rb
@@ -795,9 +795,9 @@ describe 'Git HTTP requests' do
     let(:path) { 'doesnt/exist.git' }
 
     before do
-      allow(Gitlab::Auth::LDAP::Config).to receive(:enabled?).and_return(true)
-      allow(Gitlab::Auth::LDAP::Authentication).to receive(:login).and_return(nil)
-      allow(Gitlab::Auth::LDAP::Authentication).to receive(:login).with(user.username, user.password).and_return(user)
+      allow(Gitlab::Auth::OAuth::Provider).to receive(:enabled?).and_return(true)
+      allow_any_instance_of(Gitlab::Auth::LDAP::Authentication).to receive(:login).and_return(nil)
+      allow_any_instance_of(Gitlab::Auth::LDAP::Authentication).to receive(:login).with(user.username, user.password).and_return(user)
     end
 
     it_behaves_like 'pulls require Basic HTTP Authentication'