diff --git a/app/assets/javascripts/discourse/app/components/group-manage-email-settings.hbs b/app/assets/javascripts/discourse/app/components/group-manage-email-settings.hbs index 38a3119d37a..0948b6e16d5 100644 --- a/app/assets/javascripts/discourse/app/components/group-manage-email-settings.hbs +++ b/app/assets/javascripts/discourse/app/components/group-manage-email-settings.hbs @@ -17,6 +17,7 @@ {{/if}} 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 index 8eaef3a7a03..38ef87ca910 100644 --- a/app/assets/javascripts/discourse/app/components/group-manage-email-settings.js +++ b/app/assets/javascripts/discourse/app/components/group-manage-email-settings.js @@ -65,6 +65,11 @@ export default Component.extend({ ); }, + @action + onChangeSmtpSettingsValid(valid) { + this.set("smtpSettingsValid", valid); + }, + @action smtpEnabledChange(event) { if ( diff --git a/app/assets/javascripts/discourse/app/components/group-smtp-email-settings.gjs b/app/assets/javascripts/discourse/app/components/group-smtp-email-settings.gjs new file mode 100644 index 00000000000..193bf6eb8a4 --- /dev/null +++ b/app/assets/javascripts/discourse/app/components/group-smtp-email-settings.gjs @@ -0,0 +1,308 @@ +import Component from "@glimmer/component"; +import { tracked } from "@glimmer/tracking"; +import { fn } from "@ember/helper"; +import { on } from "@ember/modifier"; +import { action } from "@ember/object"; +import { LinkTo } from "@ember/routing"; +import { inject as service } from "@ember/service"; +import { isEmpty } from "@ember/utils"; +import { TrackedObject } from "@ember-compat/tracked-built-ins"; +import { or } from "truth-helpers"; +import ConditionalLoadingSpinner from "discourse/components/conditional-loading-spinner"; +import DButton from "discourse/components/d-button"; +import formatDate from "discourse/helpers/format-date"; +import withEventValue from "discourse/helpers/with-event-value"; +import { ajax } from "discourse/lib/ajax"; +import { popupAjaxError } from "discourse/lib/ajax-error"; +import { GROUP_SMTP_SSL_MODES } from "discourse/lib/constants"; +import emailProviderDefaultSettings from "discourse/lib/email-provider-default-settings"; +import dIcon from "discourse-common/helpers/d-icon"; +import i18n from "discourse-common/helpers/i18n"; +import I18n from "I18n"; +import ComboBox from "select-kit/components/combo-box"; + +export default class GroupSmtpEmailSettings extends Component { + @service currentUser; + + @tracked smtpSettingsValid = false; + @tracked testingSettings = false; + + form = new TrackedObject({ + email_username: this.args.group.email_username, + email_password: this.args.group.email_password, + email_from_alias: this.args.group.email_from_alias, + smtp_server: this.args.group.smtp_server, + smtp_port: (this.args.group.smtp_port || "").toString(), + smtp_ssl_mode: this.args.group.smtp_ssl_mode || GROUP_SMTP_SSL_MODES.none, + }); + + get sslModes() { + return Object.keys(GROUP_SMTP_SSL_MODES).map((key) => { + return { + value: GROUP_SMTP_SSL_MODES[key], + name: I18n.t(`groups.manage.email.ssl_modes.${key}`), + }; + }); + } + + get missingSettings() { + if (!this.form) { + return true; + } + return [ + this.form.email_username, + this.form.email_password, + this.form.smtp_server, + this.form.smtp_port, + ].some((value) => isEmpty(value)); + } + + @action + changeSmtpSettingsValid(newValidValue) { + this.smtpSettingsValid = newValidValue; + this.args.onChangeSmtpSettingsValid(newValidValue); + } + + @action + onChangeSslMode(newMode) { + this.form.smtp_ssl_mode = newMode; + this.changeSmtpSettingsValid(false); + } + + @action + changeEmailUsername(newValue) { + this.form.email_username = newValue; + this.changeSmtpSettingsValid(false); + } + + @action + changeEmailPassword(newValue) { + this.form.email_password = newValue; + this.changeSmtpSettingsValid(false); + } + + @action + changeEmailFromAlias(newValue) { + this.form.email_from_alias = newValue; + this.changeSmtpSettingsValid(false); + } + + @action + changeSmtpServer(newValue) { + this.form.smtp_server = newValue; + this.changeSmtpSettingsValid(false); + } + + @action + changeSmtpPort(newValue) { + this.form.smtp_port = newValue; + this.changeSmtpSettingsValid(false); + } + + @action + prefillSettings(provider, event) { + event?.preventDefault(); + Object.assign(this.form, emailProviderDefaultSettings(provider, "smtp")); + } + + @action + testSmtpSettings() { + const settings = { + host: this.form.smtp_server, + port: this.form.smtp_port, + ssl_mode: this.form.smtp_ssl_mode, + username: this.form.email_username, + password: this.form.email_password, + }; + + this.testingSettings = true; + this.changeSmtpSettingsValid(false); + + return ajax(`/groups/${this.args.group.id}/test_email_settings`, { + type: "POST", + data: Object.assign(settings, { protocol: "smtp" }), + }) + .then(() => { + this.changeSmtpSettingsValid(true); + this.args.group.setProperties({ + smtp_server: this.form.smtp_server, + smtp_port: this.form.smtp_port, + smtp_ssl_mode: this.form.smtp_ssl_mode, + email_username: this.form.email_username, + email_from_alias: this.form.email_from_alias, + email_password: this.form.email_password, + }); + }) + .catch(popupAjaxError) + .finally(() => (this.testingSettings = false)); + } + + +} diff --git a/app/assets/javascripts/discourse/app/components/group-smtp-email-settings.hbs b/app/assets/javascripts/discourse/app/components/group-smtp-email-settings.hbs deleted file mode 100644 index a5f4ef7b9a7..00000000000 --- a/app/assets/javascripts/discourse/app/components/group-smtp-email-settings.hbs +++ /dev/null @@ -1,139 +0,0 @@ -
-
-
-
- - -
- -
- - -
- - -
- -
-
- - -
- -
- - -
-
- -
-
- - -

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

-
-
-
- -
-
- {{i18n "groups.manage.email.prefill.title"}} - {{i18n "groups.manage.email.prefill.gmail"}} -
-
- -
- - - - - {{#if this.smtpSettingsValid}} - - {{d-icon "check-circle"}} - {{i18n "groups.manage.email.smtp_settings_valid"}} - - {{/if}} -
- - {{#if this.group.smtp_updated_at}} -
- - {{i18n "groups.manage.email.last_updated"}} - {{format-date - this.group.smtp_updated_at - leaveAgo="true" - }} - {{i18n "groups.manage.email.last_updated_by"}} - {{this.group.smtp_updated_by.username}} - -
- {{/if}} -
\ No newline at end of file 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 deleted file mode 100644 index 1f0ca0cfc66..00000000000 --- a/app/assets/javascripts/discourse/app/components/group-smtp-email-settings.js +++ /dev/null @@ -1,82 +0,0 @@ -import Component from "@ember/component"; -import EmberObject, { action } from "@ember/object"; -import { isEmpty } from "@ember/utils"; -import { ajax } from "discourse/lib/ajax"; -import { popupAjaxError } from "discourse/lib/ajax-error"; -import emailProviderDefaultSettings from "discourse/lib/email-provider-default-settings"; -import discourseComputed, { on } from "discourse-common/utils/decorators"; - -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, - email_from_alias: this.group.email_from_alias, - smtp_server: this.group.smtp_server, - smtp_port: (this.group.smtp_port || "").toString(), - smtp_ssl: this.group.smtp_ssl, - }) - ); - }, - - @action - prefillSettings(provider, event) { - event?.preventDefault(); - 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_from_alias: this.form.email_from_alias, - email_password: this.form.email_password, - }); - }) - .catch(popupAjaxError) - .finally(() => this.set("testingSettings", false)); - }, -}); diff --git a/app/assets/javascripts/discourse/app/lib/constants.js b/app/assets/javascripts/discourse/app/lib/constants.js index bbc9f9fae81..3dd3f53935f 100644 --- a/app/assets/javascripts/discourse/app/lib/constants.js +++ b/app/assets/javascripts/discourse/app/lib/constants.js @@ -69,6 +69,8 @@ export const AUTO_GROUPS = { }, }; +export const GROUP_SMTP_SSL_MODES = { none: 0, ssl_tls: 1, starttls: 2 }; + export const MAX_NOTIFICATIONS_LIMIT_PARAMS = 60; export const TOPIC_VISIBILITY_REASONS = { 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 index fa2939f6ace..1176623042b 100644 --- a/app/assets/javascripts/discourse/app/lib/email-provider-default-settings.js +++ b/app/assets/javascripts/discourse/app/lib/email-provider-default-settings.js @@ -1,3 +1,5 @@ +import { GROUP_SMTP_SSL_MODES } from "discourse/lib/constants"; + const GMAIL = { imap: { imap_server: "imap.gmail.com", @@ -7,7 +9,23 @@ const GMAIL = { smtp: { smtp_server: "smtp.gmail.com", smtp_port: "587", - smtp_ssl: true, + smtp_ssl_mode: GROUP_SMTP_SSL_MODES.starttls, + }, +}; + +const OUTLOOK = { + smtp: { + smtp_server: "smtp-mail.outlook.com", + smtp_port: "587", + smtp_ssl_mode: GROUP_SMTP_SSL_MODES.starttls, + }, +}; + +const OFFICE365 = { + smtp: { + smtp_server: "smtp.office365.com", + smtp_port: "587", + smtp_ssl_mode: GROUP_SMTP_SSL_MODES.starttls, }, }; @@ -18,5 +36,9 @@ export default function emailProviderDefaultSettings(provider, protocol) { switch (provider) { case "gmail": return GMAIL[protocol]; + case "office365": + return OFFICE365[protocol]; + case "outlook": + return OUTLOOK[protocol]; } } diff --git a/app/assets/javascripts/discourse/app/models/group.js b/app/assets/javascripts/discourse/app/models/group.js index e42de3e3585..2fc65d3b2b1 100644 --- a/app/assets/javascripts/discourse/app/models/group.js +++ b/app/assets/javascripts/discourse/app/models/group.js @@ -344,7 +344,7 @@ export default class Group extends RestModel { incoming_email: this.incoming_email, smtp_server: this.smtp_server, smtp_port: this.smtp_port, - smtp_ssl: this.smtp_ssl, + smtp_ssl_mode: this.smtp_ssl_mode, smtp_enabled: this.smtp_enabled, imap_server: this.imap_server, imap_port: this.imap_port, 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 index 48207de0850..9d8adb3fdd8 100644 --- 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 @@ -1,5 +1,6 @@ import { click, currentRouteName, fillIn, visit } from "@ember/test-helpers"; import { test } from "qunit"; +import { GROUP_SMTP_SSL_MODES } from "discourse/lib/constants"; import { acceptance, exists, @@ -85,22 +86,23 @@ acceptance( await click("#prefill_smtp_gmail"); assert.strictEqual( - query("input[name='smtp_server']").value, + query(".group-smtp-form__smtp-server").value, "smtp.gmail.com", "prefills SMTP server settings for gmail" ); assert.strictEqual( - query("input[name='smtp_port']").value, + query(".group-smtp-form__smtp-port").value, "587", "prefills SMTP port settings for gmail" ); - assert.ok( - exists("#enable_ssl:checked"), - "prefills SMTP ssl settings for gmail" + assert.strictEqual( + selectKit(".group-smtp-form__smtp-ssl-mode").header().value(), + GROUP_SMTP_SSL_MODES.starttls.toString(), + "prefills SSL mode to STARTTLS for gmail" ); assert.ok( - exists(".test-smtp-settings:disabled"), + exists(".group-smtp-form__test-smtp-settings:disabled"), "does not allow testing settings if not all fields are filled" ); @@ -108,9 +110,12 @@ acceptance( await fillIn('input[name="password"]', "password@gmail.com"); await fillIn("#from_alias", "akasomegroup@example.com"); - await click(".test-smtp-settings"); + await click(".group-smtp-form__test-smtp-settings"); - assert.ok(exists(".smtp-settings-ok"), "tested settings are ok"); + assert.ok( + exists(".group-smtp-form__smtp-settings-ok"), + "tested settings are ok" + ); await click(".group-manage-save"); @@ -141,7 +146,7 @@ acceptance( 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-smtp-form__test-smtp-settings"); await click(".group-manage-save"); assert.notOk( @@ -168,7 +173,7 @@ acceptance( "prefills IMAP port settings for gmail" ); assert.ok( - exists("#enable_ssl:checked"), + exists("#enable_ssl_imap:checked"), "prefills IMAP ssl settings for gmail" ); await click(".test-imap-settings"); @@ -239,7 +244,7 @@ acceptance( message_count: 2, smtp_server: "smtp.gmail.com", smtp_port: 587, - smtp_ssl: true, + smtp_ssl_mode: GROUP_SMTP_SSL_MODES.starttls, smtp_enabled: true, smtp_updated_at: "2021-06-16T02:58:12.739Z", smtp_updated_by: { @@ -324,13 +329,6 @@ acceptance( ), "shows last updated imap details" ); - assert.ok(exists(".group-email-last-updated-details.for-smtp")); - assert.ok( - regex.test( - query(".group-email-last-updated-details.for-smtp").innerText.trim() - ), - "shows last updated smtp details" - ); }); } ); @@ -359,7 +357,7 @@ acceptance( 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-smtp-form__test-smtp-settings"); assert.strictEqual( query(".dialog-body").innerText.trim(), diff --git a/app/assets/stylesheets/common/base/group.scss b/app/assets/stylesheets/common/base/group.scss index 2cffb6217fb..4a196868ee7 100644 --- a/app/assets/stylesheets/common/base/group.scss +++ b/app/assets/stylesheets/common/base/group.scss @@ -269,6 +269,25 @@ table.group-category-permissions { } } +.group-smtp-prefill-options { + ul { + display: inline; + margin: 0; + + li { + display: inline-block; + + &:before { + content: "|"; + } + + &:first-child:before { + content: ""; + } + } + } +} + .group-smtp-email-settings, .group-imap-email-settings { background-color: var(--primary-very-low); @@ -289,10 +308,6 @@ table.group-category-permissions { } } -.groups-form__enable-ssl { - margin-bottom: 1em; -} - .group-manage-email-additional-settings-wrapper { margin-top: 1em; } diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 01b0f4a2cee..b638b352088 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -715,7 +715,6 @@ class GroupsController < ApplicationController params.require(:host) params.require(:username) params.require(:password) - params.require(:ssl) group = Group.find(params[:group_id]) guardian.ensure_can_edit!(group) @@ -723,7 +722,6 @@ class GroupsController < ApplicationController 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 !%w[smtp imap].include?(params[:protocol]) @@ -734,13 +732,19 @@ class GroupsController < ApplicationController begin case params[:protocol] when "smtp" - enable_starttls_auto = false - settings.delete(:ssl) + raise Discourse::InvalidParameters if params[:ssl_mode].blank? + + settings.delete(:ssl_mode) + + if params[:ssl_mode].blank? || + !Group.smtp_ssl_modes.values.include?(params[:ssl_mode].to_i) + raise Discourse::InvalidParameters.new("SSL mode must be present and valid") + end final_settings = settings.merge( - enable_tls: enable_tls, - enable_starttls_auto: enable_starttls_auto, + enable_tls: params[:ssl_mode].to_i == Group.smtp_ssl_modes[:ssl_tls], + enable_starttls_auto: params[:ssl_mode].to_i == Group.smtp_ssl_modes[:starttls], ).permit(:host, :port, :username, :password, :enable_tls, :enable_starttls_auto, :debug) EmailSettingsValidator.validate_as_user( current_user, @@ -748,8 +752,17 @@ class GroupsController < ApplicationController **final_settings.to_h.symbolize_keys, ) when "imap" + raise Discourse::InvalidParameters if params[:ssl].blank? + final_settings = - settings.merge(ssl: enable_tls).permit(:host, :port, :username, :password, :ssl, :debug) + settings.merge(ssl: settings[:ssl] == "true").permit( + :host, + :port, + :username, + :password, + :ssl, + :debug, + ) EmailSettingsValidator.validate_as_user( current_user, "imap", @@ -812,7 +825,7 @@ class GroupsController < ApplicationController :incoming_email, :smtp_server, :smtp_port, - :smtp_ssl, + :smtp_ssl_mode, :smtp_enabled, :smtp_updated_by, :smtp_updated_at, @@ -890,7 +903,7 @@ class GroupsController < ApplicationController if should_clear_smtp attributes[:smtp_server] = nil - attributes[:smtp_ssl] = false + attributes[:smtp_ssl_mode] = false attributes[:smtp_port] = nil attributes[:email_username] = nil attributes[:email_password] = nil diff --git a/app/mailers/group_smtp_mailer.rb b/app/mailers/group_smtp_mailer.rb index 7b6cc26b44d..d1d7ef71762 100644 --- a/app/mailers/group_smtp_mailer.rb +++ b/app/mailers/group_smtp_mailer.rb @@ -18,7 +18,8 @@ class GroupSmtpMailer < ActionMailer::Base # NOTE: Might be better at some point to store this authentication method in the database # against the group. authentication: SmtpProviderOverrides.authentication_override(from_group.smtp_server), - enable_starttls_auto: from_group.smtp_ssl, + enable_starttls_auto: from_group.smtp_ssl_mode == Group.smtp_ssl_modes[:starttls], + enable_ssl: from_group.smtp_ssl_mode == Group.smtp_ssl_modes[:ssl_tls], return_response: true, open_timeout: GlobalSetting.group_smtp_open_timeout, read_timeout: GlobalSetting.group_smtp_read_timeout, diff --git a/app/models/group.rb b/app/models/group.rb index 445df74c5df..3c4b8ff78d0 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -3,7 +3,9 @@ require "net/imap" class Group < ActiveRecord::Base - self.ignored_columns = %w[flair_url] # TODO: Remove when 20240212034010_drop_deprecated_columns has been promoted to pre-deploy + # TODO: Remove flair_url when 20240212034010_drop_deprecated_columns has been promoted to pre-deploy + # TODO: Remove smtp_ssl when db/post_migrate/20240717053710_drop_groups_smtp_ssl has been promoted to pre-deploy + self.ignored_columns = %w[flair_url smtp_ssl] include HasCustomFields include AnonCacheInvalidator @@ -145,6 +147,10 @@ class Group < ActiveRecord::Base @visibility_levels = Enum.new(public: 0, logged_on_users: 1, members: 2, staff: 3, owners: 4) end + def self.smtp_ssl_modes + @visibility_levels = Enum.new(none: 0, ssl_tls: 1, starttls: 2) + end + def self.auto_groups_between(lower, upper) lower_group = Group::AUTO_GROUPS[lower.to_sym] upper_group = Group::AUTO_GROUPS[upper.to_sym] @@ -1292,7 +1298,6 @@ end # mentionable_level :integer default(0) # smtp_server :string # smtp_port :integer -# smtp_ssl :boolean # imap_server :string # imap_port :integer # imap_ssl :boolean @@ -1316,6 +1321,7 @@ end # imap_updated_at :datetime # imap_updated_by_id :integer # email_from_alias :string +# smtp_ssl_mode :integer default(0), not null # # Indexes # diff --git a/app/serializers/group_show_serializer.rb b/app/serializers/group_show_serializer.rb index f70c379a4a2..9b5015678ec 100644 --- a/app/serializers/group_show_serializer.rb +++ b/app/serializers/group_show_serializer.rb @@ -24,7 +24,7 @@ class GroupShowSerializer < BasicGroupSerializer admin_attributes :automatic_membership_email_domains, :smtp_server, :smtp_port, - :smtp_ssl, + :smtp_ssl_mode, :smtp_enabled, :smtp_updated_at, :smtp_updated_by, diff --git a/app/services/email_settings_validator.rb b/app/services/email_settings_validator.rb index f7227ce2a9c..0d2ee6899f3 100644 --- a/app/services/email_settings_validator.rb +++ b/app/services/email_settings_validator.rb @@ -62,9 +62,6 @@ class EmailSettingsValidator # Attempts to start an SMTP session and if that raises an error then it is # assumed the credentials or other settings are wrong. # - # For Gmail, the port should be 587, enable_starttls_auto should be true, - # and enable_tls should be false. - # # @param domain [String] - Used for HELO, should be the FQDN of the server sending the mail # localhost can be used in development mode. # See https://datatracker.ietf.org/doc/html/rfc788#section-4 @@ -83,9 +80,6 @@ class EmailSettingsValidator debug: Rails.env.development? ) begin - port, enable_tls, enable_starttls_auto = - SmtpProviderOverrides.ssl_override(host, port, enable_tls, enable_starttls_auto) - if enable_tls && enable_starttls_auto raise ArgumentError, "TLS and STARTTLS are mutually exclusive" end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index d1d4fe84480..ff700b26582 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -873,11 +873,17 @@ en: prefill: title: "Prefill with settings for:" gmail: "Gmail" + outlook: "Outlook.com" + office365: "Microsoft 365" + ssl_modes: + none: "None" + ssl_tls: "SSL/TLS" + starttls: "STARTTLS" credentials: title: "Credentials" smtp_server: "SMTP Server" smtp_port: "SMTP Port" - smtp_ssl: "Use SSL for SMTP" + smtp_ssl_mode: "SSL Mode" imap_server: "IMAP Server" imap_port: "IMAP Port" imap_ssl: "Use SSL for IMAP" diff --git a/db/migrate/20240715073605_add_smtp_ssl_mode_to_groups.rb b/db/migrate/20240715073605_add_smtp_ssl_mode_to_groups.rb new file mode 100644 index 00000000000..4de8b4b11a7 --- /dev/null +++ b/db/migrate/20240715073605_add_smtp_ssl_mode_to_groups.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true +class AddSmtpSslModeToGroups < ActiveRecord::Migration[7.1] + def up + add_column :groups, :smtp_ssl_mode, :integer, default: 0, null: false + + execute <<~SQL + UPDATE groups SET smtp_ssl_mode = (CASE WHEN smtp_ssl THEN 2 ELSE 0 END) + SQL + + Migration::ColumnDropper.mark_readonly(:groups, :smtp_ssl) + end + + def down + Migration::ColumnDropper.drop_readonly(:groups, :smtp_ssl) + remove_column :groups, :smtp_ssl_mode + end +end diff --git a/db/post_migrate/20240717053710_drop_groups_smtp_ssl.rb b/db/post_migrate/20240717053710_drop_groups_smtp_ssl.rb new file mode 100644 index 00000000000..3c4f2faf830 --- /dev/null +++ b/db/post_migrate/20240717053710_drop_groups_smtp_ssl.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class DropGroupsSmtpSsl < ActiveRecord::Migration[7.1] + DROPPED_COLUMNS ||= { groups: %i[smtp_ssl] } + + def up + DROPPED_COLUMNS.each { |table, columns| Migration::ColumnDropper.execute_drop(table, columns) } + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/lib/smtp_provider_overrides.rb b/lib/smtp_provider_overrides.rb index 0263ba4e06f..6c33ee34ccd 100644 --- a/lib/smtp_provider_overrides.rb +++ b/lib/smtp_provider_overrides.rb @@ -9,20 +9,4 @@ class SmtpProviderOverrides return "login" if %w[smtp.office365.com smtp-mail.outlook.com].include?(host) GlobalSetting.smtp_authentication end - - def self.ssl_override(host, port, enable_tls, enable_starttls_auto) - # Certain mail servers act 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 %w[smtp.gmail.com smtp.office365.com smtp-mail.outlook.com].include?(host) - 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/lib/tasks/javascript.rake b/lib/tasks/javascript.rake index 0c23adb50a3..b06733c89f8 100644 --- a/lib/tasks/javascript.rake +++ b/lib/tasks/javascript.rake @@ -154,6 +154,8 @@ task "javascript:update_constants" => :environment do export const AUTO_GROUPS = #{auto_groups.to_json}; + export const GROUP_SMTP_SSL_MODES = #{Group.smtp_ssl_modes.to_json}; + export const MAX_NOTIFICATIONS_LIMIT_PARAMS = #{NotificationsController::INDEX_LIMIT}; export const TOPIC_VISIBILITY_REASONS = #{Topic.visibility_reasons.to_json}; diff --git a/plugins/automation/spec/triggers/pm_created_spec.rb b/plugins/automation/spec/triggers/pm_created_spec.rb index ec62517cd9c..5cd32144dec 100644 --- a/plugins/automation/spec/triggers/pm_created_spec.rb +++ b/plugins/automation/spec/triggers/pm_created_spec.rb @@ -164,7 +164,7 @@ describe "PMCreated" do incoming_email: "team@somesmtpaddress.com|suppor+team@bar.com", smtp_server: "smtp.test.com", smtp_port: 587, - smtp_ssl: true, + smtp_ssl_mode: Group.smtp_ssl_modes[:starttls], smtp_enabled: true, ) SiteSetting.email_in = true diff --git a/spec/fabricators/group_fabricator.rb b/spec/fabricators/group_fabricator.rb index c0a13a3f39d..bbc4d5df0b6 100644 --- a/spec/fabricators/group_fabricator.rb +++ b/spec/fabricators/group_fabricator.rb @@ -10,7 +10,7 @@ end Fabricator(:imap_group, from: :group) do smtp_server "smtp.ponyexpress.com" smtp_port 587 - smtp_ssl true + smtp_ssl_mode Group.smtp_ssl_modes[:starttls] smtp_enabled true imap_server "imap.ponyexpress.com" imap_port 993 @@ -26,7 +26,7 @@ end Fabricator(:smtp_group, from: :group) do smtp_server "smtp.ponyexpress.com" smtp_port 587 - smtp_ssl true + smtp_ssl_mode Group.smtp_ssl_modes[:starttls] smtp_enabled true email_username "discourseteam@ponyexpress.com" email_password "test" diff --git a/spec/lib/email/receiver_spec.rb b/spec/lib/email/receiver_spec.rb index aa8ed472fff..db28035f47a 100644 --- a/spec/lib/email/receiver_spec.rb +++ b/spec/lib/email/receiver_spec.rb @@ -740,7 +740,7 @@ RSpec.describe Email::Receiver do Please find some text file attached. [details="#{I18n.t("emails.incoming.attachments")}"] - + #{UploadMarkdown.new(upload).to_markdown} [/details] @@ -1223,7 +1223,7 @@ RSpec.describe Email::Receiver do incoming_email: "team@somesmtpaddress.com|support+team@bar.com", smtp_server: "smtp.test.com", smtp_port: 587, - smtp_ssl: true, + smtp_ssl_mode: Group.smtp_ssl_modes[:starttls], smtp_enabled: true, ) end @@ -1318,7 +1318,7 @@ RSpec.describe Email::Receiver do incoming_email: "team@somesmtpaddress.com|suppor+team@bar.com", smtp_server: "smtp.test.com", smtp_port: 587, - smtp_ssl: true, + smtp_ssl_mode: Group.smtp_ssl_modes[:starttls], smtp_enabled: true, ) process(:email_to_group_email_username_1) diff --git a/spec/mailers/group_smtp_mailer_spec.rb b/spec/mailers/group_smtp_mailer_spec.rb index 5d5f1de2656..10ca7f1c6dd 100644 --- a/spec/mailers/group_smtp_mailer_spec.rb +++ b/spec/mailers/group_smtp_mailer_spec.rb @@ -10,7 +10,7 @@ RSpec.describe GroupSmtpMailer do full_name: "Testers Group", smtp_server: "smtp.gmail.com", smtp_port: 587, - smtp_ssl: true, + smtp_ssl_mode: Group.smtp_ssl_modes[:starttls], smtp_enabled: true, imap_server: "imap.gmail.com", imap_port: 993, @@ -128,6 +128,7 @@ RSpec.describe GroupSmtpMailer do password: "super$secret$password", authentication: GlobalSetting.smtp_authentication, enable_starttls_auto: true, + enable_ssl: false, return_response: true, open_timeout: GlobalSetting.group_smtp_open_timeout, read_timeout: GlobalSetting.group_smtp_read_timeout, diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 980879ff255..e467e7a894d 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -1397,7 +1397,7 @@ RSpec.describe Group do it "enables smtp and records the change" do group.update( smtp_port: 587, - smtp_ssl: true, + smtp_ssl_mode: Group.smtp_ssl_modes[:starttls], smtp_server: "smtp.gmail.com", email_username: "test@gmail.com", email_password: "password", @@ -1414,7 +1414,7 @@ RSpec.describe Group do it "records the change for singular setting changes" do group.update( smtp_port: 587, - smtp_ssl: true, + smtp_ssl_mode: Group.smtp_ssl_modes[:starttls], smtp_server: "smtp.gmail.com", email_username: "test@gmail.com", email_password: "password", @@ -1448,7 +1448,7 @@ RSpec.describe Group do it "disables smtp and records the change" do group.update( smtp_port: 587, - smtp_ssl: true, + smtp_ssl_mode: Group.smtp_ssl_modes[:starttls], smtp_server: "smtp.gmail.com", email_username: "test@gmail.com", email_password: "password", @@ -1460,7 +1460,7 @@ RSpec.describe Group do group.update( smtp_port: nil, - smtp_ssl: false, + smtp_ssl_mode: Group.smtp_ssl_modes[:none], smtp_server: nil, email_username: nil, email_password: nil, diff --git a/spec/requests/api/schemas/json/group_response.json b/spec/requests/api/schemas/json/group_response.json index 9163e33f9dc..61cf8910c39 100644 --- a/spec/requests/api/schemas/json/group_response.json +++ b/spec/requests/api/schemas/json/group_response.json @@ -170,9 +170,9 @@ "null" ] }, - "smtp_ssl": { + "smtp_ssl_mode": { "type": [ - "string", + "integer", "null" ] }, @@ -363,7 +363,7 @@ "automatic_membership_email_domains", "smtp_server", "smtp_port", - "smtp_ssl", + "smtp_ssl_mode", "imap_server", "imap_port", "imap_ssl", diff --git a/spec/requests/groups_controller_spec.rb b/spec/requests/groups_controller_spec.rb index 6d042a7f502..ad3b71fd2be 100644 --- a/spec/requests/groups_controller_spec.rb +++ b/spec/requests/groups_controller_spec.rb @@ -2766,6 +2766,7 @@ RSpec.describe GroupsController do let(:params) do { protocol: protocol, + ssl_mode: ssl_mode, ssl: ssl, port: port, host: host, @@ -2784,7 +2785,8 @@ RSpec.describe GroupsController do let(:username) { "test@gmail.com" } let(:password) { "password" } let(:domain) { nil } - let(:ssl) { true } + let(:ssl_mode) { Group.smtp_ssl_modes[:starttls] } + let(:ssl) { nil } let(:host) { "smtp.somemailsite.com" } let(:port) { 587 } @@ -2811,11 +2813,12 @@ RSpec.describe GroupsController do let(:password) { "password" } let(:domain) { nil } let(:ssl) { true } + let(:ssl_mode) { nil } 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)) + EmailSettingsValidator.expects(:validate_imap).with(has_entries(ssl: true)) post "/groups/#{group.id}/test_email_settings.json", params: params expect(response.status).to eq(200) end @@ -2844,6 +2847,7 @@ RSpec.describe GroupsController do let(:username) { "test@gmail.com" } let(:password) { "password" } let(:ssl) { true } + let(:ssl_mode) { nil } context "when the protocol is not accepted" do let(:protocol) { "sigma" } diff --git a/spec/services/email_settings_validator_spec.rb b/spec/services/email_settings_validator_spec.rb index ca41c0352d4..e7387889f6e 100644 --- a/spec/services/email_settings_validator_spec.rb +++ b/spec/services/email_settings_validator_spec.rb @@ -275,30 +275,6 @@ RSpec.describe EmailSettingsValidator do }.to raise_error(ArgumentError) end - it "corrects tls settings for gmail based on port 587" do - net_smtp_stub.expects(:enable_starttls_auto).once - net_smtp_stub.expects(:enable_tls).never - described_class.validate_smtp( - host: host, - port: 587, - username: username, - password: password, - domain: domain, - ) - end - - it "corrects tls settings for gmail based on port 465" do - net_smtp_stub.expects(:enable_starttls_auto).never - net_smtp_stub.expects(:enable_tls).once - described_class.validate_smtp( - host: host, - port: 465, - username: username, - password: password, - domain: domain, - ) - end - it "corrects authentication method to login for office365" do net_smtp_stub.expects(:start).with("office365.com", username, password, :login) described_class.validate_smtp( diff --git a/spec/services/post_alerter_spec.rb b/spec/services/post_alerter_spec.rb index 2667926580e..d858813d9f7 100644 --- a/spec/services/post_alerter_spec.rb +++ b/spec/services/post_alerter_spec.rb @@ -2408,7 +2408,7 @@ RSpec.describe PostAlerter do :group, smtp_server: "smtp.gmail.com", smtp_port: 587, - smtp_ssl: true, + smtp_ssl_mode: Group.smtp_ssl_modes[:starttls], imap_server: "imap.gmail.com", imap_port: 993, imap_ssl: true,