-
-
Notifications
You must be signed in to change notification settings - Fork 529
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
New: Native DKIM capability for emails #16421
Conversation
@krisznet You just need to commit translations in |
Had that same issue and got this answers on slack: |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## 3.x #16421 +/- ##
============================================
- Coverage 21.75% 21.59% -0.17%
- Complexity 10478 10566 +88
============================================
Files 561 561
Lines 31590 31946 +356
============================================
+ Hits 6872 6898 +26
- Misses 24718 25048 +330 ☔ View full report in Codecov by Sentry. |
@Bruno17 The problem is that some company has strict email policy and 2FA for emails, so the SMTP not an option. And it is 2023, DKIM and DMARC is essential. |
Huh? In what case is the company completely fine with you setting a random DKIM key in the DNS, but won't provide you with SMTP credentials for an account authorized to send for the domain? Storing these in a web application is a terrifying prospect, the mail server is a much better place to be signing emails. Furthermore, if an email fails to send one time with mail(), it won't be retried. Mail servers retry every so often on an exponential back off. This is better for the internet, sometimes receiving mail servers go down (even Google!). |
@Omeryl I agree that SMTP is more reliable but sometimes you can't use it because of the technical limitations. If 2FA is required for emails SMTP is not an option. You can set up DKIM selectors, so you can just create a key solely for this purpose. If it is compromised, you can delete it and set up a new one. The private key can be stored in a file or DB and can be encrypted. If your database and/or filesystem are compromised, you are already in big trouble. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All lexicon entries other than the en/ sources must be removed from this PR.
I'm also not clear on how this works by just setting some options.
my idea was to add a plugin event to modPHPMailer::send |
I guess my question is why wouldn't this be settings of an extra that installs this plugin? |
Yeah. But thats currently not possible, because there isn't an event like that. |
But this PR does not add an event. I guess I'm still confused how this is supposed to work as is in the PR. |
oh.. its not my PR. The Opener has added his new systemsettings as default attributes into modMail |
From what I can see, all of these "settings" are part of modPHPMailer. modMail is an interface that is intended to define common settings for any mailer implementation. In order to set the modPHPMailer settings, those entries can be manually added to system settings when needed, and they will just work. So I'm not sure of the actual necessity of this core change. |
Hey guys, if you read the original PR and the Issue it is described clearly. DKIM capability is already in modPHPMailer but you can't use it through modMail with any existing extension like FormIt. I added the settings and also modified modMail, so it uses those settings. That is it. It is much easier than creating a plugin and it is not much code and also not a difficult change. |
@opengeek You are right but not 100% right. You can use DKIM if you write a custom snippet but can't use it with emails sent by modX itself or an extra like FormIt. |
I see now. If the default attributes are not set, the initialization does not look for those attributes in the settings. Let me think on this. It would, in my opinion, be better if we modified the initialization of modPHPMailer to look for attributes from MODX settings in all cases. Otherwise, we will run into other options in this mailer—or in any mailer implementation—that would have to manually be added to the modMail interface. And this is not really the job of an interface. |
All other settings are in the modMail interface as well like SMTP settings and charset, etc. That is why I put it there. |
This comment was marked as resolved.
This comment was marked as resolved.
@opengeek I removed the unwanted lexicon changes. |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Mark-H @opengeek mail_dkim_privatekeyfile
, mail_dkim_privatekeystring
and mail_dkim_passphrase
should also be added to the list of things we need to unset in the following two places right?
unset($c['password'], $c['username'], $c['mail_smtp_pass'], $c['mail_smtp_user'], $c['proxy_password'], $c['proxy_username'], $c['connections'], $c['connection_init'], $c['connection_mutable'], $c['dbname'], $c['database'], $c['table_prefix'], $c['driverOptions'], $c['dsn'], $c['session_name'], $c['assets_path'], $c['base_path'], $c['cache_path'], $c['connectors_path'], $c['core_path'], $c['friendly_alias_translit_class_path'], $c['manager_path'], $c['processors_path']); |
revolution/core/src/Revolution/modX.php
Line 583 in 39a5672
unset($c['password'], $c['username'], $c['mail_smtp_pass'], $c['mail_smtp_user'], $c['proxy_password'], $c['proxy_username'], $c['connections'], $c['connection_init'], $c['connection_mutable'], $c['dbname'], $c['database'], $c['table_prefix'], $c['driverOptions'], $c['dsn'], $c['session_name'], $c['cache_path'], $c['connectors_path'], $c['friendly_alias_translit_class_path'], $c['manager_path'], $c['processors_path']); |
Co-authored-by: Joshua Lückers - Jacobsen <joshualuckers@me.com>
What does it do?
Adding native DKIM capabilities to MODX without creating a custom email script. After setting up the correct values in system settings, all mail sent by MODx will be signed.
Why is it needed?
More and more companies moving to DKIM and also using 2-factor authentication for email accounts, so even secure SMTP is not an option for those. SPF record is often not enough to be able to deliver email reliably.
How to test
Regular mail and SMTP should work like before. If you add DKIM details in settings the Emailer should use it and the email should be signed with the correct key.
Related issue(s)/PR(s)
Resolves #16396