FIX: paginating posts should allow for deletions and PMs (#27098)

Note this may have performance issues in some cases, will need to be monitored

Previous to this change we were bracketing on 50 id windows. They may end up
having zero posts we are searching for leading to posts.rss and .json returning
no results.

- avoids Post.last.id which is expensive
- order by id desc which is better cause we bracket on id

app/controllers/posts_controller.rb

@@ -61,41 +61,55 @@ class PostsController < ApplicationController
   def latest
     params.permit(:before)
     last_post_id = params[:before].to_i
-    last_post_id = Post.last.id if last_post_id <= 0
+    last_post_id = nil if last_post_id <= 0
 
     if params[:id] == "private_posts"
       raise Discourse::NotFound if current_user.nil?
+
+      allowed_private_topics = TopicAllowedUser.where(user_id: current_user.id).select(:topic_id)
+
+      allowed_groups = GroupUser.where(user_id: current_user.id).select(:group_id)
+      allowed_private_topics_by_group =
+        TopicAllowedGroup.where(group_id: allowed_groups).select(:topic_id)
+
+      all_allowed =
+        Topic
+          .where(id: allowed_private_topics)
+          .or(Topic.where(id: allowed_private_topics_by_group))
+          .select(:id)
+
       posts =
         Post
           .private_posts
-          .order(created_at: :desc)
+          .order(id: :desc)
-          .where("posts.id <= ?", last_post_id)
-          .where("posts.id > ?", last_post_id - 50)
           .includes(topic: :category)
           .includes(user: %i[primary_group flair_group])
           .includes(:reply_to_user)
           .limit(50)
       rss_description = I18n.t("rss_description.private_posts")
+
+      posts = posts.where(topic_id: all_allowed) if !current_user.admin?
     else
       posts =
         Post
           .public_posts
           .visible
           .where(post_type: Post.types[:regular])
-          .order(created_at: :desc)
+          .order(id: :desc)
-          .where("posts.id <= ?", last_post_id)
-          .where("posts.id > ?", last_post_id - 50)
           .includes(topic: :category)
           .includes(user: %i[primary_group flair_group])
           .includes(:reply_to_user)
+          .where("categories.id" => Category.secured(guardian).select(:id))
           .limit(50)
+
       rss_description = I18n.t("rss_description.posts")
       @use_canonical = true
     end
 
-    # Remove posts the user doesn't have permission to see
+    posts = posts.where("posts.id <= ?", last_post_id) if last_post_id
-    # This isn't leaking any information we weren't already through the post ID numbers
+
-    posts = posts.reject { |post| !guardian.can_see?(post) || post.topic.blank? }
+    posts = posts.to_a
+
     counts = PostAction.counts_for(posts, current_user)
 
     respond_to do |format|

spec/requests/posts_controller_spec.rb

@@ -92,7 +92,6 @@ RSpec.describe PostsController do
   let(:topicless_post) { Fabricate(:post, user: user, raw: "<p>Car 54, where are you?</p>") }
 
   let(:private_topic) { Fabricate(:topic, archetype: Archetype.private_message, category_id: nil) }
-
   let(:private_post) { Fabricate(:post, user: user, topic: private_topic) }
 
   describe "#show" do
@@ -2440,7 +2439,7 @@ RSpec.describe PostsController do
     it "can show whole topics" do
       topic = Fabricate(:topic)
       post = Fabricate(:post, topic: topic, post_number: 1, raw: "123456789")
-      post_2 = Fabricate(:post, topic: topic, post_number: 2, raw: "abcdefghij")
+      _post_2 = Fabricate(:post, topic: topic, post_number: 2, raw: "abcdefghij")
       post.save
       get "/raw/#{topic.id}"
       expect(response.status).to eq(200)
@@ -2580,6 +2579,23 @@ RSpec.describe PostsController do
         expect(body).to_not include(public_post.url)
       end
 
+      it "properly secures private posts" do
+        sign_in(user)
+
+        private_post
+
+        pm = Fabricate(:private_message_topic, recipient: user)
+        post_id = Fabricate(:post, topic: pm).id
+
+        get "/private-posts.json"
+        expect(response.status).to eq(200)
+
+        json = response.parsed_body
+        post_ids = json["private_posts"].map { |p| p["id"] }
+
+        expect(post_ids).to eq([post_id])
+      end
+
       it "returns private posts for json" do
         sign_in(admin)
 
@@ -2614,6 +2630,24 @@ RSpec.describe PostsController do
         expect(body).to_not include(private_post.url)
       end
 
+      it "doesn't include posts from secured categories you have no access to" do
+        public_post
+        private_post
+
+        category = Fabricate(:category, read_restricted: true)
+        topic = Fabricate(:topic, category: category)
+        secure_post = Fabricate(:post, topic: topic)
+
+        get "/posts.json"
+
+        expect(response.status).to eq(200)
+
+        body = response.parsed_body
+        ids = body["latest_posts"].map { |p| p["id"] }
+
+        expect(ids).not_to include secure_post.id
+      end
+
       it "doesn't include posts from hidden topics" do
         public_post.topic.update!(visible: false)