Skip to content

Commit

Permalink
Fixes theforeman#4238 - Prevent login brute forcing
Browse files Browse the repository at this point in the history
After 30 failed attempts from the same ip, login will be blocked for 5
minutes from that ip.
  • Loading branch information
tbrisker committed Dec 5, 2017
1 parent a4bf524 commit 9b0f0a1
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 3 deletions.
20 changes: 18 additions & 2 deletions app/controllers/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ class UsersController < ApplicationController
skip_before_action :require_login, :authorize, :session_expiry, :update_activity_time, :set_taxonomy, :set_gettext_locale_db, :only => [:login, :logout, :extlogout]
skip_before_action :authorize, :only => :extlogin
after_action :update_activity_time, :only => :login
before_action :verify_active_session, :only => :login
before_action :verify_active_session, :prevent_bruteforce, :only => :login

def index
@users = User.authorized(:view_users).except_hidden.search_for(params[:search], :order => params[:order]).includes(:auth_source, :cached_usergroups).paginate(:page => params[:page], :per_page => params[:per_page])
Expand Down Expand Up @@ -64,7 +64,7 @@ def destroy
# Stores the user id in the session and redirects required URL or default homepage
def login
User.current = nil
if request.post?
if request.post? && !@brute_force_attempt
backup_session_content { reset_session }
intercept = SSO::FormIntercept.new(self)
if intercept.available? && intercept.authenticated?
Expand All @@ -75,6 +75,7 @@ def login
if user.nil?
#failed to authenticate, and/or to generate the account on the fly
inline_error _("Incorrect username or password")
count_failure(request.ip)
redirect_to login_users_path
else
#valid user
Expand All @@ -83,6 +84,9 @@ def login
login_user(user)
end
else
if @brute_force_attempt
inline_error _("Too many tries, please try again in a few minutes.")
end
if params[:status] && params[:status] == "401"
render :layout => 'login', :status => params[:status]
else
Expand Down Expand Up @@ -165,4 +169,16 @@ def verify_active_session
return
end
end

def count_failure(ip)
Rails.cache.write("failed_login_#{ip}", get_failures(ip)+1, :expires_in => 5.minutes)
end

def get_failures(ip)
Rails.cache.fetch("failed_login_#{ip}") {0}
end

def prevent_bruteforce
@brute_force_attempt = get_failures(request.ip) > 30
end
end
2 changes: 1 addition & 1 deletion app/views/users/login.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
</div>
<div class="form-group">
<div class="col-xs-offset-8 col-xs-4 submit">
<%= submit_tag(_("Log In").html_safe, :class => "btn btn-primary", :data => { :disable_with => _("Log In") }) %>
<%= submit_tag(_("Log In").html_safe, :class => "btn btn-primary", :data => { :disable_with => _("Log In") }, :disabled => (@brute_force_attempt)) %>
</div>
</div>
<% end %>
Expand Down
8 changes: 8 additions & 0 deletions test/controllers/users_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,14 @@ class UsersControllerTest < ActionController::TestCase
assert flash[:inline][:error].present?
end

test "#login prevents brute-force login attempts" do
User.expects(:try_to_login).times(31).returns(nil)
32.times do
post :login, {:login => {'login' => 'admin', 'password' => 'password'}}
end
assert_equal "Too many tries, please try again in a few minutes.", flash[:inline][:error]
end

test "#login retains taxonomy session attributes in new session" do
post :login, {:login => {'login' => users(:admin).login, 'password' => 'secret'}},
{:location_id => taxonomies(:location1).id,
Expand Down

0 comments on commit 9b0f0a1

Please sign in to comment.