From c89edd9e86870f97a770816210d71400f09181f2 Mon Sep 17 00:00:00 2001 From: Blake Erickson Date: Wed, 22 Nov 2023 18:03:28 -0700 Subject: [PATCH] DEV: Convert email_in_min_trust to groups (#24515) This change converts the `email_in_min_trust` site setting to `email_in_allowed_groups`. See: https://meta.discourse.org/t/283408 - Hides the old setting - Adds the new site setting - Add a deprecation warning - Updates to use the new setting - Adds a migration to fill in the new setting if the old setting was changed - Adds an entry to the site_setting.keywords section - Updates tests to account for the new change After a couple of months we will remove the `email_in_min_trust` setting entirely. Internal ref: /t/115696 --- config/locales/server.en.yml | 3 ++ config/site_settings.yml | 7 +++ ...wed_groups_based_on_deprecated_settings.rb | 27 ++++++++++ lib/email/receiver.rb | 7 ++- lib/site_settings/deprecated_settings.rb | 1 + spec/lib/email/receiver_spec.rb | 54 +++++++++++++++---- 6 files changed, 86 insertions(+), 13 deletions(-) create mode 100644 db/migrate/20231122212122_fill_email_in_allowed_groups_based_on_deprecated_settings.rb diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 2873122775b..44bac35c43c 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -2140,6 +2140,8 @@ en: log_mail_processing_failures: "Log all email processing failures to /logs" email_in: "Allow users to post new topics via email. After enabling this setting, you will be able to configure incoming email addresses for groups and categories." email_in_min_trust: "The minimum trust level a user needs to have to be allowed to post new topics via email." + email_in_allowed_groups: "Groups that are allowed to post new topics via email." + email_in_authserv_id: "The identifier of the service doing authentication checks on incoming emails. See https://meta.discourse.org/t/134358 for instructions on how to configure this." email_in_spam_header: "The email header to detect spam." @@ -2547,6 +2549,7 @@ en: shared_drafts_allowed_groups: "shared_drafts_min_trust_level" approve_unless_allowed_groups: "approve_unless_trust_level" approve_new_topics_unless_allowed_groups: "approve_new_topics_unless_trust_level" + email_in_allowed_groups: "email_in_min_trust" placeholder: discourse_connect_provider_secrets: diff --git a/config/site_settings.yml b/config/site_settings.yml index 977c365a564..8142c92606e 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1254,6 +1254,13 @@ email: email_in_min_trust: default: 2 enum: "TrustLevelSetting" + hidden: true + email_in_allowed_groups: + default: 12 + type: group_list + allow_any: false + refresh: true + validator: "AtLeastOneGroupValidator" email_in_authserv_id: default: "" email_in_spam_header: diff --git a/db/migrate/20231122212122_fill_email_in_allowed_groups_based_on_deprecated_settings.rb b/db/migrate/20231122212122_fill_email_in_allowed_groups_based_on_deprecated_settings.rb new file mode 100644 index 00000000000..66ed9c29c22 --- /dev/null +++ b/db/migrate/20231122212122_fill_email_in_allowed_groups_based_on_deprecated_settings.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +class FillEmailInAllowedGroupsBasedOnDeprecatedSettings < ActiveRecord::Migration[7.0] + def up + email_in_min_trust_raw = + DB.query_single( + "SELECT value FROM site_settings WHERE name = 'email_in_min_trust' LIMIT 1", + ).first + + # Default for old setting is TL0, we only need to do anything if it's been changed in the DB. + if email_in_min_trust_raw.present? + # Matches Group::AUTO_GROUPS to the trust levels. + email_in_allowed_groups = "1#{email_in_min_trust_raw}" + + # Data_type 20 is group_list + DB.exec( + "INSERT INTO site_settings(name, value, data_type, created_at, updated_at) + VALUES('email_in_allowed_groups', :setting, '20', NOW(), NOW())", + setting: email_in_allowed_groups, + ) + end + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/lib/email/receiver.rb b/lib/email/receiver.rb index 93ecc91aa57..58716f6b65d 100644 --- a/lib/email/receiver.rb +++ b/lib/email/receiver.rb @@ -859,7 +859,8 @@ module Email user ||= stage_from_user - if !user.has_trust_level?(SiteSetting.email_in_min_trust) && !sent_to_mailinglist_mirror? + if user.groups.any? && !user.in_any_groups?(SiteSetting.email_in_allowed_groups_map) && + !sent_to_mailinglist_mirror? raise InsufficientTrustLevelError end @@ -1067,7 +1068,9 @@ module Email ) elsif destination.is_a?(Category) return false if user.staged? && !destination.email_in_allow_strangers - return false if !user.has_trust_level?(SiteSetting.email_in_min_trust) + if user.groups.any? && !user.in_any_groups?(SiteSetting.email_in_allowed_groups_map) + return false + end topic_user = embedded_user&.call || user create_topic( diff --git a/lib/site_settings/deprecated_settings.rb b/lib/site_settings/deprecated_settings.rb index 72761d4c8cf..a81c2314a07 100644 --- a/lib/site_settings/deprecated_settings.rb +++ b/lib/site_settings/deprecated_settings.rb @@ -18,6 +18,7 @@ module SiteSettings::DeprecatedSettings false, "3.3", ], + ["email_in_min_trust", "email_in_allowed_groups", false, "3.3"], ] def setup_deprecated_methods diff --git a/spec/lib/email/receiver_spec.rb b/spec/lib/email/receiver_spec.rb index 5fd60d37e16..986c3b8aded 100644 --- a/spec/lib/email/receiver_spec.rb +++ b/spec/lib/email/receiver_spec.rb @@ -948,6 +948,7 @@ RSpec.describe Email::Receiver do Fabricate.build(:secondary_email, email: "discourse@bar.com"), Fabricate.build(:secondary_email, email: "someone@else.com"), ], + refresh_auto_groups: true, ) user2 = @@ -958,6 +959,7 @@ RSpec.describe Email::Receiver do Fabricate.build(:secondary_email, email: "team@bar.com"), Fabricate.build(:secondary_email, email: "wat@bar.com"), ], + refresh_auto_groups: true, ) expect { process(:cc) }.to change(Topic, :count) @@ -1534,8 +1536,8 @@ RSpec.describe Email::Receiver do end it "raises an InsufficientTrustLevelError when user's trust level isn't enough" do - Fabricate(:user, email: "existing@bar.com", trust_level: 3) - SiteSetting.email_in_min_trust = 4 + Fabricate(:user, email: "existing@bar.com", trust_level: 3, refresh_auto_groups: true) + SiteSetting.email_in_allowed_groups = Group::AUTO_GROUPS[:trust_level_4] expect { process(:existing_user) }.to raise_error( Email::Receiver::InsufficientTrustLevelError, ) @@ -1584,7 +1586,12 @@ RSpec.describe Email::Receiver do it "creates visible topic for ham" do SiteSetting.email_in_spam_header = "none" - Fabricate(:user, email: "existing@bar.com", trust_level: SiteSetting.email_in_min_trust) + Fabricate( + :user, + email: "existing@bar.com", + trust_level: SiteSetting.email_in_min_trust, + refresh_auto_groups: true, + ) expect { process(:existing_user) }.to change { Topic.count }.by(1) # Topic created topic = Topic.last @@ -1600,7 +1607,12 @@ RSpec.describe Email::Receiver do SiteSetting.email_in_spam_header = "X-Spam-Flag" user = - Fabricate(:user, email: "existing@bar.com", trust_level: SiteSetting.email_in_min_trust) + Fabricate( + :user, + email: "existing@bar.com", + trust_level: SiteSetting.email_in_min_trust, + refresh_auto_groups: true, + ) expect { process(:spam_x_spam_flag) }.to change { ReviewableQueuedPost.count }.by(1) expect(user.reload.silenced?).to be(true) end @@ -1609,7 +1621,12 @@ RSpec.describe Email::Receiver do SiteSetting.email_in_spam_header = "X-Spam-Status" user = - Fabricate(:user, email: "existing@bar.com", trust_level: SiteSetting.email_in_min_trust) + Fabricate( + :user, + email: "existing@bar.com", + trust_level: SiteSetting.email_in_min_trust, + refresh_auto_groups: true, + ) expect { process(:spam_x_spam_status) }.to change { ReviewableQueuedPost.count }.by(1) expect(user.reload.silenced?).to be(true) end @@ -1618,7 +1635,12 @@ RSpec.describe Email::Receiver do SiteSetting.email_in_spam_header = "X-SES-Spam-Verdict" user = - Fabricate(:user, email: "existing@bar.com", trust_level: SiteSetting.email_in_min_trust) + Fabricate( + :user, + email: "existing@bar.com", + trust_level: SiteSetting.email_in_min_trust, + refresh_auto_groups: true, + ) expect { process(:spam_x_ses_spam_verdict) }.to change { ReviewableQueuedPost.count }.by(1) expect(user.reload.silenced?).to be(true) end @@ -1627,7 +1649,12 @@ RSpec.describe Email::Receiver do SiteSetting.email_in_authserv_id = "example.com" user = - Fabricate(:user, email: "existing@bar.com", trust_level: SiteSetting.email_in_min_trust) + Fabricate( + :user, + email: "existing@bar.com", + trust_level: SiteSetting.email_in_min_trust, + refresh_auto_groups: true, + ) expect { process(:dmarc_fail) }.to change { ReviewableQueuedPost.count }.by(1) expect(user.reload.silenced?).to be(false) end @@ -1635,7 +1662,12 @@ RSpec.describe Email::Receiver do it "adds the 'elided' part of the original message when always_show_trimmed_content is enabled" do SiteSetting.always_show_trimmed_content = true - Fabricate(:user, email: "existing@bar.com", trust_level: SiteSetting.email_in_min_trust) + Fabricate( + :user, + email: "existing@bar.com", + trust_level: SiteSetting.email_in_min_trust, + refresh_auto_groups: true, + ) expect { process(:forwarded_email_to_category) }.to change { Topic.count }.by(1) # Topic created new_post, = Post.last @@ -1698,13 +1730,13 @@ RSpec.describe Email::Receiver do end it "lets an email in from a high-TL user" do - Fabricate(:user, email: "tl4@bar.com", trust_level: TrustLevel[4]) + Fabricate(:user, email: "tl4@bar.com", trust_level: TrustLevel[4], refresh_auto_groups: true) expect { process(:tl4_user) }.to change(Topic, :count) end it "fails on email from a low-TL user" do - SiteSetting.email_in_min_trust = 4 - Fabricate(:user, email: "tl3@bar.com", trust_level: TrustLevel[3]) + SiteSetting.email_in_allowed_groups = Group::AUTO_GROUPS[:trust_level_4] + Fabricate(:user, email: "tl3@bar.com", trust_level: TrustLevel[3], refresh_auto_groups: true) expect { process(:tl3_user) }.to raise_error(Email::Receiver::InsufficientTrustLevelError) end end