diff options
author | Sean McGivern <sean@mcgivern.me.uk> | 2018-08-03 15:22:15 +0300 |
---|---|---|
committer | Sean McGivern <sean@mcgivern.me.uk> | 2018-08-03 15:22:15 +0300 |
commit | d3a6712e419bc6fc2c11a584d17ec83c2c2d3522 (patch) | |
tree | b584406549fe84443a5913f0d756d7fc6d454078 /lib/gitlab | |
parent | c3d02481519a29927e63d55b417b3e5ef71f1bd1 (diff) | |
parent | 932e80ed0ec0002e76a1eca03ac7d5a642b8d580 (diff) |
Merge branch 'fix/gb/improve-blocked-user-tracking' into 'master'
Improve blocked user tracking and fire some events only once
Closes #49784
See merge request gitlab-org/gitlab-ce!20959
Diffstat (limited to 'lib/gitlab')
-rw-r--r-- | lib/gitlab/auth/activity.rb | 9 | ||||
-rw-r--r-- | lib/gitlab/auth/blocked_user_tracker.rb | 55 |
2 files changed, 14 insertions, 50 deletions
diff --git a/lib/gitlab/auth/activity.rb b/lib/gitlab/auth/activity.rb index 9f84c578d4f..761f0819c60 100644 --- a/lib/gitlab/auth/activity.rb +++ b/lib/gitlab/auth/activity.rb @@ -18,8 +18,7 @@ module Gitlab user_blocked: 'Counter of sign in attempts when user is blocked' }.freeze - def initialize(user, opts) - @user = user + def initialize(opts) @opts = opts end @@ -32,8 +31,6 @@ module Gitlab when :invalid self.class.user_password_invalid_counter_increment! end - - self.class.user_blocked_counter_increment! if @user&.blocked? end def user_authenticated! @@ -51,6 +48,10 @@ module Gitlab end end + def user_blocked! + self.class.user_blocked_counter_increment! + end + def user_session_destroyed! self.class.user_session_destroyed_counter_increment! end diff --git a/lib/gitlab/auth/blocked_user_tracker.rb b/lib/gitlab/auth/blocked_user_tracker.rb index b6d2adc834b..50712d7eac2 100644 --- a/lib/gitlab/auth/blocked_user_tracker.rb +++ b/lib/gitlab/auth/blocked_user_tracker.rb @@ -2,58 +2,21 @@ module Gitlab module Auth class BlockedUserTracker - include Gitlab::Utils::StrongMemoize - - ACTIVE_RECORD_REQUEST_PARAMS = 'action_dispatch.request.request_parameters' - - def initialize(env) - @env = env - end - - def user_blocked? - user&.blocked? + def initialize(user, auth) + @user = user + @auth = auth end - def user - return unless has_user_blocked_message? + def log_activity! + return unless @user.blocked? - strong_memoize(:user) do - # Check for either LDAP or regular GitLab account logins - login = @env.dig(ACTIVE_RECORD_REQUEST_PARAMS, 'username') || - @env.dig(ACTIVE_RECORD_REQUEST_PARAMS, 'user', 'login') + Gitlab::AppLogger.info <<~INFO + "Failed login for blocked user: user=#{@user.username} ip=#{@auth.request.ip}") + INFO - User.by_login(login) if login.present? - end + SystemHooksService.new.execute_hooks_for(@user, :failed_login) rescue TypeError end - - def log_blocked_user_activity! - return unless user_blocked? - - Gitlab::AppLogger.info("Failed login for blocked user: user=#{user.username} ip=#{@env['REMOTE_ADDR']}") - SystemHooksService.new.execute_hooks_for(user, :failed_login) - true - rescue TypeError - end - - private - - ## - # Devise calls User#active_for_authentication? on the User model and then - # throws an exception to Warden with User#inactive_message: - # https://github.com/plataformatec/devise/blob/v4.2.1/lib/devise/hooks/activatable.rb#L8 - # - # Since Warden doesn't pass the user record to the failure handler, we - # need to do a database lookup with the username. We can limit the - # lookups to happen when the user was blocked by checking the inactive - # message passed along by Warden. - # - def has_user_blocked_message? - strong_memoize(:user_blocked_message) do - message = @env.dig('warden.options', :message) - message == User::BLOCKED_MESSAGE - end - end end end end |