Don't expect raises from user receive

This commit is contained in:
Raphael 2010-11-17 15:35:18 -08:00
parent 6c97899d5d
commit 4853a55d9b
5 changed files with 58 additions and 57 deletions

View file

@ -14,6 +14,7 @@ class CommentsController < ApplicationController
@comment = current_user.comment(text, :on => target) if target
if @comment
Rails.logger.info("event=comment_create user=#{current_user.inspect} status=success comment=#{@comment.inspect}")
render :nothing => true, :status => 201
else
render :nothing => true, :status => 401

View file

@ -11,8 +11,10 @@ class RegistrationsController < Devise::RegistrationsController
flash[:notice] = I18n.t 'registrations.create.success'
@user.seed_aspects
sign_in_and_redirect(:user, @user)
Rails.logger.info("event=registration status=successful user=#{@user.inspect}")
else
flash[:error] = @user.errors.full_messages.join(', ')
Rails.logger.info("event=registration status=failure errors=#{@user.errors.full_messages.join(', ')}")
render :new
end
end

View file

@ -29,7 +29,8 @@ class Retraction
Rails.logger.debug "Performing retraction for #{post_id}"
if self.type.constantize.find_by_id(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"
Rails.logger.info("event=retraction status=abort reason='no post found authored by retractor' sender=#{person.diaspora_handle} post_id=#{post_id}")
raise "#{person.inspect} is trying to retract a post that either doesn't exist or is not by them"
end
begin
@ -38,7 +39,7 @@ class Retraction
target.unsocket_from_uid receiving_user_id if target.respond_to? :unsocket_from_uid
target.delete
rescue NameError
Rails.logger.info("Retraction for unknown type recieved.")
Rails.logger.info("event=retraction status=abort reason='unknown type'")
end
end
end

View file

@ -22,7 +22,7 @@ module Diaspora
def receive xml, salmon_author
object = Diaspora::Parser.from_xml(xml)
Rails.logger.debug("event=receive recipient=#{self.inspect} object=#{object.inspect} sender=#{salmon_author.inspect}")
Rails.logger.info("event=receive status=start recipient=#{self.diaspora_handle} object=#{object.inspect} sender=#{salmon_author.diaspora_handle}")
if object.is_a?(Request)
salmon_author.save
@ -36,25 +36,22 @@ module Diaspora
end
if (salmon_author.diaspora_handle != xml_author)
raise "Malicious Post, #{salmon_author.real_name} with handle #{salmon_author.diaspora_handle} is sending a #{object.class} as #{xml_author} "
Rails.logger.info("event=receive status=abort reason='author in xml does not match retrieved person' recipient=#{self.diaspora_handle} sender=#{salmon_author.diaspora_handle} payload=#{object.inspect}")
return
end
if object.is_a?(Comment) || object.is_a?(Post)|| object.is_a?(Request) || object.is_a?(Retraction) || object.is_a?(Profile)
e = EMWebfinger.new(object.diaspora_handle)
e = EMWebfinger.new(object.diaspora_handle)
e.on_person do |person|
if person.class == Person
object.person = person if object.respond_to? :person=
unless object.is_a?(Request) || self.contact_for(salmon_author)
raise "Not connected to that person"
else
return receive_object(object,person)
end
e.on_person do |person|
if person.class == Person
object.person = person if object.respond_to? :person=
unless object.is_a?(Request) || self.contact_for(salmon_author)
Rails.logger.info("event=receive status=abort reason='sender not connected to recipient' recipient=#{self.diaspora_handle} sender=#{salmon_author.diaspora_handle} payload=#{object.inspect}")
return
else
return receive_object(object,person)
end
end
else
raise "you messed up"
end
end
@ -76,7 +73,8 @@ module Diaspora
def receive_retraction retraction
if retraction.type == 'Person'
unless retraction.person.id.to_s == retraction.post_id.to_s
raise "#{retraction.diaspora_handle} trying to disconnect #{retraction.post_id} from #{self.id}"
Rails.logger.info("event=receive status=abort reason='sender is not the person he is trying to retract' recipient=#{self.diaspora_handle} sender=#{salmon_author.diaspora_handle} payload=#{retraction.inspect}")
return
end
Rails.logger.info( "the person id is #{retraction.post_id} the contact found is #{visible_person_by_id(retraction.post_id).inspect}")
disconnected_by visible_person_by_id(retraction.post_id)
@ -100,7 +98,10 @@ module Diaspora
end
def receive_comment comment
raise "In receive for #{self.real_name}, signature was not valid on: #{comment.inspect}" unless comment.post.person == self.person || comment.verify_post_creator_signature
unless comment.post.person == self.person || comment.verify_post_creator_signature
Rails.logger.info("event=receive status=abort reason='comment signature not valid' recipient=#{self.diaspora_handle} sender=#{salmon_author.diaspora_handle} payload=#{retraction.inspect}")
return
end
self.visible_people = self.visible_people | [comment.person]
self.save
Rails.logger.debug("The person parsed from comment xml is #{comment.person.inspect}") unless comment.person.nil?

View file

@ -19,18 +19,16 @@ describe "attack vectors" do
context 'non-contact valid user' do
it 'raises if receives post by non-contact' do
it 'does not save a post from a non-contact' do
post_from_non_contact = bad_user.build_post( :status_message, :message => 'hi')
xml = bad_user.salmon(post_from_non_contact).xml_for(user.person)
post_from_non_contact.delete
bad_user.delete
post_count = Post.count
proc{ user.receive_salmon(xml) }.should raise_error /Not connected to that person/
user.receive_salmon(xml)
user.raw_visible_posts.include?(post_from_non_contact).should be false
Post.count.should == post_count
end
@ -52,33 +50,34 @@ describe "attack vectors" do
connect_users(user, aspect, user3, aspect3)
end
describe 'mass assignment on id' do
it "does not save a message over an old message with a different author" do
original_message = user2.post :status_message, :message => 'store this!', :to => aspect2.id
it 'overwrites messages with a different user' 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.receive_salmon(user2.salmon(original_message).xml_for(user.person))
user.raw_visible_posts.count.should be 1
lambda {
malicious_message = Factory.build( :status_message, :id => original_message.id, :message => 'BAD!!!', :person => user3.person)
user.receive_salmon(user3.salmon(malicious_message).xml_for(user.person))
}.should_not change{user.reload.raw_visible_posts.count}
malicious_message = Factory.build( :status_message, :id => original_message.id, :message => 'BAD!!!', :person => user3.person)
proc{user.receive_salmon(user3.salmon(malicious_message).xml_for(user.person))}.should raise_error /Malicious Post/
original_message.reload.message.should == "store this!"
user.raw_visible_posts.first.message.should == "store this!"
end
it 'does not save a message over an old message with the same author' 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
user.raw_visible_posts.first.message.should == "store this!"
lambda {
malicious_message = Factory.build( :status_message, :id => original_message.id, :message => 'BAD!!!', :person => user2.person)
user.receive_salmon(user3.salmon(malicious_message).xml_for(user.person))
}.should_not change{user.reload.raw_visible_posts.count}
original_message.reload.message.should == "store this!"
user.raw_visible_posts.first.message.should == "store this!"
end
end
it 'overwrites messages which apear to be from the same user' 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
malicious_message = Factory.build( :status_message, :id => original_message.id, :message => 'BAD!!!', :person => user2.person)
proc{user.receive_salmon(user3.salmon(malicious_message).xml_for(user.person))}.should raise_error /Malicious Post/
user.raw_visible_posts.count.should be 1
user.raw_visible_posts.first.message.should == "store this!"
end
it 'should not overwrite another persons profile profile' do
profile = user2.profile.clone
profile.first_name = "Not BOB"
@ -86,12 +85,12 @@ describe "attack vectors" do
user2.reload
first_name = user2.profile.first_name
proc{user.receive_salmon(user3.salmon(profile).xml_for(user.person))}.should raise_error /Malicious Post/
user.receive_salmon(user3.salmon(profile).xml_for(user.person))
user2.reload
user2.profile.first_name.should == first_name
end
it 'should not receive retractions on post you do not own' do
it "ignores retractions on a post not owned by the retraction's sender" 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
@ -101,12 +100,12 @@ describe "attack vectors" do
ret.diaspora_handle = user3.person.diaspora_handle
ret.type = original_message.class.to_s
proc{ user.receive_salmon(user3.salmon(ret).xml_for(user.person)) }.should raise_error /is trying to retract a post they do not own/
user.receive_salmon(user3.salmon(ret).xml_for(user.person))
StatusMessage.count.should be 1
user.reload.raw_visible_posts.count.should be 1
end
it 'should disregard retractions for a non-existant posts' do
it "disregards retractions for non-existent posts that are from someone other than the post's author" do
original_message = user2.post :status_message, :message => 'store this!', :to => aspect2.id
id = original_message.reload.id
@ -131,8 +130,9 @@ describe "attack vectors" do
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
lambda {
user.receive_salmon(user3.salmon(ret).xml_for(user.person))
}.should_not change(StatusMessage, :count)
user.reload.raw_visible_posts.count.should be 1
end
@ -144,9 +144,7 @@ describe "attack vectors" do
proc{
user.receive_salmon(user3.salmon(ret).xml_for(user.person))
}.should raise_error /#{user3.diaspora_handle} trying to disconnect #{user2.person.id} from #{user.id}/
user.reload.contacts.count.should == 2
}.should_not change{user.reload.contacts.count}
end
it 'it should not allow you to send retractions with xml and salmon handle mismatch' do
@ -157,9 +155,7 @@ describe "attack vectors" do
proc{
user.receive_salmon(user3.salmon(ret).xml_for(user.person))
}.should raise_error /Malicious Post/
user.reload.contacts.count.should == 2
}.should_not change{user.reload.contacts.count}
end
it 'does not let me update other persons post' do