diff --git a/app/controllers/casino/sessions_controller.rb b/app/controllers/casino/sessions_controller.rb index 50c77ad4..5f9ea1d7 100644 --- a/app/controllers/casino/sessions_controller.rb +++ b/app/controllers/casino/sessions_controller.rb @@ -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) diff --git a/app/helpers/casino/sessions_helper.rb b/app/helpers/casino/sessions_helper.rb index 13d27d74..5d6cf574 100644 --- a/app/helpers/casino/sessions_helper.rb +++ b/app/helpers/casino/sessions_helper.rb @@ -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 @@ -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 @@ -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 diff --git a/app/models/casino/login_attempt.rb b/app/models/casino/login_attempt.rb index 73ca13ca..b4477635 100644 --- a/app/models/casino/login_attempt.rb +++ b/app/models/casino/login_attempt.rb @@ -2,4 +2,8 @@ class CASino::LoginAttempt < ActiveRecord::Base include CASino::ModelConcern::BrowserInfo belongs_to :user + + def failed? + !successful? + end end diff --git a/app/models/casino/user.rb b/app/models/casino/user.rb index 2bd2a72b..b110d02c 100644 --- a/app/models/casino/user.rb +++ b/app/models/casino/user.rb @@ -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 diff --git a/db/migrate/20160810113208_add_locked_until_to_users.rb b/db/migrate/20160810113208_add_locked_until_to_users.rb new file mode 100644 index 00000000..085b184e --- /dev/null +++ b/db/migrate/20160810113208_add_locked_until_to_users.rb @@ -0,0 +1,5 @@ +class AddLockedUntilToUsers < ActiveRecord::Migration + def change + add_column :casino_users, :locked_until, :datetime + end +end diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index 79ecb3f8..264898c0 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -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 diff --git a/spec/dummy/db/migrate/20160810122605_add_locked_until_to_users.casino.rb b/spec/dummy/db/migrate/20160810122605_add_locked_until_to_users.casino.rb new file mode 100644 index 00000000..a09eea44 --- /dev/null +++ b/spec/dummy/db/migrate/20160810122605_add_locked_until_to_users.casino.rb @@ -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 diff --git a/spec/dummy/db/schema.rb b/spec/dummy/db/schema.rb index 8cbc2295..57bb4f29 100644 --- a/spec/dummy/db/schema.rb +++ b/spec/dummy/db/schema.rb @@ -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 @@ -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 diff --git a/spec/model/login_attempt_spec.rb b/spec/model/login_attempt_spec.rb index 2fe7af25..42d95535 100644 --- a/spec/model/login_attempt_spec.rb +++ b/spec/model/login_attempt_spec.rb @@ -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 diff --git a/spec/model/user_spec.rb b/spec/model/user_spec.rb new file mode 100644 index 00000000..3bfcd54e --- /dev/null +++ b/spec/model/user_spec.rb @@ -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