diff --git a/lib/diaspora_federation/entities/comment.rb b/lib/diaspora_federation/entities/comment.rb index 99c8319..7332a9b 100644 --- a/lib/diaspora_federation/entities/comment.rb +++ b/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 diff --git a/lib/diaspora_federation/entities/event_participation.rb b/lib/diaspora_federation/entities/event_participation.rb index 4c2dc22..efa526d 100644 --- a/lib/diaspora_federation/entities/event_participation.rb +++ b/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 diff --git a/lib/diaspora_federation/entities/like.rb b/lib/diaspora_federation/entities/like.rb index 02324ca..b1b7b2e 100644 --- a/lib/diaspora_federation/entities/like.rb +++ b/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 diff --git a/lib/diaspora_federation/entities/poll_participation.rb b/lib/diaspora_federation/entities/poll_participation.rb index 8ef8f9e..d1ce464 100644 --- a/lib/diaspora_federation/entities/poll_participation.rb +++ b/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 diff --git a/lib/diaspora_federation/entities/relayable.rb b/lib/diaspora_federation/entities/relayable.rb index ce30e69..b1c97bd 100644 --- a/lib/diaspora_federation/entities/relayable.rb +++ b/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 diff --git a/spec/entities.rb b/spec/entities.rb index e452fa9..8200935 100644 --- a/spec/entities.rb +++ b/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 diff --git a/spec/lib/diaspora_federation/entities/comment_spec.rb b/spec/lib/diaspora_federation/entities/comment_spec.rb index f6b32d9..f9016a2 100644 --- a/spec/lib/diaspora_federation/entities/comment_spec.rb +++ b/spec/lib/diaspora_federation/entities/comment_spec.rb @@ -15,10 +15,10 @@ module DiasporaFederation let(:xml) { <<-XML } + #{data[:author]} #{data[:guid]} #{parent.guid} #{data[:text]} - #{data[:author]} #{data[:author_signature]} #{data[:parent_author_signature]} @@ -37,10 +37,10 @@ XML "created_at": "#{data[:created_at].iso8601}" }, "property_order": [ + "author", "guid", "parent_guid", - "text", - "author" + "text" ] } JSON diff --git a/spec/lib/diaspora_federation/entities/like_spec.rb b/spec/lib/diaspora_federation/entities/like_spec.rb index 08fbc5b..4d17754 100644 --- a/spec/lib/diaspora_federation/entities/like_spec.rb +++ b/spec/lib/diaspora_federation/entities/like_spec.rb @@ -14,11 +14,11 @@ module DiasporaFederation let(:xml) { <<-XML } - #{data[:positive]} - #{data[:guid]} - #{parent.entity_type} - #{parent.guid} #{data[:author]} + #{data[:guid]} + #{parent.guid} + #{parent.entity_type} + #{data[:positive]} #{data[:author_signature]} #{data[:parent_author_signature]} @@ -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 diff --git a/spec/lib/diaspora_federation/entities/poll_participation_spec.rb b/spec/lib/diaspora_federation/entities/poll_participation_spec.rb index b63f261..270277a 100644 --- a/spec/lib/diaspora_federation/entities/poll_participation_spec.rb +++ b/spec/lib/diaspora_federation/entities/poll_participation_spec.rb @@ -13,9 +13,9 @@ module DiasporaFederation let(:xml) { <<-XML } + #{data[:author]} #{data[:guid]} #{parent.guid} - #{data[:author]} #{data[:poll_answer_guid]} #{data[:author_signature]} #{data[: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" ] } diff --git a/spec/lib/diaspora_federation/entities/relayable_spec.rb b/spec/lib/diaspora_federation/entities/relayable_spec.rb index 0c02c4e..5016527 100644 --- a/spec/lib/diaspora_federation/entities/relayable_spec.rb +++ b/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 - hash[:author_signature] = sign_with_key(author_pkey, legacy_signature_data) - hash[:parent_author_signature] = sign_with_key(parent_pkey, legacy_signature_data) + it "doesn't raise anything if correct signatures were passed" do + 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).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 - hash[:author_signature] = sign_with_key(author_pkey, legacy_signature_data) + it "raises when no parent author signature was passed" do + 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 - 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 + expect { Entities::SomeRelayable.new(hash, signature_order).verify_signatures }.not_to raise_error 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(parent_pkey, parent_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, 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[:parent_author_signature] = sign_with_key(parent_pkey, legacy_signature_data) + hash[:author_signature] = sign_with_key(author_pkey, 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), - parent_author_signature: sign_with_key(parent_pkey, legacy_signature_data) + author_signature: sign_with_key(author_pkey, signature_data), + parent_author_signature: sign_with_key(parent_pkey, signature_data) } entity = Entities::SomeRelayable.from_hash(entity_data, property_order) diff --git a/spec/support/shared_entity_specs.rb b/spec/support/shared_entity_specs.rb index 2191dd1..b66cd27 100644 --- a/spec/support/shared_entity_specs.rb +++ b/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)