FIX: moderators should not be able to see site setting changes in the staff action logs. Fixes #2027

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
 

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

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

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