Merge pull request #81 from SuperTux88/relay-likes-for-comments

Relay likes for comments

docs/federation/relayable.md

@@ -2,10 +2,14 @@
 title: Relayable
 ---
 
-If a person participates on an entity, it needs to be relayed via the author of the parent entity, because only the
+If a person participates on an entity, it needs to be relayed via the author of the root entity, because only the
-parent author knows, to whom they shared the original entity.
+root author knows, to whom they shared the original entity.
 
-Such entities are:
+The root is the most top parent of the relayable: If the parent of a relayable is a relayable itself,
+the parent of that should be used, until it isn't a relayable anymore.
+For example, for both, a `Like` for a `Post` and a `Like` for a `Comment`, the author of the `Post` should be used.
+
+Such relayable entities are:
 
 * [Comment][comment]
 * [Like][like]
@@ -24,16 +28,16 @@ All relayables have the following properties:
 
 ## Relaying
 
-If the author is not the same as the parent author, the author of the relayable sends the entity to the parent author
+If the author is not the same as the root author, the author of the relayable sends the entity to the root author
 and the author must include the `author_signature`.
 
-The parent author then must envelop it in a new [Magic Envelope][magicsig] and send the entity to all the recipients
+The root author then must envelop it in a new [Magic Envelope][magicsig] and send the entity to all the recipients
-of the parent entity. If the author and the parent author are on the same server, the author must sign the
+of the root entity. If the author and the root author are on the same server, the author must sign the
-`author_signature` and the parent author needs to sign the Magic Envelope.
+`author_signature` and the root author needs to sign the Magic Envelope.
 
-If someone other then the parent author receives a relayable without a valid Magic Envelope signed from
+If someone other then the root author receives a relayable without a valid Magic Envelope signed from
-the parent author, it must be ignored. If the author is not the same as the parent author and the `author_signature`
+the root author, it must be ignored. If the author is not the same as the root author and the `author_signature`
-is missing or invalid, it also must be ignored. If the author is the same as the parent author, the `author_signature`
+is missing or invalid, it also must be ignored. If the author is the same as the root author, the `author_signature`
 can be missing, because a valid signature in the Magic Envelope from the author is enough in that case.
 
 ## Signatures
@@ -41,15 +45,15 @@ can be missing, because a valid signature in the Magic Envelope from the author
 The string to sign is built with the content of all properties (except the `author_signature` itself),
 concatenated using `;` as separator in the same order as they appear in the XML. The order in the XML is not specified.
 
-This ensures that relayables even work, if the parent author or another recipient does not know all properties of the
+This ensures that relayables even work, if the root author or another recipient does not know all properties of the
 relayable entity (e.g. older version of diaspora\*).
 
 This string is then signed with the private RSA key using the RSA-SHA256 algorithm and base64-encoded.
 
-The parent author must use the same order as the relayable author. Unknown properties must be included again in the XML
+The root author must use the same order as the relayable author. Unknown properties must be included again in the XML
 and the signature.
 
-To support fetching of the relayables, the parent author should save the following information:
+To support fetching of the relayables, the root author should save the following information:
 
 * order of the received XML
 * additional (unknown) properties
@@ -57,13 +61,13 @@ To support fetching of the relayables, the parent author should save the followi
 
 ## Retraction / Reject
 
-The parent author is allowed to retract the entity, so there are no additional signatures required for the
+The root author is allowed to retract the entity, so there are no additional signatures required for the
 [Retraction][retraction] (only the [Salmon Magic Signature][magicsig]).
 
-If the author retracts the entity, they send a [Retraction][retraction] to the parent author. The parent author also
+If the author retracts the entity, they send a [Retraction][retraction] to the root author. The root author also
-must relay this retraction to all recipients of the parent entity.
+must relay this retraction to all recipients of the root entity.
 
-If the parent author wants to reject the entity (e.g. if they ignore the author of the relayable), they can simply send
+If the root author wants to reject the entity (e.g. if they ignore the author of the relayable), they can simply send
 a [Retraction][retraction] for it back to the author.
 
 

lib/diaspora_federation/entities/related_entity.rb

@@ -24,6 +24,14 @@ module DiasporaFederation
       #   @return [RelatedEntity] parent entity
       entity :parent, Entities::RelatedEntity, default: nil
 
+      # The root entity, this entity is responsible for relaying relayables
+      # @return [RelatedEntity] absolute parent entity
+      def root
+        root = self
+        root = root.parent until root.parent.nil?
+        root
+      end
+
       # Get related entity from the backend or fetch it from remote if not available locally
       # @return [RelatedEntity] fetched related entity
       def self.fetch(author, type, guid)

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

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

spec/lib/diaspora_federation/entities/related_entity_spec.rb

@@ -5,6 +5,20 @@ module DiasporaFederation
 
     it_behaves_like "an Entity subclass"
 
+    describe "#root" do
+      it "returns self if it's already the root" do
+        entity = Fabricate(:related_entity, parent: nil)
+        expect(entity.root).to eq(entity)
+      end
+
+      it "returns the root entity if the current entity has parents" do
+        root = Fabricate(:related_entity, parent: nil)
+        parent = Fabricate(:related_entity, parent: root)
+        entity = Fabricate(:related_entity, parent: parent)
+        expect(entity.root).to eq(root)
+      end
+    end
+
     describe ".fetch" do
       let(:guid) { Fabricate.sequence(:guid) }
       let(:entity) { Fabricate(:related_entity) }

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

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