Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
49c3fe0
Add EmailNormalizer
jc-gsa Aug 30, 2023
48c774c
Add unconfirmed rate limit
jc-gsa Aug 31, 2023
d031588
Add confirmed rate limit
jc-gsa Aug 31, 2023
d51c61e
Normalize email upon registration
jc-gsa Aug 31, 2023
c906227
Add normalized email lookup
jc-gsa Aug 31, 2023
192d783
Add normalized email
jc-gsa Sep 1, 2023
404f44b
Add test rate limit when email is taken
jc-gsa Sep 1, 2023
8f4a65f
Add test rate limit when email is taken and unconfirmed
jc-gsa Sep 1, 2023
e5d10d7
Temporarily disable email validity test
jc-gsa Sep 1, 2023
427451e
Sanitize email
jc-gsa Sep 1, 2023
300a3f3
sanitize
jc-gsa Sep 1, 2023
afb3792
lint
jc-gsa Sep 1, 2023
4c4fade
Add IDN domain support
jc-gsa Sep 5, 2023
c2764bf
Add Gemfile.lock
jc-gsa Sep 5, 2023
2cef49c
Cleanup
jc-gsa Sep 5, 2023
eaef747
Add memo
jc-gsa Sep 5, 2023
73f749e
Remove redundant calls
jc-gsa Sep 5, 2023
841ac61
Cleanup
jc-gsa Sep 5, 2023
b7328a0
Use email_fingerprint
jc-gsa Sep 6, 2023
c192fe8
Add google_mx_lookup flag
jc-gsa Sep 6, 2023
3d59e2f
Add spec
jc-gsa Sep 6, 2023
81c5347
Refactor
jc-gsa Sep 6, 2023
0883f83
Revert "Add Gemfile.lock"
jc-gsa Sep 7, 2023
155116d
Reveal internationalized domain name failure
jc-gsa Sep 7, 2023
6a23828
Handle unexpected input
jc-gsa Sep 7, 2023
195b367
Consolidate normalized_email
jc-gsa Sep 7, 2023
374e1c2
Reduce lookup
jc-gsa Sep 8, 2023
0007d0e
Add refactor
jc-gsa Sep 11, 2023
db3837c
Empty commit
jc-gsa Sep 15, 2023
4b5ecb8
Merge branch 'main' into LG-10693-registration-rate-limit-normalized-…
jc-gsa Sep 15, 2023
d5f8e6d
Limit now includes sign up
jc-gsa Sep 18, 2023
12b3491
Limit now includes sign up
jc-gsa Sep 18, 2023
eb20775
Merge branch 'main' of github.com:18F/identity-idp
jc-gsa Sep 25, 2023
e69760c
Merge branch 'main' into LG-10693-registration-rate-limit-normalized-…
jc-gsa Sep 25, 2023
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
73 changes: 56 additions & 17 deletions app/forms/register_user_email_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,26 @@ def user
@user ||= User.new
end

def rate_limited?
@rate_limited
end

def email
email_address&.email
end

def email_fingerprint
email_address&.email_fingerprint
end

def normalized_email
@normalized_email ||= EmailNormalizer.new(email).normalized_email
end

def digested_base_email
@digested_base_email ||= OpenSSL::Digest::SHA256.hexdigest(normalized_email)
end

def validate_terms_accepted
return if @terms_accepted || email_address_record&.user&.accepted_terms_at.present?

Expand Down Expand Up @@ -93,13 +109,7 @@ def process_successful_submission(request_id, instructions)
elsif email_taken?
send_sign_up_confirmed_email
else
user.accepted_terms_at = Time.zone.now
user.save!
SendSignUpEmailConfirmation.new(user).call(
request_id: email_request_id(request_id),
instructions: instructions,
password_reset_requested: password_reset_requested?,
)
send_sign_up_email(request_id, instructions)
end
end

Expand All @@ -114,16 +124,46 @@ def extra_analytics_attributes
email_already_exists: email_taken?,
user_id: user.uuid || existing_user.uuid,
domain_name: email&.split('@')&.last,
rate_limited: @rate_limited,
rate_limited: rate_limited?,
}
end

def send_sign_up_unconfirmed_email(request_id)
rate_limiter = RateLimiter.new(user: existing_user, rate_limit_type: :reg_unconfirmed_email)
def rate_limit!(rate_limit_type)
rate_limiter = RateLimiter.new(
target: digested_base_email,
rate_limit_type: rate_limit_type,
)

rate_limiter.increment!
@rate_limited = rate_limiter.limited?
end

def send_sign_up_email(request_id, instructions)
rate_limit!(:reg_unconfirmed_email)

if rate_limited?
@analytics.rate_limit_reached(
limiter_type: :reg_unconfirmed_email,
)
@attempts_tracker.user_registration_email_submission_rate_limited(
email: email, email_already_registered: false,
)
else
user.accepted_terms_at = Time.zone.now
user.save!

SendSignUpEmailConfirmation.new(user).call(
request_id: email_request_id(request_id),
instructions: instructions,
password_reset_requested: password_reset_requested?,
)
end
end

if @rate_limited
def send_sign_up_unconfirmed_email(request_id)
rate_limit!(:reg_unconfirmed_email)

if rate_limited?
@analytics.rate_limit_reached(
limiter_type: :reg_unconfirmed_email,
)
Expand All @@ -136,11 +176,9 @@ def send_sign_up_unconfirmed_email(request_id)
end

def send_sign_up_confirmed_email
rate_limiter = RateLimiter.new(user: existing_user, rate_limit_type: :reg_confirmed_email)
rate_limiter.increment!
@rate_limited = rate_limiter.limited?
rate_limit!(:reg_confirmed_email)

if @rate_limited
if rate_limited?
@analytics.rate_limit_reached(
limiter_type: :reg_confirmed_email,
)
Expand All @@ -166,8 +204,8 @@ def user_unconfirmed?

def email_address_record
return @email_address_record if defined?(@email_address_record)

@email_address_record = EmailAddress.find_with_email(email)
@email_address_record
end

def existing_user
Expand All @@ -180,6 +218,7 @@ def email_request_id(request_id)

def blocked_email_address
return @blocked_email_address if defined?(@blocked_email_address)
@blocked_email_address = SuspendedEmail.find_with_email(email)

@blocked_email_address = SuspendedEmail.find_with_email_digest(digested_base_email)
end
end
4 changes: 4 additions & 0 deletions app/models/suspended_email.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,9 @@ def create_from_email_address!(email_address)
def find_with_email(email)
find_by(digested_base_email: generate_email_digest(email))&.email_address
end

def find_with_email_digest(digest)
find_by(digested_base_email: digest)&.email_address
end
end
end
1 change: 1 addition & 0 deletions app/services/email_normalizer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ def gmail?

def google_mx_record?
return false if ENV['RAILS_OFFLINE']
return false if email.domain.blank? || !email.domain.to_s.ascii_only?

mx_records(email.domain).any? { |domain| domain.end_with?('google.com') }
end
Expand Down
12 changes: 7 additions & 5 deletions spec/features/visitors/sign_up_with_email_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,14 +72,16 @@
sign_up_with(email)

starting_count = unread_emails_for(email).size
max_attempts = IdentityConfig.store.reg_unconfirmed_email_max_attempts
(max_attempts - 1).times do |i|
remaining_attempts = IdentityConfig.store.
reg_unconfirmed_email_max_attempts - 1 - starting_count

1.upto(remaining_attempts) do |i|
sign_up_with(email)
expect(unread_emails_for(email).size).to eq(starting_count + i + 1)
expect(unread_emails_for(email).size).to eq(starting_count + i)
end

expect(unread_emails_for(email).size).to eq(starting_count + max_attempts - 1)
expect(unread_emails_for(email).size).to eq(starting_count + remaining_attempts)
sign_up_with(email)
expect(unread_emails_for(email).size).to eq(starting_count + max_attempts - 1)
expect(unread_emails_for(email).size).to eq(starting_count + remaining_attempts)
end
end
52 changes: 50 additions & 2 deletions spec/forms/register_user_email_form_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@
it_behaves_like 'email validation'

describe '#submit' do
let(:anonymous_uuid) { 'anonymous-uuid' }
let(:email_domain) { 'gmail.com' }
let(:registered_email_address) { 'taken@' + email_domain }
let(:variation_of_preexisting_email) { 'TAKEN@' + email_domain }
let(:unregistered_email_address) { 'not_taken@' + email_domain }
let(:registered_and_confirmed_user) do
[:user, :fully_registered, **{ email: registered_email_address }]
Expand Down Expand Up @@ -94,7 +96,6 @@
end
end

let(:variation_of_preexisting_email) { 'TAKEN@' + email_domain }
context 'when email is already taken' do
let!(:existing_user) { create(*registered_and_confirmed_user) }
let(:extra_params) do
Expand Down Expand Up @@ -191,6 +192,30 @@
limiter_type: :reg_unconfirmed_email,
)
end

context 'with the same normalized email address' do
let(:rate_limit) { IdentityConfig.store.reg_unconfirmed_email_max_attempts }

it 'creates rate_limiter events after reaching rate_limiter limit' do
expect(attempts_tracker).to receive(
:user_registration_email_submission_rate_limited,
).with(
email: "taken+#{rate_limit}@gmail.com", email_already_registered: false,
)

1.upto(rate_limit) do |i|
RegisterUserEmailForm.new(analytics: analytics, attempts_tracker: attempts_tracker).
submit(
email: "taken+#{i}@gmail.com", terms_accepted: '1',
)
end

expect(analytics).to have_logged_event(
'Rate Limit Reached',
limiter_type: :reg_unconfirmed_email,
)
end
end
end

context 'when the email exists but is unconfirmed and on a confirmed user' do
Expand Down Expand Up @@ -240,9 +265,32 @@

expect(User.find_with_email(unregistered_email_address).email_language).to eq('fr')
end

context 'with the same normalized email address' do
let(:rate_limit) { IdentityConfig.store.reg_unconfirmed_email_max_attempts }

it 'creates rate_limiter events after reaching rate_limiter limit' do
expect(attempts_tracker).to receive(
:user_registration_email_submission_rate_limited,
).with(
email: "taken+#{rate_limit}@gmail.com", email_already_registered: false,
)

1.upto(rate_limit) do |i|
RegisterUserEmailForm.new(analytics: analytics, attempts_tracker: attempts_tracker).
submit(
email: "taken+#{i}@gmail.com", terms_accepted: '1',
)
end

expect(analytics).to have_logged_event(
'Rate Limit Reached',
limiter_type: :reg_unconfirmed_email,
)
end
end
end

let(:anonymous_uuid) { 'anonymous-uuid' }
context 'when email is invalid' do
it 'returns false and adds errors to the form object' do
invalid_email = 'invalid_email'
Expand Down
25 changes: 25 additions & 0 deletions spec/models/suspended_email_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,29 @@
end
end
end

describe '.find_with_email_digest' do
context 'when the email is not blocked' do
it 'returns nil' do
email = 'not_blocked@example.com'
digested_base_email = Digest::SHA256.hexdigest(email)

expect(SuspendedEmail.find_with_email_digest(digested_base_email)).to be_nil
end
end

context 'when the email is blocked' do
it 'returns the original email address' do
blocked_email = FactoryBot.create(:email_address, email: 'blocked@example.com')
digested_base_email = Digest::SHA256.hexdigest('blocked@example.com')
FactoryBot.create(
:suspended_email,
digested_base_email: digested_base_email,
email_address: blocked_email,
)

expect(SuspendedEmail.find_with_email_digest(digested_base_email)).to eq(blocked_email)
end
end
end
end
27 changes: 27 additions & 0 deletions spec/services/email_normalizer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,22 @@
describe '#normalized_email' do
subject(:normalized_email) { normalizer.normalized_email }

context 'when email is nil' do
let(:email) { nil }

it 'is an empty string' do
expect(normalized_email).to eq(email.to_s)
end
end

context 'with an invalid email' do
let(:email) { 'invalid_email' }

it 'is the same string' do
expect(normalized_email).to eq(email)
end
end

context 'with a non-gmail domain' do
let(:email) { 'foobar+123@example.com' }

Expand Down Expand Up @@ -57,5 +73,16 @@
end
end
end

context 'with an internationalized domain name' do
let(:email) { 'test+1@çà.com' }
let(:rails_offline) { false }

before { stub_const('ENV', 'RAILS_OFFLINE' => (rails_offline ? 'TRUE' : nil)) }

it 'is the same email' do
expect(normalized_email).to eq(email)
end
end
end
end