From 06e9f58bb0645d6bd56b2e76a3935b35e83e8dde Mon Sep 17 00:00:00 2001 From: Janne Forsman Date: Thu, 5 Feb 2026 12:38:34 +0200 Subject: [PATCH] feat: Use DEFAULT_FROM_EMAIL instead of hardcoding to management command * wrapper for django send_mail function that always sets from_email to None * Will make django to use DEFAULT_FROM_EMAIL * test file moved to correct location * CI/CD changes to environment variables and config map needed to have different from_email values env by env Refs: LIIK-906 --- cityinfra/settings.py | 3 +- .../management/commands/send_test_email.py | 44 +-- traffic_control/services/email.py | 62 ++++ .../commands}/test_send_test_email.py | 39 ++- traffic_control/tests/services/test_email.py | 272 ++++++++++++++++++ 5 files changed, 378 insertions(+), 42 deletions(-) create mode 100644 traffic_control/services/email.py rename traffic_control/tests/{ => management/commands}/test_send_test_email.py (78%) create mode 100644 traffic_control/tests/services/test_email.py diff --git a/cityinfra/settings.py b/cityinfra/settings.py index 1bb71ab8..b49b9161 100644 --- a/cityinfra/settings.py +++ b/cityinfra/settings.py @@ -54,6 +54,7 @@ EMAIL_USE_TLS=(bool, True), EMAIL_HOST_USER=(str, ""), EMAIL_HOST_PASSWORD=(str, ""), + DEFAULT_FROM_EMAIL=(str, "cityinfra@hel.fi"), SENTRY_DSN=(str, ""), SENTRY_DEBUG=(bool, False), VERSION=(str, ""), @@ -135,7 +136,7 @@ EMAIL_USE_TLS = env("EMAIL_USE_TLS") EMAIL_HOST_USER = env("EMAIL_HOST_USER") EMAIL_HOST_PASSWORD = env("EMAIL_HOST_PASSWORD") -DEFAULT_FROM_EMAIL = "cityinfra@hel.fi" +DEFAULT_FROM_EMAIL = env("DEFAULT_FROM_EMAIL") # Determine which email backend to use based on environment TESTING = "test" in sys.argv or "pytest" in sys.argv[0] if sys.argv else False diff --git a/traffic_control/management/commands/send_test_email.py b/traffic_control/management/commands/send_test_email.py index ac89094b..2cb9434b 100644 --- a/traffic_control/management/commands/send_test_email.py +++ b/traffic_control/management/commands/send_test_email.py @@ -3,10 +3,10 @@ import smtplib from typing import Any -from django.core.exceptions import ValidationError -from django.core.mail import send_mail +from django.conf import settings from django.core.management.base import BaseCommand, CommandError, CommandParser -from django.core.validators import validate_email + +from traffic_control.services.email import send_email class Command(BaseCommand): @@ -31,62 +31,42 @@ def handle(self, *args: Any, **options: Any) -> None: """Execute the command to send test email. Security measures implemented: - - Email addresses are validated using Django's validate_email() - - Maximum 10 recipients to prevent abuse + - Email validation is handled by the send_email service function - Email body, subject, and sender are hardcoded (no user input) - Log output is sanitized to prevent log injection - - Django's send_mail() automatically sanitizes email headers Args: *args: Positional arguments. **options: Command options including 'recipient'. Raises: - CommandError: If email sending fails due to SMTP or configuration errors. + CommandError: If email sending fails due to SMTP, configuration, or validation errors. """ recipient_input = options["recipient"] recipients = [email.strip() for email in recipient_input.split(",") if email.strip()] - # Security: Validate all recipient email addresses to prevent injection attacks - # This prevents malformed addresses and email header injection attempts - for recipient in recipients: - try: - validate_email(recipient) - except ValidationError: - # Sanitize error message to prevent log injection - safe_recipient = recipient.replace("\n", "").replace("\r", "")[:100] - raise CommandError(f"Invalid email address: {safe_recipient}") - - if not recipients: - raise CommandError("No valid recipient email addresses provided") - - # Security: Limit number of recipients to prevent abuse and spam - max_recipients = 10 - if len(recipients) > max_recipients: - raise CommandError(f"Too many recipients. Maximum allowed: {max_recipients}") - # Security: All email content is hardcoded - no user input in email body, subject, or sender # This prevents email content injection attacks - sender = "cityinfra@hel.fi" subject = "Cityinfra email test" message = "Test message" - # Note: recipients are already validated above, but we sanitize for logging - # to prevent log injection (remove newlines/carriage returns) - safe_recipients_for_log = [r.replace("\n", "").replace("\r", "") for r in recipients] + # Sanitize recipient list for logging to prevent log injection (remove newlines/carriage returns) + safe_recipients_for_log = [r.replace("\n", "").replace("\r", "")[:100] for r in recipients] self.stdout.write(f"Sending test email to: {', '.join(safe_recipients_for_log)}") - self.stdout.write(f"From: {sender}") + self.stdout.write(f"From: {settings.DEFAULT_FROM_EMAIL}") self.stdout.write(f"Subject: {subject}") try: - send_mail( + send_email( subject=subject, message=message, - from_email=sender, recipient_list=recipients, fail_silently=False, ) self.stdout.write(self.style.SUCCESS(f"Successfully sent test email to {len(recipients)} recipient(s)")) + except ValueError as e: + # Validation errors from send_email service + raise CommandError(str(e)) except smtplib.SMTPException as e: raise CommandError(f"SMTP error occurred. Please check your email server configuration. Error: {str(e)}") except Exception as e: diff --git a/traffic_control/services/email.py b/traffic_control/services/email.py new file mode 100644 index 00000000..e9f3f433 --- /dev/null +++ b/traffic_control/services/email.py @@ -0,0 +1,62 @@ +from django.core.exceptions import ValidationError +from django.core.mail import send_mail +from django.core.validators import validate_email + + +def send_email( + subject: str, + message: str, + recipient_list: list[str], + fail_silently: bool = False, + html_message: str | None = None, + max_recipients: int = 10, +) -> int: + """ + Sends an email using Django's send_mail with from_email set to None. + This will use the DEFAULT_FROM_EMAIL from Django settings. + + Security measures implemented: + - Email addresses are validated using Django's validate_email() + - Maximum recipients limit to prevent abuse + - Django's send_mail() automatically sanitizes email headers + + Args: + subject (str): The subject line of the email. + message (str): The plain text message body. + recipient_list (list[str]): List of recipient email addresses. + fail_silently (bool): If True, exceptions during send are suppressed. Defaults to False. + html_message (str | None): Optional HTML version of the message. Defaults to None. + max_recipients (int): Maximum number of recipients allowed. Defaults to 10. + + Returns: + int: Number of successfully sent emails. + + Raises: + ValueError: If any email address is invalid, recipient list is empty, or too many recipients. + """ + # Validate recipient list is not empty + if not recipient_list: + raise ValueError("No recipient email addresses provided") + + # Validate all recipient email addresses to prevent injection attacks + # This prevents malformed addresses and email header injection attempts + for recipient in recipient_list: + try: + validate_email(recipient) + except ValidationError: + # Sanitize error message to prevent log injection + safe_recipient = recipient.replace("\n", "").replace("\r", "")[:100] + raise ValueError(f"Invalid email address: {safe_recipient}") + + # Limit number of recipients to prevent abuse and spam + if len(recipient_list) > max_recipients: + raise ValueError(f"Too many recipients. Maximum allowed: {max_recipients}") + + return send_mail( + subject=subject, + message=message, + from_email=None, + recipient_list=recipient_list, + fail_silently=fail_silently, + html_message=html_message, + ) diff --git a/traffic_control/tests/test_send_test_email.py b/traffic_control/tests/management/commands/test_send_test_email.py similarity index 78% rename from traffic_control/tests/test_send_test_email.py rename to traffic_control/tests/management/commands/test_send_test_email.py index bf8806ce..1b029dde 100644 --- a/traffic_control/tests/test_send_test_email.py +++ b/traffic_control/tests/management/commands/test_send_test_email.py @@ -1,32 +1,52 @@ -"""Tests for send_test_email management command.""" - import pytest from django.core import mail from django.core.management import call_command from django.core.management.base import CommandError +from django.test import override_settings +@pytest.mark.parametrize( + "from_email", + [ + "cityinfra@hel.fi", + "custom@example.com", + "another@test.org", + ], +) @pytest.mark.django_db -def test_send_test_email_single_recipient() -> None: +def test_send_test_email_single_recipient(from_email: str) -> None: """Test sending email to a single recipient. Verifies that the command successfully sends an email with correct sender, subject, and recipient information. + + Args: + from_email: The email address to use as the sender. """ + recipient = "test@example.com" - call_command("send_test_email", "--recipient", recipient) + with override_settings(DEFAULT_FROM_EMAIL=from_email): + call_command("send_test_email", "--recipient", recipient) assert len(mail.outbox) == 1 email = mail.outbox[0] assert email.subject == "Cityinfra email test" assert email.body == "Test message" - assert email.from_email == "cityinfra@hel.fi" + assert email.from_email == from_email assert email.to == [recipient] +@pytest.mark.parametrize( + "from_email", + [ + "cityinfra@hel.fi", + "custom@example.com", + "another@test.org", + ], +) @pytest.mark.django_db -def test_send_test_email_multiple_recipients() -> None: +def test_send_test_email_multiple_recipients(from_email: str) -> None: """Test sending email to multiple comma-separated recipients. Verifies that the command correctly parses comma-separated email @@ -35,12 +55,13 @@ def test_send_test_email_multiple_recipients() -> None: recipients = "test1@example.com,test2@example.com,test3@example.com" expected_recipients = ["test1@example.com", "test2@example.com", "test3@example.com"] - call_command("send_test_email", "--recipient", recipients) + with override_settings(DEFAULT_FROM_EMAIL=from_email): + call_command("send_test_email", "--recipient", recipients) assert len(mail.outbox) == 1 email = mail.outbox[0] assert email.subject == "Cityinfra email test" - assert email.from_email == "cityinfra@hel.fi" + assert email.from_email == from_email assert email.to == expected_recipients @@ -111,5 +132,5 @@ def test_send_test_email_empty_recipients() -> None: Verifies that the command rejects empty or whitespace-only recipient strings. """ - with pytest.raises(CommandError, match="No valid recipient email addresses"): + with pytest.raises(CommandError, match="No recipient email addresses provided"): call_command("send_test_email", "--recipient", " , , ") diff --git a/traffic_control/tests/services/test_email.py b/traffic_control/tests/services/test_email.py new file mode 100644 index 00000000..783fedd6 --- /dev/null +++ b/traffic_control/tests/services/test_email.py @@ -0,0 +1,272 @@ +"""Tests for email service.""" + +import pytest +from django.core import mail +from django.test import override_settings + +from traffic_control.services.email import send_email + + +@pytest.mark.django_db +def test_send_email_single_recipient() -> None: + """Test sending email to a single valid recipient. + + Verifies that the email is successfully sent with correct content. + """ + subject = "Test Subject" + message = "Test message body" + recipient = "test@example.com" + + result = send_email( + subject=subject, + message=message, + recipient_list=[recipient], + ) + + assert result == 1 + assert len(mail.outbox) == 1 + email = mail.outbox[0] + assert email.subject == subject + assert email.body == message + assert email.to == [recipient] + + +@pytest.mark.django_db +def test_send_email_multiple_recipients() -> None: + """Test sending email to multiple valid recipients. + + Verifies that the email is sent to all recipients successfully. + """ + subject = "Test Subject" + message = "Test message body" + recipients = ["test1@example.com", "test2@example.com", "test3@example.com"] + + result = send_email( + subject=subject, + message=message, + recipient_list=recipients, + ) + + assert result == 1 + assert len(mail.outbox) == 1 + email = mail.outbox[0] + assert email.to == recipients + + +@pytest.mark.django_db +def test_send_email_with_html_message() -> None: + """Test sending email with HTML content. + + Verifies that HTML message is properly included in the email. + """ + subject = "Test Subject" + message = "Plain text message" + html_message = "

HTML message

" + recipient = "test@example.com" + + result = send_email( + subject=subject, + message=message, + recipient_list=[recipient], + html_message=html_message, + ) + + assert result == 1 + assert len(mail.outbox) == 1 + email = mail.outbox[0] + assert email.body == message + assert email.alternatives[0][0] == html_message + assert email.alternatives[0][1] == "text/html" + + +@pytest.mark.parametrize( + "from_email", + [ + "cityinfra@hel.fi", + "custom@example.com", + "another@test.org", + ], +) +@pytest.mark.django_db +def test_send_email_uses_default_from_email(from_email: str) -> None: + """Test that send_email uses DEFAULT_FROM_EMAIL setting. + + Args: + from_email: The email address to use as the sender. + """ + with override_settings(DEFAULT_FROM_EMAIL=from_email): + send_email( + subject="Test", + message="Test", + recipient_list=["test@example.com"], + ) + + assert len(mail.outbox) == 1 + assert mail.outbox[0].from_email == from_email + + +@pytest.mark.django_db +def test_send_email_empty_recipient_list() -> None: + """Test that empty recipient list raises ValueError. + + Verifies that the function validates recipient list is not empty. + """ + with pytest.raises(ValueError, match="No recipient email addresses provided"): + send_email( + subject="Test", + message="Test", + recipient_list=[], + ) + + +@pytest.mark.parametrize( + "invalid_email", + [ + "not-an-email", + "missing@domain", + "@nodomain.com", + "spaces in@email.com", + "injection\nBcc: attacker@evil.com", + "injection\rBcc: attacker@evil.com", + "multiple@@@signs@email.com", + "", + ], +) +@pytest.mark.django_db +def test_send_email_invalid_email_address(invalid_email: str) -> None: + """Test that invalid email addresses raise ValueError. + + Verifies that the function validates email address format to prevent + injection attacks and malformed addresses. + + Args: + invalid_email: Invalid email address to test. + """ + with pytest.raises(ValueError, match="Invalid email address"): + send_email( + subject="Test", + message="Test", + recipient_list=[invalid_email], + ) + + +@pytest.mark.django_db +def test_send_email_invalid_email_sanitized_in_error() -> None: + """Test that error message for invalid email is sanitized. + + Verifies that newlines and carriage returns are removed from + error messages to prevent log injection. + """ + invalid_email = "injection\nBcc: attacker@evil.com" + + with pytest.raises(ValueError) as exc_info: + send_email( + subject="Test", + message="Test", + recipient_list=[invalid_email], + ) + + error_message = str(exc_info.value) + assert "\n" not in error_message + assert "\r" not in error_message + assert "Invalid email address" in error_message + + +@pytest.mark.django_db +def test_send_email_too_many_recipients() -> None: + """Test that exceeding max recipients raises ValueError. + + Verifies that the function enforces maximum recipient limit + to prevent abuse. + """ + # Create 11 recipients (default max is 10) + recipients = [f"test{i}@example.com" for i in range(11)] + + with pytest.raises(ValueError, match="Too many recipients. Maximum allowed: 10"): + send_email( + subject="Test", + message="Test", + recipient_list=recipients, + ) + + +@pytest.mark.django_db +def test_send_email_custom_max_recipients() -> None: + """Test that custom max_recipients parameter works correctly. + + Verifies that the max_recipients parameter can be customized. + """ + # Create 5 recipients with custom max of 3 + recipients = [f"test{i}@example.com" for i in range(5)] + + with pytest.raises(ValueError, match="Too many recipients. Maximum allowed: 3"): + send_email( + subject="Test", + message="Test", + recipient_list=recipients, + max_recipients=3, + ) + + +@pytest.mark.django_db +def test_send_email_custom_max_recipients_allows_more() -> None: + """Test that custom max_recipients allows more than default. + + Verifies that max_recipients can be increased beyond the default of 10. + """ + # Create 15 recipients with custom max of 20 + recipients = [f"test{i}@example.com" for i in range(15)] + + result = send_email( + subject="Test", + message="Test", + recipient_list=recipients, + max_recipients=20, + ) + + assert result == 1 + assert len(mail.outbox) == 1 + assert len(mail.outbox[0].to) == 15 + + +@pytest.mark.django_db +def test_send_email_exactly_max_recipients() -> None: + """Test sending email with exactly max recipients allowed. + + Verifies that the function allows exactly the maximum number. + """ + # Create exactly 10 recipients (default max) + recipients = [f"test{i}@example.com" for i in range(10)] + + result = send_email( + subject="Test", + message="Test", + recipient_list=recipients, + ) + + assert result == 1 + assert len(mail.outbox) == 1 + assert len(mail.outbox[0].to) == 10 + + +@pytest.mark.django_db +def test_send_email_mixed_valid_and_invalid() -> None: + """Test that one invalid email in list fails entire operation. + + Verifies that validation checks all recipients and fails if any are invalid. + """ + recipients = [ + "valid1@example.com", + "invalid-email", + "valid2@example.com", + ] + + with pytest.raises(ValueError, match="Invalid email address"): + send_email( + subject="Test", + message="Test", + recipient_list=recipients, + ) + + # Verify no email was sent + assert len(mail.outbox) == 0