diff --git a/lib/topic_view.rb b/lib/topic_view.rb index 54483efd5..333e155b4 100644 --- a/lib/topic_view.rb +++ b/lib/topic_view.rb @@ -355,8 +355,8 @@ class TopicView end def find_topic(topic_id) - finder = Topic.where(id: topic_id).includes(:category) - finder = finder.with_deleted if @guardian.can_see_deleted_topics? + # with_deleted covered in #check_and_raise_exceptions + finder = Topic.with_deleted.where(id: topic_id).includes(:category) finder.first end diff --git a/spec/controllers/topics_controller_spec.rb b/spec/controllers/topics_controller_spec.rb index fd927ebac..e87a27f48 100644 --- a/spec/controllers/topics_controller_spec.rb +++ b/spec/controllers/topics_controller_spec.rb @@ -1,5 +1,23 @@ require 'spec_helper' +def topics_controller_show_gen_perm_tests(expected, ctx) + expected.each do |sym, status| + params = "topic_id: #{sym}.id, slug: #{sym}.slug" + if sym == :nonexist + params = "topic_id: nonexist_topic_id" + end + ctx.instance_eval(" +it 'returns #{status} for #{sym}' do + begin + xhr :get, :show, #{params} + expect(response.status).to eq(#{status}) + rescue Discourse::NotLoggedIn + expect(302).to eq(#{status}) + end +end") + end +end + describe TopicsController do context 'wordpress' do @@ -554,6 +572,108 @@ describe TopicsController do end end + context 'permission errors' do + let(:allowed_user) { Fabricate(:user) } + let(:allowed_group) { Fabricate(:group) } + let(:secure_category) { + c = Fabricate(:category) + c.permissions = [[allowed_group, :full]] + c.save + allowed_user.groups = [allowed_group] + allowed_user.save + c } + let(:normal_topic) { Fabricate(:topic) } + let(:secure_topic) { Fabricate(:topic, category: secure_category) } + let(:private_topic) { Fabricate(:private_message_topic, user: allowed_user) } + let(:deleted_topic) { Fabricate(:deleted_topic) } + let(:nonexist_topic_id) { Topic.last.id + 10000 } + + context 'anonymous' do + expected = { + :normal_topic => 200, + :secure_topic => 403, + :private_topic => 302, + :deleted_topic => 403, + :nonexist => 404 + } + topics_controller_show_gen_perm_tests(expected, self) + end + + context 'anonymous with login required' do + before do + SiteSetting.login_required = true + end + expected = { + :normal_topic => 302, + :secure_topic => 302, + :private_topic => 302, + :deleted_topic => 302, + :nonexist => 302 + } + topics_controller_show_gen_perm_tests(expected, self) + end + + context 'normal user' do + before do + log_in(:user) + end + + expected = { + :normal_topic => 200, + :secure_topic => 403, + :private_topic => 403, + :deleted_topic => 403, + :nonexist => 404 + } + topics_controller_show_gen_perm_tests(expected, self) + end + + context 'allowed user' do + before do + log_in_user(allowed_user) + end + + expected = { + :normal_topic => 200, + :secure_topic => 200, + :private_topic => 200, + :deleted_topic => 403, + :nonexist => 404 + } + topics_controller_show_gen_perm_tests(expected, self) + end + + context 'moderator' do + before do + log_in(:moderator) + end + + expected = { + :normal_topic => 200, + :secure_topic => 403, + :private_topic => 403, + :deleted_topic => 200, + :nonexist => 404 + } + topics_controller_show_gen_perm_tests(expected, self) + end + + context 'admin' do + before do + log_in(:admin) + end + + expected = { + :normal_topic => 200, + :secure_topic => 200, + :private_topic => 200, + :deleted_topic => 200, + :nonexist => 404 + } + topics_controller_show_gen_perm_tests(expected, self) + end + end + it 'records a view' do expect { xhr :get, :show, topic_id: topic.id, slug: topic.slug }.to change(TopicViewItem, :count).by(1) end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 63b3b45b0..701c0de01 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -144,7 +144,6 @@ Spork.prefork do FileUtils.cp("#{Rails.root}/spec/fixtures/images/#{filename}", "#{Rails.root}/tmp/spec/#{filename}") File.new("#{Rails.root}/tmp/spec/#{filename}") end - end Spork.each_run do