From 01b791237cf6a1b7deaee3da3df6541e0b5107d1 Mon Sep 17 00:00:00 2001
From: Jan-Willem van der Meer <mail@jewilmeer.nl>
Date: Mon, 13 Oct 2014 17:24:05 +0200
Subject: [PATCH] Refactor lib files for multiple LDAP groups
---
lib/gitlab/auth.rb | 6 +-
lib/gitlab/ldap/access.rb | 32 +++++---
lib/gitlab/ldap/adapter.rb | 63 ++++-----------
lib/gitlab/ldap/authentication.rb | 68 ++++++++++++++++
lib/gitlab/ldap/config.rb | 115 ++++++++++++++++++++++++++++
lib/gitlab/ldap/person.rb | 34 ++++----
lib/gitlab/ldap/user.rb | 48 +++---------
spec/lib/gitlab/ldap/access_spec.rb | 26 +++----
8 files changed, 262 insertions(+), 130 deletions(-)
create mode 100644 lib/gitlab/ldap/authentication.rb
create mode 100644 lib/gitlab/ldap/config.rb
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index 955abc1bedd..f97c0247b6f 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -3,11 +3,13 @@ module Gitlab
def find(login, password)
user = User.find_by(email: login) || User.find_by(username: 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
- return nil unless ldap_conf.enabled
+ return nil unless Gitlab::LDAP::Config.enabled?
- Gitlab::LDAP::User.authenticate(login, password)
+ Gitlab::LDAP::Authentication.login(login, password)
else
user if user.valid_password?(password)
end
diff --git a/lib/gitlab/ldap/access.rb b/lib/gitlab/ldap/access.rb
index d2235d2e3bc..111c750226f 100644
--- a/lib/gitlab/ldap/access.rb
+++ b/lib/gitlab/ldap/access.rb
@@ -1,18 +1,21 @@
+# LDAP authorization model
+#
+# * Check if we are allowed access (not blocked)
+#
module Gitlab
module LDAP
class Access
- attr_reader :adapter
+ attr_reader :adapter, :provider, :user
- def self.open(&block)
- Gitlab::LDAP::Adapter.open do |adapter|
- block.call(self.new(adapter))
+ def self.open(user, &block)
+ Gitlab::LDAP::Adapter.open(user.provider) do |adapter|
+ block.call(self.new(user, adapter))
end
end
def self.allowed?(user)
- self.open do |access|
- if access.allowed?(user)
- # GitLab EE LDAP code goes here
+ self.open(user) do |access|
+ if access.allowed?
user.last_credential_check_at = Time.now
user.save
true
@@ -22,21 +25,26 @@ module Gitlab
end
end
- def initialize(adapter=nil)
+ def initialize(user, adapter=nil)
@adapter = adapter
+ @user = user
+ @provider = user.provider
end
- def allowed?(user)
+ def allowed?
if Gitlab::LDAP::Person.find_by_dn(user.extern_uid, adapter)
- if Gitlab.config.ldap.active_directory
- !Gitlab::LDAP::Person.disabled_via_active_directory?(user.extern_uid, adapter)
- end
+ return true unless ldap_config.active_directory
+ !Gitlab::LDAP::Person.disabled_via_active_directory?(user.extern_uid, adapter)
else
false
end
rescue
false
end
+
+ def adapter
+ @adapter ||= Gitlab::LDAP::Adapter.new(provider)
+ end
end
end
end
diff --git a/lib/gitlab/ldap/adapter.rb b/lib/gitlab/ldap/adapter.rb
index 68ac1b22909..c4d0a20d89a 100644
--- a/lib/gitlab/ldap/adapter.rb
+++ b/lib/gitlab/ldap/adapter.rb
@@ -1,52 +1,25 @@
module Gitlab
module LDAP
class Adapter
- attr_reader :ldap
+ attr_reader :provider, :ldap
- def self.open(&block)
- Net::LDAP.open(adapter_options) do |ldap|
- block.call(self.new(ldap))
+ def self.open(provider, &block)
+ Net::LDAP.open(config(provider).adapter_options) do |ldap|
+ block.call(self.new(provider, ldap))
end
end
- def self.config
- Gitlab.config.ldap
+ def self.config(provider)
+ Gitlab::LDAP::Config.new(provider)
end
- def self.adapter_options
- encryption =
- case config['method'].to_s
- when 'ssl'
- :simple_tls
- when 'tls'
- :start_tls
- else
- nil
- end
-
- options = {
- host: config['host'],
- port: config['port'],
- encryption: encryption
- }
-
- auth_options = {
- auth: {
- method: :simple,
- username: config['bind_dn'],
- password: config['password']
- }
- }
-
- if config['password'] || config['bind_dn']
- options.merge!(auth_options)
- end
- options
+ def initialize(provider, ldap=nil)
+ @provider = provider
+ @ldap = ldap || Net::LDAP.new(config.adapter_options)
end
-
- def initialize(ldap=nil)
- @ldap = ldap || Net::LDAP.new(self.class.adapter_options)
+ def config
+ Gitlab::LDAP::Config.new(provider)
end
def users(field, value)
@@ -57,13 +30,13 @@ module Gitlab
}
else
options = {
- base: config['base'],
+ base: config.base,
filter: Net::LDAP::Filter.eq(field, value)
}
end
- if config['user_filter'].present?
- user_filter = Net::LDAP::Filter.construct(config['user_filter'])
+ if config.user_filter.present?
+ user_filter = Net::LDAP::Filter.construct(config.user_filter)
options[:filter] = if options[:filter]
Net::LDAP::Filter.join(options[:filter], user_filter)
@@ -77,7 +50,7 @@ module Gitlab
end
entries.map do |entry|
- Gitlab::LDAP::Person.new(entry)
+ Gitlab::LDAP::Person.new(entry, provider)
end
end
@@ -105,12 +78,6 @@ module Gitlab
results
end
end
-
- private
-
- def config
- @config ||= self.class.config
- end
end
end
end
diff --git a/lib/gitlab/ldap/authentication.rb b/lib/gitlab/ldap/authentication.rb
new file mode 100644
index 00000000000..0eca9b26133
--- /dev/null
+++ b/lib/gitlab/ldap/authentication.rb
@@ -0,0 +1,68 @@
+# This calls helps to authenticate to LDAP by providing username and password
+#
+# Since multiple LDAP servers are supported, it will loop through all of them
+# until a valid bind is found
+#
+
+module Gitlab
+ module LDAP
+ class Authentication
+ def self.login(login, password)
+ return unless Gitlab::LDAP::Config.enabled?
+ return unless login.present? && password.present?
+
+ auth = nil
+ # loop through providers until valid bind
+ providers.find do |provider|
+ auth = new(provider)
+ auth.login(login, password) # true will exit the loop
+ end
+
+ auth.user
+ end
+
+ def self.providers
+ Gitlab::LDAP::Config.providers
+ end
+
+ attr_accessor :provider, :ldap_user
+
+ def initialize(provider)
+ @provider = provider
+ end
+
+ def login(login, password)
+ @ldap_user = adapter.bind_as(
+ filter: user_filter(login),
+ size: 1,
+ password: password
+ )
+ end
+
+ def adapter
+ OmniAuth::LDAP::Adaptor.new(config.options)
+ end
+
+ def config
+ Gitlab::LDAP::Config.new(provider)
+ end
+
+ def user_filter(login)
+ Net::LDAP::Filter.eq(config.uid, login).tap do |filter|
+ # Apply LDAP user filter if present
+ if config.user_filter.present?
+ Net::LDAP::Filter.join(
+ filter,
+ Net::LDAP::Filter.construct(config.user_filter)
+ )
+ end
+ end
+ end
+
+ def user
+ return nil unless ldap_user
+ Gitlab::LDAP::User.find_by_uid_and_provider(ldap_user.dn, provider)
+ end
+ end
+ end
+end
\ No newline at end of file
diff --git a/lib/gitlab/ldap/config.rb b/lib/gitlab/ldap/config.rb
new file mode 100644
index 00000000000..697b66dcdaa
--- /dev/null
+++ b/lib/gitlab/ldap/config.rb
@@ -0,0 +1,115 @@
+# Load a specific server configuration
+module Gitlab
+ module LDAP
+ class Config
+ attr_accessor :provider, :options
+
+ def self.enabled?
+ Gitlab.config.ldap.enabled
+ end
+
+ def self.servers
+ Gitlab.config.ldap.servers
+ end
+
+ def self.providers
+ servers.map &:provider_name
+ end
+
+ def initialize(provider)
+ @provider = provider
+ invalid_provider unless valid_provider?
+ @options = config_for(provider)
+ end
+
+ def enabled?
+ base_config.enabled
+ end
+
+ def adapter_options
+ {
+ host: options['host'],
+ port: options['port'],
+ encryption: encryption
+ }.tap do |options|
+ options.merge!(auth_options) if has_auth?
+ end
+ end
+
+ def base
+ options['base']
+ end
+
+ def uid
+ options['uid']
+ end
+
+ def sync_ssh_keys?
+ sync_ssh_keys.present?
+ end
+
+ # The LDAP attribute in which the ssh keys are stored
+ def sync_ssh_keys
+ options['sync_ssh_keys']
+ end
+
+ def user_filter
+ options['user_filter']
+ end
+
+ def group_base
+ options['group_base']
+ end
+
+ def admin_group
+ options['admin_group']
+ end
+
+ def active_directory
+ options['active_directory']
+ end
+
+ protected
+ def base_config
+ Gitlab.config.ldap
+ end
+
+ def config_for(provider)
+ base_config.servers.find { |server| server.provider_name == provider }
+ end
+
+ def encryption
+ case options['method'].to_s
+ when 'ssl'
+ :simple_tls
+ when 'tls'
+ :start_tls
+ else
+ nil
+ end
+ end
+
+ def valid_provider?
+ self.class.providers.include?(provider)
+ end
+
+ def invalid_provider
+ raise "Unknown provider (#{provider}). Available providers: #{self.class.providers}"
+ end
+
+ def auth_options
+ {
+ auth: {
+ method: :simple,
+ username: options['bind_dn'],
+ password: options['password']
+ }
+ }
+ end
+
+ def has_auth?
+ options['password'] || options['bind_dn']
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/ldap/person.rb b/lib/gitlab/ldap/person.rb
index 87c3d711db4..a35fd22073e 100644
--- a/lib/gitlab/ldap/person.rb
+++ b/lib/gitlab/ldap/person.rb
@@ -6,24 +6,24 @@ module Gitlab
# Source: http://ctogonewild.com/2009/09/03/bitmask-searches-in-ldap/
AD_USER_DISABLED = Net::LDAP::Filter.ex("userAccountControl:1.2.840.113556.1.4.803", "2")
- def self.find_by_uid(uid, adapter=nil)
- adapter ||= Gitlab::LDAP::Adapter.new
- adapter.user(config.uid, uid)
+ attr_accessor :entry, :provider
+
+ def self.find_by_uid(uid, adapter)
+ adapter.user(Gitlab.config.ldap.uid, uid)
end
- def self.find_by_dn(dn, adapter=nil)
- adapter ||= Gitlab::LDAP::Adapter.new
+ def self.find_by_dn(dn, adapter)
adapter.user('dn', dn)
end
- def self.disabled_via_active_directory?(dn, adapter=nil)
- adapter ||= Gitlab::LDAP::Adapter.new
+ def self.disabled_via_active_directory?(dn, adapter)
adapter.dn_matches_filter?(dn, AD_USER_DISABLED)
end
- def initialize(entry)
+ def initialize(entry, provider)
Rails.logger.debug { "Instantiating #{self.class.name} with LDIF:\n#{entry.to_ldif}" }
@entry = entry
+ @provider = provider
end
def name
@@ -38,22 +38,30 @@ module Gitlab
uid
end
+ def email
+ entry.try(:mail)
+ end
+
def dn
entry.dn
end
+ def ssh_keys
+ if config.sync_ssh_keys? && entry.respond_to?(config.sync_ssh_keys)
+ entry[config.sync_ssh_keys.to_sym]
+ else
+ []
+ end
+ end
+
private
def entry
@entry
end
- def adapter
- @adapter ||= Gitlab::LDAP::Adapter.new
- end
-
def config
- @config ||= Gitlab.config.ldap
+ @config ||= Gitlab::LDAP::Config.new(provider)
end
end
end
diff --git a/lib/gitlab/ldap/user.rb b/lib/gitlab/ldap/user.rb
index 006ef170726..3069027a421 100644
--- a/lib/gitlab/ldap/user.rb
+++ b/lib/gitlab/ldap/user.rb
@@ -10,45 +10,11 @@ module Gitlab
module LDAP
class User < Gitlab::OAuth::User
class << self
- def authenticate(login, password)
- # Check user against LDAP backend if user is not authenticated
- # Only check with valid login and password to prevent anonymous bind results
- return nil unless ldap_conf.enabled && login.present? && password.present?
-
- ldap_user = adapter.bind_as(
- filter: user_filter(login),
- size: 1,
- password: password
- )
-
- find_by_uid(ldap_user.dn) if ldap_user
- end
-
- def adapter
- @adapter ||= OmniAuth::LDAP::Adaptor.new(ldap_conf)
- end
-
- def user_filter(login)
- filter = Net::LDAP::Filter.eq(adapter.uid, login)
- # Apply LDAP user filter if present
- if ldap_conf['user_filter'].present?
- user_filter = Net::LDAP::Filter.construct(ldap_conf['user_filter'])
- filter = Net::LDAP::Filter.join(filter, user_filter)
- end
- filter
- end
-
- def ldap_conf
- Gitlab.config.ldap
- end
-
- def find_by_uid(uid)
+ def find_by_uid_and_provider(uid, provider)
# LDAP distinguished name is case-insensitive
- model.where("provider = ? and lower(extern_uid) = ?", provider, uid.downcase).last
- end
-
- def provider
- 'ldap'
+ ::User.
+ where(provider: [provider, :ldap]).
+ where('lower(extern_uid) = ?', uid.downcase).last
end
end
@@ -65,7 +31,7 @@ module Gitlab
def find_by_uid_and_provider
# LDAP distinguished name is case-insensitive
model.
- where(provider: auth_hash.provider).
+ where(provider: [auth_hash.provider, :ldap]).
where('lower(extern_uid) = ?', auth_hash.uid.downcase).last
end
@@ -88,6 +54,10 @@ module Gitlab
def needs_blocking?
false
end
+
+ def allowed?
+ Gitlab::LDAP::Access.allowed?(gl_user)
+ end
end
end
end
diff --git a/spec/lib/gitlab/ldap/access_spec.rb b/spec/lib/gitlab/ldap/access_spec.rb
index d50f605e050..f4d5a927396 100644
--- a/spec/lib/gitlab/ldap/access_spec.rb
+++ b/spec/lib/gitlab/ldap/access_spec.rb
@@ -1,11 +1,11 @@
require 'spec_helper'
describe Gitlab::LDAP::Access do
- let(:access) { Gitlab::LDAP::Access.new }
- let(:user) { create(:user) }
+ let(:access) { Gitlab::LDAP::Access.new user }
+ let(:user) { create(:user, :ldap) }
describe :allowed? do
- subject { access.allowed?(user) }
+ subject { access.allowed? }
context 'when the user cannot be found' do
before { Gitlab::LDAP::Person.stub(find_by_dn: nil) }
@@ -28,20 +28,14 @@ describe Gitlab::LDAP::Access do
it { should be_true }
end
- context 'and has no disabled flag in active diretory' do
- before {
- Gitlab::LDAP::Person.stub(disabled_via_active_directory?: false)
- Gitlab.config.ldap['enabled'] = true
- Gitlab.config.ldap['active_directory'] = false
- }
-
- after {
- Gitlab.config.ldap['enabled'] = false
- Gitlab.config.ldap['active_directory'] = true
- }
+ context 'without ActiveDirectory enabled' do
+ before do
+ Gitlab::LDAP::Config.stub(enabled?: true)
+ Gitlab::LDAP::Config.any_instance.stub(active_directory: false)
+ end
- it { should be_false }
+ it { should be_true }
end
end
end
-end
+end
\ No newline at end of file
--
GitLab