Discource-C
/
discourse
mirror of https://github.com/discourse/discourse.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

FEATURE: Improve group email settings UI (#13083) 

This overhauls the user interface for the group email settings management, aiming to make it a lot easier to test the settings entered and confirm they are correct before proceeding. We do this by forcing the user to test the settings before they can be saved to the database. It also includes some quality of life improvements around setting up IMAP and SMTP for our first supported provider, GMail. This PR does not remove the old group email config, that will come in a subsequent PR. This is related to https://meta.discourse.org/t/imap-support-for-group-inboxes/160588 so read that if you would like more backstory.

### UI

Both site settings of `enable_imap` and `enable_smtp` must be true to test this. You must enable SMTP first to enable IMAP.

You can prefill the SMTP settings with GMail configuration. To proceed with saving these settings you must test them, which is handled by the EmailSettingsValidator.

If there is an issue with the configuration or credentials a meaningful error message should be shown.

IMAP settings must also be validated when IMAP is enabled, before saving.

When saving IMAP, we fetch the mailboxes for that account and populate them. This mailbox must be selected and saved for IMAP to work (the feature acts as though it is disabled until the mailbox is selected and saved):

### Database & Backend

This adds several columns to the Groups table. The purpose of this change is to make it much more explicit that SMTP/IMAP is enabled for a group, rather than relying on settings not being null. Also included is an UPDATE query to backfill these columns. These columns are automatically filled when updating the group.

For GMail, we now filter the mailboxes returned. This is so users cannot use a mailbox like Sent or Trash for syncing, which would generally be disastrous.

There is a new group endpoint for testing email settings. This may be useful in the future for other places in our UI, at which point it can be extracted to a more generic endpoint or module to be included.
This commit is contained in:
Martin Brennan 2021-05-28 09:28:18 +10:00 committed by GitHub
parent 2082abbb75
commit 964da21817
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
41 changed files with 1564 additions and 186 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
app/assets/javascripts/application.js
View File

@ -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

91
app/assets/javascripts/discourse/app/components/group-imap-email-settings.js Normal file
View File

@ -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));
},
});

116
app/assets/javascripts/discourse/app/components/group-manage-email-settings.js Normal file
View File

@ -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();
});
},
});

9
app/assets/javascripts/discourse/app/components/group-manage-save-button.js
View File

@ -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));

82
app/assets/javascripts/discourse/app/components/group-smtp-email-settings.js Normal file
View File

@ -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));
},
});

6
app/assets/javascripts/discourse/app/lib/ajax-error.js
View File

@ -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

22
app/assets/javascripts/discourse/app/lib/email-provider-default-settings.js Normal file
View File

@ -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];
}
}

2
app/assets/javascripts/discourse/app/models/group.js
View File

@ -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,

3
app/assets/javascripts/discourse/app/routes/group-manage-email.js
View File

@ -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");
}
},

80
app/assets/javascripts/discourse/app/templates/components/group-imap-email-settings.hbs Normal file
View File

@ -0,0 +1,80 @@
<div class="group-imap-email-settings">
<form class="groups-form form-horizontal groups-form-imap">
<div>
<div class="control-group">
<label for="imap_server">{{i18n "groups.manage.email.credentials.imap_server"}}</label>
{{input type="text" name="imap_server" value=form.imap_server tabindex="8" onChange=(action "resetSettingsValid")}}
</div>
<label for="enable_ssl_imap">
{{input type="checkbox" checked=form.imap_ssl id="enable_ssl_imap" tabindex="11" onChange=(action "resetSettingsValid")}}
{{i18n "groups.manage.email.credentials.imap_ssl"}}
</label>
</div>
<div>
<div class="control-group">
<label for="imap_port">{{i18n "groups.manage.email.credentials.imap_port"}}</label>
{{input type="text" name="imap_port" value=form.imap_port tabindex="9" onChange=(action "resetSettingsValid" form.imap_port)}}
</div>
</div>
<div>
<div class="control-group group-imap-mailboxes">
{{#if mailboxes}}
<label for="imap_mailbox_name">{{i18n "groups.manage.email.mailboxes.synchronized"}}</label>
{{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}}
</div>
</div>
</form>
<div class="control-group">
<div class="group-imap-prefill-options">
{{i18n "groups.manage.email.prefill.title"}} <a id="prefill_imap_gmail" href {{action "prefillSettings" "gmail"}}>{{i18n "groups.manage.email.prefill.gmail"}}</a>
</div>
</div>
{{#unless mailboxSelected}}
<div class="alert alert-error imap-no-mailbox-selected">
{{i18n "groups.manage.email.imap_mailbox_not_selected"}}
</div>
{{/unless}}
<div class="control-group buttons">
{{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}}
<span class="imap-settings-ok">
{{d-icon "check-circle"}} {{i18n "groups.manage.email.imap_settings_valid"}}
</span>
{{/if}}
</div>
<div class="control-group">
<h3>{{i18n "groups.manage.email.imap_additional_settings"}}</h3>
<label class="control-group-inline" for="allow_unknown_sender_topic_replies">
{{input type="checkbox" name="allow_unknown_sender_topic_replies" id="allow_unknown_sender_topic_replies" checked=group.allow_unknown_sender_topic_replies tabindex="13"}}
<span>{{i18n "groups.manage.email.settings.allow_unknown_sender_topic_replies"}}</span>
</label>
<p>{{i18n "groups.manage.email.settings.allow_unknown_sender_topic_replies_hint"}}</p>
</div>
</div>

38
app/assets/javascripts/discourse/app/templates/components/group-manage-email-settings.hbs Normal file
View File

@ -0,0 +1,38 @@
<div class="group-manage-email-settings">
<h3>{{i18n "groups.manage.email.smtp_title"}}</h3>
<p>{{i18n "groups.manage.email.smtp_instructions"}}</p>
<label for="enable_smtp">
{{input type="checkbox" checked=group.smtp_enabled id="enable_smtp" change=(action "smtpEnabledChange") tabindex="1"}}
{{i18n "groups.manage.email.enable_smtp"}}
</label>
{{#if group.smtp_enabled}}
{{group-smtp-email-settings group=group smtpSettingsValid=smtpSettingsValid}}
{{/if}}
{{#if siteSettings.enable_imap}}
<div class="group-manage-email-imap-wrapper">
<br>
<h3>{{i18n "groups.manage.email.imap_title"}}</h3>
<p>
{{html-safe (i18n "groups.manage.email.imap_instructions")}}
</p>
<div class="alert alert-warning">{{i18n "groups.manage.email.imap_alpha_warning"}}</div>
<label for="enable_imap">
{{input type="checkbox" disabled=(not enableImapSettings) checked=group.imap_enabled id="enable_imap" change=(action "imapEnabledChange") tabindex="8"}}
{{i18n "groups.manage.email.enable_imap"}}
</label>
{{#if group.imap_enabled}}
{{group-imap-email-settings group=group imapSettingsValid=imapSettingsValid}}
{{/if}}
</div>
{{/if}}
<br>
{{group-manage-save-button model=group disabled=(not emailSettingsValid) beforeSave=beforeSave afterSave=afterSave tabindex="14"}}
</div>

4
app/assets/javascripts/discourse/app/templates/components/group-manage-save-button.hbs
View File

@ -1,6 +1,6 @@
<div class="control-group buttons">
<div class="control-group buttons group-manage-save-button">
{{d-button action=(action "save")
disabled=saving
disabled=(or disabled saving)
class="btn btn-primary group-manage-save"
translatedLabel=savingText
}}

58
app/assets/javascripts/discourse/app/templates/components/group-smtp-email-settings.hbs Normal file
View File

@ -0,0 +1,58 @@
<div class="group-smtp-email-settings">
<form class="groups-form form-horizontal">
<div>
<div class="control-group">
<label for="username">{{i18n "groups.manage.email.credentials.username"}}</label>
{{input type="text" name="username" value=form.email_username tabindex="1" onChange=(action "resetSettingsValid")}}
</div>
<div class="control-group">
<label for="smtp_server">{{i18n "groups.manage.email.credentials.smtp_server"}}</label>
{{input type="text" name="smtp_server" value=form.smtp_server tabindex="3" onChange=(action "resetSettingsValid")}}
</div>
<label for="enable_ssl">
{{input type="checkbox" checked=form.smtp_ssl id="enable_ssl" tabindex="5" onChange=(action "resetSettingsValid")}}
{{i18n "groups.manage.email.credentials.smtp_ssl"}}
</label>
</div>
<div>
<div class="control-group">
<label for="password">{{i18n "groups.manage.email.credentials.password"}}</label>
{{input type="password" name="password" value=form.email_password tabindex="2" onChange=(action "resetSettingsValid")}}
</div>
<div class="control-group">
<label for="smtp_port">{{i18n "groups.manage.email.credentials.smtp_port"}}</label>
{{input type="text" name="smtp_port" value=form.smtp_port tabindex="4" onChange=(action "resetSettingsValid" form.smtp_port)}}
</div>
</div>
</form>
<div class="control-group">
<div class="group-smtp-prefill-options">
{{i18n "groups.manage.email.prefill.title"}} <a id="prefill_smtp_gmail" href {{action "prefillSettings" "gmail"}}>{{i18n "groups.manage.email.prefill.gmail"}}</a>
</div>
</div>
<div class="control-group buttons">
{{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}}
<span class="smtp-settings-ok">
{{d-icon "check-circle"}} {{i18n "groups.manage.email.smtp_settings_valid"}}
</span>
{{/if}}
</div>
</div>

5
app/assets/javascripts/discourse/app/templates/group/manage/email.hbs
View File

@ -1,4 +1 @@
<form class="groups-form form-vertical">
{{groups-form-email-fields model=model}}
{{group-manage-save-button model=model}}
</form>
{{group-manage-email-settings group=model}}

230
app/assets/javascripts/discourse/tests/acceptance/group-manage-email-settings-test.js Normal file
View File

@ -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");
});
}
);

6
app/assets/javascripts/discourse/tests/fixtures/group-fixtures.js vendored
View File

@ -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"]

4
app/assets/stylesheets/common/base/alert.scss
View File

@ -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);

24
app/assets/stylesheets/common/base/group.scss
View File

@ -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%;
}
}
}

17
app/assets/stylesheets/mobile/group.scss
View File

@ -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;
}
}
}

82
app/controllers/groups_controller.rb
View File

@ -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 @@ class GroupsController < ApplicationController
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 @@ class GroupsController < ApplicationController
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 @@ class GroupsController < ApplicationController
: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 @@ class GroupsController < ApplicationController
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

2
app/mailers/group_smtp_mailer.rb
View File

@ -48,7 +48,7 @@ class GroupSmtpMailer < ActionMailer::Base
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,

69
app/models/group.rb
View File

@ -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 @@ class Group < ActiveRecord::Base
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 @@ class Group < ActiveRecord::Base
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 @@ class Group < ActiveRecord::Base
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 @@ class Group < ActiveRecord::Base
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 @@ class Group < ActiveRecord::Base
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 @@ class Group < ActiveRecord::Base
}
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 @@ end
# 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 @@ end
# 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
#

9
app/serializers/group_show_serializer.rb
View File

@ -12,15 +12,24 @@ class GroupShowSerializer < BasicGroupSerializer
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,

128
app/services/email_settings_exception_handler.rb Normal file
View File

@ -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

91
app/services/email_settings_validator.rb
View File

@ -12,52 +12,13 @@ require 'net/pop'
# # 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 @@ class EmailSettingsValidator
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 @@ class EmailSettingsValidator
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 @@ class EmailSettingsValidator
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 @@ class EmailSettingsValidator
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 @@ class EmailSettingsValidator
port:,
username:,
password:,
open_timeout: 10,
open_timeout: 5,
ssl: true,
debug: false
)
@ -188,4 +167,20 @@ class EmailSettingsValidator
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

8
app/services/post_alerter.rb
View File

@ -579,13 +579,7 @@ class PostAlerter
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)

21
config/locales/client.en.yml
View File

@ -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 <a target=\"_blank\" href=\"https://meta.discourse.org/t/imap-support-for-group-inboxes/160588\">feature announcement on Discourse Meta</a>."
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

2
config/locales/server.en.yml
View File

@ -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 <a target=\"_blank\" href=\"https://support.google.com/accounts/answer/185833\">this Google Help article</a>"
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}"

1
config/routes.rb
View File

@ -568,6 +568,7 @@ Discourse::Application.routes.draw do
get 'mentionable'
get 'messageable'
get 'logs' => 'groups#histories'
post 'test_email_settings'
collection do
get "check-name" => 'groups#check_name'

35
db/migrate/20210517061815_add_dedicated_enable_imap_smtp_columns_for_group.rb Normal file
View File

@ -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

13
lib/imap/providers/generic.rb
View File

@ -143,6 +143,11 @@ module Imap
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 @@ module Imap
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

9
lib/imap/providers/gmail.rb
View File

@ -120,6 +120,15 @@ module Imap
{ 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)

2
spec/fabricators/group_fabricator.rb
View File

@ -13,12 +13,14 @@ Fabricator(:imap_group, from: :group) do
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

19
spec/lib/imap/providers/gmail_spec.rb
View File

@ -70,4 +70,23 @@ RSpec.describe Imap::Providers::Gmail do
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

6
spec/mailers/group_smtp_mailer_spec.rb
View File

@ -11,9 +11,11 @@ describe GroupSmtpMailer do
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 @@ describe GroupSmtpMailer do
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

109
spec/models/group_spec.rb
View File

@ -995,6 +995,7 @@ describe Group do
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 @@ describe Group do
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 @@ describe Group do
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 @@ describe Group do
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

30
spec/requests/api/schemas/json/group_response.json
View File

@ -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",

112
spec/requests/groups_controller_spec.rb
View File

@ -1997,4 +1997,116 @@ describe GroupsController do
)
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

100
spec/services/email_settings_exception_handler_spec.rb Normal file
View File

@ -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

101
spec/services/email_settings_validator_spec.rb
View File

@ -6,87 +6,6 @@ RSpec.describe EmailSettingsValidator do
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 @@ RSpec.describe EmailSettingsValidator do
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 @@ RSpec.describe EmailSettingsValidator do
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 @@ RSpec.describe EmailSettingsValidator do
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

3
spec/services/post_alerter_spec.rb
View File

@ -1304,7 +1304,8 @@ describe PostAlerter do
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