SECURITY: Do not expose private group members (#345)

config/locales/server.en.yml

@@ -81,6 +81,8 @@ en:
           invalid_timezone: "Timezone not recognized."
           acting_user_not_allowed_to_create_event: "Current user is not allowed to create events."
           acting_user_not_allowed_to_act_on_this_event: "Current user is not allowed to act on this event."
+          invalid_allowed_groups: "Invalid allowed groups."
+          acting_user_not_allowed_to_invite_these_groups: "Current user is not allowed to invite these groups."
           custom_field_is_invalid: "The custom field `%{field}` is not allowed."
           name:
             length: "Event name length must be between %{minimum} and %{maximum} characters."

jobs/regular/discourse_post_event/bulk_invite.rb

@@ -33,6 +33,8 @@ def execute(args)
     private
 
     def process_invitees(invitees)
+      invitees = filter_out_unavailable_groups(invitees)
+
       max_bulk_invitees = SiteSetting.discourse_post_event_max_bulk_invitees
 
       invitees.each do |invitee|
@@ -113,5 +115,18 @@ def notify_user
         end
       end
     end
+
+    def invitee_groups(invitees)
+      Group.where(name: invitees.map { |i| i[:identifier] })
+    end
+
+    def filter_out_unavailable_groups(invitees)
+      groups = invitee_groups(invitees)
+      invitees.filter do |i|
+        group = groups.find { |g| g.name === i[:identifier] }
+
+        !group || (@guardian.can_see_group?(group) && @guardian.can_see_group_members?(group))
+      end
+    end
   end
 end

lib/discourse_post_event/event_validator.rb

@@ -33,6 +33,8 @@ def validate_event
 
       extracted_event = extracted_events.first
 
+      return false unless can_invite_groups?(extracted_event)
+
       if @post.acting_user && @post.event
         if !@post.acting_user.can_act_on_discourse_post_event?(@post.event)
           @post.errors.add(:base, I18n.t("discourse_post_event.errors.models.event.acting_user_not_allowed_to_act_on_this_event"))
@@ -83,5 +85,28 @@ def validate_event
 
       true
     end
+
+    private
+
+    def can_invite_groups?(event)
+      guardian = Guardian.new(@post.acting_user)
+      return true unless event[:"allowed-groups"]
+
+      event[:"allowed-groups"].split(',').each do |group_name|
+        group = Group.find_by(name: group_name)
+
+        if !group || !guardian.can_see_group?(group)
+          @post.errors.add(:base, I18n.t("discourse_post_event.errors.models.event.invalid_allowed_groups"))
+          return false
+        end
+
+        if !guardian.can_see_group_members?(group)
+          @post.errors.add(:base, I18n.t("discourse_post_event.errors.models.event.acting_user_not_allowed_to_invite_these_groups"))
+          return false
+        end
+      end
+
+      true
+    end
   end
 end

spec/integration/post_spec.rb

@@ -252,6 +252,9 @@
           end
 
           it 'works with allowedGroups attribute' do
+            Fabricate(:group, name: "euro")
+            Fabricate(:group, name: "america")
+
             post = create_post_with_event(user, 'allowedGroups="euro"').reload
             expect(post.event.raw_invitees).to eq([])
 
@@ -569,6 +572,37 @@
         expect(event_1.status).to eq(Event.statuses[:public])
       end
     end
+
+    it "rejects private groups in allowedGroups" do
+      moderator = Fabricate(:user, moderator: true)
+      private_group = Fabricate(
+        :group,
+        visibility_level: Group.visibility_levels[:owners])
+
+      expect {
+        create_post_with_event(moderator, "allowedGroups='#{private_group.name}'")
+      }.to raise_error(ActiveRecord::RecordNotSaved)
+    end
+
+    it "rejects non-existent groups in allowedGroups" do
+      moderator = Fabricate(:user, moderator: true)
+
+      expect {
+        create_post_with_event(moderator, "allowedGroups='non-existent_group_name'")
+      }.to raise_error(ActiveRecord::RecordNotSaved)
+    end
+
+    it "rejects public groups with private members in allowedGroups" do
+      moderator = Fabricate(:user, moderator: true)
+      public_group_with_private_members = Fabricate(
+        :group,
+        visibility_level: Group.visibility_levels[:public],
+        members_visibility_level: Group.visibility_levels[:owners])
+
+      expect {
+        create_post_with_event(moderator, "allowedGroups='#{public_group_with_private_members.name}'")
+      }.to raise_error(ActiveRecord::RecordNotSaved)
+    end
   end
 
   context "with holiday events" do

spec/requests/events_controller_spec.rb

@@ -10,13 +10,13 @@ module DiscoursePostEvent
       SiteSetting.displayed_invitees_limit = 3
     end
 
-    let(:user) { Fabricate(:user, admin: true) }
-    let(:topic) { Fabricate(:topic, user: user) }
-    let(:post1) { Fabricate(:post, user: user, topic: topic) }
-    let(:invitee1) { Fabricate(:user) }
-    let(:invitee2) { Fabricate(:user) }
-
     context 'with an existing post' do
+      let(:user) { Fabricate(:user, admin: true) }
+      let(:topic) { Fabricate(:topic, user: user) }
+      let(:post1) { Fabricate(:post, user: user, topic: topic) }
+      let(:invitee1) { Fabricate(:user) }
+      let(:invitee2) { Fabricate(:user) }
+
       context 'with an existing event' do
         let(:event_1) { Fabricate(:event, post: post1) }
 
@@ -224,5 +224,100 @@ module DiscoursePostEvent
         end
       end
     end
+
+    context 'with a private event' do
+      let(:moderator) { Fabricate(:user, moderator: true) }
+      let(:topic) { Fabricate(:topic, user: moderator) }
+      let(:first_post) { Fabricate(:post, user: moderator, topic: topic) }
+      let(:private_event) { Fabricate(:event, post: first_post, status: Event.statuses[:private]) }
+
+      before do
+        sign_in(moderator)
+      end
+
+      context 'when bulk inviting via CSV file' do
+        def csv_file(content)
+          file = Tempfile.new("invites.csv")
+          file.write(content)
+          file.rewind
+          file
+        end
+
+        it "doesn't invite a private group" do
+          private_group = Fabricate(:group, visibility_level: Group.visibility_levels[:owners])
+
+          file = csv_file("#{private_group.name},going\n")
+          params = { file: fixture_file_upload(file) }
+          post "/discourse-post-event/events/#{private_event.id}/csv-bulk-invite.json", { params: params }
+
+          expect(response.status).to eq(200)
+          private_event.reload
+          expect(private_event.raw_invitees).to be_nil
+        end
+
+        it "returns 200 when inviting a non-existent group" do
+          file = csv_file("non-existent group name,going\n")
+          params = { file: fixture_file_upload(file) }
+          post "/discourse-post-event/events/#{private_event.id}/csv-bulk-invite.json", { params: params }
+
+          expect(response.status).to eq(200)
+        end
+
+        it "doesn't invite a public group with private members" do
+          public_group_with_private_members = Fabricate(
+            :group,
+            visibility_level: Group.visibility_levels[:public],
+            members_visibility_level: Group.visibility_levels[:owners])
+
+          file = csv_file("#{public_group_with_private_members.name},going\n")
+          params = { file: fixture_file_upload(file) }
+          post "/discourse-post-event/events/#{private_event.id}/csv-bulk-invite.json", { params: params }
+
+          expect(response.status).to eq(200)
+          private_event.reload
+          expect(private_event.raw_invitees).to be_nil
+        end
+      end
+
+      context 'when doing bulk inviting via UI' do
+        it "doesn't invite a private group" do
+          private_group = Fabricate(:group, visibility_level: Group.visibility_levels[:owners])
+
+          params = { invitees: [
+            { 'identifier' => private_group.name, 'attendance' => 'going' }
+          ] }
+          post "/discourse-post-event/events/#{private_event.id}/bulk-invite.json", { params: params }
+
+          expect(response.status).to eq(200)
+          private_event.reload
+          expect(private_event.raw_invitees).to be_nil
+        end
+
+        it "returns 200 when inviting a non-existent group" do
+          params = { invitees: [
+            { 'identifier' => 'non-existent group name', 'attendance' => 'going' }
+          ] }
+          post "/discourse-post-event/events/#{private_event.id}/bulk-invite.json", { params: params }
+
+          expect(response.status).to eq(200)
+        end
+
+        it "doesn't invite a public group with private members" do
+          public_group_with_private_members = Fabricate(
+            :group,
+            visibility_level: Group.visibility_levels[:public],
+            members_visibility_level: Group.visibility_levels[:owners])
+
+          params = { invitees: [
+            { 'identifier' => public_group_with_private_members.name, 'attendance' => 'going' }
+          ] }
+          post "/discourse-post-event/events/#{private_event.id}/bulk-invite.json", { params: params }
+
+          expect(response.status).to eq(200)
+          private_event.reload
+          expect(private_event.raw_invitees).to be_nil
+        end
+      end
+    end
   end
 end