FEATURE: Validate domain settings for blocked_onebox_domain only (#15754)

We want to prevent the user from adding ? or * minimally
when setting domains in sitesettings

app/assets/javascripts/admin/addon/components/site-settings/host-list.js

@@ -0,0 +1,21 @@
+import Component from "@ember/component";
+import { action, computed } from "@ember/object";
+
+export default Component.extend({
+  tokenSeparator: "|",
+  choices: null,
+
+  @computed("value")
+  get settingValue() {
+    return this.value.toString().split(this.tokenSeparator).filter(Boolean);
+  },
+
+  @action
+  onChange(value) {
+    if (value.some((v) => v.includes("?") || v.includes("*"))) {
+      return;
+    }
+
+    this.set("value", value.join(this.tokenSeparator));
+  },
+});

app/assets/javascripts/admin/addon/templates/components/site-settings/host-list.hbs

@@ -1,3 +1,12 @@
-{{value-list values=value addKey="admin.site_settings.add_host"}}
+{{list-setting
+  value=settingValue
+  settingName=setting.setting
+  choices=settingValue
+  onChange=(action "onChange")
+  options=(hash
+    allowAny=allowAny
+  )
+}}
+
 {{setting-validation-message message=validationMessage}}
 <div class="desc">{{html-safe setting.description}}</div>

app/assets/javascripts/discourse/tests/acceptance/admin-site-settings-test.js

@@ -13,6 +13,9 @@ import {
 } from "@ember/test-helpers";
 import siteSettingFixture from "discourse/tests/fixtures/site-settings";
 import { test } from "qunit";
+import pretender from "discourse/tests/helpers/create-pretender";
+
+const ENTER_KEYCODE = 13;
 
 acceptance("Admin - Site Settings", function (needs) {
   let updatedTitle;
@@ -105,7 +108,7 @@ acceptance("Admin - Site Settings", function (needs) {
     );
 
     await fillIn(".input-setting-string", "Test");
-    await triggerKeyEvent(".input-setting-string", "keydown", 13); // enter
+    await triggerKeyEvent(".input-setting-string", "keydown", ENTER_KEYCODE);
     assert.ok(
       exists(".row.setting.overridden"),
       "saving via Enter key marks setting as overriden"
@@ -163,4 +166,30 @@ acceptance("Admin - Site Settings", function (needs) {
       "/admin/site_settings/category/all_results?filter=contact"
     );
   });
+
+  test("filters * and ? for domain lists", async (assert) => {
+    pretender.put("/admin/site_settings/blocked_onebox_domains", () => [200]);
+
+    await visit("/admin/site_settings");
+    await fillIn("#setting-filter", "domains");
+
+    await click(".select-kit-header.multi-select-header");
+
+    await fillIn(".select-kit-filter input", "cat.?.domain");
+    await triggerKeyEvent(".select-kit-filter input", "keydown", ENTER_KEYCODE);
+
+    await fillIn(".select-kit-filter input", "*.domain");
+    await triggerKeyEvent(".select-kit-filter input", "keydown", ENTER_KEYCODE);
+
+    await fillIn(".select-kit-filter input", "proper.com");
+    await triggerKeyEvent(".select-kit-filter input", "keydown", ENTER_KEYCODE);
+
+    await click("button.ok");
+
+    assert.strictEqual(
+      pretender.handledRequests[pretender.handledRequests.length - 1]
+        .requestBody,
+      "blocked_onebox_domains=proper.com"
+    );
+  });
 });

app/assets/javascripts/discourse/tests/fixtures/site-settings.js

@@ -64,7 +64,19 @@ export default {
         secret: false,
         type: "upload",
         plugin: "discourse-logo"
-      }
+      },
+      {
+        category: "onebox",
+        default: "",
+        description:
+          "A list of domains that will never be oneboxed e.g. wikipedia.org\n(Wildcard symbols * ? not supported)",
+        placeholder: null,
+        preview: null,
+        secret: false,
+        setting: "blocked_onebox_domains",
+        type: "host_list",
+        value: "",
+      },
     ],
     diags: {
       last_message_processed: null

app/assets/javascripts/discourse/tests/integration/components/select-kit/host-list-test.js

@@ -0,0 +1,31 @@
+import componentTest, {
+  setupRenderingTest,
+} from "discourse/tests/helpers/component-test";
+import { discourseModule, query } from "discourse/tests/helpers/qunit-helpers";
+import hbs from "htmlbars-inline-precompile";
+
+discourseModule(
+  "Integration | Component | site-setting | host-list",
+  function (hooks) {
+    setupRenderingTest(hooks);
+
+    componentTest("displays setting value", {
+      template: hbs`{{site-setting setting=setting}}`,
+
+      beforeEach() {
+        this.set("setting", {
+          setting: "blocked_onebox_domains",
+          value: "a.com|b.com",
+          type: "host_list",
+        });
+      },
+
+      async test(assert) {
+        assert.strictEqual(
+          query(".formated-selection").innerText,
+          "a.com, b.com"
+        );
+      },
+    });
+  }
+);

config/locales/server.en.yml

@@ -1530,7 +1530,7 @@ en:
     show_pinned_excerpt_mobile: "Show excerpt on pinned topics in mobile view."
     show_pinned_excerpt_desktop: "Show excerpt on pinned topics in desktop view."
     post_onebox_maxlength: "Maximum length of a oneboxed Discourse post in characters."
-    blocked_onebox_domains: "A list of domains that will never be oneboxed."
+    blocked_onebox_domains: "A list of domains that will never be oneboxed e.g. wikipedia.org\n(Wildcard symbols * ? not supported)"
     allowed_inline_onebox_domains: "A list of domains that will be oneboxed in miniature form if linked without a title"
     enable_inline_onebox_on_all_domains: "Ignore inline_onebox_domain_allowlist site setting and allow inline onebox on all domains."
     force_custom_user_agent_hosts: "Hosts for which to use the custom onebox user agent on all requests. (Especially useful for hosts that limit access by user agent)."
@@ -2365,6 +2365,7 @@ en:
       invalid_json: "Invalid JSON."
       invalid_reply_by_email_address: "Value must contain '%{reply_key}' and be different from the notification email."
       invalid_alternative_reply_by_email_addresses: "All values must contain '%{reply_key}' and be different from the notification email."
+      invalid_domain_hostname: "Must not include * or ? characters."
       pop3_polling_host_is_empty: "You must set a 'pop3 polling host' before enabling POP3 polling."
       pop3_polling_username_is_empty: "You must set a 'pop3 polling username' before enabling POP3 polling."
       pop3_polling_password_is_empty: "You must set a 'pop3 polling password' before enabling POP3 polling."

config/site_settings.yml

@@ -1700,7 +1700,7 @@ onebox:
       zh_TW: 200
   blocked_onebox_domains:
     default: ""
-    type: list
+    type: host_list
     list_type: compact
   max_oneboxes_per_post:
     default: 50

lib/site_settings/type_supervisor.rb

@@ -269,6 +269,8 @@ class SiteSettings::TypeSupervisor
       RegexSettingValidator
     when self.class.types[:string], self.class.types[:list], self.class.types[:enum]
       StringSettingValidator
+    when self.class.types[:host_list]
+      HostListSettingValidator
     else nil
     end
   end

lib/validators/host_list_setting_validator.rb

@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+class HostListSettingValidator
+  def initialize(opts = {})
+    @opts = opts
+  end
+
+  def valid_value?(val)
+    val.exclude?("*") && val.exclude?("?")
+  end
+
+  def error_message
+    I18n.t('site_settings.errors.invalid_domain_hostname')
+  end
+end

spec/components/validators/host_list_setting_validator_spec.rb

@@ -0,0 +1,31 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe HostListSettingValidator do
+  subject(:validator) { described_class.new() }
+
+  describe '#valid_value?' do
+    describe "returns false for values containing *" do
+      it { expect(validator.valid_value?("*")).to eq false }
+      it { expect(validator.valid_value?("**")).to eq false }
+      it { expect(validator.valid_value?(".*")).to eq false }
+      it { expect(validator.valid_value?("a")).to eq true }
+    end
+
+    describe "returns false for values containing ?" do
+      it { expect(validator.valid_value?("?")).to eq false }
+      it { expect(validator.valid_value?("??")).to eq false }
+      it { expect(validator.valid_value?(".?")).to eq false }
+      it { expect(validator.valid_value?("a")).to eq true }
+    end
+  end
+
+  describe "#error_message" do
+    it "returns invalid domain hostname error" do
+      expect(validator.error_message).to eq(I18n.t(
+        'site_settings.errors.invalid_domain_hostname'
+      ))
+    end
+  end
+end