From ae648fc4ff414eb33ef17744d4434a0e4a43e606 Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Wed, 18 Feb 2015 13:27:19 +0000 Subject: [PATCH 1/6] Add User#banned? - Redefined User#public_banned? to User#banned? - Add specs for User#banned? - Deprecate User#public_banned? - Replace use of User#public_banned? with User#banned? --- app/models/user.rb | 12 ++++++++++-- app/views/user/show.html.erb | 2 +- spec/models/user_spec.rb | 18 ++++++++++++++++++ 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index c953e52f2a..920c0da46c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -207,7 +207,7 @@ def name if not name.nil? name.strip! end - if public_banned? + if banned? # Use interpolation to return a string rather than a SafeBuffer so that # gsub can be called on it until we upgrade to Rails 3.2. The name returned # is not marked as HTML safe so will be escaped automatically in views. We @@ -294,10 +294,18 @@ def super? def admin_page_links? super? end + # Is it public that they are banned? + def banned? + !ban_text.empty? + end + def public_banned? - !ban_text.empty? + warn %q([DEPRECATION] User#public_banned? will be replaced with + User#banned? as of 0.22).squish + banned? end + # Various ways the user can be banned, and text to describe it if failed def can_file_requests? ban_text.empty? && !exceeded_limit? diff --git a/app/views/user/show.html.erb b/app/views/user/show.html.erb index 7c8d52568f..78b513d6a9 100644 --- a/app/views/user/show.html.erb +++ b/app/views/user/show.html.erb @@ -78,7 +78,7 @@ <% end %>

- <% if @display_user.public_banned? %> + <% if @display_user.banned? %>

diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 7dcd3ab8a3..2245a024f0 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -369,3 +369,21 @@ end + +describe User do + + describe :banned? do + + it 'is banned if the user has ban_text' do + user = FactoryGirl.build(:user, :ban_text => 'banned') + expect(user).to be_banned + end + + it 'is not banned if the user has no ban_text' do + user = FactoryGirl.build(:user, :ban_text => '') + expect(user).to_not be_banned + end + + end + +end From 362a7b967819ca0a58dd251ab77842ab18aa7f64 Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Wed, 18 Feb 2015 14:43:58 +0000 Subject: [PATCH 2/6] Add specs to AboutMeValidator --- app/controllers/user_controller.rb | 6 +++ spec/models/about_me_validator_spec.rb | 53 ++++++++++++++++++++++++++ 2 files changed, 59 insertions(+) create mode 100644 spec/models/about_me_validator_spec.rb diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb index 56f42891d8..32b6978eaa 100644 --- a/app/controllers/user_controller.rb +++ b/app/controllers/user_controller.rb @@ -569,6 +569,12 @@ def set_profile_about_me return end + if @user.banned? + flash[:error] = _('Banned users cannot edit their profile') + redirect_to set_profile_about_me_path + return + end + @about_me = AboutMeValidator.new(params[:about_me]) if !@about_me.valid? render :action => 'set_profile_about_me' diff --git a/spec/models/about_me_validator_spec.rb b/spec/models/about_me_validator_spec.rb new file mode 100644 index 0000000000..5610cead8c --- /dev/null +++ b/spec/models/about_me_validator_spec.rb @@ -0,0 +1,53 @@ +require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') + +describe AboutMeValidator do + + describe :new do + + it 'sets each supported attribute on the instance' do + params = { :about_me => 'My description' } + validator = AboutMeValidator.new(params) + expect(validator.about_me).to eq('My description') + end + + end + + describe :valid? do + + it 'is valid if about_me is =< 500' do + params = { :about_me => 'a'*500 } + validator = AboutMeValidator.new(params) + expect(validator).to be_valid + end + + it 'is valid if about_me is blank' do + params = { :about_me => '' } + validator = AboutMeValidator.new(params) + expect(validator).to be_valid + end + + it 'is valid if about_me is nil' do + params = { :about_me => nil } + validator = AboutMeValidator.new(params) + expect(validator).to be_valid + end + + it 'is invalid if about_me is > 500' do + params = { :about_me => 'a'*501 } + validator = AboutMeValidator.new(params) + expect(validator).to have(1).error_on(:about_me) + end + + end + + describe :about_me do + + it 'has an attribute accessor' do + params = { :about_me => 'My description' } + validator = AboutMeValidator.new(params) + expect(validator.about_me).to eq('My description') + end + + end + +end From 9aa1074dca82aea9b968c5aa67c70dd8a844d969 Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Wed, 18 Feb 2015 13:29:38 +0000 Subject: [PATCH 3/6] Disable about_me text field if user is banned Stops the user editing their about me if they have been banned. https://www.righttoknow.org.au/ reported that spam accounts were being created and even though the user accounts were getting banned, the spam user/bots could still edit the about me field to propagate more spam. --- app/views/user/set_profile_about_me.html.erb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/app/views/user/set_profile_about_me.html.erb b/app/views/user/set_profile_about_me.html.erb index fb7de7e972..42607ddf8d 100644 --- a/app/views/user/set_profile_about_me.html.erb +++ b/app/views/user/set_profile_about_me.html.erb @@ -17,8 +17,12 @@

- - <%= f.text_area :about_me, :rows => 5, :cols => 55 %> + + <% about_me_opts = { :rows => 5, :cols => 55 } %> + <% about_me_opts.merge!({ :disabled => 'disabled' }) if @user.banned? %> + <%= f.text_area :about_me, about_me_opts %>

From 782ea13f061f57563a9671894035bc91baef10ab Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Wed, 18 Feb 2015 15:17:29 +0000 Subject: [PATCH 4/6] Fully prevent banned users editing their about_me --- spec/controllers/user_controller_spec.rb | 29 ++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/spec/controllers/user_controller_spec.rb b/spec/controllers/user_controller_spec.rb index 413d395c56..cde4c9188a 100644 --- a/spec/controllers/user_controller_spec.rb +++ b/spec/controllers/user_controller_spec.rb @@ -1,6 +1,35 @@ # coding: utf-8 require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') +describe UserController do + + describe :set_profile_about_me do + + context 'user is banned' do + + before(:each) do + @user = FactoryGirl.create(:user, :ban_text => 'Causing trouble') + session[:user_id] = @user.id + + post :set_profile_about_me, :submitted_about_me => '1', + :about_me => 'Bad stuff' + end + + it 'redirects to the profile page' do + expect(response).to redirect_to(set_profile_about_me_path) + end + + it 'renders an error message' do + msg = 'Banned users cannot edit their profile' + expect(flash[:error]).to eq(msg) + end + + end + + end + +end + # TODO: Use route_for or params_from to check /c/ links better # http://rspec.rubyforge.org/rspec-rails/1.1.12/classes/Spec/Rails/Example/ControllerExampleGroup.html describe UserController, "when redirecting a show request to a canonical url" do From 8341ff286c42cc09dfd56d3f76521375ee4d0219 Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Wed, 18 Feb 2015 15:25:39 +0000 Subject: [PATCH 5/6] Disable profile_photo file field if user is banned Stops the user editing their profile photo if they have been banned. https://www.righttoknow.org.au/ reported that spam accounts were being created and even though the user accounts were getting banned, the spam user/bots couls still edit the about me field to propagate more spam. This prevents the profile photo being modified so that they cannot add offensive images. --- app/views/user/set_draft_profile_photo.html.erb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/views/user/set_draft_profile_photo.html.erb b/app/views/user/set_draft_profile_photo.html.erb index b4bdd80f36..ba44f54f43 100644 --- a/app/views/user/set_draft_profile_photo.html.erb +++ b/app/views/user/set_draft_profile_photo.html.erb @@ -11,7 +11,9 @@ <%= form_tag 'set_photo', :id => 'set_draft_profile_photo_form', :multipart => true do %>

- <%= file_field_tag :file, :size => 35, :id => 'file_1' %> + <% file_opts = { :size => 35, :id => 'file_1' } %> + <% file_opts.merge!({ :disabled => true }) if @user.banned? %> + <%= file_field_tag :file, file_opts %>

    From d8b9ea8bfe9fdf534504044774f0dcdb4bba20f2 Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Wed, 18 Feb 2015 15:37:11 +0000 Subject: [PATCH 6/6] Fully prevent banned users editing their photo --- app/controllers/user_controller.rb | 6 +++++ spec/controllers/user_controller_spec.rb | 28 ++++++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb index 32b6978eaa..d66b4aa8e0 100644 --- a/app/controllers/user_controller.rb +++ b/app/controllers/user_controller.rb @@ -460,6 +460,12 @@ def set_profile_photo return end if !params[:submitted_draft_profile_photo].nil? + if @user.banned? + flash[:error]= _('Banned users cannot edit their profile') + redirect_to set_profile_photo_path + return + end + # check for uploaded image file_name = nil file_content = nil diff --git a/spec/controllers/user_controller_spec.rb b/spec/controllers/user_controller_spec.rb index cde4c9188a..443856cf3c 100644 --- a/spec/controllers/user_controller_spec.rb +++ b/spec/controllers/user_controller_spec.rb @@ -3,6 +3,34 @@ describe UserController do + describe :set_profile_photo do + + context 'user is banned' do + + before(:each) do + @user = FactoryGirl.create(:user, :ban_text => 'Causing trouble') + session[:user_id] = @user.id + @uploadedfile = fixture_file_upload("/files/parrot.png") + + post :set_profile_photo, :id => @user.id, + :file => @uploadedfile, + :submitted_draft_profile_photo => 1, + :automatically_crop => 1 + end + + it 'redirects to the profile page' do + expect(response).to redirect_to(set_profile_photo_path) + end + + it 'renders an error message' do + msg = 'Banned users cannot edit their profile' + expect(flash[:error]).to eq(msg) + end + + end + + end + describe :set_profile_about_me do context 'user is banned' do