Merge branch 'ldap-lease-8.5' into '8-5-stable'

Use leases for LDAP checks in 8.5

Back-port of https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/3143

See merge request !3181

6 files changed, 84 insertions, 1 deletions

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index fb74919ea23..1f55b18e0b1 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -246,6 +246,8 @@ class ApplicationController < ActionController::Base
 
   def ldap_security_check
     if current_user && current_user.requires_ldap_check?
+      return unless current_user.try_obtain_ldap_lease
+
       unless Gitlab::LDAP::Access.allowed?(current_user)
         sign_out current_user
         flash[:alert] = "Access denied for your LDAP account."
diff --git a/app/models/user.rb b/app/models/user.rb
index 2ef8d851b26..b8d4841e659 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -603,6 +603,13 @@ class User < ActiveRecord::Base
     end
   end
 
+  def try_obtain_ldap_lease
+    # After obtaining this lease LDAP checks will be blocked for 600 seconds
+    # (10 minutes) for this user.
+    lease = Gitlab::ExclusiveLease.new("user_ldap_check:#{id}", timeout: 600)
+    lease.try_obtain
+  end
+
   def solo_owned_groups
     @solo_owned_groups ||= owned_groups.select do |group|
       group.owners == [self]
diff --git a/config/initializers/redis_config.rb b/config/initializers/redis_config.rb
new file mode 100644
index 00000000000..1cfecc2dd00
--- /dev/null
+++ b/config/initializers/redis_config.rb
@@ -0,0 +1,12 @@
+# This is a quick hack to get ExclusiveLease working in GitLab 8.5
+
+module Gitlab
+  REDIS_URL = begin
+    redis_config_file = Rails.root.join('config/resque.yml')
+    if File.exists?(redis_config_file)
+      YAML.load_file(redis_config_file)[Rails.env]
+    else
+      'redis://localhost:6379'
+    end
+  end
+end
diff --git a/lib/gitlab/exclusive_lease.rb b/lib/gitlab/exclusive_lease.rb
new file mode 100644
index 00000000000..abf5fbe7ff1
--- /dev/null
+++ b/lib/gitlab/exclusive_lease.rb
@@ -0,0 +1,41 @@
+module Gitlab
+  # This class implements an 'exclusive lease'. We call it a 'lease'
+  # because it has a set expiry time. We call it 'exclusive' because only
+  # one caller may obtain a lease for a given key at a time. The
+  # implementation is intended to work across GitLab processes and across
+  # servers. It is a 'cheap' alternative to using SQL queries and updates:
+  # you do not need to change the SQL schema to start using
+  # ExclusiveLease.
+  #
+  # It is important to choose the timeout wisely. If the timeout is very
+  # high (1 hour) then the throughput of your operation gets very low (at
+  # most once an hour). If the timeout is lower than how long your
+  # operation may take then you cannot count on exclusivity. For example,
+  # if the timeout is 10 seconds and you do an operation which may take 20
+  # seconds then two overlapping operations may hold a lease for the same
+  # key at the same time.
+  #
+  class ExclusiveLease
+    def initialize(key, timeout:)
+      @key, @timeout = key, timeout
+    end
+
+    # Try to obtain the lease. Return true on success,
+    # false if the lease is already taken.
+    def try_obtain
+      # Performing a single SET is atomic
+      !!redis.set(redis_key, '1', nx: true, ex: @timeout)
+    end
+
+    private
+
+    def redis
+      # Maybe someday we want to use a connection pool...
+      @redis ||= Redis.new(url: Gitlab::REDIS_URL)
+    end
+
+    def redis_key
+      "gitlab:exclusive_lease:#{@key}"
+    end
+  end
+end
diff --git a/lib/gitlab/user_access.rb b/lib/gitlab/user_access.rb
index 4885baf9526..d1b42c1f9b9 100644
--- a/lib/gitlab/user_access.rb
+++ b/lib/gitlab/user_access.rb
@@ -3,7 +3,7 @@ module Gitlab
     def self.allowed?(user)
       return false if user.blocked?
 
-      if user.requires_ldap_check?
+      if user.requires_ldap_check? && user.try_obtain_ldap_lease
         return false unless Gitlab::LDAP::Access.allowed?(user)
       end
 
diff --git a/spec/lib/gitlab/exclusive_lease_spec.rb b/spec/lib/gitlab/exclusive_lease_spec.rb
new file mode 100644
index 00000000000..fbdb7ea34ac
--- /dev/null
+++ b/spec/lib/gitlab/exclusive_lease_spec.rb
@@ -0,0 +1,21 @@
+require 'spec_helper'
+
+describe Gitlab::ExclusiveLease do
+  it 'cannot obtain twice before the lease has expired' do
+    lease = Gitlab::ExclusiveLease.new(unique_key, timeout: 3600)
+    expect(lease.try_obtain).to eq(true)
+    expect(lease.try_obtain).to eq(false)
+  end
+
+  it 'can obtain after the lease has expired' do
+    timeout = 1
+    lease = Gitlab::ExclusiveLease.new(unique_key, timeout: timeout)
+    lease.try_obtain # start the lease
+    sleep(2 * timeout) # lease should have expired now
+    expect(lease.try_obtain).to eq(true)
+  end
+
+  def unique_key
+    SecureRandom.hex(10)
+  end
+end