SECURITY: Check for no group assignments in data-explorer queries

plugins/discourse-data-explorer/plugin.rb

@@ -50,7 +50,7 @@ after_initialize do
   add_to_class(:guardian, :user_can_access_query?) do |query|
     return false if !current_user
     return true if current_user.admin?
-    query.groups.blank? || query.groups.any? { |group| user_is_a_member_of_group?(group) }
+    query.groups.any? { |group| user_is_a_member_of_group?(group) }
   end
 
   add_to_class(:guardian, :group_and_user_can_access_query?) do |group, query|

plugins/discourse-data-explorer/spec/guardian_spec.rb

@@ -35,6 +35,30 @@ describe Guardian do
     end
   end
 
+  describe "#user_can_access_query?" do
+    it "is true when the user is an admin" do
+      expect(Guardian.new(admin).user_can_access_query?(make_query)).to eq(true)
+    end
+
+    it "is false when the query has no group assignments" do
+      expect(Guardian.new(user).user_can_access_query?(make_query)).to eq(false)
+    end
+
+    it "is true when the user is a member of a group assigned to the query" do
+      query = make_query(["#{group.id}"])
+      group.add(user)
+
+      expect(Guardian.new(user).user_can_access_query?(query)).to eq(true)
+    end
+
+    it "is false when the user is not a member of any group assigned to the query" do
+      other_group = Fabricate(:group)
+      query = make_query(["#{other_group.id}"])
+
+      expect(Guardian.new(user).user_can_access_query?(query)).to eq(false)
+    end
+  end
+
   describe "#group_and_user_can_access_query?" do
     it "is true if the user is an admin" do
       expect(Guardian.new(admin).group_and_user_can_access_query?(group, make_query)).to eq(true)

plugins/discourse-data-explorer/spec/report_generator_spec.rb

@@ -29,6 +29,7 @@ describe DiscourseDataExplorer::ReportGenerator do
     end
 
     it "returns a list of pms for authorised users" do
+      Fabricate(:query_group, query: query, group: group)
       SiteSetting.personal_message_enabled_groups = group.id
       DiscourseDataExplorer::ResultToMarkdown.expects(:convert).returns("le table")
       freeze_time
@@ -126,6 +127,7 @@ describe DiscourseDataExplorer::ReportGenerator do
     end
 
     it "works with duplicate recipients" do
+      Fabricate(:query_group, query: query, group: group)
       DiscourseDataExplorer::ResultToMarkdown.expects(:convert).returns("le table")
       freeze_time
 
@@ -231,6 +233,7 @@ describe DiscourseDataExplorer::ReportGenerator do
     end
 
     it "works with attached csv file" do
+      Fabricate(:query_group, query: query, group: group)
       SiteSetting.personal_message_enabled_groups = group.id
       DiscourseDataExplorer::ResultToMarkdown.expects(:convert).returns("le table")
       freeze_time

plugins/discourse-data-explorer/spec/requests/query_controller_spec.rb

@@ -710,6 +710,13 @@ describe DiscourseDataExplorer::QueryController do
         get "/data-explorer/queries/#{query.id}/run.json"
         expect(response.status).to eq(404)
       end
+
+      it "returns 404 when the query has no group assignments" do
+        query = make_query("SELECT 1 as value", {}, [])
+
+        get "/data-explorer/queries/#{query.id}/run.json"
+        expect(response.status).to eq(404)
+      end
     end
   end