From 61c00b35968a7ed9c579dc9510047ac7b25c8b06 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Tue, 5 Sep 2017 23:54:11 +0200 Subject: [PATCH] Use top parent when relaying relayables of relayables --- lib/diaspora_federation/entities/relayable.rb | 6 +- .../entities/retraction.rb | 2 +- .../entities/relayable_spec.rb | 62 ++++++++++++++++++- .../entities/retraction_spec.rb | 31 ++++++++-- 4 files changed, 90 insertions(+), 11 deletions(-) diff --git a/lib/diaspora_federation/entities/relayable.rb b/lib/diaspora_federation/entities/relayable.rb index a61a467..6c44ebb 100644 --- a/lib/diaspora_federation/entities/relayable.rb +++ b/lib/diaspora_federation/entities/relayable.rb @@ -76,11 +76,11 @@ module DiasporaFederation # @raise [SignatureVerificationFailed] if the signature is not valid # @raise [PublicKeyNotFound] if no public key is found def verify_signature - super(author, :author_signature) unless author == parent.author + super(author, :author_signature) unless author == parent.root.author end def sender_valid?(sender) - (sender == author && parent.local) || sender == parent.author + (sender == author && parent.root.local) || sender == parent.root.author end # @return [String] string representation of this object @@ -121,7 +121,7 @@ module DiasporaFederation # Sign with parent author key, if the parent author is local (if the private key is found) # @return [String] A Base64 encoded signature of #signature_data with key def sign_with_parent_author_if_available - privkey = DiasporaFederation.callbacks.trigger(:fetch_private_key, parent.author) + privkey = DiasporaFederation.callbacks.trigger(:fetch_private_key, parent.root.author) return unless privkey sign_with_key(privkey).tap do diff --git a/lib/diaspora_federation/entities/retraction.rb b/lib/diaspora_federation/entities/retraction.rb index 67799b8..217c0a6 100644 --- a/lib/diaspora_federation/entities/retraction.rb +++ b/lib/diaspora_federation/entities/retraction.rb @@ -28,7 +28,7 @@ module DiasporaFederation def sender_valid?(sender) case target_type when "Comment", "Like", "PollParticipation" - sender == target.author || sender == target.parent.author + sender == target.author || sender == target.root.author else sender == target.author end diff --git a/spec/lib/diaspora_federation/entities/relayable_spec.rb b/spec/lib/diaspora_federation/entities/relayable_spec.rb index bbac670..c861c55 100644 --- a/spec/lib/diaspora_federation/entities/relayable_spec.rb +++ b/spec/lib/diaspora_federation/entities/relayable_spec.rb @@ -78,6 +78,16 @@ module DiasporaFederation }.not_to raise_error end + it "doesn't raise when no author signature was passed, but the author is also the author of the root entity" do + hash[:author_signature] = nil + root = Fabricate(:related_entity, author: author, local: false) + hash[:parent] = Fabricate(:related_entity, author: Fabricate.sequence(:diaspora_id), local: false, parent: root) + + expect { + Entities::SomeRelayable.new(hash, signature_order).verify_signature + }.not_to raise_error + end + it "raises when bad author signature was passed" do hash[:author_signature] = sign_with_key(author_pkey, "bad signed string") @@ -198,6 +208,22 @@ XML expect(verify_signature(parent_pkey, parent_author_signature, signature_data)).to be_truthy end + it "computes correct signatures for the entity when the parent is a relayable itself" do + intermediate_author = Fabricate.sequence(:diaspora_id) + parent = Fabricate(:related_entity, author: intermediate_author, local: true, parent: local_parent) + expect_callback(:fetch_private_key, author).and_return(author_pkey) + expect_callback(:fetch_private_key, local_parent.author).and_return(parent_pkey) + expect(DiasporaFederation.callbacks).not_to receive(:trigger).with(:fetch_private_key, intermediate_author) + + xml = Entities::SomeRelayable.new(hash.merge(parent: parent)).to_xml + + author_signature = xml.at_xpath("author_signature").text + parent_author_signature = xml.at_xpath("parent_author_signature").text + + expect(verify_signature(author_pkey, author_signature, signature_data)).to be_truthy + expect(verify_signature(parent_pkey, parent_author_signature, signature_data)).to be_truthy + end + it "computes correct signatures for the entity with new unknown xml elements" do expect_callback(:fetch_private_key, author).and_return(author_pkey) expect_callback(:fetch_private_key, local_parent.author).and_return(parent_pkey) @@ -549,9 +575,9 @@ XML end it "allows parent author" do - entity = Entities::SomeRelayable.new(hash) + entity = Entities::SomeRelayable.new(hash.merge(parent: remote_parent)) - expect(entity.sender_valid?(local_parent.author)).to be_truthy + expect(entity.sender_valid?(remote_parent.author)).to be_truthy end it "does not allow any random author" do @@ -560,6 +586,38 @@ XML expect(entity.sender_valid?(invalid_author)).to be_falsey end + + context "multi-layer relayable" do + let(:intermediate_author) { Fabricate.sequence(:diaspora_id) } + + it "allows author if the root entity is local" do + parent = Fabricate(:related_entity, author: intermediate_author, local: false, parent: local_parent) + entity = Entities::SomeRelayable.new(hash.merge(parent: parent)) + + expect(entity.sender_valid?(author)).to be_truthy + end + + it "does not allow the author if the root entity is not local" do + parent = Fabricate(:related_entity, author: intermediate_author, local: true, parent: remote_parent) + entity = Entities::SomeRelayable.new(hash.merge(parent: parent)) + + expect(entity.sender_valid?(author)).to be_falsey + end + + it "allows root entity author" do + parent = Fabricate(:related_entity, author: intermediate_author, local: false, parent: remote_parent) + entity = Entities::SomeRelayable.new(hash.merge(parent: parent)) + + expect(entity.sender_valid?(remote_parent.author)).to be_truthy + end + + it "does not allow an intermediate parent author" do + parent = Fabricate(:related_entity, author: intermediate_author, local: false, parent: remote_parent) + entity = Entities::SomeRelayable.new(hash.merge(parent: parent)) + + expect(entity.sender_valid?(intermediate_author)).to be_falsey + end + end end end end diff --git a/spec/lib/diaspora_federation/entities/retraction_spec.rb b/spec/lib/diaspora_federation/entities/retraction_spec.rb index 5b8328b..c3d1e9d 100644 --- a/spec/lib/diaspora_federation/entities/retraction_spec.rb +++ b/spec/lib/diaspora_federation/entities/retraction_spec.rb @@ -62,27 +62,48 @@ XML ) } let(:relayable_data) { data.merge(target_type: target_type, target: relayable_target) } + let(:entity) { Entities::Retraction.new(relayable_data) } it "allows target author" do - entity = Entities::Retraction.new(relayable_data) - expect(entity.sender_valid?(bob.diaspora_id)).to be_truthy end it "allows target parent author" do - entity = Entities::Retraction.new(relayable_data) - expect(entity.sender_valid?(alice.diaspora_id)).to be_truthy end it "does not allow any random author" do - entity = Entities::Retraction.new(relayable_data) invalid_author = Fabricate.sequence(:diaspora_id) expect(entity.sender_valid?(invalid_author)).to be_falsey end end end + + context "Like of a Comment" do + let(:comment) { + Fabricate( + :related_entity, + author: Fabricate.sequence(:diaspora_id), + parent: Fabricate(:related_entity, author: alice.diaspora_id) + ) + } + let(:relayable_target) { Fabricate(:related_entity, author: bob.diaspora_id, parent: comment) } + let(:relayable_data) { data.merge(target_type: "Like", target: relayable_target) } + let(:entity) { Entities::Retraction.new(relayable_data) } + + it "allows target author" do + expect(entity.sender_valid?(bob.diaspora_id)).to be_truthy + end + + it "allows target root entity author" do + expect(entity.sender_valid?(alice.diaspora_id)).to be_truthy + end + + it "does not allow the comment author" do + expect(entity.sender_valid?(comment.author)).to be_falsey + end + end end end end