FIX: Do not error when anon user looks at secure upload for deleted post (#19792)

If a secure upload's access_control_post was trashed, and an anon user
tried to look at that upload, they would get a 500 error rather than
the correct 403 because of an error inside the PostGuardian logic.

lib/guardian/post_guardian.rb

@@ -239,10 +239,17 @@ module PostGuardian
     return false unless can_see_post_topic?(post)
     return false unless post.user == @user || Topic.visible_post_types(@user).include?(post.post_type)
     return true if is_moderator? || is_category_group_moderator?(post.topic.category)
-    return true if post.deleted_at.blank? || (post.deleted_by_id == @user.id && @user.has_trust_level?(TrustLevel[4]))
+    return true if !post.trashed? || can_see_deleted_post?(post)
     false
   end
 
+  def can_see_deleted_post?(post)
+    return false if !post.trashed?
+    return false if @user.anonymous?
+    return true if is_staff?
+    post.deleted_by_id == @user.id && @user.has_trust_level?(TrustLevel[4])
+  end
+
   def can_view_edit_history?(post)
     return false unless post
 

spec/lib/guardian_spec.rb

@@ -785,6 +785,46 @@ RSpec.describe Guardian do
     end
   end
 
+  describe "can_see_deleted_post?" do
+    fab!(:post) { Fabricate(:post) }
+
+    before do
+      post.trash!(user)
+    end
+
+    it "returns false for post that is not deleted" do
+      post.recover!
+      expect(Guardian.new(admin).can_see_deleted_post?(post)).to be_falsey
+    end
+
+    it "returns false for anon" do
+      expect(Guardian.new.can_see_deleted_post?(post)).to be_falsey
+    end
+
+    it "returns true for admin" do
+      expect(Guardian.new(admin).can_see_deleted_post?(post)).to be_truthy
+    end
+
+    it "returns true for mods" do
+      expect(Guardian.new(moderator).can_see_deleted_post?(post)).to be_truthy
+    end
+
+    it "returns false for < TL4 users" do
+      user.update!(trust_level: TrustLevel[1])
+      expect(Guardian.new(user).can_see_deleted_post?(post)).to be_falsey
+    end
+
+    it "returns false if not ther person who deleted it" do
+      post.update!(deleted_by: another_user)
+      expect(Guardian.new(user).can_see_deleted_post?(post)).to be_falsey
+    end
+
+    it "returns true for TL4 users' own posts" do
+      user.update!(trust_level: TrustLevel[4])
+      expect(Guardian.new(user).can_see_deleted_post?(post)).to be_truthy
+    end
+  end
+
   describe 'can_see?' do
 
     it 'returns false with a nil object' do

spec/requests/uploads_controller_spec.rb

@@ -505,13 +505,41 @@ RSpec.describe UploadsController do
         let!(:private_category) { Fabricate(:private_category, group: Fabricate(:group)) }
 
         before do
-          sign_in(user)
           upload.update(access_control_post_id: post.id)
         end
 
+        context "when the user is anon" do
+          it "should return signed url for public posts" do
+            get secure_url
+            expect(response.status).to eq(302)
+            expect(response.redirect_url).to match("Amz-Expires")
+          end
+
+          it "should return 403 for deleted posts" do
+            post.trash!
+            get secure_url
+            expect(response.status).to eq(403)
+          end
+
+          context "when the user does not have access to the post via guardian" do
+            before do
+              post.topic.change_category_to_id(private_category.id)
+            end
+
+            it "returns a 403" do
+              get secure_url
+              expect(response.status).to eq(403)
+            end
+          end
+        end
+
+        context "when the user is logged in" do
+          before do
+            sign_in(user)
+          end
+
           context "when the user has access to the post via guardian" do
             it "should return signed url for legitimate request" do
-            sign_in(user)
               get secure_url
               expect(response.status).to eq(302)
               expect(response.redirect_url).to match("Amz-Expires")
@@ -524,12 +552,12 @@ RSpec.describe UploadsController do
             end
 
             it "returns a 403" do
-            sign_in(user)
               get secure_url
               expect(response.status).to eq(403)
             end
           end
         end
+      end
 
       context "when the upload is an attachment file" do
         before do