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

[EMAIL] Improve email sending reliability and security #307

Merged
merged 5 commits into from
Apr 10, 2024

Conversation

delano
Copy link
Collaborator

@delano delano commented Apr 10, 2024

User description

This pull request fixes the issue where emails are not being sent. It includes several improvements to the email functionality, such as wrapping URLs in email templates with HTML tags to make them clickable, improving log handling, fixing the reset password request logic, and addressing the email from address. These changes should ensure that emails are sent successfully and improve the overall email functionality.

Fixes #280, #285


Type

Bug fix


Changes walkthrough


PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

Relevant files
Error handling
base.rb
Improve error handling and logging in API                               

lib/onetime/app/api/base.rb

  • Removed verbose debug logging of locale information
  • Improved error handling for rate limiting exceptions
  • +0/-1     
    helpers.rb
    Enhance rate limiting error handling and logging                 

    lib/onetime/app/helpers.rb

  • Obscured customer identifiers in rate limiting logs using a hashing
    function
  • Replaced verbose debug logging with targeted error handling
  • Improved error messages for rate limiting exceptions
  • +2/-2     

    This change wraps all URLs in email templates with HTML <a> tags to make them clickable and improve usability for users accessing these emails across different mail clients. Links were added to all occurrences of base and internal URIs in welcome, password reset, secret sharing and test emails. This should help avoid confusion and allow direct navigation from the email content.
    
    Signed-off-by: delano <delano@onetimesecret.com>
    - Obscure customer identifiers in rate limiting logs using a hashing
    function
    - Replace verbose debug logging with targeted error handling
    - Improve error messages for rate limiting exceptions
    
    Signed-off-by: delano <delano@onetimesecret.com>
    Updates the email sending logic to extract the configuration into variables for easier maintenance and customization. Also wraps the delivery in a begin/rescue block to capture any errors. This avoids unintentionally failing silently and ensures proper reporting when emails cannot be sent.
    
    Signed-off-by: delano <delano@onetimesecret.com>
    Email not working #280
    E-mails are not being sent #285
    
    - Avoid delivery failures by complying with sending platform
    requirements
    - Add reply-to address
    - Improve traceability with logging of from, to, subject at start of
    send
    - Facilitate troubleshooting by logging specific error types separately
    
    Signed-off-by: delano <delano@onetimesecret.com>
    @delano delano self-assigned this Apr 10, 2024
    @delano delano added security ruby Pull requests that update Ruby code maintenance ux Related to user experience Bug fix ops Operational issues labels Apr 10, 2024
    @delano delano marked this pull request as ready for review April 10, 2024 18:39
    @onetimesecret
    Copy link
    Owner

    PR Description updated to latest commit (0b514bc)

    @onetimesecret
    Copy link
    Owner

    PR Review

    ⏱️ Estimated effort to review [1-5]

    3, because the PR includes several changes across multiple files, including error handling, logging, email sending, and email template updates. The changes seem focused on improving the reliability and security of the email functionality, which is an important part of the application.

    🧪 Relevant tests

    No

    🔍 Possible issues

    No

    🔒 Security concerns

    No

    Signed-off-by: delano <delano@onetimesecret.com>
    Copy link
    Collaborator Author

    @delano delano left a comment

    Choose a reason for hiding this comment

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

    lib/onetime/app/api/base.rb Show resolved Hide resolved
    @delano delano merged commit d436eb1 into rel/0.12.0-ruby26 Apr 10, 2024
    @delano delano deleted the fix/280-emailer-from-ruby26 branch April 10, 2024 19:19
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    Bug fix maintenance ops Operational issues Review effort [1-5]: 3 ruby Pull requests that update Ruby code security ux Related to user experience
    Projects
    Status: Done
    Development

    Successfully merging this pull request may close these issues.

    None yet

    2 participants