Merge branch 'ldap-code' into 'master'

app/controllers/application_controller.rb

@@ -6,6 +6,7 @@ class ApplicationController < ActionController::Base
   before_filter :check_password_expiration
   around_filter :set_current_user_for_thread
   before_filter :add_abilities
+  before_filter :ldap_security_check
   before_filter :dev_tools if Rails.env == 'development'
   before_filter :default_headers
   before_filter :add_gon_variables
@@ -179,11 +180,28 @@ class ApplicationController < ActionController::Base
     end
   end
 
+  def ldap_security_check
+    if current_user && current_user.requires_ldap_check?
+      if gitlab_ldap_access.allowed?(current_user)
+        current_user.last_credential_check_at = Time.now
+        current_user.save
+      else
+        sign_out current_user
+        flash[:alert] = "Access denied for your LDAP account."
+        redirect_to new_user_session_path
+      end
+    end
+  end
+
   def event_filter
     filters = cookies['event_filter'].split(',') if cookies['event_filter'].present?
     @event_filter ||= EventFilter.new(filters)
   end
 
+  def gitlab_ldap_access
+    Gitlab::LDAP::Access.new
+  end
+
   # JSON for infinite scroll via Pager object
   def pager_json(partial, count)
     html = render_to_string(

app/models/user.rb

@@ -185,7 +185,7 @@ class User < ActiveRecord::Base
         where(conditions).first
       end
     end
-    
+
     def find_for_commit(email, name)
       # Prefer email match over name match
       User.where(email: email).first ||
@@ -275,7 +275,9 @@ class User < ActiveRecord::Base
   # Projects user has access to
   def authorized_projects
     @authorized_projects ||= begin
-                               project_ids = (personal_projects.pluck(:id) + groups_projects.pluck(:id) + projects.pluck(:id)).uniq
+                               project_ids = personal_projects.pluck(:id)
+                               project_ids += groups_projects.pluck(:id)
+                               project_ids += projects.pluck(:id).uniq
                                Project.where(id: project_ids).joins(:namespace).order('namespaces.name ASC')
                              end
   end
@@ -406,6 +408,14 @@ class User < ActiveRecord::Base
     end
   end
 
+  def requires_ldap_check?
+    if ldap_user?
+      !last_credential_check_at || (last_credential_check_at + 1.hour) < Time.now
+    else
+      false
+    end
+  end
+
   def solo_owned_groups
     @solo_owned_groups ||= owned_groups.select do |group|
       group.owners == [self]

config/gitlab.yml.example

@@ -121,7 +121,6 @@ production: &base
   ldap:
     enabled: false
     host: '_your_ldap_server'
-    base: '_the_base_where_you_search_for_users'
     port: 636
     uid: 'sAMAccountName'
     method: 'ssl' # "tls" or "ssl" or "plain"
@@ -138,6 +137,20 @@ production: &base
     # disable this setting, because the userPrincipalName contains an '@'.
     allow_username_or_email_login: true
 
+    # Base where we can search for users
+    #
+    #   Ex. ou=People,dc=gitlab,dc=example
+    #
+    base: ''
+
+    # Filter LDAP users
+    #
+    #   Format: RFC 4515
+    #   Ex. (employeeType=developer)
+    #
+    user_filter: ''
+
+
   ## OmniAuth settings
   omniauth:
     # Allow login via Twitter, Google, etc. using OmniAuth providers

db/migrate/20130809124851_add_permission_check_to_user.rb

+class AddPermissionCheckToUser < ActiveRecord::Migration
+  def change
+    add_column :users, :last_credential_check_at, :datetime
+  end
+end

db/schema.rb

@@ -76,8 +76,8 @@ ActiveRecord::Schema.define(version: 20140304005354) do
     t.integer  "assignee_id"
     t.integer  "author_id"
     t.integer  "project_id"
-    t.datetime "created_at",               null: false
-    t.datetime "updated_at",               null: false
+    t.datetime "created_at"
+    t.datetime "updated_at"
     t.integer  "position",     default: 0
     t.string   "branch_name"
     t.text     "description"
@@ -95,8 +95,8 @@ ActiveRecord::Schema.define(version: 20140304005354) do
 
   create_table "keys", force: true do |t|
     t.integer  "user_id"
-    t.datetime "created_at",  null: false
-    t.datetime "updated_at",  null: false
+    t.datetime "created_at"
+    t.datetime "updated_at"
     t.text     "key"
     t.string   "title"
     t.string   "type"
@@ -123,8 +123,8 @@ ActiveRecord::Schema.define(version: 20140304005354) do
     t.integer  "author_id"
     t.integer  "assignee_id"
     t.string   "title"
-    t.datetime "created_at",        null: false
-    t.datetime "updated_at",        null: false
+    t.datetime "created_at"
+    t.datetime "updated_at"
     t.integer  "milestone_id"
     t.string   "state"
     t.string   "merge_status"
@@ -176,8 +176,8 @@ ActiveRecord::Schema.define(version: 20140304005354) do
     t.text     "note"
     t.string   "noteable_type"
     t.integer  "author_id"
-    t.datetime "created_at",                    null: false
-    t.datetime "updated_at",                    null: false
+    t.datetime "created_at"
+    t.datetime "updated_at"
     t.integer  "project_id"
     t.string   "attachment"
     t.string   "line_code"
@@ -199,8 +199,8 @@ ActiveRecord::Schema.define(version: 20140304005354) do
     t.string   "name"
     t.string   "path"
     t.text     "description"
-    t.datetime "created_at",                                null: false
-    t.datetime "updated_at",                                null: false
+    t.datetime "created_at"
+    t.datetime "updated_at"
     t.integer  "creator_id"
     t.boolean  "issues_enabled",         default: true,     null: false
     t.boolean  "wall_enabled",           default: true,     null: false
@@ -252,8 +252,8 @@ ActiveRecord::Schema.define(version: 20140304005354) do
     t.text     "content",    limit: 2147483647
     t.integer  "author_id",                                    null: false
     t.integer  "project_id"
-    t.datetime "created_at",                                   null: false
-    t.datetime "updated_at",                                   null: false
+    t.datetime "created_at"
+    t.datetime "updated_at"
     t.string   "file_name"
     t.datetime "expires_at"
     t.boolean  "private",                       default: true, null: false
@@ -275,45 +275,42 @@ ActiveRecord::Schema.define(version: 20140304005354) do
     t.datetime "created_at"
   end
 
-  add_index "taggings", ["tag_id"], name: "index_taggings_on_tag_id", using: :btree
-  add_index "taggings", ["taggable_id", "taggable_type", "context"], name: "index_taggings_on_taggable_id_and_taggable_type_and_context", using: :btree
-
   create_table "tags", force: true do |t|
     t.string "name"
   end
 
   create_table "users", force: true do |t|
-    t.string   "email",                  default: "",    null: false
-    t.string   "encrypted_password",     default: "",    null: false
+    t.string   "email",                                default: "",    null: false
+    t.string   "encrypted_password",       limit: 128, default: "",    null: false
     t.string   "reset_password_token"
     t.datetime "reset_password_sent_at"
     t.datetime "remember_created_at"
-    t.integer  "sign_in_count",          default: 0
+    t.integer  "sign_in_count",                        default: 0
     t.datetime "current_sign_in_at"
     t.datetime "last_sign_in_at"
     t.string   "current_sign_in_ip"
     t.string   "last_sign_in_ip"
-    t.datetime "created_at",                             null: false
-    t.datetime "updated_at",                             null: false
+    t.datetime "created_at"
+    t.datetime "updated_at"
     t.string   "name"
-    t.boolean  "admin",                  default: false, null: false
-    t.integer  "projects_limit",         default: 10
-    t.string   "skype",                  default: "",    null: false
-    t.string   "linkedin",               default: "",    null: false
-    t.string   "twitter",                default: "",    null: false
+    t.boolean  "admin",                                default: false, null: false
+    t.integer  "projects_limit",                       default: 10
+    t.string   "skype",                                default: "",    null: false
+    t.string   "linkedin",                             default: "",    null: false
+    t.string   "twitter",                              default: "",    null: false
     t.string   "authentication_token"
-    t.integer  "theme_id",               default: 1,     null: false
+    t.integer  "theme_id",                             default: 1,     null: false
     t.string   "bio"
-    t.integer  "failed_attempts",        default: 0
+    t.integer  "failed_attempts",                      default: 0
     t.datetime "locked_at"
     t.string   "extern_uid"
     t.string   "provider"
     t.string   "username"
-    t.boolean  "can_create_group",       default: true,  null: false
-    t.boolean  "can_create_team",        default: true,  null: false
+    t.boolean  "can_create_group",                     default: true,  null: false
+    t.boolean  "can_create_team",                      default: true,  null: false
     t.string   "state"
-    t.integer  "color_scheme_id",        default: 1,     null: false
-    t.integer  "notification_level",     default: 1,     null: false
+    t.integer  "color_scheme_id",                      default: 1,     null: false
+    t.integer  "notification_level",                   default: 1,     null: false
     t.datetime "password_expires_at"
     t.integer  "created_by_id"
     t.string   "avatar"
@@ -321,15 +318,15 @@ ActiveRecord::Schema.define(version: 20140304005354) do
     t.datetime "confirmed_at"
     t.datetime "confirmation_sent_at"
     t.string   "unconfirmed_email"
-    t.boolean  "hide_no_ssh_key",        default: false
-    t.string   "website_url",            default: "",    null: false
+    t.boolean  "hide_no_ssh_key",                      default: false
+    t.string   "website_url",                          default: "",    null: false
+    t.datetime "last_credential_check_at"
   end
 
   add_index "users", ["admin"], name: "index_users_on_admin", using: :btree
   add_index "users", ["authentication_token"], name: "index_users_on_authentication_token", unique: true, using: :btree
   add_index "users", ["confirmation_token"], name: "index_users_on_confirmation_token", unique: true, using: :btree
   add_index "users", ["email"], name: "index_users_on_email", unique: true, using: :btree
-  add_index "users", ["extern_uid", "provider"], name: "index_users_on_extern_uid_and_provider", unique: true, using: :btree
   add_index "users", ["name"], name: "index_users_on_name", using: :btree
   add_index "users", ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true, using: :btree
   add_index "users", ["username"], name: "index_users_on_username", using: :btree
@@ -348,8 +345,8 @@ ActiveRecord::Schema.define(version: 20140304005354) do
   create_table "users_projects", force: true do |t|
     t.integer  "user_id",                        null: false
     t.integer  "project_id",                     null: false
-    t.datetime "created_at",                     null: false
-    t.datetime "updated_at",                     null: false
+    t.datetime "created_at"
+    t.datetime "updated_at"
     t.integer  "project_access",     default: 0, null: false
     t.integer  "notification_level", default: 3, null: false
   end
@@ -361,8 +358,8 @@ ActiveRecord::Schema.define(version: 20140304005354) do
   create_table "web_hooks", force: true do |t|
     t.string   "url"
     t.integer  "project_id"
-    t.datetime "created_at",                                    null: false
-    t.datetime "updated_at",                                    null: false
+    t.datetime "created_at"
+    t.datetime "updated_at"
     t.string   "type",                  default: "ProjectHook"
     t.integer  "service_id"
     t.boolean  "push_events",           default: true,          null: false

lib/api/internal.rb

@@ -35,8 +35,14 @@ module API
           user = key.user
 
           return false if user.blocked?
+
           if Gitlab.config.ldap.enabled
-            return false if user.ldap_user? && Gitlab::LDAP::User.blocked?(user.extern_uid)
+            if user.ldap_user?
+              # Check if LDAP user exists and match LDAP user_filter
+              unless Gitlab::LDAP::Access.new.allowed?(user)
+                return false
+              end
+            end
           end
 
           action = case git_cmd

lib/gitlab/ldap/access.rb

+module Gitlab
+  module LDAP
+    class Access
+      def allowed?(user)
+        !!Gitlab::LDAP::Person.find_by_dn(user.extern_uid)
+      rescue
+        false
+      end
+    end
+  end
+end

lib/gitlab/ldap/adapter.rb

+module Gitlab
+  module LDAP
+    class Adapter
+      attr_reader :ldap
+
+      def initialize
+        encryption = config['method'].to_s == 'ssl' ? :simple_tls : nil
+
+        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
+
+        @ldap = Net::LDAP.new(options)
+      end
+
+      def users(field, value)
+        if field.to_sym == :dn
+          options = {
+            base: value
+          }
+        else
+          options = {
+            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'])
+
+          options[:filter] = if options[:filter]
+                               Net::LDAP::Filter.join(options[:filter], user_filter)
+                             else
+                               user_filter
+                             end
+        end
+
+        entries = ldap.search(options).select do |entry|
+          entry.respond_to? config.uid
+        end
+
+        entries.map do |entry|
+          Gitlab::LDAP::Person.new(entry)
+        end
+      end
+
+      def user(*args)
+        users(*args).first
+      end
+
+      private
+
+      def config
+        @config ||= Gitlab.config.ldap
+      end
+    end
+  end
+end

lib/gitlab/ldap/person.rb

+module Gitlab
+  module LDAP
+    class Person
+      def self.find_by_uid(uid)
+        Gitlab::LDAP::Adapter.new.user(config.uid, uid)
+      end
+
+      def self.find_by_dn(dn)
+        Gitlab::LDAP::Adapter.new.user('dn', dn)
+      end
+
+      def initialize(entry)
+        Rails.logger.debug { "Instantiating #{self.class.name} with LDIF:\n#{entry.to_ldif}" }
+        @entry = entry
+      end
+
+      def name
+        entry.cn.first
+      end
+
+      def uid
+        entry.send(config.uid).first
+      end
+
+      def username
+        uid
+      end
+
+      def dn
+        entry.dn
+      end
+
+      private
+
+      def entry
+        @entry
+      end
+
+      def adapter
+        @adapter ||= Gitlab::LDAP::Adapter.new
+      end
+
+      def config
+        @config ||= Gitlab.config.ldap
+      end
+    end
+  end
+end

lib/gitlab/ldap/user.rb

@@ -13,8 +13,8 @@ module Gitlab
         def find_or_create(auth)
           @auth = auth
 
-          if uid.blank? || email.blank?
-            raise_error("Account must provide an uid and email address")
+          if uid.blank? || email.blank? || username.blank?
+            raise_error("Account must provide a dn, uid and email address")
           end
 
           user = find(auth)
@@ -62,8 +62,16 @@ module Gitlab
           return nil unless ldap_conf.enabled && login.present? && password.present?
 
           ldap = OmniAuth::LDAP::Adaptor.new(ldap_conf)
+          filter = Net::LDAP::Filter.eq(ldap.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
+
           ldap_user = ldap.bind_as(
-            filter: Net::LDAP::Filter.eq(ldap.uid, login),
+            filter: filter,
             size: 1,
             password: password
           )
@@ -71,22 +79,20 @@ module Gitlab
           find_by_uid(ldap_user.dn) if ldap_user
         end
 
-        # Check LDAP user existance by dn. User in git over ssh check
-        #
-        # It covers 2 cases:
-        # * when ldap account was removed
-        # * when ldap account was deactivated by change of OU membership in 'dn'
-        def blocked?(dn)
-          ldap = OmniAuth::LDAP::Adaptor.new(ldap_conf)
-          ldap.connection.search(base: dn, scope: Net::LDAP::SearchScope_BaseObject, size: 1).blank?
-        end
-
         private
 
         def find_by_uid(uid)
           model.where(provider: provider, extern_uid: uid).last
         end
 
+        def username
+          (auth.info.nickname || samaccountname).to_s.force_encoding("utf-8")
+        end
+
+        def samaccountname
+          (auth.extra[:raw_info][:samaccountname] || []).first
+        end
+
         def provider
           'ldap'
         end

spec/lib/gitlab/ldap/ldap_user_auth_spec.rb

@@ -9,7 +9,8 @@ describe Gitlab::LDAP do
     @info = double(
       uid: '12djsak321',
       name: 'John',
-      email: 'john@mail.com'
+      email: 'john@mail.com',
+      nickname: 'john'
     )
   end