Skip to content

Commit

Permalink
fix local SMTP email sending (#2751)
Browse files Browse the repository at this point in the history
* allow saving an SMTP provider config with no username/password

* hide user/pass input fields in email settings when local provider selected

* use nodemailer-wellknown to populate email settings in database too

* allow custom email provider to not have user/pass

* allow configuring localhost mail providers with the “custom” option

* better debug output on email settings verify

* reset email settings component state on preset change
  • Loading branch information
jshimko authored and brent-hoover committed Aug 29, 2017
1 parent 5906791 commit 972c631
Show file tree
Hide file tree
Showing 6 changed files with 87 additions and 47 deletions.
7 changes: 4 additions & 3 deletions imports/plugins/core/email/client/actions/settings.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import _ from "lodash";
import { Meteor } from "meteor/meteor";
import Alert from "sweetalert2";
import { i18next } from "/client/api";
Expand All @@ -17,12 +18,12 @@ export default {
return callback();
}

if (service !== "custom" && (!user || !password)) {
if (service !== "custom" && service !== "Maildev" && (!user || !password)) {
Alert(i18next.t("app.error"), i18next.t("mail.alerts.userPassRequired", { service }), "error");
return callback();
}

if (service === "custom" && (!host || !port || !user || !password)) {
if (service === "custom" && (!host || !port)) {
Alert(i18next.t("app.error"), i18next.t("mail.alerts.allFieldsRequired"), "error");
return callback();
}
Expand All @@ -36,7 +37,7 @@ export default {
}

const save = () => {
Meteor.call("email/saveSettings", settings, (err) => {
Meteor.call("email/saveSettings", _.pick(settings, ["service", "host", "port", "user", "password"]), (err) => {
if (err) {
return Alert(i18next.t("app.error"),
i18next.t("mail.alerts.saveFailed", { error: err.reason }),
Expand Down
59 changes: 34 additions & 25 deletions imports/plugins/core/email/client/components/emailSettings.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ class EmailSettings extends Component {

this.state = {
settings: props.settings,
hasAuth: !(props.settings.service === "Maildev"),
isSaving: false
};

Expand All @@ -24,23 +25,27 @@ class EmailSettings extends Component {

handleSubmit(e) {
e.preventDefault();
const { saveSettings } = this.props;
const { saveSettings, providers } = this.props;
const { settings } = this.state;
let newSettings = settings;
if (settings.service !== "custom") {
newSettings = Object.assign({}, settings, providers[settings.service]);
}
this.setState({ isSaving: true });
saveSettings(settings, () => this.setState({ isSaving: false }));
saveSettings(newSettings, () => this.setState({ isSaving: false }));
}

handleSelect(e) {
const { settings } = this.state;
settings.service = e;
this.setState({ settings });
handleSelect(service) {
this.setState({ settings: { service }, hasAuth: !(service === "Maildev") });
}

render() {
const { providers } = this.props;
const { settings, isSaving } = this.state;
const { settings, hasAuth, isSaving } = this.state;

const emailProviders = providers.map((name) => (
const providerNames = Object.keys(providers);

const emailProviders = providerNames.map((name) => (
{ label: name, value: name }
));

Expand Down Expand Up @@ -79,22 +84,26 @@ class EmailSettings extends Component {
/>
</div>
}
<Components.TextField
label="User"
i18nKeyLabel="admin.settings.user"
type="text"
name="user"
value={settings.user}
onChange={this.handleStateChange}
/>
<Components.TextField
label="Password"
i18nKeyLabel="admin.settings.password"
type="password"
name="password"
value={settings.password}
onChange={this.handleStateChange}
/>
{hasAuth &&
<div>
<Components.TextField
label="User"
i18nKeyLabel="admin.settings.user"
type="text"
name="user"
value={settings.user}
onChange={this.handleStateChange}
/>
<Components.TextField
label="Password"
i18nKeyLabel="admin.settings.password"
type="password"
name="password"
value={settings.password}
onChange={this.handleStateChange}
/>
</div>
}
<Components.Button
primary={true}
bezelStyle="solid"
Expand All @@ -112,7 +121,7 @@ class EmailSettings extends Component {
}

EmailSettings.propTypes = {
providers: PropTypes.arrayOf(PropTypes.string).isRequired,
providers: PropTypes.object.isRequired,
saveSettings: PropTypes.func.isRequired,
settings: PropTypes.shape({
service: PropTypes.string,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { Reaction } from "/client/api";
import actions from "../actions";
import EmailSettings from "../components/emailSettings";

const providers = Object.keys(require("nodemailer-wellknown/services.json"));
const providers = require("nodemailer-wellknown/services.json");

const composer = ({}, onData) => {
if (Meteor.subscribe("Packages", Reaction.getShopId()).ready()) {
Expand Down
38 changes: 28 additions & 10 deletions server/api/core/email/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,14 @@ export function getMailUrl() {

// create a mail url from well-known provider settings (if they exist)
// https://github.com/nodemailer/nodemailer-wellknown
if (service && service !== "custom" && user && password) {
if (service && service !== "custom") {
const conf = getServiceConfig(service);

if (conf) {
// account for local test providers like Maildev
if (!conf.host) {
mailString = `smtp://localhost:${conf.port}`;
} else {
} else if (user && password) {
mailString = `smtp://${encodeURIComponent(user)}:${password}@${conf.host}:${conf.port}`;
}
}
Expand Down Expand Up @@ -100,6 +100,11 @@ export function getMailConfig() {
};
}

// don't enforce checking TLS on localhost
if (parsedUrl.hostname === "localhost") {
config.ignoreTLS = true;
}

return config;
}

Expand All @@ -116,7 +121,7 @@ export function getMailConfig() {

// if a service provider preset was chosen, return a Nodemailer config for it
// https://github.com/nodemailer/nodemailer-wellknown
if (service && service !== "custom" && user && password) {
if (service && service !== "custom") {
Logger.debug(`Using ${service} to send email`);

// get the config from nodemailer-wellknown
Expand All @@ -127,24 +132,37 @@ export function getMailConfig() {
return conf;
}

// add the credentials to the config
conf.auth = { user, pass: password };
// add any credentials to the config
if (user && password) {
conf.auth = { user, pass: password };
}

return conf;
}

// if a custom config was chosen and all necessary fields exist in the database,
// return the custom Nodemailer config
if ((!service || service === "custom") && user && password && host && port) {
Logger.debug(`Using ${host} to send email`);

return {
if ((!service || service === "custom") && host && port) {
const conf = {
host,
port,
secure: port === 465,
auth: { user, pass: password },
logger: process.env.EMAIL_DEBUG === "true"
};

// don't enforce checking TLS on localhost
if (conf.host === "localhost") {
conf.ignoreTLS = true;
}

// add any credentials to the config
if (user && password) {
conf.auth = { user, pass: password };
}

Logger.debug(`Using ${host} to send email`);

return conf;
}

// else, return the direct mail config and a warning
Expand Down
5 changes: 3 additions & 2 deletions server/jobs/email.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ export default function () {
upsert: true
});

if (!Reaction.Email.getMailUrl()) {
const config = Reaction.Email.getMailConfig();

if (config.direct) {
Emails.update({ jobId }, {
$set: {
status: "failed"
Expand All @@ -49,7 +51,6 @@ export default function () {
return job.fail(msg);
}

const config = Reaction.Email.getMailConfig();
Logger.debug(config, "Sending email with config");

const transport = nodemailer.createTransport(config);
Expand Down
23 changes: 17 additions & 6 deletions server/methods/email.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,20 +26,31 @@ Meteor.methods({
if (typeof settings === "object") {
const { service, host, port, user, password } = settings;

if (service === "custom" && host && port && user && password) {
if (service === "custom" && host && port) {
// create a custom Nodemailer config
config = { host, port, auth: { user, pass: password } };
} else if (service && user && password) {
config = { host, port };

if (host === "localhost") {
config.ignoreTLS = true;
}
} else if (service) {
// create a Nodemailer config from the nodemailer-wellknown services
config = getServiceConfig(service) || {};
}

if (user && password) {
config.auth = { user, pass: password };
}
}

const { Email } = Reaction;

const conf = config || Email.getMailConfig();

Logger.debug(conf, "Verifying email config settings");

try {
return Meteor.wrapAsync(Email.verifyConfig)(config || Email.getMailConfig());
return Meteor.wrapAsync(Email.verifyConfig)(conf);
} catch (e) {
Logger.error(e);
throw new Meteor.Error(e.responseCode, e.response);
Expand All @@ -62,8 +73,8 @@ Meteor.methods({
service: String,
host: Match.Optional(String),
port: Match.Optional(Number),
user: String,
password: String
user: Match.Optional(String),
password: Match.Optional(String)
});

Packages.update({ name: "core", shopId: Reaction.getShopId() }, {
Expand Down

0 comments on commit 972c631

Please sign in to comment.