FIX: Return 422 when creating topics with tags w/out permission (#10400)

The UI prevents users from trying to create tags on topics when they
don't have permission, but if you are trying to add tags to a topic via
the API and you don't have permission before this change it would
silently succeed in creating the topic, but it wouldn't have any tags.

Now a 422 error will be returned with an error message when trying to
create a topic with tags when tagging is disabled or you don't have
enough trust level to add tags to a topic.

Bug report: https://meta.discourse.org/t/-/70525/14
This commit is contained in:
Blake Erickson 2020-08-10 16:14:15 -06:00 committed by GitHub
parent 1972364d0f
commit 2032c11f78
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 65 additions and 6 deletions

View File

@ -530,6 +530,7 @@ en:
reply_by_email_disabled: "Reply by email has been disabled." reply_by_email_disabled: "Reply by email has been disabled."
target_user_not_found: "One of the users you are sending this message to could not be found." target_user_not_found: "One of the users you are sending this message to could not be found."
unable_to_update: "There was an error updating that topic." unable_to_update: "There was an error updating that topic."
unable_to_tag: "There was an error tagging the topic."
featured_link: featured_link:
invalid: "is invalid. URL should include http:// or https://." invalid: "is invalid. URL should include http:// or https://."
invalid_category: "can't be edited in this category." invalid_category: "can't be edited in this category."

View File

@ -78,7 +78,7 @@ module DiscourseTagging
parent_tags_map = DB.query(" parent_tags_map = DB.query("
SELECT tgm.tag_id, tg.parent_tag_id SELECT tgm.tag_id, tg.parent_tag_id
FROM tag_groups tg FROM tag_groups tg
INNER JOIN tag_group_memberships tgm INNER JOIN tag_group_memberships tgm
ON tgm.tag_group_id = tg.id ON tgm.tag_group_id = tg.id
WHERE tg.parent_tag_id IS NOT NULL WHERE tg.parent_tag_id IS NOT NULL
AND tgm.tag_id IN (?) AND tgm.tag_id IN (?)
@ -112,8 +112,9 @@ module DiscourseTagging
topic.tags = [] topic.tags = []
end end
topic.tags_changed = true topic.tags_changed = true
return true
end end
true false
end end
def self.validate_min_required_tags_for_category(guardian, topic, category, tags = []) def self.validate_min_required_tags_for_category(guardian, topic, category, tags = [])

View File

@ -165,7 +165,10 @@ class TopicCreator
end end
else else
valid_tags = DiscourseTagging.tag_topic_by_names(topic, @guardian, @opts[:tags]) valid_tags = DiscourseTagging.tag_topic_by_names(topic, @guardian, @opts[:tags])
rollback_from_errors!(topic) unless valid_tags unless valid_tags
topic.errors.add(:base, :unable_to_tag)
rollback_from_errors!(topic)
end
end end
end end

View File

@ -289,6 +289,9 @@ describe NewPostManager do
it "calls custom enqueuing handlers" do it "calls custom enqueuing handlers" do
Reviewable.set_priorities(high: 20.5) Reviewable.set_priorities(high: 20.5)
SiteSetting.reviewable_default_visibility = 'high' SiteSetting.reviewable_default_visibility = 'high'
SiteSetting.tagging_enabled = true
SiteSetting.min_trust_to_create_tag = 0
SiteSetting.min_trust_level_to_tag_topics = 0
manager = NewPostManager.new( manager = NewPostManager.new(
topic.user, topic.user,

View File

@ -406,7 +406,7 @@ describe PostCreator do
it "doesn't create tags" do it "doesn't create tags" do
expect { @post = creator_with_tags.create }.to change { Tag.count }.by(0) expect { @post = creator_with_tags.create }.to change { Tag.count }.by(0)
expect(@post.topic.tags.size).to eq(0) expect(@post.topic&.tags&.size).to eq(nil)
end end
end end

View File

@ -143,6 +143,12 @@ RSpec.describe ReviewableQueuedPost, type: :model do
context "creating a topic" do context "creating a topic" do
let(:reviewable) { Fabricate(:reviewable_queued_post_topic, category: category) } let(:reviewable) { Fabricate(:reviewable_queued_post_topic, category: category) }
before do
SiteSetting.tagging_enabled = true
SiteSetting.min_trust_to_create_tag = 0
SiteSetting.min_trust_level_to_tag_topics = 0
end
context "editing" do context "editing" do
it "is editable and returns the fields" do it "is editable and returns the fields" do

View File

@ -939,7 +939,7 @@ describe PostsController do
expect(response.status).to eq(422) expect(response.status).to eq(422)
end end
it 'can not create a post in a disallowed category' do it 'cannot create a post in a disallowed category' do
category.set_permissions(staff: :full) category.set_permissions(staff: :full)
category.save! category.save!
@ -953,7 +953,7 @@ describe PostsController do
expect(response.status).to eq(403) expect(response.status).to eq(403)
end end
it 'can not create a post with a tag that is restricted' do it 'cannot create a post with a tag that is restricted' do
SiteSetting.tagging_enabled = true SiteSetting.tagging_enabled = true
tag = Fabricate(:tag) tag = Fabricate(:tag)
category.allowed_tags = [tag.name] category.allowed_tags = [tag.name]
@ -970,6 +970,51 @@ describe PostsController do
expect(json['errors']).to be_present expect(json['errors']).to be_present
end end
it 'cannot create a post with a tag when tagging is disabled' do
SiteSetting.tagging_enabled = false
tag = Fabricate(:tag)
post "/posts.json", params: {
raw: 'this is the test content',
title: 'this is the test title for the topic',
tags: [tag.name],
}
expect(response.status).to eq(422)
json = response.parsed_body
expect(json['errors']).to be_present
end
it 'cannot create a post with a tag without tagging permission' do
SiteSetting.tagging_enabled = true
SiteSetting.min_trust_level_to_tag_topics = 4
tag = Fabricate(:tag)
post "/posts.json", params: {
raw: 'this is the test content',
title: 'this is the test title for the topic',
tags: [tag.name],
}
expect(response.status).to eq(422)
json = response.parsed_body
expect(json['errors']).to be_present
end
it 'can create a post with a tag when tagging is enabled' do
SiteSetting.tagging_enabled = true
tag = Fabricate(:tag)
post "/posts.json", params: {
raw: 'this is the test content',
title: 'this is the test title for the topic',
tags: [tag.name],
}
expect(response.status).to eq(200)
expect(Post.last.topic.tags.count).to eq(1)
end
it 'creates the post' do it 'creates the post' do
post "/posts.json", params: { post "/posts.json", params: {
raw: 'this is the test content', raw: 'this is the test content',