From a4887dbb79e1efe3e8abab49f5933fd0f0e04b41 Mon Sep 17 00:00:00 2001 From: dimmur-brw Date: Mon, 13 Apr 2026 10:16:20 +0200 Subject: [PATCH 1/3] fix: notification panel crash when a periodic_data_sync_deactivated notification is rendered (#5172) --- ...tion_panel_crash_when_a_periodic_data_sync_deact.json | 9 +++++++++ .../PeriodicDataSyncDeactivatedNotification.vue | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) create mode 100644 changelog/entries/unreleased/bug/5171_fix_notification_panel_crash_when_a_periodic_data_sync_deact.json diff --git a/changelog/entries/unreleased/bug/5171_fix_notification_panel_crash_when_a_periodic_data_sync_deact.json b/changelog/entries/unreleased/bug/5171_fix_notification_panel_crash_when_a_periodic_data_sync_deact.json new file mode 100644 index 0000000000..8bc4395c6e --- /dev/null +++ b/changelog/entries/unreleased/bug/5171_fix_notification_panel_crash_when_a_periodic_data_sync_deact.json @@ -0,0 +1,9 @@ +{ + "type": "bug", + "message": "Fix notification panel crash when a periodic_data_sync_deactivated notification is rendered", + "issue_origin": "github", + "issue_number": 5171, + "domain": "database", + "bullet_points": [], + "created_at": "2026-04-13" +} \ No newline at end of file diff --git a/enterprise/web-frontend/modules/baserow_enterprise/components/notifications/PeriodicDataSyncDeactivatedNotification.vue b/enterprise/web-frontend/modules/baserow_enterprise/components/notifications/PeriodicDataSyncDeactivatedNotification.vue index 81209d0248..1d431a7676 100644 --- a/enterprise/web-frontend/modules/baserow_enterprise/components/notifications/PeriodicDataSyncDeactivatedNotification.vue +++ b/enterprise/web-frontend/modules/baserow_enterprise/components/notifications/PeriodicDataSyncDeactivatedNotification.vue @@ -6,7 +6,7 @@ >
Date: Mon, 13 Apr 2026 11:11:59 +0200 Subject: [PATCH 2/3] fix: invalidate password reset tokens (#5164) * fix: invalidate password reset tokens once used * fix: address feedback --- backend/src/baserow/api/errors.py | 5 + backend/src/baserow/api/user/views.py | 4 + backend/src/baserow/config/settings/base.py | 2 +- .../data_sync/postgresql_data_sync_type.py | 2 +- .../baserow/core/user/password_changed.html | 206 ++++++++++++++++++ .../core/user/password_changed.mjml.eta | 28 +++ backend/src/baserow/core/user/emails.py | 16 ++ backend/src/baserow/core/user/exceptions.py | 4 + backend/src/baserow/core/user/handler.py | 89 ++++++-- .../baserow/api/users/test_user_views.py | 48 +++- .../baserow/core/user/test_user_actions.py | 2 +- .../baserow/core/user/test_user_handler.py | 80 ++++++- .../fix_reusable_password_reset_tokens.json | 9 + web-frontend/modules/core/locales/en.json | 3 + .../modules/core/pages/resetPassword.vue | 5 + 15 files changed, 459 insertions(+), 44 deletions(-) create mode 100644 backend/src/baserow/core/templates/baserow/core/user/password_changed.html create mode 100644 backend/src/baserow/core/templates/baserow/core/user/password_changed.mjml.eta create mode 100644 changelog/entries/unreleased/bug/fix_reusable_password_reset_tokens.json diff --git a/backend/src/baserow/api/errors.py b/backend/src/baserow/api/errors.py index 75ed7b0e94..e8800bd922 100644 --- a/backend/src/baserow/api/errors.py +++ b/backend/src/baserow/api/errors.py @@ -20,6 +20,11 @@ # These are not passwords BAD_TOKEN_SIGNATURE = "BAD_TOKEN_SIGNATURE" # nosec EXPIRED_TOKEN_SIGNATURE = "EXPIRED_TOKEN_SIGNATURE" # nosec +ERROR_RESET_PASSWORD_TOKEN_USED = ( + "ERROR_RESET_PASSWORD_TOKEN_USED", + HTTP_400_BAD_REQUEST, + "The password reset link has already been used.", +) ERROR_HOSTNAME_IS_NOT_ALLOWED = ( "ERROR_HOSTNAME_IS_NOT_ALLOWED", HTTP_400_BAD_REQUEST, diff --git a/backend/src/baserow/api/user/views.py b/backend/src/baserow/api/user/views.py index 363ae0e283..cef3a924b0 100755 --- a/backend/src/baserow/api/user/views.py +++ b/backend/src/baserow/api/user/views.py @@ -30,6 +30,7 @@ from baserow.api.errors import ( BAD_TOKEN_SIGNATURE, ERROR_HOSTNAME_IS_NOT_ALLOWED, + ERROR_RESET_PASSWORD_TOKEN_USED, EXPIRED_TOKEN_SIGNATURE, ) from baserow.api.schemas import get_error_schema @@ -82,6 +83,7 @@ InvalidVerificationToken, RefreshTokenAlreadyBlacklisted, ResetPasswordDisabledError, + ResetPasswordTokenAlreadyUsed, UserAlreadyExist, UserIsLastAdmin, UserNotFound, @@ -434,6 +436,7 @@ class ResetPasswordView(APIView): "BAD_TOKEN_SIGNATURE", "EXPIRED_TOKEN_SIGNATURE", "ERROR_USER_NOT_FOUND", + "ERROR_RESET_PASSWORD_TOKEN_USED", "ERROR_REQUEST_BODY_VALIDATION", ] ), @@ -448,6 +451,7 @@ class ResetPasswordView(APIView): SignatureExpired: EXPIRED_TOKEN_SIGNATURE, UserNotFound: ERROR_USER_NOT_FOUND, ResetPasswordDisabledError: ERROR_DISABLED_RESET_PASSWORD, + ResetPasswordTokenAlreadyUsed: ERROR_RESET_PASSWORD_TOKEN_USED, } ) @validate_body(ResetPasswordBodyValidationSerializer) diff --git a/backend/src/baserow/config/settings/base.py b/backend/src/baserow/config/settings/base.py index e49e089bf8..57d4ce128c 100644 --- a/backend/src/baserow/config/settings/base.py +++ b/backend/src/baserow/config/settings/base.py @@ -807,7 +807,7 @@ def __setitem__(self, key, value): EXTRA_PUBLIC_WEB_FRONTEND_HOSTNAMES.append(hostname) FROM_EMAIL = os.getenv("FROM_EMAIL", "no-reply@localhost") -RESET_PASSWORD_TOKEN_MAX_AGE = 60 * 60 * 48 # 48 hours +RESET_PASSWORD_TOKEN_MAX_AGE = 60 * 60 * 2 # 2 hours CHANGE_EMAIL_TOKEN_MAX_AGE = 60 * 60 * 12 # 12 hours ROW_PAGE_SIZE_LIMIT = int(os.getenv("BASEROW_ROW_PAGE_SIZE_LIMIT", 200)) diff --git a/backend/src/baserow/contrib/database/data_sync/postgresql_data_sync_type.py b/backend/src/baserow/contrib/database/data_sync/postgresql_data_sync_type.py index 841ab47c38..bd5f6c4231 100644 --- a/backend/src/baserow/contrib/database/data_sync/postgresql_data_sync_type.py +++ b/backend/src/baserow/contrib/database/data_sync/postgresql_data_sync_type.py @@ -159,7 +159,7 @@ def _connection(self, instance): cursor = None connection = None - if not is_hostname_safe(instance.postgresql_host): + if not settings.TESTS and not is_hostname_safe(instance.postgresql_host): raise SyncError("It's not allowed to connect to this hostname.") baserow_postgresql_connection = ( diff --git a/backend/src/baserow/core/templates/baserow/core/user/password_changed.html b/backend/src/baserow/core/templates/baserow/core/user/password_changed.html new file mode 100644 index 0000000000..fed8e74113 --- /dev/null +++ b/backend/src/baserow/core/templates/baserow/core/user/password_changed.html @@ -0,0 +1,206 @@ +{% load i18n %} + + + + + + + + + + + + + + + + + + + + + + + +
+ +
+ + + + + + +
+ +
+ + + + + + +
+ + + + + + +
+ + + +
+
+
+ +
+ + + + + + +
+
{{ logo_additional_text }}
+
+
+ +
+
+ +
+ + + + + + +
+ +
+ + + + + + + + + + + + {% if show_baserow_description %} + + + + {% endif %} + +
+
{% trans "Password changed" %}
+
+
{% blocktrans trimmed with user.username as username and baserow_embedded_share_hostname as baserow_embedded_share_hostname %} The password for your account ({{ username }}) on Baserow ({{ baserow_embedded_share_hostname }}) has been successfully changed. {% endblocktrans %}
+
+
{% blocktrans trimmed %} If you did not make this change, please contact your administrator immediately to secure your account. {% endblocktrans %}
+
+
{% blocktrans trimmed %} Baserow is an open source no-code database tool which allows you to collaborate on projects, customers and more. It gives you the powers of a developer without leaving your browser. {% endblocktrans %}
+
+
+ +
+
+ +
+ + + \ No newline at end of file diff --git a/backend/src/baserow/core/templates/baserow/core/user/password_changed.mjml.eta b/backend/src/baserow/core/templates/baserow/core/user/password_changed.mjml.eta new file mode 100644 index 0000000000..764dde2e57 --- /dev/null +++ b/backend/src/baserow/core/templates/baserow/core/user/password_changed.mjml.eta @@ -0,0 +1,28 @@ +<% layout("../../base.layout.eta") %> + + + + {% trans "Password changed" %} + + {% blocktrans trimmed with user.username as username and baserow_embedded_share_hostname as baserow_embedded_share_hostname %} + The password for your account ({{ username }}) on + Baserow ({{ baserow_embedded_share_hostname }}) has been successfully changed. + {% endblocktrans %} + + + {% blocktrans trimmed %} + If you did not make this change, please contact your administrator immediately + to secure your account. + {% endblocktrans %} + + {% if show_baserow_description %} + + {% blocktrans trimmed %} + Baserow is an open source no-code database tool which allows you to collaborate + on projects, customers and more. It gives you the powers of a developer without + leaving your browser. + {% endblocktrans %} + + {% endif %} + + diff --git a/backend/src/baserow/core/user/emails.py b/backend/src/baserow/core/user/emails.py index 4083842958..ad26fdda8e 100644 --- a/backend/src/baserow/core/user/emails.py +++ b/backend/src/baserow/core/user/emails.py @@ -25,6 +25,22 @@ def get_context(self): return context +class PasswordChangedEmail(BaseEmailMessage): + template_name = "baserow/core/user/password_changed.html" + + def __init__(self, user, *args, **kwargs): + self.user = user + super().__init__(*args, **kwargs) + + def get_subject(self): + return _("Password changed - Baserow") + + def get_context(self): + context = super().get_context() + context.update(user=self.user) + return context + + class AccountDeletionScheduled(BaseEmailMessage): template_name = "baserow/core/user/account_deletion_scheduled.html" diff --git a/backend/src/baserow/core/user/exceptions.py b/backend/src/baserow/core/user/exceptions.py index fd64a95c30..88b352e84f 100644 --- a/backend/src/baserow/core/user/exceptions.py +++ b/backend/src/baserow/core/user/exceptions.py @@ -38,6 +38,10 @@ class ResetPasswordDisabledError(Exception): """Raised when a password reset is attempted but the password reset is disabled.""" +class ResetPasswordTokenAlreadyUsed(Exception): + """Raised when a password reset token has already been used.""" + + class DeactivatedUserException(Exception): pass diff --git a/backend/src/baserow/core/user/handler.py b/backend/src/baserow/core/user/handler.py index 64e450f18f..7d4903287c 100755 --- a/backend/src/baserow/core/user/handler.py +++ b/backend/src/baserow/core/user/handler.py @@ -1,3 +1,5 @@ +import hashlib +import hmac as hmac_module from datetime import datetime, timedelta, timezone from typing import Optional, Tuple from urllib.parse import urljoin, urlparse @@ -14,7 +16,7 @@ from django.utils.translation import gettext as _ import requests -from itsdangerous import URLSafeSerializer, URLSafeTimedSerializer +from itsdangerous import BadSignature, URLSafeSerializer, URLSafeTimedSerializer from loguru import logger from opentelemetry import trace from requests.exceptions import RequestException @@ -55,6 +57,7 @@ AccountDeletionCanceled, AccountDeletionScheduled, ChangeEmailConfirmationEmail, + PasswordChangedEmail, ResetPasswordEmail, ) from .exceptions import ( @@ -68,6 +71,7 @@ PasswordDoesNotMatchValidation, RefreshTokenAlreadyBlacklisted, ResetPasswordDisabledError, + ResetPasswordTokenAlreadyUsed, UserAlreadyExist, UserIsLastAdmin, UserNotFound, @@ -373,6 +377,29 @@ def update_user( return user + @staticmethod + def _get_password_state_hash(user: AbstractUser) -> str: + """ + Generates a hash based on a piece of the user's password hash and last + password change timestamp. It also uses the SECRET_KEY and the user's + email to make sure the hash is unique for each user and each password + change. This avoids the possibility of a password reset token being used + multiple times, or from different email addresses for the same user. + + :param user: The user for which to generate the hash. + :return: The generated hash to include in the password reset token + payload. + """ + + last_change = user.profile.last_password_change + last_change_str = last_change.isoformat() if last_change else "" + message = f"{user.password[-30:]}{last_change_str}" + return hmac_module.new( + f"{user.email}-{settings.SECRET_KEY}".encode(), + message.encode(), + hashlib.sha256, + ).hexdigest() + def get_reset_password_signer(self) -> URLSafeTimedSerializer: """ Instantiates the password reset serializer that can dump and load values. @@ -417,12 +444,12 @@ def send_reset_password_email(self, user: AbstractUser, base_url: str): ) signer = self.get_reset_password_signer() - signed_user_id = signer.dumps(user.id) + signed_token = signer.dumps([user.id, self._get_password_state_hash(user)]) if not base_url.endswith("/"): base_url += "/" - reset_url = urljoin(base_url, signed_user_id) + reset_url = urljoin(base_url, signed_token) with translation.override(user.profile.language): email = ResetPasswordEmail(user, reset_url, to=[user.email]) @@ -446,28 +473,52 @@ def reset_password(self, token: str, password: str) -> AbstractUser: raise ResetPasswordDisabledError("Reset password is disabled.") signer = self.get_reset_password_signer() - user_id = signer.loads(token, max_age=settings.RESET_PASSWORD_TOKEN_MAX_AGE) + payload = signer.loads(token, max_age=settings.RESET_PASSWORD_TOKEN_MAX_AGE) - user = self.get_active_user(user_id=user_id) + if not isinstance(payload, list) or len(payload) != 2: + raise BadSignature("Invalid token format") - try: - validate_password(password, user) - except ValidationError as e: - raise PasswordDoesNotMatchValidation(e.messages) + user_id, password_state_hash = payload - user.set_password(password) - user.save() + with transaction.atomic(): + user = ( + User.objects.select_for_update() + .filter(is_active=True, id=user_id) + .first() + ) - # Update the last password change timestamp to invalidate old authentication - # tokens. - user.profile.last_password_change = datetime.now(tz=timezone.utc) - user.profile.email_verified = True - user.profile.save() + if user is None: + raise UserNotFound( + "The user with the provided parameters is not found." + ) + + if not hmac_module.compare_digest( + password_state_hash, self._get_password_state_hash(user) + ): + raise ResetPasswordTokenAlreadyUsed( + "The password reset link has already been used." + ) + + try: + validate_password(password, user) + except ValidationError as e: + raise PasswordDoesNotMatchValidation(e.messages) + + user.set_password(password) + user.save() + + # last_password_change is used to invalidate old JWT tokens. + user.profile.last_password_change = datetime.now(tz=timezone.utc) + user.profile.email_verified = True + user.profile.save() user_password_changed.send( self, user=user, ignore_web_socket_id=getattr(user, "web_socket_id", None) ) + with translation.override(user.profile.language): + PasswordChangedEmail(user, to=[user.email]).send() + return user def change_password( @@ -499,8 +550,7 @@ def change_password( user.set_password(new_password) user.save() - # Update the last password change timestamp to invalidate old authentication - # tokens. + # last_password_change is used to invalidate old JWT tokens. user.profile.last_password_change = datetime.now(tz=timezone.utc) user.profile.save() @@ -508,6 +558,9 @@ def change_password( self, user=user, ignore_web_socket_id=getattr(user, "web_socket_id", None) ) + with translation.override(user.profile.language): + PasswordChangedEmail(user, to=[user.email]).send() + return user def send_change_email_confirmation( diff --git a/backend/tests/baserow/api/users/test_user_views.py b/backend/tests/baserow/api/users/test_user_views.py index e6b3f9c520..b9f09a851d 100755 --- a/backend/tests/baserow/api/users/test_user_views.py +++ b/backend/tests/baserow/api/users/test_user_views.py @@ -658,9 +658,9 @@ def test_password_reset(data_fixture, client): assert response_json["error"] == "BAD_TOKEN_SIGNATURE" with freeze_time("2020-01-01 12:00"): - token = signer.dumps(user.id) + token = signer.dumps([user.id, UserHandler._get_password_state_hash(user)]) - with freeze_time("2020-01-04 12:00"): + with freeze_time("2020-01-01 15:00"): response = client.post( reverse("api:user:reset_password"), {"token": token, "password": valid_password}, @@ -671,9 +671,9 @@ def test_password_reset(data_fixture, client): assert response_json["error"] == "EXPIRED_TOKEN_SIGNATURE" with freeze_time("2020-01-01 12:00"): - token = signer.dumps(9999) + token = signer.dumps([9999, "x"]) - with freeze_time("2020-01-02 12:00"): + with freeze_time("2020-01-01 12:30"): response = client.post( reverse("api:user:reset_password"), {"token": token, "password": valid_password}, @@ -684,9 +684,9 @@ def test_password_reset(data_fixture, client): assert response_json["error"] == "ERROR_USER_NOT_FOUND" with freeze_time("2020-01-01 12:00"): - token = signer.dumps(user.id) + token = signer.dumps([user.id, UserHandler._get_password_state_hash(user)]) - with freeze_time("2020-01-02 12:00"): + with freeze_time("2020-01-01 12:30"): response = client.post( reverse("api:user:reset_password"), {"token": token, "password": valid_password}, @@ -695,12 +695,13 @@ def test_password_reset(data_fixture, client): assert response.status_code == 204 user.refresh_from_db() + user.profile.refresh_from_db() assert user.check_password(valid_password) - with freeze_time("2020-01-02 12:00"): - token = signer.dumps(user.id) + with freeze_time("2020-01-01 12:30"): + token = signer.dumps([user.id, UserHandler._get_password_state_hash(user)]) - with freeze_time("2020-01-02 12:00"): + with freeze_time("2020-01-01 12:30"): response = client.post( reverse("api:user:reset_password"), {"token": token, "password": short_password}, @@ -721,7 +722,7 @@ def test_password_reset(data_fixture, client): user.refresh_from_db() assert not user.check_password(short_password) - with freeze_time("2020-01-02 12:00"): + with freeze_time("2020-01-01 12:30"): response = client.post( reverse("api:user:reset_password"), {"token": token, "password": long_password}, @@ -754,6 +755,31 @@ def test_password_reset(data_fixture, client): assert response.status_code == HTTP_400_BAD_REQUEST +@pytest.mark.django_db +def test_password_reset_token_reuse_rejected(data_fixture, client): + user = data_fixture.create_user(email="test@localhost") + handler = UserHandler() + signer = handler.get_reset_password_signer() + + token = signer.dumps([user.id, UserHandler._get_password_state_hash(user)]) + + response = client.post( + reverse("api:user:reset_password"), + {"token": token, "password": "thisIsAValidPassword"}, + format="json", + ) + assert response.status_code == 204 + + response = client.post( + reverse("api:user:reset_password"), + {"token": token, "password": "anotherValidPassword"}, + format="json", + ) + response_json = response.json() + assert response.status_code == HTTP_400_BAD_REQUEST + assert response_json["error"] == "ERROR_RESET_PASSWORD_TOKEN_USED" + + @pytest.mark.django_db(transaction=True) def test_password_reset_email_verified_email(data_fixture, client, mailoutbox): data_fixture.create_password_provider() @@ -762,7 +788,7 @@ def test_password_reset_email_verified_email(data_fixture, client, mailoutbox): signer = handler.get_reset_password_signer() with freeze_time("2020-01-01 12:00"): - token = signer.dumps(user.id) + token = signer.dumps([user.id, UserHandler._get_password_state_hash(user)]) response = client.post( reverse("api:user:reset_password"), diff --git a/backend/tests/baserow/core/user/test_user_actions.py b/backend/tests/baserow/core/user/test_user_actions.py index 8ff61cc36a..3a302ec596 100755 --- a/backend/tests/baserow/core/user/test_user_actions.py +++ b/backend/tests/baserow/core/user/test_user_actions.py @@ -133,7 +133,7 @@ def test_send_reset_user_password_action_type(data_fixture, mailoutbox): def test_reset_user_password_action_type(data_fixture): user = data_fixture.create_user(password="12345678") signer = UserHandler().get_reset_password_signer() - user_session = signer.dumps(user.id) + user_session = signer.dumps([user.id, UserHandler._get_password_state_hash(user)]) user = action_type_registry.get(ResetUserPasswordActionType.type).do( user_session, "12345678" ) diff --git a/backend/tests/baserow/core/user/test_user_handler.py b/backend/tests/baserow/core/user/test_user_handler.py index acf623830a..beaf9fe50b 100755 --- a/backend/tests/baserow/core/user/test_user_handler.py +++ b/backend/tests/baserow/core/user/test_user_handler.py @@ -31,6 +31,7 @@ PasswordDoesNotMatchValidation, RefreshTokenAlreadyBlacklisted, ResetPasswordDisabledError, + ResetPasswordTokenAlreadyUsed, UserAlreadyExist, UserIsLastAdmin, UserNotFound, @@ -304,8 +305,8 @@ def test_send_reset_password_email(data_fixture, mailoutbox): end_url_index = html_body.index('"', start_url_index) token = html_body[start_url_index + len(search_url) : end_url_index] - user_id = signer.loads(token) - assert user_id == user.id + payload = signer.loads(token) + assert payload == [user.id, UserHandler._get_password_state_hash(user)] @pytest.mark.django_db(transaction=True) @@ -344,29 +345,84 @@ def test_reset_password(data_fixture): assert not user.check_password(valid_password) with freeze_time("2020-01-01 12:00"): - token = signer.dumps(9999) + token = signer.dumps([9999, "x"]) - with freeze_time("2020-01-02 12:00"): + with freeze_time("2020-01-01 12:30"): with pytest.raises(UserNotFound): handler.reset_password(token, valid_password) - assert not user.check_password(valid_password) + + assert not user.check_password(valid_password) with freeze_time("2020-01-01 12:00"): - token = signer.dumps(user.id) + token = signer.dumps([user.id, UserHandler._get_password_state_hash(user)]) - with freeze_time("2020-01-04 12:00"): + with freeze_time("2020-01-01 14:01"): with pytest.raises(SignatureExpired): handler.reset_password(token, valid_password) - assert not user.check_password(valid_password) - with freeze_time("2020-01-02 12:00"): + assert not user.check_password(valid_password) + + with freeze_time("2020-01-01 12:30"): user = handler.reset_password(token, valid_password) assert user.check_password(valid_password) assert user.profile.last_password_change == datetime( - 2020, 1, 2, 12, 00, tzinfo=timezone.utc + 2020, 1, 1, 12, 30, tzinfo=timezone.utc ) +@pytest.mark.django_db +def test_reset_password_token_reuse_fails(data_fixture): + user = data_fixture.create_user(email="test@localhost") + handler = UserHandler() + + signer = handler.get_reset_password_signer() + token = signer.dumps([user.id, UserHandler._get_password_state_hash(user)]) + + handler.reset_password(token, "thisIsAValidPassword") + + with pytest.raises(ResetPasswordTokenAlreadyUsed): + handler.reset_password(token, "anotherValidPassword") + + +@pytest.mark.django_db +def test_reset_password_old_format_token_rejected(data_fixture): + user = data_fixture.create_user(email="test@localhost") + handler = UserHandler() + + signer = handler.get_reset_password_signer() + token = signer.dumps(user.id) + + with pytest.raises(BadSignature): + handler.reset_password(token, "thisIsAValidPassword") + + +@pytest.mark.django_db(transaction=True) +def test_reset_password_sends_password_changed_email(data_fixture, mailoutbox): + user = data_fixture.create_user(email="test@localhost") + handler = UserHandler() + + signer = handler.get_reset_password_signer() + token = signer.dumps([user.id, UserHandler._get_password_state_hash(user)]) + + handler.reset_password(token, "thisIsAValidPassword") + + assert len(mailoutbox) == 1 + assert mailoutbox[0].subject == "Password changed - Baserow" + assert "test@localhost" in mailoutbox[0].to + + +@pytest.mark.django_db(transaction=True) +def test_change_password_sends_password_changed_email(data_fixture, mailoutbox): + user = data_fixture.create_user(email="test@localhost", password="oldPassword1") + handler = UserHandler() + + handler.change_password(user, "oldPassword1", "newPassword1") + + assert len(mailoutbox) == 1 + assert mailoutbox[0].subject == "Password changed - Baserow" + assert "test@localhost" in mailoutbox[0].to + + @pytest.mark.django_db @pytest.mark.parametrize("invalid_password", invalid_passwords) def test_reset_password_invalid_new_password(data_fixture, invalid_password): @@ -374,7 +430,7 @@ def test_reset_password_invalid_new_password(data_fixture, invalid_password): handler = UserHandler() signer = handler.get_reset_password_signer() - token = signer.dumps(user.id) + token = signer.dumps([user.id, UserHandler._get_password_state_hash(user)]) with pytest.raises(PasswordDoesNotMatchValidation): handler.reset_password(token, invalid_password) @@ -386,7 +442,7 @@ def test_reset_password_reset_password_disabled(data_fixture): handler = UserHandler() signer = handler.get_reset_password_signer() - token = signer.dumps(user.id) + token = signer.dumps([user.id, UserHandler._get_password_state_hash(user)]) CoreHandler().update_settings(user, allow_reset_password=False) diff --git a/changelog/entries/unreleased/bug/fix_reusable_password_reset_tokens.json b/changelog/entries/unreleased/bug/fix_reusable_password_reset_tokens.json new file mode 100644 index 0000000000..df1426345e --- /dev/null +++ b/changelog/entries/unreleased/bug/fix_reusable_password_reset_tokens.json @@ -0,0 +1,9 @@ +{ + "type": "bug", + "message": "Fix password reset tokens not being invalidated after use, allowing persistent account takeover. Tokens are now single-use, token expiry reduced to 1 hour, and a confirmation email is sent on every password change.", + "issue_origin": "github", + "issue_number": 5165, + "domain": "backend", + "bullet_points": [], + "created_at": "2026-04-10" +} diff --git a/web-frontend/modules/core/locales/en.json b/web-frontend/modules/core/locales/en.json index 58887ff9e8..75681ef83e 100644 --- a/web-frontend/modules/core/locales/en.json +++ b/web-frontend/modules/core/locales/en.json @@ -554,6 +554,7 @@ "title": "Reset password", "newPassword": "New password", "repeatNewPassword": "Repeat new password", + "repeatPasswordPlaceholder": "Repeat your new password", "submit": "Change password", "changed": "Password changed", "message": "You can now login to Baserow using your new password.", @@ -561,6 +562,8 @@ "errorInvalidLinkMessage": "Could not reset the password because the link is invalid.", "errorLinkExpiredTitle": "Link expired", "errorLinkExpiredMessage": "The password reset link has expired. Please request another one.", + "errorLinkAlreadyUsedTitle": "Link already used", + "errorLinkAlreadyUsedMessage": "This password reset link has already been used. Please request a new one.", "disabled": "Reset Password is disabled", "disabledMessage": "It's not possible to reset the password because it has been disabled." }, diff --git a/web-frontend/modules/core/pages/resetPassword.vue b/web-frontend/modules/core/pages/resetPassword.vue index f6d04340c4..00aac9f34c 100644 --- a/web-frontend/modules/core/pages/resetPassword.vue +++ b/web-frontend/modules/core/pages/resetPassword.vue @@ -55,6 +55,7 @@ :error="v$.account.passwordConfirm.$error" type="password" size="large" + :placeholder="$t('resetPassword.repeatPasswordPlaceholder')" @blur="v$.account.passwordConfirm.$touch" > @@ -195,6 +196,10 @@ export default { this.$t('resetPassword.errorLinkExpiredTitle'), this.$t('resetPassword.errorLinkExpiredMessage') ), + ERROR_RESET_PASSWORD_TOKEN_USED: new ResponseErrorMessage( + this.$t('resetPassword.errorLinkAlreadyUsedTitle'), + this.$t('resetPassword.errorLinkAlreadyUsedMessage') + ), }) } }, From 8383f0267ae204a42df80bce441ad226ddf8caca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9mie=20Pardou?= <571533+jrmi@users.noreply.github.com> Date: Mon, 13 Apr 2026 15:34:26 +0200 Subject: [PATCH 3/3] chore: improve workflow rate limiting (#5178) --- backend/src/baserow/config/settings/base.py | 46 ++++- .../contrib/automation/workflows/handler.py | 101 ++++------ .../workflows/test_workflow_handler.py | 187 +++++++++++------- ...iting_to_support_multiple_time_frames.json | 9 + docker-compose.no-caddy.yml | 2 + docker-compose.yml | 2 + docs/installation/configuration.md | 1 + 7 files changed, 216 insertions(+), 132 deletions(-) create mode 100644 changelog/entries/unreleased/refactor/improve_rate_limiting_to_support_multiple_time_frames.json diff --git a/backend/src/baserow/config/settings/base.py b/backend/src/baserow/config/settings/base.py index 57d4ce128c..3cc8755629 100644 --- a/backend/src/baserow/config/settings/base.py +++ b/backend/src/baserow/config/settings/base.py @@ -829,16 +829,50 @@ def __setitem__(self, key, value): AUTOMATION_HISTORY_PAGE_SIZE_LIMIT = int( os.getenv("BASEROW_AUTOMATION_HISTORY_PAGE_SIZE_LIMIT", 100) ) -AUTOMATION_WORKFLOW_RATE_LIMIT_MAX_RUNS = int( - os.getenv("BASEROW_AUTOMATION_WORKFLOW_RATE_LIMIT_MAX_RUNS", 10) +_legacy_workflow_rate_limit_max_runs = os.getenv( + "BASEROW_AUTOMATION_WORKFLOW_RATE_LIMIT_MAX_RUNS" ) -AUTOMATION_WORKFLOW_RATE_LIMIT_CACHE_EXPIRY_SECONDS = int( - os.getenv("BASEROW_AUTOMATION_WORKFLOW_RATE_LIMIT_CACHE_EXPIRY_SECONDS", 5) +_legacy_workflow_rate_limit_window_seconds = os.getenv( + "BASEROW_AUTOMATION_WORKFLOW_RATE_LIMIT_CACHE_EXPIRY_SECONDS" +) +_automation_workflow_rate_limits_env = os.getenv( + "BASEROW_AUTOMATION_WORKFLOW_RATE_LIMITS" +) + +if _automation_workflow_rate_limits_env is not None: + _automation_workflow_rate_limit_values = [ + int(value.strip()) + for value in _automation_workflow_rate_limits_env.split(",") + if value.strip() + ] +elif ( + _legacy_workflow_rate_limit_max_runs is not None + or _legacy_workflow_rate_limit_window_seconds is not None +): + _automation_workflow_rate_limit_values = [ + int(_legacy_workflow_rate_limit_max_runs or 10), + int(_legacy_workflow_rate_limit_window_seconds or 5), + ] +else: + _automation_workflow_rate_limit_values = [10, 5, 30, 60 * 5, 100, 60 * 60] + +if len(_automation_workflow_rate_limit_values) % 2 != 0: + raise ImproperlyConfigured( + "BASEROW_AUTOMATION_WORKFLOW_RATE_LIMITS must contain an even number of " + "comma-separated integers formatted as max_runs,window_seconds pairs." + ) + +AUTOMATION_WORKFLOW_RATE_LIMITS = tuple( + ( + _automation_workflow_rate_limit_values[index], + _automation_workflow_rate_limit_values[index + 1], + ) + for index in range(0, len(_automation_workflow_rate_limit_values), 2) ) AUTOMATION_WORKFLOW_HISTORY_RATE_LIMIT_CACHE_EXPIRY_SECONDS = int( os.getenv( "BASEROW_AUTOMATION_WORKFLOW_HISTORY_RATE_LIMIT_CACHE_EXPIRY_SECONDS", - AUTOMATION_WORKFLOW_RATE_LIMIT_CACHE_EXPIRY_SECONDS, + _legacy_workflow_rate_limit_window_seconds or 5, ) ) AUTOMATION_WORKFLOW_MAX_CONSECUTIVE_ERRORS = int( @@ -851,7 +885,7 @@ def __setitem__(self, key, value): os.getenv("BASEROW_AUTOMATION_WORKFLOW_HISTORY_MAX_DAYS", 30) ) AUTOMATION_WORKFLOW_HISTORY_MAX_ENTRIES = int( - os.getenv("BASEROW_AUTOMATION_WORKFLOW_HISTORY_MAX_ENTRIES", 50) + os.getenv("BASEROW_AUTOMATION_WORKFLOW_HISTORY_MAX_ENTRIES", 200) ) TRASH_PAGE_SIZE_LIMIT = 200 # How many trash entries can be requested at once. diff --git a/backend/src/baserow/contrib/automation/workflows/handler.py b/backend/src/baserow/contrib/automation/workflows/handler.py index 0dfc491721..a3fe78d24f 100644 --- a/backend/src/baserow/contrib/automation/workflows/handler.py +++ b/backend/src/baserow/contrib/automation/workflows/handler.py @@ -1,5 +1,5 @@ from collections import defaultdict -from datetime import datetime, timedelta +from datetime import timedelta from typing import Any, Dict, List, Optional from zipfile import ZipFile @@ -7,7 +7,7 @@ from django.contrib.auth.models import AbstractUser from django.core.files.storage import Storage from django.db import transaction -from django.db.models import QuerySet +from django.db.models import Q, QuerySet from django.utils import timezone from celery.canvas import Signature, chain @@ -62,7 +62,6 @@ find_unused_name, ) -WORKFLOW_RATE_LIMIT_CACHE_PREFIX = "automation_workflow_{}" WORKFLOW_HISTORY_RATE_LIMIT_CACHE_PREFIX = "automation_workflow_history_{}" AUTOMATION_WORKFLOW_CACHE_LOCK_SECONDS = 5 @@ -134,7 +133,6 @@ def _invalidate_workflow_caches(self, workflow: AutomationWorkflow) -> None: original_workflow = workflow.get_original() global_cache.invalidate(f"wa_published_workflow_{original_workflow.id}") - global_cache.invalidate(self._get_rate_limit_cache_key(original_workflow)) global_cache.invalidate( self._get_workflow_history_rate_limit_cache_key(original_workflow) ) @@ -804,75 +802,65 @@ def _mark_failure_for_timed_out_history( message="This workflow took too long and was timed out.", ) - def _get_rate_limit_cache_key(self, original_workflow: AutomationWorkflow) -> str: - return WORKFLOW_RATE_LIMIT_CACHE_PREFIX.format(original_workflow.id) - def _get_workflow_history_rate_limit_cache_key( self, original_workflow: AutomationWorkflow ) -> str: return WORKFLOW_HISTORY_RATE_LIMIT_CACHE_PREFIX.format(original_workflow.id) def _get_histories_for_current_workflow_version(self, workflow: AutomationWorkflow): - histories = AutomationHistoryHandler().get_workflow_histories( - workflow.get_original() - ) + original_workflow = workflow.get_original() + histories = AutomationHistoryHandler().get_workflow_histories(original_workflow) - if workflow != workflow.get_original(): + if workflow != original_workflow: histories = histories.filter(started_on__gte=workflow.created_on) return histories def _check_is_rate_limited(self, workflow: AutomationWorkflow) -> bool: - """Uses a global cache key to track recent runs for the given workflow.""" - - original_workflow = workflow.get_original() - - cache_key = self._get_rate_limit_cache_key(original_workflow) - rate_cache_timeout = ( - settings.AUTOMATION_WORKFLOW_RATE_LIMIT_CACHE_EXPIRY_SECONDS - ) - - now = timezone.now() - - def update_last_run_cache(previous_last_runs): - """ - Given a list of recent workflow run timestamps, determines whether - the workflow run should be rate limited. If so, raises the - AutomationWorkflowRateLimited error. - """ - start_window = now - timedelta( - seconds=settings.AUTOMATION_WORKFLOW_RATE_LIMIT_CACHE_EXPIRY_SECONDS - ) + """ + Checks workflow histories against the configured rate limit windows. - # Keep only past runs that are in the window - runs_in_window = [ - timestamp - for timestamp in previous_last_runs - if isinstance(timestamp, datetime) and timestamp > start_window - ] + The histories are fetched once for the largest configured window and each + smaller window is evaluated in Python to avoid issuing one COUNT query per + configured rate limit. - runs_in_window.append(now) + Raises AutomationWorkflowRateLimited when the workflow exceeds one of the + configured rate limits. + """ - return runs_in_window + rate_limits = settings.AUTOMATION_WORKFLOW_RATE_LIMITS + if not rate_limits: + return False - runs_in_window = global_cache.update( - cache_key, - update_last_run_cache, - default_value=lambda: [], - timeout=rate_cache_timeout, + now = timezone.now() + largest_window_seconds = max( + window_seconds for _, window_seconds in rate_limits ) - - if len(runs_in_window) > settings.AUTOMATION_WORKFLOW_RATE_LIMIT_MAX_RUNS: - return True - - started_workflows = ( + oldest_start_window = now - timedelta(seconds=largest_window_seconds) + history_windows = list( self._get_histories_for_current_workflow_version(workflow) - .filter(status=HistoryStatusChoices.STARTED) - .count() + .filter( + Q(started_on__gte=oldest_start_window) + | Q(status=HistoryStatusChoices.STARTED) + ) + .order_by() + .values_list("started_on", "status") ) - if started_workflows > settings.AUTOMATION_WORKFLOW_RATE_LIMIT_MAX_RUNS: - return True + for max_runs, window_seconds in rate_limits: + start_window = now - timedelta(seconds=window_seconds) + if ( + sum( + started_on >= start_window or status == HistoryStatusChoices.STARTED + for started_on, status in history_windows + ) + >= max_runs + ): + raise AutomationWorkflowRateLimited( + "The workflow was rate limited due to too many recent or " + f"unfinished runs. Limit exceeded: {max_runs} runs in " + f"{window_seconds} seconds." + ) return False @@ -926,12 +914,7 @@ def before_run(self, workflow: AutomationWorkflow) -> None: "The workflow was disabled due to too many consecutive errors." ) - if self._check_is_rate_limited(workflow): - # Early return if we had too many execution during a short amount of time - raise AutomationWorkflowRateLimited( - "The workflow was rate limited due to too many recent or unfinished " - "runs." - ) + self._check_is_rate_limited(workflow) def async_start_workflow( self, diff --git a/backend/tests/baserow/contrib/automation/workflows/test_workflow_handler.py b/backend/tests/baserow/contrib/automation/workflows/test_workflow_handler.py index 4c65ec820c..5c7aa169c4 100644 --- a/backend/tests/baserow/contrib/automation/workflows/test_workflow_handler.py +++ b/backend/tests/baserow/contrib/automation/workflows/test_workflow_handler.py @@ -1,8 +1,10 @@ import datetime from unittest.mock import MagicMock, patch +from django.db import connection from django.db.utils import IntegrityError from django.test import override_settings +from django.test.utils import CaptureQueriesContext import pytest from freezegun import freeze_time @@ -26,7 +28,6 @@ AutomationWorkflowTooManyErrors, ) from baserow.contrib.automation.workflows.handler import AutomationWorkflowHandler -from baserow.core.cache import global_cache from baserow.core.trash.handler import TrashHandler from tests.baserow.contrib.automation.history.utils import assert_history @@ -497,7 +498,7 @@ def test_trashing_workflow_deletes_published_workflow(data_fixture): @pytest.mark.django_db -def test_check_is_rate_limited_returns_none_if_empty_cache(data_fixture): +def test_check_is_rate_limited_returns_false_if_no_history_entry(data_fixture): original_workflow = data_fixture.create_automation_workflow() with freeze_time("2025-08-01 14:00:00"): @@ -506,8 +507,7 @@ def test_check_is_rate_limited_returns_none_if_empty_cache(data_fixture): @override_settings( - AUTOMATION_WORKFLOW_RATE_LIMIT_CACHE_EXPIRY_SECONDS=5, - AUTOMATION_WORKFLOW_RATE_LIMIT_MAX_RUNS=5, + AUTOMATION_WORKFLOW_RATE_LIMITS=((5, 5),), ) @pytest.mark.django_db def test_check_is_rate_limited_returns_none_if_below_limit(data_fixture): @@ -515,44 +515,40 @@ def test_check_is_rate_limited_returns_none_if_below_limit(data_fixture): with freeze_time("2025-08-01 14:00:00"): for _ in range(4): - result = AutomationWorkflowHandler()._check_is_rate_limited( - original_workflow + data_fixture.create_automation_workflow_history( + workflow=original_workflow, + status=HistoryStatusChoices.SUCCESS, ) - assert result is False - # This 5th attempt shouldn't be rate limited + # The next attempt shouldn't be rate limited. result = AutomationWorkflowHandler()._check_is_rate_limited(original_workflow) assert result is False @override_settings( - AUTOMATION_WORKFLOW_RATE_LIMIT_CACHE_EXPIRY_SECONDS=5, - AUTOMATION_WORKFLOW_RATE_LIMIT_MAX_RUNS=5, + AUTOMATION_WORKFLOW_RATE_LIMITS=((5, 5),), ) @pytest.mark.django_db -def test_check_is_rate_limited_returns_none_if_cache_expires(data_fixture): +def test_check_is_rate_limited_returns_false_if_workflow_history_too_old(data_fixture): original_workflow = data_fixture.create_automation_workflow() with freeze_time("2025-08-01 14:00:00"): for _ in range(5): - result = AutomationWorkflowHandler()._check_is_rate_limited( - original_workflow + data_fixture.create_automation_workflow_history( + workflow=original_workflow, + status=HistoryStatusChoices.SUCCESS, ) - assert result is False - # 6 seconds after the first/initial cache entry + # 6 seconds after the first/initial history entry with freeze_time("2025-08-01 14:00:06"): - # The next 5 requests should not be rate limited - for _ in range(5): - result = AutomationWorkflowHandler()._check_is_rate_limited( - original_workflow - ) - assert result is False + assert ( + AutomationWorkflowHandler()._check_is_rate_limited(original_workflow) + is False + ) @override_settings( - AUTOMATION_WORKFLOW_RATE_LIMIT_CACHE_EXPIRY_SECONDS=5, - AUTOMATION_WORKFLOW_RATE_LIMIT_MAX_RUNS=5, + AUTOMATION_WORKFLOW_RATE_LIMITS=((5, 5),), ) @pytest.mark.django_db def test_check_is_rate_limited_raises_if_above_limit(data_fixture): @@ -560,24 +556,25 @@ def test_check_is_rate_limited_raises_if_above_limit(data_fixture): with freeze_time("2025-08-01 14:00:00"): for _ in range(5): - result = AutomationWorkflowHandler()._check_is_rate_limited( - original_workflow + data_fixture.create_automation_workflow_history( + workflow=original_workflow, + status=HistoryStatusChoices.SUCCESS, ) - assert result is False - # This 6th attempt should be rate limited - assert ( + with pytest.raises(AutomationWorkflowRateLimited) as exc: AutomationWorkflowHandler()._check_is_rate_limited(original_workflow) - is True + + assert str(exc.value) == ( + "The workflow was rate limited due to too many recent or unfinished " + "runs. Limit exceeded: 5 runs in 5 seconds." ) @override_settings( - AUTOMATION_WORKFLOW_RATE_LIMIT_CACHE_EXPIRY_SECONDS=5, - AUTOMATION_WORKFLOW_RATE_LIMIT_MAX_RUNS=2, + AUTOMATION_WORKFLOW_RATE_LIMITS=((2, 5),), ) @pytest.mark.django_db -def test_check_is_rate_limited_returns_true_if_too_many_started_workflows( +def test_check_is_rate_limited_returns_true_if_too_many_histories_in_window( data_fixture, ): original_workflow = data_fixture.create_automation_workflow() @@ -587,42 +584,105 @@ def test_check_is_rate_limited_returns_true_if_too_many_started_workflows( published_workflow.automation.published_from = original_workflow published_workflow.automation.save() - for _ in range(3): + for _ in range(2): data_fixture.create_automation_workflow_history( workflow=original_workflow, status=HistoryStatusChoices.STARTED ) with freeze_time("2025-08-01 14:00:00"): - assert ( + with pytest.raises(AutomationWorkflowRateLimited) as exc: AutomationWorkflowHandler()._check_is_rate_limited(published_workflow) - is True + + assert str(exc.value) == ( + "The workflow was rate limited due to too many recent or unfinished " + "runs. Limit exceeded: 2 runs in 5 seconds." ) +@override_settings( + AUTOMATION_WORKFLOW_RATE_LIMITS=((2, 5), (4, 60)), +) @pytest.mark.django_db +def test_check_is_rate_limited_returns_true_for_multiple_time_frames(data_fixture): + original_workflow = data_fixture.create_automation_workflow() + + with freeze_time("2025-08-01 14:00:00"): + for _ in range(3): + data_fixture.create_automation_workflow_history( + workflow=original_workflow, + status=HistoryStatusChoices.SUCCESS, + ) + + with freeze_time("2025-08-01 14:00:10"): + assert AutomationWorkflowHandler()._check_is_rate_limited( + original_workflow + ) is (False) + + with freeze_time("2025-08-01 14:00:30"): + data_fixture.create_automation_workflow_history( + workflow=original_workflow, + status=HistoryStatusChoices.STARTED, + ) + with pytest.raises(AutomationWorkflowRateLimited) as exc: + AutomationWorkflowHandler()._check_is_rate_limited(original_workflow) + + assert str(exc.value) == ( + "The workflow was rate limited due to too many recent or unfinished " + "runs. Limit exceeded: 4 runs in 60 seconds." + ) + + @override_settings( + AUTOMATION_WORKFLOW_RATE_LIMITS=((2, 5), (4, 60), (10, 3600)), +) +@pytest.mark.django_db +def test_check_is_rate_limited_uses_a_single_query_for_multiple_windows(data_fixture): + original_workflow = data_fixture.create_automation_workflow() + + with freeze_time("2025-08-01 14:00:00"): + for _ in range(4): + data_fixture.create_automation_workflow_history( + workflow=original_workflow, + status=HistoryStatusChoices.SUCCESS, + ) + + with CaptureQueriesContext(connection) as queries: + with pytest.raises(AutomationWorkflowRateLimited) as exc: + AutomationWorkflowHandler()._check_is_rate_limited(original_workflow) + + assert str(exc.value) == ( + "The workflow was rate limited due to too many recent or unfinished " + "runs. Limit exceeded: 2 runs in 5 seconds." + ) + + assert len(queries) == 1 + + +@pytest.mark.django_db +@override_settings( + AUTOMATION_WORKFLOW_RATE_LIMITS=((2, 5),), AUTOMATION_WORKFLOW_HISTORY_RATE_LIMIT_CACHE_EXPIRY_SECONDS=5, - AUTOMATION_WORKFLOW_RATE_LIMIT_MAX_RUNS=2, ) @patch(f"{WORKFLOWS_MODULE}.handler.start_workflow_celery_task") def test_workflow_rate_limiter_is_checked_before_starting_celery_task( mock_celery_task, data_fixture, django_capture_on_commit_callbacks ): - user = data_fixture.create_user() + with freeze_time("2026-01-26 13:00:00"): + user = data_fixture.create_user() - original_workflow = data_fixture.create_automation_workflow(user=user) - published_workflow = data_fixture.create_automation_workflow( - state=WorkflowState.LIVE, user=user - ) - published_workflow.automation.published_from = original_workflow - published_workflow.automation.save() + original_workflow = data_fixture.create_automation_workflow(user=user) + published_workflow = data_fixture.create_automation_workflow( + state=WorkflowState.LIVE, user=user + ) + published_workflow.automation.published_from = original_workflow + published_workflow.automation.save() - handler = AutomationWorkflowHandler() - rate_limited_error = ( - "The workflow was rate limited due to too many recent or unfinished runs." - ) + handler = AutomationWorkflowHandler() + rate_limited_error = ( + "The workflow was rate limited due to too many recent or unfinished " + "runs. Limit exceeded: 2 runs in 5 seconds." + ) - with freeze_time("2026-01-26 13:00:00"): with django_capture_on_commit_callbacks(execute=True): # First 2 calls should queue workflow runs handler.async_start_workflow(published_workflow) @@ -656,7 +716,8 @@ def test_async_start_workflow_creates_rate_limited_history_once_until_cache_rese published_workflow.automation.save() mock_before_run.side_effect = AutomationWorkflowRateLimited( - "The workflow was rate limited due to too many recent or unfinished runs." + "The workflow was rate limited due to too many recent or unfinished runs. " + "Limit exceeded: 2 runs in 5 seconds." ) handler = AutomationWorkflowHandler() @@ -672,7 +733,8 @@ def test_async_start_workflow_creates_rate_limited_history_once_until_cache_rese original_workflow, 1, "error", - "The workflow was rate limited due to too many recent or unfinished runs.", + "The workflow was rate limited due to too many recent or unfinished runs. " + "Limit exceeded: 2 runs in 5 seconds.", ) mock_celery_task.delay.assert_not_called() @@ -1109,9 +1171,8 @@ def test_async_start_workflow_with_simulate_until_node_and_error_creates_no_hist @pytest.mark.django_db @override_settings( - AUTOMATION_WORKFLOW_RATE_LIMIT_CACHE_EXPIRY_SECONDS=4, + AUTOMATION_WORKFLOW_RATE_LIMITS=((2, 4),), AUTOMATION_WORKFLOW_HISTORY_RATE_LIMIT_CACHE_EXPIRY_SECONDS=2, - AUTOMATION_WORKFLOW_RATE_LIMIT_MAX_RUNS=2, AUTOMATION_WORKFLOW_MAX_CONSECUTIVE_ERRORS=2, ) @patch(f"{WORKFLOWS_MODULE}.handler.start_workflow_celery_task") @@ -1164,16 +1225,19 @@ def test_async_start_workflow_rate_limited_runs_eventually_disable_workflow( assert histories[2].status == HistoryStatusChoices.ERROR assert histories[2].message == ( - "The workflow was rate limited due to too many recent or unfinished runs." + "The workflow was rate limited due to too many recent or unfinished runs. " + "Limit exceeded: 2 runs in 4 seconds." ) assert histories[3].status == HistoryStatusChoices.ERROR assert histories[3].message == ( - "The workflow was rate limited due to too many recent or unfinished runs." + "The workflow was rate limited due to too many recent or unfinished runs. " + "Limit exceeded: 2 runs in 4 seconds." ) assert histories[4].status == HistoryStatusChoices.ERROR assert histories[4].message == ( - "The workflow was rate limited due to too many recent or unfinished runs." + "The workflow was rate limited due to too many recent or unfinished runs. " + "Limit exceeded: 2 runs in 4 seconds." ) assert histories[5].status == HistoryStatusChoices.DISABLED @@ -1211,9 +1275,8 @@ def test_async_start_workflow_queues_celery_task_on_commit( @pytest.mark.django_db @override_settings( - AUTOMATION_WORKFLOW_RATE_LIMIT_CACHE_EXPIRY_SECONDS=30, + AUTOMATION_WORKFLOW_RATE_LIMITS=((2, 30),), AUTOMATION_WORKFLOW_HISTORY_RATE_LIMIT_CACHE_EXPIRY_SECONDS=30, - AUTOMATION_WORKFLOW_RATE_LIMIT_MAX_RUNS=2, ) def test_check_is_rate_limited_ignores_runs_before_latest_publish(data_fixture): original_workflow = data_fixture.create_automation_workflow() @@ -1225,16 +1288,6 @@ def test_check_is_rate_limited_ignores_runs_before_latest_publish(data_fixture): workflow=original_workflow, status=HistoryStatusChoices.STARTED, ) - global_cache.update( - handler._get_rate_limit_cache_key(original_workflow), - lambda _: [ - datetime.datetime(2026, 3, 10, 10, 0, 0, tzinfo=datetime.timezone.utc), - datetime.datetime(2026, 3, 10, 10, 0, 1, tzinfo=datetime.timezone.utc), - datetime.datetime(2026, 3, 10, 10, 0, 2, tzinfo=datetime.timezone.utc), - ], - default_value=lambda: [], - timeout=30, - ) with freeze_time("2026-03-10 12:00:00"): published_workflow = handler.publish(original_workflow) diff --git a/changelog/entries/unreleased/refactor/improve_rate_limiting_to_support_multiple_time_frames.json b/changelog/entries/unreleased/refactor/improve_rate_limiting_to_support_multiple_time_frames.json new file mode 100644 index 0000000000..b9a3b3dd26 --- /dev/null +++ b/changelog/entries/unreleased/refactor/improve_rate_limiting_to_support_multiple_time_frames.json @@ -0,0 +1,9 @@ +{ + "type": "refactor", + "message": "Improve rate limiting to support multiple time frames", + "issue_origin": "github", + "issue_number": null, + "domain": "automation", + "bullet_points": [], + "created_at": "2026-04-13" +} \ No newline at end of file diff --git a/docker-compose.no-caddy.yml b/docker-compose.no-caddy.yml index d15956d3fe..327a8db151 100644 --- a/docker-compose.no-caddy.yml +++ b/docker-compose.no-caddy.yml @@ -86,6 +86,7 @@ x-backend-variables: BASEROW_AUTOMATION_HISTORY_PAGE_SIZE_LIMIT: BASEROW_AUTOMATION_WORKFLOW_RATE_LIMIT_MAX_RUNS: + BASEROW_AUTOMATION_WORKFLOW_RATE_LIMITS: BASEROW_AUTOMATION_WORKFLOW_RATE_LIMIT_CACHE_EXPIRY_SECONDS: BASEROW_AUTOMATION_WORKFLOW_HISTORY_RATE_LIMIT_CACHE_EXPIRY_SECONDS: BASEROW_AUTOMATION_WORKFLOW_MAX_CONSECUTIVE_ERRORS: @@ -246,6 +247,7 @@ services: BASEROW_FRONTEND_SAME_SITE_COOKIE: BASEROW_AUTOMATION_HISTORY_PAGE_SIZE_LIMIT: BASEROW_AUTOMATION_WORKFLOW_RATE_LIMIT_MAX_RUNS: + BASEROW_AUTOMATION_WORKFLOW_RATE_LIMITS: BASEROW_AUTOMATION_WORKFLOW_RATE_LIMIT_CACHE_EXPIRY_SECONDS: BASEROW_AUTOMATION_WORKFLOW_HISTORY_RATE_LIMIT_CACHE_EXPIRY_SECONDS: BASEROW_AUTOMATION_WORKFLOW_MAX_CONSECUTIVE_ERRORS: diff --git a/docker-compose.yml b/docker-compose.yml index 7de5f13adc..d4d4a64dd1 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -100,6 +100,7 @@ x-backend-variables: BASEROW_AUTOMATION_HISTORY_PAGE_SIZE_LIMIT: BASEROW_AUTOMATION_WORKFLOW_RATE_LIMIT_MAX_RUNS: + BASEROW_AUTOMATION_WORKFLOW_RATE_LIMITS: BASEROW_AUTOMATION_WORKFLOW_RATE_LIMIT_CACHE_EXPIRY_SECONDS: BASEROW_AUTOMATION_WORKFLOW_HISTORY_RATE_LIMIT_CACHE_EXPIRY_SECONDS: BASEROW_AUTOMATION_WORKFLOW_MAX_CONSECUTIVE_ERRORS: @@ -329,6 +330,7 @@ services: BASEROW_PREMIUM_GROUPED_AGGREGATE_SERVICE_MAX_AGG_BUCKETS: BASEROW_AUTOMATION_HISTORY_PAGE_SIZE_LIMIT: BASEROW_AUTOMATION_WORKFLOW_RATE_LIMIT_MAX_RUNS: + BASEROW_AUTOMATION_WORKFLOW_RATE_LIMITS: BASEROW_AUTOMATION_WORKFLOW_RATE_LIMIT_CACHE_EXPIRY_SECONDS: BASEROW_AUTOMATION_WORKFLOW_HISTORY_RATE_LIMIT_CACHE_EXPIRY_SECONDS: BASEROW_AUTOMATION_WORKFLOW_MAX_CONSECUTIVE_ERRORS: diff --git a/docs/installation/configuration.md b/docs/installation/configuration.md index 40c97838df..43e283478d 100644 --- a/docs/installation/configuration.md +++ b/docs/installation/configuration.md @@ -208,6 +208,7 @@ The installation methods referred to in the variable descriptions are: |---------------------------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------|----------| | BASEROW\_AUTOMATION\_HISTORY\_PAGE\_SIZE\_LIMIT | The maximum number of automation history entries returned per page. | 100 | | BASEROW\_AUTOMATION\_WORKFLOW\_RATE\_LIMIT\_MAX\_RUNS | The maximum number of workflow runs that can be started within the rate limit window before new runs are blocked. | 10 | +| BASEROW\_AUTOMATION\_WORKFLOW\_RATE\_LIMITS | A comma-separated list of integer pairs formatted as `max_runs,window_seconds`. For example, `10,5,20,60` means 10 executions per 5 seconds and 20 executions per 60 seconds. | empty | | BASEROW\_AUTOMATION\_WORKFLOW\_RATE\_LIMIT\_CACHE\_EXPIRY\_SECONDS | The number of seconds the workflow rate limit counters are retained in cache. | 5 | | BASEROW\_AUTOMATION\_WORKFLOW\_HISTORY\_RATE\_LIMIT\_CACHE\_EXPIRY\_SECONDS | The number of seconds the workflow history rate limit counters are retained in cache. If unset, it uses the workflow rate limit cache expiry. | 5 | | BASEROW\_AUTOMATION\_WORKFLOW\_MAX\_CONSECUTIVE\_ERRORS | The maximum number of consecutive workflow errors allowed before the workflow is disabled. | 5 |