Skip to content
This repository has been archived by the owner on Feb 6, 2024. It is now read-only.

Commit

Permalink
Lock users after x failed attempts
Browse files Browse the repository at this point in the history
After a configurable amount of consecutive failed login attempts the new attribute `locked_until` will be set.
The default amount of attempts is 5 and the default time the user is locked is 5 minutes.
After this commit locked users will still be able to log in. Locking out locked users is part of the next
commit.
  • Loading branch information
Philippe Hässig committed Aug 10, 2016
1 parent 9ebf812 commit 967fa33
Show file tree
Hide file tree
Showing 10 changed files with 129 additions and 3 deletions.
2 changes: 1 addition & 1 deletion app/controllers/casino/sessions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def new
def create
validation_result = validate_login_credentials(params[:username], params[:password])
if !validation_result
log_failed_login params[:username]
handle_failed_login params[:username]
show_login_error I18n.t('login_credential_acceptor.invalid_login_credentials')
else
sign_in(validation_result, long_term: params[:rememberMe], credentials_supplied: true)
Expand Down
9 changes: 8 additions & 1 deletion app/helpers/casino/sessions_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ module CASino::SessionsHelper
include CASino::TicketGrantingTicketProcessor
include CASino::ServiceTicketProcessor

LOCK_TIMEOUT = 5.minutes

def current_ticket_granting_ticket?(ticket_granting_ticket)
ticket_granting_ticket.ticket == cookies[:tgt]
end
Expand Down Expand Up @@ -51,9 +53,10 @@ def sign_out
cookies.delete :tgt
end

def log_failed_login(username)
def handle_failed_login(username)
CASino::User.where(username: username).each do |user|
create_login_attempt(user, false)
lock_user(user) if user.max_failed_logins_reached?(CASino.config.max_failed_login_attempts)
end
end

Expand Down Expand Up @@ -91,4 +94,8 @@ def handle_signed_in_with_service(tgt, options)
redirect_to url, status: :see_other
end
end

def lock_user(user)
user.update locked_until: LOCK_TIMEOUT.from_now
end
end
4 changes: 4 additions & 0 deletions app/models/casino/login_attempt.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,8 @@ class CASino::LoginAttempt < ActiveRecord::Base
include CASino::ModelConcern::BrowserInfo

belongs_to :user

def failed?
!successful?
end
end
10 changes: 10 additions & 0 deletions app/models/casino/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,14 @@ class CASino::User < ActiveRecord::Base
def active_two_factor_authenticator
self.two_factor_authenticators.where(active: true).first
end

def locked?
return false unless locked_until
locked_until.future?
end

def max_failed_logins_reached?(max)
max ||= 5
login_attempts.last(max).count(&:failed?) == max
end
end
5 changes: 5 additions & 0 deletions db/migrate/20160810113208_add_locked_until_to_users.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddLockedUntilToUsers < ActiveRecord::Migration
def change
add_column :casino_users, :locked_until, :datetime
end
end
18 changes: 18 additions & 0 deletions spec/controllers/sessions_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,24 @@
expect(CASino::LoginAttempt.last.user).to eq user
expect(CASino::LoginAttempt.last.successful).to eq false
end

it 'does not lock the user' do
expect do
post :create, params
end.to_not change { user.reload.locked? }
end

context 'when the maximum of failed login attempts is reached' do
before do
allow(CASino.config).to receive(:max_failed_login_attempts).and_return(1)
end

it 'deactivates the user' do
expect do
post :create, params
end.to change { user.reload.locked? }.from(false).to(true)
end
end
end

context 'with valid credentials' do
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# This migration comes from casino (originally 20160810113208)
class AddLockedUntilToUsers < ActiveRecord::Migration
def change
add_column :casino_users, :locked_until, :datetime
end
end
3 changes: 2 additions & 1 deletion spec/dummy/db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema.define(version: 20160502074450) do
ActiveRecord::Schema.define(version: 20160810122605) do

create_table "casino_auth_token_tickets", force: true do |t|
t.string "ticket", null: false
Expand Down Expand Up @@ -119,6 +119,7 @@
t.text "extra_attributes"
t.datetime "created_at"
t.datetime "updated_at"
t.datetime "locked_until"
end

add_index "casino_users", ["authenticator", "username"], name: "index_casino_users_on_authenticator_and_username", unique: true
Expand Down
12 changes: 12 additions & 0 deletions spec/model/login_attempt_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,16 @@
subject { described_class.new user_agent: 'TestBrowser' }

it_behaves_like 'has browser info'

describe '#failed?' do
it 'is true when it is not successful' do
login_attempt = FactoryGirl.create :login_attempt, successful: false
expect(login_attempt).to be_failed
end

it 'is false when it is successful' do
login_attempt = FactoryGirl.create :login_attempt, successful: true
expect(login_attempt).to_not be_failed
end
end
end
63 changes: 63 additions & 0 deletions spec/model/user_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
require 'spec_helper'

RSpec.describe CASino::User do
let(:user) { FactoryGirl.create :user }

describe '#locked?' do
it 'is true when locked_until is in the future' do
user = FactoryGirl.create :user, locked_until: 1.hour.from_now
expect(user).to be_locked
end

it 'is false when locked_until is in the past' do
user = FactoryGirl.create :user, locked_until: 1.hour.ago
expect(user).to_not be_locked
end

it 'is false when locked_until is empty' do
user = FactoryGirl.create :user, locked_until: nil
expect(user).to_not be_locked
end
end

describe '#max_failed_logins_reached?' do
let(:max_failed_attempts) { 2 }

subject { user.max_failed_logins_reached?(max_failed_attempts) }

context 'when the user has no login attempts' do
it { is_expected.to eq false }
end

context 'when the user has only successful logins' do
it { is_expected.to eq false }
end

context 'when the maximum of attempts is reached' do
before { FactoryGirl.create_list :login_attempt, 2, successful: false, user: user }

context 'in a row' do
it { is_expected.to eq true }
end

context 'but the last attempt was successful' do
before { FactoryGirl.create :login_attempt, successful: true, user: user }
it { is_expected.to eq false }
end

context 'but a successful between' do
before do
FactoryGirl.create :login_attempt, successful: true, user: user
FactoryGirl.create :login_attempt, successful: false, user: user
end

it { is_expected.to eq false }
end
end

context 'when the user has less then the maximum failed attempts' do
before { FactoryGirl.create :login_attempt, successful: false, user: user }
it { is_expected.to eq false }
end
end
end

0 comments on commit 967fa33

Please sign in to comment.