diff --git a/app/assets/javascripts/discourse/app/controllers/topic.js b/app/assets/javascripts/discourse/app/controllers/topic.js index f48dda98231..d41257b8afd 100644 --- a/app/assets/javascripts/discourse/app/controllers/topic.js +++ b/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; } diff --git a/app/assets/javascripts/discourse/app/models/composer.js b/app/assets/javascripts/discourse/app/models/composer.js index b93ab8c6e8a..85c69ad880c 100644 --- a/app/assets/javascripts/discourse/app/models/composer.js +++ b/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 || diff --git a/app/models/topic.rb b/app/models/topic.rb index 601c9759775..ce78ed1433a 100644 --- a/app/models/topic.rb +++ b/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 diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 2d4f846f5da..c5c22c7d75e 100644 --- a/config/locales/server.en.yml +++ b/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: diff --git a/lib/guardian.rb b/lib/guardian.rb index 54822b95b92..77e32aa3d02 100644 --- a/lib/guardian.rb +++ b/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) diff --git a/lib/guardian/topic_guardian.rb b/lib/guardian/topic_guardian.rb index 98e51453e7c..a8504cb8b34 100644 --- a/lib/guardian/topic_guardian.rb +++ b/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 diff --git a/lib/post_revisor.rb b/lib/post_revisor.rb index eee848607e7..1cfc60a8dc0 100644 --- a/lib/post_revisor.rb +++ b/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 diff --git a/lib/topic_creator.rb b/lib/topic_creator.rb index 7ba59e945aa..a250ef69fbf 100644 --- a/lib/topic_creator.rb +++ b/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 diff --git a/lib/url_helper.rb b/lib/url_helper.rb index b1f10df26cd..f55b40e02ab 100644 --- a/lib/url_helper.rb +++ b/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) diff --git a/lib/validators/urls_in_topic_title_validator.rb b/lib/validators/urls_in_topic_title_validator.rb new file mode 100644 index 00000000000..1bb03bc95b1 --- /dev/null +++ b/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 diff --git a/spec/components/guardian_spec.rb b/spec/components/guardian_spec.rb index e07f63f5e81..9695a98da90 100644 --- a/spec/components/guardian_spec.rb +++ b/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 diff --git a/spec/components/post_revisor_spec.rb b/spec/components/post_revisor_spec.rb index fb748471aba..59bd2ad2a6b 100644 --- a/spec/components/post_revisor_spec.rb +++ b/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) } diff --git a/spec/fabricators/user_fabricator.rb b/spec/fabricators/user_fabricator.rb index 759a5a43d3e..9c60bc95f7d 100644 --- a/spec/fabricators/user_fabricator.rb +++ b/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}" } } diff --git a/spec/requests/posts_controller_spec.rb b/spec/requests/posts_controller_spec.rb index 5c0c00d7044..424fa1279f2 100644 --- a/spec/requests/posts_controller_spec.rb +++ b/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 diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb index 46bcfab2e00..9cd8c369c7b 100644 --- a/spec/requests/topics_controller_spec.rb +++ b/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