Commit acd9bc02 authored by Jacob Vosmaer's avatar Jacob Vosmaer

Acquire lock before LDAP sync

parent 3eb7ea49
...@@ -246,6 +246,8 @@ class ApplicationController < ActionController::Base ...@@ -246,6 +246,8 @@ class ApplicationController < ActionController::Base
def ldap_security_check def ldap_security_check
if current_user && current_user.requires_ldap_check? if current_user && current_user.requires_ldap_check?
return unless Gitlab::LDAP::Access.try_lock_user(user)
unless Gitlab::LDAP::Access.allowed?(current_user) unless Gitlab::LDAP::Access.allowed?(current_user)
sign_out current_user sign_out current_user
flash[:alert] = "Access denied for your LDAP account." flash[:alert] = "Access denied for your LDAP account."
......
module Gitlab
# This class implements a distributed self-expiring lock.
#
# [2] pry(main)> l = Gitlab::ExpiringLock.new('foobar', 5)
# => #<Gitlab::ExpiringLock:0x007ffb9d7cb7f8 @key="foobar", @timeout=5>
# [3] pry(main)> l.try_lock
# => true
# [4] pry(main)> l.try_lock # Only the first try_lock succeeds
# => false
# [5] pry(main)> l.locked?
# => true
# [6] pry(main)> sleep 5
# => 5
# [7] pry(main)> l.locked? # After the timeout the lock is released
# => false
#
class ExpiringLock
def initialize(key, timeout)
@key, @timeout = key, timeout
end
# Try to obtain the lock. Return true on succes,
# false if the lock is already taken.
def try_lock
# INCR does not change the key TTL
if redis.incr(redis_key) == 1
# We won the race to insert the key into Redis
redis.expire(redis_key, @timeout)
true
else
# Somebody else won the race
false
end
end
# Check if somebody somewhere locked this key
def locked?
!!redis.get(redis_key)
end
private
def redis
# Maybe someday we want to use a connection pool...
@redis ||= Redis.new(url: Gitlab::RedisConfig.url)
end
def redis_key
"gitlab:expiring_lock:#{@key}"
end
end
end
...@@ -7,6 +7,12 @@ module Gitlab ...@@ -7,6 +7,12 @@ module Gitlab
class Access class Access
attr_reader :provider, :user attr_reader :provider, :user
LOCK_TIMEOUT = 600
def self.try_lock_user(user)
Gitlab::ExpiringLock.new("user_ldap_check:#{user.id}", LOCK_TIMEOUT).try_lock
end
def self.open(user, &block) def self.open(user, &block)
Gitlab::LDAP::Adapter.open(user.ldap_identity.provider) do |adapter| Gitlab::LDAP::Adapter.open(user.ldap_identity.provider) do |adapter|
block.call(self.new(user, adapter)) block.call(self.new(user, adapter))
......
...@@ -3,8 +3,9 @@ module Gitlab ...@@ -3,8 +3,9 @@ module Gitlab
def self.allowed?(user) def self.allowed?(user)
return false if user.blocked? return false if user.blocked?
if user.requires_ldap_check? if user.requires_ldap_check? && Gitlab::LDAP::Access.try_lock_user(user)
return false unless Gitlab::LDAP::Access.allowed?(user) return Gitlab::LDAP::Access.allowed?(user)
end
end end
true true
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment