diff options
author | Rémy Coutable <remy@rymai.me> | 2016-03-15 11:30:49 +0000 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2016-03-15 11:30:49 +0000 |
commit | 42b785a9a8dbefcdc65cfcb50f744277c4370b2a (patch) | |
tree | ef8fb0c44aa52dbbe02ca7d668372cc25ce0911a | |
parent | 11f388aaac6216e49cccba3b440fcabc52cb9129 (diff) | |
parent | bf253a10873af6b6fdb28f424e6f1db8c515be25 (diff) | |
download | gitlab-ce-42b785a9a8dbefcdc65cfcb50f744277c4370b2a.tar.gz |
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
-rw-r--r-- | app/controllers/application_controller.rb | 2 | ||||
-rw-r--r-- | app/models/user.rb | 7 | ||||
-rw-r--r-- | config/initializers/redis_config.rb | 12 | ||||
-rw-r--r-- | lib/gitlab/exclusive_lease.rb | 41 | ||||
-rw-r--r-- | lib/gitlab/user_access.rb | 2 | ||||
-rw-r--r-- | spec/lib/gitlab/exclusive_lease_spec.rb | 21 |
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 |