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

Commit 5167b81

Browse files
committed
Defers credential check, take two.
Prevents timing attack concern by checking the user lock status first. The concern seemed very valid and was raised in #169 (comment) .
1 parent fc49a9a commit 5167b81

2 files changed

Lines changed: 11 additions & 12 deletions

File tree

app/controllers/casino/sessions_controller.rb

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,14 @@ def new
2121
end
2222

2323
def create
24+
return show_login_error I18n.t('login_credential_acceptor.user_is_locked') if user_locked?(params[:username])
25+
2426
validation_result = validate_login_credentials(params[:username], params[:password])
25-
if !validation_result
27+
if validation_result
28+
sign_in(validation_result, long_term: params[:rememberMe], credentials_supplied: true)
29+
else
2630
handle_failed_login params[:username]
2731
show_login_error I18n.t('login_credential_acceptor.invalid_login_credentials')
28-
elsif user_from_validation_result(validation_result).locked?
29-
show_login_error I18n.t('login_credential_acceptor.user_is_locked')
30-
else
31-
sign_in(validation_result, long_term: params[:rememberMe], credentials_supplied: true)
3232
end
3333
end
3434

@@ -85,11 +85,4 @@ def load_ticket_granting_ticket_from_parameter
8585
@ticket_granting_ticket = find_valid_ticket_granting_ticket(params[:tgt], request.user_agent, ignore_two_factor: true)
8686
redirect_to login_path if @ticket_granting_ticket.nil?
8787
end
88-
89-
def user_from_validation_result(validation_result)
90-
user_data = validation_result[:user_data]
91-
load_or_initialize_user(validation_result[:authenticator],
92-
user_data[:username],
93-
user_data[:extra_attributes])
94-
end
9588
end

app/helpers/casino/sessions_helper.rb

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,12 @@ def sign_out
5151
cookies.delete :tgt
5252
end
5353

54+
def user_locked?(username)
55+
CASino::User.where(username: username).to_a.any? do |user|
56+
user.locked?
57+
end
58+
end
59+
5460
def handle_failed_login(username)
5561
CASino::User.where(username: username).each do |user|
5662
create_login_attempt(user, false)

0 commit comments

Comments
 (0)