From 07df335c8a595c166950fce9fb3358fd117fa511 Mon Sep 17 00:00:00 2001 From: Ee Durbin Date: Wed, 28 Sep 2022 18:08:30 -0400 Subject: [PATCH] rate limit email sends for reverifying emails --- tests/unit/accounts/test_core.py | 2 + tests/unit/accounts/test_views.py | 36 +++++++----- tests/unit/email/test_init.py | 2 +- tests/unit/manage/test_views.py | 54 ++++++++++++++++- tests/unit/test_config.py | 1 + warehouse/accounts/__init__.py | 8 +++ warehouse/accounts/views.py | 5 ++ warehouse/config.py | 6 ++ warehouse/locale/messages.pot | 96 +++++++++++++++---------------- warehouse/manage/views.py | 32 ++++++++--- 10 files changed, 168 insertions(+), 74 deletions(-) diff --git a/tests/unit/accounts/test_core.py b/tests/unit/accounts/test_core.py index 185a1277af7d..6c1a720e05a5 100644 --- a/tests/unit/accounts/test_core.py +++ b/tests/unit/accounts/test_core.py @@ -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", } ), @@ -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) diff --git a/tests/unit/accounts/test_views.py b/tests/unit/accounts/test_views.py index 60e7eaaed1d4..12f88ba33746 100644 --- a/tests/unit/accounts/test_views.py +++ b/tests/unit/accounts/test_views.py @@ -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( @@ -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] @@ -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, @@ -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( diff --git a/tests/unit/email/test_init.py b/tests/unit/email/test_init.py index 1282999b6b3b..677fc984b6d2 100644 --- a/tests/unit/email/test_init.py +++ b/tests/unit/email/test_init.py @@ -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) diff --git a/tests/unit/manage/test_views.py b/tests/unit/manage/test_views.py index e41c5066009b..f32c9571bb7d 100644 --- a/tests/unit/manage/test_views.py +++ b/tests/unit/manage/test_views.py @@ -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", @@ -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 diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index 01bf4e209f9e..6a81494280bf 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -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", diff --git a/warehouse/accounts/__init__.py b/warehouse/accounts/__init__.py index f389fce25fe3..520bc8363459 100644 --- a/warehouse/accounts/__init__.py +++ b/warehouse/accounts/__init__.py @@ -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", + ) diff --git a/warehouse/accounts/views.py b/warehouse/accounts/views.py index c4747335e25a..6d28a88416fc 100644 --- a/warehouse/accounts/views.py +++ b/warehouse/accounts/views.py @@ -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 ) @@ -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)) @@ -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") @@ -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._( diff --git a/warehouse/config.py b/warehouse/config.py index 5733f4d74820..08a7ad3f56fc 100644 --- a/warehouse/config.py +++ b/warehouse/config.py @@ -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", diff --git a/warehouse/locale/messages.pot b/warehouse/locale/messages.pot index e6c818b1eea5..533f27116d7e 100644 --- a/warehouse/locale/messages.pot +++ b/warehouse/locale/messages.pot @@ -121,7 +121,7 @@ msgstr "" msgid "Successful WebAuthn assertion" msgstr "" -#: warehouse/accounts/views.py:447 warehouse/manage/views.py:939 +#: warehouse/accounts/views.py:447 warehouse/manage/views.py:953 msgid "Recovery code accepted. The supplied code cannot be used again." msgstr "" @@ -131,126 +131,126 @@ msgid "" "#admin-intervention for details." msgstr "" -#: warehouse/accounts/views.py:651 +#: warehouse/accounts/views.py:653 msgid "Expired token: request a new password reset link" msgstr "" -#: warehouse/accounts/views.py:653 +#: warehouse/accounts/views.py:655 msgid "Invalid token: request a new password reset link" msgstr "" -#: warehouse/accounts/views.py:655 warehouse/accounts/views.py:753 -#: warehouse/accounts/views.py:850 warehouse/accounts/views.py:1017 +#: warehouse/accounts/views.py:657 warehouse/accounts/views.py:756 +#: warehouse/accounts/views.py:855 warehouse/accounts/views.py:1022 msgid "Invalid token: no token supplied" msgstr "" -#: warehouse/accounts/views.py:659 +#: warehouse/accounts/views.py:661 msgid "Invalid token: not a password reset token" msgstr "" -#: warehouse/accounts/views.py:664 +#: warehouse/accounts/views.py:666 msgid "Invalid token: user not found" msgstr "" -#: warehouse/accounts/views.py:675 +#: warehouse/accounts/views.py:677 msgid "Invalid token: user has logged in since this token was requested" msgstr "" -#: warehouse/accounts/views.py:693 +#: warehouse/accounts/views.py:695 msgid "" "Invalid token: password has already been changed since this token was " "requested" msgstr "" -#: warehouse/accounts/views.py:722 +#: warehouse/accounts/views.py:724 msgid "You have reset your password" msgstr "" -#: warehouse/accounts/views.py:749 +#: warehouse/accounts/views.py:752 msgid "Expired token: request a new email verification link" msgstr "" -#: warehouse/accounts/views.py:751 +#: warehouse/accounts/views.py:754 msgid "Invalid token: request a new email verification link" msgstr "" -#: warehouse/accounts/views.py:757 +#: warehouse/accounts/views.py:760 msgid "Invalid token: not an email verification token" msgstr "" -#: warehouse/accounts/views.py:766 +#: warehouse/accounts/views.py:769 msgid "Email not found" msgstr "" -#: warehouse/accounts/views.py:769 +#: warehouse/accounts/views.py:772 msgid "Email already verified" msgstr "" -#: warehouse/accounts/views.py:784 +#: warehouse/accounts/views.py:789 msgid "You can now set this email as your primary address" msgstr "" -#: warehouse/accounts/views.py:788 +#: warehouse/accounts/views.py:793 msgid "This is your primary address" msgstr "" -#: warehouse/accounts/views.py:793 +#: warehouse/accounts/views.py:798 msgid "Email address ${email_address} verified. ${confirm_message}." msgstr "" -#: warehouse/accounts/views.py:846 +#: warehouse/accounts/views.py:851 msgid "Expired token: request a new organization invitation" msgstr "" -#: warehouse/accounts/views.py:848 +#: warehouse/accounts/views.py:853 msgid "Invalid token: request a new organization invitation" msgstr "" -#: warehouse/accounts/views.py:854 +#: warehouse/accounts/views.py:859 msgid "Invalid token: not an organization invitation token" msgstr "" -#: warehouse/accounts/views.py:858 +#: warehouse/accounts/views.py:863 msgid "Organization invitation is not valid." msgstr "" -#: warehouse/accounts/views.py:867 +#: warehouse/accounts/views.py:872 msgid "Organization invitation no longer exists." msgstr "" -#: warehouse/accounts/views.py:916 +#: warehouse/accounts/views.py:921 msgid "Invitation for '${organization_name}' is declined." msgstr "" -#: warehouse/accounts/views.py:979 +#: warehouse/accounts/views.py:984 msgid "You are now ${role} of the '${organization_name}' organization." msgstr "" -#: warehouse/accounts/views.py:1013 +#: warehouse/accounts/views.py:1018 msgid "Expired token: request a new project role invitation" msgstr "" -#: warehouse/accounts/views.py:1015 +#: warehouse/accounts/views.py:1020 msgid "Invalid token: request a new project role invitation" msgstr "" -#: warehouse/accounts/views.py:1021 +#: warehouse/accounts/views.py:1026 msgid "Invalid token: not a collaboration invitation token" msgstr "" -#: warehouse/accounts/views.py:1025 +#: warehouse/accounts/views.py:1030 msgid "Role invitation is not valid." msgstr "" -#: warehouse/accounts/views.py:1040 +#: warehouse/accounts/views.py:1045 msgid "Role invitation no longer exists." msgstr "" -#: warehouse/accounts/views.py:1052 +#: warehouse/accounts/views.py:1057 msgid "Invitation for '${project_name}' is declined." msgstr "" -#: warehouse/accounts/views.py:1119 +#: warehouse/accounts/views.py:1124 msgid "You are now ${role} of the '${project_name}' project." msgstr "" @@ -336,73 +336,73 @@ msgstr "" msgid "Email ${email_address} added - check your email for a verification link" msgstr "" -#: warehouse/manage/views.py:887 +#: warehouse/manage/views.py:901 msgid "Recovery codes already generated" msgstr "" -#: warehouse/manage/views.py:888 +#: warehouse/manage/views.py:902 msgid "Generating new recovery codes will invalidate your existing codes." msgstr "" -#: warehouse/manage/views.py:1955 +#: warehouse/manage/views.py:1969 msgid "User '${username}' already has ${role_name} role for organization" msgstr "" -#: warehouse/manage/views.py:1966 +#: warehouse/manage/views.py:1980 msgid "" "User '${username}' does not have a verified primary email address and " "cannot be added as a ${role_name} for organization" msgstr "" -#: warehouse/manage/views.py:1980 warehouse/manage/views.py:4152 +#: warehouse/manage/views.py:1994 warehouse/manage/views.py:4166 msgid "User '${username}' already has an active invite. Please try again later." msgstr "" -#: warehouse/manage/views.py:2037 warehouse/manage/views.py:4219 +#: warehouse/manage/views.py:2051 warehouse/manage/views.py:4233 msgid "Invitation sent to '${username}'" msgstr "" -#: warehouse/manage/views.py:2077 +#: warehouse/manage/views.py:2091 msgid "Could not find organization invitation." msgstr "" -#: warehouse/manage/views.py:2091 warehouse/manage/views.py:4263 +#: warehouse/manage/views.py:2105 warehouse/manage/views.py:4277 msgid "Invitation already expired." msgstr "" -#: warehouse/manage/views.py:2124 warehouse/manage/views.py:4287 +#: warehouse/manage/views.py:2138 warehouse/manage/views.py:4301 msgid "Invitation revoked from '${username}'." msgstr "" -#: warehouse/manage/views.py:2514 +#: warehouse/manage/views.py:2528 msgid "User '${username}' is already a team member" msgstr "" -#: warehouse/manage/views.py:2934 +#: warehouse/manage/views.py:2948 msgid "" "There have been too many attempted OpenID Connect registrations. Try " "again later." msgstr "" -#: warehouse/manage/views.py:3929 +#: warehouse/manage/views.py:3943 msgid "Team '${team_name}' already has ${role_name} role for project" msgstr "" -#: warehouse/manage/views.py:4037 +#: warehouse/manage/views.py:4051 msgid "User '${username}' already has ${role_name} role for project" msgstr "" -#: warehouse/manage/views.py:4106 +#: warehouse/manage/views.py:4120 msgid "${username} is now ${role} of the '${project_name}' project." msgstr "" -#: warehouse/manage/views.py:4139 +#: warehouse/manage/views.py:4153 msgid "" "User '${username}' does not have a verified primary email address and " "cannot be added as a ${role_name} for project" msgstr "" -#: warehouse/manage/views.py:4252 +#: warehouse/manage/views.py:4266 msgid "Could not find role invitation." msgstr "" diff --git a/warehouse/manage/views.py b/warehouse/manage/views.py index 532045ec760e..286f4eb8ff6f 100644 --- a/warehouse/manage/views.py +++ b/warehouse/manage/views.py @@ -473,16 +473,30 @@ def reverify_email(self): if email.verified: self.request.session.flash("Email is already verified", queue="error") else: - send_email_verification_email(self.request, (self.request.user, email)) - email.user.record_event( - tag="account:email:reverify", - ip_address=self.request.remote_addr, - additional={"email": email.email}, - ) + verify_email_ratelimit = self.request.find_service( + IRateLimiter, name="email.verify" + ) + if verify_email_ratelimit.test(self.request.user.id): + send_email_verification_email(self.request, (self.request.user, email)) + verify_email_ratelimit.hit(self.request.user.id) + email.user.record_event( + tag="account:email:reverify", + ip_address=self.request.remote_addr, + additional={"email": email.email}, + ) - self.request.session.flash( - f"Verification email for {email.email} resent", queue="success" - ) + self.request.session.flash( + f"Verification email for {email.email} resent", queue="success" + ) + else: + self.request.session.flash( + ( + "Too many incomplete attempts to verify email address(es) for " + f"{self.request.user.username}. Complete a pending " + "verification or wait before attempting again." + ), + queue="error", + ) return HTTPSeeOther(self.request.path)