From b01364d26b48efeb589bed588b92d2e94b53765c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Tue, 23 Jul 2024 19:21:13 +0200
Subject: [PATCH] Make sure we only return public records in the API

When returning a collection of records in the API, we were making sure
we only returned public ones. However, when returning individual
records, we were not checking that.

In practice, this wasn't a big issue, since most `public_for_api`
methods return all records, but it could affect Consul Democracy
installations which might have customized their `public_for_api` method.
The only exception was the `budget` method, since it was returning
budgets that were still in drafting.
---
 app/graphql/types/budget_type.rb      |   2 +-
 app/graphql/types/query_type.rb       |  20 ++---
 spec/graphql/types/query_type_spec.rb | 109 ++++++++++++++++++++++++++
 3 files changed, 120 insertions(+), 11 deletions(-)

diff --git a/app/graphql/types/budget_type.rb b/app/graphql/types/budget_type.rb
index 92ac2dfbe..87e2cef3a 100644
--- a/app/graphql/types/budget_type.rb
+++ b/app/graphql/types/budget_type.rb
@@ -13,7 +13,7 @@ module Types
     end
 
     def investment(id:)
-      Budget::Investment.find(id)
+      investments.find(id)
     end
   end
 end
diff --git a/app/graphql/types/query_type.rb b/app/graphql/types/query_type.rb
index cb823c8dd..ca90a77e3 100644
--- a/app/graphql/types/query_type.rb
+++ b/app/graphql/types/query_type.rb
@@ -62,7 +62,7 @@ module Types
     end
 
     def budget(id:)
-      Budget.find(id)
+      budgets.find(id)
     end
 
     def comments
@@ -70,7 +70,7 @@ module Types
     end
 
     def comment(id:)
-      Comment.find(id)
+      comments.find(id)
     end
 
     def debates
@@ -78,7 +78,7 @@ module Types
     end
 
     def debate(id:)
-      Debate.find(id)
+      debates.find(id)
     end
 
     def geozones
@@ -86,7 +86,7 @@ module Types
     end
 
     def geozone(id:)
-      Geozone.find(id)
+      geozones.find(id)
     end
 
     def milestones
@@ -94,7 +94,7 @@ module Types
     end
 
     def milestone(id:)
-      Milestone.find(id)
+      milestones.find(id)
     end
 
     def proposals
@@ -102,7 +102,7 @@ module Types
     end
 
     def proposal(id:)
-      Proposal.find(id)
+      proposals.find(id)
     end
 
     def proposal_notifications
@@ -110,7 +110,7 @@ module Types
     end
 
     def proposal_notification(id:)
-      ProposalNotification.find(id)
+      proposal_notifications.find(id)
     end
 
     def tags
@@ -118,7 +118,7 @@ module Types
     end
 
     def tag(id:)
-      Tag.find(id)
+      tags.find(id)
     end
 
     def users
@@ -126,7 +126,7 @@ module Types
     end
 
     def user(id:)
-      User.find(id)
+      users.find(id)
     end
 
     def votes
@@ -134,7 +134,7 @@ module Types
     end
 
     def vote(id:)
-      Vote.find(id)
+      votes.find(id)
     end
   end
 end
diff --git a/spec/graphql/types/query_type_spec.rb b/spec/graphql/types/query_type_spec.rb
index 17ac4fc4a..6e050fd0c 100644
--- a/spec/graphql/types/query_type_spec.rb
+++ b/spec/graphql/types/query_type_spec.rb
@@ -204,6 +204,16 @@ describe Types::QueryType do
     end
   end
 
+  describe "#comment" do
+    it "does not find comments that are not public" do
+      comment = create(:comment, :valuation, body: "Valuation comment")
+
+      expect do
+        execute("{ comment(id: #{comment.id}) { body } }")
+      end.to raise_exception ActiveRecord::RecordNotFound
+    end
+  end
+
   describe "#budgets" do
     it "does not include unpublished budgets" do
       create(:budget, :drafting, name: "Draft")
@@ -215,6 +225,16 @@ describe Types::QueryType do
     end
   end
 
+  describe "#budget" do
+    it "does not find budgets that are not public" do
+      budget = create(:budget, :drafting)
+
+      expect do
+        execute("{ budget(id: #{budget.id}) { name } }")
+      end.to raise_exception ActiveRecord::RecordNotFound
+    end
+  end
+
   describe "#debates" do
     it "does not include hidden debates" do
       create(:debate, title: "Visible")
@@ -240,6 +260,17 @@ describe Types::QueryType do
     end
   end
 
+  describe "#debate" do
+    it "does not find debates that are not public" do
+      allow(Debate).to receive(:public_for_api).and_return(Debate.none)
+      debate = create(:debate)
+
+      expect do
+        execute("{ debate(id: #{debate.id}) { title } } ")
+      end.to raise_exception ActiveRecord::RecordNotFound
+    end
+  end
+
   describe "#proposals" do
     it "does not include hidden proposals" do
       create(:proposal, title: "Visible")
@@ -265,6 +296,17 @@ describe Types::QueryType do
     end
   end
 
+  describe "#proposal" do
+    it "does not find proposals that are not public" do
+      allow(Proposal).to receive(:public_for_api).and_return(Proposal.none)
+      proposal = create(:proposal)
+
+      expect do
+        execute("{ proposal(id: #{proposal.id}) { title } } ")
+      end.to raise_exception ActiveRecord::RecordNotFound
+    end
+  end
+
   describe "#geozones" do
     it "returns geozones" do
       geozone_names = [create(:geozone), create(:geozone)].map(&:name)
@@ -276,6 +318,28 @@ describe Types::QueryType do
     end
   end
 
+  describe "#geozone" do
+    it "does not find geozones that are not public" do
+      allow(Geozone).to receive(:public_for_api).and_return(Geozone.none)
+      geozone = create(:geozone)
+
+      expect do
+        execute("{ geozone(id: #{geozone.id}) { name } }")
+      end.to raise_exception ActiveRecord::RecordNotFound
+    end
+  end
+
+  describe "#milestone" do
+    it "does not find milestones that are not public" do
+      investment = create(:budget_investment, budget: create(:budget, :drafting))
+      milestone = create(:milestone, milestoneable: investment)
+
+      expect do
+        execute("{ milestone(id: #{milestone.id}) { title } }")
+      end.to raise_exception ActiveRecord::RecordNotFound
+    end
+  end
+
   describe "#proposal_notifications" do
     it "does not include proposal notifications for hidden proposals" do
       visible_proposal = create(:proposal)
@@ -301,6 +365,17 @@ describe Types::QueryType do
     end
   end
 
+  describe "#proposal_notification" do
+    it "does not find proposal notifications that are not public" do
+      allow(Proposal).to receive(:public_for_api).and_return(Proposal.none)
+      notification = create(:proposal_notification)
+
+      expect do
+        execute("{ proposal_notification(id: #{notification.id}) { title } } ")
+      end.to raise_exception ActiveRecord::RecordNotFound
+    end
+  end
+
   describe "#tags" do
     it "only display tags with kind nil or category" do
       create(:tag, name: "Parks")
@@ -373,6 +448,29 @@ describe Types::QueryType do
     end
   end
 
+  describe "#tag" do
+    it "does not find tags that are not public" do
+      allow(Proposal).to receive(:public_for_api).and_return(Proposal.none)
+      tag = create(:tag, name: "Health")
+      create(:proposal, tag_list: "Health")
+
+      expect do
+        execute("{ tag(id: #{tag.id}) { name } } ")
+      end.to raise_exception ActiveRecord::RecordNotFound
+    end
+  end
+
+  describe "#user" do
+    it "does not find users that are not public" do
+      allow(User).to receive(:public_for_api).and_return(User.none)
+      user = create(:user)
+
+      expect do
+        execute("{ user(id: #{user.id}) { username } } ")
+      end.to raise_exception ActiveRecord::RecordNotFound
+    end
+  end
+
   describe "#votes" do
     it "only returns votes from proposals, debates and comments" do
       create(:proposal, voters: [create(:user)])
@@ -472,4 +570,15 @@ describe Types::QueryType do
       expect(received_votables).not_to include(not_public_comment.id)
     end
   end
+
+  describe "#vote" do
+    it "does not find votes that are not public" do
+      allow(Debate).to receive(:public_for_api).and_return(Debate.none)
+      vote = create(:vote, votable: create(:debate))
+
+      expect do
+        execute("{ vote(id: #{vote.id}) { votable_id } }")
+      end.to raise_exception ActiveRecord::RecordNotFound
+    end
+  end
 end