Merge branch 'enhance-logging' into 'master'

Improved file logging for authentication events

See merge request gitlab-org/gitlab-ce!9196

5 files changed, 28 insertions, 3 deletions

diff --git a/app/controllers/concerns/authenticates_with_two_factor.rb b/app/controllers/concerns/authenticates_with_two_factor.rb
index b75e401a8df..db8c362f125 100644
--- a/app/controllers/concerns/authenticates_with_two_factor.rb
+++ b/app/controllers/concerns/authenticates_with_two_factor.rb
@@ -59,6 +59,7 @@ module AuthenticatesWithTwoFactor
       sign_in(user)
     else
       user.increment_failed_attempts!
+      Gitlab::AppLogger.info("Failed Login: user=#{user.username} ip=#{request.remote_ip} method=OTP")
       flash.now[:alert] = 'Invalid two-factor code.'
       prompt_for_two_factor(user)
     end
@@ -75,6 +76,7 @@ module AuthenticatesWithTwoFactor
       sign_in(user)
     else
       user.increment_failed_attempts!
+      Gitlab::AppLogger.info("Failed Login: user=#{user.username} ip=#{request.remote_ip} method=U2F")
       flash.now[:alert] = 'Authentication via U2F device failed.'
       prompt_for_two_factor(user)
     end
diff --git a/app/controllers/confirmations_controller.rb b/app/controllers/confirmations_controller.rb
index 10d2665c06a..0c2646d7bf0 100644
--- a/app/controllers/confirmations_controller.rb
+++ b/app/controllers/confirmations_controller.rb
@@ -14,6 +14,7 @@ class ConfirmationsController < Devise::ConfirmationsController
     if signed_in?(resource_name)
       after_sign_in(resource)
     else
+      Gitlab::AppLogger.info("Email Confirmed: username=#{resource.username} email=#{resource.email} ip=#{request.remote_ip}")
       flash[:notice] += " Please sign in."
       new_session_path(resource_name)
     end
diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb
index 1bc6520370a..5ea3a5d5562 100644
--- a/app/controllers/registrations_controller.rb
+++ b/app/controllers/registrations_controller.rb
@@ -42,10 +42,12 @@ class RegistrationsController < Devise::RegistrationsController
   end
 
   def after_sign_up_path_for(user)
+    Gitlab::AppLogger.info("User Created: username=#{user.username} email=#{user.email} ip=#{request.remote_ip} confirmed:#{user.confirmed?}")
     user.confirmed? ? dashboard_projects_path : users_almost_there_path
   end
 
-  def after_inactive_sign_up_path_for(_resource)
+  def after_inactive_sign_up_path_for(resource)
+    Gitlab::AppLogger.info("User Created: username=#{resource.username} email=#{resource.email} ip=#{request.remote_ip} confirmed:false")
     users_almost_there_path
   end
 
diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb
index fe3bb117410..4223c6171a6 100644
--- a/app/controllers/sessions_controller.rb
+++ b/app/controllers/sessions_controller.rb
@@ -13,6 +13,8 @@ class SessionsController < Devise::SessionsController
   before_action :auto_sign_in_with_provider, only: [:new]
   before_action :load_recaptcha
 
+  after_action :log_failed_login, only: [:new]
+
   def new
     set_minimum_password_length
     @ldap_servers = Gitlab::LDAP::Config.available_servers
@@ -29,12 +31,13 @@ class SessionsController < Devise::SessionsController
       end
       # hide the signed-in notification
       flash[:notice] = nil
-      log_audit_event(current_user, with: authentication_method)
+      log_audit_event(current_user, resource, with: authentication_method)
       log_user_activity(current_user)
     end
   end
 
   def destroy
+    Gitlab::AppLogger.info("User Logout: username=#{current_user.username} ip=#{request.remote_ip}")
     super
     # hide the signed_out notice
     flash[:notice] = nil
@@ -42,6 +45,16 @@ class SessionsController < Devise::SessionsController
 
   private
 
+  def log_failed_login
+    return unless failed_login?
+
+    Gitlab::AppLogger.info("Failed Login: username=#{user_params[:login]} ip=#{request.remote_ip}")
+  end
+
+  def failed_login?
+    (options = env["warden.options"]) && options[:action] == "unauthenticated"
+  end
+
   def login_counter
     @login_counter ||= Gitlab::Metrics.counter(:user_session_logins_total, 'User sign in count')
   end
@@ -123,7 +136,8 @@ class SessionsController < Devise::SessionsController
       user.invalidate_otp_backup_code!(user_params[:otp_attempt])
   end
 
-  def log_audit_event(user, options = {})
+  def log_audit_event(user, resource, options = {})
+    Gitlab::AppLogger.info("Successful Login: username=#{resource.username} ip=#{request.remote_ip} method=#{options[:with]} admin=#{resource.admin?}")
     AuditEventService.new(user, user, options)
       .for_authentication.security_event
   end
diff --git a/app/models/user.rb b/app/models/user.rb
index e5f345e93ae..4e71a3e11c2 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -1067,6 +1067,12 @@ class User < ActiveRecord::Base
     user_synced_attributes_metadata&.read_only?(attribute)
   end
 
+  # override, from Devise
+  def lock_access!
+    Gitlab::AppLogger.info("Account Locked: username=#{username}")
+    super
+  end
+
   protected
 
   # override, from Devise::Validatable