add mailer #14
add mailer #14
Conversation
WalkthroughEmail functionality has been integrated into the API. This includes new configuration settings for mail servers, initialization of the Flask-Mail extension, utility functions for sending various types of emails, and new API routes to trigger email sending. Supporting code and dependencies have been added and updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API
participant EmailUtils
participant MailServer
Client->>API: POST /test-email (email type, recipient)
API->>EmailUtils: send_welcome_email / send_password_reset_email / send_email
EmailUtils->>MailServer: Send email via Flask-Mail
MailServer-->>EmailUtils: Delivery status
EmailUtils-->>API: Success/Failure
API-->>Client: JSON response (success or error)
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (2)
mind_matter_api/api/__init__.py (1)
15-17: Email routes initialized correctlyThe email routes are properly integrated into the application. However, I notice that
init_email_routes(app)is called before the survey service is set up, but after the user service. Consider maintaining a consistent order of initializing services and routes.init_user_routes(app) -init_email_routes(app) app.survey_service = SurveyService(SurveyRepository(), SurveyQuestionRepository(), SurveyResponseRepository(), SurveyAnswerRepository()) init_survey_routes(app) +init_email_routes(app)Alternatively, group the service setup before initializing any routes:
# --- wire up your services --- app.user_service = UserService(UserRepository()) +app.survey_service = SurveyService(SurveyRepository(), + SurveyQuestionRepository(), + SurveyResponseRepository(), + SurveyAnswerRepository()) # --- mount your route init functions --- init_user_routes(app) init_email_routes(app) - -app.survey_service = SurveyService(SurveyRepository(), - SurveyQuestionRepository(), - SurveyResponseRepository(), - SurveyAnswerRepository()) init_survey_routes(app)Also applies to: 24-24
mind_matter_api/utils/email.py (1)
1-8: Add missing imports and improve module documentationThe email utility module will need additional imports for the suggested improvements, and could benefit from better documentation.
# -*- coding: utf-8 -*- -"""Email utility functions.""" +"""Email utility functions. + +This module provides functionality for sending various types of emails +through the Flask-Mail extension. It includes both generic email sending +capability and specialized functions for common email types like welcome +emails and password reset emails. +""" import logging +import html +import time from typing import List, Optional, Union from flask import current_app from flask_mail import Message from mind_matter_api.extensions import mail
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
mind_matter_api/api/__init__.py(1 hunks)mind_matter_api/api/email.py(1 hunks)mind_matter_api/api/routes.py(1 hunks)mind_matter_api/app.py(2 hunks)mind_matter_api/extensions.py(2 hunks)mind_matter_api/settings.py(1 hunks)mind_matter_api/utils/email.py(1 hunks)requirements/prod.txt(1 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
mind_matter_api/api/email.py
1-1: flask.Blueprint imported but unused
Remove unused import: flask.Blueprint
(F401)
🔇 Additional comments (3)
mind_matter_api/extensions.py (1)
9-9: Flask-Mail extension properly integratedThe Flask-Mail extension has been correctly added to the extensions module following the established pattern of other extensions in this project.
Also applies to: 22-22
mind_matter_api/app.py (1)
25-25: Mail extension properly initializedThe mail extension is correctly imported and initialized in the Flask application following the established pattern for other extensions.
Also applies to: 91-91
mind_matter_api/api/__init__.py (1)
1-31:Details
✅ Verification successful
Verify email implementation completeness
The email-related imports and route initialization have been added, but the PR lacks the core email implementation files that should be reviewed as well, such as:
mind_matter_api/api/email.py- containing route definitionsmind_matter_api/utils/email.py- containing email utility functionsmind_matter_api/settings.py- containing mail server configurations
🏁 Script executed:
#!/bin/bash # Check for the existence and content of email implementation files # Check email API file if [ -f "mind_matter_api/api/email.py" ]; then echo "Email API file exists. Contents:" cat "mind_matter_api/api/email.py" else echo "Email API file is missing!" fi # Check email utils if [ -f "mind_matter_api/utils/email.py" ]; then echo "Email utils file exists. Contents:" cat "mind_matter_api/utils/email.py" else echo "Email utils file is missing!" fi # Check settings for mail configuration if [ -f "mind_matter_api/settings.py" ]; then echo "Settings file exists. Mail-related configurations:" grep -i "mail\|smtp\|email" "mind_matter_api/settings.py" else echo "Settings file is missing!" fi # Check for possible duplicate routes if [ -f "mind_matter_api/api/routes.py" ]; then echo "Routes file exists. Email-related routes:" grep -i "email\|mail" "mind_matter_api/api/routes.py" else echo "Routes file does not exist." fiLength of output: 7589
Email implementation files verified
All expected email modules are present and appear correctly implemented:
mind_matter_api/api/email.py: definesinit_email_routeswith the/test-emailendpoint and proper error handling.mind_matter_api/utils/email.py: providessend_email,send_welcome_email, andsend_password_reset_emailusing Flask-Mail.mind_matter_api/settings.py: includes allMAIL_*SMTP configurations.No missing email files detected; implementation is complete.
| WebTest==3.0.2 | ||
| flasgger | ||
|
|
||
| Flask-Mail |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Specify version for Flask-Mail dependency
Unlike other dependencies in this file which have pinned versions, Flask-Mail is added without a version constraint. This could lead to unexpected behavior if future versions introduce breaking changes.
-Flask-Mail
+Flask-Mail==0.9.1🌐 Web query:
What is the latest stable version of Flask-Mail?
💡 Result:
The latest stable version of Flask-Mail is 0.10.0, released on May 23, 2024. (flask-mail.readthedocs.io) This release introduced several significant changes, including:
-
Dropped support for Python versions earlier than 3.8: Ensuring compatibility with more recent Python releases.
-
Adoption of
pyproject.tomlfor packaging metadata: Aligning with modern Python packaging standards. -
Transition to
flit_coreas the build backend: Streamlining the build process. -
Implementation of code formatting and linting tools: Enhancing code quality and consistency.
-
Addition of static type annotations: Improving code readability and maintainability.
-
Deprecation of the
__version__attribute: Encouraging the use of feature detection orimportlib.metadata.version("flask-mail")for version retrieval. -
Indication that the deprecated
is_bad_headerswill be removed in the next version: Preparing users for upcoming changes. -
Fix to the
email_dispatchedsignal: Ensuring the current app is passed as the sender and the message as an argument, rather than the other way around. -
Enhancements to the
Attachmentclass: AllowingAttachment.datato beNoneand enabling automatic detection ofAttachment.content_typebased onfilenameanddata.
For a comprehensive list of changes, you can refer to the Flask-Mail changelog. (flask-mail.readthedocs.io)
Citations:
- 1: https://flask-mail.readthedocs.io/en/latest/changes/?utm_source=openai
- 2: https://flask-mail.readthedocs.io/en/latest/changes/?utm_source=openai
Pin Flask-Mail to a specific version
To avoid unexpected breaking changes, the Flask-Mail dependency should be version-pinned like the other packages in requirements/prod.txt.
• File: requirements/prod.txt
• Line: 54
Suggested diff:
-Flask-Mail
+Flask-Mail==0.10.0([Latest stable release is 0.10.0, May 23 2024][1])
Citations:
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Flask-Mail | |
| Flask-Mail==0.10.0 |
| # Mail settings | ||
| MAIL_SERVER = env.str("MAIL_SERVER", default="smtp.gmail.com") | ||
| MAIL_PORT = env.int("MAIL_PORT", default=587) | ||
| MAIL_USE_TLS = env.bool("MAIL_USE_TLS", default=True) | ||
| MAIL_USERNAME = env.str("MAIL_USERNAME", default="") | ||
| MAIL_PASSWORD = env.str("MAIL_PASSWORD", default="") | ||
| MAIL_DEFAULT_SENDER = env.str("MAIL_DEFAULT_SENDER", default="") | ||
| MAIL_MAX_EMAILS = env.int("MAIL_MAX_EMAILS", default=None) | ||
| MAIL_ASCII_ATTACHMENTS = env.bool("MAIL_ASCII_ATTACHMENTS", default=False) | ||
| MAIL_SUPPRESS_SEND = env.bool("MAIL_SUPPRESS_SEND", default=False) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider additional security measures for mail configuration
The mail settings are configured with empty string defaults for credentials and sender address, which is appropriate for development but could lead to issues in production:
- Empty credentials in production might cause authentication failures
- Missing default sender could cause email sending to fail
- Gmail specifically may require app passwords for less secure apps
Consider adding validation logic that warns when using empty credentials in production mode:
+# Mail settings
MAIL_SERVER = env.str("MAIL_SERVER", default="smtp.gmail.com")
MAIL_PORT = env.int("MAIL_PORT", default=587)
MAIL_USE_TLS = env.bool("MAIL_USE_TLS", default=True)
MAIL_USERNAME = env.str("MAIL_USERNAME", default="")
MAIL_PASSWORD = env.str("MAIL_PASSWORD", default="")
-MAIL_DEFAULT_SENDER = env.str("MAIL_DEFAULT_SENDER", default="")
+MAIL_DEFAULT_SENDER = env.str("MAIL_DEFAULT_SENDER", default="noreply@mindmatter.app")
MAIL_MAX_EMAILS = env.int("MAIL_MAX_EMAILS", default=None)
MAIL_ASCII_ATTACHMENTS = env.bool("MAIL_ASCII_ATTACHMENTS", default=False)
MAIL_SUPPRESS_SEND = env.bool("MAIL_SUPPRESS_SEND", default=False)
+
+# Warn if mail credentials are empty in production
+if ENV == "production" and (not MAIL_USERNAME or not MAIL_PASSWORD):
+ import warnings
+ warnings.warn("Mail credentials are not set in production environment")Committable suggestion skipped: line range outside the PR's diff.
| @@ -0,0 +1,37 @@ | |||
| from flask import Blueprint, jsonify, request | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unused Blueprint import
The Blueprint import is not being used in this file as routes are being registered directly on the app object.
-from flask import Blueprint, jsonify, request
+from flask import jsonify, request📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from flask import Blueprint, jsonify, request | |
| -from flask import Blueprint, jsonify, request | |
| +from flask import jsonify, request |
🧰 Tools
🪛 Ruff (0.8.2)
1-1: flask.Blueprint imported but unused
Remove unused import: flask.Blueprint
(F401)
| def init_email_routes(app): | ||
| """Initialize email-related API routes.""" | ||
|
|
||
| @app.route('/test-email', methods=['POST']) | ||
| def test_email(): | ||
| """Test endpoint for sending emails.""" | ||
| try: | ||
| data = request.get_json() | ||
| email_type = data.get('type', 'general') | ||
| to_email = data.get('to') | ||
|
|
||
| if not to_email: | ||
| return jsonify({'error': 'Email address is required'}), 400 | ||
|
|
||
| if email_type == 'welcome': | ||
| success = send_welcome_email(to=to_email, name='Test User') | ||
| elif email_type == 'reset': | ||
| success = send_password_reset_email(to=to_email, reset_token='test-token-123') | ||
| else: | ||
| # Send general test email | ||
| success = send_email( | ||
| to=to_email, | ||
| subject='Test Email from Mind Matter', | ||
| html='<h1>Test Email</h1><p>This is a test email from Mind Matter API.</p>', | ||
| text='Test Email\nThis is a test email from Mind Matter API.' | ||
| ) | ||
|
|
||
| if success: | ||
| return jsonify({'message': f'Test {email_type} email sent successfully'}), 200 | ||
| else: | ||
| return jsonify({'error': 'Failed to send email'}), 500 | ||
|
|
||
| except Exception as e: |
There was a problem hiding this comment.
Code duplication with routes.py detected
This file defines a test email endpoint that appears to be duplicated in mind_matter_api/api/routes.py. Having the same functionality in two places will lead to maintenance issues.
Choose one approach for route registration:
- Keep this implementation if you want to register routes directly on the app
- Use the Blueprint approach from routes.py if you prefer modular organization
Remove the duplicate implementation once you've decided which to keep.
Also, consider securing this test endpoint:
@app.route('/test-email', methods=['POST'])
def test_email():
"""Test endpoint for sending emails."""
+ # Only enable this endpoint in development mode
+ if not app.debug:
+ return jsonify({'error': 'This endpoint is only available in development mode'}), 403
try:
data = request.get_json()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def init_email_routes(app): | |
| """Initialize email-related API routes.""" | |
| @app.route('/test-email', methods=['POST']) | |
| def test_email(): | |
| """Test endpoint for sending emails.""" | |
| try: | |
| data = request.get_json() | |
| email_type = data.get('type', 'general') | |
| to_email = data.get('to') | |
| if not to_email: | |
| return jsonify({'error': 'Email address is required'}), 400 | |
| if email_type == 'welcome': | |
| success = send_welcome_email(to=to_email, name='Test User') | |
| elif email_type == 'reset': | |
| success = send_password_reset_email(to=to_email, reset_token='test-token-123') | |
| else: | |
| # Send general test email | |
| success = send_email( | |
| to=to_email, | |
| subject='Test Email from Mind Matter', | |
| html='<h1>Test Email</h1><p>This is a test email from Mind Matter API.</p>', | |
| text='Test Email\nThis is a test email from Mind Matter API.' | |
| ) | |
| if success: | |
| return jsonify({'message': f'Test {email_type} email sent successfully'}), 200 | |
| else: | |
| return jsonify({'error': 'Failed to send email'}), 500 | |
| except Exception as e: | |
| def init_email_routes(app): | |
| """Initialize email-related API routes.""" | |
| @app.route('/test-email', methods=['POST']) | |
| def test_email(): | |
| """Test endpoint for sending emails.""" | |
| # Only enable this endpoint in development mode | |
| if not app.debug: | |
| return jsonify({'error': 'This endpoint is only available in development mode'}), 403 | |
| try: | |
| data = request.get_json() | |
| email_type = data.get('type', 'general') | |
| to_email = data.get('to') | |
| if not to_email: | |
| return jsonify({'error': 'Email address is required'}), 400 | |
| if email_type == 'welcome': | |
| success = send_welcome_email(to=to_email, name='Test User') | |
| elif email_type == 'reset': | |
| success = send_password_reset_email(to=to_email, reset_token='test-token-123') | |
| else: | |
| # Send general test email | |
| success = send_email( | |
| to=to_email, | |
| subject='Test Email from Mind Matter', | |
| html='<h1>Test Email</h1><p>This is a test email from Mind Matter API.</p>', | |
| text='Test Email\nThis is a test email from Mind Matter API.' | |
| ) | |
| if success: | |
| return jsonify({'message': f'Test {email_type} email sent successfully'}), 200 | |
| else: | |
| return jsonify({'error': 'Failed to send email'}), 500 | |
| except Exception as e: | |
| # existing exception handling... | |
| raise |
| from flask import Blueprint, jsonify, request | ||
| from mind_matter_api.utils.email import send_email, send_welcome_email, send_password_reset_email | ||
|
|
||
| api = Blueprint('api', __name__) | ||
|
|
||
| @api.route('/test-email', methods=['POST']) | ||
| def test_email(): | ||
| """Test endpoint for sending emails.""" | ||
| try: | ||
| data = request.get_json() | ||
| email_type = data.get('type', 'general') | ||
| to_email = data.get('to') | ||
|
|
||
| if not to_email: | ||
| return jsonify({'error': 'Email address is required'}), 400 | ||
|
|
||
| if email_type == 'welcome': | ||
| success = send_welcome_email(to=to_email, name='Test User') | ||
| elif email_type == 'reset': | ||
| success = send_password_reset_email(to=to_email, reset_token='test-token-123') | ||
| else: | ||
| # Send general test email | ||
| success = send_email( | ||
| to=to_email, | ||
| subject='Test Email from Mind Matter', | ||
| html='<h1>Test Email</h1><p>This is a test email from Mind Matter API.</p>', | ||
| text='Test Email\nThis is a test email from Mind Matter API.' | ||
| ) | ||
|
|
||
| if success: | ||
| return jsonify({'message': f'Test {email_type} email sent successfully'}), 200 | ||
| else: | ||
| return jsonify({'error': 'Failed to send email'}), 500 | ||
|
|
||
| except Exception as e: | ||
| return jsonify({'error': str(e)}), 500 No newline at end of file |
There was a problem hiding this comment.
Duplicate email test endpoint with different implementation approach
This file implements the same test email functionality as mind_matter_api/api/email.py but uses a Blueprint approach instead of direct route registration. This duplication creates confusion and maintenance challenges.
Decide on a consistent route registration approach:
-
If using Blueprints throughout the application:
- Keep this implementation
- Remove the duplicate in email.py
- Update the initialization code to register this Blueprint instead of calling
init_email_routes
-
If registering routes directly on the app:
- Remove this implementation
- Keep the version in email.py
Also, consider adding security restrictions to this test endpoint:
@api.route('/test-email', methods=['POST'])
def test_email():
"""Test endpoint for sending emails."""
+ # Only enable this endpoint in development mode
+ from flask import current_app
+ if not current_app.debug:
+ return jsonify({'error': 'This endpoint is only available in development mode'}), 403
try:
data = request.get_json()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from flask import Blueprint, jsonify, request | |
| from mind_matter_api.utils.email import send_email, send_welcome_email, send_password_reset_email | |
| api = Blueprint('api', __name__) | |
| @api.route('/test-email', methods=['POST']) | |
| def test_email(): | |
| """Test endpoint for sending emails.""" | |
| try: | |
| data = request.get_json() | |
| email_type = data.get('type', 'general') | |
| to_email = data.get('to') | |
| if not to_email: | |
| return jsonify({'error': 'Email address is required'}), 400 | |
| if email_type == 'welcome': | |
| success = send_welcome_email(to=to_email, name='Test User') | |
| elif email_type == 'reset': | |
| success = send_password_reset_email(to=to_email, reset_token='test-token-123') | |
| else: | |
| # Send general test email | |
| success = send_email( | |
| to=to_email, | |
| subject='Test Email from Mind Matter', | |
| html='<h1>Test Email</h1><p>This is a test email from Mind Matter API.</p>', | |
| text='Test Email\nThis is a test email from Mind Matter API.' | |
| ) | |
| if success: | |
| return jsonify({'message': f'Test {email_type} email sent successfully'}), 200 | |
| else: | |
| return jsonify({'error': 'Failed to send email'}), 500 | |
| except Exception as e: | |
| return jsonify({'error': str(e)}), 500 | |
| from flask import Blueprint, jsonify, request | |
| from mind_matter_api.utils.email import send_email, send_welcome_email, send_password_reset_email | |
| api = Blueprint('api', __name__) | |
| @api.route('/test-email', methods=['POST']) | |
| def test_email(): | |
| """Test endpoint for sending emails.""" | |
| # Only enable this endpoint in development mode | |
| from flask import current_app | |
| if not current_app.debug: | |
| return jsonify({'error': 'This endpoint is only available in development mode'}), 403 | |
| try: | |
| data = request.get_json() | |
| email_type = data.get('type', 'general') | |
| to_email = data.get('to') | |
| if not to_email: | |
| return jsonify({'error': 'Email address is required'}), 400 | |
| if email_type == 'welcome': | |
| success = send_welcome_email(to=to_email, name='Test User') | |
| elif email_type == 'reset': | |
| success = send_password_reset_email(to=to_email, reset_token='test-token-123') | |
| else: | |
| # Send general test email | |
| success = send_email( | |
| to=to_email, | |
| subject='Test Email from Mind Matter', | |
| html='<h1>Test Email</h1><p>This is a test email from Mind Matter API.</p>', | |
| text='Test Email\nThis is a test email from Mind Matter API.' | |
| ) | |
| if success: | |
| return jsonify({'message': f'Test {email_type} email sent successfully'}), 200 | |
| else: | |
| return jsonify({'error': 'Failed to send email'}), 500 | |
| except Exception as e: | |
| return jsonify({'error': str(e)}), 500 |
| def send_welcome_email(to: str, name: str) -> bool: | ||
| """Send a welcome email to new users. | ||
| Args: | ||
| to: Email address of the recipient | ||
| name: Name of the recipient | ||
| Returns: | ||
| bool: True if email was sent successfully, False otherwise | ||
| """ | ||
| html = f""" | ||
| <h1>Welcome to Mind Matter!</h1> | ||
| <p>Hello {name},</p> | ||
| <p>Thank you for joining Mind Matter. We're excited to have you on board!</p> | ||
| <p>You can now log in to your account and start exploring our features.</p> | ||
| """ | ||
|
|
||
| text = f""" | ||
| Welcome to Mind Matter! | ||
| Hello {name}, | ||
| Thank you for joining Mind Matter. We're excited to have you on board! | ||
| You can now log in to your account and start exploring our features. | ||
| """ | ||
|
|
||
| return send_email( | ||
| to=to, | ||
| subject="Welcome to Mind Matter!", | ||
| html=html, | ||
| text=text | ||
| ) No newline at end of file |
There was a problem hiding this comment.
Protect against HTML injection in welcome email
The welcome email function doesn't sanitize the user's name, which could allow HTML injection if the name contains HTML tags.
Add proper escaping of the user's name:
def send_welcome_email(to: str, name: str) -> bool:
"""Send a welcome email to new users.
Args:
to: Email address of the recipient
name: Name of the recipient
Returns:
bool: True if email was sent successfully, False otherwise
"""
+ # Sanitize name to prevent HTML injection
+ import html
+ sanitized_name = html.escape(name) if name else "there"
+
html = f"""
<h1>Welcome to Mind Matter!</h1>
- <p>Hello {name},</p>
+ <p>Hello {sanitized_name},</p>
<p>Thank you for joining Mind Matter. We're excited to have you on board!</p>
<p>You can now log in to your account and start exploring our features.</p>
"""
text = f"""
Welcome to Mind Matter!
- Hello {name},
+ Hello {name if name else "there"},
Thank you for joining Mind Matter. We're excited to have you on board!
You can now log in to your account and start exploring our features.
"""📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def send_welcome_email(to: str, name: str) -> bool: | |
| """Send a welcome email to new users. | |
| Args: | |
| to: Email address of the recipient | |
| name: Name of the recipient | |
| Returns: | |
| bool: True if email was sent successfully, False otherwise | |
| """ | |
| html = f""" | |
| <h1>Welcome to Mind Matter!</h1> | |
| <p>Hello {name},</p> | |
| <p>Thank you for joining Mind Matter. We're excited to have you on board!</p> | |
| <p>You can now log in to your account and start exploring our features.</p> | |
| """ | |
| text = f""" | |
| Welcome to Mind Matter! | |
| Hello {name}, | |
| Thank you for joining Mind Matter. We're excited to have you on board! | |
| You can now log in to your account and start exploring our features. | |
| """ | |
| return send_email( | |
| to=to, | |
| subject="Welcome to Mind Matter!", | |
| html=html, | |
| text=text | |
| ) | |
| def send_welcome_email(to: str, name: str) -> bool: | |
| """Send a welcome email to new users. | |
| Args: | |
| to: Email address of the recipient | |
| name: Name of the recipient | |
| Returns: | |
| bool: True if email was sent successfully, False otherwise | |
| """ | |
| # Sanitize name to prevent HTML injection | |
| import html | |
| sanitized_name = html.escape(name) if name else "there" | |
| html = f""" | |
| <h1>Welcome to Mind Matter!</h1> | |
| <p>Hello {sanitized_name},</p> | |
| <p>Thank you for joining Mind Matter. We're excited to have you on board!</p> | |
| <p>You can now log in to your account and start exploring our features.</p> | |
| """ | |
| text = f""" | |
| Welcome to Mind Matter! | |
| Hello {name if name else "there"}, | |
| Thank you for joining Mind Matter. We're excited to have you on board! | |
| You can now log in to your account and start exploring our features. | |
| """ | |
| return send_email( | |
| to=to, | |
| subject="Welcome to Mind Matter!", | |
| html=html, | |
| text=text | |
| ) |
| def send_password_reset_email(to: str, reset_token: str) -> bool: | ||
| """Send a password reset email. | ||
| Args: | ||
| to: Email address of the recipient | ||
| reset_token: Password reset token | ||
| Returns: | ||
| bool: True if email was sent successfully, False otherwise | ||
| """ | ||
| reset_url = f"{current_app.config.get('FRONTEND_URL', '')}/reset-password?token={reset_token}" | ||
|
|
||
| html = f""" | ||
| <h1>Password Reset Request</h1> | ||
| <p>You have requested to reset your password. Click the link below to proceed:</p> | ||
| <p><a href="{reset_url}">Reset Password</a></p> | ||
| <p>If you did not request this, please ignore this email.</p> | ||
| <p>This link will expire in 1 hour.</p> | ||
| """ | ||
|
|
||
| text = f""" | ||
| Password Reset Request | ||
| You have requested to reset your password. Visit the link below to proceed: | ||
| {reset_url} | ||
| If you did not request this, please ignore this email. | ||
| This link will expire in 1 hour. | ||
| """ | ||
|
|
||
| return send_email( | ||
| to=to, | ||
| subject="Password Reset Request", | ||
| html=html, | ||
| text=text | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance password reset email security and validation
The password reset email function needs improved validation and security measures.
Consider these security enhancements:
- Validate the reset token format
- Make the token expiration time configurable
- Add security notices about not sharing the link
def send_password_reset_email(to: str, reset_token: str) -> bool:
"""Send a password reset email.
Args:
to: Email address of the recipient
reset_token: Password reset token
Returns:
bool: True if email was sent successfully, False otherwise
"""
+ # Validate reset token format
+ if not reset_token or not isinstance(reset_token, str) or len(reset_token) < 10:
+ logger.error("Invalid reset token format")
+ return False
+
+ # Get configurable expiration time
+ token_expiry_hours = current_app.config.get('PASSWORD_RESET_EXPIRY_HOURS', 1)
+
reset_url = f"{current_app.config.get('FRONTEND_URL', '')}/reset-password?token={reset_token}"
html = f"""
<h1>Password Reset Request</h1>
<p>You have requested to reset your password. Click the link below to proceed:</p>
<p><a href="{reset_url}">Reset Password</a></p>
<p>If you did not request this, please ignore this email.</p>
- <p>This link will expire in 1 hour.</p>
+ <p>This link will expire in {token_expiry_hours} hour{'s' if token_expiry_hours != 1 else ''}.</p>
+ <p>For security reasons, please do not share this link with anyone.</p>
+ <p>If you have any concerns about this email, please contact support.</p>
"""
text = f"""
Password Reset Request
You have requested to reset your password. Visit the link below to proceed:
{reset_url}
If you did not request this, please ignore this email.
- This link will expire in 1 hour.
+ This link will expire in {token_expiry_hours} hour{'s' if token_expiry_hours != 1 else ''}.
+ For security reasons, please do not share this link with anyone.
+ If you have any concerns about this email, please contact support.
"""Committable suggestion skipped: line range outside the PR's diff.
| def send_email( | ||
| to: Union[str, List[str]], | ||
| subject: str, | ||
| html: str, | ||
| text: Optional[str] = None, | ||
| cc: Optional[Union[str, List[str]]] = None, | ||
| bcc: Optional[Union[str, List[str]]] = None, | ||
| attachments: Optional[List[tuple]] = None | ||
| ) -> bool: | ||
| """Send an email using Flask-Mail. | ||
| Args: | ||
| to: Email address(es) of the recipient(s) | ||
| subject: Subject of the email | ||
| html: HTML content of the email | ||
| text: Plain text content of the email (optional) | ||
| cc: Email address(es) to CC (optional) | ||
| bcc: Email address(es) to BCC (optional) | ||
| attachments: List of attachments as (filename, content_type, data) tuples (optional) | ||
| Returns: | ||
| bool: True if email was sent successfully, False otherwise | ||
| """ | ||
| try: | ||
| msg = Message( | ||
| subject=subject, | ||
| recipients=[to] if isinstance(to, str) else to, | ||
| html=html, | ||
| body=text, | ||
| cc=[cc] if isinstance(cc, str) else cc, | ||
| bcc=[bcc] if isinstance(bcc, str) else bcc, | ||
| attachments=attachments, | ||
| sender=current_app.config['MAIL_DEFAULT_SENDER'] | ||
| ) | ||
|
|
||
| mail.send(msg) | ||
| logger.info(f"Email sent successfully to {to}") | ||
| return True | ||
|
|
||
| except Exception as e: | ||
| logger.error(f"Failed to send email to {to}: {str(e)}") | ||
| return False | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve security and add rate limiting to send_email
The email sending function has good error handling and type hints, but could be improved for security and reliability.
Consider these enhancements:
- Mask email addresses in logs for privacy
- Add rate limiting to prevent abuse
- Implement retries for transient failures
def send_email(
to: Union[str, List[str]],
subject: str,
html: str,
text: Optional[str] = None,
cc: Optional[Union[str, List[str]]] = None,
bcc: Optional[Union[str, List[str]]] = None,
- attachments: Optional[List[tuple]] = None
+ attachments: Optional[List[tuple]] = None,
+ retry_attempts: int = 3
) -> bool:
"""Send an email using Flask-Mail.
Args:
to: Email address(es) of the recipient(s)
subject: Subject of the email
html: HTML content of the email
text: Plain text content of the email (optional)
cc: Email address(es) to CC (optional)
bcc: Email address(es) to BCC (optional)
attachments: List of attachments as (filename, content_type, data) tuples (optional)
+ retry_attempts: Number of retry attempts for transient failures (default: 3)
Returns:
bool: True if email was sent successfully, False otherwise
"""
+ # Simple function to mask email for logging
+ def mask_email(email: str) -> str:
+ if '@' not in email:
+ return email
+ username, domain = email.split('@', 1)
+ if len(username) <= 2:
+ masked_username = '*' * len(username)
+ else:
+ masked_username = username[0] + '*' * (len(username) - 2) + username[-1]
+ return f"{masked_username}@{domain}"
+
+ # Get recipient list for logging
+ recipients = [to] if isinstance(to, str) else to
+ masked_recipients = [mask_email(r) for r in recipients]
+
try:
+ # Rate limiting check (example implementation)
+ if hasattr(current_app, 'email_rate_limiter') and not current_app.email_rate_limiter.is_allowed():
+ logger.warning(f"Rate limit exceeded for email to {', '.join(masked_recipients)}")
+ return False
+
msg = Message(
subject=subject,
- recipients=[to] if isinstance(to, str) else to,
+ recipients=recipients,
html=html,
body=text,
cc=[cc] if isinstance(cc, str) else cc,
bcc=[bcc] if isinstance(bcc, str) else bcc,
attachments=attachments,
sender=current_app.config['MAIL_DEFAULT_SENDER']
)
- mail.send(msg)
- logger.info(f"Email sent successfully to {to}")
- return True
+ # Implement retry logic for transient failures
+ attempt = 0
+ while attempt < retry_attempts:
+ try:
+ mail.send(msg)
+ logger.info(f"Email sent successfully to {', '.join(masked_recipients)}")
+ return True
+ except Exception as retry_error:
+ attempt += 1
+ if attempt >= retry_attempts:
+ raise retry_error
+ logger.warning(f"Retry {attempt}/{retry_attempts} after email send failure: {str(retry_error)}")
+ time.sleep(2 ** attempt) # Exponential backoff
except Exception as e:
- logger.error(f"Failed to send email to {to}: {str(e)}")
+ logger.error(f"Failed to send email to {', '.join(masked_recipients)}: {str(e)}")
return FalseNote: You'll need to add import time at the top of the file for the retry logic.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def send_email( | |
| to: Union[str, List[str]], | |
| subject: str, | |
| html: str, | |
| text: Optional[str] = None, | |
| cc: Optional[Union[str, List[str]]] = None, | |
| bcc: Optional[Union[str, List[str]]] = None, | |
| attachments: Optional[List[tuple]] = None | |
| ) -> bool: | |
| """Send an email using Flask-Mail. | |
| Args: | |
| to: Email address(es) of the recipient(s) | |
| subject: Subject of the email | |
| html: HTML content of the email | |
| text: Plain text content of the email (optional) | |
| cc: Email address(es) to CC (optional) | |
| bcc: Email address(es) to BCC (optional) | |
| attachments: List of attachments as (filename, content_type, data) tuples (optional) | |
| Returns: | |
| bool: True if email was sent successfully, False otherwise | |
| """ | |
| try: | |
| msg = Message( | |
| subject=subject, | |
| recipients=[to] if isinstance(to, str) else to, | |
| html=html, | |
| body=text, | |
| cc=[cc] if isinstance(cc, str) else cc, | |
| bcc=[bcc] if isinstance(bcc, str) else bcc, | |
| attachments=attachments, | |
| sender=current_app.config['MAIL_DEFAULT_SENDER'] | |
| ) | |
| mail.send(msg) | |
| logger.info(f"Email sent successfully to {to}") | |
| return True | |
| except Exception as e: | |
| logger.error(f"Failed to send email to {to}: {str(e)}") | |
| return False | |
| # At the top of the file | |
| import time | |
| def send_email( | |
| to: Union[str, List[str]], | |
| subject: str, | |
| html: str, | |
| text: Optional[str] = None, | |
| cc: Optional[Union[str, List[str]]] = None, | |
| bcc: Optional[Union[str, List[str]]] = None, | |
| attachments: Optional[List[tuple]] = None, | |
| retry_attempts: int = 3 | |
| ) -> bool: | |
| """Send an email using Flask-Mail. | |
| Args: | |
| to: Email address(es) of the recipient(s) | |
| subject: Subject of the email | |
| html: HTML content of the email | |
| text: Plain text content of the email (optional) | |
| cc: Email address(es) to CC (optional) | |
| bcc: Email address(es) to BCC (optional) | |
| attachments: List of attachments as (filename, content_type, data) tuples (optional) | |
| retry_attempts: Number of retry attempts for transient failures (default: 3) | |
| Returns: | |
| bool: True if email was sent successfully, False otherwise | |
| """ | |
| # Simple function to mask email for logging | |
| def mask_email(email: str) -> str: | |
| if '@' not in email: | |
| return email | |
| username, domain = email.split('@', 1) | |
| if len(username) <= 2: | |
| masked_username = '*' * len(username) | |
| else: | |
| masked_username = username[0] + '*' * (len(username) - 2) + username[-1] | |
| return f"{masked_username}@{domain}" | |
| recipients = [to] if isinstance(to, str) else to | |
| masked_recipients = [mask_email(r) for r in recipients] | |
| try: | |
| # Rate limiting check (example implementation) | |
| if hasattr(current_app, 'email_rate_limiter') and not current_app.email_rate_limiter.is_allowed(): | |
| logger.warning(f"Rate limit exceeded for email to {', '.join(masked_recipients)}") | |
| return False | |
| msg = Message( | |
| subject=subject, | |
| recipients=recipients, | |
| html=html, | |
| body=text, | |
| cc=[cc] if isinstance(cc, str) else cc, | |
| bcc=[bcc] if isinstance(bcc, str) else bcc, | |
| attachments=attachments, | |
| sender=current_app.config['MAIL_DEFAULT_SENDER'] | |
| ) | |
| # Implement retry logic for transient failures | |
| attempt = 0 | |
| while attempt < retry_attempts: | |
| try: | |
| mail.send(msg) | |
| logger.info(f"Email sent successfully to {', '.join(masked_recipients)}") | |
| return True | |
| except Exception as retry_error: | |
| attempt += 1 | |
| if attempt >= retry_attempts: | |
| raise retry_error | |
| logger.warning(f"Retry {attempt}/{retry_attempts} after email send failure: {retry_error}") | |
| time.sleep(2 ** attempt) | |
| except Exception as e: | |
| logger.error(f"Failed to send email to {', '.join(masked_recipients)}: {e}") | |
| return False |
Description
Brief summary of the changes and any relevant context (design doc, screenshots of bugs...).
Initial capacity for mailer
Fixes # (issue)
Changes
Testing
Describe tests run to verify changes. If manual testing has been done, provide instructions to reproduce.
Checklist:
Summary by CodeRabbit