Skip to content

Commit

Permalink
feat(2fa): reset rate limiter after changing the password (#82081)
Browse files Browse the repository at this point in the history
  • Loading branch information
oioki authored and evanh committed Dec 17, 2024
1 parent 34466cd commit 036ba53
Show file tree
Hide file tree
Showing 7 changed files with 28 additions and 0 deletions.
3 changes: 3 additions & 0 deletions src/sentry/ratelimits/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,6 @@ def is_limited_with_value(

def validate(self) -> None:
raise NotImplementedError

def reset(self, key: str, project: Project | None = None, window: int | None = None) -> None:
return
4 changes: 4 additions & 0 deletions src/sentry/ratelimits/redis.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,3 +120,7 @@ def is_limited_with_value(
return False, 0, reset_time

return result > limit, result, reset_time

def reset(self, key: str, project: Project | None = None, window: int | None = None) -> None:
redis_key = self._construct_redis_key(key, project=project, window=window)
self.client.delete(redis_key)
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,5 @@ <h3>Suspicious Activity Detected</h3>
Date: {{ datetime|date:"N j, Y, P e" }}</pre></p>
<p>If you have lost your 2FA credentials, you can follow our <a href="https://sentry.zendesk.com/hc/en-us/articles/23703310917659-How-do-I-recover-my-account-if-I-lost-my-2FA-credentials">account recovery steps here</a>.</p>
<p>If these logins are not from you, we recommend you log in to your Sentry account and reset your password under <a href="{{ url }}">your account security settings</a>. On the same account security page, we also recommend you click the “Sign out of all devices” button to remove all currently logged-in sessions of your account.</p>
<p>If you are unable to log in to your Sentry account for the password reset, you can use <a href="{{ recover_url }}">Password Recovery</a>.</p>
{% endblock %}
3 changes: 3 additions & 0 deletions src/sentry/templates/sentry/emails/mfa-too-many-attempts.txt
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,6 @@ If these logins are not from you, we recommend you log in to your Sentry account
{{ url }}

On the same account security page, we also recommend you click the “Sign out of all devices” button to remove all currently logged-in sessions of your account.

If you are unable to log in to your Sentry account for the password reset, you can use Password Recovery:
{{ recover_url }}
5 changes: 5 additions & 0 deletions src/sentry/users/api/endpoints/user_password.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from sentry.api.api_publish_status import ApiPublishStatus
from sentry.api.base import control_silo_endpoint
from sentry.auth import password_validation
from sentry.ratelimits import backend as ratelimiter
from sentry.security.utils import capture_security_activity
from sentry.types.ratelimit import RateLimit, RateLimitCategory
from sentry.users.api.bases.user import UserEndpoint
Expand Down Expand Up @@ -89,4 +90,8 @@ def put(self, request: Request, user: User) -> Response:
ip_address=request.META["REMOTE_ADDR"],
send_email=True,
)

ratelimiter.reset(f"auth-2fa:user:{user.id}", window=20)
ratelimiter.reset(f"auth-2fa-long:user:{user.id}", window=60 * 60)

return Response(status=status.HTTP_204_NO_CONTENT)
5 changes: 5 additions & 0 deletions src/sentry/web/frontend/twofactor.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import logging
import time
from base64 import b64encode
from urllib.parse import urlencode

from django.http import HttpRequest, HttpResponse, HttpResponseRedirect
from django.urls import reverse
Expand Down Expand Up @@ -112,12 +113,16 @@ def validate_otp(self, otp, selected_interface, all_interfaces=None):
return interface

def send_notification_email(self, email, ip_address):
recover_uri = "{path}?{query}".format(
path=reverse("sentry-account-recover"), query=urlencode({"email": email})
)
context = {
"datetime": timezone.now(),
"email": email,
"geo": geo_by_addr(ip_address),
"ip_address": ip_address,
"url": absolute_uri(reverse("sentry-account-settings-security")),
"recover_url": absolute_uri(recover_uri),
}

subject = "Suspicious Activity Detected"
Expand Down
7 changes: 7 additions & 0 deletions tests/sentry/ratelimits/test_redis.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,3 +64,10 @@ def test_is_limited_with_value(self):
assert not limited
assert value == 1
assert reset_time == expected_reset_time + 5

def test_reset(self):
with freeze_time("2000-01-01"):
assert not self.backend.is_limited("foo", 1, self.project)
assert self.backend.is_limited("foo", 1, self.project)
self.backend.reset("foo", self.project)
assert not self.backend.is_limited("foo", 1, self.project)

0 comments on commit 036ba53

Please sign in to comment.