Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions app/controllers/users/sessions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def create

rate_limit_password_failure = true

return process_failed_captcha unless recaptcha_response.success? || log_captcha_failures_only?
return process_failed_captcha unless recaptcha_response.success?

self.resource = warden.authenticate!(auth_options)
handle_valid_authentication
Expand Down Expand Up @@ -326,10 +326,6 @@ def randomize_check_password?
SecureRandom.random_number(IdentityConfig.store.compromised_password_randomizer_value) >=
IdentityConfig.store.compromised_password_randomizer_threshold
end

def log_captcha_failures_only?
IdentityConfig.store.sign_in_recaptcha_log_failures_only
end
end

def unsafe_redirect_error(_exception)
Expand Down
1 change: 0 additions & 1 deletion config/application.yml.default
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,6 @@ short_term_phone_otp_max_attempts: 2
show_unsupported_passkey_platform_authentication_setup: false
show_user_attribute_deprecation_warnings: false
sign_in_recaptcha_annotation_enabled: false
sign_in_recaptcha_log_failures_only: false
sign_in_recaptcha_percent_tested: 0
sign_in_recaptcha_score_threshold: 0.0
sign_in_user_id_per_ip_attempt_window_exponential_factor: 1.1
Expand Down
1 change: 0 additions & 1 deletion lib/identity_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,6 @@ def self.store
config.add(:sign_in_user_id_per_ip_attempt_window_max_minutes, type: :integer)
config.add(:sign_in_user_id_per_ip_max_attempts, type: :integer)
config.add(:sign_in_recaptcha_annotation_enabled, type: :boolean)
config.add(:sign_in_recaptcha_log_failures_only, type: :boolean)
config.add(:sign_in_recaptcha_percent_tested, type: :integer)
config.add(:sign_in_recaptcha_score_threshold, type: :float)
config.add(:skip_encryption_allowed_list, type: :json)
Expand Down
65 changes: 20 additions & 45 deletions spec/controllers/users/sessions_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -344,58 +344,33 @@
expect(controller.session[:sign_in_recaptcha_assessment_id]).to be_kind_of(String)
end

context 'when configured to log failures only' do
before do
allow(IdentityConfig.store).to receive(:sign_in_recaptcha_log_failures_only)
.and_return(true)
end
it 'tracks unsuccessful authentication for failed reCAPTCHA' do
user = create(:user, :fully_registered)

it 'redirects unsuccessful authentication for failed reCAPTCHA to failed page' do
user = create(:user, :fully_registered)
stub_analytics(user:)

post :create, params: { user: { email: user.email,
password: user.password,
score: 0.1,
recaptcha_token: 'token' } }
post :create, params: { user: { email: user.email, password: user.password, score: 0.1 } }

expect(response).to redirect_to user_two_factor_authentication_url
end
expect(@analytics).to have_logged_event(
'Email and Password Authentication',
success: false,
error_details: { recaptcha_token: { blank: true } },
user_locked_out: false,
rate_limited: false,
valid_captcha_result: false,
captcha_validation_performed: true,
sign_in_failure_count: 1,
remember_device: false,
sp_request_url_present: false,
)
end

context 'when not configured to log failures only' do
before do
allow(IdentityConfig.store).to receive(:sign_in_recaptcha_log_failures_only)
.and_return(false)
end

it 'tracks unsuccessful authentication for failed reCAPTCHA' do
user = create(:user, :fully_registered)

stub_analytics(user:)

post :create, params: { user: { email: user.email, password: user.password, score: 0.1 } }

expect(@analytics).to have_logged_event(
'Email and Password Authentication',
success: false,
error_details: { recaptcha_token: { blank: true } },
user_locked_out: false,
rate_limited: false,
valid_captcha_result: false,
captcha_validation_performed: true,
sign_in_failure_count: 1,
remember_device: false,
sp_request_url_present: false,
)
end

it 'redirects unsuccessful authentication for failed reCAPTCHA to failed page' do
user = create(:user, :fully_registered)
it 'redirects unsuccessful authentication for failed reCAPTCHA to failed page' do
user = create(:user, :fully_registered)

post :create, params: { user: { email: user.email, password: user.password, score: 0.1 } }
post :create, params: { user: { email: user.email, password: user.password, score: 0.1 } }

expect(response).to redirect_to sign_in_security_check_failed_url
end
expect(response).to redirect_to sign_in_security_check_failed_url
end

context 'recaptcha lock out' do
Expand Down
9 changes: 0 additions & 9 deletions spec/features/users/sign_in_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -923,8 +923,6 @@
before do
allow(FeatureManagement).to receive(:sign_in_recaptcha_enabled?).and_return(true)
allow(IdentityConfig.store).to receive(:recaptcha_mock_validator).and_return(true)
allow(IdentityConfig.store).to receive(:sign_in_recaptcha_log_failures_only)
.and_return(sign_in_recaptcha_log_failures_only)
allow(IdentityConfig.store).to receive(:sign_in_recaptcha_score_threshold).and_return(0.2)
allow(IdentityConfig.store).to receive(:sign_in_recaptcha_percent_tested).and_return(100)
reload_ab_tests
Expand All @@ -934,14 +932,7 @@
reload_ab_tests
end

context 'when configured to log failures only' do
let(:sign_in_recaptcha_log_failures_only) { true }
it_behaves_like 'logs reCAPTCHA event and redirects appropriately',
successful_sign_in: true
end

context 'when not configured to log failures only' do
let(:sign_in_recaptcha_log_failures_only) { false }
it_behaves_like 'logs reCAPTCHA event and redirects appropriately',
successful_sign_in: false
end
Expand Down