From 10c09752d22a79b51f4312f2ae378b453e04f1b4 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Sun, 21 Feb 2016 18:15:01 +0100 Subject: [PATCH] xml_order and additional_xml_elements only for Relayables --- lib/diaspora_federation/entities/relayable.rb | 54 ++++++++++++ lib/diaspora_federation/entity.rb | 52 +++-------- .../entities/relayable_spec.rb | 88 ++++++++++++++++++- spec/lib/diaspora_federation/entity_spec.rb | 39 -------- 4 files changed, 150 insertions(+), 83 deletions(-) diff --git a/lib/diaspora_federation/entities/relayable.rb b/lib/diaspora_federation/entities/relayable.rb index 0e77b33..2360690 100644 --- a/lib/diaspora_federation/entities/relayable.rb +++ b/lib/diaspora_federation/entities/relayable.rb @@ -10,6 +10,14 @@ module DiasporaFederation # digest instance used for signing DIGEST = OpenSSL::Digest::SHA256.new + # order from the parsed xml for signature + # @return [Array] order from xml + attr_reader :xml_order + + # additional properties from parsed xml + # @return [Hash] additional xml elements + attr_reader :additional_xml_elements + # on inclusion of this module the required properties for a relayable are added to the object that includes it # # @!attribute [r] author @@ -49,6 +57,21 @@ module DiasporaFederation property :author_signature, default: nil property :parent_author_signature, default: nil end + + entity.extend ParseXML + end + + # Initializes a new relayable Entity with order and additional xml elements + # + # @param [Hash] data entity data + # @param [Array] xml_order order from xml + # @param [Hash] additional_xml_elements additional xml elements + # @see DiasporaFederation::Entity#initialize + def initialize(data, xml_order=nil, additional_xml_elements={}) + @xml_order = xml_order + @additional_xml_elements = additional_xml_elements + + super(data) end # verifies the signatures (+author_signature+ and +parent_author_signature+ if needed) @@ -147,6 +170,37 @@ module DiasporaFederation signature_order.map {|name| data[name] }.join(";") end + # override class methods from {Entity} to parse the xml + module ParseXML + private + + # @param [Nokogiri::XML::Element] root_node xml nodes + # @return [Entity] instance + def populate_entity(root_node) + # Use all known properties to build the Entity (entity_data). All additional xml elements + # are respected and attached to a hash as string (additional_xml_elements). It also remembers + # the order of the xml-nodes (xml_order). This is needed to support receiving objects from + # the future versions of Diaspora, where new elements may have been added. + entity_data = {} + additional_xml_elements = {} + + xml_order = root_node.element_children.map do |child| + xml_name = child.name + property = find_property_for_xml_name(xml_name) + + if property + entity_data[property] = parse_element_from_node(xml_name, class_props[property], root_node) + property + else + additional_xml_elements[xml_name] = child.text + xml_name + end + end + + new(entity_data, xml_order, additional_xml_elements).tap(&:verify_signatures) + end + end + # Exception raised when creating the author_signature failes, because the private key was not found class AuthorPrivateKeyNotFound < RuntimeError end diff --git a/lib/diaspora_federation/entity.rb b/lib/diaspora_federation/entity.rb index 85923f8..7e12bae 100644 --- a/lib/diaspora_federation/entity.rb +++ b/lib/diaspora_federation/entity.rb @@ -35,12 +35,6 @@ module DiasporaFederation class Entity extend PropertiesDSL - attr_reader :xml_order - - # additional properties from parsed xml - # @return [Hash] additional xml elements - attr_reader :additional_xml_elements - # Initializes the Entity with the given attribute hash and freezes the created # instance it returns. # @@ -54,9 +48,8 @@ module DiasporaFederation # {PropertiesDSL#entity}) get discarded silently. # # @param [Hash] data entity data - # @param [Hash] additional_xml_elements additional xml elements # @return [Entity] new instance - def initialize(data, xml_order=nil, additional_xml_elements={}) + def initialize(data) raise ArgumentError, "expected a Hash" unless data.is_a?(Hash) entity_data = self.class.resolv_aliases(data) missing_props = self.class.missing_props(entity_data) @@ -64,9 +57,6 @@ module DiasporaFederation raise ArgumentError, "missing required properties: #{missing_props.join(', ')}" end - @xml_order = xml_order - @additional_xml_elements = additional_xml_elements - self.class.default_values.merge(entity_data).each do |name, value| instance_variable_set("@#{name}", nilify(value)) if setable?(name, value) end @@ -209,44 +199,25 @@ module DiasporaFederation # @param [Nokogiri::XML::Element] root_node xml nodes # @return [Entity] instance def self.populate_entity(root_node) - # Use all known properties to build the Entity (entity_data). All additional xml elements - # are respected and attached to a hash as string (additional_xml_elements). It is intended - # to build a hash invariable of an Entity definition, in order to support receiving objects - # from the future versions of Diaspora, where new elements may have been added. - entity_data = {} - xml_order = [] - additional_xml_elements = {} + entity_data = Hash[class_props.map {|name, type| + [name, parse_element_from_node(name, type, root_node)] + }] - root_node.element_children.each do |child| - xml_name = child.name - property = find_property_for_xml_name(xml_name) - - if property - entity_data[property] = parse_element_from_node(class_props[property], xml_name, root_node) - xml_order << property - else - additional_xml_elements[xml_name] = child.text - xml_order << xml_name - end - end - - new(entity_data, xml_order, additional_xml_elements).tap do |entity| - entity.verify_signatures if entity.respond_to? :verify_signatures - end + new(entity_data) end private_class_method :populate_entity + # @param [String] name property name to parse # @param [Class] type target type to parse - # @param [String] xml_name xml tag to parse - # @param [Nokogiri::XML::Element] node XML node to parse + # @param [Nokogiri::XML::Element] root_node XML node to parse # @return [Object] parsed data - def self.parse_element_from_node(type, xml_name, node) + def self.parse_element_from_node(name, type, root_node) if type == String - parse_string_from_node(xml_name, node) + parse_string_from_node(name, root_node) elsif type.instance_of?(Array) - parse_array_from_node(type.first, node) + parse_array_from_node(type.first, root_node) elsif type.ancestors.include?(Entity) - parse_entity_from_node(type, node) + parse_entity_from_node(type, root_node) end end private_class_method :parse_element_from_node @@ -258,6 +229,7 @@ module DiasporaFederation # @return [String] data def self.parse_string_from_node(name, root_node) node = root_node.xpath(name.to_s) + node = root_node.xpath(xml_names[name].to_s) if node.empty? node.first.text if node.any? end private_class_method :parse_string_from_node diff --git a/spec/lib/diaspora_federation/entities/relayable_spec.rb b/spec/lib/diaspora_federation/entities/relayable_spec.rb index e63586f..99747b5 100644 --- a/spec/lib/diaspora_federation/entities/relayable_spec.rb +++ b/spec/lib/diaspora_federation/entities/relayable_spec.rb @@ -24,15 +24,15 @@ module DiasporaFederation end end + def sign_with_key(privkey, signature_data) + Base64.strict_encode64(privkey.sign(OpenSSL::Digest::SHA256.new, signature_data)) + end + def verify_signature(pubkey, signature, signed_string) pubkey.verify(OpenSSL::Digest::SHA256.new, Base64.decode64(signature), signed_string) end describe "#verify_signatures" do - def sign_with_key(privkey, signature_data) - Base64.strict_encode64(privkey.sign(OpenSSL::Digest::SHA256.new, signature_data)) - end - 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) @@ -285,5 +285,85 @@ XML expect(xml.at_xpath("parent_author_signature").text).to eq("") end end + + describe ".from_xml" do + context "parsing" do + before do + expect(DiasporaFederation.callbacks).to receive(:trigger).with( + :fetch_public_key_by_diaspora_id, author + ).and_return(author_pkey.public_key) + + expect(DiasporaFederation.callbacks).to receive(:trigger).with( + :fetch_author_public_key_by_entity_guid, "Parent", parent_guid + ).and_return(parent_pkey.public_key) + + expect(DiasporaFederation.callbacks).to receive(:trigger).with( + :entity_author_is_local?, "Parent", parent_guid + ).and_return(false) + end + + let(:new_signature_data) { "#{author};#{guid};#{parent_guid};#{new_property};#{property}" } + let(:new_xml) { + <<-XML + + #{author} + #{guid} + #{parent_guid} + #{new_property} + #{property} + #{sign_with_key(author_pkey, new_signature_data)} + #{sign_with_key(parent_pkey, new_signature_data)} + +XML + } + + it "doesn't drop unknown properties" do + entity = SomeRelayable.from_xml(Nokogiri::XML::Document.parse(new_xml).root) + + expect(entity).to be_an_instance_of SomeRelayable + expect(entity.property).to eq(property) + expect(entity.additional_xml_elements).to eq( + "new_property" => new_property + ) + end + + it "hand over the order in the xml to the instance" do + entity = SomeRelayable.from_xml(Nokogiri::XML::Document.parse(new_xml).root) + + expect(entity.xml_order).to eq( + [:author, :guid, :parent_guid, "new_property", :property, :author_signature, :parent_author_signature] + ) + end + + it "creates Entity with empty 'additional_xml_elements' 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) + + xml = SomeRelayable.new(hash).to_xml + + entity = SomeRelayable.from_xml(xml) + + expect(entity).to be_an_instance_of SomeRelayable + expect(entity.property).to eq(property) + expect(entity.additional_xml_elements).to be_empty + end + end + + context "relayable signature verification feature support" do + it "calls signatures verification on relayable unpack" do + hash.merge!(author_signature: "aa", parent_author_signature: "bb") + + xml = SomeRelayable.new(hash).to_xml + + expect(DiasporaFederation.callbacks).to receive(:trigger).with( + :fetch_public_key_by_diaspora_id, author + ).and_return(author_pkey.public_key) + + expect { + SomeRelayable.from_xml(xml) + }.to raise_error DiasporaFederation::Entities::Relayable::SignatureVerificationFailed + end + end + end end end diff --git a/spec/lib/diaspora_federation/entity_spec.rb b/spec/lib/diaspora_federation/entity_spec.rb index cb71554..c9eec4a 100644 --- a/spec/lib/diaspora_federation/entity_spec.rb +++ b/spec/lib/diaspora_federation/entity_spec.rb @@ -168,45 +168,6 @@ XML expect(entity.test).to eq("asdf") expect(entity.qwer).to eq("qwer") end - - it "doesn't drop unknown properties" do - xml = <<-XML - - some value - asdf - another value - -XML - - entity = Entities::TestEntity.from_xml(Nokogiri::XML::Document.parse(xml).root) - - expect(entity).to be_an_instance_of Entities::TestEntity - expect(entity.test).to eq("asdf") - expect(entity.additional_xml_elements).to eq( - "a_prop_from_newer_diaspora_version" => "some value", - "some_random_property" => "another value" - ) - end - - it "creates Entity with nil 'additional_xml_elements' if the xml has only known properties" do - entity = Entities::TestEntity.from_xml(entity_xml) - - expect(entity).to be_an_instance_of Entities::TestEntity - expect(entity.test).to eq("asdf") - expect(entity.additional_xml_elements).to be_empty - end - end - - context "relayable signature verification feature support" do - it "calls signatures verification on relayable unpack" do - entity = FactoryGirl.build(:comment_entity, author: alice.diaspora_id) - entity_xml = entity.to_xml - entity_xml.at_xpath("author_signature").content = nil - - expect { - Entities::Comment.from_xml(entity_xml) - }.to raise_error DiasporaFederation::Entities::Relayable::SignatureVerificationFailed - end end context "nested entities" do