Remove legacy signature ordering

Relayables are now ordered by the order of the properties of the entity.

Related to #26, but still compatible with pods older than 0.6.3.0.

lib/diaspora_federation/entities/comment.rb

@@ -4,10 +4,6 @@ module DiasporaFederation
     #
     # @see Validators::CommentValidator
     class Comment < Entity
-      # Old signature order
-      # @deprecated
-      LEGACY_SIGNATURE_ORDER = %i(guid parent_guid text author).freeze
-
       # The {Comment} parent is a {Post}
       PARENT_TYPE = "Post".freeze
 
@@ -21,6 +17,14 @@ module DiasporaFederation
       #   Comment entity creation time
       #   @return [Time] creation time
       property :created_at, :timestamp, default: -> { Time.now.utc }
+
+      private
+
+      # Remove "created_at" when no order was received.
+      # @deprecated TODO: Remove this, this will break compatibility with pods older than 0.6.3.0.
+      def signature_order
+        super.tap {|order| order.delete(:created_at) unless xml_order }
+      end
     end
   end
 end

lib/diaspora_federation/entities/event_participation.rb

@@ -4,10 +4,6 @@ module DiasporaFederation
     #
     # @see Validators::EventParticipationValidator
     class EventParticipation < Entity
-      # Old signature order
-      # @deprecated
-      LEGACY_SIGNATURE_ORDER = %i(author guid parent_guid status).freeze
-
       # The {EventParticipation} parent is an {Event}
       PARENT_TYPE = "Event".freeze
 

lib/diaspora_federation/entities/like.rb

@@ -4,24 +4,20 @@ module DiasporaFederation
     #
     # @see Validators::LikeValidator
     class Like < Entity
-      # Old signature order
-      # @deprecated
-      LEGACY_SIGNATURE_ORDER = %i(positive guid parent_type parent_guid author).freeze
-
       include Relayable
 
-      # @!attribute [r] positive
-      #   If +true+ set a like, if +false+, set a dislike (dislikes are currently not
-      #   implemented in the diaspora* frontend).
-      #   @return [Boolean] is it a like or a dislike
-      property :positive, :boolean
-
       # @!attribute [r] parent_type
       #   A string describing the type of the parent
       #   Can be "Post" or "Comment" (Comments are currently not implemented in the
       #   diaspora* frontend).
       #   @return [String] parent type
       property :parent_type, :string, xml_name: :target_type
+
+      # @!attribute [r] positive
+      #   If +true+ set a like, if +false+, set a dislike (dislikes are currently not
+      #   implemented in the diaspora* frontend).
+      #   @return [Boolean] is it a like or a dislike
+      property :positive, :boolean
     end
   end
 end

lib/diaspora_federation/entities/poll_participation.rb

@@ -4,10 +4,6 @@ module DiasporaFederation
     #
     # @see Validators::PollParticipationValidator
     class PollParticipation < Entity
-      # Old signature order
-      # @deprecated
-      LEGACY_SIGNATURE_ORDER = %i(guid parent_guid author poll_answer_guid).freeze
-
       # The {PollParticipation} parent is a {Poll}
       PARENT_TYPE = "Poll".freeze
 

lib/diaspora_federation/entities/relayable.rb

@@ -154,7 +154,7 @@ module DiasporaFederation
           prop_names = self.class.class_props.keys.map(&:to_s)
           xml_order.map {|name| prop_names.include?(name) ? name.to_sym : name }
         else
-          self.class::LEGACY_SIGNATURE_ORDER
+          self.class.class_props.keys - %i(author_signature parent_author_signature parent)
         end
       end
 

spec/entities.rb

@@ -58,17 +58,11 @@ module DiasporaFederation
     end
 
     class SomeRelayable < DiasporaFederation::Entity
-      LEGACY_SIGNATURE_ORDER = %i(guid author property parent_guid).freeze
-
       PARENT_TYPE = "Parent".freeze
 
       include Entities::Relayable
 
       property :property, :string
-
-      def parent_type
-        PARENT_TYPE
-      end
     end
   end
 

spec/lib/diaspora_federation/entities/comment_spec.rb

@@ -15,10 +15,10 @@ module DiasporaFederation
 
     let(:xml) { <<-XML }
 <comment>
+  <diaspora_handle>#{data[:author]}</diaspora_handle>
   <guid>#{data[:guid]}</guid>
   <parent_guid>#{parent.guid}</parent_guid>
   <text>#{data[:text]}</text>
-  <diaspora_handle>#{data[:author]}</diaspora_handle>
   <author_signature>#{data[:author_signature]}</author_signature>
   <parent_author_signature>#{data[:parent_author_signature]}</parent_author_signature>
 </comment>
@@ -37,10 +37,10 @@ XML
     "created_at": "#{data[:created_at].iso8601}"
   },
   "property_order": [
+    "author",
     "guid",
     "parent_guid",
-    "text",
+    "text"
-    "author"
   ]
 }
 JSON

spec/lib/diaspora_federation/entities/like_spec.rb

@@ -14,11 +14,11 @@ module DiasporaFederation
 
     let(:xml) { <<-XML }
 <like>
-  <positive>#{data[:positive]}</positive>
-  <guid>#{data[:guid]}</guid>
-  <target_type>#{parent.entity_type}</target_type>
-  <parent_guid>#{parent.guid}</parent_guid>
   <diaspora_handle>#{data[:author]}</diaspora_handle>
+  <guid>#{data[:guid]}</guid>
+  <parent_guid>#{parent.guid}</parent_guid>
+  <target_type>#{parent.entity_type}</target_type>
+  <positive>#{data[:positive]}</positive>
   <author_signature>#{data[:author_signature]}</author_signature>
   <parent_author_signature>#{data[:parent_author_signature]}</parent_author_signature>
 </like>
@@ -33,15 +33,15 @@ XML
     "parent_guid": "#{parent.guid}",
     "author_signature": "#{data[:author_signature]}",
     "parent_author_signature": "#{data[:parent_author_signature]}",
-    "positive": #{data[:positive]},
+    "parent_type": "#{parent.entity_type}",
-    "parent_type": "#{parent.entity_type}"
+    "positive": #{data[:positive]}
   },
   "property_order": [
-    "positive",
+    "author",
     "guid",
-    "parent_type",
     "parent_guid",
-    "author"
+    "parent_type",
+    "positive"
   ]
 }
 JSON

spec/lib/diaspora_federation/entities/poll_participation_spec.rb

@@ -13,9 +13,9 @@ module DiasporaFederation
 
     let(:xml) { <<-XML }
 <poll_participation>
+  <diaspora_handle>#{data[:author]}</diaspora_handle>
   <guid>#{data[:guid]}</guid>
   <parent_guid>#{parent.guid}</parent_guid>
-  <diaspora_handle>#{data[:author]}</diaspora_handle>
   <poll_answer_guid>#{data[:poll_answer_guid]}</poll_answer_guid>
   <author_signature>#{data[:author_signature]}</author_signature>
   <parent_author_signature>#{data[:parent_author_signature]}</parent_author_signature>
@@ -34,9 +34,9 @@ XML
     "poll_answer_guid": "#{data[:poll_answer_guid]}"
   },
   "property_order": [
+    "author",
     "guid",
     "parent_guid",
-    "author",
     "poll_answer_guid"
   ]
 }

spec/lib/diaspora_federation/entities/relayable_spec.rb

@@ -13,7 +13,8 @@ module DiasporaFederation
     let(:hash) { {guid: guid, author: author, parent_guid: parent_guid, parent: local_parent, property: property} }
     let(:hash_with_fake_signatures) { hash.merge!(author_signature: "aa", parent_author_signature: "bb") }
 
-    let(:legacy_signature_data) { "#{guid};#{author};#{property};#{parent_guid}" }
+    let(:signature_order) { %i(author guid parent_guid property) }
+    let(:signature_data) { "#{author};#{guid};#{parent_guid};#{property}" }
 
     describe "#initialize" do
       it "filters signatures from order" do
@@ -25,49 +26,75 @@ module DiasporaFederation
     end
 
     describe "#verify_signatures" do
-      it "doesn't raise anything if correct signatures with legacy-string were passed" do
+      it "doesn't raise anything if correct signatures were passed" do
-        hash[:author_signature] = sign_with_key(author_pkey, legacy_signature_data)
+        hash[:author_signature] = sign_with_key(author_pkey, signature_data)
-        hash[:parent_author_signature] = sign_with_key(parent_pkey, legacy_signature_data)
+        hash[:parent_author_signature] = sign_with_key(parent_pkey, signature_data)
         hash[:parent] = remote_parent
 
         expect_callback(:fetch_public_key, author).and_return(author_pkey.public_key)
         expect_callback(:fetch_public_key, remote_parent.author).and_return(parent_pkey.public_key)
 
-        expect { Entities::SomeRelayable.new(hash).verify_signatures }.not_to raise_error
+        expect { Entities::SomeRelayable.new(hash, signature_order).verify_signatures }.not_to raise_error
+      end
+
+      it "doesn't raise anything if correct signatures with new property were passed" do
+        signature_order = [:author, :guid, :parent_guid, :property, "new_property"]
+        signature_data_with_new_property = "#{author};#{guid};#{parent_guid};#{property};#{new_property}"
+
+        hash[:author_signature] = sign_with_key(author_pkey, signature_data_with_new_property)
+        hash[:parent_author_signature] = sign_with_key(parent_pkey, signature_data_with_new_property)
+        hash[:parent] = remote_parent
+
+        expect_callback(:fetch_public_key, author).and_return(author_pkey.public_key)
+        expect_callback(:fetch_public_key, remote_parent.author).and_return(parent_pkey.public_key)
+
+        expect {
+          Entities::SomeRelayable.new(hash, signature_order, "new_property" => new_property).verify_signatures
+        }.not_to raise_error
       end
 
       it "raises when no public key for author was fetched" do
         expect_callback(:fetch_public_key, anything).and_return(nil)
 
         expect {
-          Entities::SomeRelayable.new(hash).verify_signatures
+          Entities::SomeRelayable.new(hash, signature_order).verify_signatures
         }.to raise_error Entities::Relayable::PublicKeyNotFound
       end
 
-      it "raises when bad author signature was passed" do
+      it "raises when no author signature was passed" do
         hash[:author_signature] = nil
 
         expect_callback(:fetch_public_key, author).and_return(author_pkey.public_key)
 
         expect {
-          Entities::SomeRelayable.new(hash).verify_signatures
+          Entities::SomeRelayable.new(hash, signature_order).verify_signatures
+        }.to raise_error Entities::Relayable::SignatureVerificationFailed
+      end
+
+      it "raises when bad author signature was passed" do
+        hash[:author_signature] = sign_with_key(author_pkey, "bad signed string")
+
+        expect_callback(:fetch_public_key, author).and_return(author_pkey.public_key)
+
+        expect {
+          Entities::SomeRelayable.new(hash, signature_order).verify_signatures
         }.to raise_error Entities::Relayable::SignatureVerificationFailed
       end
 
       it "raises when no public key for parent author was fetched" do
-        hash[:author_signature] = sign_with_key(author_pkey, legacy_signature_data)
+        hash[:author_signature] = sign_with_key(author_pkey, signature_data)
         hash[:parent] = remote_parent
 
         expect_callback(:fetch_public_key, author).and_return(author_pkey.public_key)
         expect_callback(:fetch_public_key, remote_parent.author).and_return(nil)
 
         expect {
-          Entities::SomeRelayable.new(hash).verify_signatures
+          Entities::SomeRelayable.new(hash, signature_order).verify_signatures
         }.to raise_error Entities::Relayable::PublicKeyNotFound
       end
 
-      it "raises when bad parent author signature was passed" do
+      it "raises when no parent author signature was passed" do
-        hash[:author_signature] = sign_with_key(author_pkey, legacy_signature_data)
+        hash[:author_signature] = sign_with_key(author_pkey, signature_data)
         hash[:parent_author_signature] = nil
         hash[:parent] = remote_parent
 
@@ -75,61 +102,31 @@ module DiasporaFederation
         expect_callback(:fetch_public_key, remote_parent.author).and_return(parent_pkey.public_key)
 
         expect {
-          Entities::SomeRelayable.new(hash).verify_signatures
+          Entities::SomeRelayable.new(hash, signature_order).verify_signatures
+        }.to raise_error Entities::Relayable::SignatureVerificationFailed
+      end
+
+      it "raises when bad parent author signature was passed" do
+        hash[:author_signature] = sign_with_key(author_pkey, signature_data)
+        hash[:parent_author_signature] = sign_with_key(parent_pkey, "bad signed string")
+        hash[:parent] = remote_parent
+
+        expect_callback(:fetch_public_key, author).and_return(author_pkey.public_key)
+        expect_callback(:fetch_public_key, remote_parent.author).and_return(parent_pkey.public_key)
+
+        expect {
+          Entities::SomeRelayable.new(hash, signature_order).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
-        hash[:author_signature] = sign_with_key(author_pkey, legacy_signature_data)
+        hash[:author_signature] = sign_with_key(author_pkey, signature_data)
         hash[:parent_author_signature] = nil
         hash[:parent] = local_parent
 
         expect_callback(:fetch_public_key, author).and_return(author_pkey.public_key)
 
-        expect { Entities::SomeRelayable.new(hash).verify_signatures }.not_to raise_error
+        expect { Entities::SomeRelayable.new(hash, signature_order).verify_signatures }.not_to raise_error
-      end
-
-      context "new signatures" do
-        it "doesn't raise anything if correct signatures with new order were passed" do
-          xml_order = %i(author guid parent_guid property)
-          signature_data = "#{author};#{guid};#{parent_guid};#{property}"
-
-          hash[:author_signature] = sign_with_key(author_pkey, signature_data)
-          hash[:parent_author_signature] = sign_with_key(parent_pkey, signature_data)
-          hash[:parent] = remote_parent
-
-          expect_callback(:fetch_public_key, author).and_return(author_pkey.public_key)
-          expect_callback(:fetch_public_key, remote_parent.author).and_return(parent_pkey.public_key)
-
-          expect { Entities::SomeRelayable.new(hash, xml_order).verify_signatures }.not_to raise_error
-        end
-
-        it "doesn't raise anything if correct signatures with new property were passed" do
-          xml_order = [:author, :guid, :parent_guid, :property, "new_property"]
-          signature_data_with_new_property = "#{author};#{guid};#{parent_guid};#{property};#{new_property}"
-
-          hash[:author_signature] = sign_with_key(author_pkey, signature_data_with_new_property)
-          hash[:parent_author_signature] = sign_with_key(parent_pkey, signature_data_with_new_property)
-          hash[:parent] = remote_parent
-
-          expect_callback(:fetch_public_key, author).and_return(author_pkey.public_key)
-          expect_callback(:fetch_public_key, remote_parent.author).and_return(parent_pkey.public_key)
-
-          expect {
-            Entities::SomeRelayable.new(hash, xml_order, "new_property" => new_property).verify_signatures
-          }.not_to raise_error
-        end
-
-        it "raises with legacy-signatures and with new property and order" do
-          hash[:author_signature] = sign_with_key(author_pkey, legacy_signature_data)
-
-          expect_callback(:fetch_public_key, author).and_return(author_pkey.public_key)
-
-          xml_order = [:author, :guid, :parent_guid, :property, "new_property"]
-          expect {
-            Entities::SomeRelayable.new(hash, xml_order, "new_property" => new_property).verify_signatures
-          }.to raise_error Entities::Relayable::SignatureVerificationFailed
-        end
       end
     end
 
@@ -177,8 +174,8 @@ 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, legacy_signature_data)).to be_truthy
+        expect(verify_signature(author_pkey, author_signature, signature_data)).to be_truthy
-        expect(verify_signature(parent_pkey, parent_author_signature, legacy_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
@@ -260,8 +257,8 @@ XML
         end
 
         it "creates Entity with empty 'additional_data' if the xml has only known properties" do
-          hash[:author_signature] = sign_with_key(author_pkey, legacy_signature_data)
+          hash[:author_signature] = sign_with_key(author_pkey, signature_data)
-          hash[:parent_author_signature] = sign_with_key(parent_pkey, legacy_signature_data)
+          hash[:parent_author_signature] = sign_with_key(parent_pkey, signature_data)
 
           xml = Entities::SomeRelayable.new(hash).to_xml
 
@@ -300,11 +297,11 @@ XML
         expect(json).to include_json(property_order: property_order.map(&:to_s))
       end
 
-      it "uses legacy order for filling property_order when no xml_order supplied" do
+      it "uses property order for filling property_order when no xml_order supplied" do
         entity = entity_class.new(hash_with_fake_signatures)
         expect(
           entity.to_json.to_json
-        ).to include_json(property_order: entity_class::LEGACY_SIGNATURE_ORDER.map(&:to_s))
+        ).to include_json(property_order: %w(author guid parent_guid property))
       end
 
       it "adds new unknown elements to the json again" do
@@ -433,15 +430,15 @@ XML
         end
 
         it "creates Entity with empty 'additional_data' if it has only known properties" do
-          property_order = %w(guid author property parent_guid)
+          property_order = %w(author guid parent_guid property)
 
           entity_data = {
             guid:                    guid,
             author:                  author,
             property:                property,
             parent_guid:             parent_guid,
-            author_signature:        sign_with_key(author_pkey, legacy_signature_data),
+            author_signature:        sign_with_key(author_pkey, signature_data),
-            parent_author_signature: sign_with_key(parent_pkey, legacy_signature_data)
+            parent_author_signature: sign_with_key(parent_pkey, signature_data)
           }
 
           entity = Entities::SomeRelayable.from_hash(entity_data, property_order)

spec/support/shared_entity_specs.rb

@@ -106,7 +106,8 @@ shared_examples "a relayable Entity" do
     end
 
     it "computes correct signatures for the entity" do
-      signed_string = described_class::LEGACY_SIGNATURE_ORDER.map {|name| data[name] }.join(";")
+      order = described_class.class_props.keys - %i(author_signature parent_author_signature parent created_at)
+      signed_string = order.map {|name| data[name] }.join(";")
 
       xml = DiasporaFederation::Salmon::XmlPayload.pack(instance)