From 4382fb5facb035f5b414c6c7257dc828327a57c7 Mon Sep 17 00:00:00 2001
From: David Taylor <david@taylorhq.com>
Date: Tue, 4 Sep 2018 11:45:36 +0100
Subject: [PATCH] DEV: Allow plugins to whitelist specific user custom_fields
 for editing (#6358)

---
 app/controllers/users_controller.rb    |  3 ++-
 app/models/user.rb                     | 18 +++++++++++++
 lib/plugin/instance.rb                 |  6 +++++
 spec/requests/users_controller_spec.rb | 37 +++++++++++++++++++++++---
 4 files changed, 60 insertions(+), 4 deletions(-)

diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index a5bf263b949..065f89732a8 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -104,7 +104,7 @@ class UsersController < ApplicationController
     attributes.delete(:username)
 
     if params[:user_fields].present?
-      attributes[:custom_fields] = {}
+      attributes[:custom_fields] ||= {}
 
       fields = UserField.all
       fields = fields.where(editable: true) unless current_user.staff?
@@ -1167,6 +1167,7 @@ class UsersController < ApplicationController
       :card_background
     ]
 
+    permitted << { custom_fields: User.editable_user_custom_fields } unless User.editable_user_custom_fields.blank?
     permitted.concat UserUpdater::OPTION_ATTR
     permitted.concat UserUpdater::CATEGORY_IDS.keys.map { |k| { k => [] } }
     permitted.concat UserUpdater::TAG_NAMES.keys
diff --git a/app/models/user.rb b/app/models/user.rb
index 9fe43dc6fc5..82c97f9c638 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -222,6 +222,24 @@ class User < ActiveRecord::Base
     end
   end
 
+  def self.plugin_editable_user_custom_fields
+    @plugin_editable_user_custom_fields ||= {}
+  end
+
+  def self.register_plugin_editable_user_custom_field(custom_field_name, plugin)
+    plugin_editable_user_custom_fields[custom_field_name] = plugin
+  end
+
+  def self.editable_user_custom_fields
+    fields = []
+
+    plugin_editable_user_custom_fields.each do |k, v|
+      fields << k if v.enabled?
+    end
+
+    fields.uniq
+  end
+
   def self.plugin_staff_user_custom_fields
     @plugin_staff_user_custom_fields ||= {}
   end
diff --git a/lib/plugin/instance.rb b/lib/plugin/instance.rb
index b47539fc532..21c4b82d88c 100644
--- a/lib/plugin/instance.rb
+++ b/lib/plugin/instance.rb
@@ -121,6 +121,12 @@ class Plugin::Instance
     end
   end
 
+  def register_editable_user_custom_field(field)
+    reloadable_patch do |plugin|
+      ::User.register_plugin_editable_user_custom_field(field, plugin) # plugin.enabled? is checked at runtime
+    end
+  end
+
   def custom_avatar_column(column)
     reloadable_patch do |plugin|
       AvatarLookup.lookup_columns << column
diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb
index 7960bfd74e6..9956eae29b8 100644
--- a/spec/requests/users_controller_spec.rb
+++ b/spec/requests/users_controller_spec.rb
@@ -1517,12 +1517,43 @@ describe UsersController do
           end
 
           context "custom_field" do
-            it "does not update the custom field" do
-              put "/u/#{user.username}.json", params: { custom_fields: { test: :it } }
+            before do
+              plugin = Plugin::Instance.new
+              plugin.register_editable_user_custom_field :test2
+            end
+
+            after do
+              User.plugin_editable_user_custom_fields.clear
+            end
+
+            it "only updates allowed user fields" do
+              put "/u/#{user.username}.json", params: { custom_fields: { test1: :hello1, test2: :hello2 } }
 
               expect(response.status).to eq(200)
-              expect(user.custom_fields["test"]).to be_blank
+              expect(user.custom_fields["test1"]).to be_blank
+              expect(user.custom_fields["test2"]).to eq("hello2")
             end
+
+            it "works alongside a user field" do
+              user_field = Fabricate(:user_field, editable: true)
+              put "/u/#{user.username}.json", params: { custom_fields: { test1: :hello1, test2: :hello2 }, user_fields: { user_field.id.to_s => 'happy' } }
+              expect(response.status).to eq(200)
+              expect(user.custom_fields["test1"]).to be_blank
+              expect(user.custom_fields["test2"]).to eq("hello2")
+              expect(user.user_fields[user_field.id.to_s]).to eq('happy')
+            end
+
+            it "is secure when there are no registered editable fields" do
+              User.plugin_editable_user_custom_fields.clear
+              put "/u/#{user.username}.json", params: { custom_fields: { test1: :hello1, test2: :hello2 } }
+              expect(response.status).to eq(200)
+              expect(user.custom_fields["test1"]).to be_blank
+              expect(user.custom_fields["test2"]).to be_blank
+
+              put "/u/#{user.username}.json", params: { custom_fields: ["arrayitem1", "arrayitem2"] }
+              expect(response.status).to eq(200)
+            end
+
           end
         end