FEATURE: Disallow putting urls in the title for TL-0 users (#13947)

This disallows putting URLs in topic titles for TL0 users, which means that:

If a TL-0 user puts a link into the title, a topic featured link won't be generated (as if it was disabled in the site settings)
Server methods for creating and updating topics will be refusing featured links when they are called by TL-0 users
TL-0 users won't be able to put any link into the topic title. For example, the title "Hey, take a look at https://my-site.com" will be rejected.

Also, it improves a bit server behavior when creating or updating feature links on topics in the categories with disabled featured links. Before the server just silently ignored a featured link field that was passed to him, now it will be returning 422 response.

app/assets/javascripts/discourse/app/controllers/topic.js

@@ -172,6 +172,10 @@ export default Controller.extend(bufferedProperty("model"), {
 
   @discourseComputed("model.isPrivateMessage", "model.category.id")
   canEditTopicFeaturedLink(isPrivateMessage, categoryId) {
+    if (this.currentUser && this.currentUser.trust_level === 0) {
+      return false;
+    }
+
     if (!this.siteSettings.topic_featured_link_enabled || isPrivateMessage) {
       return false;
     }

app/assets/javascripts/discourse/app/models/composer.js

@@ -280,8 +280,22 @@ const Composer = RestModel.extend({
     "notPrivateMessage"
   ),
 
-  @discourseComputed("canEditTitle", "creatingPrivateMessage", "categoryId")
-  canEditTopicFeaturedLink(canEditTitle, creatingPrivateMessage, categoryId) {
+  @discourseComputed(
+    "canEditTitle",
+    "creatingPrivateMessage",
+    "categoryId",
+    "user.trust_level"
+  )
+  canEditTopicFeaturedLink(
+    canEditTitle,
+    creatingPrivateMessage,
+    categoryId,
+    userTrustLevel
+  ) {
+    if (userTrustLevel === 0) {
+      return false;
+    }
+
     if (
       !this.siteSettings.topic_featured_link_enabled ||
       !canEditTitle ||

app/models/topic.rb

@@ -164,6 +164,7 @@ class Topic < ActiveRecord::Base
                     topic_title_length: true,
                     censored_words: true,
                     watched_words: true,
+                    urls_in_topic_title: true,
                     quality_title: { unless: :private_message? },
                     max_emojis: true,
                     unique_among: { unless: Proc.new { |t| (SiteSetting.allow_duplicate_topic_titles? || t.private_message?) },
@@ -190,8 +191,9 @@ class Topic < ActiveRecord::Base
 
   validates :featured_link, allow_nil: true, url: true
   validate if: :featured_link do
-    errors.add(:featured_link, :invalid_category) unless !featured_link_changed? ||
-      Guardian.new.can_edit_featured_link?(category_id)
+    if featured_link_changed? && !Guardian.new(user).can_edit_featured_link?(category_id)
+      errors.add(:featured_link)
+    end
   end
 
   before_validation do

config/locales/server.en.yml

@@ -358,6 +358,7 @@ en:
     other: "Sorry, new users can only put %{count} attachments in a post."
   no_links_allowed: "Sorry, new users can't put links in posts."
   links_require_trust: "Sorry, you can't include links in your posts."
+  urls_in_title_require_trust_level: "Sorry, new users can't include links in topic titles."
   too_many_links:
     one: "Sorry, new users can only put one link in a post."
     other: "Sorry, new users can only put %{count} links in a post."
@@ -579,7 +580,6 @@ en:
               unable_to_tag: "There was an error tagging the topic."
             featured_link:
               invalid: "is invalid. URL should include http:// or https://."
-              invalid_category: "can't be edited in this category."
         user:
           attributes:
             password:

lib/guardian.rb

@@ -540,6 +540,10 @@ class Guardian
     !SiteSetting.login_required? || authenticated?
   end
 
+  def can_put_urls_in_topic_title?
+    @user.trust_level >= TrustLevel.levels[:basic]
+  end
+
   def auth_token
     if cookie = request&.cookies[Auth::DefaultCurrentUserProvider::TOKEN_COOKIE]
       UserAuthToken.hash_token(cookie)

lib/guardian/topic_guardian.rb

@@ -206,6 +206,7 @@ module TopicGuardian
 
   def can_edit_featured_link?(category_id)
     return false unless SiteSetting.topic_featured_link_enabled
+    return false unless @user.trust_level >= TrustLevel.levels[:basic]
     Category.where(id: category_id || SiteSetting.uncategorized_category_id, topic_featured_link_allowed: true).exists?
   end
 
@@ -251,5 +252,4 @@ module TopicGuardian
   def affected_by_slow_mode?(topic)
     topic&.slow_mode_seconds.to_i > 0 && @user.human? && !is_staff?
   end
-
 end

lib/post_revisor.rb

@@ -67,14 +67,28 @@ class PostRevisor
     end
   end
 
-  # Fields we want to record revisions for by default
-  %i{title archetype}.each do |field|
-    track_topic_field(field) do |tc, attribute|
-      tc.record_change(field, tc.topic.public_send(field), attribute)
-      tc.topic.public_send("#{field}=", attribute)
+  def self.track_and_revise(topic_changes, field, attribute)
+    topic_changes.record_change(
+      field,
+      topic_changes.topic.public_send(field),
+      attribute
+    )
+    topic_changes.topic.public_send("#{field}=", attribute)
+  end
+
+  track_topic_field(:title) do |topic_changes, attribute|
+    if UrlHelper.contains_url?(attribute) && !topic_changes.guardian.can_put_urls_in_topic_title?
+      topic_changes.topic.errors.add(:base, I18n.t("urls_in_title_require_trust_level"))
+      topic_changes.check_result(false)
+    else
+      track_and_revise topic_changes, :title, attribute
     end
   end
 
+  track_topic_field(:archetype) do |topic_changes, attribute|
+    track_and_revise topic_changes, :archetype, attribute
+  end
+
   track_topic_field(:category_id) do |tc, category_id, fields|
     if category_id == 0 && tc.topic.private_message?
       tc.record_change('category_id', tc.topic.category_id, nil)
@@ -111,9 +125,10 @@ class PostRevisor
   end
 
   track_topic_field(:featured_link) do |topic_changes, featured_link|
-    if SiteSetting.topic_featured_link_enabled &&
-       topic_changes.guardian.can_edit_featured_link?(topic_changes.topic.category_id)
-
+    if !SiteSetting.topic_featured_link_enabled ||
+      !topic_changes.guardian.can_edit_featured_link?(topic_changes.topic.category_id)
+      topic_changes.check_result(false)
+    else
       topic_changes.record_change('featured_link', topic_changes.topic.featured_link, featured_link)
       topic_changes.topic.featured_link = featured_link
     end

lib/topic_creator.rb

@@ -132,15 +132,10 @@ class TopicCreator
     end
 
     topic_params[:category_id] = category.id if category.present?
-
     topic_params[:created_at] = convert_time(@opts[:created_at]) if @opts[:created_at].present?
-
     topic_params[:pinned_at] = convert_time(@opts[:pinned_at]) if @opts[:pinned_at].present?
     topic_params[:pinned_globally] = @opts[:pinned_globally] if @opts[:pinned_globally].present?
-
-    if SiteSetting.topic_featured_link_enabled && @opts[:featured_link].present? && @guardian.can_edit_featured_link?(topic_params[:category_id])
-      topic_params[:featured_link] = @opts[:featured_link]
-    end
+    topic_params[:featured_link] = @opts[:featured_link]
 
     topic_params
   end

lib/url_helper.rb

@@ -65,6 +65,11 @@ class UrlHelper
     Addressable::URI.normalized_encode(uri)
   end
 
+  def self.contains_url?(string)
+    uri_regexp = Discourse::Utils::URI_REGEXP
+    uri_regexp.match?(string)
+  end
+
   def self.rails_route_from_url(url)
     path = URI.parse(encode(url)).path
     Rails.application.routes.recognize_path(path)

lib/validators/urls_in_topic_title_validator.rb

@@ -0,0 +1,20 @@
+# frozen_string_literal: true
+
+class UrlsInTopicTitleValidator < ActiveModel::Validator
+  def validate(record)
+    if UrlHelper.contains_url?(record.title) && !can_put_urls?(record)
+      record.errors.add(:base, error_message)
+    end
+  end
+
+  private
+
+  def can_put_urls?(topic)
+    guardian = Guardian.new(topic.acting_user)
+    guardian.can_put_urls_in_topic_title?
+  end
+
+  def error_message
+    I18n.t("urls_in_title_require_trust_level")
+  end
+end

spec/components/guardian_spec.rb

@@ -3688,7 +3688,7 @@ describe Guardian do
 
   context 'topic featured link category restriction' do
     before { SiteSetting.topic_featured_link_enabled = true }
-    let(:guardian) { Guardian.new }
+    let(:guardian) { Guardian.new(user) }
     let(:uncategorized) { Category.find(SiteSetting.uncategorized_category_id) }
 
     context "uncategorized" do

spec/components/post_revisor_spec.rb

@@ -661,6 +661,34 @@ describe PostRevisor do
       expect(post.raw).to eq("    <-- whitespaces -->")
     end
 
+    it "revises and tracks changes of topic titles" do
+      new_title = "New topic title"
+      result = subject.revise!(
+        post.user,
+        { title: new_title },
+        revised_at: post.updated_at + 10.minutes
+      )
+
+      expect(result).to eq(true)
+      post.reload
+      expect(post.topic.title).to eq(new_title)
+      expect(post.revisions.first.modifications["title"][1]).to eq(new_title)
+    end
+
+    it "revises and tracks changes of topic archetypes" do
+      new_archetype = Archetype.banner
+      result = subject.revise!(
+        post.user,
+        { archetype: new_archetype },
+        revised_at: post.updated_at + 10.minutes
+      )
+
+      expect(result).to eq(true)
+      post.reload
+      expect(post.topic.archetype).to eq(new_archetype)
+      expect(post.revisions.first.modifications["archetype"][1]).to eq(new_archetype)
+    end
+
     context "#publish_changes" do
       let!(:post) { Fabricate(:post, topic: topic) }
 

spec/fabricators/user_fabricator.rb

@@ -87,6 +87,14 @@ Fabricator(:leader, from: :user) do
   trust_level TrustLevel[3]
 end
 
+Fabricator(:trust_level_0, from: :user) do
+  trust_level TrustLevel[0]
+end
+
+Fabricator(:trust_level_1, from: :user) do
+  trust_level TrustLevel[1]
+end
+
 Fabricator(:trust_level_4, from: :user) do
   name 'Leader McElderson'
   username { sequence(:username) { |i| "tl4#{i}" } }

spec/requests/posts_controller_spec.rb

@@ -81,6 +81,8 @@ describe PostsController do
   fab!(:admin) { Fabricate(:admin) }
   fab!(:moderator) { Fabricate(:moderator) }
   fab!(:user) { Fabricate(:user) }
+  fab!(:user_trust_level_0) { Fabricate(:trust_level_0) }
+  fab!(:user_trust_level_1) { Fabricate(:trust_level_1) }
   fab!(:category) { Fabricate(:category) }
   fab!(:topic) { Fabricate(:topic) }
   fab!(:post_by_user) { Fabricate(:post, user: user) }
@@ -1457,6 +1459,85 @@ describe PostsController do
       end
     end
 
+    describe "urls in the title" do
+      let!(:title_with_url) { "A title with the URL https://google.com" }
+
+      it "doesn't allow TL0 users to put urls into the title" do
+        sign_in(user_trust_level_0)
+
+        post "/posts.json", params: {
+          raw: 'this is the test content',
+          title: title_with_url
+        }
+
+        expect(response.status).to eq(422)
+        expect(response.body).to include(I18n.t('urls_in_title_require_trust_level'))
+      end
+
+      it "allows TL1 users to put urls into the title" do
+        sign_in(user_trust_level_1)
+
+        post "/posts.json", params: {
+          raw: 'this is the test content',
+          title: title_with_url
+        }
+
+        expect(response.status).to eq(200)
+      end
+    end
+
+    describe "featured links" do
+      it "allows to create topics with featured links" do
+        sign_in(user_trust_level_1)
+
+        post "/posts.json", params: {
+          title: "this is the test title for the topic",
+          raw: "this is the test content",
+          featured_link: "https://discourse.org"
+        }
+
+        expect(response.status).to eq(200)
+      end
+
+      it "doesn't allow TL0 users to create topics with featured links" do
+        sign_in(user_trust_level_0)
+
+        post "/posts.json", params: {
+          title: "this is the test title for the topic",
+          raw: "this is the test content",
+          featured_link: "https://discourse.org"
+        }
+
+        expect(response.status).to eq(422)
+      end
+
+      it "doesn't allow to create topics with featured links if featured links are disabled in settings" do
+        SiteSetting.topic_featured_link_enabled = false
+        sign_in(user_trust_level_1)
+
+        post "/posts.json", params: {
+          title: "this is the test title for the topic",
+          raw: "this is the test content",
+          featured_link: "https://discourse.org"
+        }
+
+        expect(response.status).to eq(422)
+      end
+
+      it "doesn't allow to create topics with featured links in the category with forbidden feature links" do
+        category = Fabricate(:category, topic_featured_link_allowed: false)
+        sign_in(user_trust_level_1)
+
+        post "/posts.json", params: {
+          title: "this is the test title for the topic",
+          raw: "this is the test content",
+          featured_link: "https://discourse.org",
+          category: category.id
+        }
+
+        expect(response.status).to eq(422)
+      end
+    end
   end
 
   describe '#revisions' do

spec/requests/topics_controller_spec.rb

@@ -9,6 +9,8 @@ RSpec.describe TopicsController do
   fab!(:user) { Fabricate(:user) }
   fab!(:moderator) { Fabricate(:moderator) }
   fab!(:admin) { Fabricate(:admin) }
+  fab!(:trust_level_0) { Fabricate(:trust_level_0) }
+  fab!(:trust_level_1) { Fabricate(:trust_level_1) }
   fab!(:trust_level_4) { Fabricate(:trust_level_4) }
 
   fab!(:category) { Fabricate(:category) }
@@ -1650,6 +1652,83 @@ RSpec.describe TopicsController do
         end
       end
     end
+
+    describe "urls in the title" do
+      let!(:title_with_url) { "A title with the URL https://google.com" }
+
+      it "doesn't allow TL0 users to put urls into the title" do
+        sign_in(trust_level_0)
+        topic = Fabricate(:topic, user: trust_level_0)
+        Fabricate(:post, topic: topic)
+        put "/t/#{topic.slug}/#{topic.id}.json", params: { title: title_with_url }
+
+        expect(response.status).to eq(422)
+        expect(response.body).to include(I18n.t('urls_in_title_require_trust_level'))
+      end
+
+      it "allows TL1 users to put urls into the title" do
+        sign_in(trust_level_1)
+        topic = Fabricate(:topic, user: trust_level_1)
+        Fabricate(:post, topic: topic)
+        put "/t/#{topic.slug}/#{topic.id}.json", params: { title: title_with_url }
+
+        expect(response.status).to eq(200)
+      end
+    end
+
+    describe "featured links" do
+      def fabricate_topic(user, category = nil)
+        topic = Fabricate(:topic, user: user, category: category)
+        Fabricate(:post, topic: topic)
+        topic
+      end
+
+      it "allows to update topic featured link" do
+        sign_in(trust_level_1)
+
+        topic = fabricate_topic(trust_level_1)
+        put "/t/#{topic.slug}/#{topic.id}.json", params: {
+          featured_link: "https://discourse.org"
+        }
+
+        expect(response.status).to eq(200)
+      end
+
+      it "doesn't allow TL0 users to update topic featured link" do
+        sign_in(trust_level_0)
+
+        topic = fabricate_topic(trust_level_0)
+        put "/t/#{topic.slug}/#{topic.id}.json", params: {
+          featured_link: "https://discourse.org"
+        }
+
+        expect(response.status).to eq(422)
+      end
+
+      it "doesn't allow to update topic featured link if featured links are disabled in settings" do
+        sign_in(trust_level_1)
+
+        SiteSetting.topic_featured_link_enabled = false
+        topic = fabricate_topic(trust_level_1)
+        put "/t/#{topic.slug}/#{topic.id}.json", params: {
+          featured_link: "https://discourse.org"
+        }
+
+        expect(response.status).to eq(422)
+      end
+
+      it "doesn't allow to update topic featured link in the category with forbidden feature links" do
+        sign_in(trust_level_1)
+
+        category = Fabricate(:category, topic_featured_link_allowed: false)
+        topic = fabricate_topic(trust_level_1, category)
+        put "/t/#{topic.slug}/#{topic.id}.json", params: {
+          featured_link: "https://discourse.org"
+        }
+
+        expect(response.status).to eq(422)
+      end
+    end
   end
 
   describe '#show' do