FIX: Issue 404 for invalid `/tags/c/...` routes

Previously we would issue a 403 for all invalid routes under `/tags/c/...`, which is not semantically correct. In some cases, these 403'd routes would then be handled successfully in the Ember app, leading to some very confusing behavior.
This commit is contained in:
David Taylor 2022-03-21 16:53:38 +00:00
parent b8a98708d8
commit 80dd769530
2 changed files with 16 additions and 2 deletions

View File

@ -375,14 +375,15 @@ class TagsController < ::ApplicationController
@filter_on_category = Category.query_category(slug_or_id, nil) @filter_on_category = Category.query_category(slug_or_id, nil)
end end
guardian.ensure_can_see!(@filter_on_category)
if !@filter_on_category if !@filter_on_category
permalink = Permalink.find_by_url("c/#{params[:category_slug_path_with_id]}") permalink = Permalink.find_by_url("c/#{params[:category_slug_path_with_id]}")
if permalink.present? && permalink.category_id if permalink.present? && permalink.category_id
return redirect_to "#{Discourse::base_path}/tags#{permalink.target_url}/#{params[:tag_id]}", status: :moved_permanently return redirect_to "#{Discourse::base_path}/tags#{permalink.target_url}/#{params[:tag_id]}", status: :moved_permanently
end end
end
if !(@filter_on_category && guardian.can_see?(@filter_on_category))
# 404 on 'access denied' to avoid leaking existence of category
raise Discourse::NotFound raise Discourse::NotFound
end end
end end

View File

@ -285,6 +285,19 @@ describe TagsController do
expect(response.parsed_body['topic_list']['more_topics_url']) expect(response.parsed_body['topic_list']['more_topics_url'])
.to start_with("/tags/c/#{category.slug_path.join('/')}/#{category.id}/#{tag.name}") .to start_with("/tags/c/#{category.slug_path.join('/')}/#{category.id}/#{tag.name}")
end end
it "should 404 for invalid category path" do
get "/tags/c/#{category.slug_path.join("/")}/#{category.id}/somerandomstring/#{tag.name}.json?per_page=1"
expect(response.status).to eq(404)
end
it "should 404 for secure categories" do
c = Fabricate(:private_category, group: Fabricate(:group))
get "/tags/c/#{c.slug_path.join("/")}/#{c.id}/#{tag.name}.json"
expect(response.status).to eq(404)
end
end end
context "with a subcategory in the path" do context "with a subcategory in the path" do