Merge branch 'master' of github.com:diaspora/diaspora

app/helpers/sockets_helper.rb

@@ -23,7 +23,7 @@ module SocketsHelper
       action_hash[:photo_hash] = object.thumb_hash
     end
 
-    if object.person.owner_id == uid
+    if object.person && object.person.owner_id == uid
       action_hash[:mine?] = true
     end
 

app/models/retraction.rb

@@ -7,12 +7,13 @@ class Retraction
   include Diaspora::Webhooks
 
   xml_accessor :post_id
-  xml_accessor :person_id
+  xml_accessor :diaspora_handle
   xml_accessor :type
 
   attr_accessor :post_id
-  attr_accessor :person_id
+  attr_accessor :diaspora_handle
   attr_accessor :type
+  attr_accessor :person
 
   def self.for(object)
     retraction = self.new
@@ -23,12 +24,16 @@ class Retraction
       retraction.post_id = object.id
       retraction.type = object.class.to_s
     end
-    retraction.person_id = person_id_from(object)
+    retraction.diaspora_handle = object.diaspora_handle 
     retraction
   end
 
   def perform receiving_user_id
     Rails.logger.debug "Performing retraction for #{post_id}"
+    unless Post.first(:diaspora_handle => person.diaspora_handle, :id => post_id) 
+      raise "#{person.inspect} is trying to retract a post they do not own"
+    end
+
     begin
       Rails.logger.debug("Retracting #{self.type} id: #{self.post_id}")
       target = self.type.constantize.first(:id => self.post_id)
@@ -38,13 +43,4 @@ class Retraction
       Rails.logger.info("Retraction for unknown type recieved.")
     end
   end
-
-  def self.person_id_from(object)
-    object.is_a?(Person) ? object.id : object.person.id
-  end
-
-  def person
-    Person.find_by_id(self.person_id)
-  end
-
 end

config/deploy_config.yml

@@ -6,7 +6,7 @@ cross_server:
   deploy_to: '/usr/local/app/diaspora'
   user: 'root'
   repo: 'git://github.com/diaspora/diaspora.git'
-  branch: 'diaspora-handle-request'
+  branch: 'master'
   default_env: 'development'
 servers:
   tom:

lib/diaspora/user/receiving.rb

@@ -21,13 +21,14 @@ module Diaspora
         Rails.logger.debug("From: #{object.person.inspect}") if object.person
 
 
-        if object.is_a?(Comment) || object.is_a?(Post)|| object.is_a?(Request)
+        if object.is_a?(Comment) || object.is_a?(Post)|| object.is_a?(Request) || object.is_a?(Retraction)
           e = EMWebfinger.new(object.diaspora_handle)
 
           e.on_person { |person|
 
             if person.class == Person
               object.person = person
+
               sender_in_xml = sender(object, xml, person)
               if (salmon_author != sender_in_xml)
                 raise "Malicious Post, #{salmon_author.real_name} with id #{salmon_author.id} is sending a #{object.class} as #{sender_in_xml.real_name} with id #{sender_in_xml.id} "
@@ -41,6 +42,8 @@ module Diaspora
 
               if object.is_a?(Comment) 
                 receive_comment object, xml
+              elsif object.is_a? Retraction
+                receive_retraction object, xml
               else
                 receive_post object, xml
               end
@@ -57,18 +60,15 @@ module Diaspora
 
           raise "Not friends with that person" unless self.contact_for(salmon_author)
 
-          if object.is_a? Retraction
-            receive_retraction object, xml
-          elsif object.is_a? Profile
+
+          if object.is_a? Profile
             receive_profile object, xml
           end
         end
       end
 
       def sender(object, xml, webfingered_person = nil)
-        if object.is_a? Retraction
-          sender = object.person
-        elsif object.is_a? Profile
+        if object.is_a? Profile
           sender = Diaspora::Parser.owner_id_from_xml xml
 
         else
@@ -83,6 +83,9 @@ module Diaspora
 
       def receive_retraction retraction, xml
         if retraction.type == 'Person'
+          unless retraction.person.id.to_s == retraction.post_id.to_s
+            raise "#{retraction.diaspora_handle} trying to unfriend #{retraction.post_id} from #{self.id}"
+          end
           Rails.logger.info( "the person id is #{retraction.post_id} the friend found is #{visible_person_by_id(retraction.post_id).inspect}")
           unfriended_by visible_person_by_id(retraction.post_id)
         else

spec/lib/diaspora/parser_spec.rb

@@ -27,7 +27,7 @@ describe Diaspora::Parser do
 
     it 'should accept retractions' do
       friend_users(user, aspect, user2, aspect2)
-      message = Factory.create(:status_message, :person => user2.person)
+      message = user2.post(:status_message, :message => "cats", :to => aspect2.id)
       retraction = Retraction.for(message)
       xml = retraction.to_diaspora_xml
 

spec/models/user/attack_vectors_spec.rb

@@ -80,20 +80,60 @@ describe "attack vectors" do
       user2.profile.first_name.should == first_name
     end
 
-    it 'can send retractions on post you do not own' do
-      pending
+    it 'should not receive retractions on post you do not own' do
       original_message = user2.post :status_message, :message => 'store this!', :to => aspect2.id
       user.receive_salmon(user2.salmon(original_message).xml_for(user.person))
       user.raw_visible_posts.count.should be 1
 
       ret = Retraction.new
       ret.post_id = original_message.id
-      ret.person_id = user3.person.id
+      ret.diaspora_handle = user3.person.diaspora_handle
       ret.type = original_message.class.to_s
 
-      user.receive_salmon(user3.salmon(ret).xml_for(user.person))
+      proc{ user.receive_salmon(user3.salmon(ret).xml_for(user.person)) }.should raise_error /is trying to retract a post they do not own/
       StatusMessage.count.should be 1
       user.reload.raw_visible_posts.count.should be 1
     end
+
+    it 'should not receive retractions where the retractor and the salmon author do not match' do
+      original_message = user2.post :status_message, :message => 'store this!', :to => aspect2.id
+      user.receive_salmon(user2.salmon(original_message).xml_for(user.person))
+      user.raw_visible_posts.count.should be 1
+
+      ret = Retraction.new
+      ret.post_id = original_message.id
+      ret.diaspora_handle = user2.person.diaspora_handle
+      ret.type = original_message.class.to_s
+
+      proc{ user.receive_salmon(user3.salmon(ret).xml_for(user.person)) }.should raise_error /Malicious Post/
+      StatusMessage.count.should be 1
+      user.reload.raw_visible_posts.count.should be 1
+    end
+
+    it 'it should not allow you to send retractions for other people' do
+      ret = Retraction.new
+      ret.post_id = user2.person.id
+      ret.diaspora_handle = user3.person.diaspora_handle
+      ret.type = user2.person.class.to_s
+
+      proc{ 
+        user.receive_salmon(user3.salmon(ret).xml_for(user.person)) 
+      }.should raise_error /#{user3.diaspora_handle} trying to unfriend #{user2.person.id} from #{user.id}/
+    
+      user.reload.friends.count.should == 2
+    end
+
+    it 'it should not allow you to send retractions with xml and salmon handle mismatch' do
+      ret = Retraction.new
+      ret.post_id = user2.person.id
+      ret.diaspora_handle = user2.person.diaspora_handle
+      ret.type = user2.person.class.to_s
+
+      proc{ 
+        user.receive_salmon(user3.salmon(ret).xml_for(user.person)) 
+      }.should raise_error /Malicious Post/
+    
+      user.reload.friends.count.should == 2
+    end
   end
 end