From 6532e0de935625ffc4f93c6629762ec37b9cf8ab Mon Sep 17 00:00:00 2001 From: Son NK <> Date: Thu, 24 Jun 2021 09:47:01 +0200 Subject: [PATCH] Return 550 instead of 421 when rate limited. Rename greylisting to rate limit --- app/config.py | 2 +- app/{greylisting.py => email/rate_limit.py} | 22 +++++----- app/email/status.py | 12 ++++-- email_handler.py | 9 ++--- tests/email/__init__.py | 0 .../test_rate_limit.py} | 40 +++++++++---------- 6 files changed, 43 insertions(+), 42 deletions(-) rename app/{greylisting.py => email/rate_limit.py} (76%) create mode 100644 tests/email/__init__.py rename tests/{test_greylisting.py => email/test_rate_limit.py} (66%) diff --git a/app/config.py b/app/config.py index 4014ec8f..2abd8e22 100644 --- a/app/config.py +++ b/app/config.py @@ -265,7 +265,7 @@ PAGE_LIMIT = 20 LOCAL_FILE_UPLOAD = "LOCAL_FILE_UPLOAD" in os.environ UPLOAD_DIR = None -# Greylisting features +# Rate Limiting # nb max of activity (forward/reply) an alias can have during 1 min MAX_ACTIVITY_DURING_MINUTE_PER_ALIAS = 10 diff --git a/app/greylisting.py b/app/email/rate_limit.py similarity index 76% rename from app/greylisting.py rename to app/email/rate_limit.py index ef38f5c7..0fec2619 100644 --- a/app/greylisting.py +++ b/app/email/rate_limit.py @@ -11,7 +11,7 @@ from app.log import LOG from app.models import Alias, EmailLog, Contact -def greylisting_needed_for_alias(alias: Alias) -> bool: +def rate_limited_for_alias(alias: Alias) -> bool: min_time = arrow.now().shift(minutes=-1) # get the nb of activity on this alias @@ -37,7 +37,7 @@ def greylisting_needed_for_alias(alias: Alias) -> bool: return False -def greylisting_needed_for_mailbox(alias: Alias) -> bool: +def rate_limited_for_mailbox(alias: Alias) -> bool: min_time = arrow.now().shift(minutes=-1) # get nb of activity on this mailbox @@ -65,13 +65,11 @@ def greylisting_needed_for_mailbox(alias: Alias) -> bool: return False -def greylisting_needed_forward_phase(alias_address: str) -> bool: +def rate_limited_forward_phase(alias_address: str) -> bool: alias = Alias.get_by(email=alias_address) if alias: - return greylisting_needed_for_alias(alias) or greylisting_needed_for_mailbox( - alias - ) + return rate_limited_for_alias(alias) or rate_limited_for_mailbox(alias) else: LOG.d( @@ -80,29 +78,29 @@ def greylisting_needed_forward_phase(alias_address: str) -> bool: ) alias = try_auto_create(alias_address) if alias: - return greylisting_needed_for_mailbox(alias) + return rate_limited_for_mailbox(alias) return False -def greylisting_needed_reply_phase(reply_email: str) -> bool: +def rate_limited_reply_phase(reply_email: str) -> bool: contact = Contact.get_by(reply_email=reply_email) if not contact: return False alias = contact.alias - return greylisting_needed_for_alias(alias) or greylisting_needed_for_mailbox(alias) + return rate_limited_for_alias(alias) or rate_limited_for_mailbox(alias) -def greylisting_needed(mail_from: str, rcpt_tos: [str]) -> bool: +def rate_limited(mail_from: str, rcpt_tos: [str]) -> bool: for rcpt_to in rcpt_tos: if is_reply_email(rcpt_to): - if greylisting_needed_reply_phase(rcpt_to): + if rate_limited_reply_phase(rcpt_to): return True else: # Forward phase address = rcpt_to # alias@SL - if greylisting_needed_forward_phase(address): + if rate_limited_forward_phase(address): return True return False diff --git a/app/email/status.py b/app/email/status.py index 1b2256cd..97d4c747 100644 --- a/app/email/status.py +++ b/app/email/status.py @@ -10,10 +10,10 @@ E206 = "250 SL E206 Out of office" # 4** errors # E401 = "421 SL E401 Retry later" -E402 = "421 SL E402 Retry later" -E403 = "421 SL E403 Retry later" -E404 = "421 SL E404 Retry later" -E405 = "421 SL E405 Retry later" +E402 = "421 SL E402 Encryption failed - Retry later" +# E403 = "421 SL E403 Retry later" +E404 = "421 SL E404 Unexpected error - Retry later" +E405 = "421 SL E405 Mailbox domain problem - Retry later" E406 = "421 SL E406 Retry later" # 5** errors @@ -38,3 +38,7 @@ E518 = "550 SL E518 Disabled mailbox" E519 = "550 SL E519 Email detected as spam" E520 = "550 SL E520 Email cannot be sent to contact" E521 = "550 SL E521 Cannot reach mailbox" +E522 = ( + "550 SL E522 The user you are trying to contact is receiving mail " + "at a rate that prevents additional messages from being delivered." +) diff --git a/email_handler.py b/email_handler.py index 8944e515..b827caf7 100644 --- a/email_handler.py +++ b/email_handler.py @@ -110,7 +110,7 @@ from app.email_utils import ( get_queue_id, ) from app.extensions import db -from app.greylisting import greylisting_needed +from app.email.rate_limit import rate_limited from app.log import LOG, set_message_id from app.models import ( Alias, @@ -1636,10 +1636,9 @@ def handle(envelope: Envelope) -> str: ) return handle_bounce(envelope, email_log, msg) - # Whether it's necessary to apply greylisting - if greylisting_needed(mail_from, rcpt_tos): - LOG.w("Grey listing applied for mail_from:%s rcpt_tos:%s", mail_from, rcpt_tos) - return status.E403 + if rate_limited(mail_from, rcpt_tos): + LOG.w("Rate Limiting applied for mail_from:%s rcpt_tos:%s", mail_from, rcpt_tos) + return status.E522 # Handle "out of office" auto notice. An automatic response is sent for every forwarded email # todo: remove logging diff --git a/tests/email/__init__.py b/tests/email/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/test_greylisting.py b/tests/email/test_rate_limit.py similarity index 66% rename from tests/test_greylisting.py rename to tests/email/test_rate_limit.py index 4928e2ae..34c58a0b 100644 --- a/tests/test_greylisting.py +++ b/tests/email/test_rate_limit.py @@ -3,27 +3,27 @@ from app.config import ( MAX_ACTIVITY_DURING_MINUTE_PER_MAILBOX, ) from app.extensions import db -from app.greylisting import ( - greylisting_needed_forward_phase, - greylisting_needed_for_alias, - greylisting_needed_for_mailbox, - greylisting_needed_reply_phase, +from app.email.rate_limit import ( + rate_limited_forward_phase, + rate_limited_for_alias, + rate_limited_for_mailbox, + rate_limited_reply_phase, ) from app.models import User, Alias, EmailLog, Contact -def test_greylisting_needed_forward_phase_for_alias(flask_client): +def test_rate_limited_forward_phase_for_alias(flask_client): user = User.create( email="a@b.c", password="password", name="Test User", activated=True ) db.session.commit() - # no greylisting for a new alias + # no rate limiting for a new alias alias = Alias.create_new_random(user) db.session.commit() - assert not greylisting_needed_for_alias(alias) + assert not rate_limited_for_alias(alias) - # greylisting when there's a previous activity on alias + # rate limit when there's a previous activity on alias contact = Contact.create( user_id=user.id, alias_id=alias.id, @@ -35,10 +35,10 @@ def test_greylisting_needed_forward_phase_for_alias(flask_client): EmailLog.create(user_id=user.id, contact_id=contact.id) db.session.commit() - assert greylisting_needed_for_alias(alias) + assert rate_limited_for_alias(alias) -def test_greylisting_needed_forward_phase_for_mailbox(flask_client): +def test_rate_limited_forward_phase_for_mailbox(flask_client): user = User.create( email="a@b.c", password="password", name="Test User", activated=True ) @@ -61,20 +61,20 @@ def test_greylisting_needed_forward_phase_for_mailbox(flask_client): EmailLog.create(user_id=user.id, contact_id=contact.id) # Create another alias with the same mailbox - # will be greylisted as there's a previous activity on mailbox + # will be rate limited as there's a previous activity on mailbox alias2 = Alias.create_new_random(user) db.session.commit() - assert greylisting_needed_for_mailbox(alias2) + assert rate_limited_for_mailbox(alias2) -def test_greylisting_needed_forward_phase(flask_client): - # no greylisting when alias not exist - assert not greylisting_needed_forward_phase("not-exist@alias.com") +def test_rate_limited_forward_phase(flask_client): + # no rate limiting when alias does not exist + assert not rate_limited_forward_phase("not-exist@alias.com") -def test_greylisting_needed_reply_phase(flask_client): - # no greylisting when reply_email not exist - assert not greylisting_needed_reply_phase("not-exist-reply@alias.com") +def test_rate_limited_reply_phase(flask_client): + # no rate limiting when reply_email does not exist + assert not rate_limited_reply_phase("not-exist-reply@alias.com") user = User.create( email="a@b.c", password="password", name="Test User", activated=True @@ -95,4 +95,4 @@ def test_greylisting_needed_reply_phase(flask_client): EmailLog.create(user_id=user.id, contact_id=contact.id) db.session.commit() - assert greylisting_needed_reply_phase("rep@sl.local") + assert rate_limited_reply_phase("rep@sl.local")