Support TLS Reject Unauthorized in config

[jafeltra]

Summary

This supports setting TLS rejectUnauthorized through configuration.

New behavior

For some reason unknown to me, it doesn't appear that nodemailer is using NODE_EXTRA_CA_CERTS properly, so if you are using that but don't have NODE_TLS_REJECT_UNAUTHORIZED="0" set, nodemailer would throw an error saying unable to verify the first certificate. I couldn't figure out how to get the NODE_EXTRA_CA_CERTS to work properly with nodemailer, so instead this PR supports setting tls.rejectUnauthorized from through a configuration field tlsRejectUnauthorized (I thought the flatter configuration structure was simpler). This is an optional configuration property. If it is left off, tls.rejectUnauthorized defaults to true when setting up the nodemailer transport, which is the behavior we would want by default.

Code changes

Supports setting tls.rejectUnauthorized if it is in the config, checks that it is set properly in a test (the case that it is not set when not specified is tested implicitly by other tests), and adds information about using the property in the README.

Testing guidance

If there are errors during extraction, you have notificationInfo in your config file, you don't have NODE_TLS_REJECT_UNAUTHORIZED="0" set, and you do have NODE_EXTRA_CA_CERTS set, you should get an error when trying to send an email. If you add the tlsRejectUnauthorized field to your configuration, you should be able to send an email.

Note: The README and example config changes here should be included in all the other repos, but I'll add them once they are reviewed and all set here. They should be exactly the same changes.

[Dtphelan1]

DP1 - Do we want to update all README's and linkages across all our repos for this? .... Just saw the comment in the description.

[jafeltra]

DP1 - Do we want to update all README's and linkages across all our repos for this? .... Just saw the comment in the description.

Yeah I was feeling lazy and didn't want to update them in all four places as we iterate on the changes.

[Dtphelan1]

DP1 - Could we add a test that setting tlsRejectUnauthorized: true in the config results in correctly calling the emailNotification's createTransportSpy function with tls: { rejectUnauthorized: true }?

[jafeltra]

DP1 - Could we add a test that setting tlsRejectUnauthorized: true in the config results in correctly calling the emailNotification's createTransportSpy function with tls: { rejectUnauthorized: true }?

Is the concern that something unexpected would happen because it's a boolean value? I'm just trying to understand why it would be different from checking that the false value is added since the code for it doesn't differ between the two cases.

[jafeltra]

@Dtphelan1 I pushed up a new test for the various ways you could set tlsRejectUnauthorized in the config file. I also added a warning if that value isn't a boolean.

I did change the functionality a little to only add the property if a boolean true or false is provided. I can easily swap back to setting any value if that seems preferred. Adding an unsupported value didn't see to harm anything and it just behaved like the default case (tls.rejectUnauthorized is set to true). Let me know what you think or if I can add anything else to address DP1!

[Dtphelan1]

I'm pleased with this approach! Great work as always JA! 🚀 🚀 🚀 🚀 🚀 🚀

[jafeltra]

I bonus added a brief section about masking patient data to the readme.

Also, when this is approved, I will add equivalent sections to the other 3 repos for the new email config property, the masked patient data, and the new property in the example config files.

[mgramigna]

Looks great and resolves the error for me confirmed