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

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;

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

@@ -11,17 +11,6 @@ export default {
         secret: false,
         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:

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

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"

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)

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

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