From 8dfb1be4d1912e44fece17b80f12d952d843a3c3 Mon Sep 17 00:00:00 2001 From: Sam Date: Tue, 22 Aug 2017 17:53:45 -0400 Subject: [PATCH] FEATURE: unlisted *only* means not listed in topic lists Remove security by obscurity feature that tries for exact slug match If you need to hide a topic from users either move to a secure category or convert to a PM --- app/controllers/topics_controller.rb | 8 ++------ spec/controllers/topics_controller_spec.rb | 13 ++++++++++--- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb index 728ce790564..996dab4d263 100644 --- a/app/controllers/topics_controller.rb +++ b/app/controllers/topics_controller.rb @@ -67,7 +67,7 @@ class TopicsController < ApplicationController # up that particular number if params[:id] && params[:id] =~ /^\d+[^\d\\]+$/ topic = Topic.find_by(slug: params[:id].downcase) - return redirect_to_correct_topic(topic, opts[:post_number]) if topic && topic.visible + return redirect_to_correct_topic(topic, opts[:post_number]) if topic end if opts[:print] @@ -84,7 +84,7 @@ class TopicsController < ApplicationController rescue Discourse::NotFound if params[:id] topic = Topic.find_by(slug: params[:id].downcase) - return redirect_to_correct_topic(topic, opts[:post_number]) if topic && topic.visible + return redirect_to_correct_topic(topic, opts[:post_number]) if topic end raise Discourse::NotFound end @@ -96,10 +96,6 @@ class TopicsController < ApplicationController discourse_expires_in 1.minute - if !@topic_view.topic.visible && @topic_view.topic.slug != params[:slug] && !request.format.json? - raise Discourse::NotFound - end - if slugs_do_not_match || (!request.format.json? && params[:slug].nil?) redirect_to_correct_topic(@topic_view.topic, opts[:post_number]) return diff --git a/spec/controllers/topics_controller_spec.rb b/spec/controllers/topics_controller_spec.rb index b34445c4168..0dc914785b1 100644 --- a/spec/controllers/topics_controller_spec.rb +++ b/spec/controllers/topics_controller_spec.rb @@ -568,7 +568,14 @@ describe TopicsController do end describe 'show unlisted' do - it 'returns 404 unless exact correct URL' do + it 'returns 301 even if slug does not match URL' do + # in the past we had special logic for unlisted topics + # we would require slug unless you made a json call + # this was not really providing any security + # + # we no longer require a topic be visible to perform url correction + # if you need to properly hide a topic for users use a secure category + # or a PM topic = Fabricate(:topic, visible: false) Fabricate(:post, topic: topic) @@ -576,10 +583,10 @@ describe TopicsController do expect(response).to be_success xhr :get, :show, topic_id: topic.id, slug: "just-guessing" - expect(response.code).to eq("404") + expect(response.code).to eq("301") xhr :get, :show, id: topic.slug - expect(response.code).to eq("404") + expect(response.code).to eq("301") end end