diff --git a/app/api/views/mailbox.py b/app/api/views/mailbox.py index 27475b86..2d8fa83c 100644 --- a/app/api/views/mailbox.py +++ b/app/api/views/mailbox.py @@ -13,8 +13,8 @@ from app.db import Session from app.email_utils import ( mailbox_already_used, email_can_be_used_as_mailbox, - is_valid_email, ) +from app.email_validation import is_valid_email from app.log import LOG from app.models import Mailbox, Job from app.utils import sanitize_email diff --git a/app/dashboard/views/alias_contact_manager.py b/app/dashboard/views/alias_contact_manager.py index 0e7d8624..730d2f6b 100644 --- a/app/dashboard/views/alias_contact_manager.py +++ b/app/dashboard/views/alias_contact_manager.py @@ -13,10 +13,10 @@ from app import config, parallel_limiter from app.dashboard.base import dashboard_bp from app.db import Session from app.email_utils import ( - is_valid_email, generate_reply_email, parse_full_address, ) +from app.email_validation import is_valid_email from app.errors import ( CannotCreateContactForReverseAlias, ErrContactErrorUpgradeNeeded, diff --git a/app/dashboard/views/mailbox.py b/app/dashboard/views/mailbox.py index 6e7c2033..ee113c2e 100644 --- a/app/dashboard/views/mailbox.py +++ b/app/dashboard/views/mailbox.py @@ -19,8 +19,8 @@ from app.email_utils import ( mailbox_already_used, render, send_email, - is_valid_email, ) +from app.email_validation import is_valid_email from app.log import LOG from app.models import Mailbox, Job from app.utils import CSRFValidationForm diff --git a/app/email_utils.py b/app/email_utils.py index 70cec703..67e406ff 100644 --- a/app/email_utils.py +++ b/app/email_utils.py @@ -828,19 +828,6 @@ def should_add_dkim_signature(domain: str) -> bool: return False -def is_valid_email(email_address: str) -> bool: - """ - Used to check whether an email address is valid - NOT run MX check. - NOT allow unicode. - """ - try: - validate_email(email_address, check_deliverability=False, allow_smtputf8=False) - return True - except EmailNotValidError: - return False - - class EmailEncoding(enum.Enum): BASE64 = "base64" QUOTED = "quoted-printable" @@ -1116,26 +1103,6 @@ def is_reverse_alias(address: str) -> bool: ) -# allow also + and @ that are present in a reply address -_ALLOWED_CHARS = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_-.+@" - - -def normalize_reply_email(reply_email: str) -> str: - """Handle the case where reply email contains *strange* char that was wrongly generated in the past""" - if not reply_email.isascii(): - reply_email = convert_to_id(reply_email) - - ret = [] - # drop all control characters like shift, separator, etc - for c in reply_email: - if c not in _ALLOWED_CHARS: - ret.append("_") - else: - ret.append(c) - - return "".join(ret) - - def should_disable(alias: Alias) -> (bool, str): """ Return whether an alias should be disabled and if yes, the reason why diff --git a/app/email_validation.py b/app/email_validation.py new file mode 100644 index 00000000..f17a45d0 --- /dev/null +++ b/app/email_validation.py @@ -0,0 +1,38 @@ +from email_validator import ( + validate_email, + EmailNotValidError, +) + +from app.utils import convert_to_id + +# allow also + and @ that are present in a reply address +_ALLOWED_CHARS = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_-.+@" + + +def is_valid_email(email_address: str) -> bool: + """ + Used to check whether an email address is valid + NOT run MX check. + NOT allow unicode. + """ + try: + validate_email(email_address, check_deliverability=False, allow_smtputf8=False) + return True + except EmailNotValidError: + return False + + +def normalize_reply_email(reply_email: str) -> str: + """Handle the case where reply email contains *strange* char that was wrongly generated in the past""" + if not reply_email.isascii(): + reply_email = convert_to_id(reply_email) + + ret = [] + # drop all control characters like shift, separator, etc + for c in reply_email: + if c not in _ALLOWED_CHARS: + ret.append("_") + else: + ret.append(c) + + return "".join(ret) diff --git a/app/errors.py b/app/errors.py index 217b6b9e..b951eb16 100644 --- a/app/errors.py +++ b/app/errors.py @@ -84,6 +84,14 @@ class ErrAddressInvalid(SLException): return f"{self.address} is not a valid email address" +class InvalidContactEmailError(SLException): + def __init__(self, website_email: str): # noqa: F821 + self.website_email = website_email + + def error_for_user(self) -> str: + return f"Cannot create contact with invalid email {self.website_email}" + + class ErrContactAlreadyExists(SLException): """raised when a contact already exists""" diff --git a/app/models.py b/app/models.py index faec2e19..9242b528 100644 --- a/app/models.py +++ b/app/models.py @@ -30,11 +30,13 @@ from sqlalchemy_utils import ArrowType from app import config from app import s3 from app.db import Session +from app.email_validation import is_valid_email, normalize_reply_email from app.errors import ( AliasInTrashError, DirectoryInTrashError, SubdomainInTrashError, CannotCreateContactForReverseAlias, + InvalidContactEmailError, ) from app.handler.unsubscribe_encoder import UnsubscribeAction, UnsubscribeEncoder from app.log import LOG @@ -577,6 +579,7 @@ class User(Base, ModelMixin, UserMixin, PasswordOracle): @classmethod def create(cls, email, name="", password=None, from_partner=False, **kwargs): + email = sanitize_email(email) user: User = super(User, cls).create(email=email, name=name[:100], **kwargs) if password: @@ -1802,11 +1805,11 @@ class Contact(Base, ModelMixin): commit = kw.pop("commit", False) flush = kw.pop("flush", False) - new_contact = cls(**kw) - - website_email = kw["website_email"] # make sure email is lowercase and doesn't have any whitespace - website_email = sanitize_email(website_email) + website_email = sanitize_email(kw["website_email"]) + kw["website_email"] = website_email + if "reply_email" in kw: + kw["reply_email"] = normalize_reply_email(sanitize_email(kw["reply_email"])) # make sure contact.website_email isn't a reverse alias if website_email != config.NOREPLY: @@ -1814,6 +1817,10 @@ class Contact(Base, ModelMixin): if orig_contact: raise CannotCreateContactForReverseAlias(str(orig_contact)) + if not is_valid_email(website_email): + raise InvalidContactEmailError(website_email) + + new_contact = cls(**kw) Session.add(new_contact) if commit: @@ -2300,6 +2307,7 @@ class CustomDomain(Base, ModelMixin): @classmethod def create(cls, **kwargs): domain = kwargs.get("domain") + kwargs["domain"] = domain.replace("\n", "") if DeletedSubdomain.get_by(domain=domain): raise SubdomainInTrashError @@ -2599,6 +2607,12 @@ class Mailbox(Base, ModelMixin): return ret + @classmethod + def create(cls, **kw): + if "email" in kw: + kw["email"] = sanitize_email(kw["email"]) + return super().create(**kw) + def __repr__(self): return f"" diff --git a/app/utils.py b/app/utils.py index bc7035ca..e6577b77 100644 --- a/app/utils.py +++ b/app/utils.py @@ -99,7 +99,7 @@ def sanitize_email(email_address: str, not_lower=False) -> str: email_address = email_address.strip().replace(" ", "").replace("\n", " ") if not not_lower: email_address = email_address.lower() - return email_address + return email_address.replace("\u200f", "") class NextUrlSanitizer: diff --git a/cron.py b/cron.py index 64c65327..7c3cd2cc 100644 --- a/cron.py +++ b/cron.py @@ -22,10 +22,9 @@ from app.email_utils import ( render, email_can_be_used_as_mailbox, send_email_with_rate_control, - normalize_reply_email, - is_valid_email, get_email_domain_part, ) +from app.email_validation import is_valid_email, normalize_reply_email from app.errors import ProtonPartnerNotSetUp from app.log import LOG from app.mail_sender import load_unsent_mails_from_fs_and_resend diff --git a/crontab.yml b/crontab.yml index ba47303f..ece8ac1f 100644 --- a/crontab.yml +++ b/crontab.yml @@ -5,62 +5,64 @@ jobs: schedule: "0 0 * * *" captureStderr: true - - name: SimpleLogin Notify Trial Ends - command: python /code/cron.py -j notify_trial_end - shell: /bin/bash - schedule: "0 8 * * *" - captureStderr: true - - - name: SimpleLogin Notify Manual Subscription Ends - command: python /code/cron.py -j notify_manual_subscription_end - shell: /bin/bash - schedule: "0 9 * * *" - captureStderr: true - - - name: SimpleLogin Notify Premium Ends - command: python /code/cron.py -j notify_premium_end - shell: /bin/bash - schedule: "0 10 * * *" - captureStderr: true - - - name: SimpleLogin Delete Logs - command: python /code/cron.py -j delete_logs - shell: /bin/bash - schedule: "0 11 * * *" - captureStderr: true - - - name: SimpleLogin Poll Apple Subscriptions - command: python /code/cron.py -j poll_apple_subscription - shell: /bin/bash - schedule: "0 12 * * *" - captureStderr: true - - name: SimpleLogin Delete Old Monitoring records command: python /code/cron.py -j delete_old_monitoring shell: /bin/bash - schedule: "0 14 * * *" + schedule: "15 1 * * *" captureStderr: true - name: SimpleLogin Custom Domain check command: python /code/cron.py -j check_custom_domain shell: /bin/bash - schedule: "0 15 * * *" + schedule: "15 2 * * *" captureStderr: true - name: SimpleLogin HIBP check command: python /code/cron.py -j check_hibp shell: /bin/bash - schedule: "0 18 * * *" + schedule: "15 3 * * *" captureStderr: true concurrencyPolicy: Forbid - name: SimpleLogin Notify HIBP breaches command: python /code/cron.py -j notify_hibp shell: /bin/bash - schedule: "0 19 * * *" + schedule: "15 4 * * *" captureStderr: true concurrencyPolicy: Forbid + - name: SimpleLogin Delete Logs + command: python /code/cron.py -j delete_logs + shell: /bin/bash + schedule: "15 5 * * *" + captureStderr: true + + - name: SimpleLogin Poll Apple Subscriptions + command: python /code/cron.py -j poll_apple_subscription + shell: /bin/bash + schedule: "15 6 * * *" + captureStderr: true + + - name: SimpleLogin Notify Trial Ends + command: python /code/cron.py -j notify_trial_end + shell: /bin/bash + schedule: "15 8 * * *" + captureStderr: true + + - name: SimpleLogin Notify Manual Subscription Ends + command: python /code/cron.py -j notify_manual_subscription_end + shell: /bin/bash + schedule: "15 9 * * *" + captureStderr: true + + - name: SimpleLogin Notify Premium Ends + command: python /code/cron.py -j notify_premium_end + shell: /bin/bash + schedule: "15 10 * * *" + captureStderr: true + + + - name: SimpleLogin send unsent emails command: python /code/cron.py -j send_undelivered_mails shell: /bin/bash diff --git a/email_handler.py b/email_handler.py index bce0bfe6..2136fa74 100644 --- a/email_handler.py +++ b/email_handler.py @@ -106,8 +106,6 @@ from app.email_utils import ( get_header_unicode, generate_reply_email, is_reverse_alias, - normalize_reply_email, - is_valid_email, replace, should_disable, parse_id_from_bounce, @@ -123,6 +121,7 @@ from app.email_utils import ( generate_verp_email, sl_formataddr, ) +from app.email_validation import is_valid_email, normalize_reply_email from app.errors import ( NonReverseAliasInReplyPhase, VERPTransactional, diff --git a/tests/test_email_utils.py b/tests/test_email_utils.py index 749603a7..e33b3fb1 100644 --- a/tests/test_email_utils.py +++ b/tests/test_email_utils.py @@ -19,10 +19,8 @@ from app.email_utils import ( copy, get_spam_from_header, get_header_from_bounce, - is_valid_email, add_header, generate_reply_email, - normalize_reply_email, get_encoding, encode_text, EmailEncoding, @@ -41,6 +39,7 @@ from app.email_utils import ( get_verp_info_from_email, sl_formataddr, ) +from app.email_validation import is_valid_email, normalize_reply_email from app.models import ( CustomDomain, Alias, diff --git a/tests/test_models.py b/tests/test_models.py index 8b4d6b94..82da0586 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -78,20 +78,20 @@ def test_website_send_to(flask_client): user_id=user.id, alias_id=alias.id, website_email=f"{prefix}@example.com", - reply_email="rep@SL", + reply_email="rep@sl", name="First Last", ) - assert c1.website_send_to() == f'"First Last | {prefix} at example.com" ' + assert c1.website_send_to() == f'"First Last | {prefix} at example.com" ' # empty name, ascii website_from, easy case c1.name = None c1.website_from = f"First Last <{prefix}@example.com>" - assert c1.website_send_to() == f'"First Last | {prefix} at example.com" ' + assert c1.website_send_to() == f'"First Last | {prefix} at example.com" ' # empty name, RFC 2047 website_from c1.name = None c1.website_from = f"=?UTF-8?B?TmjGoW4gTmd1eeG7hW4=?= <{prefix}@example.com>" - assert c1.website_send_to() == f'"Nhơn Nguyễn | {prefix} at example.com" ' + assert c1.website_send_to() == f'"Nhơn Nguyễn | {prefix} at example.com" ' def test_new_addr_default_sender_format(flask_client): @@ -103,16 +103,16 @@ def test_new_addr_default_sender_format(flask_client): user_id=user.id, alias_id=alias.id, website_email=f"{prefix}@example.com", - reply_email="rep@SL", + reply_email="rep@sl", name="First Last", commit=True, ) - assert contact.new_addr() == f'"First Last - {prefix} at example.com" ' + assert contact.new_addr() == f'"First Last - {prefix} at example.com" ' # Make sure email isn't duplicated if sender name equals email contact.name = f"{prefix}@example.com" - assert contact.new_addr() == f'"{prefix} at example.com" ' + assert contact.new_addr() == f'"{prefix} at example.com" ' def test_new_addr_a_sender_format(flask_client): @@ -126,12 +126,12 @@ def test_new_addr_a_sender_format(flask_client): user_id=user.id, alias_id=alias.id, website_email=f"{prefix}@example.com", - reply_email="rep@SL", + reply_email="rep@sl", name="First Last", commit=True, ) - assert contact.new_addr() == f'"First Last - {prefix}(a)example.com" ' + assert contact.new_addr() == f'"First Last - {prefix}(a)example.com" ' def test_new_addr_no_name_sender_format(flask_client): @@ -145,12 +145,12 @@ def test_new_addr_no_name_sender_format(flask_client): user_id=user.id, alias_id=alias.id, website_email=f"{prefix}@example.com", - reply_email="rep@SL", + reply_email="rep@sl", name="First Last", commit=True, ) - assert contact.new_addr() == "rep@SL" + assert contact.new_addr() == "rep@sl" def test_new_addr_name_only_sender_format(flask_client): @@ -164,12 +164,12 @@ def test_new_addr_name_only_sender_format(flask_client): user_id=user.id, alias_id=alias.id, website_email=f"{prefix}@example.com", - reply_email="rep@SL", + reply_email="rep@sl", name="First Last", commit=True, ) - assert contact.new_addr() == "First Last " + assert contact.new_addr() == "First Last " def test_new_addr_at_only_sender_format(flask_client): @@ -183,12 +183,12 @@ def test_new_addr_at_only_sender_format(flask_client): user_id=user.id, alias_id=alias.id, website_email=f"{prefix}@example.com", - reply_email="rep@SL", + reply_email="rep@sl", name="First Last", commit=True, ) - assert contact.new_addr() == f'"{prefix} at example.com" ' + assert contact.new_addr() == f'"{prefix} at example.com" ' def test_new_addr_unicode(flask_client): @@ -200,14 +200,14 @@ def test_new_addr_unicode(flask_client): user_id=user.id, alias_id=alias.id, website_email=f"{random_prefix}@example.com", - reply_email="rep@SL", + reply_email="rep@sl", name="Nhơn Nguyễn", commit=True, ) assert ( contact.new_addr() - == f"=?utf-8?q?Nh=C6=A1n_Nguy=E1=BB=85n_-_{random_prefix}_at_example=2Ecom?= " + == f"=?utf-8?q?Nh=C6=A1n_Nguy=E1=BB=85n_-_{random_prefix}_at_example=2Ecom?= " ) # sanity check