summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDouwe Maan <douwe@gitlab.com>2018-03-05 22:26:41 +0000
committerDouwe Maan <douwe@gitlab.com>2018-03-05 22:26:41 +0000
commit6ec655f5c46aab93f435ac9218c7354b0a712160 (patch)
treebc74adccb8307a36671d53de503542d3f6e7ab0a
parent62d6f1ed6648c5aa90cb43761ef8677957a6da83 (diff)
parent6d3cb7e22ea3567887fa521d8056b7d5618aa699 (diff)
downloadgitlab-ce-6ec655f5c46aab93f435ac9218c7354b0a712160.tar.gz
Merge branch 'feature/oauth_generic_provider' into 'master'
Make oauth provider login generic See merge request gitlab-org/gitlab-ce!8809
-rw-r--r--changelogs/unreleased/oauth_generic_provider.yml4
-rw-r--r--lib/gitlab/auth.rb30
-rw-r--r--lib/gitlab/auth/database/authentication.rb16
-rw-r--r--lib/gitlab/auth/ldap/authentication.rb10
-rw-r--r--lib/gitlab/auth/o_auth/authentication.rb21
-rw-r--r--lib/gitlab/auth/o_auth/provider.rb17
-rw-r--r--lib/gitlab/auth/o_auth/user.rb2
-rw-r--r--spec/requests/git_http_spec.rb6
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'