From f96f534f3e45d78058d9bb9f0b2b7198cf217cc8 Mon Sep 17 00:00:00 2001 From: Arpit Jalan Date: Wed, 19 May 2021 11:45:24 +0530 Subject: [PATCH] FIX: do not include contact url & email in client site settings payload (#13004) --- .../discourse/app/controllers/about.js | 17 +++---- .../discourse/tests/fixtures/site-settings.js | 11 ----- app/serializers/about_serializer.rb | 34 +++++++++++-- config/site_settings.yml | 2 - lib/guardian.rb | 4 ++ spec/components/guardian_spec.rb | 30 ++++++++++++ spec/serializers/about_serializer_spec.rb | 48 +++++++++++++++++++ 7 files changed, 117 insertions(+), 29 deletions(-) create mode 100644 spec/serializers/about_serializer_spec.rb diff --git a/app/assets/javascripts/discourse/app/controllers/about.js b/app/assets/javascripts/discourse/app/controllers/about.js index 26f3761bfd8..b22ce0d0cd5 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: - "" + - this.siteSettings.contact_url + - "", + contact_info: `${url}`, }); - } 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 c35e10b069e..f5fca9c6613 100644 --- a/app/assets/javascripts/discourse/tests/fixtures/site-settings.js +++ b/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: diff --git a/app/serializers/about_serializer.rb b/app/serializers/about_serializer.rb index 8230c590a4c..f55bdd34767 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 8da736a7efb..3cc158a0e3e 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 8bb2566c105..7e5a11cc8ab 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 54735be65b1..38478cea97d 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 00000000000..86f86b21750 --- /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