Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Password change email notification #1791

Merged
merged 2 commits into from
Jan 10, 2025

Conversation

logerzerox
Copy link
Contributor

@logerzerox logerzerox commented Jan 9, 2025

Description

Added email notification when a user changes their password for better security practices in production environments.

Changes

  • Added password change email notification in R2R auth provider
  • Ensures users are notified when their password is changed
  • Uses existing email provider infrastructure

Testing

  • Tested password change notification with SendGrid provider
  • Verified email template data passing correctly
  • Confirmed fallback plain text email works

Important

Adds email notification for password changes using existing email infrastructure across multiple providers.

  • Behavior:
    • Adds send_password_changed_email() method to EmailProvider and its implementations in sendgrid.py, smtp.py, and console_mock.py.
    • Sends email notification when a user changes their password in change_password() and confirm_password_reset() in r2r_auth.py.
    • Uses existing email infrastructure and templates.
  • Configuration:
    • Adds password_changed_template_id to EmailConfig in email.py.
  • Error Handling:
    • Logs errors if email sending fails in r2r_auth.py.
  • Testing:
    • Tested with SendGrid provider, verified email template data passing and fallback plain text email.

This description was created by Ellipsis for 796ae48. It will automatically update as commits are pushed.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Looks good to me! Reviewed everything up to 796ae48 in 58 seconds

More details
  • Looked at 208 lines of code in 5 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. py/core/providers/auth/r2r_auth.py:426
  • Draft comment:
    Consider using a consistent method to extract the first name across different email sending functions. For example, user.name.split(" ")[0] if user.name else 'User' is used in some places, while user.name.split(" ")[0] or 'User' is used here. Ensure consistency for clarity and maintainability.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The code for sending password change notifications is consistent across different email providers. However, there is a minor inconsistency in the way the first name is extracted and used in the dynamic template data. This can be improved for consistency and clarity.
2. py/core/providers/auth/r2r_auth.py:499
  • Draft comment:
    Consider using a consistent method to extract the first name across different email sending functions. For example, user.name.split(" ")[0] if user.name else 'User' is used in some places, while user.name.split(" ")[0] or 'User' is used here. Ensure consistency for clarity and maintainability.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The code for sending password change notifications is consistent across different email providers. However, there is a minor inconsistency in the way the first name is extracted and used in the dynamic template data. This can be improved for consistency and clarity.

Workflow ID: wflow_PVFxGK455QvvDW40


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@logerzerox logerzerox changed the title Send password changed email Password change email notification Jan 9, 2025
@NolanTrem NolanTrem merged commit 3d853e0 into SciPhi-AI:main Jan 10, 2025
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants