From 8fa87ea3fb862bdae624aec360c80b12cda3905c Mon Sep 17 00:00:00 2001 From: Brian Neel Date: Wed, 23 Aug 2017 00:40:16 -0400 Subject: [PATCH] # This is a combination of 1 commit. # This is the 1st commit message: Add logging for all web authentication events # This is the commit message #2: Re-add underscore to after_inactive_sign_up_path_for # This is the commit message #3: Standardize on username= # This is the commit message #4: after_filter -> after_action, _resource -> resource # This is the commit message #5: Add two-factor login failures and account lockouts # This is the commit message #6: Move logging from two-factor concern to user model # This is the commit message #7: Add spaces around default parameter assignments # This is the commit message #8: Move logs out of user model # This is the commit message #9: Replace filtered_params with user_params # This is the commit message #10: Standardize case # This is the commit message #1: Fixes for username and AppLogger.info --- .../concerns/authenticates_with_two_factor.rb | 2 ++ app/controllers/confirmations_controller.rb | 1 + app/controllers/registrations_controller.rb | 4 +++- app/controllers/sessions_controller.rb | 16 ++++++++++++++-- app/models/user.rb | 6 ++++++ 5 files changed, 26 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..5281dbe11c4 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 306afb65f10..1b87ae0ccb2 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_path_for(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 be6491d042c..a2cb5b692af 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,14 @@ class SessionsController < Devise::SessionsController private + def log_failed_login + Gitlab::AppLogger.info("Failed login: username=#{user_params[:login]} ip=#{request.remote_ip}") if failed_login? + 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 +134,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("User 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 09c9b3250eb..9fc6d2806c3 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1069,6 +1069,12 @@ class User < ActiveRecord::Base super end + # override, from Devise + def lock_access! + Gitlab::AppLogger.info("Account Locked: username=#{username} reason=invalid_login_attempts") + super + end + private def ci_projects_union