From b40313559b09baf5f1a14b643f235d375a681ab6 Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Fri, 28 Feb 2014 16:30:45 -0500 Subject: [PATCH] FIX: moderators should not be able to see site setting changes in the staff action logs. Fixes #2027 --- .../admin/staff_action_logs_controller.rb | 2 +- app/models/user_history.rb | 18 ++++++++++++++++ ...205743_add_admin_only_to_user_histories.rb | 10 +++++++++ spec/models/user_history_spec.rb | 21 ++++++++++++++++++- 4 files changed, 49 insertions(+), 2 deletions(-) create mode 100644 db/migrate/20140228205743_add_admin_only_to_user_histories.rb diff --git a/app/controllers/admin/staff_action_logs_controller.rb b/app/controllers/admin/staff_action_logs_controller.rb index 4c84be309..e60168a52 100644 --- a/app/controllers/admin/staff_action_logs_controller.rb +++ b/app/controllers/admin/staff_action_logs_controller.rb @@ -1,7 +1,7 @@ class Admin::StaffActionLogsController < Admin::AdminController def index - staff_action_logs = UserHistory.with_filters(params.slice(:action_name, :acting_user, :target_user, :subject)).only_staff_actions.limit(200).order('id DESC').includes(:acting_user, :target_user).to_a + staff_action_logs = UserHistory.staff_action_records(current_user, params.slice(:action_name, :acting_user, :target_user, :subject)).to_a render_serialized(staff_action_logs, UserHistorySerializer) end diff --git a/app/models/user_history.rb b/app/models/user_history.rb index 3ae417b26..1188e62db 100644 --- a/app/models/user_history.rb +++ b/app/models/user_history.rb @@ -9,6 +9,8 @@ class UserHistory < ActiveRecord::Base scope :only_staff_actions, ->{ where("action IN (?)", UserHistory.staff_action_ids) } + before_save :set_admin_only + def self.actions @actions ||= Enum.new( :delete_user, :change_trust_level, @@ -38,6 +40,10 @@ class UserHistory < ActiveRecord::Base @staff_action_ids ||= staff_actions.map { |a| actions[a] } end + def self.admin_only_action_ids + @admin_only_action_ids ||= [actions[:change_site_setting]] + end + def self.with_filters(filters) query = self if filters[:action_name] and action_id = UserHistory.actions[filters[:action_name].to_sym] @@ -63,6 +69,18 @@ class UserHistory < ActiveRecord::Base result.exists? end + def self.staff_action_records(viewer, opts={}) + query = self.with_filters(opts.slice(:action_name, :acting_user, :target_user, :subject)).only_staff_actions.limit(200).order('id DESC').includes(:acting_user, :target_user) + query = query.where(admin_only: false) unless viewer && viewer.admin? + query + end + + + def set_admin_only + self.admin_only = UserHistory.admin_only_action_ids.include?(self.action) + self + end + def new_value_is_json? [UserHistory.actions[:change_site_customization], UserHistory.actions[:delete_site_customization]].include?(action) end diff --git a/db/migrate/20140228205743_add_admin_only_to_user_histories.rb b/db/migrate/20140228205743_add_admin_only_to_user_histories.rb new file mode 100644 index 000000000..7e55ab6a0 --- /dev/null +++ b/db/migrate/20140228205743_add_admin_only_to_user_histories.rb @@ -0,0 +1,10 @@ +class AddAdminOnlyToUserHistories < ActiveRecord::Migration + def up + add_column :user_histories, :admin_only, :boolean, default: false + execute "UPDATE user_histories SET admin_only = true WHERE action = #{UserHistory.actions[:change_site_setting]}" + end + + def down + remove_column :user_histories, :admin_only + end +end diff --git a/spec/models/user_history_spec.rb b/spec/models/user_history_spec.rb index 0a05ab412..67d137124 100644 --- a/spec/models/user_history_spec.rb +++ b/spec/models/user_history_spec.rb @@ -1,5 +1,24 @@ require 'spec_helper' describe UserHistory do - # Nothing fancy going on in this model. See StaffActionLogger. + + describe '#staff_action_records' do + context "with some records" do + before do + @change_site_setting = UserHistory.create!({action: UserHistory.actions[:change_site_setting], subject: "title", previous_value: "Old", new_value: "New"}) + @change_trust_level = UserHistory.create!({action: UserHistory.actions[:change_trust_level], target_user_id: Fabricate(:user).id, details: "stuff happened"}) + end + + it "returns all records for admins" do + records = described_class.staff_action_records(Fabricate(:admin)).to_a + records.size.should == 2 + end + + it "doesn't return records to moderators that only admins should see" do + records = described_class.staff_action_records(Fabricate(:moderator)).to_a + records.should == [@change_trust_level] + end + end + end + end