diff options
author | Dmitriy Zaporozhets <dzaporozhets@gitlab.com> | 2014-10-14 15:01:37 +0000 |
---|---|---|
committer | Dmitriy Zaporozhets <dzaporozhets@gitlab.com> | 2014-10-14 15:01:37 +0000 |
commit | e3bd17a7ba5238c147a79d0770e8503fd913610c (patch) | |
tree | 4833babe1357f34f9f856e45b139f8b86bceaa3a | |
parent | 4bebdc09463e29d26eac0117e0e3b45a9448c600 (diff) | |
parent | b4f7b387d0dfaef1766a82040249abb933632930 (diff) | |
download | gitlab-ce-e3bd17a7ba5238c147a79d0770e8503fd913610c.tar.gz |
Merge branch 'feature-multiple-ldap-servers' into 'master'
Feature multiple ldap servers
Update the code so Gitlab-EE can support multiple LDAP servers
See merge request !1172
26 files changed, 496 insertions, 236 deletions
diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index fa5685938f0..f46b36568f3 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -24,7 +24,7 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController gl_user.remember_me = true if @user.persisted? # Do additional LDAP checks for the user filter and EE features - if Gitlab::LDAP::Access.allowed?(gl_user) + if @user.allowed? sign_in_and_redirect(gl_user) else flash[:alert] = "Access denied for your LDAP account." diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 1bdba75c5e7..5ced98152a5 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -18,6 +18,10 @@ class SessionsController < Devise::SessionsController store_location_for(:redirect, redirect_path) end + if Gitlab.config.ldap.enabled + @ldap_servers = Gitlab::LDAP::Config.servers + end + super end diff --git a/app/helpers/oauth_helper.rb b/app/helpers/oauth_helper.rb index c0177dacbf8..7024483b8b3 100644 --- a/app/helpers/oauth_helper.rb +++ b/app/helpers/oauth_helper.rb @@ -1,6 +1,6 @@ module OauthHelper def ldap_enabled? - Devise.omniauth_providers.include?(:ldap) + Gitlab.config.ldap.enabled end def default_providers diff --git a/app/models/user.rb b/app/models/user.rb index c6baa7ee704..42faea0070e 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -178,8 +178,7 @@ class User < ActiveRecord::Base scope :not_in_team, ->(team){ where('users.id NOT IN (:ids)', ids: team.member_ids) } scope :not_in_project, ->(project) { project.users.present? ? where("id not in (:ids)", ids: project.users.map(&:id) ) : all } scope :without_projects, -> { where('id NOT IN (SELECT DISTINCT(user_id) FROM members)') } - scope :ldap, -> { where(provider: 'ldap') } - + scope :ldap, -> { where('provider LIKE ?', 'ldap%') } scope :potential_team_members, ->(team) { team.members.any? ? active.not_in_team(team) : active } # @@ -407,7 +406,7 @@ class User < ActiveRecord::Base end def ldap_user? - extern_uid && provider == 'ldap' + extern_uid && provider.start_with?('ldap') end def accessible_deploy_keys diff --git a/app/views/devise/sessions/_new_ldap.html.haml b/app/views/devise/sessions/_new_ldap.html.haml index 6c5a878e904..01584611493 100644 --- a/app/views/devise/sessions/_new_ldap.html.haml +++ b/app/views/devise/sessions/_new_ldap.html.haml @@ -1,4 +1,4 @@ -= form_tag(user_omniauth_callback_path(:ldap), id: 'new_ldap_user' ) do += form_tag(user_omniauth_callback_path(provider), id: 'new_ldap_user' ) do = text_field_tag :username, nil, {class: "form-control top", placeholder: "LDAP Login", autofocus: "autofocus"} = password_field_tag :password, nil, {class: "form-control bottom", placeholder: "Password"} %br/ diff --git a/app/views/devise/sessions/new.html.haml b/app/views/devise/sessions/new.html.haml index b70b0d66172..b9832787446 100644 --- a/app/views/devise/sessions/new.html.haml +++ b/app/views/devise/sessions/new.html.haml @@ -4,20 +4,22 @@ .login-body - if ldap_enabled? && gitlab_config.signin_enabled %ul.nav.nav-tabs - %li.active - = link_to 'LDAP', '#tab-ldap', 'data-toggle' => 'tab' + - @ldap_servers.each_with_index do |server, i| + %li{class: (:active if i==0)} + = link_to server['label'], "#tab-#{server['provider_name']}", 'data-toggle' => 'tab' %li = link_to 'Standard', '#tab-signin', 'data-toggle' => 'tab' .tab-content - %div#tab-ldap.tab-pane.active - = render partial: 'devise/sessions/new_ldap' + - @ldap_servers.each_with_index do |server,i| + %div.tab-pane{id: "tab-#{server['provider_name']}", class: (:active if i==0)} + = render 'devise/sessions/new_ldap', provider: server['provider_name'] %div#tab-signin.tab-pane - = render partial: 'devise/sessions/new_base' + = render 'devise/sessions/new_base' - elsif ldap_enabled? - = render partial: 'devise/sessions/new_ldap' + = render 'devise/sessions/new_ldap', ldap_servers: @ldap_servers - elsif gitlab_config.signin_enabled - = render partial: 'devise/sessions/new_base' + = render 'devise/sessions/new_base' - else %div No authentication methods configured. @@ -36,7 +38,6 @@ %span.light Did not receive confirmation email? = link_to "Send again", new_confirmation_path(resource_name) - - if extra_config.has_key?('sign_in_text') %hr = markdown(extra_config.sign_in_text) diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index 7f624f92a8b..e7a8d08dc83 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -135,43 +135,61 @@ production: &base # bundle exec rake gitlab:ldap:check RAILS_ENV=production ldap: enabled: false - host: '_your_ldap_server' - port: 636 - uid: 'sAMAccountName' - method: 'ssl' # "tls" or "ssl" or "plain" - bind_dn: '_the_full_dn_of_the_user_you_will_bind_with' - password: '_the_password_of_the_bind_user' - - # This setting specifies if LDAP server is Active Directory LDAP server. - # For non AD servers it skips the AD specific queries. - # If your LDAP server is not AD, set this to false. - active_directory: true - - # If allow_username_or_email_login is enabled, GitLab will ignore everything - # after the first '@' in the LDAP username submitted by the user on login. - # - # Example: - # - the user enters 'jane.doe@example.com' and 'p@ssw0rd' as LDAP credentials; - # - GitLab queries the LDAP server with 'jane.doe' and 'p@ssw0rd'. - # - # If you are using "uid: 'userPrincipalName'" on ActiveDirectory you need to - # disable this setting, because the userPrincipalName contains an '@'. - allow_username_or_email_login: false - - # Base where we can search for users - # - # Ex. ou=People,dc=gitlab,dc=example - # - base: '' - - # Filter LDAP users - # - # Format: RFC 4515 http://tools.ietf.org/search/rfc4515 - # Ex. (employeeType=developer) - # - # Note: GitLab does not support omniauth-ldap's custom filter syntax. - # - user_filter: '' + servers: + main: # 'main' is the GitLab 'provider ID' of this LDAP server + ## label + # + # A human-friendly name for your LDAP server. It is OK to change the label later, + # for instance if you find out it is too large to fit on the web page. + # + # Example: 'Paris' or 'Acme, Ltd.' + label: 'LDAP' + + host: '_your_ldap_server' + port: 636 + uid: 'sAMAccountName' + method: 'ssl' # "tls" or "ssl" or "plain" + bind_dn: '_the_full_dn_of_the_user_you_will_bind_with' + password: '_the_password_of_the_bind_user' + + # This setting specifies if LDAP server is Active Directory LDAP server. + # For non AD servers it skips the AD specific queries. + # If your LDAP server is not AD, set this to false. + active_directory: true + + # If allow_username_or_email_login is enabled, GitLab will ignore everything + # after the first '@' in the LDAP username submitted by the user on login. + # + # Example: + # - the user enters 'jane.doe@example.com' and 'p@ssw0rd' as LDAP credentials; + # - GitLab queries the LDAP server with 'jane.doe' and 'p@ssw0rd'. + # + # If you are using "uid: 'userPrincipalName'" on ActiveDirectory you need to + # disable this setting, because the userPrincipalName contains an '@'. + allow_username_or_email_login: false + + # Base where we can search for users + # + # Ex. ou=People,dc=gitlab,dc=example + # + base: '' + + # Filter LDAP users + # + # Format: RFC 4515 http://tools.ietf.org/search/rfc4515 + # Ex. (employeeType=developer) + # + # Note: GitLab does not support omniauth-ldap's custom filter syntax. + # + user_filter: '' + + # GitLab EE only: add more LDAP servers + # Choose an ID made of a-z and 0-9 . This ID will be stored in the database + # so that GitLab can remember which LDAP server a user belongs to. + # uswest2: + # label: + # host: + # .... ## OmniAuth settings @@ -300,6 +318,20 @@ test: project_url: "http://redmine/projects/:issues_tracker_id" issues_url: "http://redmine/:project_id/:issues_tracker_id/:id" new_issue_url: "http://redmine/projects/:issues_tracker_id/issues/new" + ldap: + enabled: false + servers: + main: + label: ldap + host: 127.0.0.1 + port: 3890 + uid: 'uid' + method: 'plain' # "tls" or "ssl" or "plain" + base: 'dc=example,dc=com' + user_filter: '' + group_base: 'ou=groups,dc=example,dc=com' + admin_group: '' + sync_ssh_keys: false staging: <<: *base diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 0d11ae6f33f..7e7c91ced77 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -56,9 +56,25 @@ end # Default settings Settings['ldap'] ||= Settingslogic.new({}) Settings.ldap['enabled'] = false if Settings.ldap['enabled'].nil? -Settings.ldap['allow_username_or_email_login'] = false if Settings.ldap['allow_username_or_email_login'].nil? -Settings.ldap['active_directory'] = true if Settings.ldap['active_directory'].nil? +# backwards compatibility, we only have one host +if Settings.ldap['enabled'] || Rails.env.test? + if Settings.ldap['host'].present? + server = Settings.ldap.except('sync_time') + server['label'] = 'LDAP' + server['provider_name'] = 'ldap' + Settings.ldap['servers'] = { + 'ldap' => server + } + end + + Settings.ldap['servers'].each do |key, server| + server['allow_username_or_email_login'] = false if server['allow_username_or_email_login'].nil? + server['active_directory'] = true if server['active_directory'].nil? + server['provider_name'] ||= "ldap#{key}".downcase + server['provider_class'] = OmniAuth::Utils.camelize(server['provider_name']) + end +end Settings['omniauth'] ||= Settingslogic.new({}) Settings.omniauth['enabled'] = false if Settings.omniauth['enabled'].nil? diff --git a/config/initializers/7_omniauth.rb b/config/initializers/7_omniauth.rb new file mode 100644 index 00000000000..7ef5c10da08 --- /dev/null +++ b/config/initializers/7_omniauth.rb @@ -0,0 +1,9 @@ +module OmniAuth::Strategies + server = Gitlab.config.ldap.servers.values.first + const_set(server['provider_class'], Class.new(LDAP)) +end + +OmniauthCallbacksController.class_eval do + server = Gitlab.config.ldap.servers.values.first + alias_method server['provider_name'], :ldap +end
\ No newline at end of file diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index 34f4f386988..226cacfe0d1 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -205,21 +205,23 @@ Devise.setup do |config| # end if Gitlab.config.ldap.enabled - if Gitlab.config.ldap.allow_username_or_email_login - email_stripping_proc = ->(name) {name.gsub(/@.*$/,'')} - else - email_stripping_proc = ->(name) {name} + Gitlab.config.ldap.servers.values.each do |server| + if server['allow_username_or_email_login'] + email_stripping_proc = ->(name) {name.gsub(/@.*$/,'')} + else + email_stripping_proc = ->(name) {name} + end + + config.omniauth server['provider_name'], + host: server['host'], + base: server['base'], + uid: server['uid'], + port: server['port'], + method: server['method'], + bind_dn: server['bind_dn'], + password: server['password'], + name_proc: email_stripping_proc end - - config.omniauth :ldap, - host: Gitlab.config.ldap['host'], - base: Gitlab.config.ldap['base'], - uid: Gitlab.config.ldap['uid'], - port: Gitlab.config.ldap['port'], - method: Gitlab.config.ldap['method'], - bind_dn: Gitlab.config.ldap['bind_dn'], - password: Gitlab.config.ldap['password'], - name_proc: email_stripping_proc end Gitlab.config.omniauth.providers.each do |provider| diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb index 955abc1bedd..ae33c529b93 100644 --- a/lib/gitlab/auth.rb +++ b/lib/gitlab/auth.rb @@ -3,22 +3,16 @@ 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 end - - def log - Gitlab::AppLogger - end - - def ldap_conf - @ldap_conf ||= Gitlab.config.ldap - end end end diff --git a/lib/gitlab/ldap/access.rb b/lib/gitlab/ldap/access.rb index d2235d2e3bc..eb2c4e48ff2 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,30 @@ 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 + + def ldap_config + Gitlab::LDAP::Config.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..a5944f96983 --- /dev/null +++ b/lib/gitlab/ldap/authentication.rb @@ -0,0 +1,71 @@ +# 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 + + # If (login, password) was invalid for all providers, the value of auth is now the last + # Gitlab::LDAP::Authentication instance we tried. + 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) + filter = Net::LDAP::Filter.eq(config.uid, login) + + # Apply LDAP user filter if present + if config.user_filter.present? + filter = Net::LDAP::Filter.join( + filter, + Net::LDAP::Filter.construct(config.user_filter) + ) + end + filter + 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..d41bfba9b0f --- /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.values + end + + def self.providers + servers.map {|server| server['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.values.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..3e0b3e6cbf8 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(adapter.config.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,6 +38,10 @@ module Gitlab uid end + def email + entry.try(:mail) + end + def dn entry.dn end @@ -48,12 +52,8 @@ module Gitlab @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..3176e9790a7 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 @@ -63,10 +29,8 @@ module Gitlab end def find_by_uid_and_provider - # LDAP distinguished name is case-insensitive - model. - where(provider: auth_hash.provider). - where('lower(extern_uid) = ?', auth_hash.uid.downcase).last + self.class.find_by_uid_and_provider( + auth_hash.uid.downcase, auth_hash.provider) end def find_by_email @@ -88,6 +52,10 @@ module Gitlab def needs_blocking? false end + + def allowed? + Gitlab::LDAP::Access.allowed?(gl_user) + end end end end diff --git a/lib/gitlab/oauth/user.rb b/lib/gitlab/oauth/user.rb index 699258baee4..133445d3d05 100644 --- a/lib/gitlab/oauth/user.rb +++ b/lib/gitlab/oauth/user.rb @@ -70,10 +70,6 @@ module Gitlab Gitlab::AppLogger end - def raise_error(message) - raise OmniAuth::Error, "(OAuth) " + message - end - def needs_blocking? Gitlab.config.omniauth['block_auto_created_users'] end diff --git a/spec/factories.rb b/spec/factories.rb index a960571206c..15899d8c3c4 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -24,6 +24,11 @@ FactoryGirl.define do admin true end + trait :ldap do + provider 'ldapmain' + extern_uid 'my-ldap-id' + end + factory :admin, traits: [:admin] end diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb index 551fb3fb5f6..1f3e1a4a3c1 100644 --- a/spec/lib/gitlab/auth_spec.rb +++ b/spec/lib/gitlab/auth_spec.rb @@ -28,17 +28,16 @@ describe Gitlab::Auth do end context "with ldap enabled" do - before { Gitlab.config.ldap['enabled'] = true } - after { Gitlab.config.ldap['enabled'] = false } + before { Gitlab::LDAP::Config.stub(enabled?: true) } it "tries to autheticate with db before ldap" do - expect(Gitlab::LDAP::User).not_to receive(:authenticate) + expect(Gitlab::LDAP::Authentication).not_to receive(:login) gl_auth.find(username, password) end it "uses ldap as fallback to for authentication" do - expect(Gitlab::LDAP::User).to receive(:authenticate) + expect(Gitlab::LDAP::Authentication).to receive(:login) gl_auth.find('ldap_user', 'password') 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 diff --git a/spec/lib/gitlab/ldap/adapter_spec.rb b/spec/lib/gitlab/ldap/adapter_spec.rb index c3f07334431..19347e47378 100644 --- a/spec/lib/gitlab/ldap/adapter_spec.rb +++ b/spec/lib/gitlab/ldap/adapter_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Gitlab::LDAP::Adapter do - let(:adapter) { Gitlab::LDAP::Adapter.new } + let(:adapter) { Gitlab::LDAP::Adapter.new 'ldapmain' } describe :dn_matches_filter? do let(:ldap) { double(:ldap) } diff --git a/spec/lib/gitlab/ldap/authentication_spec.rb b/spec/lib/gitlab/ldap/authentication_spec.rb new file mode 100644 index 00000000000..0eb7c443b8b --- /dev/null +++ b/spec/lib/gitlab/ldap/authentication_spec.rb @@ -0,0 +1,53 @@ +require 'spec_helper' + +describe Gitlab::LDAP::Authentication do + let(:klass) { Gitlab::LDAP::Authentication } + let(:user) { create(:user, :ldap, extern_uid: dn) } + let(:dn) { 'uid=john,ou=people,dc=example,dc=com' } + let(:login) { 'john' } + let(:password) { 'password' } + + describe :login do + let(:adapter) { double :adapter } + before do + Gitlab::LDAP::Config.stub(enabled?: true) + end + + it "finds the user if authentication is successful" do + user + # try only to fake the LDAP call + klass.any_instance.stub(adapter: double(:adapter, + bind_as: double(:ldap_user, dn: dn) + )) + expect(klass.login(login, password)).to be_true + end + + it "is false if the user does not exist" do + # try only to fake the LDAP call + klass.any_instance.stub(adapter: double(:adapter, + bind_as: double(:ldap_user, dn: dn) + )) + expect(klass.login(login, password)).to be_false + end + + it "is false if authentication fails" do + user + # try only to fake the LDAP call + klass.any_instance.stub(adapter: double(:adapter, bind_as: nil)) + expect(klass.login(login, password)).to be_false + end + + it "fails if ldap is disabled" do + Gitlab::LDAP::Config.stub(enabled?: false) + expect(klass.login(login, password)).to be_false + end + + it "fails if no login is supplied" do + expect(klass.login('', password)).to be_false + end + + it "fails if no password is supplied" do + expect(klass.login(login, '')).to be_false + end + end +end
\ No newline at end of file diff --git a/spec/lib/gitlab/ldap/config_spec.rb b/spec/lib/gitlab/ldap/config_spec.rb new file mode 100644 index 00000000000..76cc7f95c47 --- /dev/null +++ b/spec/lib/gitlab/ldap/config_spec.rb @@ -0,0 +1,20 @@ +require 'spec_helper' + +describe Gitlab::LDAP::Config do + let(:config) { Gitlab::LDAP::Config.new provider } + let(:provider) { 'ldapmain' } + + describe :initalize do + it 'requires a provider' do + expect{ Gitlab::LDAP::Config.new }.to raise_error ArgumentError + end + + it "works" do + expect(config).to be_a described_class + end + + it "raises an error if a unknow provider is used" do + expect{ Gitlab::LDAP::Config.new 'unknown' }.to raise_error + end + end +end
\ No newline at end of file diff --git a/spec/lib/gitlab/ldap/user_spec.rb b/spec/lib/gitlab/ldap/user_spec.rb index a1aec0bb96f..726c9764e3d 100644 --- a/spec/lib/gitlab/ldap/user_spec.rb +++ b/spec/lib/gitlab/ldap/user_spec.rb @@ -10,12 +10,12 @@ describe Gitlab::LDAP::User do } end let(:auth_hash) do - double(uid: 'my-uid', provider: 'ldap', info: double(info)) + double(uid: 'my-uid', provider: 'ldapmain', info: double(info)) end describe :find_or_create do it "finds the user if already existing" do - existing_user = create(:user, extern_uid: 'my-uid', provider: 'ldap') + existing_user = create(:user, extern_uid: 'my-uid', provider: 'ldapmain') expect{ gl_user.save }.to_not change{ User.count } end @@ -26,27 +26,11 @@ describe Gitlab::LDAP::User do existing_user.reload expect(existing_user.extern_uid).to eql 'my-uid' - expect(existing_user.provider).to eql 'ldap' + expect(existing_user.provider).to eql 'ldapmain' end it "creates a new user if not found" do expect{ gl_user.save }.to change{ User.count }.by(1) end end - - describe "authenticate" do - let(:login) { 'john' } - let(:password) { 'my-secret' } - - before { - Gitlab.config.ldap['enabled'] = true - Gitlab.config.ldap['user_filter'] = 'employeeType=developer' - } - after { Gitlab.config.ldap['enabled'] = false } - - it "send an authentication request to ldap" do - expect( Gitlab::LDAP::User.adapter ).to receive(:bind_as) - Gitlab::LDAP::User.authenticate(login, password) - end - end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 8c79bf5f3c2..6ad57b06e06 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -346,6 +346,25 @@ describe User do end end + describe :ldap_user? do + let(:user) { build(:user, :ldap) } + + it "is true if provider name starts with ldap" do + user.provider = 'ldapmain' + expect( user.ldap_user? ).to be_true + end + + it "is false for other providers" do + user.provider = 'other-provider' + expect( user.ldap_user? ).to be_false + end + + it "is false if no extern_uid is provided" do + user.extern_uid = nil + expect( user.ldap_user? ).to be_false + end + end + describe '#full_website_url' do let(:user) { create(:user) } |