refactoring relayable signature-checking

verify signature after creating the entity instance

lib/diaspora_federation/entities/comment.rb

@@ -21,6 +21,12 @@ module DiasporaFederation
       #   @see Person#diaspora_id
       #   @return [String] diaspora ID
       property :diaspora_id, xml_name: :diaspora_handle
+
+      # The {Comment} parent is a Post
+      # @return [String] target type
+      def target_type
+        "Post"
+      end
     end
   end
 end

lib/diaspora_federation/entities/message.rb

@@ -35,8 +35,8 @@ module DiasporaFederation
       property :conversation_guid
 
       # The {Message} parent is a {Conversation}
-      # @return [String] parent entity type
-      def self.get_target_entity_type(*)
+      # @return [String] target type
+      def target_type
         "Conversation"
       end
     end

lib/diaspora_federation/entities/poll_participation.rb

@@ -25,8 +25,8 @@ module DiasporaFederation
       property :poll_answer_guid
 
       # The {PollParticipation} parent is a {Poll}
-      # @return [String] parent entity type
-      def self.get_target_entity_type(*)
+      # @return [String] target type
+      def target_type
         "Poll"
       end
     end

lib/diaspora_federation/entities/relayable.rb

@@ -31,11 +31,25 @@ module DiasporaFederation
           property :parent_guid
           property :parent_author_signature, default: nil
           property :author_signature, default: nil
+        end
+      end
 
-          # get the type of the parent entity
-          # @return [String] parent entity type
-          def self.get_target_entity_type(data)
-            data[:target_type] || "Post"
+      # Adds signatures to the hash with the keys of the author and the parent
+      # if the signatures are not in the hash yet and if the keys are available.
+      #
+      # @return [Hash] entity data hash with updated signatures
+      def to_signed_h
+        to_h.tap do |hash|
+          if author_signature.nil?
+            privkey = DiasporaFederation.callbacks.trigger(:fetch_private_key_by_diaspora_id, diaspora_id)
+            hash[:author_signature] = Signing.sign_with_key(hash, privkey) unless privkey.nil?
+          end
+
+          if parent_author_signature.nil?
+            privkey = DiasporaFederation.callbacks.trigger(
+              :fetch_author_private_key_by_entity_guid, target_type, parent_guid
+            )
+            hash[:parent_author_signature] = Signing.sign_with_key(hash, privkey) unless privkey.nil?
           end
         end
       end
@@ -45,9 +59,7 @@ module DiasporaFederation
       # @return [Nokogiri::XML::Element] root element containing properties as child elements
       def to_xml
         entity_xml.tap do |xml|
-          hash = to_h
-          Relayable.update_signatures!(hash, self.class)
-
+          hash = to_signed_h
           xml.at_xpath("author_signature").content = hash[:author_signature]
           xml.at_xpath("parent_author_signature").content = hash[:parent_author_signature]
         end
@@ -58,61 +70,29 @@ module DiasporaFederation
       end
 
       # verifies the signatures (+author_signature+ and +parent_author_signature+ if needed)
-      # @param [Hash] data hash with data to verify
-      # @param [Class] klass entity type
       # @raise [SignatureVerificationFailed] if the signature is not valid or no public key is found
-      def self.verify_signatures(data, klass)
-        pubkey = DiasporaFederation.callbacks.trigger(:fetch_public_key_by_diaspora_id, data[:diaspora_id])
-        raise SignatureVerificationFailed, "failed to fetch public key for #{data[:diaspora_id]}" if pubkey.nil?
+      def verify_signatures
+        pubkey = DiasporaFederation.callbacks.trigger(:fetch_public_key_by_diaspora_id, diaspora_id)
+        raise SignatureVerificationFailed, "failed to fetch public key for #{diaspora_id}" if pubkey.nil?
         raise SignatureVerificationFailed, "wrong author_signature" unless Signing.verify_signature(
-          data, data[:author_signature], pubkey
+          data, author_signature, pubkey
         )
 
-        author_is_local = DiasporaFederation.callbacks.trigger(
-          :entity_author_is_local?,
-          klass.get_target_entity_type(data),
-          data[:parent_guid]
-        )
-        verify_parent_signature(data, klass) unless author_is_local
+        author_is_local = DiasporaFederation.callbacks.trigger(:entity_author_is_local?, target_type, parent_guid)
+        verify_parent_signature unless author_is_local
       end
 
+      private
+
       # this happens only on downstream federation
-      # @param [Hash] data hash with data to verify
-      # @param [Class] klass entity type
-      def self.verify_parent_signature(data, klass)
-        pubkey = DiasporaFederation.callbacks.trigger(
-          :fetch_author_public_key_by_entity_guid,
-          klass.get_target_entity_type(data),
-          data[:parent_guid]
-        )
-        raise SignatureVerificationFailed,
-              "failed to fetch public key for author of #{data[:parent_guid]}" if pubkey.nil?
+      def verify_parent_signature
+        pubkey = DiasporaFederation.callbacks.trigger(:fetch_author_public_key_by_entity_guid, target_type, parent_guid)
+
+        raise SignatureVerificationFailed, "failed to fetch public key for author of #{parent_guid}" if pubkey.nil?
         raise SignatureVerificationFailed, "wrong parent_author_signature" unless Signing.verify_signature(
-          data, data[:parent_author_signature], pubkey
+          data, parent_author_signature, pubkey
         )
       end
-      private_class_method :verify_parent_signature
-
-      # Adds signatures to a given hash with the keys of the author and the parent
-      # if the signatures are not in the hash yet and if the keys are available.
-      #
-      # @param [Hash] data hash given for a signing
-      # @param [Class] klass entity type
-      def self.update_signatures!(data, klass)
-        if data[:author_signature].nil?
-          privkey = DiasporaFederation.callbacks.trigger(:fetch_private_key_by_diaspora_id, data[:diaspora_id])
-          data[:author_signature] = Signing.sign_with_key(data, privkey) unless privkey.nil?
-        end
-
-        if data[:parent_author_signature].nil?
-          privkey = DiasporaFederation.callbacks.trigger(
-            :fetch_author_private_key_by_entity_guid,
-            klass.get_target_entity_type(data),
-            data[:parent_guid]
-          )
-          data[:parent_author_signature] = Signing.sign_with_key(data, privkey) unless privkey.nil?
-        end
-      end
     end
   end
 end

lib/diaspora_federation/entity.rb

@@ -35,6 +35,10 @@ module DiasporaFederation
   class Entity
     extend PropertiesDSL
 
+    # the original data hash with which the entity was created
+    # @return [Hash] original data
+    attr_reader :data
+
     # Initializes the Entity with the given attribute hash and freezes the created
     # instance it returns.
     #
@@ -56,6 +60,8 @@ module DiasporaFederation
         raise ArgumentError, "missing required properties: #{missing_props.join(', ')}"
       end
 
+      @data = data
+
       self.class.default_values.merge(data).each do |k, v|
         instance_variable_set("@#{k}", nilify(v)) if setable?(k, v)
       end

lib/diaspora_federation/salmon/xml_payload.rb

@@ -97,11 +97,9 @@ module DiasporaFederation
           end
         }]
 
-        if klass.included_modules.include?(Entities::Relayable)
-          Entities::Relayable.verify_signatures(data, klass)
+        klass.new(data).tap do |entity|
+          entity.verify_signatures if entity.respond_to? :verify_signatures
         end
-
-        klass.new(data)
       end
       private_class_method :populate_entity
 

lib/diaspora_federation/test.rb

@@ -21,10 +21,7 @@ module DiasporaFederation
     # @param [Symbol] factory_name the factory to generate attributes for (normally entity name)
     # @return [Hash] hash with correct signatures
     def self.relayable_attributes_with_signatures(factory_name)
-      klass = FactoryGirl.factory_by_name(factory_name).build_class
-      sort_hash(FactoryGirl.attributes_for(factory_name), klass).tap do |data|
-        DiasporaFederation::Entities::Relayable.update_signatures!(data, klass)
-      end
+      FactoryGirl.build(factory_name).to_signed_h
     end
 
     # Generates attributes for signed retraction entity constructor with correct signatures in it

spec/lib/diaspora_federation/entities/comment_spec.rb

@@ -21,9 +21,9 @@ XML
 
     it_behaves_like "a relayable Entity"
 
-    describe ".get_target_entity_type" do
+    describe "#target_type" do
       it "returns \"Post\" as target type" do
-        expect(described_class.get_target_entity_type(data)).to eq("Post")
+        expect(described_class.new(data).target_type).to eq("Post")
       end
     end
   end

spec/lib/diaspora_federation/entities/like_spec.rb

@@ -22,9 +22,9 @@ XML
 
     it_behaves_like "a relayable Entity"
 
-    describe ".get_target_entity_type" do
+    describe "#target_type" do
       it "returns data[:target_type] as target type" do
-        expect(described_class.get_target_entity_type(data)).to eq(data[:target_type])
+        expect(described_class.new(data).target_type).to eq(data[:target_type])
       end
     end
   end

spec/lib/diaspora_federation/entities/message_spec.rb

@@ -23,9 +23,9 @@ XML
 
     it_behaves_like "a relayable Entity"
 
-    describe ".get_target_entity_type" do
+    describe "#target_type" do
       it "returns \"Conversation\" as target type" do
-        expect(described_class.get_target_entity_type(data)).to eq("Conversation")
+        expect(described_class.new(data).target_type).to eq("Conversation")
       end
     end
   end

spec/lib/diaspora_federation/entities/participation_spec.rb

@@ -21,9 +21,9 @@ XML
 
     it_behaves_like "a relayable Entity"
 
-    describe ".get_target_entity_type" do
+    describe "#target_type" do
       it "returns data[:target_type] as target type" do
-        expect(described_class.get_target_entity_type(data)).to eq(data[:target_type])
+        expect(described_class.new(data).target_type).to eq(data[:target_type])
       end
     end
   end

spec/lib/diaspora_federation/entities/poll_participation_spec.rb

@@ -21,9 +21,9 @@ XML
 
     it_behaves_like "a relayable Entity"
 
-    describe ".get_target_entity_type" do
+    describe "#target_type" do
       it "returns \"Poll\" as target type" do
-        expect(described_class.get_target_entity_type(data)).to eq("Poll")
+        expect(described_class.new(data).target_type).to eq("Poll")
       end
     end
   end

spec/lib/diaspora_federation/entities/relayable_spec.rb

@@ -12,9 +12,15 @@ module DiasporaFederation
 
     class SomeRelayable < Entity
       include Entities::Relayable
+
+      property :diaspora_id, xml_name: :diaspora_handle
+
+      def target_type
+        "Target"
+      end
     end
 
-    describe ".verify_signatures" do
+    describe "#verify_signatures" do
       it "doesn't raise anything if correct data were passed" do
         hash[:author_signature] = Signing.sign_with_key(hash, author_pkey)
         hash[:parent_author_signature] = Signing.sign_with_key(hash, parent_pkey)
@@ -24,14 +30,14 @@ module DiasporaFederation
         ).and_return(author_pkey.public_key)
 
         expect(DiasporaFederation.callbacks).to receive(:trigger).with(
-          :fetch_author_public_key_by_entity_guid, "Post", hash[:parent_guid]
+          :fetch_author_public_key_by_entity_guid, "Target", hash[:parent_guid]
         ).and_return(parent_pkey.public_key)
 
         expect(DiasporaFederation.callbacks).to receive(:trigger).with(
-          :entity_author_is_local?, "Post", hash[:parent_guid]
+          :entity_author_is_local?, "Target", hash[:parent_guid]
         ).and_return(false)
 
-        expect { Entities::Relayable.verify_signatures(hash, SomeRelayable) }.not_to raise_error
+        expect { SomeRelayable.new(hash).verify_signatures }.not_to raise_error
       end
 
       it "raises when no public key for author was fetched" do
@@ -39,9 +45,9 @@ module DiasporaFederation
           :fetch_public_key_by_diaspora_id, anything
         ).and_return(nil)
 
-        expect { Entities::Relayable.verify_signatures(hash, SomeRelayable) }.to raise_error(
-          Entities::Relayable::SignatureVerificationFailed
-        )
+        expect {
+          SomeRelayable.new(hash).verify_signatures
+        }.to raise_error Entities::Relayable::SignatureVerificationFailed
       end
 
       it "raises when bad author signature was passed" do
@@ -51,9 +57,9 @@ module DiasporaFederation
           :fetch_public_key_by_diaspora_id, hash[:diaspora_id]
         ).and_return(author_pkey.public_key)
 
-        expect { Entities::Relayable.verify_signatures(hash, SomeRelayable) }.to raise_error(
-          Entities::Relayable::SignatureVerificationFailed
-        )
+        expect {
+          SomeRelayable.new(hash).verify_signatures
+        }.to raise_error Entities::Relayable::SignatureVerificationFailed
       end
 
       it "raises when no public key for parent author was fetched" do
@@ -64,16 +70,16 @@ module DiasporaFederation
         ).and_return(author_pkey.public_key)
 
         expect(DiasporaFederation.callbacks).to receive(:trigger).with(
-          :fetch_author_public_key_by_entity_guid, "Post", hash[:parent_guid]
+          :fetch_author_public_key_by_entity_guid, "Target", hash[:parent_guid]
         ).and_return(nil)
 
         expect(DiasporaFederation.callbacks).to receive(:trigger).with(
-          :entity_author_is_local?, "Post", hash[:parent_guid]
+          :entity_author_is_local?, "Target", hash[:parent_guid]
         ).and_return(false)
 
-        expect { Entities::Relayable.verify_signatures(hash, SomeRelayable) }.to raise_error(
-          Entities::Relayable::SignatureVerificationFailed
-        )
+        expect {
+          SomeRelayable.new(hash).verify_signatures
+        }.to raise_error Entities::Relayable::SignatureVerificationFailed
       end
 
       it "raises when bad parent author signature was passed" do
@@ -85,16 +91,16 @@ module DiasporaFederation
         ).and_return(author_pkey.public_key)
 
         expect(DiasporaFederation.callbacks).to receive(:trigger).with(
-          :fetch_author_public_key_by_entity_guid, "Post", hash[:parent_guid]
+          :fetch_author_public_key_by_entity_guid, "Target", hash[:parent_guid]
         ).and_return(parent_pkey.public_key)
 
         expect(DiasporaFederation.callbacks).to receive(:trigger).with(
-          :entity_author_is_local?, "Post", hash[:parent_guid]
+          :entity_author_is_local?, "Target", hash[:parent_guid]
         ).and_return(false)
 
-        expect { Entities::Relayable.verify_signatures(hash, SomeRelayable) }.to raise_error(
-          Entities::Relayable::SignatureVerificationFailed
-        )
+        expect {
+          SomeRelayable.new(hash).verify_signatures
+        }.to raise_error Entities::Relayable::SignatureVerificationFailed
       end
 
       it "doesn't raise if parent_author_signature isn't set but we're on upstream federation" do
@@ -106,35 +112,33 @@ module DiasporaFederation
         ).and_return(author_pkey.public_key)
 
         expect(DiasporaFederation.callbacks).to receive(:trigger).with(
-          :entity_author_is_local?, "Post", hash[:parent_guid]
+          :entity_author_is_local?, "Target", hash[:parent_guid]
         ).and_return(true)
 
-        expect { Entities::Relayable.verify_signatures(hash, SomeRelayable) }.not_to raise_error
+        expect { SomeRelayable.new(hash).verify_signatures }.not_to raise_error
       end
     end
 
-    describe ".update_singatures!" do
+    describe "#to_signed_h" do
       it "updates signatures when they were nil and keys were supplied" do
         expect(DiasporaFederation.callbacks).to receive(:trigger).with(
           :fetch_private_key_by_diaspora_id, hash[:diaspora_id]
         ).and_return(author_pkey)
 
         expect(DiasporaFederation.callbacks).to receive(:trigger).with(
-          :fetch_author_private_key_by_entity_guid, "Post", hash[:parent_guid]
+          :fetch_author_private_key_by_entity_guid, "Target", hash[:parent_guid]
         ).and_return(parent_pkey)
 
-        Entities::Relayable.update_signatures!(hash, SomeRelayable)
-        expect(Signing.verify_signature(hash, hash[:author_signature], author_pkey)).to be_truthy
-        expect(Signing.verify_signature(hash, hash[:parent_author_signature], parent_pkey)).to be_truthy
+        signed_hash = SomeRelayable.new(hash).to_signed_h
+
+        expect(Signing.verify_signature(signed_hash, signed_hash[:author_signature], author_pkey)).to be_truthy
+        expect(Signing.verify_signature(signed_hash, signed_hash[:parent_author_signature], parent_pkey)).to be_truthy
       end
 
       it "doesn't change signatures if they are already set" do
-        signatures = {author_signature: "aa", parent_author_signature: "bb"}
-        hash.merge!(signatures)
+        hash.merge!(author_signature: "aa", parent_author_signature: "bb").delete(:some_other_data)
 
-        Entities::Relayable.update_signatures!(hash, SomeRelayable)
-        expect(hash[:author_signature]).to eq(signatures[:author_signature])
-        expect(hash[:parent_author_signature]).to eq(signatures[:parent_author_signature])
+        expect(SomeRelayable.new(hash).to_signed_h).to eq(hash)
       end
 
       it "doesn't change signatures if keys weren't supplied" do
@@ -143,12 +147,13 @@ module DiasporaFederation
         ).and_return(nil)
 
         expect(DiasporaFederation.callbacks).to receive(:trigger).with(
-          :fetch_author_private_key_by_entity_guid, "Post", hash[:parent_guid]
+          :fetch_author_private_key_by_entity_guid, "Target", hash[:parent_guid]
         ).and_return(nil)
 
-        Entities::Relayable.update_signatures!(hash, SomeRelayable)
-        expect(hash[:author_signature]).to eq(nil)
-        expect(hash[:parent_author_signature]).to eq(nil)
+        signed_hash = SomeRelayable.new(hash).to_signed_h
+
+        expect(signed_hash[:author_signature]).to eq(nil)
+        expect(signed_hash[:parent_author_signature]).to eq(nil)
       end
     end
   end

spec/lib/diaspora_federation/salmon/xml_payload_spec.rb

@@ -147,7 +147,7 @@ XML
         it "calls signatures verification on relayable unpack" do
           entity = FactoryGirl.build(:comment_entity)
           payload = Salmon::XmlPayload.pack(entity)
-          expect(Entities::Relayable).to receive(:verify_signatures).once
+          expect(Signing).to receive(:verify_signature).twice.and_call_original
           Salmon::XmlPayload.unpack(payload)
         end
       end