FIX: Do not serialize user fields unless they are specified for display (#6736)

FIX: Do not serialize user fields unless they are specified for display (#6736)
From 5e09398c5ba1a1cdcc6c10ced4bc92030b7fc48d Mon Sep 17 00:00:00 2001
From: David Taylor <david@taylorhq.com>
Date: Fri, 7 Dec 2018 10:57:28 +0000
Subject: [PATCH] FIX: Do not serialize user fields unless they are specified
 for display (#6736)


diff --git a/app/serializers/user_serializer.rb b/app/serializers/user_serializer.rb
index cc39293..325f4dd 100644
--- a/app/serializers/user_serializer.rb
+++ b/app/serializers/user_serializer.rb
@@ -420,7 +420,8 @@ class UserSerializer < BasicUserSerializer
   end
 
   def user_fields
-    object.user_fields
+    allowed_keys = scope.allowed_user_field_ids(object).map(&:to_s)
+    object.user_fields&.select { |k, v| allowed_keys.include?(k) }
   end
 
   def include_user_fields?
diff --git a/lib/guardian/user_guardian.rb b/lib/guardian/user_guardian.rb
index aa50295..e6393e1 100644
--- a/lib/guardian/user_guardian.rb
+++ b/lib/guardian/user_guardian.rb
@@ -104,4 +104,15 @@ module UserGuardian
     true
   end
 
+  def allowed_user_field_ids(user)
+    @allowed_user_field_ids ||= {}
+    @allowed_user_field_ids[user.id] ||=
+      begin
+        if is_staff? || is_me?(user)
+          UserField.pluck(:id)
+        else
+          UserField.where("show_on_profile OR show_on_user_card").pluck(:id)
+        end
+      end
+  end
 end
diff --git a/spec/components/guardian/user_guardian_spec.rb b/spec/components/guardian/user_guardian_spec.rb
index e37245c..e599760 100644
--- a/spec/components/guardian/user_guardian_spec.rb
+++ b/spec/components/guardian/user_guardian_spec.rb
@@ -135,4 +135,41 @@ describe UserGuardian do
 
     end
   end
+
+  describe "#allowed_user_field_ids" do
+    let! :fields do
+      [
+        Fabricate(:user_field),
+        Fabricate(:user_field),
+        Fabricate(:user_field, show_on_profile: true),
+        Fabricate(:user_field, show_on_user_card: true),
+        Fabricate(:user_field, show_on_user_card: true, show_on_profile: true)
+      ]
+    end
+
+    let :user2 do
+      Fabricate.build(:user, id: 4)
+    end
+
+    it "returns all fields for staff" do
+      guardian = Guardian.new(admin)
+      expect(guardian.allowed_user_field_ids(user)).to contain_exactly(*fields.map(&:id))
+    end
+
+    it "returns all fields for self" do
+      guardian = Guardian.new(user)
+      expect(guardian.allowed_user_field_ids(user)).to contain_exactly(*fields.map(&:id))
+    end
+
+    it "returns only public fields for others" do
+      guardian = Guardian.new(user)
+      expect(guardian.allowed_user_field_ids(user2)).to contain_exactly(*fields[2..5].map(&:id))
+    end
+
+    it "has a different cache per user" do
+      guardian = Guardian.new(user)
+      expect(guardian.allowed_user_field_ids(user2)).to contain_exactly(*fields[2..5].map(&:id))
+      expect(guardian.allowed_user_field_ids(user)).to contain_exactly(*fields.map(&:id))
+    end
+  end
 end
diff --git a/spec/serializers/user_serializer_spec.rb b/spec/serializers/user_serializer_spec.rb
index b977a3a..505294d 100644
--- a/spec/serializers/user_serializer_spec.rb
+++ b/spec/serializers/user_serializer_spec.rb
@@ -213,6 +213,31 @@ describe UserSerializer do
     end
   end
 
+  context "with user fields" do
+    let(:user) { Fabricate(:user) }
+
+    let! :fields do
+      [
+        Fabricate(:user_field),
+        Fabricate(:user_field),
+        Fabricate(:user_field, show_on_profile: true),
+        Fabricate(:user_field, show_on_user_card: true),
+        Fabricate(:user_field, show_on_user_card: true, show_on_profile: true)
+      ]
+    end
+
+    let(:other_user_json) { UserSerializer.new(user, scope: Guardian.new(Fabricate(:user)), root: false).as_json }
+    let(:self_json) { UserSerializer.new(user, scope: Guardian.new(user), root: false).as_json }
+    let(:admin_json) { UserSerializer.new(user, scope: Guardian.new(Fabricate(:admin)), root: false).as_json }
+
+    it "includes the correct fields for each audience" do
+      expect(admin_json[:user_fields].keys).to contain_exactly(*fields.map { |f| f.id.to_s })
+      expect(other_user_json[:user_fields].keys).to contain_exactly(*fields[2..5].map { |f| f.id.to_s })
+      expect(self_json[:user_fields].keys).to contain_exactly(*fields.map { |f| f.id.to_s })
+    end
+
+  end
+
   context "with user_api_keys" do
     let(:user) { Fabricate(:user) }

GitHub

2 Likes