From bb79e6aff7acd95d1dac8fcf367978af2ad81162 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Wed, 28 Oct 2015 19:56:08 +0100 Subject: [PATCH] FEATURE: new hide_user_profiles_from_public site setting --- .../components/hamburger-menu.js.es6 | 7 + .../discourse/controllers/user-card.js.es6 | 5 + .../javascripts/discourse/routes/user.js.es6 | 6 + .../javascripts/discourse/routes/users.js.es6 | 6 + .../templates/components/hamburger-menu.hbs | 2 +- app/controllers/users_controller.rb | 3 +- config/locales/server.en.yml | 2 + config/site_settings.yml | 3 + lib/search.rb | 2 + spec/components/search_spec.rb | 16 ++- spec/controllers/users_controller_spec.rb | 134 ++++++++++-------- 11 files changed, 127 insertions(+), 59 deletions(-) diff --git a/app/assets/javascripts/discourse/components/hamburger-menu.js.es6 b/app/assets/javascripts/discourse/components/hamburger-menu.js.es6 index 910b8d661..e10c01c84 100644 --- a/app/assets/javascripts/discourse/components/hamburger-menu.js.es6 +++ b/app/assets/javascripts/discourse/components/hamburger-menu.js.es6 @@ -56,6 +56,13 @@ export default Ember.Component.extend({ }); }, + @computed() + showUserDirectoryLink() { + if (!this.siteSettings.enable_user_directory) return false; + if (this.siteSettings.hide_user_profiles_from_public && !this.currentUser) return false; + return true; + }, + actions: { keyboardShortcuts() { this.sendAction('showKeyboardAction'); diff --git a/app/assets/javascripts/discourse/controllers/user-card.js.es6 b/app/assets/javascripts/discourse/controllers/user-card.js.es6 index ce2e065a8..c67125cdb 100644 --- a/app/assets/javascripts/discourse/controllers/user-card.js.es6 +++ b/app/assets/javascripts/discourse/controllers/user-card.js.es6 @@ -39,6 +39,11 @@ export default Ember.Controller.extend({ // XSS protection (should be encapsulated) username = username.toString().replace(/[^A-Za-z0-9_\.\-]/g, ""); + // No user card for anon + if (this.siteSettings.hide_user_profiles_from_public && !this.currentUser) { + return; + } + // Don't show on mobile if (Discourse.Mobile.mobileView) { const url = "/users/" + username; diff --git a/app/assets/javascripts/discourse/routes/user.js.es6 b/app/assets/javascripts/discourse/routes/user.js.es6 index 7fb8fffe6..040f949b5 100644 --- a/app/assets/javascripts/discourse/routes/user.js.es6 +++ b/app/assets/javascripts/discourse/routes/user.js.es6 @@ -33,6 +33,12 @@ export default Discourse.Route.extend({ } }, + beforeModel() { + if (this.siteSettings.hide_user_profiles_from_public && !this.currentUser) { + this.replaceWith("discovery"); + } + }, + model(params) { // If we're viewing the currently logged in user, return that object instead const currentUser = this.currentUser; diff --git a/app/assets/javascripts/discourse/routes/users.js.es6 b/app/assets/javascripts/discourse/routes/users.js.es6 index beb25276f..ab3cbe320 100644 --- a/app/assets/javascripts/discourse/routes/users.js.es6 +++ b/app/assets/javascripts/discourse/routes/users.js.es6 @@ -23,6 +23,12 @@ export default Discourse.Route.extend({ } }, + beforeModel() { + if (this.siteSettings.hide_user_profiles_from_public && !this.currentUser) { + this.replaceWith("discovery"); + } + }, + model(params) { // If we refresh via `refreshModel` set the old model to loading this._params = params; diff --git a/app/assets/javascripts/discourse/templates/components/hamburger-menu.hbs b/app/assets/javascripts/discourse/templates/components/hamburger-menu.hbs index 0deea8e1a..3d875eb60 100644 --- a/app/assets/javascripts/discourse/templates/components/hamburger-menu.hbs +++ b/app/assets/javascripts/discourse/templates/components/hamburger-menu.hbs @@ -57,7 +57,7 @@
  • {{d-link route="badges" class="badge-link" label="badges.title"}}
    • {{/if}} - {{#if siteSettings.enable_user_directory}} + {{#if showUserDirectoryLink}}
  • {{d-link route="users" class="user-directory-link" label="directory.title"}}
    • {{/if}} diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 12abf12a9..1c2be6d1b 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -29,6 +29,8 @@ class UsersController < ApplicationController end def show + raise Discourse::InvalidAccess if SiteSetting.hide_user_profiles_from_public && !current_user + @user = fetch_user_from_params user_serializer = UserSerializer.new(@user, scope: guardian, root: 'user') if params[:stats].to_s == "false" @@ -162,7 +164,6 @@ class UsersController < ApplicationController end def my_redirect - raise Discourse::NotFound if params[:path] !~ /^[a-z\-\/]+$/ if current_user.blank? diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index dd4240c33..c5784bd24 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1162,6 +1162,8 @@ en: anonymous_posting_min_trust_level: "Minimum trust level required to enable anonymous posting" anonymous_account_duration_minutes: "To protect anonymity create a new anonymous account every N minutes for each user. Example: if set to 600, as soon as 600 minutes elapse from last post AND user switches to anon, a new anonymous account is created." + hide_user_profiles_from_public: "Disable user cards, user profiles and user directory for anonymous users." + allow_profile_backgrounds: "Allow users to upload profile backgrounds." sequential_replies_threshold: "Number posts a user has to make in a row in a topic before being reminded about too many sequential replies. " diff --git a/config/site_settings.yml b/config/site_settings.yml index 73c801b2c..149713872 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -342,6 +342,9 @@ users: client: true anonymous_account_duration_minutes: default: 10080 + hide_user_profiles_from_public: + default: false + client: true posting: min_post_length: diff --git a/lib/search.rb b/lib/search.rb index e99e60361..4c60c318f 100644 --- a/lib/search.rb +++ b/lib/search.rb @@ -379,6 +379,8 @@ class Search end def user_search + return if SiteSetting.hide_user_profiles_from_public && !@guardian.user + users = User.includes(:user_search_data) .where("active = true AND user_search_data.search_data @@ #{ts_query("simple")}") .order("CASE WHEN username_lower = '#{@original_term.downcase}' THEN 0 ELSE 1 END") diff --git a/spec/components/search_spec.rb b/spec/components/search_spec.rb index 462b11911..093aeb766 100644 --- a/spec/components/search_spec.rb +++ b/spec/components/search_spec.rb @@ -85,6 +85,21 @@ describe Search do expect(result.users.length).to eq(1) expect(result.users[0].id).to eq(user.id) end + + context 'hiding user profiles' do + before { SiteSetting.stubs(:hide_user_profiles_from_public).returns(true) } + + it 'returns no result for anon' do + expect(result.users.length).to eq(0) + end + + it 'returns a result for logged in users' do + result = Search.execute('bruce', type_filter: 'user', guardian: Guardian.new(user)) + expect(result.users.length).to eq(1) + end + + end + end context 'inactive users' do @@ -119,7 +134,6 @@ describe Search do TopicAllowedUser.create!(user_id: reply.user_id, topic_id: topic.id) TopicAllowedUser.create!(user_id: post.user_id, topic_id: topic.id) - results = Search.execute('mars', type_filter: 'private_messages', guardian: Guardian.new(reply.user)) diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index e693da022..fb49b404c 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -3,71 +3,93 @@ require 'spec_helper' describe UsersController do describe '.show' do - let(:user) { log_in } - it 'returns success' do - xhr :get, :show, username: user.username, format: :json - expect(response).to be_success - json = JSON.parse(response.body) + context "anon" do - expect(json["user"]["has_title_badges"]).to eq(false) + let(:user) { Discourse.system_user } - end - - it "returns not found when the username doesn't exist" do - xhr :get, :show, username: 'madeuppity' - expect(response).not_to be_success - end - - it 'returns not found when the user is inactive' do - inactive = Fabricate(:user, active: false) - xhr :get, :show, username: inactive.username - expect(response).not_to be_success - end - - it "raises an error on invalid access" do - Guardian.any_instance.expects(:can_see?).with(user).returns(false) - xhr :get, :show, username: user.username - expect(response).to be_forbidden - end - - describe "user profile views" do - let(:other_user) { Fabricate(:user) } - - it "should track a user profile view for a signed in user" do - UserProfileView.expects(:add).with(other_user.user_profile.id, request.remote_ip, user.id) - xhr :get, :show, username: other_user.username - end - - it "should not track a user profile view for a user viewing his own profile" do - UserProfileView.expects(:add).never - xhr :get, :show, username: user.username - end - - it "should track a user profile view for an anon user" do - UserProfileView.expects(:add).with(other_user.user_profile.id, request.remote_ip, nil) - xhr :get, :show, username: other_user.username - end - - it "skips tracking" do - UserProfileView.expects(:add).never - xhr :get, :show, { username: user.username, skip_track_visit: true } - end - end - - context "fetching a user by external_id" do - before { user.create_single_sign_on_record(external_id: '997', last_payload: '') } - - it "returns fetch for a matching external_id" do - xhr :get, :show, external_id: '997' + it "returns success" do + xhr :get, :show, username: user.username, format: :json expect(response).to be_success end - it "returns not found when external_id doesn't match" do - xhr :get, :show, external_id: '99' + it "raises an error for anon when profiles are hidden" do + SiteSetting.stubs(:hide_user_profiles_from_public).returns(true) + xhr :get, :show, username: user.username, format: :json expect(response).not_to be_success end + end + + context "logged in" do + + let(:user) { log_in } + + it 'returns success' do + xhr :get, :show, username: user.username, format: :json + expect(response).to be_success + json = JSON.parse(response.body) + + expect(json["user"]["has_title_badges"]).to eq(false) + end + + it "returns not found when the username doesn't exist" do + xhr :get, :show, username: 'madeuppity' + expect(response).not_to be_success + end + + it 'returns not found when the user is inactive' do + inactive = Fabricate(:user, active: false) + xhr :get, :show, username: inactive.username + expect(response).not_to be_success + end + + it "raises an error on invalid access" do + Guardian.any_instance.expects(:can_see?).with(user).returns(false) + xhr :get, :show, username: user.username + expect(response).to be_forbidden + end + + describe "user profile views" do + let(:other_user) { Fabricate(:user) } + + it "should track a user profile view for a signed in user" do + UserProfileView.expects(:add).with(other_user.user_profile.id, request.remote_ip, user.id) + xhr :get, :show, username: other_user.username + end + + it "should not track a user profile view for a user viewing his own profile" do + UserProfileView.expects(:add).never + xhr :get, :show, username: user.username + end + + it "should track a user profile view for an anon user" do + UserProfileView.expects(:add).with(other_user.user_profile.id, request.remote_ip, nil) + xhr :get, :show, username: other_user.username + end + + it "skips tracking" do + UserProfileView.expects(:add).never + xhr :get, :show, { username: user.username, skip_track_visit: true } + end + end + + context "fetching a user by external_id" do + before { user.create_single_sign_on_record(external_id: '997', last_payload: '') } + + it "returns fetch for a matching external_id" do + xhr :get, :show, external_id: '997' + expect(response).to be_success + end + + it "returns not found when external_id doesn't match" do + xhr :get, :show, external_id: '99' + expect(response).not_to be_success + end + end + + end + end describe '.user_preferences_redirect' do