diff --git a/app/assets/javascripts/application.js b/app/assets/javascripts/application.js index a42435af3a9e71..0e9654e0ddb0d8 100644 --- a/app/assets/javascripts/application.js +++ b/app/assets/javascripts/application.js @@ -22,6 +22,7 @@ //= require ./discourse/app/lib/offset-calculator //= require ./discourse/app/lib/lock-on //= require ./discourse/app/lib/url +//= require ./discourse/app/lib/email-provider-default-settings //= require ./discourse/app/lib/debounce //= require ./discourse/app/lib/quote //= require ./discourse/app/lib/key-value-store diff --git a/app/assets/javascripts/discourse/app/components/group-imap-email-settings.js b/app/assets/javascripts/discourse/app/components/group-imap-email-settings.js new file mode 100644 index 00000000000000..012c1178691992 --- /dev/null +++ b/app/assets/javascripts/discourse/app/components/group-imap-email-settings.js @@ -0,0 +1,91 @@ +import Component from "@ember/component"; +import emailProviderDefaultSettings from "discourse/lib/email-provider-default-settings"; +import { isEmpty } from "@ember/utils"; +import { popupAjaxError } from "discourse/lib/ajax-error"; +import discourseComputed, { on } from "discourse-common/utils/decorators"; +import EmberObject, { action } from "@ember/object"; +import { ajax } from "discourse/lib/ajax"; + +export default Component.extend({ + tagName: "", + form: null, + + @discourseComputed( + "group.email_username", + "group.email_password", + "form.imap_server", + "form.imap_port" + ) + missingSettings(email_username, email_password, imap_server, imap_port) { + return [ + email_username, + email_password, + imap_server, + imap_port, + ].some((value) => isEmpty(value)); + }, + + @discourseComputed("group.imap_mailboxes") + mailboxes(imapMailboxes) { + if (!imapMailboxes) { + return []; + } + return imapMailboxes.map((mailbox) => ({ name: mailbox, value: mailbox })); + }, + + @discourseComputed("group.imap_mailbox_name", "mailboxes.length") + mailboxSelected(mailboxName, mailboxesSize) { + return mailboxesSize === 0 || !isEmpty(mailboxName); + }, + + @action + resetSettingsValid() { + this.set("imapSettingsValid", false); + }, + + @on("init") + _fillForm() { + this.set( + "form", + EmberObject.create({ + imap_server: this.group.imap_server, + imap_port: (this.group.imap_port || "").toString(), + imap_ssl: this.group.imap_ssl, + }) + ); + }, + + @action + prefillSettings(provider) { + this.form.setProperties(emailProviderDefaultSettings(provider, "imap")); + }, + + @action + testImapSettings() { + const settings = { + host: this.form.imap_server, + port: this.form.imap_port, + ssl: this.form.imap_ssl, + username: this.group.email_username, + password: this.group.email_password, + }; + + this.set("testingSettings", true); + this.set("imapSettingsValid", false); + + return ajax(`/groups/${this.group.id}/test_email_settings`, { + type: "POST", + data: Object.assign(settings, { protocol: "imap" }), + }) + .then(() => { + this.set("imapSettingsValid", true); + this.group.setProperties({ + imap_server: this.form.imap_server, + imap_port: this.form.imap_port, + imap_ssl: this.form.imap_ssl, + }); + }) + .catch(popupAjaxError) + .finally(() => this.set("testingSettings", false)); + }, +}); diff --git a/app/assets/javascripts/discourse/app/components/group-manage-email-settings.js b/app/assets/javascripts/discourse/app/components/group-manage-email-settings.js new file mode 100644 index 00000000000000..ffa586b32a31f8 --- /dev/null +++ b/app/assets/javascripts/discourse/app/components/group-manage-email-settings.js @@ -0,0 +1,116 @@ +import Component from "@ember/component"; +import { isEmpty } from "@ember/utils"; +import discourseComputed, { on } from "discourse-common/utils/decorators"; +import I18n from "I18n"; +import bootbox from "bootbox"; +import { action } from "@ember/object"; + +export default Component.extend({ + tagName: "", + + imapSettingsValid: false, + smtpSettingsValid: false, + + @on("init") + _determineSettingsValid() { + this.set( + "imapSettingsValid", + this.group.imap_enabled && this.group.imap_server + ); + this.set( + "smtpSettingsValid", + this.group.smtp_enabled && this.group.smtp_server + ); + }, + + @discourseComputed( + "emailSettingsValid", + "group.smtp_enabled", + "group.imap_enabled" + ) + enableImapSettings(emailSettingsValid, smtpEnabled, imapEnabled) { + return smtpEnabled && (emailSettingsValid || imapEnabled); + }, + + @discourseComputed( + "smtpSettingsValid", + "imapSettingsValid", + "group.smtp_enabled", + "group.imap_enabled" + ) + emailSettingsValid( + smtpSettingsValid, + imapSettingsValid, + smtpEnabled, + imapEnabled + ) { + return ( + (!smtpEnabled || smtpSettingsValid) && (!imapEnabled || imapSettingsValid) + ); + }, + + _anySmtpFieldsFilled() { + return [ + this.group.smtp_server, + this.group.smtp_port, + this.group.email_username, + this.group.email_password, + ].some((value) => !isEmpty(value)); + }, + + _anyImapFieldsFilled() { + return [this.group.imap_server, this.group.imap_port].some( + (value) => !isEmpty(value) + ); + }, + + @action + smtpEnabledChange(event) { + if ( + !event.target.checked && + this.group.smtp_enabled && + this._anySmtpFieldsFilled() + ) { + bootbox.confirm( + I18n.t("groups.manage.email.smtp_disable_confirm"), + (result) => { + if (!result) { + this.group.set("smtp_enabled", true); + } else { + this.group.set("imap_enabled", false); + } + } + ); + } + + this.group.set("smtp_enabled", event.target.checked); + }, + + @action + imapEnabledChange(event) { + if ( + !event.target.checked && + this.group.imap_enabled && + this._anyImapFieldsFilled() + ) { + bootbox.confirm( + I18n.t("groups.manage.email.imap_disable_confirm"), + (result) => { + if (!result) { + this.group.set("imap_enabled", true); + } + } + ); + } + + this.group.set("imap_enabled", event.target.checked); + }, + + @action + afterSave() { + // reload the group to get the updated imap_mailboxes + this.store.find("group", this.group.name).then(() => { + this._determineSettingsValid(); + }); + }, +}); diff --git a/app/assets/javascripts/discourse/app/components/group-manage-save-button.js b/app/assets/javascripts/discourse/app/components/group-manage-save-button.js index 0a92d4bd843b0d..82cc182b7d7879 100644 --- a/app/assets/javascripts/discourse/app/components/group-manage-save-button.js +++ b/app/assets/javascripts/discourse/app/components/group-manage-save-button.js @@ -7,6 +7,7 @@ import { popupAutomaticMembershipAlert } from "discourse/controllers/groups-new" export default Component.extend({ saving: null, + disabled: false, @discourseComputed("saving") savingText(saving) { @@ -15,6 +16,10 @@ export default Component.extend({ actions: { save() { + if (this.beforeSave) { + this.beforeSave(); + } + this.set("saving", true); const group = this.model; @@ -31,6 +36,10 @@ export default Component.extend({ } this.set("saved", true); + + if (this.afterSave) { + this.afterSave(); + } }) .catch(popupAjaxError) .finally(() => this.set("saving", false)); diff --git a/app/assets/javascripts/discourse/app/components/group-smtp-email-settings.js b/app/assets/javascripts/discourse/app/components/group-smtp-email-settings.js new file mode 100644 index 00000000000000..d9758d2c4631a6 --- /dev/null +++ b/app/assets/javascripts/discourse/app/components/group-smtp-email-settings.js @@ -0,0 +1,82 @@ +import Component from "@ember/component"; +import emailProviderDefaultSettings from "discourse/lib/email-provider-default-settings"; +import { isEmpty } from "@ember/utils"; +import { popupAjaxError } from "discourse/lib/ajax-error"; +import discourseComputed, { on } from "discourse-common/utils/decorators"; +import EmberObject, { action } from "@ember/object"; +import { ajax } from "discourse/lib/ajax"; + +export default Component.extend({ + tagName: "", + form: null, + + @discourseComputed( + "form.email_username", + "form.email_password", + "form.smtp_server", + "form.smtp_port" + ) + missingSettings(email_username, email_password, smtp_server, smtp_port) { + return [ + email_username, + email_password, + smtp_server, + smtp_port, + ].some((value) => isEmpty(value)); + }, + + @action + resetSettingsValid() { + this.set("smtpSettingsValid", false); + }, + + @on("init") + _fillForm() { + this.set( + "form", + EmberObject.create({ + email_username: this.group.email_username, + email_password: this.group.email_password, + smtp_server: this.group.smtp_server, + smtp_port: (this.group.smtp_port || "").toString(), + smtp_ssl: this.group.smtp_ssl, + }) + ); + }, + + @action + prefillSettings(provider) { + this.form.setProperties(emailProviderDefaultSettings(provider, "smtp")); + }, + + @action + testSmtpSettings() { + const settings = { + host: this.form.smtp_server, + port: this.form.smtp_port, + ssl: this.form.smtp_ssl, + username: this.form.email_username, + password: this.form.email_password, + }; + + this.set("testingSettings", true); + this.set("smtpSettingsValid", false); + + return ajax(`/groups/${this.group.id}/test_email_settings`, { + type: "POST", + data: Object.assign(settings, { protocol: "smtp" }), + }) + .then(() => { + this.set("smtpSettingsValid", true); + this.group.setProperties({ + smtp_server: this.form.smtp_server, + smtp_port: this.form.smtp_port, + smtp_ssl: this.form.smtp_ssl, + email_username: this.form.email_username, + email_password: this.form.email_password, + }); + }) + .catch(popupAjaxError) + .finally(() => this.set("testingSettings", false)); + }, +}); diff --git a/app/assets/javascripts/discourse/app/lib/ajax-error.js b/app/assets/javascripts/discourse/app/lib/ajax-error.js index 0f3feba41caaee..34a2b3604b4e63 100644 --- a/app/assets/javascripts/discourse/app/lib/ajax-error.js +++ b/app/assets/javascripts/discourse/app/lib/ajax-error.js @@ -1,4 +1,5 @@ import I18n from "I18n"; +import { isTesting } from "discourse-common/config/environment"; import bootbox from "bootbox"; export function extractError(error, defaultMessage) { @@ -69,6 +70,11 @@ export function popupAjaxError(error) { } bootbox.alert(extractError(error)); + // in testing mode we want to be able to test these ajax popup messages + if (isTesting()) { + return; + } + error._discourse_displayed = true; // We re-throw in a catch to not swallow the exception diff --git a/app/assets/javascripts/discourse/app/lib/email-provider-default-settings.js b/app/assets/javascripts/discourse/app/lib/email-provider-default-settings.js new file mode 100644 index 00000000000000..fa2939f6ace814 --- /dev/null +++ b/app/assets/javascripts/discourse/app/lib/email-provider-default-settings.js @@ -0,0 +1,22 @@ +const GMAIL = { + imap: { + imap_server: "imap.gmail.com", + imap_port: "993", + imap_ssl: true, + }, + smtp: { + smtp_server: "smtp.gmail.com", + smtp_port: "587", + smtp_ssl: true, + }, +}; + +export default function emailProviderDefaultSettings(provider, protocol) { + provider = provider.toLowerCase(); + protocol = protocol.toLowerCase(); + + switch (provider) { + case "gmail": + return GMAIL[protocol]; + } +} diff --git a/app/assets/javascripts/discourse/app/models/group.js b/app/assets/javascripts/discourse/app/models/group.js index 429dcd05953e71..23a8e54f26acc9 100644 --- a/app/assets/javascripts/discourse/app/models/group.js +++ b/app/assets/javascripts/discourse/app/models/group.js @@ -217,10 +217,12 @@ const Group = RestModel.extend({ smtp_server: this.smtp_server, smtp_port: this.smtp_port, smtp_ssl: this.smtp_ssl, + smtp_enabled: this.smtp_enabled, imap_server: this.imap_server, imap_port: this.imap_port, imap_ssl: this.imap_ssl, imap_mailbox_name: this.imap_mailbox_name, + imap_enabled: this.imap_enabled, email_username: this.email_username, email_password: this.email_password, flair_icon: null, diff --git a/app/assets/javascripts/discourse/app/routes/group-manage-email.js b/app/assets/javascripts/discourse/app/routes/group-manage-email.js index 1ba2f5d5b5142a..5f8a26e43b73e5 100644 --- a/app/assets/javascripts/discourse/app/routes/group-manage-email.js +++ b/app/assets/javascripts/discourse/app/routes/group-manage-email.js @@ -5,7 +5,8 @@ export default DiscourseRoute.extend({ showFooter: true, beforeModel() { - if (!this.siteSettings.enable_imap && !this.siteSettings.enable_smtp) { + // cannot configure IMAP without SMTP being enabled + if (!this.siteSettings.enable_smtp) { return this.transitionTo("group.manage.profile"); } }, diff --git a/app/assets/javascripts/discourse/app/templates/components/group-imap-email-settings.hbs b/app/assets/javascripts/discourse/app/templates/components/group-imap-email-settings.hbs new file mode 100644 index 00000000000000..2c6025d4ac4cd2 --- /dev/null +++ b/app/assets/javascripts/discourse/app/templates/components/group-imap-email-settings.hbs @@ -0,0 +1,80 @@ +
+
+
+
+ + {{input type="text" name="imap_server" value=form.imap_server tabindex="8" onChange=(action "resetSettingsValid")}} +
+ + +
+ +
+
+ + {{input type="text" name="imap_port" value=form.imap_port tabindex="9" onChange=(action "resetSettingsValid" form.imap_port)}} +
+
+ +
+
+ {{#if mailboxes}} + + {{combo-box name="imap_mailbox_name" + value=group.imap_mailbox_name + valueProperty="value" + content=mailboxes + none="groups.manage.email.mailboxes.disabled" + tabindex="10" + onChange=(action (mut group.imap_mailbox_name)) + }} + {{/if}} +
+ +
+
+ +
+
+ {{i18n "groups.manage.email.prefill.title"}} {{i18n "groups.manage.email.prefill.gmail"}} +
+
+ + {{#unless mailboxSelected}} +
+ {{i18n "groups.manage.email.imap_mailbox_not_selected"}} +
+ {{/unless}} + +
+ {{d-button + disabled=(or missingSettings testingSettings) + class="btn-primary test-imap-settings" + action=(action "testImapSettings") + icon="cog" + label="groups.manage.email.test_settings" + tabindex="12" + title="groups.manage.email.settings_required" + }} + + {{conditional-loading-spinner size="small" condition=testingSettings}} + + {{#if imapSettingsValid}} + + {{d-icon "check-circle"}} {{i18n "groups.manage.email.imap_settings_valid"}} + + {{/if}} +
+ +
+

{{i18n "groups.manage.email.imap_additional_settings"}}

+ +

{{i18n "groups.manage.email.settings.allow_unknown_sender_topic_replies_hint"}}

+
+
diff --git a/app/assets/javascripts/discourse/app/templates/components/group-manage-email-settings.hbs b/app/assets/javascripts/discourse/app/templates/components/group-manage-email-settings.hbs new file mode 100644 index 00000000000000..97f08c53483235 --- /dev/null +++ b/app/assets/javascripts/discourse/app/templates/components/group-manage-email-settings.hbs @@ -0,0 +1,38 @@ +
+

{{i18n "groups.manage.email.smtp_title"}}

+

{{i18n "groups.manage.email.smtp_instructions"}}

+ + + + {{#if group.smtp_enabled}} + {{group-smtp-email-settings group=group smtpSettingsValid=smtpSettingsValid}} + {{/if}} + + {{#if siteSettings.enable_imap}} +
+
+ +

{{i18n "groups.manage.email.imap_title"}}

+

+ {{html-safe (i18n "groups.manage.email.imap_instructions")}} +

+ +
{{i18n "groups.manage.email.imap_alpha_warning"}}
+ + + + {{#if group.imap_enabled}} + {{group-imap-email-settings group=group imapSettingsValid=imapSettingsValid}} + {{/if}} +
+ {{/if}} + +
+ {{group-manage-save-button model=group disabled=(not emailSettingsValid) beforeSave=beforeSave afterSave=afterSave tabindex="14"}} +
diff --git a/app/assets/javascripts/discourse/app/templates/components/group-manage-save-button.hbs b/app/assets/javascripts/discourse/app/templates/components/group-manage-save-button.hbs index 11b4167c70f316..d4b358d00f62b3 100644 --- a/app/assets/javascripts/discourse/app/templates/components/group-manage-save-button.hbs +++ b/app/assets/javascripts/discourse/app/templates/components/group-manage-save-button.hbs @@ -1,6 +1,6 @@ -
+
{{d-button action=(action "save") - disabled=saving + disabled=(or disabled saving) class="btn btn-primary group-manage-save" translatedLabel=savingText }} diff --git a/app/assets/javascripts/discourse/app/templates/components/group-smtp-email-settings.hbs b/app/assets/javascripts/discourse/app/templates/components/group-smtp-email-settings.hbs new file mode 100644 index 00000000000000..fbe3f88b5cbc5e --- /dev/null +++ b/app/assets/javascripts/discourse/app/templates/components/group-smtp-email-settings.hbs @@ -0,0 +1,58 @@ +
+
+
+
+ + {{input type="text" name="username" value=form.email_username tabindex="1" onChange=(action "resetSettingsValid")}} +
+ +
+ + {{input type="text" name="smtp_server" value=form.smtp_server tabindex="3" onChange=(action "resetSettingsValid")}} +
+ + +
+ +
+
+ + {{input type="password" name="password" value=form.email_password tabindex="2" onChange=(action "resetSettingsValid")}} +
+ +
+ + {{input type="text" name="smtp_port" value=form.smtp_port tabindex="4" onChange=(action "resetSettingsValid" form.smtp_port)}} +
+
+
+ +
+
+ {{i18n "groups.manage.email.prefill.title"}} {{i18n "groups.manage.email.prefill.gmail"}} +
+
+ +
+ {{d-button + disabled=(or missingSettings testingSettings) + class="btn-primary test-smtp-settings" + action=(action "testSmtpSettings") + icon="cog" + label="groups.manage.email.test_settings" + tabindex="6" + title="groups.manage.email.settings_required" + }} + + {{conditional-loading-spinner size="small" condition=testingSettings}} + + {{#if smtpSettingsValid}} + + {{d-icon "check-circle"}} {{i18n "groups.manage.email.smtp_settings_valid"}} + + {{/if}} +
+
diff --git a/app/assets/javascripts/discourse/app/templates/group/manage/email.hbs b/app/assets/javascripts/discourse/app/templates/group/manage/email.hbs index 6ef5aada78984d..394dd2df081bd2 100644 --- a/app/assets/javascripts/discourse/app/templates/group/manage/email.hbs +++ b/app/assets/javascripts/discourse/app/templates/group/manage/email.hbs @@ -1,4 +1 @@ -
- {{groups-form-email-fields model=model}} - {{group-manage-save-button model=model}} -
+{{group-manage-email-settings group=model}} diff --git a/app/assets/javascripts/discourse/tests/acceptance/group-manage-email-settings-test.js b/app/assets/javascripts/discourse/tests/acceptance/group-manage-email-settings-test.js new file mode 100644 index 00000000000000..963f51ae171289 --- /dev/null +++ b/app/assets/javascripts/discourse/tests/acceptance/group-manage-email-settings-test.js @@ -0,0 +1,230 @@ +import { acceptance, queryAll } from "discourse/tests/helpers/qunit-helpers"; +import selectKit from "discourse/tests/helpers/select-kit-helper"; +import { click, currentRouteName, fillIn, visit } from "@ember/test-helpers"; +import I18n from "I18n"; +import { test } from "qunit"; + +acceptance("Managing Group Email Settings - SMTP Disabled", function (needs) { + needs.user(); + + test("When SiteSetting.enable_smtp is false", async function (assert) { + await visit("/g/discourse/manage/email"); + assert.equal( + currentRouteName(), + "group.manage.profile", + "it redirects to the group profile page" + ); + }); +}); + +acceptance( + "Managing Group Email Settings - SMTP Enabled, IMAP Disabled", + function (needs) { + needs.user(); + needs.settings({ enable_smtp: true }); + + test("When SiteSetting.enable_smtp is true but SiteSetting.enable_imap is false", async function (assert) { + await visit("/g/discourse/manage/email"); + assert.equal( + currentRouteName(), + "group.manage.email", + "it redirects to the group email page" + ); + assert.notOk( + exists(".group-manage-email-imap-wrapper"), + "does not show IMAP settings" + ); + }); + } +); + +acceptance( + "Managing Group Email Settings - SMTP and IMAP Enabled", + function (needs) { + needs.user(); + needs.settings({ enable_smtp: true, enable_imap: true }); + + needs.pretender((server, helper) => { + server.post("/groups/47/test_email_settings", () => { + return helper.response({ + success: "OK", + }); + }); + server.put("/groups/47", () => { + return helper.response({ + success: "OK", + }); + }); + }); + + test("enabling SMTP, testing, and saving", async function (assert) { + await visit("/g/discourse/manage/email"); + assert.ok( + exists("#enable_imap:disabled"), + "IMAP is disabled until SMTP settings are valid" + ); + + await click("#enable_smtp"); + assert.ok(exists(".group-smtp-email-settings")); + + await click("#prefill_smtp_gmail"); + assert.equal( + queryAll("input[name='smtp_server']").val(), + "smtp.gmail.com", + "prefills SMTP server settings for gmail" + ); + assert.equal( + queryAll("input[name='smtp_port']").val(), + "587", + "prefills SMTP port settings for gmail" + ); + assert.ok( + exists("#enable_ssl:checked"), + "prefills SMTP ssl settings for gmail" + ); + + assert.ok( + exists(".test-smtp-settings:disabled"), + "does not allow testing settings if not all fields are filled" + ); + + await fillIn('input[name="username"]', "myusername@gmail.com"); + await fillIn('input[name="password"]', "password@gmail.com"); + await click(".test-smtp-settings"); + + assert.ok(exists(".smtp-settings-ok"), "tested settings are ok"); + + await click(".group-manage-save"); + + assert.equal( + queryAll(".group-manage-save-button > span").text(), + "Saved!" + ); + + assert.notOk( + exists("#enable_imap:disabled"), + "IMAP is able to be enabled now that SMTP is saved" + ); + + await click("#enable_smtp"); + assert.equal( + queryAll(".modal-body").text(), + I18n.t("groups.manage.email.smtp_disable_confirm"), + "shows a confirm dialogue warning SMTP settings will be wiped" + ); + + await click(".modal-footer .btn.btn-primary"); + }); + + test("enabling IMAP, testing, and saving", async function (assert) { + await visit("/g/discourse/manage/email"); + + await click("#enable_smtp"); + await click("#prefill_smtp_gmail"); + await fillIn('input[name="username"]', "myusername@gmail.com"); + await fillIn('input[name="password"]', "password@gmail.com"); + await click(".test-smtp-settings"); + await click(".group-manage-save"); + + assert.notOk( + exists("#enable_imap:disabled"), + "IMAP is able to be enabled now that IMAP is saved" + ); + + await click("#enable_imap"); + + assert.ok( + exists(".test-imap-settings:disabled"), + "does not allow testing settings if not all fields are filled" + ); + + await click("#prefill_imap_gmail"); + assert.equal( + queryAll("input[name='imap_server']").val(), + "imap.gmail.com", + "prefills IMAP server settings for gmail" + ); + assert.equal( + queryAll("input[name='imap_port']").val(), + "993", + "prefills IMAP port settings for gmail" + ); + assert.ok( + exists("#enable_ssl:checked"), + "prefills IMAP ssl settings for gmail" + ); + await click(".test-imap-settings"); + + assert.ok(exists(".imap-settings-ok"), "tested settings are ok"); + + await click(".group-manage-save"); + + assert.equal( + queryAll(".group-manage-save-button > span").text(), + "Saved!" + ); + + assert.ok( + exists(".imap-no-mailbox-selected"), + "shows a message saying no IMAP mailbox is selected" + ); + + await selectKit( + ".control-group.group-imap-mailboxes .combo-box" + ).expand(); + await selectKit( + ".control-group.group-imap-mailboxes .combo-box" + ).selectRowByValue("All Mail"); + await click(".group-manage-save"); + + assert.notOk( + exists(".imap-no-mailbox-selected"), + "no longer shows a no mailbox selected message" + ); + + await click("#enable_imap"); + assert.equal( + queryAll(".modal-body").text(), + I18n.t("groups.manage.email.imap_disable_confirm"), + "shows a confirm dialogue warning IMAP settings will be wiped" + ); + await click(".modal-footer .btn.btn-primary"); + }); + } +); + +acceptance( + "Managing Group Email Settings - SMTP and IMAP Enabled - Email Test Invalid", + function (needs) { + needs.user(); + needs.settings({ enable_smtp: true, enable_imap: true }); + + needs.pretender((server, helper) => { + server.post("/groups/47/test_email_settings", () => { + return helper.response(422, { + success: false, + errors: [ + "There was an issue with the SMTP credentials provided, check the username and password and try again.", + ], + }); + }); + }); + + test("enabling IMAP, testing, and saving", async function (assert) { + await visit("/g/discourse/manage/email"); + + await click("#enable_smtp"); + await click("#prefill_smtp_gmail"); + await fillIn('input[name="username"]', "myusername@gmail.com"); + await fillIn('input[name="password"]', "password@gmail.com"); + await click(".test-smtp-settings"); + + assert.equal( + queryAll(".modal-body").text(), + "There was an issue with the SMTP credentials provided, check the username and password and try again.", + "shows a dialogue with the error message from the server" + ); + await click(".modal-footer .btn.btn-primary"); + }); + } +); diff --git a/app/assets/javascripts/discourse/tests/fixtures/group-fixtures.js b/app/assets/javascripts/discourse/tests/fixtures/group-fixtures.js index 615fe7508645a4..04e15383ace017 100644 --- a/app/assets/javascripts/discourse/tests/fixtures/group-fixtures.js +++ b/app/assets/javascripts/discourse/tests/fixtures/group-fixtures.js @@ -48,7 +48,11 @@ export default { messageable: true, can_see_members: true, has_messages: true, - message_count: 2 + message_count: 2, + imap_mailboxes: [ + "All Mail", + "Important" + ] }, extras: { visible_group_names: ["discourse"] diff --git a/app/assets/stylesheets/common/base/alert.scss b/app/assets/stylesheets/common/base/alert.scss index 64211b17522754..8ade16c7188d07 100644 --- a/app/assets/stylesheets/common/base/alert.scss +++ b/app/assets/stylesheets/common/base/alert.scss @@ -23,6 +23,10 @@ background-color: var(--danger-low); color: var(--primary); } + &.alert-warning { + background-color: var(--highlight-low); + color: var(--primary); + } &.alert-info { background-color: var(--tertiary-low); color: var(--primary); diff --git a/app/assets/stylesheets/common/base/group.scss b/app/assets/stylesheets/common/base/group.scss index a8c0577ed2c5bb..c39e6bee8fedda 100644 --- a/app/assets/stylesheets/common/base/group.scss +++ b/app/assets/stylesheets/common/base/group.scss @@ -241,3 +241,27 @@ table.group-category-permissions { } } } + +.group-smtp-email-settings, +.group-imap-email-settings { + .groups-form { + display: grid; + grid-template-columns: 1fr 3fr; + margin-bottom: 0; + + &.groups-form-imap { + grid-template-columns: 1fr 1fr 2fr; + } + } + + background-color: var(--primary-very-low); + padding: 1em; + margin: 1em 0; + border: 1px solid var(--primary-low); + + .group-imap-mailboxes { + .combo-box { + width: 50%; + } + } +} diff --git a/app/assets/stylesheets/mobile/group.scss b/app/assets/stylesheets/mobile/group.scss index d54562b81a8084..7053aa3ee464ba 100644 --- a/app/assets/stylesheets/mobile/group.scss +++ b/app/assets/stylesheets/mobile/group.scss @@ -47,3 +47,20 @@ margin-top: 0.5em; } } + +.group-smtp-email-settings, +.group-imap-email-settings { + .groups-form { + grid-template-columns: 1fr; + + input[type="text"], + input[type="password"], + .group-imap-mailboxes .combo-box { + width: 100%; + } + + &.groups-form-imap { + grid-template-columns: 1fr; + } + } +} diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 69dff9cf26dac3..3d6e6262bc3d38 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -10,7 +10,8 @@ class GroupsController < ApplicationController :histories, :request_membership, :search, - :new + :new, + :test_email_settings ] skip_before_action :preload_json, :check_xhr, only: [:posts_feed, :mentions_feed] @@ -150,8 +151,13 @@ def update group = Group.find(params[:id]) guardian.ensure_can_edit!(group) unless guardian.can_admin_group?(group) - if group.update(group_params(automatic: group.automatic)) + params_with_permitted = group_params(automatic: group.automatic) + clear_disabled_email_settings(group, params_with_permitted) + + if group.update(params_with_permitted) GroupActionLogger.new(current_user, group, skip_guardian: true).log_change_group_settings + group.record_email_setting_changes!(current_user) + group.expire_imap_mailbox_cache if guardian.can_see?(group) render json: success_json @@ -580,6 +586,52 @@ def permissions render_serialized(category_groups.sort_by { |category_group| category_group.category.name }, CategoryGroupSerializer) end + def test_email_settings + params.require(:group_id) + params.require(:protocol) + params.require(:port) + params.require(:host) + params.require(:username) + params.require(:password) + params.require(:ssl) + + group = Group.find(params[:group_id]) + guardian.ensure_can_edit!(group) + + RateLimiter.new(current_user, "group_test_email_settings", 5, 1.minute).performed! + + settings = params.except(:group_id, :protocol) + enable_tls = settings[:ssl] == "true" + email_host = params[:host] + + if !["smtp", "imap"].include?(params[:protocol]) + raise Discourse::InvalidParameters.new("Valid protocols to test are smtp and imap") + end + + hijack do + begin + case params[:protocol] + when "smtp" + enable_starttls_auto = false + settings.delete(:ssl) + + final_settings = settings.merge(enable_tls: enable_tls, enable_starttls_auto: enable_starttls_auto) + .permit(:host, :port, :username, :password, :enable_tls, :enable_starttls_auto, :debug) + EmailSettingsValidator.validate_as_user(current_user, "smtp", **final_settings.to_h.symbolize_keys) + when "imap" + final_settings = settings.merge(ssl: enable_tls) + .permit(:host, :port, :username, :password, :ssl, :debug) + EmailSettingsValidator.validate_as_user(current_user, "imap", **final_settings.to_h.symbolize_keys) + end + rescue *EmailSettingsExceptionHandler::EXPECTED_EXCEPTIONS, StandardError => err + return render_json_error( + EmailSettingsExceptionHandler.friendly_exception_message(err, email_host) + ) + end + render json: success_json + end + end + private def group_params(automatic: false) @@ -620,10 +672,16 @@ def group_params(automatic: false) :smtp_server, :smtp_port, :smtp_ssl, + :smtp_enabled, + :smtp_updated_by, + :smtp_updated_at, :imap_server, :imap_port, :imap_ssl, :imap_mailbox_name, + :imap_enabled, + :imap_updated_by, + :imap_updated_at, :email_username, :email_password, :primary_group, @@ -678,4 +736,24 @@ def users_from_params end users end + + def clear_disabled_email_settings(group, params_with_permitted) + should_clear_imap = group.imap_enabled && params_with_permitted.key?(:imap_enabled) && params_with_permitted[:imap_enabled] == "false" + should_clear_smtp = group.smtp_enabled && params_with_permitted.key?(:smtp_enabled) && params_with_permitted[:smtp_enabled] == "false" + + if should_clear_imap || should_clear_smtp + params_with_permitted[:imap_server] = nil + params_with_permitted[:imap_ssl] = false + params_with_permitted[:imap_port] = nil + params_with_permitted[:imap_mailbox_name] = "" + end + + if should_clear_smtp + params_with_permitted[:smtp_server] = nil + params_with_permitted[:smtp_ssl] = false + params_with_permitted[:smtp_port] = nil + params_with_permitted[:email_username] = nil + params_with_permitted[:email_password] = nil + end + end end diff --git a/app/mailers/group_smtp_mailer.rb b/app/mailers/group_smtp_mailer.rb index 58fcbc4a1b80d2..f8510deae34e5a 100644 --- a/app/mailers/group_smtp_mailer.rb +++ b/app/mailers/group_smtp_mailer.rb @@ -48,7 +48,7 @@ def send_mail(from_group, to_address, post) group_name: from_group.name, allow_reply_by_email: true, only_reply_by_email: true, - use_from_address_for_reply_to: from_group.imap_enabled?, + use_from_address_for_reply_to: SiteSetting.enable_imap && from_group.imap_enabled?, private_reply: post.topic.private_message?, participants: participants(post), include_respond_instructions: true, diff --git a/app/models/group.rb b/app/models/group.rb index 16c47fd19670fe..4aad4ea7c80b5d 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -34,6 +34,8 @@ class Group < ActiveRecord::Base has_many :group_tag_notification_defaults, dependent: :destroy belongs_to :flair_upload, class_name: 'Upload' + belongs_to :smtp_updated_by, class_name: 'User' + belongs_to :imap_updated_by, class_name: 'User' has_and_belongs_to_many :web_hooks @@ -59,6 +61,10 @@ class Group < ActiveRecord::Base def expire_cache ApplicationSerializer.expire_cache_fragment!("group_names") SvgSprite.expire_cache + expire_imap_mailbox_cache + end + + def expire_imap_mailbox_cache Discourse.cache.delete("group_imap_mailboxes_#{self.id}") end @@ -88,6 +94,22 @@ def remove_review_groups AUTO_GROUP_IDS = Hash[*AUTO_GROUPS.to_a.flatten.reverse] STAFF_GROUPS = [:admins, :moderators, :staff] + IMAP_SETTING_ATTRIBUTES = [ + "imap_server", + "imap_port", + "imap_ssl", + "email_username", + "email_password" + ] + + SMTP_SETTING_ATTRIBUTES = [ + "imap_server", + "imap_port", + "imap_ssl", + "email_username", + "email_password" + ] + ALIAS_LEVELS = { nobody: 0, only_admins: 1, @@ -112,7 +134,7 @@ def self.visibility_levels validates :mentionable_level, inclusion: { in: ALIAS_LEVELS.values } validates :messageable_level, inclusion: { in: ALIAS_LEVELS.values } - scope :with_imap_configured, -> { where.not(imap_mailbox_name: '') } + scope :with_imap_configured, -> { where(imap_enabled: true).where.not(imap_mailbox_name: '') } scope :visible_groups, Proc.new { |user, order, opts| groups = self.order(order || "name ASC") @@ -281,6 +303,23 @@ def cook_bio end end + def record_email_setting_changes!(user) + if (self.previous_changes.keys & IMAP_SETTING_ATTRIBUTES).any? + self.imap_updated_at = Time.zone.now + self.imap_updated_by_id = user.id + end + + if (self.previous_changes.keys & SMTP_SETTING_ATTRIBUTES).any? + self.smtp_updated_at = Time.zone.now + self.smtp_updated_by_id = user.id + end + + self.smtp_enabled = [self.smtp_port, self.smtp_server, self.email_password, self.email_username].all?(&:present?) + self.imap_enabled = [self.imap_port, self.imap_server, self.email_password, self.email_username].all?(&:present?) + + self.save + end + def incoming_email_validator return if self.automatic || self.incoming_email.blank? @@ -796,10 +835,7 @@ def set_default_notifications end def imap_mailboxes - return [] if self.imap_server.blank? || - self.email_username.blank? || - self.email_password.blank? || - !SiteSetting.enable_imap + return [] if !self.imap_enabled || !SiteSetting.enable_imap Discourse.cache.fetch("group_imap_mailboxes_#{self.id}", expires_in: 30.minutes) do Rails.logger.info("[IMAP] Refreshing mailboxes list for group #{self.name}") @@ -810,7 +846,7 @@ def imap_mailboxes self.imap_config ) imap_provider.connect! - mailboxes = imap_provider.list_mailboxes + mailboxes = imap_provider.filter_mailboxes(imap_provider.list_mailboxes_with_attributes) imap_provider.disconnect! update_columns(imap_last_error: nil) @@ -833,11 +869,6 @@ def imap_config } end - def imap_enabled? - return false if !SiteSetting.enable_imap - imap_config.values.compact.length == imap_config.keys.length - end - def email_username_regex user, domain = email_username.split('@') if user.present? && domain.present? @@ -1042,10 +1073,6 @@ def enqueue_update_mentions_job # membership_request_template :text # messageable_level :integer default(0) # mentionable_level :integer default(0) -# publish_read_state :boolean default(FALSE), not null -# members_visibility_level :integer default(0), not null -# flair_icon :string -# flair_upload_id :integer # smtp_server :string # smtp_port :integer # smtp_ssl :boolean @@ -1057,10 +1084,20 @@ def enqueue_update_mentions_job # imap_last_uid :integer default(0), not null # email_username :string # email_password :string +# publish_read_state :boolean default(FALSE), not null +# members_visibility_level :integer default(0), not null # imap_last_error :text # imap_old_emails :integer # imap_new_emails :integer -# allow_unknown_sender_topic_replies :boolean default(FALSE) +# flair_icon :string +# flair_upload_id :integer +# allow_unknown_sender_topic_replies :boolean default(FALSE), not null +# smtp_enabled :boolean default(FALSE) +# smtp_updated_at :datetime +# smtp_updated_by_id :integer +# imap_enabled :boolean default(FALSE) +# imap_updated_at :datetime +# imap_updated_by_id :integer # # Indexes # diff --git a/app/serializers/group_show_serializer.rb b/app/serializers/group_show_serializer.rb index 1b0bdf8b789106..9cdf1f842d3057 100644 --- a/app/serializers/group_show_serializer.rb +++ b/app/serializers/group_show_serializer.rb @@ -12,15 +12,24 @@ def self.admin_attributes(*attrs) end end + has_one :smtp_updated_by, embed: :object, serializer: BasicUserSerializer + has_one :imap_updated_by, embed: :object, serializer: BasicUserSerializer + admin_attributes :automatic_membership_email_domains, :smtp_server, :smtp_port, :smtp_ssl, + :smtp_enabled, + :smtp_updated_at, + :smtp_updated_by, :imap_server, :imap_port, :imap_ssl, :imap_mailbox_name, :imap_mailboxes, + :imap_enabled, + :imap_updated_at, + :imap_updated_by, :email_username, :email_password, :imap_last_error, diff --git a/app/services/email_settings_exception_handler.rb b/app/services/email_settings_exception_handler.rb new file mode 100644 index 00000000000000..ddb3fb908b8194 --- /dev/null +++ b/app/services/email_settings_exception_handler.rb @@ -0,0 +1,128 @@ +# frozen_string_literal: true + +require 'net/imap' +require 'net/smtp' +require 'net/pop' + +class EmailSettingsExceptionHandler + EXPECTED_EXCEPTIONS = [ + Net::POPAuthenticationError, + Net::IMAP::NoResponseError, + Net::IMAP::Error, + Net::SMTPAuthenticationError, + Net::SMTPServerBusy, + Net::SMTPSyntaxError, + Net::SMTPFatalError, + Net::SMTPUnknownError, + Net::OpenTimeout, + Net::ReadTimeout, + SocketError, + Errno::ECONNREFUSED + ] + + class GenericProvider + def initialize(exception) + @exception = exception + end + + def message + case @exception + when Net::POPAuthenticationError + net_pop_authentication_error + when Net::IMAP::NoResponseError + net_imap_no_response_error + when Net::IMAP::Error + net_imap_unhandled_error + when Net::SMTPAuthenticationError + net_smtp_authentication_error + when Net::SMTPServerBusy + net_smtp_server_busy + when Net::SMTPSyntaxError, Net::SMTPFatalError, Net::SMTPUnknownError + net_smtp_unhandled_error + when SocketError, Errno::ECONNREFUSED + socket_connection_error + when Net::OpenTimeout, Net::ReadTimeout + net_timeout_error + else + unhandled_error + end + end + + private + + def net_pop_authentication_error + I18n.t("email_settings.pop3_authentication_error") + end + + def net_imap_no_response_error + # Most of IMAP's errors are lumped under the NoResponseError, including invalid + # credentials errors, because it is raised when a "NO" response is + # raised from the IMAP server https://datatracker.ietf.org/doc/html/rfc3501#section-7.1.2 + # + # Generally, it should be fairly safe to just return the error message as is. + if @exception.message.match(/Invalid credentials/) + I18n.t("email_settings.imap_authentication_error") + else + I18n.t("email_settings.imap_no_response_error", message: @exception.message.gsub(" (Failure)", "")) + end + end + + def net_imap_unhandled_error + I18n.t("email_settings.imap_unhandled_error", message: @exception.message) + end + + def net_smtp_authentication_error + I18n.t("email_settings.smtp_authentication_error") + end + + def net_smtp_server_busy + I18n.t("email_settings.smtp_server_busy_error") + end + + def net_smtp_unhandled_error + I18n.t("email_settings.smtp_unhandled_error", message: @exception.message) + end + + def socket_connection_error + I18n.t("email_settings.connection_error") + end + + def net_timeout_error + I18n.t("email_settings.timeout_error") + end + + def unhandled_error + I18n.t("email_settings.unhandled_error", message: @exception.message) + end + end + + class GmailProvider < GenericProvider + def net_smtp_authentication_error + # Gmail requires use of application-specific passwords when 2FA is enabled and return + # a special error message calling this out. + if @exception.message.match(/Application-specific password required/) + I18n.t("email_settings.authentication_error_gmail_app_password") + else + super + end + end + + def net_imap_no_response_error + # Gmail requires use of application-specific passwords when 2FA is enabled and return + # a special error message calling this out. + if @exception.message.match(/Application-specific password required/) + I18n.t("email_settings.authentication_error_gmail_app_password") + else + super + end + end + end + + def self.friendly_exception_message(exception, host) + if host.include?("gmail.com") + EmailSettingsExceptionHandler::GmailProvider.new(exception).message + else + EmailSettingsExceptionHandler::GenericProvider.new(exception).message + end + end +end diff --git a/app/services/email_settings_validator.rb b/app/services/email_settings_validator.rb index 190d8720e2e2fd..238b5869627cd0 100644 --- a/app/services/email_settings_validator.rb +++ b/app/services/email_settings_validator.rb @@ -12,52 +12,13 @@ # # or for specific host preset # EmailSettingsValidator.validate_imap(**{ username: "test@gmail.com", password: "test" }.merge(Email.gmail_imap_settings)) # -# rescue *EmailSettingsValidator::FRIENDLY_EXCEPTIONS => err -# EmailSettingsValidator.friendly_exception_message(err) +# rescue *EmailSettingsExceptionHandler::EXPECTED_EXCEPTIONS => err +# EmailSettingsExceptionHandler.friendly_exception_message(err, host) # end class EmailSettingsValidator - EXPECTED_EXCEPTIONS = [ - Net::POPAuthenticationError, - Net::IMAP::NoResponseError, - Net::SMTPAuthenticationError, - Net::SMTPServerBusy, - Net::SMTPSyntaxError, - Net::SMTPFatalError, - Net::SMTPUnknownError, - Net::OpenTimeout, - Net::ReadTimeout, - SocketError, - Errno::ECONNREFUSED - ] - - def self.friendly_exception_message(exception) - case exception - when Net::POPAuthenticationError - I18n.t("email_settings.pop3_authentication_error") - when Net::IMAP::NoResponseError - - # Most of IMAP's errors are lumped under the NoResponseError, including invalid - # credentials errors, because it is raised when a "NO" response is - # raised from the IMAP server https://datatracker.ietf.org/doc/html/rfc3501#section-7.1.2 - # - # Generally, it should be fairly safe to just return the error message as is. - if exception.message.match(/Invalid credentials/) - I18n.t("email_settings.imap_authentication_error") - else - I18n.t("email_settings.imap_no_response_error", message: exception.message.gsub(" (Failure)", "")) - end - when Net::SMTPAuthenticationError - I18n.t("email_settings.smtp_authentication_error") - when Net::SMTPServerBusy - I18n.t("email_settings.smtp_server_busy_error") - when Net::SMTPSyntaxError, Net::SMTPFatalError, Net::SMTPUnknownError - I18n.t("email_settings.smtp_unhandled_error", message: exception.message) - when SocketError, Errno::ECONNREFUSED - I18n.t("email_settings.connection_error") - when Net::OpenTimeout, Net::ReadTimeout - I18n.t("email_settings.timeout_error") - else - I18n.t("email_settings.unhandled_error", message: exception.message) + def self.validate_as_user(user, protocol, **kwargs) + DistributedMutex.synchronize("validate_#{protocol}_#{user.id}", validity: 10) do + self.public_send("validate_#{protocol}", **kwargs) end end @@ -74,7 +35,7 @@ def self.validate_pop3( password:, ssl: SiteSetting.pop3_polling_ssl, openssl_verify: SiteSetting.pop3_polling_openssl_verify, - debug: false + debug: Rails.env.development? ) begin pop3 = Net::POP3.new(host, port) @@ -113,16 +74,20 @@ def self.validate_pop3( def self.validate_smtp( host:, port:, - domain:, username:, password:, + domain: nil, authentication: GlobalSetting.smtp_authentication, enable_starttls_auto: GlobalSetting.smtp_enable_start_tls, enable_tls: GlobalSetting.smtp_force_tls, openssl_verify_mode: GlobalSetting.smtp_openssl_verify_mode, - debug: false + debug: Rails.env.development? ) begin + port, enable_tls, enable_starttls_auto = provider_specific_ssl_overrides( + host, port, enable_tls, enable_starttls_auto + ) + if enable_tls && enable_starttls_auto raise ArgumentError, "TLS and STARTTLS are mutually exclusive" end @@ -131,6 +96,17 @@ def self.validate_smtp( raise ArgumentError, "Invalid authentication method. Must be plain, login, or cram_md5." end + if domain.blank? + if Rails.env.development? + domain = "localhost" + else + + # Because we are using the SMTP settings here to send emails, + # the domain should just be the TLD of the host. + domain = MiniSuffix.domain(host) + end + end + smtp = Net::SMTP.new(host, port) # These SSL options are cribbed from the Mail gem, which is used internally @@ -150,6 +126,9 @@ def self.validate_smtp( smtp.enable_starttls_auto(ssl_context) if enable_starttls_auto smtp.enable_tls(ssl_context) if enable_tls + smtp.open_timeout = 5 + smtp.read_timeout = 5 + smtp.start(domain, username, password, authentication.to_sym) smtp.finish rescue => err @@ -168,7 +147,7 @@ def self.validate_imap( port:, username:, password:, - open_timeout: 10, + open_timeout: 5, ssl: true, debug: false ) @@ -188,4 +167,20 @@ def self.log_and_raise(err, debug) end raise err end + + def self.provider_specific_ssl_overrides(host, port, enable_tls, enable_starttls_auto) + # Gmail acts weirdly if you do not use the correct combinations of + # TLS settings based on the port, we clean these up here for the user. + if host == "smtp.gmail.com" + if port.to_i == 587 + enable_starttls_auto = true + enable_tls = false + elsif port.to_i == 465 + enable_starttls_auto = false + enable_tls = true + end + end + + [port, enable_tls, enable_starttls_auto] + end end diff --git a/app/services/post_alerter.rb b/app/services/post_alerter.rb index 77ef4146025bf6..2958cea298641e 100644 --- a/app/services/post_alerter.rb +++ b/app/services/post_alerter.rb @@ -579,13 +579,7 @@ def notify_users(users, type, post, opts = {}) def group_notifying_via_smtp(post) return nil if !SiteSetting.enable_smtp || post.post_type != Post.types[:regular] - - post.topic.allowed_groups - .where.not(smtp_server: nil) - .where.not(smtp_port: nil) - .where.not(email_username: nil) - .where.not(email_password: nil) - .first + post.topic.allowed_groups.where(smtp_enabled: true).first end def notify_pm_users(post, reply_to_user, notified) diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index ad257fd1bd87e2..53e21a1c8d035a 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -692,6 +692,25 @@ en: email: title: "Email" status: "Synchronized %{old_emails} / %{total_emails} emails via IMAP." + enable_smtp: "Enable SMTP" + enable_imap: "Enable IMAP" + test_settings: "Test Settings" + save_settings: "Save Settings" + settings_required: "All settings are required, please fill in all fields before validation." + smtp_settings_valid: "SMTP settings valid." + smtp_title: "SMTP" + smtp_instructions: "When you enable SMTP for the group, all outbound emails sent from the group's inbox will be sent via the SMTP settings specified here instead of the mail server configured for other emails sent by your forum." + imap_title: "IMAP" + imap_additional_settings: "Additional Settings" + imap_instructions: "When you enable IMAP for the group, emails are synced between the group inbox and the provided IMAP server and mailbox. SMTP must be enabled with valid and tested credentials before IMAP can be enabled. The email username and password used for SMTP will be used for IMAP. For more information see feature announcement on Discourse Meta." + imap_alpha_warning: "Warning: This is an alpha-stage feature. Only Gmail is officially supported. Use at your own risk!" + imap_settings_valid: "IMAP settings valid." + smtp_disable_confirm: "If you disable SMTP, all SMTP and IMAP settings will be reset and the associated functionality will be disabled. Are you sure you want to continue?" + imap_disable_confirm: "If you disable IMAP all IMAP settings will be reset and the associated functionality will be disabled. Are you sure you want to continue?" + imap_mailbox_not_selected: "You must select a Mailbox for this IMAP configuration or no mailboxes will be synced!" + prefill: + title: "Prefill with settings for:" + gmail: "GMail" credentials: title: "Credentials" smtp_server: "SMTP Server" @@ -709,7 +728,7 @@ en: mailboxes: synchronized: "Synchronized Mailbox" none_found: "No mailboxes were found in this email account." - disabled: "disabled" + disabled: "Disabled" membership: title: Membership access: Access diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 3a66d38b500168..dbaf87bb0364d3 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -994,8 +994,10 @@ en: imap_authentication_error: "There was an issue with the IMAP credentials provided, check the username and password and try again." imap_no_response_error: "An error occurred when communicating with the IMAP server. %{message}" smtp_authentication_error: "There was an issue with the SMTP credentials provided, check the username and password and try again." + authentication_error_gmail_app_password: "Application-specific password required. Learn more at this Google Help article" smtp_server_busy_error: "The SMTP server is currently busy, try again later." smtp_unhandled_error: "There was an unhandled error when communicating with the SMTP server. %{message}" + imap_unhandled_error: "There was an unhandled error when communicating with the IMAP server. %{message}" connection_error: "There was an issue connecting with the server, check the server name and port and try again." timeout_error: "Connection to the server timed out, check the server name and port and try again." unhandled_error: "Unhandled error when testing email settings. %{message}" diff --git a/config/routes.rb b/config/routes.rb index 7a88938102ca73..88f8ad353d2cb5 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -568,6 +568,7 @@ get 'mentionable' get 'messageable' get 'logs' => 'groups#histories' + post 'test_email_settings' collection do get "check-name" => 'groups#check_name' diff --git a/db/migrate/20210517061815_add_dedicated_enable_imap_smtp_columns_for_group.rb b/db/migrate/20210517061815_add_dedicated_enable_imap_smtp_columns_for_group.rb new file mode 100644 index 00000000000000..4375ba648ce727 --- /dev/null +++ b/db/migrate/20210517061815_add_dedicated_enable_imap_smtp_columns_for_group.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +class AddDedicatedEnableImapSmtpColumnsForGroup < ActiveRecord::Migration[6.1] + def up + add_column :groups, :smtp_enabled, :boolean, default: false + add_column :groups, :smtp_updated_at, :datetime, null: true + add_column :groups, :smtp_updated_by_id, :integer, null: true + + add_column :groups, :imap_enabled, :boolean, default: false + add_column :groups, :imap_updated_at, :datetime, null: true + add_column :groups, :imap_updated_by_id, :integer, null: true + + DB.exec(<<~SQL) + UPDATE groups SET smtp_enabled = true, smtp_updated_at = NOW(), smtp_updated_by_id = -1 + WHERE smtp_port IS NOT NULL AND smtp_server IS NOT NULL AND email_username IS NOT NULL AND email_password IS NOT NULL AND + smtp_server != '' AND email_username != '' AND email_password != '' + SQL + + DB.exec(<<~SQL) + UPDATE groups SET imap_enabled = true, imap_updated_at = NOW(), imap_updated_by_id = -1 + WHERE imap_port IS NOT NULL AND imap_server IS NOT NULL AND email_username IS NOT NULL AND email_password IS NOT NULL AND + imap_server != '' AND email_username != '' AND email_password != '' + SQL + end + + def down + remove_column :groups, :smtp_enabled + remove_column :groups, :smtp_updated_at + remove_column :groups, :smtp_updated_by_id + + remove_column :groups, :imap_enabled + remove_column :groups, :imap_updated_at + remove_column :groups, :imap_updated_by_id + end +end diff --git a/lib/imap/providers/generic.rb b/lib/imap/providers/generic.rb index e331c32f6ce5ab..ac18ca9f467da0 100644 --- a/lib/imap/providers/generic.rb +++ b/lib/imap/providers/generic.rb @@ -143,6 +143,11 @@ def tag_to_label(tag) end def list_mailboxes(attr_filter = nil) + # Lists all the mailboxes but just returns the names. + list_mailboxes_with_attributes(attr_filter).map(&:name) + end + + def list_mailboxes_with_attributes(attr_filter = nil) # Basically, list all mailboxes in the root of the server. # ref: https://tools.ietf.org/html/rfc3501#section-6.3.8 imap.list('', '*').reject do |m| @@ -162,11 +167,15 @@ def list_mailboxes(attr_filter = nil) else true end - end.map do |m| - m.name end end + def filter_mailboxes(mailboxes) + # we do not want to filter out any mailboxes for generic providers, + # because we do not know what they are ahead of time + mailboxes + end + def archive(uid) # do nothing by default, just removing the Inbox label should be enough end diff --git a/lib/imap/providers/gmail.rb b/lib/imap/providers/gmail.rb index 8400cc5396096a..7ac51f730422b7 100644 --- a/lib/imap/providers/gmail.rb +++ b/lib/imap/providers/gmail.rb @@ -120,6 +120,15 @@ def trash_move(uid) { trash_uid_validity: open_trash_mailbox, email_uids_to_trash: email_uids_to_trash } end + # Some mailboxes are just not useful or advisable to sync with. This is + # used for the dropdown in the UI where we allow the user to select the + # IMAP mailbox to sync with. + def filter_mailboxes(mailboxes_with_attributes) + mailboxes_with_attributes.reject do |mb| + (mb.attr & [:Drafts, :Sent, :Junk, :Flagged, :Trash]).any? + end.map(&:name) + end + private def apply_gmail_patch(imap) diff --git a/spec/fabricators/group_fabricator.rb b/spec/fabricators/group_fabricator.rb index 9f04918300bc15..da0ac73d7f483f 100644 --- a/spec/fabricators/group_fabricator.rb +++ b/spec/fabricators/group_fabricator.rb @@ -13,12 +13,14 @@ smtp_server "smtp.ponyexpress.com" smtp_port 587 smtp_ssl true + smtp_enabled true imap_server "imap.ponyexpress.com" imap_port 993 imap_ssl true imap_mailbox_name "All Mail" imap_uid_validity 0 imap_last_uid 0 + imap_enabled true email_username "discourseteam@ponyexpress.com" email_password "test" end diff --git a/spec/lib/imap/providers/gmail_spec.rb b/spec/lib/imap/providers/gmail_spec.rb index dc6880e2a2196b..014cd1bc1ea477 100644 --- a/spec/lib/imap/providers/gmail_spec.rb +++ b/spec/lib/imap/providers/gmail_spec.rb @@ -70,4 +70,23 @@ provider.archive(main_uid) end end + + describe "#filter_mailboxes" do + it "filters down the gmail mailboxes to only show the relevant ones" do + mailboxes_with_attr = [ + Net::IMAP::MailboxList.new([:Hasnochildren], "/", "INBOX"), + Net::IMAP::MailboxList.new([:All, :Hasnochildren], "/", "[Gmail]/All Mail"), + Net::IMAP::MailboxList.new([:Drafts, :Hasnochildren], "/", "[Gmail]/Drafts"), + Net::IMAP::MailboxList.new([:Hasnochildren, :Important], "/", "[Gmail]/Important"), + Net::IMAP::MailboxList.new([:Hasnochildren, :Sent], "/", "[Gmail]/Sent Mail"), + Net::IMAP::MailboxList.new([:Hasnochildren, :Junk], "/", "[Gmail]/Spam"), + Net::IMAP::MailboxList.new([:Flagged, :Hasnochildren], "/", "[Gmail]/Starred"), + Net::IMAP::MailboxList.new([:Hasnochildren, :Trash], "/", "[Gmail]/Trash") + ] + + expect(provider.filter_mailboxes(mailboxes_with_attr)).to match_array([ + "INBOX", "[Gmail]/All Mail", "[Gmail]/Important" + ]) + end + end end diff --git a/spec/mailers/group_smtp_mailer_spec.rb b/spec/mailers/group_smtp_mailer_spec.rb index 4be00abf5d11ed..5858418a239286 100644 --- a/spec/mailers/group_smtp_mailer_spec.rb +++ b/spec/mailers/group_smtp_mailer_spec.rb @@ -11,9 +11,11 @@ smtp_server: 'smtp.gmail.com', smtp_port: 587, smtp_ssl: true, + smtp_enabled: true, imap_server: 'imap.gmail.com', imap_port: 993, imap_ssl: true, + imap_enabled: true, email_username: 'bugs@gmail.com', email_password: 'super$secret$password' ) @@ -96,9 +98,7 @@ context "when IMAP is disabled for the group" do before do - group.update( - imap_server: nil - ) + group.update(imap_enabled: false) end it "uses the reply key based reply to address" do diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index b2d04acf3c1361..07e7c982650ce0 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -995,6 +995,7 @@ def configure_imap imap_server: "imap.gmail.com", imap_port: 993, imap_ssl: true, + imap_enabled: true, email_username: "test@gmail.com", email_password: "testPassword1!" ) @@ -1003,6 +1004,7 @@ def configure_imap def enable_imap SiteSetting.enable_imap = true @mocked_imap_provider.stubs(:connect!) + @mocked_imap_provider.stubs(:list_mailboxes_with_attributes).returns([stub(attr: [], name: "Inbox")]) @mocked_imap_provider.stubs(:list_mailboxes).returns(["Inbox"]) @mocked_imap_provider.stubs(:disconnect!) end @@ -1011,20 +1013,6 @@ def enable_imap Discourse.redis.del("group_imap_mailboxes_#{group.id}") end - describe "#imap_enabled?" do - it "returns true if imap is configured and enabled for the site" do - mock_imap - configure_imap - enable_imap - expect(group.imap_enabled?).to eq(true) - end - - it "returns false if imap is configured and not enabled for the site" do - configure_imap - expect(group.imap_enabled?).to eq(false) - end - end - describe "#imap_mailboxes" do it "returns an empty array if group imap is not configured" do expect(group.imap_mailboxes).to eq([]) @@ -1194,4 +1182,97 @@ def enable_imap expect(TagUser.lookup(user, :tracking).pluck(:tag_id)).to contain_exactly(tag1.id, tag2.id) end end + + describe "email setting changes" do + it "enables smtp and records the change" do + group.update( + smtp_port: 587, + smtp_ssl: true, + smtp_server: "smtp.gmail.com", + email_username: "test@gmail.com", + email_password: "password", + ) + + group.record_email_setting_changes!(user) + group.reload + + expect(group.smtp_enabled).to eq(true) + expect(group.smtp_updated_at).not_to eq(nil) + expect(group.smtp_updated_by).to eq(user) + end + + it "enables imap and records the change" do + group.update( + imap_port: 587, + imap_ssl: true, + imap_server: "imap.gmail.com", + email_username: "test@gmail.com", + email_password: "password", + ) + + group.record_email_setting_changes!(user) + group.reload + + expect(group.imap_enabled).to eq(true) + expect(group.imap_updated_at).not_to eq(nil) + expect(group.imap_updated_by).to eq(user) + end + + it "disables smtp and records the change" do + group.update( + smtp_port: 587, + smtp_ssl: true, + smtp_server: "smtp.gmail.com", + email_username: "test@gmail.com", + email_password: "password", + smtp_updated_by: user + ) + + group.record_email_setting_changes!(user) + group.reload + + group.update( + smtp_port: nil, + smtp_ssl: false, + smtp_server: nil, + email_username: nil, + email_password: nil, + ) + + group.record_email_setting_changes!(user) + group.reload + + expect(group.smtp_enabled).to eq(false) + expect(group.smtp_updated_at).not_to eq(nil) + expect(group.smtp_updated_by).to eq(user) + end + + it "disables imap and records the change" do + group.update( + imap_port: 587, + imap_ssl: true, + imap_server: "imap.gmail.com", + email_username: "test@gmail.com", + email_password: "password", + ) + + group.record_email_setting_changes!(user) + group.reload + + group.update( + imap_port: nil, + imap_ssl: false, + imap_server: nil, + email_username: nil, + email_password: nil + ) + + group.record_email_setting_changes!(user) + group.reload + + expect(group.imap_enabled).to eq(false) + expect(group.imap_updated_at).not_to eq(nil) + expect(group.imap_updated_by).to eq(user) + end + end end diff --git a/spec/requests/api/schemas/json/group_response.json b/spec/requests/api/schemas/json/group_response.json index b4bf0f15cc9557..7ba352070db85a 100644 --- a/spec/requests/api/schemas/json/group_response.json +++ b/spec/requests/api/schemas/json/group_response.json @@ -140,6 +140,21 @@ "null" ] }, + "smtp_updated_at": { + "type": [ + "string", + "null" + ] + }, + "smtp_updated_by": { + "type": [ + "object", + "null" + ] + }, + "smtp_enabled": { + "type": "boolean" + }, "smtp_server": { "type": [ "string", @@ -158,6 +173,21 @@ "null" ] }, + "imap_enabled": { + "type": "boolean" + }, + "imap_updated_at": { + "type": [ + "string", + "null" + ] + }, + "imap_updated_by": { + "type": [ + "object", + "null" + ] + }, "imap_server": { "type": [ "string", diff --git a/spec/requests/groups_controller_spec.rb b/spec/requests/groups_controller_spec.rb index fd5adca9fe5e16..8a3d46868e44d5 100644 --- a/spec/requests/groups_controller_spec.rb +++ b/spec/requests/groups_controller_spec.rb @@ -1997,4 +1997,116 @@ def expect_type_to_return_right_groups(type, expected_group_ids) ) end end + + describe "#test_email_settings" do + let(:params) do + { + protocol: protocol, + ssl: ssl, + port: port, + host: host, + username: username, + password: password + } + end + + before do + sign_in(user) + group.group_users.where(user: user).last.update(owner: user) + end + + context "validating smtp" do + let(:protocol) { "smtp" } + let(:username) { "test@gmail.com" } + let(:password) { "password" } + let(:domain) { nil } + let(:ssl) { true } + let(:host) { "smtp.somemailsite.com" } + let(:port) { 587 } + + context "when an error is raised" do + before do + EmailSettingsValidator.expects(:validate_smtp).raises(Net::SMTPAuthenticationError, "Invalid credentials") + end + it "uses the friendly error message functionality to return the message to the user" do + post "/groups/#{group.id}/test_email_settings.json", params: params + expect(response.status).to eq(422) + expect(response.parsed_body["errors"]).to include(I18n.t("email_settings.smtp_authentication_error")) + end + end + end + + context "validating imap" do + let(:protocol) { "imap" } + let(:username) { "test@gmail.com" } + let(:password) { "password" } + let(:domain) { nil } + let(:ssl) { true } + let(:host) { "imap.somemailsite.com" } + let(:port) { 993 } + + it "validates with the correct TLS settings" do + EmailSettingsValidator.expects(:validate_imap).with( + has_entry(ssl: true) + ) + post "/groups/#{group.id}/test_email_settings.json", params: params + expect(response.status).to eq(200) + end + + context "when an error is raised" do + before do + EmailSettingsValidator.expects(:validate_imap).raises( + Net::IMAP::NoResponseError, stub(data: stub(text: "Invalid credentials")) + ) + end + it "uses the friendly error message functionality to return the message to the user" do + post "/groups/#{group.id}/test_email_settings.json", params: params + expect(response.status).to eq(422) + expect(response.parsed_body["errors"]).to include(I18n.t("email_settings.imap_authentication_error")) + end + end + end + + describe "global param validation and rate limit" do + let(:protocol) { "smtp" } + let(:host) { "smtp.gmail.com" } + let(:port) { 587 } + let(:username) { "test@gmail.com" } + let(:password) { "password" } + let(:ssl) { true } + + context "when the protocol is not accepted" do + let(:protocol) { "sigma" } + it "raises an invalid params error" do + post "/groups/#{group.id}/test_email_settings.json", params: params + expect(response.status).to eq(400) + expect(response.parsed_body["errors"].first).to match(/Valid protocols to test are smtp and imap/) + end + end + + context "user does not have access to the group" do + before do + group.group_users.destroy_all + end + it "errors if the user does not have access to the group" do + post "/groups/#{group.id}/test_email_settings.json", params: params + + expect(response.status).to eq(403) + end + end + + context "rate limited" do + it "rate limits anon searches per user" do + RateLimiter.enable + RateLimiter.clear_all! + + 5.times do + post "/groups/#{group.id}/test_email_settings.json", params: params + end + post "/groups/#{group.id}/test_email_settings.json", params: params + expect(response.status).to eq(429) + end + end + end + end end diff --git a/spec/services/email_settings_exception_handler_spec.rb b/spec/services/email_settings_exception_handler_spec.rb new file mode 100644 index 00000000000000..56873a76bb0c45 --- /dev/null +++ b/spec/services/email_settings_exception_handler_spec.rb @@ -0,0 +1,100 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe EmailSettingsExceptionHandler do + describe "#friendly_exception_message" do + it "formats a Net::POPAuthenticationError" do + exception = Net::POPAuthenticationError.new("invalid credentials") + expect(subject.class.friendly_exception_message(exception, "pop.test.com")).to eq( + I18n.t("email_settings.pop3_authentication_error") + ) + end + + it "formats a Net::IMAP::NoResponseError for invalid credentials" do + exception = Net::IMAP::NoResponseError.new(stub(data: stub(text: "Invalid credentials"))) + expect(subject.class.friendly_exception_message(exception, "imap.test.com")).to eq( + I18n.t("email_settings.imap_authentication_error") + ) + end + + it "formats a general Net::IMAP::NoResponseError" do + exception = Net::IMAP::NoResponseError.new(stub(data: stub(text: "NO bad problem (Failure)"))) + expect(subject.class.friendly_exception_message(exception, "imap.test.com")).to eq( + I18n.t("email_settings.imap_no_response_error", message: "NO bad problem") + ) + end + + it "formats a general Net::IMAP::NoResponseError with application-specific password Gmail error" do + exception = Net::IMAP::NoResponseError.new(stub(data: stub(text: "NO Application-specific password required"))) + expect(subject.class.friendly_exception_message(exception, "imap.gmail.com")).to eq( + I18n.t("email_settings.authentication_error_gmail_app_password") + ) + end + + it "formats a Net::SMTPAuthenticationError" do + exception = Net::SMTPAuthenticationError.new("invalid credentials") + expect(subject.class.friendly_exception_message(exception, "smtp.test.com")).to eq( + I18n.t("email_settings.smtp_authentication_error") + ) + end + + it "formats a Net::SMTPAuthenticationError with application-specific password Gmail error" do + exception = Net::SMTPAuthenticationError.new("Application-specific password required") + expect(subject.class.friendly_exception_message(exception, "smtp.gmail.com")).to eq( + I18n.t("email_settings.authentication_error_gmail_app_password") + ) + end + + it "formats a Net::SMTPServerBusy" do + exception = Net::SMTPServerBusy.new("call me maybe later") + expect(subject.class.friendly_exception_message(exception, "smtp.test.com")).to eq( + I18n.t("email_settings.smtp_server_busy_error") + ) + end + + it "formats a Net::SMTPSyntaxError, Net::SMTPFatalError, and Net::SMTPUnknownError" do + exception = Net::SMTPSyntaxError.new("bad syntax") + expect(subject.class.friendly_exception_message(exception, "smtp.test.com")).to eq( + I18n.t("email_settings.smtp_unhandled_error", message: exception.message) + ) + exception = Net::SMTPFatalError.new("fatal") + expect(subject.class.friendly_exception_message(exception, "smtp.test.com")).to eq( + I18n.t("email_settings.smtp_unhandled_error", message: exception.message) + ) + exception = Net::SMTPUnknownError.new("unknown") + expect(subject.class.friendly_exception_message(exception, "smtp.test.com")).to eq( + I18n.t("email_settings.smtp_unhandled_error", message: exception.message) + ) + end + + it "formats a SocketError and Errno::ECONNREFUSED" do + exception = SocketError.new("bad socket") + expect(subject.class.friendly_exception_message(exception, "smtp.test.com")).to eq( + I18n.t("email_settings.connection_error") + ) + exception = Errno::ECONNREFUSED.new("no thanks") + expect(subject.class.friendly_exception_message(exception, "smtp.test.com")).to eq( + I18n.t("email_settings.connection_error") + ) + end + + it "formats a Net::OpenTimeout and Net::ReadTimeout error" do + exception = Net::OpenTimeout.new("timed out") + expect(subject.class.friendly_exception_message(exception, "smtp.test.com")).to eq( + I18n.t("email_settings.timeout_error") + ) + exception = Net::ReadTimeout.new("timed out") + expect(subject.class.friendly_exception_message(exception, "smtp.test.com")).to eq( + I18n.t("email_settings.timeout_error") + ) + end + + it "formats unhandled errors" do + exception = StandardError.new("unknown") + expect(subject.class.friendly_exception_message(exception, "smtp.test.com")).to eq( + I18n.t("email_settings.unhandled_error", message: exception.message) + ) + end + end +end diff --git a/spec/services/email_settings_validator_spec.rb b/spec/services/email_settings_validator_spec.rb index fb688b758eb535..1de20827692586 100644 --- a/spec/services/email_settings_validator_spec.rb +++ b/spec/services/email_settings_validator_spec.rb @@ -6,87 +6,6 @@ let(:username) { "kwest@gmail.com" } let(:password) { "mbdtf" } - describe "#friendly_exception_message" do - it "formats a Net::POPAuthenticationError" do - exception = Net::POPAuthenticationError.new("invalid credentials") - expect(subject.class.friendly_exception_message(exception)).to eq( - I18n.t("email_settings.pop3_authentication_error") - ) - end - - it "formats a Net::IMAP::NoResponseError for invalid credentials" do - exception = Net::IMAP::NoResponseError.new(stub(data: stub(text: "Invalid credentials"))) - expect(subject.class.friendly_exception_message(exception)).to eq( - I18n.t("email_settings.imap_authentication_error") - ) - end - - it "formats a general Net::IMAP::NoResponseError" do - exception = Net::IMAP::NoResponseError.new(stub(data: stub(text: "NO bad problem (Failure)"))) - expect(subject.class.friendly_exception_message(exception)).to eq( - I18n.t("email_settings.imap_no_response_error", message: "NO bad problem") - ) - end - - it "formats a Net::SMTPAuthenticationError" do - exception = Net::SMTPAuthenticationError.new("invalid credentials") - expect(subject.class.friendly_exception_message(exception)).to eq( - I18n.t("email_settings.smtp_authentication_error") - ) - end - - it "formats a Net::SMTPServerBusy" do - exception = Net::SMTPServerBusy.new("call me maybe later") - expect(subject.class.friendly_exception_message(exception)).to eq( - I18n.t("email_settings.smtp_server_busy_error") - ) - end - - it "formats a Net::SMTPSyntaxError, Net::SMTPFatalError, and Net::SMTPUnknownError" do - exception = Net::SMTPSyntaxError.new("bad syntax") - expect(subject.class.friendly_exception_message(exception)).to eq( - I18n.t("email_settings.smtp_unhandled_error", message: exception.message) - ) - exception = Net::SMTPFatalError.new("fatal") - expect(subject.class.friendly_exception_message(exception)).to eq( - I18n.t("email_settings.smtp_unhandled_error", message: exception.message) - ) - exception = Net::SMTPUnknownError.new("unknown") - expect(subject.class.friendly_exception_message(exception)).to eq( - I18n.t("email_settings.smtp_unhandled_error", message: exception.message) - ) - end - - it "formats a SocketError and Errno::ECONNREFUSED" do - exception = SocketError.new("bad socket") - expect(subject.class.friendly_exception_message(exception)).to eq( - I18n.t("email_settings.connection_error") - ) - exception = Errno::ECONNREFUSED.new("no thanks") - expect(subject.class.friendly_exception_message(exception)).to eq( - I18n.t("email_settings.connection_error") - ) - end - - it "formats a Net::OpenTimeout and Net::ReadTimeout error" do - exception = Net::OpenTimeout.new("timed out") - expect(subject.class.friendly_exception_message(exception)).to eq( - I18n.t("email_settings.timeout_error") - ) - exception = Net::ReadTimeout.new("timed out") - expect(subject.class.friendly_exception_message(exception)).to eq( - I18n.t("email_settings.timeout_error") - ) - end - - it "formats unhandled errors" do - exception = StandardError.new("unknown") - expect(subject.class.friendly_exception_message(exception)).to eq( - I18n.t("email_settings.unhandled_error", message: exception.message) - ) - end - end - describe "#validate_imap" do let(:host) { "imap.gmail.com" } let(:port) { 993 } @@ -175,6 +94,8 @@ obj.stubs(:finish).returns(true) obj.stubs(:enable_tls).returns(true) obj.stubs(:enable_starttls_auto).returns(true) + obj.stubs(:open_timeout=) + obj.stubs(:read_timeout=) obj end @@ -199,12 +120,12 @@ it "uses the correct ssl verify params for enable_tls if those settings are enabled" do net_smtp_stub.expects(:enable_tls) - expect { subject.class.validate_smtp(host: host, port: port, username: username, password: password, domain: domain, openssl_verify_mode: "peer", enable_tls: true, enable_starttls_auto: false) }.not_to raise_error + expect { subject.class.validate_smtp(host: host, port: 465, username: username, password: password, domain: domain, openssl_verify_mode: "peer", enable_tls: true, enable_starttls_auto: false) }.not_to raise_error end it "uses the correct ssl verify params for enable_starttls_auto if those settings are enabled" do net_smtp_stub.expects(:enable_starttls_auto) - expect { subject.class.validate_smtp(host: host, port: port, username: username, password: password, domain: domain, openssl_verify_mode: "peer", enable_tls: false, enable_starttls_auto: true) }.not_to raise_error + expect { subject.class.validate_smtp(host: host, port: 587, username: username, password: password, domain: domain, openssl_verify_mode: "peer", enable_tls: false, enable_starttls_auto: true) }.not_to raise_error end it "raises an ArgumentError if both enable_tls is true and enable_starttls_auto is true" do @@ -214,5 +135,19 @@ it "raises an ArgumentError if a bad authentication method is used" do expect { subject.class.validate_smtp(host: host, port: port, username: username, password: password, domain: domain, authentication: :rubber_stamp) }.to raise_error(ArgumentError) end + + context "when the domain is not provided" do + let(:domain) { nil } + it "gets the domain from the host" do + net_smtp_stub.expects(:start).with("gmail.com", username, password, :plain) + subject.class.validate_smtp(host: host, port: port, username: username, password: password, enable_tls: true, enable_starttls_auto: false) + end + + it "uses localhost when in development mode" do + Rails.env.stubs(:development?).returns(true) + net_smtp_stub.expects(:start).with("localhost", username, password, :plain) + subject.class.validate_smtp(host: host, port: port, username: username, password: password, enable_tls: true, enable_starttls_auto: false) + end + end end end diff --git a/spec/services/post_alerter_spec.rb b/spec/services/post_alerter_spec.rb index a07ff600540775..609100ad275547 100644 --- a/spec/services/post_alerter_spec.rb +++ b/spec/services/post_alerter_spec.rb @@ -1304,7 +1304,8 @@ def set_topic_notification_level(user, topic, level_name) smtp_server: "imap.gmail.com", smtp_port: 587, email_username: "discourse@example.com", - email_password: "discourse@example.com" + email_password: "discourse@example.com", + smtp_enabled: true ) end