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

feat: Allow usage of custom SMTP server #1219

Merged
merged 1 commit into from
Nov 28, 2023
Merged

Conversation

pikaro
Copy link
Contributor

@pikaro pikaro commented Nov 25, 2023

Pull Request Template

⚠️ Before Submitting a PR, read the Contributing Docs in full!

Summary

Mail interaction with service providers was too limiting, and NodeMailer has facilities for handling all kinds of servers. The relevant variables were exposed through the environment and documentation was added.

I initially set this up with Mailjet, by the way, and it worked without issues. So it's not only gmail that works, likely all the NodeMailer providers do. But sending a password reset link through these services if iffy from a security perspective if a trusted mail server is available.

NOTE: I am unsure what the note regarding the values needing to be set means, I might have fixed that as well but this appears to be something larger?

NOTE: Added an unrelated bit in the .env to make the user aware of DALLE_API_KEY. Took me way too long to find, this is the first thing I'm doing with JS.

Change Type

Please delete any irrelevant options.

  • Bug fix: Missing ! when checking for mail service caused the form to malfunction
  • New feature: EMAIL_HOST and related variables for mail configuration
  • Documentation update

Testing

Tested with a variety of values for the env variables, no breaking changes were observed. For complex / odd combinations, the user would have to check the Nodemailer documentation / code to be 100% sure what they all do.

NOTE: I do not consider the inclusion of the APP_TITLE and the user's name in the mail addresses breaking - that may be debatable.

Test Configuration:

Checklist

  • My code adheres to this project's style guidelines
  • I have performed a self-review of my own code
  • I have commented in any complex areas of my code
  • I have made pertinent documentation changes
  • My changes do not introduce new warnings
  • I have written tests demonstrating that my changes are effective or that my feature works
  • Local unit tests pass with my changes
  • [/] Any changes dependent on mine have been merged and published in downstream modules.

Regarding tests, no tests for the password reset exist and the setup would be extensive due to the requirement of a mail server (and a connection to at least one Nodemailer well-known provider).

@danny-avila
Copy link
Owner

Thanks so much for this!

@danny-avila danny-avila merged commit ae03267 into danny-avila:main Nov 28, 2023
1 check passed
cnkang pushed a commit to cnkang/LibreChat that referenced this pull request Feb 6, 2024
Co-authored-by: David Reis <post@d-reis.com>
jinzishuai pushed a commit to aitok-ai/LibreChat that referenced this pull request May 20, 2024
Co-authored-by: David Reis <post@d-reis.com>
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