From e92c8000babb42412e2fbe4cb4ab7ff6d65a81eb Mon Sep 17 00:00:00 2001
From: Dennis Schubert <mail@dennis-schubert.de>
Date: Thu, 2 Jul 2015 03:11:22 +0200
Subject: [PATCH] Do not leak private profile fields in JSON format

Signed-off-by: Dennis Schubert <mail@dennis-schubert.de>
---
 app/presenters/person_presenter.rb       | 43 ++++++++++++++----------
 app/presenters/profile_presenter.rb      | 26 ++++++++------
 spec/presenters/person_presenter_spec.rb |  4 +--
 3 files changed, 44 insertions(+), 29 deletions(-)

diff --git a/app/presenters/person_presenter.rb b/app/presenters/person_presenter.rb
index 37f7593534..f53087eee0 100644
--- a/app/presenters/person_presenter.rb
+++ b/app/presenters/person_presenter.rb
@@ -1,38 +1,47 @@
 class PersonPresenter < BasePresenter
   def base_hash
-    { id: id,
-      guid: guid,
-      name: name,
+    {
+      id:          id,
+      guid:        guid,
+      name:        name,
       diaspora_id: diaspora_handle
     }
   end
 
   def full_hash
-    base_hash.merge({
-      relationship: relationship,
-      block: is_blocked? ? BlockPresenter.new(current_user_person_block).base_hash : false,
-      contact: (!own_profile? && has_contact?) ? { id: current_user_person_contact.id } : false,
+    base_hash.merge(
+      relationship:   relationship,
+      block:          is_blocked? ? BlockPresenter.new(current_user_person_block).base_hash : false,
+      contact:        (!own_profile? && has_contact?) ? {id: current_user_person_contact.id} : false,
       is_own_profile: own_profile?
-    })
+    )
   end
 
   def full_hash_with_avatar
-    full_hash.merge({avatar: AvatarPresenter.new(profile).base_hash})
+    full_hash.merge(avatar: AvatarPresenter.new(profile).base_hash)
   end
 
   def full_hash_with_profile
-    full_hash.merge({profile: ProfilePresenter.new(profile).full_hash})
+    attrs = full_hash
+
+    if own_profile? || person_is_following_current_user
+      attrs.merge!(profile: ProfilePresenter.new(profile).private_hash)
+    else
+      attrs.merge!(profile: ProfilePresenter.new(profile).public_hash)
+    end
+
+    attrs
   end
 
-  def as_json(options={})
+  def as_json(_options={})
     attrs = full_hash_with_avatar
 
     if own_profile? || person_is_following_current_user
-      attrs.merge!({
-                      :location => @presentable.location,
-                      :birthday => @presentable.formatted_birthday,
-                      :bio => @presentable.bio
-                  })
+      attrs.merge!(
+        location: @presentable.location,
+        birthday: @presentable.formatted_birthday,
+        bio:      @presentable.bio
+      )
     end
 
     attrs
@@ -51,7 +60,7 @@ class PersonPresenter < BasePresenter
     contact = current_user_person_contact
     return :not_sharing unless contact
 
-    [:mutual, :sharing, :receiving].find do |status|
+    %i(mutual sharing receiving).find do |status|
       contact.public_send("#{status}?")
     end || :not_sharing
   end
diff --git a/app/presenters/profile_presenter.rb b/app/presenters/profile_presenter.rb
index 3581ef9f92..71e624fcbc 100644
--- a/app/presenters/profile_presenter.rb
+++ b/app/presenters/profile_presenter.rb
@@ -2,20 +2,26 @@ class ProfilePresenter < BasePresenter
   include PeopleHelper
 
   def base_hash
-    {  id: id,
-       tags: tags.pluck(:name),
-       bio: bio_message.plain_text_for_json,
-       location: location_message.plain_text_for_json,
-       gender: gender,
-       birthday: formatted_birthday,
-       searchable: searchable
+    {
+      id:         id,
+      searchable: searchable
     }
   end
 
-  def full_hash
-    base_hash.merge({
+  def public_hash
+    base_hash.merge(
       avatar: AvatarPresenter.new(@presentable).base_hash,
-    })
+      tags:   tags.pluck(:name)
+    )
+  end
+
+  def private_hash
+    public_hash.merge(
+      bio:      bio_message.plain_text_for_json,
+      birthday: formatted_birthday,
+      gender:   gender,
+      location: location_message.plain_text_for_json
+    )
   end
 
   def formatted_birthday
diff --git a/spec/presenters/person_presenter_spec.rb b/spec/presenters/person_presenter_spec.rb
index 97f01887cf..54424e2ee6 100644
--- a/spec/presenters/person_presenter_spec.rb
+++ b/spec/presenters/person_presenter_spec.rb
@@ -16,12 +16,12 @@ describe PersonPresenter do
       let(:presenter){ PersonPresenter.new(person, current_user) }
 
       it "doesn't share private information when the users aren't connected" do
-        expect(presenter.as_json).not_to have_key(:location)
+        expect(presenter.full_hash_with_profile[:profile]).not_to have_key(:location)
       end
 
       it "has private information when the person is sharing with the current user" do
         expect(person).to receive(:shares_with).with(current_user).and_return(true)
-        expect(presenter.as_json).to have_key(:location)
+        expect(presenter.full_hash_with_profile[:profile]).to have_key(:location)
       end
 
       it "returns the user's private information if a user is logged in as herself" do
-- 
GitLab