mirror of
https://github.com/discourse/discourse.git
synced 2026-08-06 13:08:40 +08:00
baa573d414Previously, filtering a user search by group (e.g. `/u/search/users?groups=admins`) only checked whether the requester could see the group's members, not the group itself. Because the default automatic groups are visible to logged-in users but expose their members publicly, an anonymous or non-member user could enumerate the members of a group hidden from them, even though every other path that lists users by group (the user directory, `/g/:name/members`, group topic lists) already required both checks. This adds a `Guardian#can_see_group_and_members?` helper that requires both group and member visibility, and routes the user search, user directory, and plugin group-listing paths through it. This closes the leak, fixes a related all-or-nothing bug in `can_see_groups?` when several groups are filtered at once, and keeps the rule in one place so a caller can no longer reintroduce it by checking only half.
397 lines
15 KiB
Ruby
Vendored
397 lines
15 KiB
Ruby
Vendored
# frozen_string_literal: true
|
|
|
|
RSpec.describe UserSearch do
|
|
before_all { SearchIndexer.enable } # Enable for prefabrication
|
|
before { SearchIndexer.enable } # Enable for each test
|
|
|
|
fab!(:topic)
|
|
fab!(:topic2, :topic)
|
|
fab!(:topic3, :topic)
|
|
fab!(:topic4, :topic)
|
|
fab!(:mr_b) do
|
|
Fabricate :user, username: "mrb", name: "Michael Madsen", last_seen_at: 10.days.ago
|
|
end
|
|
fab!(:mr_blue) do
|
|
Fabricate :user, username: "mrblue", name: "Eddie Code", last_seen_at: 9.days.ago
|
|
end
|
|
fab!(:mr_orange) do
|
|
Fabricate :user, username: "mrorange", name: "Tim Roth", last_seen_at: 8.days.ago
|
|
end
|
|
fab!(:mr_pink) do
|
|
Fabricate :user, username: "mrpink", name: "Steve Buscemi", last_seen_at: 7.days.ago
|
|
end
|
|
fab!(:mr_brown) do
|
|
Fabricate :user, username: "mrbrown", name: "Quentin Tarantino", last_seen_at: 6.days.ago
|
|
end
|
|
fab!(:mr_white) do
|
|
Fabricate :user, username: "mrwhite", name: "Harvey Keitel", last_seen_at: 5.days.ago
|
|
end
|
|
fab!(:inactive) { Fabricate :user, username: "Ghost", active: false }
|
|
fab!(:admin) { Fabricate :admin, username: "theadmin" }
|
|
fab!(:moderator) { Fabricate :moderator, username: "themod" }
|
|
fab!(:staged)
|
|
|
|
def search_for(*args)
|
|
# mapping "username" so it's easier to debug
|
|
UserSearch.new(*args).search.map(&:username)
|
|
end
|
|
|
|
context "with a secure category" do
|
|
fab!(:user)
|
|
fab!(:searching_user, :user)
|
|
fab!(:group)
|
|
fab!(:category) { Fabricate(:category, read_restricted: true, user: user) }
|
|
|
|
before_all do
|
|
Fabricate(:category_group, category: category, group: group)
|
|
|
|
group.add(user)
|
|
group.add(searching_user)
|
|
group.save
|
|
end
|
|
|
|
it "autocompletes with people in the category" do
|
|
results = search_for("", searching_user: searching_user, category_id: category.id)
|
|
expect(results).to eq [user.username]
|
|
end
|
|
|
|
it "will lookup the category from the topic id" do
|
|
topic = Fabricate(:topic, category: category)
|
|
Fabricate(:post, user: topic.user, topic: topic)
|
|
|
|
results = search_for("", searching_user: searching_user, topic_id: topic.id)
|
|
|
|
expect(results).to eq [topic.user, user].map(&:username)
|
|
end
|
|
|
|
it "will raise an error if the user cannot see the category" do
|
|
expect do
|
|
search_for("", searching_user: Fabricate(:user), category_id: category.id)
|
|
end.to raise_error(Discourse::InvalidAccess)
|
|
end
|
|
|
|
it "will respect the group member visibility setting" do
|
|
group.update(members_visibility_level: Group.visibility_levels[:owners])
|
|
results = search_for("", searching_user: searching_user, category_id: category.id)
|
|
expect(results).to be_blank
|
|
|
|
group.add_owner(searching_user)
|
|
results = search_for("", searching_user: searching_user, category_id: category.id)
|
|
expect(results).to eq [user.username]
|
|
end
|
|
end
|
|
|
|
it "allows for correct underscore searching" do
|
|
Fabricate(:user, username: "undertaker")
|
|
under_score = Fabricate(:user, username: "Under_Score")
|
|
|
|
expect(search_for("under_sc")).to eq [under_score.username]
|
|
expect(search_for("under_")).to eq [under_score.username]
|
|
end
|
|
|
|
it "allows filtering by group" do
|
|
sam = Fabricate(:user, username: "sam")
|
|
Fabricate(:user, username: "samantha")
|
|
|
|
group = Fabricate(:group)
|
|
group.add(sam)
|
|
|
|
results = search_for("sam", groups: [group])
|
|
expect(results).to eq [sam.username]
|
|
end
|
|
|
|
it "allows filtering by multiple groups" do
|
|
sam = Fabricate(:user, username: "sam")
|
|
samantha = Fabricate(:user, username: "samantha")
|
|
|
|
group_1 = Fabricate(:group)
|
|
group_1.add(sam)
|
|
|
|
group_2 = Fabricate(:group)
|
|
group_2.add(samantha)
|
|
|
|
results = search_for("sam", groups: [group_1, group_2])
|
|
expect(results).to eq [sam, samantha].map(&:username)
|
|
end
|
|
|
|
it "raises when filtering by a group the user cannot see, even if its members are public" do
|
|
hidden =
|
|
Fabricate(
|
|
:group,
|
|
visibility_level: Group.visibility_levels[:logged_on_users],
|
|
members_visibility_level: Group.visibility_levels[:public],
|
|
)
|
|
hidden.add(Fabricate(:user, username: "hiddenmember"))
|
|
|
|
expect { search_for("hiddenmember", groups: [hidden]) }.to raise_error(Discourse::InvalidAccess)
|
|
|
|
expect(search_for("hiddenmember", groups: [hidden], searching_user: Fabricate(:user))).to eq(
|
|
["hiddenmember"],
|
|
)
|
|
end
|
|
|
|
it "raises when a non-staff user filters by a group only staff can see" do
|
|
staff_only =
|
|
Fabricate(
|
|
:group,
|
|
visibility_level: Group.visibility_levels[:staff],
|
|
members_visibility_level: Group.visibility_levels[:public],
|
|
)
|
|
staff_only.add(Fabricate(:user, username: "staffgroupmember"))
|
|
|
|
expect {
|
|
search_for("staffgroupmember", groups: [staff_only], searching_user: Fabricate(:user))
|
|
}.to raise_error(Discourse::InvalidAccess)
|
|
|
|
expect(search_for("staffgroupmember", groups: [staff_only], searching_user: admin)).to eq(
|
|
["staffgroupmember"],
|
|
)
|
|
end
|
|
|
|
it "checks each group's visibility independently for mixed-visibility filters" do
|
|
member = Fabricate(:user, username: "mixedmember")
|
|
public_group = Fabricate(:group, visibility_level: Group.visibility_levels[:public])
|
|
logged_on_group = Fabricate(:group, visibility_level: Group.visibility_levels[:logged_on_users])
|
|
public_group.add(member)
|
|
logged_on_group.add(member)
|
|
|
|
expect(
|
|
search_for(
|
|
"mixedmember",
|
|
groups: [public_group, logged_on_group],
|
|
searching_user: Fabricate(:user),
|
|
),
|
|
).to eq(["mixedmember"])
|
|
end
|
|
|
|
it "does not let membership in one group grant visibility into a co-filtered hidden group" do
|
|
searcher = Fabricate(:user)
|
|
visible_group = Fabricate(:group, visibility_level: Group.visibility_levels[:public])
|
|
hidden_group = Fabricate(:group, visibility_level: Group.visibility_levels[:owners])
|
|
visible_group.add(searcher)
|
|
hidden_group.add(searcher)
|
|
|
|
expect {
|
|
search_for("searcher", groups: [visible_group, hidden_group], searching_user: searcher)
|
|
}.to raise_error(Discourse::InvalidAccess)
|
|
end
|
|
|
|
context "with seed data" do
|
|
fab!(:post1) { Fabricate :post, user: mr_b, topic: topic }
|
|
fab!(:post2) { Fabricate :post, user: mr_blue, topic: topic2 }
|
|
fab!(:post3) { Fabricate :post, user: mr_orange, topic: topic }
|
|
fab!(:post4) { Fabricate :post, user: mr_pink, topic: topic }
|
|
fab!(:post5) { Fabricate :post, user: mr_brown, topic: topic3 }
|
|
fab!(:post6) { Fabricate :post, user: mr_white, topic: topic }
|
|
fab!(:post7) { Fabricate :post, user: staged, topic: topic4 }
|
|
fab!(:post8) { Fabricate :post, user: mr_brown, topic: topic2, post_type: Post.types[:whisper] }
|
|
|
|
before { mr_white.update(suspended_at: 1.day.ago, suspended_till: 1.year.from_now) }
|
|
|
|
it "can search by name and username" do
|
|
# normal search
|
|
results = search_for(mr_b.name.split.first)
|
|
expect(results).to eq [mr_b.username]
|
|
|
|
# lower case
|
|
results = search_for(mr_b.name.split.first.downcase)
|
|
expect(results).to eq [mr_b.username]
|
|
|
|
# username
|
|
results = search_for(mr_pink.username)
|
|
expect(results).to eq [mr_pink.username]
|
|
|
|
# case insensitive
|
|
results = search_for(mr_pink.username.upcase)
|
|
expect(results).to eq [mr_pink.username]
|
|
end
|
|
|
|
it "handles substring search correctly" do
|
|
results = search_for("mr")
|
|
expect(results).to eq [mr_brown, mr_pink, mr_orange, mr_blue, mr_b].map(&:username)
|
|
|
|
results = search_for("mr", searching_user: mr_b)
|
|
expect(results).to eq [mr_brown, mr_pink, mr_orange, mr_blue, mr_b].map(&:username)
|
|
|
|
# only staff members see suspended users in results
|
|
results = search_for("mr", searching_user: moderator)
|
|
expect(results).to eq [mr_white, mr_brown, mr_pink, mr_orange, mr_blue, mr_b].map(&:username)
|
|
|
|
results = search_for("mr", searching_user: admin)
|
|
expect(results).to eq [mr_white, mr_brown, mr_pink, mr_orange, mr_blue, mr_b].map(&:username)
|
|
|
|
results = search_for(mr_b.username, searching_user: admin)
|
|
expect(results).to eq [mr_b, mr_brown, mr_blue].map(&:username)
|
|
|
|
results = search_for("MR", searching_user: admin)
|
|
expect(results).to eq [mr_white, mr_brown, mr_pink, mr_orange, mr_blue, mr_b].map(&:username)
|
|
|
|
results = search_for("MRB", searching_user: admin, limit: 2)
|
|
expect(results).to eq [mr_b, mr_brown].map(&:username)
|
|
end
|
|
|
|
it "prioritises topic participants" do
|
|
results = search_for(mr_b.username, topic_id: topic.id)
|
|
expect(results).to eq [mr_b, mr_brown, mr_blue].map(&:username)
|
|
|
|
results = search_for(mr_b.username, topic_id: topic2.id)
|
|
expect(results).to eq [mr_b, mr_blue, mr_brown].map(&:username)
|
|
|
|
results = search_for(mr_b.username, topic_id: topic3.id)
|
|
expect(results).to eq [mr_b, mr_brown, mr_blue].map(&:username)
|
|
end
|
|
|
|
it "prioritises the replying to user within a topic" do
|
|
results = search_for("mr", topic_id: topic.id, prioritized_user_id: mr_b.id)
|
|
expect(results).to eq [mr_b, mr_pink, mr_orange, mr_brown, mr_blue].map(&:username)
|
|
|
|
results = search_for("mr", topic_id: topic.id, prioritized_user_id: mr_orange.id)
|
|
expect(results).to eq [mr_orange, mr_pink, mr_b, mr_brown, mr_blue].map(&:username)
|
|
|
|
results = search_for("mr", topic_id: topic.id, prioritized_user_id: mr_pink.id)
|
|
expect(results).to eq [mr_pink, mr_orange, mr_b, mr_brown, mr_blue].map(&:username)
|
|
end
|
|
|
|
it "returns the replying to user if the term includes the username" do
|
|
results = search_for(mr_blue.username, topic_id: topic.id, prioritized_user_id: post1.user_id)
|
|
expect(results).to eq [mr_blue].map(&:username)
|
|
results = search_for(mr_blue.username, topic_id: topic.id, prioritized_user_id: post3.user_id)
|
|
expect(results).to eq [mr_blue].map(&:username)
|
|
end
|
|
|
|
it "returns firstly the replying to user if the term is blank" do
|
|
results = search_for("", topic_id: topic.id, prioritized_user_id: post1.user_id)
|
|
expect(results).to eq [mr_b, mr_pink, mr_orange].map(&:username)
|
|
|
|
results = search_for("", topic_id: topic.id, prioritized_user_id: post3.user_id)
|
|
expect(results).to eq [mr_orange, mr_pink, mr_b].map(&:username)
|
|
|
|
results = search_for("", topic_id: topic.id, prioritized_user_id: post4.user_id)
|
|
expect(results).to eq [mr_pink, mr_orange, mr_b].map(&:username)
|
|
end
|
|
|
|
it "does not reveal whisper users" do
|
|
results = search_for("", topic_id: topic2.id)
|
|
expect(results).to eq [mr_blue.username]
|
|
end
|
|
|
|
it "does not include deleted posts users" do
|
|
post4.trash!
|
|
results = search_for("", topic_id: topic.id)
|
|
expect(results).to eq [mr_orange, mr_b].map(&:username)
|
|
end
|
|
|
|
it "only reveals topic participants to people with permission" do
|
|
pm_topic =
|
|
Fabricate(:private_message_post, user: Fabricate(:user, refresh_auto_groups: true)).topic
|
|
|
|
# Anonymous, does not have access
|
|
expect do search_for("", topic_id: pm_topic.id) end.to raise_error(Discourse::InvalidAccess)
|
|
|
|
# Random user, does not have access
|
|
expect do search_for("", topic_id: pm_topic.id, searching_user: mr_b) end.to raise_error(
|
|
Discourse::InvalidAccess,
|
|
)
|
|
|
|
pm_topic.invite(pm_topic.user, mr_b.username)
|
|
|
|
results = search_for("", topic_id: pm_topic.id, searching_user: mr_b)
|
|
expect(results).to eq [pm_topic.user.username]
|
|
end
|
|
|
|
it "only searches by name when enabled" do
|
|
# When searching by name is enabled, it returns the record
|
|
SiteSetting.enable_names = true
|
|
results = search_for("Tarantino")
|
|
expect(results).to eq [mr_brown.username]
|
|
|
|
results = search_for("coding")
|
|
expect(results).to be_blank
|
|
|
|
results = search_for("z")
|
|
expect(results).to be_blank
|
|
|
|
# When searching by name is disabled, it will not return the record
|
|
SiteSetting.enable_names = false
|
|
results = search_for("Tarantino")
|
|
expect(results).to be_blank
|
|
end
|
|
|
|
it "does not show unapproved users when must_approve_users enabled" do
|
|
SiteSetting.must_approve_users = true
|
|
unapproved = Fabricate(:user, username: "mrunapproved", active: true, approved: false)
|
|
approved = Fabricate(:user, username: "mrapproved", active: true, approved: true)
|
|
|
|
users = search_for(unapproved.username)
|
|
expect(users).to be_blank
|
|
|
|
users = search_for(approved.username)
|
|
expect(users).not_to be_blank
|
|
end
|
|
|
|
it "prioritises exact matches" do
|
|
results = search_for("mrB")
|
|
expect(results).to eq [mr_b, mr_brown, mr_blue].map(&:username)
|
|
end
|
|
|
|
it "doesn't prioritises exact matches mentions for users who haven't been seen in over a year" do
|
|
abcdef = Fabricate(:user, username: "abcdef", last_seen_at: 2.days.ago)
|
|
abcde = Fabricate(:user, username: "abcde", last_seen_at: 2.weeks.ago)
|
|
abcd = Fabricate(:user, username: "abcd", last_seen_at: 2.months.ago)
|
|
abc = Fabricate(:user, username: "abc", last_seen_at: 2.years.ago)
|
|
|
|
results = search_for("abc", topic_id: topic.id)
|
|
expect(results).to eq [abcdef, abcde, abcd, abc].map(&:username)
|
|
end
|
|
|
|
it "does not include self, staged or inactive" do
|
|
# don't return inactive users
|
|
results = search_for(inactive.username)
|
|
expect(results).to be_blank
|
|
|
|
# don't return staged users
|
|
results = search_for(staged.username)
|
|
expect(results).to be_blank
|
|
|
|
results = search_for(staged.username, include_staged_users: true)
|
|
expect(results).to eq [staged.username]
|
|
|
|
# mrb is omitted since they're the searching user
|
|
results = search_for("", topic_id: topic.id, searching_user: mr_b)
|
|
expect(results).to eq [mr_pink, mr_orange].map(&:username)
|
|
end
|
|
|
|
it "works with last_seen_users option" do
|
|
results = search_for("", last_seen_users: true)
|
|
|
|
expect(results).not_to be_blank
|
|
expect(results[0]).to eq("mrbrown")
|
|
expect(results[1]).to eq("mrpink")
|
|
expect(results[2]).to eq("mrorange")
|
|
end
|
|
end
|
|
|
|
context "with the search_custom_fields option" do
|
|
fab!(:searchable_field) { Fabricate(:user_field, searchable: true, show_on_profile: true) }
|
|
fab!(:member) { Fabricate(:user, username: "memberx", name: "Member X") }
|
|
|
|
before do
|
|
member.set_user_field(searchable_field.id, "Acme-Corp")
|
|
member.save_custom_fields
|
|
SearchIndexer.index(member.reload, force: true)
|
|
end
|
|
|
|
it "matches a custom field value containing `_`, `.` or `-` when the option is set" do
|
|
expect(search_for("Acme-Corp", search_custom_fields: true)).to include("memberx")
|
|
end
|
|
|
|
it "matches a custom field value when the option is set and `enable_names` is disabled" do
|
|
SiteSetting.enable_names = false
|
|
expect(search_for("Acme-Corp", search_custom_fields: true)).to include("memberx")
|
|
end
|
|
|
|
it "does not match custom fields for `_`, `.` or `-` terms without the option" do
|
|
expect(search_for("Acme-Corp")).not_to include("memberx")
|
|
end
|
|
end
|
|
end
|
