From 976aca68f66e1ed33d08c7a50ea919e25edcf521 Mon Sep 17 00:00:00 2001 From: Osama Sayegh Date: Mon, 9 Dec 2024 13:07:59 +0300 Subject: [PATCH] FEATURE: Restrict profile visibility of low-trust users (#29981) We've seen in some communities abuse of user profile where bios and other fields are used in malicious ways, such as malware distribution. A common pattern between all the abuse cases we've seen is that the malicious actors tend to have 0 posts and have a low trust level. To eliminate this abuse vector, or at least make it much less effective, we're making the following changes to user profiles: 1. Anonymous, TL0 and TL1 users cannot see any user profiles for users with 0 posts except for staff users 2. Anonymous and TL0 users can only see profiles of TL1 users and above Users can always see their own profile, and they can still hide their profiles via the "Hide my public profile" preference. Staff can always see any user's profile. Internal topic: t/142853. --- lib/guardian/user_guardian.rb | 17 +- plugins/chat/spec/system/user_profile_spec.rb | 5 +- spec/lib/guardian/user_guardian_spec.rb | 197 +++++++++++++++--- spec/requests/list_controller_spec.rb | 5 +- spec/requests/posts_controller_spec.rb | 2 + spec/requests/user_actions_controller_spec.rb | 5 +- spec/requests/user_badges_controller_spec.rb | 2 + spec/requests/users_controller_spec.rb | 10 +- spec/serializers/user_serializer_spec.rb | 2 + 9 files changed, 204 insertions(+), 41 deletions(-) diff --git a/lib/guardian/user_guardian.rb b/lib/guardian/user_guardian.rb index 755f1cfbbd9..9c25c04367c 100644 --- a/lib/guardian/user_guardian.rb +++ b/lib/guardian/user_guardian.rb @@ -128,12 +128,21 @@ module UserGuardian def can_see_profile?(user) return false if user.blank? - return true if !SiteSetting.allow_users_to_hide_profile? + return true if is_me?(user) || is_staff? - # If a user has hidden their profile, restrict it to them and staff - return is_me?(user) || is_staff? if user.user_option.try(:hide_profile?) + profile_hidden = SiteSetting.allow_users_to_hide_profile && user.user_option&.hide_profile? - true + return true if user.staff? && !profile_hidden + + if user.user_stat.blank? || user.user_stat.post_count == 0 + return false if anonymous? || !@user.has_trust_level?(TrustLevel[2]) + end + + if anonymous? || !@user.has_trust_level?(TrustLevel[1]) + return user.has_trust_level?(TrustLevel[1]) && !profile_hidden + end + + !profile_hidden end def can_see_user_actions?(user, action_types) diff --git a/plugins/chat/spec/system/user_profile_spec.rb b/plugins/chat/spec/system/user_profile_spec.rb index d3d5083e946..0ae52c36af7 100644 --- a/plugins/chat/spec/system/user_profile_spec.rb +++ b/plugins/chat/spec/system/user_profile_spec.rb @@ -4,7 +4,10 @@ RSpec.describe "User profile", type: :system do fab!(:current_user) { Fabricate(:user) } fab!(:user) - before { chat_system_bootstrap } + before do + user.user_stat.update!(post_count: 1) + chat_system_bootstrap + end shared_examples "not showing chat button" do it "has no chat button" do diff --git a/spec/lib/guardian/user_guardian_spec.rb b/spec/lib/guardian/user_guardian_spec.rb index cdb3e0550bb..f5eb487b17d 100644 --- a/spec/lib/guardian/user_guardian_spec.rb +++ b/spec/lib/guardian/user_guardian_spec.rb @@ -98,44 +98,175 @@ RSpec.describe UserGuardian do end describe "#can_see_profile?" do + fab!(:tl0_user) { Fabricate(:user, trust_level: 0) } + fab!(:tl1_user) { Fabricate(:user, trust_level: 1) } + fab!(:tl2_user) { Fabricate(:user, trust_level: 2) } + + before { tl2_user.user_stat.update!(post_count: 1) } + + context "when viewing the profile of a user with 0 posts" do + before { user.user_stat.update!(post_count: 0) } + + it "they can view their own profile" do + expect(Guardian.new(user).can_see_profile?(user)).to eq(true) + end + + it "an anonymous user cannot view the user's profile" do + expect(Guardian.new.can_see_profile?(user)).to eq(false) + end + + it "a TL0 user cannot view the user's profile" do + expect(Guardian.new(tl0_user).can_see_profile?(user)).to eq(false) + end + + it "a TL1 user cannot view the user's profile" do + expect(Guardian.new(tl1_user).can_see_profile?(user)).to eq(false) + end + + it "a TL2 user can view the user's profile" do + expect(Guardian.new(tl2_user).can_see_profile?(user)).to eq(true) + end + + it "a moderator can view the user's profile" do + expect(Guardian.new(moderator).can_see_profile?(user)).to eq(true) + end + + it "an admin can view the user's profile" do + expect(Guardian.new(admin).can_see_profile?(user)).to eq(true) + end + + context "when the profile is hidden" do + before do + SiteSetting.allow_users_to_hide_profile = true + user.user_option.update!(hide_profile: true) + end + + it "they can view their own profile" do + expect(Guardian.new(user).can_see_profile?(user)).to eq(true) + end + + it "a TL2 user cannot view the user's profile" do + expect(Guardian.new(tl2_user).can_see_profile?(user)).to eq(false) + end + + it "a moderator can view the user's profile" do + expect(Guardian.new(moderator).can_see_profile?(user)).to eq(true) + end + + it "an admin can view the user's profile" do + expect(Guardian.new(admin).can_see_profile?(user)).to eq(true) + end + end + end + + context "when viewing the profile of a TL0 user with more than 0 posts" do + before { tl0_user.user_stat.update!(post_count: 1) } + + it "they can view their own profile" do + expect(Guardian.new(tl0_user).can_see_profile?(tl0_user)).to eq(true) + end + + it "an anonymous user cannot view the user's profile" do + expect(Guardian.new.can_see_profile?(tl0_user)).to eq(false) + end + + it "a TL0 user cannot view the user's profile" do + expect(Guardian.new(Fabricate(:user, trust_level: 0)).can_see_profile?(tl0_user)).to eq( + false, + ) + end + + it "a TL1 user can view the user's profile" do + expect(Guardian.new(tl1_user).can_see_profile?(tl0_user)).to eq(true) + end + + it "a TL2 user can view the user's profile" do + expect(Guardian.new(tl2_user).can_see_profile?(tl0_user)).to eq(true) + end + + it "a moderator user can view the user's profile" do + expect(Guardian.new(moderator).can_see_profile?(tl0_user)).to eq(true) + end + + it "an admin user can view the user's profile" do + expect(Guardian.new(admin).can_see_profile?(tl0_user)).to eq(true) + end + + context "when the profile is hidden" do + before do + SiteSetting.allow_users_to_hide_profile = true + tl0_user.user_option.update!(hide_profile: true) + end + + it "they can view their own profile" do + expect(Guardian.new(tl0_user).can_see_profile?(tl0_user)).to eq(true) + end + + it "a TL1 user cannot view the user's profile" do + expect(Guardian.new(tl1_user).can_see_profile?(tl0_user)).to eq(false) + end + + it "a TL2 user cannot view the user's profile" do + expect(Guardian.new(tl2_user).can_see_profile?(tl0_user)).to eq(false) + end + + it "a moderator user can view the user's profile" do + expect(Guardian.new(moderator).can_see_profile?(tl0_user)).to eq(true) + end + + it "an admin user can view the user's profile" do + expect(Guardian.new(admin).can_see_profile?(tl0_user)).to eq(true) + end + end + end + + context "when the allow_users_to_hide_profile setting is false" do + before { SiteSetting.allow_users_to_hide_profile = false } + + it "doesn't hide the profile even if the hide_profile user option is true" do + tl2_user.user_option.update!(hide_profile: true) + + expect(Guardian.new(tl0_user).can_see_profile?(tl2_user)).to eq(true) + expect(Guardian.new(tl1_user).can_see_profile?(tl2_user)).to eq(true) + expect(Guardian.new(admin).can_see_profile?(tl2_user)).to eq(true) + expect(Guardian.new(moderator).can_see_profile?(tl2_user)).to eq(true) + end + end + + context "when the allow_users_to_hide_profile setting is true" do + before { SiteSetting.allow_users_to_hide_profile = true } + + it "doesn't allow non-staff users to view the user's profile if the hide_profile user option is true" do + tl2_user.user_option.update!(hide_profile: true) + + expect(Guardian.new(tl0_user).can_see_profile?(tl2_user)).to eq(false) + expect(Guardian.new(tl1_user).can_see_profile?(tl2_user)).to eq(false) + + expect(Guardian.new(admin).can_see_profile?(tl2_user)).to eq(true) + expect(Guardian.new(moderator).can_see_profile?(tl2_user)).to eq(true) + end + + it "allows everyone to view the user's profile if the hide_profile user option is false" do + tl2_user.user_option.update!(hide_profile: false) + + expect(Guardian.new(tl0_user).can_see_profile?(tl2_user)).to eq(true) + expect(Guardian.new(tl1_user).can_see_profile?(tl2_user)).to eq(true) + + expect(Guardian.new(admin).can_see_profile?(tl2_user)).to eq(true) + expect(Guardian.new(moderator).can_see_profile?(tl2_user)).to eq(true) + end + end + it "is false for no user" do expect(Guardian.new.can_see_profile?(nil)).to eq(false) end - it "is true for a user whose profile is public" do - expect(Guardian.new.can_see_profile?(user)).to eq(true) - end + it "is true for staff users even when they have no posts" do + admin.user_stat.update!(post_count: 0) + moderator.user_stat.update!(post_count: 0) - context "with hidden profile" do - # Mixing Fabricate.build() and Fabricate() could cause ID clashes, so override :user - fab!(:user) - - let(:hidden_user) do - result = Fabricate(:user) - result.user_option.update_column(:hide_profile, true) - result - end - - it "is false for another user" do - expect(Guardian.new(user).can_see_profile?(hidden_user)).to eq(false) - end - - it "is false for an anonymous user" do - expect(Guardian.new.can_see_profile?(hidden_user)).to eq(false) - end - - it "is true for the user themselves" do - expect(Guardian.new(hidden_user).can_see_profile?(hidden_user)).to eq(true) - end - - it "is true for a staff user" do - expect(Guardian.new(admin).can_see_profile?(hidden_user)).to eq(true) - end - - it "is true if hiding profiles is disabled" do - SiteSetting.allow_users_to_hide_profile = false - expect(Guardian.new(user).can_see_profile?(hidden_user)).to eq(true) - end + expect(Guardian.new.can_see_profile?(admin)).to eq(true) + expect(Guardian.new.can_see_profile?(moderator)).to eq(true) end end diff --git a/spec/requests/list_controller_spec.rb b/spec/requests/list_controller_spec.rb index 3aa6b61c30e..93c0b69c3a6 100644 --- a/spec/requests/list_controller_spec.rb +++ b/spec/requests/list_controller_spec.rb @@ -905,7 +905,10 @@ RSpec.describe ListController do fab!(:topic2) { Fabricate(:topic, user: user) } fab!(:user2) { Fabricate(:user) } - before { sign_in(user2) } + before do + user.user_stat.update!(post_count: 1) + sign_in(user2) + end it "should respond with a list" do get "/topics/created-by/#{user.username}.json" diff --git a/spec/requests/posts_controller_spec.rb b/spec/requests/posts_controller_spec.rb index b885e3920f6..439b3a1a39c 100644 --- a/spec/requests/posts_controller_spec.rb +++ b/spec/requests/posts_controller_spec.rb @@ -2578,6 +2578,8 @@ RSpec.describe PostsController do end describe "#user_posts_feed" do + before { user.user_stat.update!(post_count: 1) } + it "returns public posts rss feed" do public_post private_post diff --git a/spec/requests/user_actions_controller_spec.rb b/spec/requests/user_actions_controller_spec.rb index 6723995f8e7..abb0f5cec77 100644 --- a/spec/requests/user_actions_controller_spec.rb +++ b/spec/requests/user_actions_controller_spec.rb @@ -19,7 +19,10 @@ RSpec.describe UserActionsController do let(:actions) { response.parsed_body["user_actions"] } let(:post) { create_post } - before { UserActionManager.enable } + before do + UserActionManager.enable + post.user.user_stat.update!(post_count: 1) + end it "renders list correctly" do user_actions diff --git a/spec/requests/user_badges_controller_spec.rb b/spec/requests/user_badges_controller_spec.rb index a4224239234..8f5a9dea442 100644 --- a/spec/requests/user_badges_controller_spec.rb +++ b/spec/requests/user_badges_controller_spec.rb @@ -5,6 +5,8 @@ RSpec.describe UserBadgesController do fab!(:admin) fab!(:badge) + before { user.user_stat.update!(post_count: 1) } + describe "#index" do fab!(:badge) { Fabricate(:badge, target_posts: true, show_posts: false) } diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 9a6350198ae..8f2aac569b4 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -4252,6 +4252,12 @@ RSpec.describe UsersController do end describe "#summary" do + before do + user.user_stat.update!(post_count: 1) + user1.user_stat.update!(post_count: 1) + user_deferred.user_stat.update!(post_count: 1) + end + it "generates summary info" do create_post(user: user) @@ -4261,7 +4267,7 @@ RSpec.describe UsersController do json = response.parsed_body expect(json["user_summary"]["topic_count"]).to eq(1) - expect(json["user_summary"]["post_count"]).to eq(0) + expect(json["user_summary"]["post_count"]).to eq(1) end context "when `hide_user_profiles_from_public` site setting is enabled" do @@ -4938,6 +4944,8 @@ RSpec.describe UsersController do fab!(:user) { Discourse.system_user } fab!(:user2) { Fabricate(:user) } + before { user2.user_stat.update!(post_count: 1) } + it "returns success" do get "/user-cards.json?user_ids=#{user.id},#{user2.id}" expect(response.status).to eq(200) diff --git a/spec/serializers/user_serializer_spec.rb b/spec/serializers/user_serializer_spec.rb index 9260e9396e1..e96d1ade673 100644 --- a/spec/serializers/user_serializer_spec.rb +++ b/spec/serializers/user_serializer_spec.rb @@ -3,6 +3,8 @@ RSpec.describe UserSerializer do fab!(:user) { Fabricate(:user, trust_level: 0) } + before { user.user_stat.update!(post_count: 1) } + context "with a TL0 user seen as anonymous" do let(:serializer) { UserSerializer.new(user, scope: Guardian.new, root: false) } let(:json) { serializer.as_json }