Skip to content

Commit

Permalink
rate limit email sends for reverifying emails (pypi#12287)
Browse files Browse the repository at this point in the history
  • Loading branch information
ewdurbin authored Sep 28, 2022
1 parent d02f16c commit e2f55ab
Show file tree
Hide file tree
Showing 10 changed files with 168 additions and 74 deletions.
2 changes: 2 additions & 0 deletions tests/unit/accounts/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,7 @@ def test_includeme(monkeypatch):
"warehouse.account.ip_login_ratelimit_string": "10 per 5 minutes",
"warehouse.account.global_login_ratelimit_string": "1000 per 5 minutes",
"warehouse.account.email_add_ratelimit_string": "2 per day",
"warehouse.account.verify_email_ratelimit_string": "3 per 6 hours",
"warehouse.account.password_reset_ratelimit_string": "5 per day",
}
),
Expand Down Expand Up @@ -369,6 +370,7 @@ def test_includeme(monkeypatch):
),
pretend.call(RateLimit("2 per day"), IRateLimiter, name="email.add"),
pretend.call(RateLimit("5 per day"), IRateLimiter, name="password.reset"),
pretend.call(RateLimit("3 per 6 hours"), IRateLimiter, name="email.verify"),
]
assert config.add_request_method.calls == [
pretend.call(accounts._user, name="user", reify=True)
Expand Down
36 changes: 21 additions & 15 deletions tests/unit/accounts/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -1340,21 +1340,23 @@ def test_register_redirect(self, db_request, monkeypatch):
)
db_request.session.record_password_timestamp = lambda ts: None
db_request.find_service = pretend.call_recorder(
lambda *args, **kwargs: pretend.stub(
csp_policy={},
merge=lambda _: {},
enabled=False,
verify_response=pretend.call_recorder(lambda _: None),
username_is_prohibited=lambda a: False,
find_userid=pretend.call_recorder(lambda _: None),
find_userid_by_email=pretend.call_recorder(lambda _: None),
update_user=lambda *args, **kwargs: None,
create_user=create_user,
add_email=add_email,
check_password=lambda pw, tags=None: False,
record_event=record_event,
get_password_timestamp=lambda uid: 0,
)
lambda svc, name=None, context=None: {
IUserService: pretend.stub(
username_is_prohibited=lambda a: False,
find_userid=pretend.call_recorder(lambda _: None),
find_userid_by_email=pretend.call_recorder(lambda _: None),
update_user=lambda *args, **kwargs: None,
create_user=create_user,
add_email=add_email,
check_password=lambda pw, tags=None: False,
record_event=record_event,
get_password_timestamp=lambda uid: 0,
),
IPasswordBreachedService: pretend.stub(
check_password=lambda pw, tags=None: False,
),
IRateLimiter: pretend.stub(hit=lambda user_id: None),
}.get(svc)
)
db_request.route_path = pretend.call_recorder(lambda name: "/")
db_request.POST.update(
Expand Down Expand Up @@ -2045,9 +2047,11 @@ def test_verify_email(
lambda token: {"action": "email-verify", "email.id": str(email.id)}
)
email_limiter = pretend.stub(clear=pretend.call_recorder(lambda a: None))
verify_limiter = pretend.stub(clear=pretend.call_recorder(lambda a: None))
services = {
"email": token_service,
"email.add": email_limiter,
"email.verify": verify_limiter,
}
db_request.find_service = pretend.call_recorder(
lambda a, name, **kwargs: services[name]
Expand All @@ -2064,6 +2068,7 @@ def test_verify_email(
assert db_request.route_path.calls == [pretend.call("manage.account")]
assert token_service.loads.calls == [pretend.call("RANDOM_KEY")]
assert email_limiter.clear.calls == [pretend.call(db_request.remote_addr)]
assert verify_limiter.clear.calls == [pretend.call(user.id)]
assert db_request.session.flash.calls == [
pretend.call(
f"Email address {email.email} verified. " + confirm_message,
Expand All @@ -2073,6 +2078,7 @@ def test_verify_email(
assert db_request.find_service.calls == [
pretend.call(ITokenService, name="email"),
pretend.call(IRateLimiter, name="email.add"),
pretend.call(IRateLimiter, name="email.verify"),
]

@pytest.mark.parametrize(
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/email/test_init.py
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ def record_event(self, user_id, tag, additional):
task = pretend.stub()
request = pretend.stub(
find_service=pretend.call_recorder(
lambda svc, context=None: {
lambda svc, context=None, name=None: {
IUserService: user_service,
IEmailSender: sender,
}.get(svc)
Expand Down
54 changes: 53 additions & 1 deletion tests/unit/manage/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -551,7 +551,12 @@ def test_reverify_email(self, monkeypatch):
)
),
session=pretend.stub(flash=pretend.call_recorder(lambda *a, **kw: None)),
find_service=lambda *a, **kw: pretend.stub(),
find_service=lambda svc, name=None, context=None: {
IRateLimiter: pretend.stub(
test=pretend.call_recorder(lambda user_id: True),
hit=pretend.call_recorder(lambda user_id: None),
)
}.get(svc, pretend.stub()),
user=pretend.stub(id=pretend.stub(), username="username", name="Name"),
remote_addr="0.0.0.0",
path="request-path",
Expand All @@ -576,6 +581,53 @@ def test_reverify_email(self, monkeypatch):
)
]

def test_reverify_email_ratelimit_exceeded(self, monkeypatch):
email = pretend.stub(
verified=False,
email="email_address",
user=pretend.stub(
record_event=pretend.call_recorder(lambda *a, **kw: None)
),
)

request = pretend.stub(
POST={"reverify_email_id": pretend.stub()},
db=pretend.stub(
query=lambda *a: pretend.stub(
filter=lambda *a: pretend.stub(one=lambda: email)
)
),
session=pretend.stub(flash=pretend.call_recorder(lambda *a, **kw: None)),
find_service=lambda svc, name=None, context=None: {
IRateLimiter: pretend.stub(
test=pretend.call_recorder(lambda user_id: False),
)
}.get(svc, pretend.stub()),
user=pretend.stub(id=pretend.stub(), username="username", name="Name"),
remote_addr="0.0.0.0",
path="request-path",
)
send_email = pretend.call_recorder(lambda *a: None)
monkeypatch.setattr(views, "send_email_verification_email", send_email)
monkeypatch.setattr(
views.ManageAccountViews, "default_response", {"_": pretend.stub()}
)
view = views.ManageAccountViews(request)

assert isinstance(view.reverify_email(), HTTPSeeOther)
assert request.session.flash.calls == [
pretend.call(
(
"Too many incomplete attempts to verify email address(es) for "
f"{request.user.username}. Complete a pending "
"verification or wait before attempting again."
),
queue="error",
)
]
assert send_email.calls == []
assert email.user.record_event.calls == []

def test_reverify_email_not_found(self, monkeypatch):
def raise_no_result():
raise NoResultFound
Expand Down
1 change: 1 addition & 0 deletions tests/unit/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@ def __init__(self):
"warehouse.account.ip_login_ratelimit_string": "10 per 5 minutes",
"warehouse.account.global_login_ratelimit_string": "1000 per 5 minutes",
"warehouse.account.email_add_ratelimit_string": "2 per day",
"warehouse.account.verify_email_ratelimit_string": "3 per 6 hours",
"warehouse.account.password_reset_ratelimit_string": "5 per day",
"warehouse.manage.oidc.user_registration_ratelimit_string": "20 per day",
"warehouse.manage.oidc.ip_registration_ratelimit_string": "20 per day",
Expand Down
8 changes: 8 additions & 0 deletions warehouse/accounts/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,3 +127,11 @@ def includeme(config):
config.register_service_factory(
RateLimit(password_reset_ratelimit_string), IRateLimiter, name="password.reset"
)
verify_email_ratelimit_string = config.registry.settings.get(
"warehouse.account.verify_email_ratelimit_string"
)
config.register_service_factory(
RateLimit(verify_email_ratelimit_string),
IRateLimiter,
name="email.verify",
)
5 changes: 5 additions & 0 deletions warehouse/accounts/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -546,6 +546,7 @@ def register(request, _form_class=RegistrationForm):
)

if request.method == "POST" and form.validate():
email_limiter = request.find_service(IRateLimiter, name="email.verify")
user = user_service.create_user(
form.username.data, form.full_name.data, form.new_password.data
)
Expand All @@ -557,6 +558,7 @@ def register(request, _form_class=RegistrationForm):
)

send_email_verification_email(request, (user, email))
email_limiter.hit(user.id)

return HTTPSeeOther(
request.route_path("index"), headers=dict(_login_user(request, user.id))
Expand Down Expand Up @@ -737,6 +739,7 @@ def _error(message):
def verify_email(request):
token_service = request.find_service(ITokenService, name="email")
email_limiter = request.find_service(IRateLimiter, name="email.add")
verify_limiter = request.find_service(IRateLimiter, name="email.verify")

def _error(message):
request.session.flash(message, queue="error")
Expand Down Expand Up @@ -779,6 +782,8 @@ def _error(message):

# Reset the email-adding rate limiter for this IP address
email_limiter.clear(request.remote_addr)
# Reset the email verification rate limiter for this User
verify_limiter.clear(request.user.id)

if not email.primary:
confirm_message = request._(
Expand Down
6 changes: 6 additions & 0 deletions warehouse/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,12 @@ def configure(settings=None):
"EMAIL_ADD_RATELIMIT_STRING",
default="2 per day",
)
maybe_set(
settings,
"warehouse.account.verify_email_ratelimit_string",
"VERIFY_EMAIL_RATELIMIT_STRING",
default="3 per 6 hours",
)
maybe_set(
settings,
"warehouse.account.password_reset_ratelimit_string",
Expand Down
Loading

0 comments on commit e2f55ab

Please sign in to comment.