Merge pull request #5449 from margori/3016_block_messages_to_not_sharing_user

BUG: new messages to person that is not sharing

Changelog.md

@@ -100,6 +100,7 @@ This is disabled by default since it requires the installation of additional pac
 * Fix prefilling publisher after getting started [#5442](https://github.com/diaspora/diaspora/pull/5442)
 * Fix overflow in profile sidebar [#5450](https://github.com/diaspora/diaspora/pull/5450)
 * Fix code overflow in SPV and improve styling for code tags [#5422](https://github.com/diaspora/diaspora/pull/5422)
+* Correctly validate if local recipients actually want to receive a conversation [#5449](https://github.com/diaspora/diaspora/pull/5449)
 
 ## Features
 * Don't pull jQuery from a CDN by default [#5105](https://github.com/diaspora/diaspora/pull/5105)

app/models/conversation.rb

@@ -15,10 +15,21 @@ class Conversation < ActiveRecord::Base
   belongs_to :author, :class_name => 'Person'
 
   validate :max_participants
+  validate :local_recipients
 
   def max_participants
     errors.add(:max_participants, "too many participants") if participants.count > 20
   end
+  
+  def local_recipients
+    recipients.each do |recipient|
+      if recipient.local?
+        if recipient.owner.contacts.where(:person_id => self.author.id).count == 0
+          errors.add(:all_recipients, "recipient not allowed")
+        end
+      end
+    end
+  end
 
   accepts_nested_attributes_for :messages
 

spec/lib/statistics_spec.rb

@@ -19,7 +19,8 @@ describe Statistics do
                  {"id" => bob.id , "count" => 1 },
                  {"id" => eve.id , "count" => 0 },
                  {"id" => local_luke.id , "count" => 0 },
-                 {"id" => local_leia.id , "count" => 0 }]
+                 {"id" => local_leia.id , "count" => 0 },
+                 {"id" => peter.id , "count" => 0 }]
   end
 
   describe '#posts_count_sql' do
@@ -68,7 +69,8 @@ describe Statistics do
                  {"id" => bob.id , "count" => 2 },
                  {"id" => eve.id , "count" => 1 },
                  {"id" => local_luke.id , "count" => 2 },
-                 {"id" => local_leia.id , "count" => 2 }]
+                 {"id" => local_leia.id , "count" => 2 },
+                 {"id" => peter.id , "count" => 1 }]
 
       result_should_equal User.connection.select_all(@stats.contacts_sharing_with_count_sql)
     end
@@ -96,7 +98,8 @@ describe Statistics do
                  {"id" => bob.id , "count" => 1, "connected" => 1 },
                  {"id" => eve.id , "count" => 0, "connected" => 1 },
                  {"id" => local_luke.id , "count" => 0, "connected" => 0 },
-                 {"id" => local_leia.id , "count" => 0, "connected" => 0 }]
+                 {"id" => local_leia.id , "count" => 0, "connected" => 0 },
+                 {"id" => peter.id , "count" => 0, "connected" => 0 }]
 
       expect(@stats.fb_connected_distribution).to match_array(@result)
     end

spec/models/conversation_spec.rb

@@ -108,4 +108,20 @@ describe Conversation, :type => :model do
       end
     end
   end
+
+  describe '#invalid parameters' do
+    before do
+      @invalid_hash = {
+        :author => peter.person,
+        :participant_ids => [peter.person.id, @user1.person.id],
+        :subject => "cool stuff",
+        :messages_attributes => [ {:author => peter.person, :text => 'hey'} ]
+      }
+    end
+    
+    it 'with invalid recipient' do
+      conversation = Conversation.create(@invalid_hash)
+      expect(conversation).to be_invalid
+    end
+  end
 end

spec/spec_helper.rb

@@ -48,6 +48,10 @@ def remote_raphael
   @remote_raphael ||= Person.where(:diaspora_handle => 'raphael@remote.net').first
 end
 
+def peter
+  @peter ||= User.where(:username => 'peter').first
+end
+
 def photo_fixture_name
   @photo_fixture_name = File.join(File.dirname(__FILE__), 'fixtures', 'button.png')
 end

spec/support/fixture_builder.rb

@@ -34,5 +34,14 @@ FixtureBuilder.configure do |fbuilder|
 
     local_leia.contacts.create(:person => remote_raphael, :aspects => [leias_aspect])
     local_luke.contacts.create(:person => remote_raphael, :aspects => [lukes_aspect])
+    
+    # Set up a follower
+    peter = FactoryGirl.create(:user_with_aspect, :username => "peter")
+    peters_aspect = peter.aspects.where(:name => "generic").first
+    
+    peter.contacts.create!(:person => alice.person,
+                           :aspects => [peters_aspect],
+                           :sharing => false,
+                           :receiving => true)
    end
-end
+end