From 6d3cb7e22ea3567887fa521d8056b7d5618aa699 Mon Sep 17 00:00:00 2001 From: Horatiu Eugen Vlad Date: Mon, 5 Mar 2018 22:26:40 +0000 Subject: [PATCH] Make oauth provider login generic --- .../unreleased/oauth_generic_provider.yml | 4 +++ lib/gitlab/auth.rb | 30 +++++++++++++------ lib/gitlab/auth/database/authentication.rb | 16 ++++++++++ lib/gitlab/auth/ldap/authentication.rb | 10 ++----- lib/gitlab/auth/o_auth/authentication.rb | 21 +++++++++++++ lib/gitlab/auth/o_auth/provider.rb | 17 +++++++++++ lib/gitlab/auth/o_auth/user.rb | 2 +- spec/requests/git_http_spec.rb | 6 ++-- 8 files changed, 86 insertions(+), 20 deletions(-) create mode 100644 changelogs/unreleased/oauth_generic_provider.yml create mode 100644 lib/gitlab/auth/database/authentication.rb create mode 100644 lib/gitlab/auth/o_auth/authentication.rb 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'