Commit 57142014 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'expiring-lock' into 'master'

Use an exclusive lease for LDAP checks

Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/14134

Merge https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/3140 first.

Only perform LDAP checks for a user after acquiring a lease. Checks
during user login do not require a lease.

The lease has a fixed expire time of 10 minutes.

See merge request !3143
parents 8ecdc0a7 7b2bf4ce
......@@ -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."
......
......@@ -612,6 +612,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]
......
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::RedisConfig.url)
end
def redis_key
"gitlab:exclusive_lease:#{@key}"
end
end
end
......@@ -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
......
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
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