Просмотр исходного кода

Fix email template formatting - convert newlines to br tags and increase header padding

Co-authored-by: cadtoolbox <12723486+cadtoolbox@users.noreply.github.com>
copilot-swe-agent[bot] 3 месяцев назад
Родитель
Сommit
3bbd1eb199
2 измененных файлов с 184 добавлено и 22 удалено
  1. 22 22
      backend/app/services/email_service.py
  2. 162 0
      backend/tests/unit/services/test_email_service.py

+ 22 - 22
backend/app/services/email_service.py

@@ -67,9 +67,7 @@ async def get_notification_template(db: AsyncSession, event_type: str) -> Notifi
     Returns:
         NotificationTemplate object or None if not found
     """
-    result = await db.execute(
-        select(NotificationTemplate).where(NotificationTemplate.event_type == event_type)
-    )
+    result = await db.execute(select(NotificationTemplate).where(NotificationTemplate.event_type == event_type))
     return result.scalar_one_or_none()
 
 
@@ -103,17 +101,19 @@ async def get_smtp_settings(db: AsyncSession) -> SMTPSettings | None:
     # Fetch all SMTP-related settings
     result = await db.execute(
         select(Settings).where(
-            Settings.key.in_([
-                "smtp_host",
-                "smtp_port",
-                "smtp_username",
-                "smtp_password",
-                "smtp_use_tls",
-                "smtp_security",
-                "smtp_auth_enabled",
-                "smtp_from_email",
-                "smtp_from_name",
-            ])
+            Settings.key.in_(
+                [
+                    "smtp_host",
+                    "smtp_port",
+                    "smtp_username",
+                    "smtp_password",
+                    "smtp_use_tls",
+                    "smtp_security",
+                    "smtp_auth_enabled",
+                    "smtp_from_email",
+                    "smtp_from_name",
+                ]
+            )
         )
     )
     settings_dict = {s.key: s.value for s in result.scalars().all()}
@@ -417,8 +417,8 @@ async def create_welcome_email_from_template(
         text_body = render_template(template.body_template, variables)
 
         # Create HTML version with embedded login button
-        # Escape text_body to prevent XSS vulnerabilities
-        escaped_text_body = html.escape(text_body)
+        # Escape text_body to prevent XSS vulnerabilities and convert newlines to <br> tags
+        escaped_text_body = html.escape(text_body).replace("\n", "<br>\n")
         html_body = f"""<!DOCTYPE html>
 <html>
 <head>
@@ -426,11 +426,11 @@ async def create_welcome_email_from_template(
     <meta name="viewport" content="width=device-width, initial-scale=1.0">
 </head>
 <body style="font-family: Arial, sans-serif; line-height: 1.6; color: #333; max-width: 600px; margin: 0 auto; padding: 20px;">
-    <div style="background: linear-gradient(135deg, #667eea 0%, #764ba2 100%); padding: 20px; border-radius: 8px 8px 0 0;">
+    <div style="background: linear-gradient(135deg, #667eea 0%, #764ba2 100%); padding: 30px; border-radius: 8px 8px 0 0;">
         <h1 style="color: white; margin: 0; font-size: 24px;">{html.escape(subject)}</h1>
     </div>
     <div style="background: #f9f9f9; padding: 30px; border-radius: 0 0 8px 8px; border: 1px solid #ddd; border-top: none;">
-        <div style="white-space: pre-wrap; font-size: 16px;">{escaped_text_body}</div>
+        <div style="font-size: 16px;">{escaped_text_body}</div>
 
         <div style="text-align: center; margin: 30px 0;">
             <a href="{login_url}" style="display: inline-block; background: #667eea; color: white; padding: 12px 30px; text-decoration: none; border-radius: 4px; font-weight: bold;">Login Now</a>
@@ -479,8 +479,8 @@ async def create_password_reset_email_from_template(
         text_body = render_template(template.body_template, variables)
 
         # Create HTML version with embedded login button
-        # Escape text_body to prevent XSS vulnerabilities
-        escaped_text_body = html.escape(text_body)
+        # Escape text_body to prevent XSS vulnerabilities and convert newlines to <br> tags
+        escaped_text_body = html.escape(text_body).replace("\n", "<br>\n")
         html_body = f"""<!DOCTYPE html>
 <html>
 <head>
@@ -488,11 +488,11 @@ async def create_password_reset_email_from_template(
     <meta name="viewport" content="width=device-width, initial-scale=1.0">
 </head>
 <body style="font-family: Arial, sans-serif; line-height: 1.6; color: #333; max-width: 600px; margin: 0 auto; padding: 20px;">
-    <div style="background: linear-gradient(135deg, #667eea 0%, #764ba2 100%); padding: 20px; border-radius: 8px 8px 0 0;">
+    <div style="background: linear-gradient(135deg, #667eea 0%, #764ba2 100%); padding: 30px; border-radius: 8px 8px 0 0;">
         <h1 style="color: white; margin: 0; font-size: 24px;">{html.escape(subject)}</h1>
     </div>
     <div style="background: #f9f9f9; padding: 30px; border-radius: 0 0 8px 8px; border: 1px solid #ddd; border-top: none;">
-        <div style="white-space: pre-wrap; font-size: 16px;">{escaped_text_body}</div>
+        <div style="font-size: 16px;">{escaped_text_body}</div>
 
         <div style="text-align: center; margin: 30px 0;">
             <a href="{login_url}" style="display: inline-block; background: #667eea; color: white; padding: 12px 30px; text-decoration: none; border-radius: 4px; font-weight: bold;">Login Now</a>

+ 162 - 0
backend/tests/unit/services/test_email_service.py

@@ -0,0 +1,162 @@
+"""Unit tests for email service.
+
+These tests verify email template rendering and HTML formatting.
+"""
+
+from unittest.mock import AsyncMock, patch
+
+import pytest
+from sqlalchemy.ext.asyncio import AsyncSession
+
+from backend.app.models.notification_template import NotificationTemplate
+from backend.app.services.email_service import (
+    create_password_reset_email_from_template,
+    create_welcome_email_from_template,
+)
+
+
+class TestEmailTemplateFormatting:
+    """Tests for email template formatting."""
+
+    @pytest.mark.asyncio
+    async def test_welcome_email_newlines_converted_to_br(self):
+        """Verify that newlines in welcome email body are converted to <br> tags."""
+        # Mock database session
+        db = AsyncMock(spec=AsyncSession)
+
+        # Mock template with newlines
+        template = NotificationTemplate(
+            event_type="user_created",
+            name="Welcome Email",
+            title_template="Welcome to {app_name}",
+            body_template="Hello {username}!\n\nYour password is: {password}\n\nPlease login at: {login_url}",
+            is_default=True,
+        )
+
+        # Patch get_notification_template to return our template
+        with patch("backend.app.services.email_service.get_notification_template", return_value=template):
+            # Generate email
+            subject, text_body, html_body = await create_welcome_email_from_template(
+                db=db,
+                username="testuser",
+                password="testpass123",
+                login_url="http://example.com/login",
+                app_name="TestApp",
+            )
+
+        # Verify subject
+        assert subject == "Welcome to TestApp"
+
+        # Verify text body has newlines
+        assert "\n\n" in text_body
+        assert "Hello testuser!" in text_body
+        assert "Your password is: testpass123" in text_body
+
+        # Verify HTML body has <br> tags instead of relying on CSS
+        assert "<br>" in html_body
+        # Should not use white-space: pre-wrap
+        assert "white-space: pre-wrap" not in html_body
+        # Should have proper structure
+        assert "<!DOCTYPE html>" in html_body
+        assert '<div style="font-size: 16px;">' in html_body
+
+        # Verify that escaped content is present (XSS protection)
+        assert "Hello testuser!<br>" in html_body
+        assert "Your password is: testpass123<br>" in html_body
+
+    @pytest.mark.asyncio
+    async def test_password_reset_email_newlines_converted_to_br(self):
+        """Verify that newlines in password reset email body are converted to <br> tags."""
+        # Mock database session
+        db = AsyncMock(spec=AsyncSession)
+
+        # Mock template with newlines
+        template = NotificationTemplate(
+            event_type="password_reset",
+            name="Password Reset",
+            title_template="{app_name} - Password Reset",
+            body_template="Hello {username},\n\nYour password has been reset.\nNew password: {password}\n\nLogin at: {login_url}",
+            is_default=True,
+        )
+
+        # Patch get_notification_template to return our template
+        with patch("backend.app.services.email_service.get_notification_template", return_value=template):
+            # Generate email
+            subject, text_body, html_body = await create_password_reset_email_from_template(
+                db=db,
+                username="testuser",
+                password="newpass456",
+                login_url="http://example.com/login",
+                app_name="TestApp",
+            )
+
+        # Verify subject
+        assert subject == "TestApp - Password Reset"
+
+        # Verify text body has newlines
+        assert "\n\n" in text_body
+        assert "Hello testuser," in text_body
+
+        # Verify HTML body has <br> tags
+        assert "<br>" in html_body
+        # Should not use white-space: pre-wrap
+        assert "white-space: pre-wrap" not in html_body
+        # Should have security alert
+        assert "Security Alert" in html_body
+
+    @pytest.mark.asyncio
+    async def test_email_header_padding(self):
+        """Verify that email header has proper padding to prevent cutoff."""
+        # Mock database session
+        db = AsyncMock(spec=AsyncSession)
+
+        # Mock template
+        template = NotificationTemplate(
+            event_type="user_created",
+            name="Welcome Email",
+            title_template="Welcome",
+            body_template="Test body",
+            is_default=True,
+        )
+
+        # Patch get_notification_template to return our template
+        with patch("backend.app.services.email_service.get_notification_template", return_value=template):
+            # Generate email
+            subject, text_body, html_body = await create_welcome_email_from_template(
+                db=db,
+                username="testuser",
+                password="testpass123",
+                login_url="http://example.com/login",
+            )
+
+        # Verify header has 30px padding (not 20px which was cutting off)
+        assert "padding: 30px; border-radius: 8px 8px 0 0;" in html_body
+
+    @pytest.mark.asyncio
+    async def test_email_xss_protection(self):
+        """Verify that HTML escaping is applied to prevent XSS attacks."""
+        # Mock database session
+        db = AsyncMock(spec=AsyncSession)
+
+        # Mock template with potential XSS content
+        template = NotificationTemplate(
+            event_type="user_created",
+            name="Welcome Email",
+            title_template="Welcome <script>alert('xss')</script>",
+            body_template="Hello <script>alert('xss')</script>\nTest",
+            is_default=True,
+        )
+
+        # Patch get_notification_template to return our template
+        with patch("backend.app.services.email_service.get_notification_template", return_value=template):
+            # Generate email
+            subject, text_body, html_body = await create_welcome_email_from_template(
+                db=db,
+                username="testuser",
+                password="testpass123",
+                login_url="http://example.com/login",
+            )
+
+        # Verify that script tags are escaped
+        assert "&lt;script&gt;" in html_body
+        assert "<script>" not in html_body or html_body.count("<script>") == 0  # No unescaped script tags