Commit 4bcf72e7 authored by Grzegorz Bizon's avatar Grzegorz Bizon

Improve blocked user tracking and fire some events only once

parent e9d04585
...@@ -108,6 +108,7 @@ class ApplicationController < ActionController::Base ...@@ -108,6 +108,7 @@ class ApplicationController < ActionController::Base
def append_info_to_payload(payload) def append_info_to_payload(payload)
super super
payload[:remote_ip] = request.remote_ip payload[:remote_ip] = request.remote_ip
logged_user = auth_user logged_user = auth_user
...@@ -122,12 +123,19 @@ class ApplicationController < ActionController::Base ...@@ -122,12 +123,19 @@ class ApplicationController < ActionController::Base
end end
end end
##
# Controllers such as GitHttpController may use alternative methods # Controllers such as GitHttpController may use alternative methods
# (e.g. tokens) to authenticate the user, whereas Devise sets current_user # (e.g. tokens) to authenticate the user, whereas Devise sets current_user.
#
# `current_user` call is going to trigger Warden::Proxy authentication
# that is going to invoke warden callbacks, so we use Warden directly here.
#
def auth_user def auth_user
return current_user if current_user.present? if warden.authenticated?(:user)
current_user
return try(:authenticated_user) else
try(:authenticated_user)
end
end end
# This filter handles personal access tokens, and atom requests with rss tokens # This filter handles personal access tokens, and atom requests with rss tokens
......
...@@ -35,7 +35,11 @@ Rails.application.configure do |config| ...@@ -35,7 +35,11 @@ Rails.application.configure do |config|
Warden::Manager.before_logout(scope: :user) do |user_warden, auth, opts| Warden::Manager.before_logout(scope: :user) do |user_warden, auth, opts|
user = user_warden || auth.user user = user_warden || auth.user
Gitlab::Auth::Activity.new(user, opts).tap do |activity|
activity.user_blocked! if user.blocked?
activity.user_session_destroyed!
end
ActiveSession.destroy(user, auth.request.session.id) ActiveSession.destroy(user, auth.request.session.id)
Gitlab::Auth::Activity.new(user, opts).user_session_destroyed!
end end
end end
...@@ -32,8 +32,6 @@ module Gitlab ...@@ -32,8 +32,6 @@ module Gitlab
when :invalid when :invalid
self.class.user_password_invalid_counter_increment! self.class.user_password_invalid_counter_increment!
end end
self.class.user_blocked_counter_increment! if @user&.blocked?
end end
def user_authenticated! def user_authenticated!
...@@ -51,6 +49,10 @@ module Gitlab ...@@ -51,6 +49,10 @@ module Gitlab
end end
end end
def user_blocked!
self.class.user_blocked_counter_increment!
end
def user_session_destroyed! def user_session_destroyed!
self.class.user_session_destroyed_counter_increment! self.class.user_session_destroyed_counter_increment!
end end
......
...@@ -71,7 +71,7 @@ describe 'Login' do ...@@ -71,7 +71,7 @@ describe 'Login' do
expect(authentication_metrics) expect(authentication_metrics)
.to increment(:user_blocked_counter) .to increment(:user_blocked_counter)
.and increment(:user_unauthenticated_counter) .and increment(:user_unauthenticated_counter)
.and increment(:user_session_destroyed_counter).twice .and increment(:user_session_destroyed_counter)
user = create(:user, :blocked) user = create(:user, :blocked)
...@@ -84,7 +84,7 @@ describe 'Login' do ...@@ -84,7 +84,7 @@ describe 'Login' do
expect(authentication_metrics) expect(authentication_metrics)
.to increment(:user_blocked_counter) .to increment(:user_blocked_counter)
.and increment(:user_unauthenticated_counter) .and increment(:user_unauthenticated_counter)
.and increment(:user_session_destroyed_counter).twice .and increment(:user_session_destroyed_counter)
user = create(:user, :blocked) user = create(:user, :blocked)
......
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