mailer: allowing overriding address for dev

[jonathaningram]

@jcscottiii if it's not obvious, the rationale here is to be able to override the delivery address when sending mail in dev. In my current task I'm emailing org managers. In dev, I don't want to email real people. The phrase "delivery address" has been used frequently in my experience, hence my choice of naming.

Again, if not obvious, you don't need to set this env var for prod.

I also cleaned up the mailer tests.

[jcscottiii]

@jonathaningram I'm very hesitant to merge test-specific code inside the main code.

1. If you want to see how and who the e-mails are going to, I would advise you to use the docker setup. It gives you a mailcatcher and you can check e-mails
2. If you want to make sure e-mails are actually going out with the right SMTP settings on a deployed instance (and not the local docker setup), we need some acceptance tests (you do the send e-mail action, the tests log in and check for the e-mail). But those are no where in a place to be done yet but I would prefer that.

With that said, there are plenty of disposable email address services you can use and check:

• https://www.mailinator.com/
• https://www.guerrillamail.com/
• https://mailtrap.io/
• http://www.emailvoid.com/

Also, depending on your email service you can sometimes append characters and create a new email while receiving to the same email

You said you want to send emails to org managers in your current task (which will be some new logic in another PR). Setting up a test org with test emails are my recommended way because if we merge this now and then do acceptance tests in bullet 2 mentioned above, we would revert this one. But bullet 2 would require the same manual test account creation before you can use it in the automated tests, so why not do it now?

[jonathaningram]

@jcscottiii thanks for the feedback. It's up to you if you want to merge this, however this is essentially just meant to be a dev feature-flag-type-thing that is opt-in, similar to how we have the LocalCF setting to override TLS (aside: perhaps that setting/env should not even exist and instead we'd just make all dev environments work with TLS always enabled).

Also note this setting/env is not related to testing whether the mail is received or not.

The main goal is to protect any real email addresses that might exist in your dev env. One might always have the CF API endpoint set to a safe dev instance, but hypothetically if you set it to a staging or prod endpoint you could actually email real people by accident. This setting just allows you to set up a safeguard in your dev env to always email yourself. To take this further, if you were to have a test or UAT or staging dashboard environment set up, you might even set the delivery address for those environments. E.g. you'd set it to "[email protected]" and you would tell your QA team that they should check that email inbox to test mails. Again, the main goal is to make sure that if there is real data in the system you don't email those people from non-prod instances (hopefully not customer data, but maybe). I've been bitten by this in a past life when there was no whitelist on the UAT env!

Also, I'm not using the docker set up, I'm just running the binary directly. I feel that the app should be able to run outside of docker entirely.

Again, just to reiterate my use case: my dev instance is connected to one of our CF environments that have real users in it. The real users in this case are cloud.gov.au developers not cloud.gov.au customers. Now, when testing my new feature (which emails all org managers when something happens), I really don't want to email those cloud.gov.au developers!

Anyway, let me know what you think. If you don't want to merge it, that's fine :) in that case, my options are to use the docker set up (but I personally feel the binary should be standalone) or, I just have to make a local change myself to modify the smtp mailer to override the sender whenever I'm testing something that will send out real mail.

[jonathaningram]

@jcscottiii I've had a think about this and will close it for now. I'll try some other routes that don't require this code change. Thanks!