search

ItzNotABug / ghosler

Send newsletter emails to your Ghost CMS subscribers & members, using your own email credentials!
Apache License 2.0
40 stars 5 forks source link

Allow Insecure Mail Transport #55

Open fritz-fritz opened 1 month ago

fritz-fritz commented 1 month ago

Changed the transport builder to be more flexible.

Defaults to secure = true if mail.secure is not specified Does not require auth to be specified if not needed

This fixes the issue revealed in #54

ItzNotABug commented 1 month ago

Hey @fritz-fritz 👋, Thanks for creating this PR!

I'm away for a while so won't be able to test this behaviour. But the changes are minimal so looks fine to me atm.

However the auth fields are marked required if I recall correctly in the view files. We'd need to change those as well.

fritz-fritz commented 1 month ago

Yes it is hardcoded there as well. There's actually several ways I'd suggest reworking the settings view to allow for a smoother user experience and more flexibility. But my goal with this commit was simply to allow for advanced configuration directly from the configuration file.

If you'd like to hold this and rework the frontend too I'd be open to that.

fritz-fritz commented 1 month ago

I went ahead and modified the view to allow setting these values via the browser.

I also fixed what seems to have been a bug where Reply-To was marked as optional but still was a required form field.

ItzNotABug commented 1 month ago

Hey @fritz-fritz, I'm a bit sceptical for a change like this.

Probable Issues - Current logic allows adding multiple email configurations, this can cause unnecessary and unwanted problems if there are multiple sources that support TLS.

Assume this scenario -

  1. A few hundred subscribers exists
  2. There's a bunch of email credentials with auth/pass and some without those
  3. Newsletters may be sent but others might end up in the spam due to sendmail, this isn't something I'd want
  4. There's a good possibility later that the domain/ip is marked as a spammy sender, worse - marked as a blacklist
  5. Also - IMO Non secure connection isn't something that should usually be used on production

Solution - Ghosler is pretty much stable in terms of usage and doesn't require that many changes, bug fixes overall, so you can host a fork with the above changes if you'd like. You wouldn't need to use Ghosler CLI for this. A simple -

pm2 start app.js --no-autorestart --name ghosler

Alternatively, if you need the features from the cli, download it, change the base url here, install it globally - 

npm install -g .

and you should be good to go!

fritz-fritz commented 1 month ago

Hey @ItzNotABug,

Totally understand the initial skepticism.

Regarding the multiple email credentials, you should check out the code in how I implemented this. It is per mail config. So that should be a non-issue.

Regarding the use of non-secure and/or non-authenticating connections. I absolutely agree that security is important. However, it is totally normal to be talking to a mail server over localhost that doesn't require either and would introduce latency and overhead to do so. As long as the mail server is local there is no issue. Ultimately my view is that this is a valid configuration to implement and concerns regarding spam and security fail on the mail server not Ghosler.