Move group sync to a separate worker process and revamp the entire strategy

app/models/group.rb

@@ -36,6 +36,10 @@ class Group < Namespace
   after_create :post_create_hook
   after_destroy :post_destroy_hook
 
+  scope :where_group_links_with_provider, ->(provider) do
+    joins(:ldap_group_links).where(ldap_group_links: { provider: provider })
+  end
+
   class << self
     # Searches for groups matching the given query.
     #

app/models/identity.rb

@@ -19,6 +19,8 @@ class Identity < ActiveRecord::Base
   validates :extern_uid, allow_blank: true, uniqueness: { scope: :provider }
   validates :user_id, uniqueness: { scope: :provider }
 
+  scope :with_provider, ->(provider) { where(provider: provider) }
+
   def ldap?
     provider.starts_with?('ldap')
   end

app/models/members/group_member.rb

@@ -31,6 +31,10 @@ class GroupMember < Member
   scope :with_group, ->(group) { where(source_id: group.id) }
   scope :with_user, ->(user) { where(user_id: user.id) }
   scope :with_ldap_dn, -> { joins(user: :identities).where("identities.provider LIKE ?", 'ldap%') }
+  scope :select_access_level_and_user, -> { select(:access_level, :user_id) }
+  scope :with_identity_provider, ->(provider) do
+    joins(user: :identities).where(identities: { provider: provider })
+  end
 
   def self.access_level_roles
     Gitlab::Access.options_with_owner

app/models/user.rb

@@ -227,6 +227,9 @@ class User < ActiveRecord::Base
   scope :ldap, -> { joins(:identities).where('identities.provider LIKE ?', 'ldap%') }
   scope :with_two_factor,    -> { where(two_factor_enabled: true) }
   scope :without_two_factor, -> { where(two_factor_enabled: false) }
+  scope :with_provider, ->(provider) do
+    joins(:identities).where(identities: { provider: provider })
+  end
 
   #
   # Class methods

app/workers/ldap_group_links_worker.rb

-class LdapGroupLinksWorker
-  include Sidekiq::Worker
-
-  def perform(user_id)
-    user = User.find(user_id)
-    logger.info "Updating LDAP group memberships for user #{user.id} (#{user.email})"
-    access = Gitlab::LDAP::Access.new(user)
-    access.update_ldap_group_links
-  end
-end

app/workers/ldap_group_sync_worker.rb

+class LdapGroupSyncWorker
+  include Sidekiq::Worker
+
+  sidekiq_options retry: false
+
+  def perform
+    logger.info 'Started LDAP group sync'
+    Gitlab::LDAP::GroupSync.execute
+    logger.info 'Finished LDAP group sync'
+  end
+end

config/initializers/1_settings.rb

@@ -330,6 +330,9 @@ Settings.cron_jobs['update_all_mirrors_worker']['job_class'] = 'UpdateAllMirrors
 Settings.cron_jobs['ldap_sync_worker'] ||= Settingslogic.new({})
 Settings.cron_jobs['ldap_sync_worker']['cron'] ||= '30 1 * * *'
 Settings.cron_jobs['ldap_sync_worker']['job_class'] = 'LdapSyncWorker'
+Settings.cron_jobs['ldap_group_sync_worker'] ||= Settingslogic.new({})
+Settings.cron_jobs['ldap_group_sync_worker']['cron'] ||= '0 * * * *'
+Settings.cron_jobs['ldap_group_sync_worker']['job_class'] = 'LdapGroupSyncWorker'
 Settings.cron_jobs['geo_bulk_notify_worker'] ||= Settingslogic.new({})
 Settings.cron_jobs['geo_bulk_notify_worker']['cron'] ||= '*/10 * * * * *'
 Settings.cron_jobs['geo_bulk_notify_worker']['job_class'] ||= 'GeoBulkNotifyWorker'

db/migrate/20160303210802_add_secondary_extern_uid_to_identities.rb

+class AddSecondaryExternUidToIdentities < ActiveRecord::Migration
+  def change
+    add_column :identities, :secondary_extern_uid, :string
+  end
+end

db/migrate/20160317191509_add_last_sync_time_to_groups.rb

+class AddLastSyncTimeToGroups < ActiveRecord::Migration
+  def change
+    add_column :namespaces, :last_ldap_sync_at, :datetime
+    add_index :namespaces, :last_ldap_sync_at
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20160316124047) do
+ActiveRecord::Schema.define(version: 20160317191509) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -455,6 +455,7 @@ ActiveRecord::Schema.define(version: 20160316124047) do
     t.integer  "user_id"
     t.datetime "created_at"
     t.datetime "updated_at"
+    t.string   "secondary_extern_uid"
   end
 
   add_index "identities", ["created_at", "id"], name: "index_identities_on_created_at_and_id", using: :btree
@@ -676,9 +677,11 @@ ActiveRecord::Schema.define(version: 20160316124047) do
     t.string   "avatar"
     t.boolean  "membership_lock",       default: false
     t.boolean  "share_with_group_lock", default: false
+    t.datetime "last_ldap_sync_at"
   end
 
   add_index "namespaces", ["created_at", "id"], name: "index_namespaces_on_created_at_and_id", using: :btree
+  add_index "namespaces", ["last_ldap_sync_at"], name: "index_namespaces_on_last_ldap_sync_at", using: :btree
   add_index "namespaces", ["name"], name: "index_namespaces_on_name", unique: true, using: :btree
   add_index "namespaces", ["name"], name: "index_namespaces_on_name_trigram", using: :gin, opclasses: {"name"=>"gin_trgm_ops"}
   add_index "namespaces", ["owner_id"], name: "index_namespaces_on_owner_id", using: :btree
@@ -831,7 +834,7 @@ ActiveRecord::Schema.define(version: 20160316124047) do
     t.boolean  "pending_delete",                   default: false
     t.boolean  "public_builds",                    default: true,     null: false
     t.string   "main_language"
-    t.integer  "pushes_since_gc",        default: 0
+    t.integer  "pushes_since_gc",                  default: 0
   end
 
   add_index "projects", ["builds_enabled", "shared_runners_enabled"], name: "index_projects_on_builds_enabled_and_shared_runners_enabled", using: :btree

lib/gitlab/ldap/access.rb

@@ -18,7 +18,6 @@ module Gitlab
         self.open(user) do |access|
           # Whether user is allowed, or not, we should update
           # permissions to keep things clean
-          access.update_permissions(options)
           if access.allowed?
             access.update_user
             user.last_credential_check_at = Time.now
@@ -75,17 +74,6 @@ module Gitlab
         update_kerberos_identity if import_kerberos_identities?
       end
 
-      def update_permissions(options)
-        if group_base.present?
-          if options[:update_ldap_group_links_synchronously]
-            update_ldap_group_links
-          else
-            LdapGroupLinksWorker.perform_async(user.id)
-          end
-        end
-        update_admin_status if admin_group.present?
-      end
-
       # Update user ssh keys if they changed in LDAP
       def update_ssh_keys
         remove_old_ssh_keys
@@ -148,63 +136,6 @@ module Gitlab
         user.update(email: ldap_email)
       end
 
-      def update_admin_status
-        admin_group = Gitlab::LDAP::Group.find_by_cn(ldap_config.admin_group, adapter)
-        admin_user = Gitlab::LDAP::Person.find_by_dn(user.ldap_identity.extern_uid, adapter)
-
-        if admin_group && admin_group.has_member?(admin_user)
-          unless user.admin?
-            user.admin = true
-            user.save
-          end
-        else
-          if user.admin?
-            user.admin = false
-            user.save
-          end
-        end
-      end
-
-      # Loop through all ldap connected groups, and update the users link with it
-      #
-      # We documented what sort of queries an LDAP server can expect from
-      # GitLab EE in doc/integration/ldap.md. Please remember to update that
-      # documentation if you change the algorithm below.
-      def update_ldap_group_links
-        gitlab_groups_with_ldap_link.each do |group|
-          active_group_links = group.ldap_group_links.where(cn: cns_with_access)
-
-          if active_group_links.any?
-            max_access = active_group_links.maximum(:group_access)
-
-            # Ensure we don't leave a group without an owner
-            if max_access < Gitlab::Access::OWNER && group.last_owner?(user)
-              logger.warn "#{self.class.name}: LDAP group sync cannot demote #{user.name} (#{user.id}) from group #{group.name} (#{group.id}) as this is the group's last owner"
-            else
-              group.add_users([user.id], max_access, skip_notification: true)
-            end
-          elsif group.last_owner?(user)
-            logger.warn "#{self.class.name}: LDAP group sync cannot remove #{user.name} (#{user.id}) from group #{group.name} (#{group.id}) as this is the group's last owner"
-          else
-            group.users.delete(user)
-          end
-        end
-      end
-
-      def ldap_groups
-        @ldap_groups ||= ::LdapGroupLink.with_provider(provider).distinct(:cn).pluck(:cn).map do |cn|
-          Gitlab::LDAP::Group.find_by_cn(cn, adapter)
-        end.compact
-      end
-
-      # returns a collection of cn strings to which the user has access
-      def cns_with_access
-        return [] unless ldap_user.present?
-        @ldap_groups_with_access ||= ldap_groups.select do |ldap_group|
-          ldap_group.has_member?(ldap_user)
-        end.map(&:cn)
-      end
-
       def sync_ssh_keys?
         ldap_config.sync_ssh_keys?
       end
@@ -214,22 +145,8 @@ module Gitlab
         ldap_config.active_directory && (Gitlab.config.kerberos.enabled || AuthHelper.kerberos_enabled? )
       end
 
-      def group_base
-        ldap_config.group_base
-      end
-
-      def admin_group
-        ldap_config.admin_group
-      end
-
       private
 
-      def gitlab_groups_with_ldap_link
-        ::Group.includes(:ldap_group_links).references(:ldap_group_links).
-          where.not(ldap_group_links: { id: nil }).
-          where(ldap_group_links: { provider: provider })
-      end
-
       def logger
         Rails.logger
       end

lib/gitlab/ldap/adapter.rb

@@ -93,6 +93,16 @@ module Gitlab
                     attributes: %w{dn}).any?
       end
 
+      def dns_for_filter(filter)
+        ldap_search(
+          base: config.base,
+          filter: filter,
+          scope: Net::LDAP::SearchScope_WholeSubtree,
+          attributes: %w{dn}
+        ).map(&:dn)
+      end
+
+
       def ldap_search(*args)
         # Net::LDAP's `time` argument doesn't work. Use Ruby `Timeout` instead.
         Timeout.timeout(config.timeout) do

lib/gitlab/ldap/group.rb

@@ -14,6 +14,10 @@ module Gitlab
         @adapter = adapter
       end
 
+      def active_directory?
+        adapter.config.active_directory
+      end
+
       def cn
         entry.cn.first
       end
@@ -34,22 +38,12 @@ module Gitlab
         entry.memberuid
       end
 
-      def has_member?(user)
-        user_uid = user.uid.downcase
-        user_dn = user.dn.downcase
-
-        if memberuid?
-          member_uids.any? { |member_uid| member_uid.downcase == user_uid }
-        elsif member_dns.any? { |member_dn| member_dn.downcase == user_dn }
-          true
-        elsif member_dns.any? { |member_dn| member_dn.downcase == "uid=" + user_uid }
-          true
-        elsif adapter.config.active_directory
-          adapter.dn_matches_filter?(user.dn, active_directory_recursive_memberof_filter)
+      def member_dns
+        if active_directory?
+          dns = adapter.dns_for_filter(active_directory_recursive_memberof_filter)
+          return dns unless dns.empty?
         end
-      end
 
-      def member_dns
         if (entry.respond_to? :member) && (entry.respond_to? :submember)
           entry.member + entry.submember
         elsif entry.respond_to? :member