FIX: do not include contact url & email in client site settings payload (#13004)

FIX: do not include contact url & email in client site settings payload (#13004)

diff --git a/app/assets/javascripts/discourse/app/controllers/about.js b/app/assets/javascripts/discourse/app/controllers/about.js
index 26f3761..b22ce0d 100644
--- a/app/assets/javascripts/discourse/app/controllers/about.js
+++ b/app/assets/javascripts/discourse/app/controllers/about.js
@@ -6,20 +6,15 @@ import { gt } from "@ember/object/computed";
 export default Controller.extend({
   faqOverriden: gt("siteSettings.faq_url.length", 0),
 
-  @discourseComputed
-  contactInfo() {
-    if (this.siteSettings.contact_url) {
+  @discourseComputed("model.contact_url", "model.contact_email")
+  contactInfo(url, email) {
+    if (url) {
       return I18n.t("about.contact_info", {
-        contact_info:
-          "<a href='" +
-          this.siteSettings.contact_url +
-          "' target='_blank'>" +
-          this.siteSettings.contact_url +
-          "</a>",
+        contact_info: `<a href='${url}' target='_blank'>${url}</a>`,
       });
-    } else if (this.siteSettings.contact_email) {
+    } else if (email) {
       return I18n.t("about.contact_info", {
-        contact_info: this.siteSettings.contact_email,
+        contact_info: email,
       });
     } else {
       return null;
diff --git a/app/assets/javascripts/discourse/tests/fixtures/site-settings.js b/app/assets/javascripts/discourse/tests/fixtures/site-settings.js
index c35e10b..f5fca9c 100644
--- a/app/assets/javascripts/discourse/tests/fixtures/site-settings.js
+++ b/app/assets/javascripts/discourse/tests/fixtures/site-settings.js
@@ -12,17 +12,6 @@ export default {
         type: "string"
       },
       {
-        setting: "contact_email",
-        description:
-          "Email address of key contact responsible for this site. Used for critical notifications and displayed on the /about page for urgent matters.",
-        default: "",
-        value: "",
-        category: "required",
-        preview: null,
-        secret: false,
-        type: "email"
-      },
-      {
         setting: "site_contact_username",
         description:
           "A valid staff username to send all automated messages from. If left blank the default System account will be used.",
diff --git a/app/serializers/about_serializer.rb b/app/serializers/about_serializer.rb
index 8230c59..f55bdd3 100644
--- a/app/serializers/about_serializer.rb
+++ b/app/serializers/about_serializer.rb
@@ -22,11 +22,9 @@ class AboutSerializer < ApplicationSerializer
              :locale,
              :version,
              :https,
-             :can_see_about_stats
-
-  def can_see_about_stats
-    scope.can_see_about_stats?
-  end
+             :can_see_about_stats,
+             :contact_url,
+             :contact_email
 
   def include_stats?
     can_see_about_stats
@@ -35,4 +33,30 @@ class AboutSerializer < ApplicationSerializer
   def stats
     object.class.fetch_cached_stats || Jobs::AboutStats.new.execute({})
   end
+
+  def include_contact_url?
+    can_see_site_contact_details
+  end
+
+  def contact_url
+    SiteSetting.contact_url
+  end
+
+  def include_contact_email?
+    can_see_site_contact_details
+  end
+
+  def contact_email
+    SiteSetting.contact_email
+  end
+
+  private
+
+  def can_see_about_stats
+    scope.can_see_about_stats?
+  end
+
+  def can_see_site_contact_details
+    scope.can_see_site_contact_details?
+  end
 end
diff --git a/config/site_settings.yml b/config/site_settings.yml
index 8da736a..3cc158a 100644
--- a/config/site_settings.yml
+++ b/config/site_settings.yml
@@ -33,11 +33,9 @@ required:
     default: ""
     client: true
   contact_email:
-    client: true
     default: ""
     type: email
   contact_url:
-    client: true
     default: ""
   notification_email:
     default: "noreply@unconfigured.discourse.org"
diff --git a/lib/guardian.rb b/lib/guardian.rb
index 8bb2566..7e5a11c 100644
--- a/lib/guardian.rb
+++ b/lib/guardian.rb
@@ -530,6 +530,10 @@ class Guardian
     true
   end
 
+  def can_see_site_contact_details?
+    !SiteSetting.login_required? || authenticated?
+  end
+
   def auth_token
     if cookie = request&.cookies[Auth::DefaultCurrentUserProvider::TOKEN_COOKIE]
       UserAuthToken.hash_token(cookie)
diff --git a/spec/components/guardian_spec.rb b/spec/components/guardian_spec.rb
index 54735be..38478ce 100644
--- a/spec/components/guardian_spec.rb
+++ b/spec/components/guardian_spec.rb
@@ -3836,4 +3836,34 @@ describe Guardian do
       end
     end
   end
+
+  describe "can_see_site_contact_details" do
+    context "login_required is enabled" do
+      before do
+        SiteSetting.login_required = true
+      end
+
+      it "is false for anonymous users" do
+        expect(Guardian.new.can_see_site_contact_details?).to eq(false)
+      end
+
+      it "is true for regular users" do
+        expect(Guardian.new(user).can_see_site_contact_details?).to eq(true)
+      end
+    end
+
+    context "login_required is disabled" do
+      before do
+        SiteSetting.login_required = false
+      end
+
+      it "is true for anonymous users" do
+        expect(Guardian.new.can_see_site_contact_details?).to eq(true)
+      end
+
+      it "is true for regular users" do
+        expect(Guardian.new(user).can_see_site_contact_details?).to eq(true)
+      end
+    end
+  end
 end
diff --git a/spec/serializers/about_serializer_spec.rb b/spec/serializers/about_serializer_spec.rb
new file mode 100644
index 0000000..86f86b2
--- /dev/null
+++ b/spec/serializers/about_serializer_spec.rb
@@ -0,0 +1,48 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe AboutSerializer do
+
+  fab!(:user) { Fabricate(:user) }
+
+  context "login_required is enabled" do
+    before do
+      SiteSetting.login_required = true
+      SiteSetting.contact_url = "https://example.com/contact"
+      SiteSetting.contact_email = "example@foobar.com"
+    end
+
+    it "contact details are hidden from anonymous users" do
+      json = AboutSerializer.new(About.new(nil), scope: Guardian.new(nil), root: nil).as_json
+      expect(json[:contact_url]).to eq(nil)
+      expect(json[:contact_email]).to eq(nil)
+    end
+
+    it "contact details are visible to regular users" do
+      json = AboutSerializer.new(About.new(user), scope: Guardian.new(user), root: nil).as_json
+      expect(json[:contact_url]).to eq(SiteSetting.contact_url)
+      expect(json[:contact_email]).to eq(SiteSetting.contact_email)
+    end
+  end
+
+  context "login_required is disabled" do
+    before do
+      SiteSetting.login_required = false
+      SiteSetting.contact_url = "https://example.com/contact"
+      SiteSetting.contact_email = "example@foobar.com"
+    end
+
+    it "contact details are visible to anonymous users" do
+      json = AboutSerializer.new(About.new(nil), scope: Guardian.new(nil), root: nil).as_json
+      expect(json[:contact_url]).to eq(SiteSetting.contact_url)
+      expect(json[:contact_email]).to eq(SiteSetting.contact_email)
+    end
+
+    it "contact details are visible to regular users" do
+      json = AboutSerializer.new(About.new(user), scope: Guardian.new(user), root: nil).as_json
+      expect(json[:contact_url]).to eq(SiteSetting.contact_url)
+      expect(json[:contact_email]).to eq(SiteSetting.contact_email)
+    end
+  end
+end

GitHub sha: f96f534f

This commit appears in #13004 which was approved by eviltrout. It was merged by SamSaffron.