From fff8b98485561b12d070c0a8c39f4e503813ab44 Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Mon, 10 Jan 2022 11:28:20 +0800 Subject: [PATCH] SECURITY: Advanced group search did not respect visiblity of groups. --- lib/search.rb | 6 +++- spec/components/search_spec.rb | 54 +++++++++++++++++++++++++++++----- 2 files changed, 52 insertions(+), 8 deletions(-) diff --git a/lib/search.rb b/lib/search.rb index c5abbc503ea..4a3ab96da1e 100644 --- a/lib/search.rb +++ b/lib/search.rb @@ -583,7 +583,11 @@ class Search end advanced_filter(/^group:(.+)$/i) do |posts, match| - group_id = Group.where('name ilike ? OR (id = ? AND id > 0)', match, match.to_i).pluck_first(:id) + group_id = Group + .visible_groups(@guardian.user) + .members_visible_groups(@guardian.user) + .where('name ilike ? OR (id = ? AND id > 0)', match, match.to_i).pluck_first(:id) + if group_id posts.where("posts.user_id IN (select gu.user_id from group_users gu where gu.group_id = ?)", group_id) else diff --git a/spec/components/search_spec.rb b/spec/components/search_spec.rb index 34947fbc24f..f47ca2438ea 100644 --- a/spec/components/search_spec.rb +++ b/spec/components/search_spec.rb @@ -1248,15 +1248,55 @@ describe Search do expect(Search.execute("@#{post_1.user.username}").posts).to contain_exactly(post_1) end - it 'supports group' do - topic = Fabricate(:topic, created_at: 3.months.ago) - post = Fabricate(:post, raw: 'hi this is a test 123 123', topic: topic) + context "searching for posts made by users of a group" do + fab!(:topic) { Fabricate(:topic, created_at: 3.months.ago) } + fab!(:user) { Fabricate(:user) } + fab!(:user_2) { Fabricate(:user) } + fab!(:user_3) { Fabricate(:user) } + fab!(:group) { Fabricate(:group, name: "Like_a_Boss").tap { |g| g.add(user) } } + fab!(:group_2) { Fabricate(:group).tap { |g| g.add(user_2) } } + let!(:post) { Fabricate(:post, raw: 'hi this is a test 123 123', topic: topic, user: user) } + let!(:post_2) { Fabricate(:post, user: user_2) } - group = Group.create!(name: "Like_a_Boss") - GroupUser.create!(user_id: post.user_id, group_id: group.id) + it 'should not return any posts if group does not exist' do + group.update!( + visibility_level: Group.visibility_levels[:public], + members_visibility_level: Group.visibility_levels[:public] + ) - expect(Search.execute('group:like_a_boss').posts.length).to eq(1) - expect(Search.execute('group:"like a brick"').posts.length).to eq(0) + expect(Search.execute('group:99999').posts).to eq([]) + end + + it 'should return the right posts for a public group' do + group.update!( + visibility_level: Group.visibility_levels[:public], + members_visibility_level: Group.visibility_levels[:public] + ) + + expect(Search.execute('group:like_a_boss').posts).to contain_exactly(post) + expect(Search.execute("group:#{group.id}").posts).to contain_exactly(post) + end + + it "should return the right posts for a public group with members' visibility restricted to logged on users" do + group.update!( + visibility_level: Group.visibility_levels[:public], + members_visibility_level: Group.visibility_levels[:logged_on_users] + ) + + expect(Search.execute("group:#{group.id}").posts).to eq([]) + expect(Search.execute("group:#{group.id}", guardian: Guardian.new(user_3)).posts).to contain_exactly(post) + end + + it "should return the right posts for a group with visibility restricted to logged on users with members' visibility restricted to members" do + group.update!( + visibility_level: Group.visibility_levels[:logged_on_users], + members_visibility_level: Group.visibility_levels[:members] + ) + + expect(Search.execute("group:#{group.id}").posts).to eq([]) + expect(Search.execute("group:#{group.id}", guardian: Guardian.new(user_3)).posts).to eq([]) + expect(Search.execute("group:#{group.id}", guardian: Guardian.new(user)).posts).to contain_exactly(post) + end end it 'supports badge' do