FIX: Validate value of custom dropdown user fields - dropdowns and multiple selects (#13890)

app/controllers/users_controller.rb

@@ -143,16 +143,16 @@ class UsersController < ApplicationController
 
       fields = UserField.all
       fields = fields.where(editable: true) unless current_user.staff?
-      fields.each do |f|
+      fields.each do |field|
-        field_id = f.id.to_s
+        field_id = field.id.to_s
         next unless params[:user_fields].has_key?(field_id)
 
-        val = params[:user_fields][field_id]
+        value = clean_custom_field_values(field)
-        val = nil if val === "false"
+        value = nil if value === "false"
-        val = val[0...UserField.max_length] if val
+        value = value[0...UserField.max_length] if value
 
-        return render_json_error(I18n.t("login.missing_user_field")) if val.blank? && f.required?
+        return render_json_error(I18n.t("login.missing_user_field")) if value.blank? && field.required?
-        attributes[:custom_fields]["#{User::USER_FIELD_PREFIX}#{f.id}"] = val
+        attributes[:custom_fields]["#{User::USER_FIELD_PREFIX}#{field.id}"] = value
       end
     end
 
@@ -1581,6 +1581,21 @@ class UsersController < ApplicationController
 
   private
 
+  def clean_custom_field_values(field)
+    field_values = params[:user_fields][field.id.to_s]
+
+    return field_values if field_values.nil? || field_values.empty?
+
+    if field.field_type == "dropdown"
+      field.user_field_options.find_by_value(field_values)&.value
+    elsif field.field_type == "multiselect"
+      bad_values = field_values - field.user_field_options.map(&:value)
+      field_values - bad_values
+    else
+      field_values
+    end
+  end
+
   def password_reset_find_user(token, committing_change:)
     if EmailToken.valid_token_format?(token)
       @user = committing_change ? EmailToken.confirm(token) : EmailToken.confirmable(token)&.user

app/models/user_field_option.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 class UserFieldOption < ActiveRecord::Base
+  belongs_to :user_field
 end
 
 # == Schema Information

spec/fabricators/user_field_option_fabricator.rb.rb

@@ -0,0 +1,6 @@
+# frozen_string_literal: true
+
+Fabricator(:user_field_option) do
+  user_field
+  value { sequence(:name) { |i| "field_option_#{i}" } }
+end

spec/requests/users_controller_spec.rb

@@ -1260,6 +1260,120 @@ describe UsersController do
       end
 
       context "with values for the fields" do
+        let(:update_user_url) { "/u/#{user.username}.json" }
+        let(:field_id) { user_field.id.to_s }
+
+        before { sign_in(user) }
+
+        context "with multple select fields" do
+          let(:valid_options) { %w[Axe Sword] }
+
+          fab!(:user_field) do
+            Fabricate(:user_field, field_type: 'multiselect') do
+              user_field_options do
+                [
+                  Fabricate(:user_field_option, value: 'Axe'),
+                  Fabricate(:user_field_option, value: 'Sword')
+                ]
+              end
+            end
+          end
+
+          it "shouldn't allow unregistered field values" do
+            expect do
+              put update_user_url, params: { user_fields: { field_id => %w[Juice] } }
+            end.not_to change { user.reload.user_fields[field_id] }
+          end
+
+          it "should filter valid values" do
+            expect do
+              put update_user_url, params: { user_fields: { field_id => %w[Axe Juice Sword] } }
+            end.to change { user.reload.user_fields[field_id] }.from(nil).to(valid_options)
+          end
+
+          it "allows registered field values" do
+            expect do
+              put update_user_url, params: { user_fields: { field_id => valid_options } }
+            end.to change { user.reload.user_fields[field_id] }.from(nil).to(valid_options)
+          end
+
+          it "value can't be nil or empty if the field is required" do
+            put update_user_url, params: { user_fields: { field_id => valid_options } }
+
+            user_field.update!(required: true)
+
+            expect do
+              put update_user_url, params: { user_fields: { field_id => nil } }
+            end.not_to change { user.reload.user_fields[field_id] }
+
+            expect do
+              put update_user_url, params: { user_fields: { field_id => "" } }
+            end.not_to change { user.reload.user_fields[field_id] }
+          end
+
+          it 'value can nil or empty if the field is not required' do
+            put update_user_url, params: { user_fields: { field_id => valid_options } }
+
+            user_field.update!(required: false)
+
+            expect do
+              put update_user_url, params: { user_fields: { field_id => nil } }
+            end.to change { user.reload.user_fields[field_id] }.from(valid_options).to(nil)
+
+            expect do
+              put update_user_url, params: { user_fields: { field_id => "" } }
+            end.to change { user.reload.user_fields[field_id] }.from(nil).to("")
+          end
+
+        end
+
+        context "with dropdown fields" do
+          let(:valid_options) { ['Black Mesa', 'Fox Hound'] }
+
+          fab!(:user_field) do
+            Fabricate(:user_field, field_type: 'dropdown') do
+              user_field_options do
+                [
+                  Fabricate(:user_field_option, value: 'Black Mesa'),
+                  Fabricate(:user_field_option, value: 'Fox Hound')
+                ]
+              end
+            end
+          end
+
+          it "shouldn't allow unregistered field values" do
+            expect do
+              put update_user_url, params: { user_fields: { field_id => 'Umbrella Corporation' } }
+            end.not_to change { user.reload.user_fields[field_id] }
+          end
+
+          it "allows registered field values" do
+            expect do
+              put update_user_url, params: { user_fields: { field_id => valid_options.first } }
+            end.to change { user.reload.user_fields[field_id] }.from(nil).to(valid_options.first)
+          end
+
+          it "value can't be nil if the field is required" do
+            put update_user_url, params: { user_fields: { field_id => valid_options.first } }
+
+            user_field.update!(required: true)
+
+            expect do
+              put update_user_url, params: { user_fields: { field_id => nil } }
+            end.not_to change { user.reload.user_fields[field_id] }
+          end
+
+          it 'value can be set to nil if the field is not required' do
+            put update_user_url, params: { user_fields: { field_id => valid_options.last } }
+
+            user_field.update!(required: false)
+
+            expect do
+              put update_user_url, params: { user_fields: { field_id => nil } }
+            end.to change { user.reload.user_fields[field_id] }.from(valid_options.last).to(nil)
+          end
+        end
+
         let(:create_params) { {
           name: @user.name,
           password: 'suChS3cuRi7y',