Merge pull request #3785 from riking/410

FIX: Return 410 Gone for deleted topics you could otherwise see

app/controllers/topics_controller.rb

@@ -93,6 +93,11 @@ class TopicsController < ApplicationController
       Notification.remove_for(current_user.id, params[:topic_id])
     end
 
+    if ex.obj && Topic === ex.obj && guardian.can_see_topic_if_not_deleted?(ex.obj)
+      rescue_discourse_actions(:not_found, 410)
+      return
+    end
+
     raise ex
   end
 

lib/discourse.rb

@@ -43,7 +43,13 @@ module Discourse
   class InvalidParameters < StandardError; end
 
   # When they don't have permission to do something
-  class InvalidAccess < StandardError; end
+  class InvalidAccess < StandardError
+    attr_reader :obj
+    def initialize(msg=nil, obj=nil)
+      super(msg)
+      @obj = obj
+    end
+  end
 
   # When something they want is not found
   class NotFound < StandardError; end

lib/guardian/topic_guardian.rb

@@ -72,6 +72,22 @@ module TopicGuardian
     !topic.read_restricted_category? || can_see_category?(topic.category)
   end
 
+  def can_see_topic_if_not_deleted?(topic)
+    return false unless topic
+    # Admins can see everything
+    return true if is_admin?
+    # Deleted topics
+    # return false if topic.deleted_at && !can_see_deleted_topics?
+
+    if topic.private_message?
+      return authenticated? &&
+        topic.all_allowed_users.where(id: @user.id).exists?
+    end
+
+    # not secure, or I can see it
+    !topic.read_restricted_category? || can_see_category?(topic.category)
+  end
+
   def filter_allowed_categories(records)
     unless is_admin?
       allowed_ids = allowed_category_ids

lib/topic_view.rb

@@ -417,7 +417,7 @@ class TopicView
     if @topic.present? && @topic.private_message? && @user.blank?
       raise Discourse::NotLoggedIn.new
     end
-    guardian.ensure_can_see!(@topic)
+    raise Discourse::InvalidAccess.new("can't see #{@topic}", @topic) unless guardian.can_see?(@topic)
   end
 
 

spec/controllers/topics_controller_spec.rb

@@ -588,6 +588,8 @@ describe TopicsController do
       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(:deleted_secure_topic) { Fabricate(:topic, category: secure_category, deleted_at: 1.day.ago) }
+      let(:deleted_private_topic) { Fabricate(:private_message_topic, user: allowed_user, deleted_at: 1.day.ago) }
       let(:nonexist_topic_id) { Topic.last.id + 10000 }
 
       context 'anonymous' do
@@ -595,7 +597,9 @@ describe TopicsController do
           :normal_topic => 200,
           :secure_topic => 403,
           :private_topic => 302,
-          :deleted_topic => 403,
+          :deleted_topic => 410,
+          :deleted_secure_topic => 403,
+          :deleted_private_topic => 302,
           :nonexist => 404
         }
         topics_controller_show_gen_perm_tests(expected, self)
@@ -610,6 +614,8 @@ describe TopicsController do
           :secure_topic => 302,
           :private_topic => 302,
           :deleted_topic => 302,
+          :deleted_secure_topic => 302,
+          :deleted_private_topic => 302,
           :nonexist => 302
         }
         topics_controller_show_gen_perm_tests(expected, self)
@@ -624,7 +630,9 @@ describe TopicsController do
           :normal_topic => 200,
           :secure_topic => 403,
           :private_topic => 403,
-          :deleted_topic => 403,
+          :deleted_topic => 410,
+          :deleted_secure_topic => 403,
+          :deleted_private_topic => 403,
           :nonexist => 404
         }
         topics_controller_show_gen_perm_tests(expected, self)
@@ -639,7 +647,9 @@ describe TopicsController do
           :normal_topic => 200,
           :secure_topic => 200,
           :private_topic => 200,
-          :deleted_topic => 403,
+          :deleted_topic => 410,
+          :deleted_secure_topic => 410,
+          :deleted_private_topic => 410,
           :nonexist => 404
         }
         topics_controller_show_gen_perm_tests(expected, self)
@@ -655,6 +665,8 @@ describe TopicsController do
           :secure_topic => 403,
           :private_topic => 403,
           :deleted_topic => 200,
+          :deleted_secure_topic => 403,
+          :deleted_private_topic => 403,
           :nonexist => 404
         }
         topics_controller_show_gen_perm_tests(expected, self)
@@ -670,6 +682,8 @@ describe TopicsController do
           :secure_topic => 200,
           :private_topic => 200,
           :deleted_topic => 200,
+          :deleted_secure_topic => 200,
+          :deleted_private_topic => 200,
           :nonexist => 404
         }
         topics_controller_show_gen_perm_tests(expected, self)