Commit 6ec655f5 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'feature/oauth_generic_provider' into 'master'

Make oauth provider login generic

See merge request gitlab-org/gitlab-ce!8809
parents 62d6f1ed 6d3cb7e2
---
title: Make oauth provider login generic
merge_request: 8809
author: Horatiu Eugen Vlad
\ No newline at end of file
...@@ -40,8 +40,8 @@ module Gitlab ...@@ -40,8 +40,8 @@ module Gitlab
end end
def find_with_user_password(login, password) def find_with_user_password(login, password)
# Avoid resource intensive login checks if password is not provided # Avoid resource intensive checks if login credentials are not provided
return unless password.present? return unless login.present? && password.present?
# Nothing to do here if internal auth is disabled and LDAP is # Nothing to do here if internal auth is disabled and LDAP is
# not configured # not configured
...@@ -50,14 +50,26 @@ module Gitlab ...@@ -50,14 +50,26 @@ module Gitlab
Gitlab::Auth::UniqueIpsLimiter.limit_user! do Gitlab::Auth::UniqueIpsLimiter.limit_user! do
user = User.by_login(login) user = User.by_login(login)
# If no user is found, or it's an LDAP server, try LDAP. return if user && !user.active?
# LDAP users are only authenticated via LDAP
if user.nil? || user.ldap_user? authenticators = []
# Second chance - try LDAP authentication
Gitlab::Auth::LDAP::Authentication.login(login, password) if user
elsif Gitlab::CurrentSettings.password_authentication_enabled_for_git? authenticators << Gitlab::Auth::OAuth::Provider.authentication(user, 'database')
user if user.active? && user.valid_password?(password)
# 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 end
authenticators.compact!
user if authenticators.find { |auth| auth.login(login, password) }
end end
end end
......
# 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
...@@ -7,7 +7,7 @@ ...@@ -7,7 +7,7 @@
module Gitlab module Gitlab
module Auth module Auth
module LDAP module LDAP
class Authentication class Authentication < Gitlab::Auth::OAuth::Authentication
def self.login(login, password) def self.login(login, password)
return unless Gitlab::Auth::LDAP::Config.enabled? return unless Gitlab::Auth::LDAP::Config.enabled?
return unless login.present? && password.present? return unless login.present? && password.present?
...@@ -28,11 +28,7 @@ module Gitlab ...@@ -28,11 +28,7 @@ module Gitlab
Gitlab::Auth::LDAP::Config.providers Gitlab::Auth::LDAP::Config.providers
end end
attr_accessor :provider, :ldap_user attr_accessor :ldap_user
def initialize(provider)
@provider = provider
end
def login(login, password) def login(login, password)
@ldap_user = adapter.bind_as( @ldap_user = adapter.bind_as(
...@@ -62,7 +58,7 @@ module Gitlab ...@@ -62,7 +58,7 @@ module Gitlab
end end
def user def user
return nil unless ldap_user return unless ldap_user
Gitlab::Auth::LDAP::User.find_by_uid_and_provider(ldap_user.dn, provider) Gitlab::Auth::LDAP::User.find_by_uid_and_provider(ldap_user.dn, provider)
end end
......
# 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
...@@ -8,11 +8,28 @@ module Gitlab ...@@ -8,11 +8,28 @@ module Gitlab
"google_oauth2" => "Google" "google_oauth2" => "Google"
}.freeze }.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 def self.providers
Devise.omniauth_providers Devise.omniauth_providers
end end
def self.enabled?(name) def self.enabled?(name)
return true if name == 'database'
providers.include?(name.to_sym) providers.include?(name.to_sym)
end end
......
...@@ -161,7 +161,7 @@ module Gitlab ...@@ -161,7 +161,7 @@ module Gitlab
def find_by_uid_and_provider def find_by_uid_and_provider
identity = Identity.with_extern_uid(auth_hash.provider, auth_hash.uid).take identity = Identity.with_extern_uid(auth_hash.provider, auth_hash.uid).take
identity && identity.user identity&.user
end end
def build_new_user def build_new_user
......
...@@ -795,9 +795,9 @@ describe 'Git HTTP requests' do ...@@ -795,9 +795,9 @@ describe 'Git HTTP requests' do
let(:path) { 'doesnt/exist.git' } let(:path) { 'doesnt/exist.git' }
before do before do
allow(Gitlab::Auth::LDAP::Config).to receive(:enabled?).and_return(true) allow(Gitlab::Auth::OAuth::Provider).to receive(:enabled?).and_return(true)
allow(Gitlab::Auth::LDAP::Authentication).to receive(:login).and_return(nil) allow_any_instance_of(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_any_instance_of(Gitlab::Auth::LDAP::Authentication).to receive(:login).with(user.username, user.password).and_return(user)
end end
it_behaves_like 'pulls require Basic HTTP Authentication' it_behaves_like 'pulls require Basic HTTP Authentication'
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment