From 0163963849cd07895e3a8849e1aaa110abeb28e2 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Tue, 29 Oct 2019 02:28:45 +0100 Subject: [PATCH 1/8] Remove old Slap and EncryptedSlap and cleanup legacy receive Closes #30 --- .../diaspora_federation/receive_controller.rb | 43 +------ lib/diaspora_federation.rb | 2 - .../federation/receiver.rb | 22 +--- lib/diaspora_federation/salmon.rb | 2 - .../salmon/encrypted_slap.rb | 113 ------------------ lib/diaspora_federation/salmon/exceptions.rb | 20 ---- lib/diaspora_federation/salmon/slap.rb | 59 --------- .../receive_controller_spec.rb | 65 +--------- .../federation/receiver_spec.rb | 30 ----- .../salmon/encrypted_slap_spec.rb | 59 --------- .../diaspora_federation/salmon/slap_spec.rb | 60 ---------- 11 files changed, 13 insertions(+), 462 deletions(-) delete mode 100644 lib/diaspora_federation/salmon/encrypted_slap.rb delete mode 100644 lib/diaspora_federation/salmon/slap.rb delete mode 100644 spec/lib/diaspora_federation/salmon/encrypted_slap_spec.rb delete mode 100644 spec/lib/diaspora_federation/salmon/slap_spec.rb diff --git a/app/controllers/diaspora_federation/receive_controller.rb b/app/controllers/diaspora_federation/receive_controller.rb index a477dfd..a9568b5 100644 --- a/app/controllers/diaspora_federation/receive_controller.rb +++ b/app/controllers/diaspora_federation/receive_controller.rb @@ -5,18 +5,14 @@ require_dependency "diaspora_federation/application_controller" module DiasporaFederation # This controller processes receiving messages. class ReceiveController < ApplicationController - before_action :check_for_xml - # Receives public messages # # POST /receive/public def public - legacy = request.content_type != "application/magic-envelope+xml" - - data = data_for_public_message(legacy) + data = request.body.read logger.debug data - DiasporaFederation.callbacks.trigger(:queue_public_receive, data, legacy) + DiasporaFederation.callbacks.trigger(:queue_public_receive, data) head :accepted end @@ -25,43 +21,12 @@ module DiasporaFederation # # POST /receive/users/:guid def private - legacy = request.content_type != "application/json" - - data = data_for_private_message(legacy) + data = request.body.read logger.debug data - success = DiasporaFederation.callbacks.trigger(:queue_private_receive, params[:guid], data, legacy) + success = DiasporaFederation.callbacks.trigger(:queue_private_receive, params[:guid], data) head success ? :accepted : :not_found end - - private - - # Checks the xml parameter for legacy salmon slaps - # @deprecated - def check_for_xml - legacy_request = request.content_type.nil? || request.content_type == "application/x-www-form-urlencoded" - head :unprocessable_entity if params[:xml].nil? && legacy_request - end - - def data_for_public_message(legacy) - if legacy - logger.info "received a public salmon slap" - CGI.unescape(params[:xml]) - else - logger.info "received a public magic envelope" - request.body.read - end - end - - def data_for_private_message(legacy) - if legacy - logger.info "received a private salmon slap for #{params[:guid]}" - CGI.unescape(params[:xml]) - else - logger.info "received a private encrypted magic envelope for #{params[:guid]}" - request.body.read - end - end end end diff --git a/lib/diaspora_federation.rb b/lib/diaspora_federation.rb index ec2b036..f5079d6 100644 --- a/lib/diaspora_federation.rb +++ b/lib/diaspora_federation.rb @@ -184,13 +184,11 @@ module DiasporaFederation # queue_public_receive # Queue a public salmon xml to process in background # @param [String] data salmon slap xml or magic envelope xml - # @param [Boolean] legacy true if it is a legacy salmon slap, false if it is a magic envelope xml # # queue_private_receive # Queue a private salmon xml to process in background # @param [String] guid guid of the receiver person # @param [String] data salmon slap xml or encrypted magic envelope json - # @param [Boolean] legacy true if it is a legacy salmon slap, false if it is a encrypted magic envelope json # @return [Boolean] true if successful, false if the user was not found # # receive_entity diff --git a/lib/diaspora_federation/federation/receiver.rb b/lib/diaspora_federation/federation/receiver.rb index 4b6eb76..7552631 100644 --- a/lib/diaspora_federation/federation/receiver.rb +++ b/lib/diaspora_federation/federation/receiver.rb @@ -8,14 +8,9 @@ module DiasporaFederation # Receive a public message # @param [String] data message to receive - # @param [Boolean] legacy use old slap parser - def self.receive_public(data, legacy=false) - magic_env = if legacy - Salmon::Slap.from_xml(data) - else - magic_env_xml = Nokogiri::XML(data).root - Salmon::MagicEnvelope.unenvelop(magic_env_xml) - end + def self.receive_public(data) + magic_env_xml = Nokogiri::XML(data).root + magic_env = Salmon::MagicEnvelope.unenvelop(magic_env_xml) Public.new(magic_env).receive rescue => e # rubocop:disable Style/RescueStandardError logger.error "failed to receive public message: #{e.class}: #{e.message}" @@ -28,16 +23,11 @@ module DiasporaFederation # @param [OpenSSL::PKey::RSA] recipient_private_key recipient private key to decrypt the message # @param [Object] recipient_id the identifier to persist the entity for the correct user, # see +receive_entity+ callback - # @param [Boolean] legacy use old slap parser - def self.receive_private(data, recipient_private_key, recipient_id, legacy=false) + def self.receive_private(data, recipient_private_key, recipient_id) raise ArgumentError, "no recipient key provided" unless recipient_private_key.instance_of?(OpenSSL::PKey::RSA) - magic_env = if legacy - Salmon::EncryptedSlap.from_xml(data, recipient_private_key) - else - magic_env_xml = Salmon::EncryptedMagicEnvelope.decrypt(data, recipient_private_key) - Salmon::MagicEnvelope.unenvelop(magic_env_xml) - end + magic_env_xml = Salmon::EncryptedMagicEnvelope.decrypt(data, recipient_private_key) + magic_env = Salmon::MagicEnvelope.unenvelop(magic_env_xml) Private.new(magic_env, recipient_id).receive rescue => e # rubocop:disable Style/RescueStandardError logger.error "failed to receive private message for #{recipient_id}: #{e.class}: #{e.message}" diff --git a/lib/diaspora_federation/salmon.rb b/lib/diaspora_federation/salmon.rb index 0fc3ef9..0267598 100644 --- a/lib/diaspora_federation/salmon.rb +++ b/lib/diaspora_federation/salmon.rb @@ -16,5 +16,3 @@ require "diaspora_federation/salmon/exceptions" require "diaspora_federation/salmon/xml_payload" require "diaspora_federation/salmon/magic_envelope" require "diaspora_federation/salmon/encrypted_magic_envelope" -require "diaspora_federation/salmon/slap" -require "diaspora_federation/salmon/encrypted_slap" diff --git a/lib/diaspora_federation/salmon/encrypted_slap.rb b/lib/diaspora_federation/salmon/encrypted_slap.rb deleted file mode 100644 index 23da393..0000000 --- a/lib/diaspora_federation/salmon/encrypted_slap.rb +++ /dev/null @@ -1,113 +0,0 @@ -# frozen_string_literal: true - -require "json" - -module DiasporaFederation - module Salmon - # +EncryptedSlap+ provides class methods for generating and parsing encrypted - # Slaps. (In principle the same as {Slap}, but with encryption.) - # - # The basic encryption mechanism used here is based on the knowledge that - # asymmetrical encryption is slow and symmetrical encryption is fast. Keeping in - # mind that a message we want to de-/encrypt may greatly vary in length, - # performance considerations must play a part of this scheme. - # - # A diaspora*-flavored encrypted magic-enveloped XML message looks like the following: - # - # - # - # {encrypted_header} - # {magic_envelope with encrypted data} - # - # - # The encrypted header is encoded in JSON like this (when in plain text): - # - # { - # "aes_key" => "...", - # "ciphertext" => "..." - # } - # - # +aes_key+ is encrypted using the recipients public key, and contains the AES - # +key+ and +iv+ used to encrypt the +ciphertext+ also encoded as JSON. - # - # { - # "key" => "...", - # "iv" => "..." - # } - # - # +ciphertext+, once decrypted, contains the +author_id+, +aes_key+ and +iv+ - # relevant to the decryption of the data in the magic_envelope and the - # verification of its signature. - # - # The decrypted cyphertext has this XML structure: - # - # - # {iv} - # {aes_key} - # {author_id} - # - # - # Finally, before decrypting the magic envelope payload, the signature should - # first be verified. - # - # @example Parsing a Salmon Slap - # recipient_privkey = however_you_retrieve_the_recipients_private_key() - # entity = EncryptedSlap.from_xml(slap_xml, recipient_privkey).payload - # - # @deprecated - class EncryptedSlap < Slap - # Creates a {MagicEnvelope} instance from the data within the given XML string - # containing an encrypted payload. - # - # @param [String] slap_xml encrypted Salmon xml - # @param [OpenSSL::PKey::RSA] privkey recipient private_key for decryption - # - # @return [MagicEnvelope] magic envelope instance with payload and sender - # - # @raise [ArgumentError] if any of the arguments is of the wrong type - # @raise [MissingHeader] if the +encrypted_header+ element is missing in the XML - # @raise [MissingMagicEnvelope] if the +me:env+ element is missing in the XML - def self.from_xml(slap_xml, privkey) - raise ArgumentError unless slap_xml.instance_of?(String) && privkey.instance_of?(OpenSSL::PKey::RSA) - - doc = Nokogiri::XML(slap_xml) - - header_elem = doc.at_xpath("d:diaspora/d:encrypted_header", Slap::NS) - raise MissingHeader if header_elem.nil? - - header = header_data(header_elem.content, privkey) - sender = header[:author_id] - cipher_params = {key: Base64.decode64(header[:aes_key]), iv: Base64.decode64(header[:iv])} - - MagicEnvelope.unenvelop(magic_env_from_doc(doc), sender, cipher_params) - end - - # Decrypts and reads the data from the encrypted XML header - # @param [String] data base64 encoded, encrypted header data - # @param [OpenSSL::PKey::RSA] privkey private key for decryption - # @return [Hash] { iv: "...", aes_key: "...", author_id: "..." } - private_class_method def self.header_data(data, privkey) - header_elem = decrypt_header(data, privkey) - raise InvalidHeader unless header_elem.name == "decrypted_header" - - iv = header_elem.at_xpath("iv").content - key = header_elem.at_xpath("aes_key").content - author_id = header_elem.at_xpath("author_id").content - - {iv: iv, aes_key: key, author_id: author_id} - end - - # Decrypts the xml header - # @param [String] data base64 encoded, encrypted header data - # @param [OpenSSL::PKey::RSA] privkey private key for decryption - # @return [Nokogiri::XML::Element] header xml document - private_class_method def self.decrypt_header(data, privkey) - cipher_header = JSON.parse(Base64.decode64(data)) - key = JSON.parse(privkey.private_decrypt(Base64.decode64(cipher_header["aes_key"]))) - - xml = AES.decrypt(cipher_header["ciphertext"], Base64.decode64(key["key"]), Base64.decode64(key["iv"])) - Nokogiri::XML(xml).root - end - end - end -end diff --git a/lib/diaspora_federation/salmon/exceptions.rb b/lib/diaspora_federation/salmon/exceptions.rb index 07061be..b9bde07 100644 --- a/lib/diaspora_federation/salmon/exceptions.rb +++ b/lib/diaspora_federation/salmon/exceptions.rb @@ -2,26 +2,6 @@ module DiasporaFederation module Salmon - # Raised, if the element containing the Magic Envelope is missing from the XML - # @deprecated - class MissingMagicEnvelope < RuntimeError - end - - # Raised, if the element containing the author is empty. - # @deprecated - class MissingAuthor < RuntimeError - end - - # Raised, if the element containing the header is missing from the XML - # @deprecated - class MissingHeader < RuntimeError - end - - # Raised, if the decrypted header has an unexpected XML structure - # @deprecated - class InvalidHeader < RuntimeError - end - # Raised, if failed to fetch the public key of the sender of the received message class SenderKeyNotFound < RuntimeError end diff --git a/lib/diaspora_federation/salmon/slap.rb b/lib/diaspora_federation/salmon/slap.rb deleted file mode 100644 index 242337d..0000000 --- a/lib/diaspora_federation/salmon/slap.rb +++ /dev/null @@ -1,59 +0,0 @@ -# frozen_string_literal: true - -module DiasporaFederation - module Salmon - # +Slap+ provides class methods to create unencrypted Slap XML from payload - # data and parse incoming XML into a Slap instance. - # - # A diaspora* flavored magic-enveloped XML message looks like the following: - # - # - # - #
- # {author} - #
- # {magic_envelope} - # - # - # @example Parsing a Salmon Slap - # entity = Slap.from_xml(slap_xml).payload - # - # @deprecated - class Slap - # Namespaces - NS = {d: Salmon::XMLNS, me: MagicEnvelope::XMLNS}.freeze - - # Parses an unencrypted Salmon XML string and returns a new instance of - # {MagicEnvelope} with the XML data. - # - # @param [String] slap_xml Salmon XML - # - # @return [MagicEnvelope] magic envelope instance with payload and sender - # - # @raise [ArgumentError] if the argument is not a String - # @raise [MissingAuthor] if the +author_id+ element is missing from the XML - # @raise [MissingMagicEnvelope] if the +me:env+ element is missing from the XML - def self.from_xml(slap_xml) - raise ArgumentError unless slap_xml.instance_of?(String) - - doc = Nokogiri::XML(slap_xml) - - author_elem = doc.at_xpath("d:diaspora/d:header/d:author_id", Slap::NS) - raise MissingAuthor if author_elem.nil? || author_elem.content.empty? - - sender = author_elem.content - - MagicEnvelope.unenvelop(magic_env_from_doc(doc), sender) - end - - # Parses the magic envelop from the document. - # - # @param [Nokogiri::XML::Document] doc Salmon XML Document - private_class_method def self.magic_env_from_doc(doc) - doc.at_xpath("d:diaspora/me:env", Slap::NS).tap do |env| - raise MissingMagicEnvelope if env.nil? - end - end - end - end -end diff --git a/spec/controllers/diaspora_federation/receive_controller_spec.rb b/spec/controllers/diaspora_federation/receive_controller_spec.rb index 0d5b684..b0aaaaa 100644 --- a/spec/controllers/diaspora_federation/receive_controller_spec.rb +++ b/spec/controllers/diaspora_federation/receive_controller_spec.rb @@ -5,32 +5,6 @@ module DiasporaFederation routes { DiasporaFederation::Engine.routes } describe "POST #public" do - context "legacy salmon slap" do - it "returns a 422 if no xml is passed" do - post :public - expect(response.code).to eq("422") - end - - it "returns a 422 if no xml is passed with content-type application/x-www-form-urlencoded" do - @request.env["CONTENT_TYPE"] = "application/x-www-form-urlencoded" - post :public - expect(response.code).to eq("422") - end - - it "returns a 202 if queued correctly" do - expect_callback(:queue_public_receive, "", true) - - post :public, params: {xml: ""} - expect(response.code).to eq("202") - end - - it "unescapes the xml before sending it to the callback" do - expect_callback(:queue_public_receive, "", true) - - post :public, params: {xml: CGI.escape("")} - end - end - context "magic envelope" do before do Mime::Type.register("application/magic-envelope+xml", :magic_envelope) @@ -38,7 +12,7 @@ module DiasporaFederation end it "returns a 202 if queued correctly" do - expect_callback(:queue_public_receive, "", false) + expect_callback(:queue_public_receive, "") post :public, body: +"" expect(response.code).to eq("202") @@ -47,39 +21,6 @@ module DiasporaFederation end describe "POST #private" do - context "legacy salmon slap" do - it "return a 404 if not queued successfully (unknown user guid)" do - expect_callback(:queue_private_receive, "any-guid", "", true).and_return(false) - - post :private, params: {guid: "any-guid", xml: ""} - expect(response.code).to eq("404") - end - - it "returns a 422 if no xml is passed" do - post :private, params: {guid: "any-guid"} - expect(response.code).to eq("422") - end - - it "returns a 422 if no xml is passed with content-type application/x-www-form-urlencoded" do - @request.env["CONTENT_TYPE"] = "application/x-www-form-urlencoded" - post :private, params: {guid: "any-guid"} - expect(response.code).to eq("422") - end - - it "returns a 202 if the callback returned true" do - expect_callback(:queue_private_receive, "any-guid", "", true).and_return(true) - - post :private, params: {guid: "any-guid", xml: ""} - expect(response.code).to eq("202") - end - - it "unescapes the xml before sending it to the callback" do - expect_callback(:queue_private_receive, "any-guid", "", true).and_return(true) - - post :private, params: {guid: "any-guid", xml: CGI.escape("")} - end - end - context "encrypted magic envelope" do before do @request.env["CONTENT_TYPE"] = "application/json" @@ -87,7 +28,7 @@ module DiasporaFederation it "return a 404 if not queued successfully (unknown user guid)" do expect_callback( - :queue_private_receive, "any-guid", "{\"aes_key\": \"key\", \"encrypted_magic_envelope\": \"env\"}", false + :queue_private_receive, "any-guid", "{\"aes_key\": \"key\", \"encrypted_magic_envelope\": \"env\"}" ).and_return(false) post :private, @@ -98,7 +39,7 @@ module DiasporaFederation it "returns a 202 if the callback returned true" do expect_callback( - :queue_private_receive, "any-guid", "{\"aes_key\": \"key\", \"encrypted_magic_envelope\": \"env\"}", false + :queue_private_receive, "any-guid", "{\"aes_key\": \"key\", \"encrypted_magic_envelope\": \"env\"}" ).and_return(true) post :private, diff --git a/spec/lib/diaspora_federation/federation/receiver_spec.rb b/spec/lib/diaspora_federation/federation/receiver_spec.rb index b5b2d33..5df17c0 100644 --- a/spec/lib/diaspora_federation/federation/receiver_spec.rb +++ b/spec/lib/diaspora_federation/federation/receiver_spec.rb @@ -23,21 +23,6 @@ module DiasporaFederation described_class.receive_public(data) end - it "parses the entity with legacy slap receiver" do - expect_callback(:fetch_public_key, post.author).and_return(sender_key) - - data = generate_legacy_salmon_slap(post, post.author, sender_key) - - expect_callback(:receive_entity, kind_of(Entities::StatusMessage), post.author, nil) do |_, entity| - expect(entity.guid).to eq(post.guid) - expect(entity.author).to eq(post.author) - expect(entity.text).to eq(post.text) - expect(entity.public).to eq("true") - end - - described_class.receive_public(data, true) - end - it "redirects exceptions from the receiver" do expect { described_class.receive_public("") @@ -64,21 +49,6 @@ module DiasporaFederation described_class.receive_private(data, recipient_key, 1234) end - it "parses the entity with legacy slap receiver" do - expect_callback(:fetch_public_key, post.author).and_return(sender_key) - - data = generate_legacy_encrypted_salmon_slap(post, post.author, sender_key, recipient_key) - - expect_callback(:receive_entity, kind_of(Entities::StatusMessage), post.author, 1234) do |_, entity| - expect(entity.guid).to eq(post.guid) - expect(entity.author).to eq(post.author) - expect(entity.text).to eq(post.text) - expect(entity.public).to eq("false") - end - - described_class.receive_private(data, recipient_key, 1234, true) - end - it "raises when recipient private key is not available" do magic_env = Salmon::MagicEnvelope.new(post, post.author).envelop(sender_key) data = Salmon::EncryptedMagicEnvelope.encrypt(magic_env, recipient_key.public_key) diff --git a/spec/lib/diaspora_federation/salmon/encrypted_slap_spec.rb b/spec/lib/diaspora_federation/salmon/encrypted_slap_spec.rb deleted file mode 100644 index 348dcfb..0000000 --- a/spec/lib/diaspora_federation/salmon/encrypted_slap_spec.rb +++ /dev/null @@ -1,59 +0,0 @@ -# frozen_string_literal: true - -module DiasporaFederation - describe Salmon::EncryptedSlap do - let(:sender) { "user_test@diaspora.example.tld" } - let(:privkey) { OpenSSL::PKey::RSA.generate(512) } # use small key for speedy specs - let(:recipient_key) { OpenSSL::PKey::RSA.generate(1024) } # use small key for speedy specs - let(:payload) { Entities::TestEntity.new(test: "qwertzuiop") } - let(:slap_xml) { generate_legacy_encrypted_salmon_slap(payload, sender, privkey, recipient_key.public_key) } - - describe ".from_xml" do - context "sanity" do - it "accepts correct params" do - expect_callback(:fetch_public_key, sender).and_return(privkey.public_key) - - expect { - Salmon::EncryptedSlap.from_xml(slap_xml, recipient_key) - }.not_to raise_error - end - - it "raises an error when the params have a wrong type" do - [1234, false, :symbol, payload, privkey].each do |val| - expect { - Salmon::EncryptedSlap.from_xml(val, val) - }.to raise_error ArgumentError - end - end - - it "verifies the existence of 'encrypted_header'" do - faulty_xml = <<~XML - - - XML - expect { - Salmon::EncryptedSlap.from_xml(faulty_xml, recipient_key) - }.to raise_error Salmon::MissingHeader - end - - it "verifies the existence of a magic envelope" do - faulty_xml = <<~XML - - - - XML - expect(Salmon::EncryptedSlap).to receive(:header_data).and_return(aes_key: "", iv: "", author_id: "") - expect { - Salmon::EncryptedSlap.from_xml(faulty_xml, recipient_key) - }.to raise_error Salmon::MissingMagicEnvelope - end - end - - context "generated instance" do - it_behaves_like "a MagicEnvelope instance" do - subject { Salmon::EncryptedSlap.from_xml(slap_xml, recipient_key) } - end - end - end - end -end diff --git a/spec/lib/diaspora_federation/salmon/slap_spec.rb b/spec/lib/diaspora_federation/salmon/slap_spec.rb deleted file mode 100644 index a79729d..0000000 --- a/spec/lib/diaspora_federation/salmon/slap_spec.rb +++ /dev/null @@ -1,60 +0,0 @@ -# frozen_string_literal: true - -module DiasporaFederation - describe Salmon::Slap do - let(:sender) { "test_user@pod.somedomain.tld" } - let(:privkey) { OpenSSL::PKey::RSA.generate(512) } # use small key for speedy specs - let(:payload) { Entities::TestEntity.new(test: "qwertzuiop") } - let(:slap_xml) { generate_legacy_salmon_slap(payload, sender, privkey) } - - describe ".from_xml" do - context "sanity" do - it "accepts salmon xml as param" do - expect_callback(:fetch_public_key, sender).and_return(privkey.public_key) - - expect { - Salmon::Slap.from_xml(slap_xml) - }.not_to raise_error - end - - it "raises an error when the param has a wrong type" do - [1234, false, :symbol, payload, privkey].each do |val| - expect { - Salmon::Slap.from_xml(val) - }.to raise_error ArgumentError - end - end - - it "verifies the existence of an author_id" do - faulty_xml = <<~XML - -
- - XML - expect { - Salmon::Slap.from_xml(faulty_xml) - }.to raise_error Salmon::MissingAuthor - end - - it "verifies the existence of a magic envelope" do - faulty_xml = <<~XML - -
- #{sender} -
- - XML - expect { - Salmon::Slap.from_xml(faulty_xml) - }.to raise_error Salmon::MissingMagicEnvelope - end - end - - context "generated instance" do - it_behaves_like "a MagicEnvelope instance" do - subject { Salmon::Slap.from_xml(slap_xml) } - end - end - end - end -end From 0ccd15cd53b921a85af93c8ee25cd5c22962cbe1 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Tue, 29 Oct 2019 02:54:07 +0100 Subject: [PATCH 2/8] Remove legacy xml unwrap code Closes #28 --- lib/diaspora_federation/salmon.rb | 1 - .../salmon/magic_envelope.rb | 3 +- lib/diaspora_federation/salmon/xml_payload.rb | 43 -------- spec/integration/comment_integration_spec.rb | 100 ++---------------- .../entities/account_migration_spec.rb | 3 +- .../entities/conversation_spec.rb | 3 +- .../entities/event_spec.rb | 3 +- .../entities/photo_spec.rb | 3 +- .../entities/profile_spec.rb | 3 +- .../entities/status_message_spec.rb | 3 +- .../salmon/xml_payload_spec.rb | 60 ----------- spec/support/shared_entity_specs.rb | 2 +- 12 files changed, 21 insertions(+), 206 deletions(-) delete mode 100644 lib/diaspora_federation/salmon/xml_payload.rb delete mode 100644 spec/lib/diaspora_federation/salmon/xml_payload_spec.rb diff --git a/lib/diaspora_federation/salmon.rb b/lib/diaspora_federation/salmon.rb index 0267598..6bcd6dc 100644 --- a/lib/diaspora_federation/salmon.rb +++ b/lib/diaspora_federation/salmon.rb @@ -13,6 +13,5 @@ require "base64" require "diaspora_federation/salmon/aes" require "diaspora_federation/salmon/exceptions" -require "diaspora_federation/salmon/xml_payload" require "diaspora_federation/salmon/magic_envelope" require "diaspora_federation/salmon/encrypted_magic_envelope" diff --git a/lib/diaspora_federation/salmon/magic_envelope.rb b/lib/diaspora_federation/salmon/magic_envelope.rb index fcc3f53..fafa1cd 100644 --- a/lib/diaspora_federation/salmon/magic_envelope.rb +++ b/lib/diaspora_federation/salmon/magic_envelope.rb @@ -114,7 +114,8 @@ module DiasporaFederation logger.debug "unenvelop message from #{sender}:\n#{data}" - new(XmlPayload.unpack(Nokogiri::XML(data).root), sender) + xml = Nokogiri::XML(data).root + new(Entity.entity_class(xml.name).from_xml(xml), sender) end private diff --git a/lib/diaspora_federation/salmon/xml_payload.rb b/lib/diaspora_federation/salmon/xml_payload.rb deleted file mode 100644 index 32ea453..0000000 --- a/lib/diaspora_federation/salmon/xml_payload.rb +++ /dev/null @@ -1,43 +0,0 @@ -# frozen_string_literal: true - -module DiasporaFederation - module Salmon - # +XmlPayload+ provides methods to wrap a XML-serialized {Entity} inside a - # common XML structure that will become the payload for federation messages. - # - # The wrapper looks like so: - # - # - # {data} - # - # - # - # (The +post+ element is there for historic reasons...) - # @deprecated - module XmlPayload - # Extracts the Entity XML from the wrapping XML structure, parses the entity - # XML and returns a new instance of the Entity that was packed inside the - # given payload. - # - # @param [Nokogiri::XML::Element] xml payload XML root node - # @return [Entity] re-constructed Entity instance - # @raise [ArgumentError] if the argument is not an - # {http://www.rubydoc.info/gems/nokogiri/Nokogiri/XML/Element Nokogiri::XML::Element} - # @raise [UnknownEntity] if the class for the entity contained inside the - # XML can't be found - def self.unpack(xml) - raise ArgumentError, "only Nokogiri::XML::Element allowed" unless xml.instance_of?(Nokogiri::XML::Element) - - data = xml_wrapped?(xml) ? xml.at_xpath("post/*[1]") : xml - - Entity.entity_class(data.name).from_xml(data) - end - - # @param [Nokogiri::XML::Element] element - private_class_method def self.xml_wrapped?(element) - (element.name == "XML" && !element.at_xpath("post").nil? && - !element.at_xpath("post").children.empty?) - end - end - end -end diff --git a/spec/integration/comment_integration_spec.rb b/spec/integration/comment_integration_spec.rb index f713699..2966631 100644 --- a/spec/integration/comment_integration_spec.rb +++ b/spec/integration/comment_integration_spec.rb @@ -65,20 +65,6 @@ module DiasporaFederation ) } - let(:legacy_format_comment_xml_alice) { <<~XML } - - - - e21589b0b41101333b870f77ba60fa73 - 9e269ae0b41201333b8c0f77ba60fa73 - XU5X1uqTh8SY6JMG9uhEVR5Rg7FURV6lpRwl/HYOu6DJ3Hd9tpA2aSFFibUxxsMgJXKNrrc5SykrrEdTiQoEei+j0QqZf3B5R7r84qgK7M46KazwIpqRPwVl2MdA/0DdQyYJLA/oavNj1nwll9vtR87M7e/C94qG6P+iQTMBQzo= - - this is a very informative comment - alice@pod-a.org - - - - XML let(:new_format_comment_xml_alice) { <<~XML } alice@pod-a.org @@ -101,20 +87,6 @@ module DiasporaFederation XML - let(:legacy_format_comment_xml_bob) { <<~XML } - - - - e21589b0b41101333b870f77ba60fa73 - 9e269ae0b41201333b8c0f77ba60fa73 - this is a very informative comment - alice@pod-a.org - XU5X1uqTh8SY6JMG9uhEVR5Rg7FURV6lpRwl/HYOu6DJ3Hd9tpA2aSFFibUxxsMgJXKNrrc5SykrrEdTiQoEei+j0QqZf3B5R7r84qgK7M46KazwIpqRPwVl2MdA/0DdQyYJLA/oavNj1nwll9vtR87M7e/C94qG6P+iQTMBQzo= - QqWSdwpb+/dcJUxuKKVe7aiz1NivXzlIdWZ71xyrxnhFxFYd+7EIittyTcp1cVehjg96pwDbn++P/rWyCffqenWu025DHvUfSmQkC93Z0dX6r3OIUlZqwEggtOdbunybiE++F3BVsGt5wC4YbAESB5ZFuhFVhBXh1X+EaZ/qoKo= - - - - XML let(:legacy_order_new_format_comment_xml_bob) { <<~XML } e21589b0b41101333b870f77ba60fa73 @@ -125,20 +97,6 @@ module DiasporaFederation QqWSdwpb+/dcJUxuKKVe7aiz1NivXzlIdWZ71xyrxnhFxFYd+7EIittyTcp1cVehjg96pwDbn++P/rWyCffqenWu025DHvUfSmQkC93Z0dX6r3OIUlZqwEggtOdbunybiE++F3BVsGt5wC4YbAESB5ZFuhFVhBXh1X+EaZ/qoKo= XML - let(:new_order_legacy_format_comment_xml_bob) { <<~XML } - - - - alice@pod-a.org - e21589b0b41101333b870f77ba60fa73 - 9e269ae0b41201333b8c0f77ba60fa73 - this is a very informative comment - SQbLeqsEpFmSl74G1fFJXKQcsq6jp5B2ZjmfEOF/LbBccYP2oZEyEqOq18K3Fa71OYTp6Nddb38hCmHWWHvnGUltGfxKBnQ0WHafJUi40VM4VmeRoU8cac6m+1hslwe5SNmK6oh47EV3mRCXlgGGjLIrw7iEwjKL2g9x1gkNp2s= - hWsagsczmZD6d36d6MFdTt3hKAdnRtupSIU6464G2kkMJ+WlExxMgbF6kWR+jVCBTeKipWCYK3Arnj0YkuIZM9d14bJGVMTsW/ZzNfJ69bXZhsyawI8dPnZnLVydo+hU/XmGJBEuh2TOj9Emq6/HCYiWzPTF5qhYAtyJ1oxJ4Yk= - - - - XML let(:new_format_comment_xml_bob) { <<~XML } alice@pod-a.org @@ -149,21 +107,6 @@ module DiasporaFederation hWsagsczmZD6d36d6MFdTt3hKAdnRtupSIU6464G2kkMJ+WlExxMgbF6kWR+jVCBTeKipWCYK3Arnj0YkuIZM9d14bJGVMTsW/ZzNfJ69bXZhsyawI8dPnZnLVydo+hU/XmGJBEuh2TOj9Emq6/HCYiWzPTF5qhYAtyJ1oxJ4Yk= XML - let(:legacy_format_new_data_comment_xml_bob) { <<~XML } - - - - alice@pod-a.org - e21589b0b41101333b870f77ba60fa73 - 9e269ae0b41201333b8c0f77ba60fa73 - this is a very informative comment - foobar - SFYXSvCX/DhTFiOUALp2Nf1kfNkGKXrnoBPikAyhaIogGydVBm+8tIlu1U/vsnpyKO3yfC3JReJ00/UBd4J16VO1IxStntq8NUqbSv4me5A/6kdK9Xg6eYbXrqQGm8fUQ5Xuh2UzeB71p7SVySXX3OZHVe0dvHCxH/lsfSDpEjc= - NxXuEUVeXwUMR77osIbaNlp2oB3bpl8rBEFgQoO6cnoN5ewDbiGADK0x6EhcmJptjwhGVcZiNJNpq7k3/pjJtKaH++3ToCAtcuZoIKwPDsneLnjPhVjE2GXM1TiZKwoHrq41qSp/8Vl5UPbtC6sPiOzIvPKaILXUG8XCiVWuB0M= - - - - XML let(:new_data_comment_xml_bob) { <<~XML } alice@pod-a.org @@ -191,7 +134,7 @@ module DiasporaFederation expect_callback(:fetch_related_entity, "Post", parent_guid).and_return(parent) xml = Nokogiri::XML(new_data_comment_xml_alice).root - Salmon::XmlPayload.unpack(xml).to_xml + Entity.entity_class(xml.name).from_xml(xml).to_xml end end @@ -202,21 +145,15 @@ module DiasporaFederation expect_callback(:fetch_related_entity, "Post", parent_guid).and_return(parent) end - it "relays legacy order" do - xml = Nokogiri::XML(legacy_format_comment_xml_alice).root - entity = Salmon::XmlPayload.unpack(xml) - expect(entity.to_xml.to_xml).to eq(legacy_order_new_format_comment_xml_bob.strip) - end - it "relays new order" do xml = Nokogiri::XML(new_format_comment_xml_alice).root - entity = Salmon::XmlPayload.unpack(xml) + entity = Entity.entity_class(xml.name).from_xml(xml) expect(entity.to_xml.to_xml).to eq(new_format_comment_xml_bob.strip) end it "relays new data" do xml = Nokogiri::XML(new_data_comment_xml_alice).root - entity = Salmon::XmlPayload.unpack(xml) + entity = Entity.entity_class(xml.name).from_xml(xml) expect(entity.to_xml.to_xml).to eq(new_data_comment_xml_bob.strip) end end @@ -229,25 +166,9 @@ module DiasporaFederation expect_callback(:fetch_related_entity, "Post", parent_guid).and_return(parent) end - it "parses legacy format" do - xml = Nokogiri::XML(legacy_format_comment_xml_bob).root - entity = Salmon::XmlPayload.unpack(xml) - - expect(entity.author).to eq(author) - expect(entity.text).to eq(text) - end - it "parses legacy order with new xml format" do xml = Nokogiri::XML(legacy_order_new_format_comment_xml_bob).root - entity = Salmon::XmlPayload.unpack(xml) - - expect(entity.author).to eq(author) - expect(entity.text).to eq(text) - end - - it "parses new order with legacy xml format" do - xml = Nokogiri::XML(new_order_legacy_format_comment_xml_bob).root - entity = Salmon::XmlPayload.unpack(xml) + entity = Entity.entity_class(xml.name).from_xml(xml) expect(entity.author).to eq(author) expect(entity.text).to eq(text) @@ -255,24 +176,15 @@ module DiasporaFederation it "parses new xml format" do xml = Nokogiri::XML(new_format_comment_xml_bob).root - entity = Salmon::XmlPayload.unpack(xml) + entity = Entity.entity_class(xml.name).from_xml(xml) expect(entity.author).to eq(author) expect(entity.text).to eq(text) end - it "parses new data with legacy xml format" do - xml = Nokogiri::XML(legacy_format_new_data_comment_xml_bob).root - entity = Salmon::XmlPayload.unpack(xml) - - expect(entity.author).to eq(author) - expect(entity.text).to eq(text) - expect(entity.additional_data["new_data"]).to eq(new_data) - end - it "parses new data with new xml format" do xml = Nokogiri::XML(new_data_comment_xml_bob).root - entity = Salmon::XmlPayload.unpack(xml) + entity = Entity.entity_class(xml.name).from_xml(xml) expect(entity.author).to eq(author) expect(entity.text).to eq(text) diff --git a/spec/lib/diaspora_federation/entities/account_migration_spec.rb b/spec/lib/diaspora_federation/entities/account_migration_spec.rb index cf79218..423ab35 100644 --- a/spec/lib/diaspora_federation/entities/account_migration_spec.rb +++ b/spec/lib/diaspora_federation/entities/account_migration_spec.rb @@ -215,7 +215,8 @@ module DiasporaFederation it "fails validation on parsing" do expect { - DiasporaFederation::Salmon::XmlPayload.unpack(Nokogiri::XML(xml).root) + parsed_xml = Nokogiri::XML(xml).root + Entity.entity_class(parsed_xml.name).from_xml(parsed_xml) }.to raise_error Entity::ValidationError end end diff --git a/spec/lib/diaspora_federation/entities/conversation_spec.rb b/spec/lib/diaspora_federation/entities/conversation_spec.rb index 9522873..ace9e08 100644 --- a/spec/lib/diaspora_federation/entities/conversation_spec.rb +++ b/spec/lib/diaspora_federation/entities/conversation_spec.rb @@ -58,7 +58,8 @@ module DiasporaFederation XML - parsed_instance = DiasporaFederation::Salmon::XmlPayload.unpack(Nokogiri::XML(minimal_xml).root) + parsed_xml = Nokogiri::XML(minimal_xml).root + parsed_instance = Entity.entity_class(parsed_xml.name).from_xml(parsed_xml) expect(parsed_instance.messages).to eq([]) end end diff --git a/spec/lib/diaspora_federation/entities/event_spec.rb b/spec/lib/diaspora_federation/entities/event_spec.rb index 78568ef..8b892a9 100644 --- a/spec/lib/diaspora_federation/entities/event_spec.rb +++ b/spec/lib/diaspora_federation/entities/event_spec.rb @@ -43,7 +43,8 @@ module DiasporaFederation XML - parsed_instance = DiasporaFederation::Salmon::XmlPayload.unpack(Nokogiri::XML(minimal_xml).root) + parsed_xml = Nokogiri::XML(minimal_xml).root + parsed_instance = Entity.entity_class(parsed_xml.name).from_xml(parsed_xml) expect(parsed_instance.end).to be_nil expect(parsed_instance.all_day).to be_falsey expect(parsed_instance.timezone).to be_nil diff --git a/spec/lib/diaspora_federation/entities/photo_spec.rb b/spec/lib/diaspora_federation/entities/photo_spec.rb index 92d55e9..b6804c0 100644 --- a/spec/lib/diaspora_federation/entities/photo_spec.rb +++ b/spec/lib/diaspora_federation/entities/photo_spec.rb @@ -62,7 +62,8 @@ module DiasporaFederation XML - parsed_instance = DiasporaFederation::Salmon::XmlPayload.unpack(Nokogiri::XML(minimal_xml).root) + parsed_xml = Nokogiri::XML(minimal_xml).root + parsed_instance = Entity.entity_class(parsed_xml.name).from_xml(parsed_xml) expect(parsed_instance.public).to be_falsey expect(parsed_instance.text).to be_nil end diff --git a/spec/lib/diaspora_federation/entities/profile_spec.rb b/spec/lib/diaspora_federation/entities/profile_spec.rb index 4cc7d59..ef594b2 100644 --- a/spec/lib/diaspora_federation/entities/profile_spec.rb +++ b/spec/lib/diaspora_federation/entities/profile_spec.rb @@ -63,7 +63,8 @@ module DiasporaFederation XML - parsed_instance = DiasporaFederation::Salmon::XmlPayload.unpack(Nokogiri::XML(minimal_xml).root) + parsed_xml = Nokogiri::XML(minimal_xml).root + parsed_instance = Entity.entity_class(parsed_xml.name).from_xml(parsed_xml) expect(parsed_instance.full_name).to be_nil expect(parsed_instance.first_name).to be_nil expect(parsed_instance.last_name).to be_nil diff --git a/spec/lib/diaspora_federation/entities/status_message_spec.rb b/spec/lib/diaspora_federation/entities/status_message_spec.rb index 9424030..bd64130 100644 --- a/spec/lib/diaspora_federation/entities/status_message_spec.rb +++ b/spec/lib/diaspora_federation/entities/status_message_spec.rb @@ -136,7 +136,8 @@ module DiasporaFederation XML - parsed_instance = DiasporaFederation::Salmon::XmlPayload.unpack(Nokogiri::XML(minimal_xml).root) + parsed_xml = Nokogiri::XML(minimal_xml).root + parsed_instance = Entity.entity_class(parsed_xml.name).from_xml(parsed_xml) expect(parsed_instance.photos).to eq([]) expect(parsed_instance.location).to be_nil expect(parsed_instance.poll).to be_nil diff --git a/spec/lib/diaspora_federation/salmon/xml_payload_spec.rb b/spec/lib/diaspora_federation/salmon/xml_payload_spec.rb deleted file mode 100644 index 180949b..0000000 --- a/spec/lib/diaspora_federation/salmon/xml_payload_spec.rb +++ /dev/null @@ -1,60 +0,0 @@ -# frozen_string_literal: true - -module DiasporaFederation - describe Salmon::XmlPayload do - let(:entity) { Entities::TestEntity.new(test: "asdf") } - let(:entity_xml) { <<~XML.strip } - - - - asdf - - - - XML - - describe ".unpack" do - context "sanity" do - it "expects an Nokogiri::XML::Element as param" do - expect { - Salmon::XmlPayload.unpack(Nokogiri::XML(entity_xml).root) - }.not_to raise_error - end - - it "raises and error when the param is not an Nokogiri::XML::Element" do - ["asdf", 1234, true, :test, entity].each do |val| - expect { - Salmon::XmlPayload.unpack(val) - }.to raise_error ArgumentError, "only Nokogiri::XML::Element allowed" - end - end - end - - context "returned object" do - subject { Salmon::XmlPayload.unpack(Nokogiri::XML(entity_xml).root) } - - it "#to_h should match entity.to_h" do - expect(subject.to_h).to eq(entity.to_h) - end - - it "returns an entity instance of the original class" do - expect(subject).to be_an_instance_of Entities::TestEntity - expect(subject.test).to eq("asdf") - end - - it "allows unwrapped entities" do - xml = <<~XML - - asdf - - XML - - entity = Salmon::XmlPayload.unpack(Nokogiri::XML(xml).root) - - expect(entity).to be_an_instance_of Entities::TestEntity - expect(entity.test).to eq("asdf") - end - end - end - end -end diff --git a/spec/support/shared_entity_specs.rb b/spec/support/shared_entity_specs.rb index 4c7ad42..4d1e256 100644 --- a/spec/support/shared_entity_specs.rb +++ b/spec/support/shared_entity_specs.rb @@ -67,7 +67,7 @@ shared_examples "an XML Entity" do |ignored_props=[]| context "parsing" do it "reads its own output" do packed_xml = instance.to_xml - parsed_instance = DiasporaFederation::Salmon::XmlPayload.unpack(packed_xml) + parsed_instance = DiasporaFederation::Entity.entity_class(packed_xml.name).from_xml(packed_xml) check_entity(instance, parsed_instance, ignored_props) end From a082bcebff8c5ddc2a7c98453b10a9dd4f65e5b9 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Tue, 29 Oct 2019 03:15:30 +0100 Subject: [PATCH 3/8] Remove old SignedRetraction and RelayableRetraction Closes #27 --- lib/diaspora_federation/entities.rb | 2 - .../entities/relayable_retraction.rb | 68 ------------------- .../entities/signed_retraction.rb | 45 ------------ .../entities/relayable_retraction_spec.rb | 46 ------------- .../entities/signed_retraction_spec.rb | 39 ----------- 5 files changed, 200 deletions(-) delete mode 100644 lib/diaspora_federation/entities/relayable_retraction.rb delete mode 100644 lib/diaspora_federation/entities/signed_retraction.rb delete mode 100644 spec/lib/diaspora_federation/entities/relayable_retraction_spec.rb delete mode 100644 spec/lib/diaspora_federation/entities/signed_retraction_spec.rb diff --git a/lib/diaspora_federation/entities.rb b/lib/diaspora_federation/entities.rb index d29388b..763a790 100644 --- a/lib/diaspora_federation/entities.rb +++ b/lib/diaspora_federation/entities.rb @@ -49,5 +49,3 @@ require "diaspora_federation/entities/retraction" # deprecated require "diaspora_federation/entities/request" -require "diaspora_federation/entities/signed_retraction" -require "diaspora_federation/entities/relayable_retraction" diff --git a/lib/diaspora_federation/entities/relayable_retraction.rb b/lib/diaspora_federation/entities/relayable_retraction.rb deleted file mode 100644 index d0365f1..0000000 --- a/lib/diaspora_federation/entities/relayable_retraction.rb +++ /dev/null @@ -1,68 +0,0 @@ -# frozen_string_literal: true - -module DiasporaFederation - module Entities - # This entity represents a claim of deletion of a previously federated - # relayable entity. ({Entities::Comment}, {Entities::Like}) - # - # There are two cases of federation of the RelayableRetraction. - # Retraction from the dowstream object owner is when an author of the - # relayable (e.g. Comment) deletes it themself. In this case only target_author_signature - # is filled and a retraction is sent to the commented post's author. Here - # the upstream object owner signs it with the parent's author key, puts - # the signature in parent_author_signature and sends it to other pods where - # other participating people are present. This is the second case - retraction - # from the upstream object owner. - # Retraction from the upstream object owner can also be performed by the - # upstream object owner themself - they have a right to delete comments on their posts. - # In any case in the retraction by the upstream author target_author_signature - # is not checked, only parent_author_signature is checked. - # - # @see Validators::RelayableRetractionValidator - # @deprecated will be replaced with {Entities::Retraction} - class RelayableRetraction < Entity - # @!attribute [r] parent_author_signature - # Contains a signature of the entity using the private key of the author of a parent post. - # This signature is mandatory only when federating from an upstream author to the subscribers. - # @see Relayable#parent_author_signature - # @return [String] parent author signature - property :parent_author_signature, :string, default: nil - - # @!attribute [r] target_guid - # Guid of a relayable to be deleted - # @see Comment#guid - # @return [String] target guid - property :target_guid, :string - - # @!attribute [r] target_type - # A string describing a type of the target - # @see Retraction#target_type - # @return [String] target type - property :target_type, :string - - # @!attribute [r] author - # The diaspora* ID of the person who deletes a relayable - # @see Person#author - # @return [String] diaspora* ID - property :author, :string, xml_name: :sender_handle - - # @!attribute [r] target_author_signature - # Contains a signature of the entity using the private key of the - # author of a federated relayable entity. ({Entities::Comment}, {Entities::Like}) - # This signature is mandatory only when federation from the subscriber to an upstream - # author is done. - # @see Relayable#author_signature - # @return [String] target author signature - property :target_author_signature, :string, default: nil - - def initialize(*) - raise "Sending RelayableRetraction is not supported anymore! Use Retraction instead!" - end - - # @return [Retraction] instance - def self.from_hash(hash) - Retraction.from_hash(hash) - end - end - end -end diff --git a/lib/diaspora_federation/entities/signed_retraction.rb b/lib/diaspora_federation/entities/signed_retraction.rb deleted file mode 100644 index 77e474c..0000000 --- a/lib/diaspora_federation/entities/signed_retraction.rb +++ /dev/null @@ -1,45 +0,0 @@ -# frozen_string_literal: true - -module DiasporaFederation - module Entities - # This entity represents a claim of deletion of a previously federated - # entity of post type. ({Entities::StatusMessage}) - # - # @see Validators::SignedRetractionValidator - # @deprecated will be replaced with {Entities::Retraction} - class SignedRetraction < Entity - # @!attribute [r] target_guid - # Guid of a post to be deleted - # @see Retraction#target_guid - # @return [String] target guid - property :target_guid, :string - - # @!attribute [r] target_type - # A string describing the type of the target - # @see Retraction#target_type - # @return [String] target type - property :target_type, :string - - # @!attribute [r] author - # The diaspora* ID of the person who deletes a post - # @see Person#author - # @return [String] diaspora* ID - property :author, :string, xml_name: :sender_handle - - # @!attribute [r] author_signature - # Contains a signature of the entity using the private key of the author of a post - # This signature is mandatory. - # @return [String] author signature - property :target_author_signature, :string, default: nil - - def initialize(*) - raise "Sending SignedRetraction is not supported anymore! Use Retraction instead!" - end - - # @return [Retraction] instance - def self.from_hash(hash) - Retraction.from_hash(hash) - end - end - end -end diff --git a/spec/lib/diaspora_federation/entities/relayable_retraction_spec.rb b/spec/lib/diaspora_federation/entities/relayable_retraction_spec.rb deleted file mode 100644 index 6bf22d6..0000000 --- a/spec/lib/diaspora_federation/entities/relayable_retraction_spec.rb +++ /dev/null @@ -1,46 +0,0 @@ -# frozen_string_literal: true - -module DiasporaFederation - describe Entities::RelayableRetraction do - let(:target) { Fabricate(:comment, author: bob) } - let(:target_entity) { - Fabricate( - :related_entity, - author: bob.diaspora_id, - parent: Fabricate(:related_entity, author: alice.diaspora_id) - ) - } - let(:data) { {author: alice.diaspora_id, target_guid: target.guid, target_type: target.entity_type} } - - let(:xml) { <<~XML } - - - #{data[:target_guid]} - #{data[:target_type]} - #{data[:author]} - - - XML - - describe "#initialize" do - it "raises because it is not supported anymore" do - expect { - Entities::RelayableRetraction.new(data) - }.to raise_error RuntimeError, - "Sending RelayableRetraction is not supported anymore! Use Retraction instead!" - end - end - - context "parse retraction" do - it "parses the xml as a retraction" do - expect(Entities::Retraction).to receive(:fetch_target).and_return(target_entity) - retraction = Entities::RelayableRetraction.from_xml(Nokogiri::XML(xml).root) - expect(retraction).to be_a(Entities::Retraction) - expect(retraction.author).to eq(data[:author]) - expect(retraction.target_guid).to eq(data[:target_guid]) - expect(retraction.target_type).to eq(data[:target_type]) - expect(retraction.target).to eq(target_entity) - end - end - end -end diff --git a/spec/lib/diaspora_federation/entities/signed_retraction_spec.rb b/spec/lib/diaspora_federation/entities/signed_retraction_spec.rb deleted file mode 100644 index d9d9513..0000000 --- a/spec/lib/diaspora_federation/entities/signed_retraction_spec.rb +++ /dev/null @@ -1,39 +0,0 @@ -# frozen_string_literal: true - -module DiasporaFederation - describe Entities::SignedRetraction do - let(:target) { Fabricate(:post, author: alice) } - let(:target_entity) { Fabricate(:related_entity, author: alice.diaspora_id) } - let(:data) { {author: alice.diaspora_id, target_guid: target.guid, target_type: target.entity_type} } - - let(:xml) { <<~XML } - - #{data[:target_guid]} - #{data[:target_type]} - #{data[:author]} - - - XML - - describe "#initialize" do - it "raises because it is not supported anymore" do - expect { - Entities::SignedRetraction.new(data) - }.to raise_error RuntimeError, - "Sending SignedRetraction is not supported anymore! Use Retraction instead!" - end - end - - context "parse retraction" do - it "parses the xml as a retraction" do - expect(Entities::Retraction).to receive(:fetch_target).and_return(target_entity) - retraction = Entities::SignedRetraction.from_xml(Nokogiri::XML(xml).root) - expect(retraction).to be_a(Entities::Retraction) - expect(retraction.author).to eq(data[:author]) - expect(retraction.target_guid).to eq(data[:target_guid]) - expect(retraction.target_type).to eq(data[:target_type]) - expect(retraction.target).to eq(target_entity) - end - end - end -end From edfcc7886ddd963fc1221df1960c8ceeac5bbd8f Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Tue, 29 Oct 2019 03:17:35 +0100 Subject: [PATCH 4/8] Remove old Request entity Closes #32 --- lib/diaspora_federation/entities.rb | 3 -- lib/diaspora_federation/entities/request.rb | 33 ------------------- .../entities/request_spec.rb | 33 ------------------- 3 files changed, 69 deletions(-) delete mode 100644 lib/diaspora_federation/entities/request.rb delete mode 100644 spec/lib/diaspora_federation/entities/request_spec.rb diff --git a/lib/diaspora_federation/entities.rb b/lib/diaspora_federation/entities.rb index 763a790..c7f33c0 100644 --- a/lib/diaspora_federation/entities.rb +++ b/lib/diaspora_federation/entities.rb @@ -46,6 +46,3 @@ require "diaspora_federation/entities/message" require "diaspora_federation/entities/conversation" require "diaspora_federation/entities/retraction" - -# deprecated -require "diaspora_federation/entities/request" diff --git a/lib/diaspora_federation/entities/request.rb b/lib/diaspora_federation/entities/request.rb deleted file mode 100644 index 6627f36..0000000 --- a/lib/diaspora_federation/entities/request.rb +++ /dev/null @@ -1,33 +0,0 @@ -# frozen_string_literal: true - -module DiasporaFederation - module Entities - # This entity represents a sharing request for a user. A user issues it - # when they start sharing with another user. - # - # @see Validators::RequestValidator - # @deprecated will be replaced with {Contact} - class Request < Entity - # @!attribute [r] author - # The diaspora* ID of the person who share their profile - # @see Person#author - # @return [String] sender ID - property :author, :string, xml_name: :sender_handle - - # @!attribute [r] recipient - # The diaspora* ID of the person who will be shared with - # @see Validation::Rule::DiasporaId - # @return [String] recipient ID - property :recipient, :string, xml_name: :recipient_handle - - def initialize(*) - raise "Sending Request is not supported anymore! Use Contact instead!" - end - - # @return [Retraction] instance - def self.from_hash(hash) - Contact.new(hash) - end - end - end -end diff --git a/spec/lib/diaspora_federation/entities/request_spec.rb b/spec/lib/diaspora_federation/entities/request_spec.rb deleted file mode 100644 index 0d07035..0000000 --- a/spec/lib/diaspora_federation/entities/request_spec.rb +++ /dev/null @@ -1,33 +0,0 @@ -# frozen_string_literal: true - -module DiasporaFederation - describe Entities::Request do - let(:data) { {author: Fabricate.sequence(:diaspora_id), recipient: Fabricate.sequence(:diaspora_id)} } - - let(:xml) { <<~XML } - - #{data[:author]} - #{data[:recipient]} - - XML - - describe "#initialize" do - it "raises because it is not supported anymore" do - expect { - Entities::Request.new(data) - }.to raise_error RuntimeError, "Sending Request is not supported anymore! Use Contact instead!" - end - end - - context "parse contact" do - it "parses the xml as a contact" do - contact = Entities::Request.from_xml(Nokogiri::XML(xml).root) - expect(contact).to be_a(Entities::Contact) - expect(contact.author).to eq(data[:author]) - expect(contact.recipient).to eq(data[:recipient]) - expect(contact.following).to be_truthy - expect(contact.sharing).to be_truthy - end - end - end -end From 1238fe0384c317394645b16012a446cf5bb9ba5d Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Wed, 30 Oct 2019 04:17:57 +0100 Subject: [PATCH 5/8] Remove old property-name mappings and cleanup xml_name functionality Closes #29 --- .../entities/account_deletion.rb | 2 +- .../entities/conversation.rb | 4 +- lib/diaspora_federation/entities/like.rb | 2 +- lib/diaspora_federation/entities/message.rb | 2 +- .../entities/participation.rb | 4 +- lib/diaspora_federation/entities/person.rb | 2 +- lib/diaspora_federation/entities/photo.rb | 2 +- lib/diaspora_federation/entities/post.rb | 2 +- lib/diaspora_federation/entities/profile.rb | 2 +- lib/diaspora_federation/entities/relayable.rb | 2 +- lib/diaspora_federation/entities/reshare.rb | 4 +- .../entities/retraction.rb | 6 +-- .../entities/status_message.rb | 2 +- lib/diaspora_federation/entity.rb | 1 - .../parsers/json_parser.rb | 2 +- lib/diaspora_federation/parsers/xml_parser.rb | 21 +++++++--- lib/diaspora_federation/properties_dsl.rb | 37 ----------------- spec/entities.rb | 5 --- .../entities/conversation_spec.rb | 2 +- .../diaspora_federation/entities/like_spec.rb | 2 +- .../entities/relayable_spec.rb | 2 +- spec/lib/diaspora_federation/entity_spec.rb | 9 ----- .../parsers/xml_parser_spec.rb | 28 ------------- .../properties_dsl_spec.rb | 40 ------------------- 24 files changed, 37 insertions(+), 148 deletions(-) diff --git a/lib/diaspora_federation/entities/account_deletion.rb b/lib/diaspora_federation/entities/account_deletion.rb index d969b24..d17d067 100644 --- a/lib/diaspora_federation/entities/account_deletion.rb +++ b/lib/diaspora_federation/entities/account_deletion.rb @@ -14,7 +14,7 @@ module DiasporaFederation # Alias for author # @see AccountDeletion#author # @return [String] diaspora* ID - property :author, :string, alias: :diaspora_id, xml_name: :diaspora_handle + property :author, :string, alias: :diaspora_id # @return [String] string representation of this object def to_s diff --git a/lib/diaspora_federation/entities/conversation.rb b/lib/diaspora_federation/entities/conversation.rb index b3720d3..60ef992 100644 --- a/lib/diaspora_federation/entities/conversation.rb +++ b/lib/diaspora_federation/entities/conversation.rb @@ -10,7 +10,7 @@ module DiasporaFederation # The diaspora* ID of the person initiated the conversation # @see Person#author # @return [String] diaspora* ID - property :author, :string, xml_name: :diaspora_handle + property :author, :string # @!attribute [r] guid # A random string of at least 16 chars @@ -29,7 +29,7 @@ module DiasporaFederation # @!attribute [r] participants # The diaspora* IDs of the persons participating the conversation separated by ";" # @return [String] participants diaspora* IDs - property :participants, :string, xml_name: :participant_handles + property :participants, :string # @!attribute [r] messages # @return [[Entities::Message]] Messages of this conversation diff --git a/lib/diaspora_federation/entities/like.rb b/lib/diaspora_federation/entities/like.rb index b589058..941bf19 100644 --- a/lib/diaspora_federation/entities/like.rb +++ b/lib/diaspora_federation/entities/like.rb @@ -13,7 +13,7 @@ module DiasporaFederation # 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 + property :parent_type, :string # @!attribute [r] positive # If +true+ set a like, if +false+, set a dislike (dislikes are currently not diff --git a/lib/diaspora_federation/entities/message.rb b/lib/diaspora_federation/entities/message.rb index 81c631a..e6f073b 100644 --- a/lib/diaspora_federation/entities/message.rb +++ b/lib/diaspora_federation/entities/message.rb @@ -10,7 +10,7 @@ module DiasporaFederation # The diaspora* ID of the author # @see Person#author # @return [String] diaspora* ID - property :author, :string, xml_name: :diaspora_handle + property :author, :string # @!attribute [r] guid # A random string of at least 16 chars diff --git a/lib/diaspora_federation/entities/participation.rb b/lib/diaspora_federation/entities/participation.rb index 554d9bd..18c49f7 100644 --- a/lib/diaspora_federation/entities/participation.rb +++ b/lib/diaspora_federation/entities/participation.rb @@ -10,7 +10,7 @@ module DiasporaFederation # The diaspora* ID of the author # @see Person#author # @return [String] diaspora* ID - property :author, :string, xml_name: :diaspora_handle + property :author, :string # @!attribute [r] guid # A random string of at least 16 chars @@ -27,7 +27,7 @@ module DiasporaFederation # A string describing a type of the target to subscribe on # Currently only "Post" is supported. # @return [String] parent type - property :parent_type, :string, xml_name: :target_type + property :parent_type, :string # @return [String] string representation of this object def to_s diff --git a/lib/diaspora_federation/entities/person.rb b/lib/diaspora_federation/entities/person.rb index e4ad225..ad35947 100644 --- a/lib/diaspora_federation/entities/person.rb +++ b/lib/diaspora_federation/entities/person.rb @@ -21,7 +21,7 @@ module DiasporaFederation # alias for author # @see Person#author # @return [String] diaspora* ID - property :author, :string, alias: :diaspora_id, xml_name: :diaspora_handle + property :author, :string, alias: :diaspora_id # @!attribute [r] url # @see Discovery::WebFinger#seed_url diff --git a/lib/diaspora_federation/entities/photo.rb b/lib/diaspora_federation/entities/photo.rb index 4f11179..fcda7aa 100644 --- a/lib/diaspora_federation/entities/photo.rb +++ b/lib/diaspora_federation/entities/photo.rb @@ -16,7 +16,7 @@ module DiasporaFederation # The diaspora* ID of the person who uploaded the photo # @see Person#author # @return [String] author diaspora* ID - property :author, :string, xml_name: :diaspora_handle + property :author, :string # @!attribute [r] public # Points if the photo is visible to everyone or only to some aspects diff --git a/lib/diaspora_federation/entities/post.rb b/lib/diaspora_federation/entities/post.rb index a5c0a48..4eb9d93 100644 --- a/lib/diaspora_federation/entities/post.rb +++ b/lib/diaspora_federation/entities/post.rb @@ -32,7 +32,7 @@ module DiasporaFederation # @param [Entity] entity the entity in which it is included def self.included(entity) entity.class_eval do - property :author, :string, xml_name: :diaspora_handle + property :author, :string property :guid, :string property :created_at, :timestamp, default: -> { Time.now.utc } property :public, :boolean, default: false diff --git a/lib/diaspora_federation/entities/profile.rb b/lib/diaspora_federation/entities/profile.rb index b6ba0ff..08b53ae 100644 --- a/lib/diaspora_federation/entities/profile.rb +++ b/lib/diaspora_federation/entities/profile.rb @@ -14,7 +14,7 @@ module DiasporaFederation # Alias for author # @see Profile#author # @return [String] diaspora* ID - property :author, :string, alias: :diaspora_id, xml_name: :diaspora_handle + property :author, :string, alias: :diaspora_id # @!attribute [r] edited_at # The timestamp when the profile was edited diff --git a/lib/diaspora_federation/entities/relayable.rb b/lib/diaspora_federation/entities/relayable.rb index 3fe94c8..49b55cd 100644 --- a/lib/diaspora_federation/entities/relayable.rb +++ b/lib/diaspora_federation/entities/relayable.rb @@ -48,7 +48,7 @@ module DiasporaFederation # @param [Entity] klass the entity in which it is included def self.included(klass) klass.class_eval do - property :author, :string, xml_name: :diaspora_handle + property :author, :string property :guid, :string property :parent_guid, :string property :author_signature, :string, default: nil diff --git a/lib/diaspora_federation/entities/reshare.rb b/lib/diaspora_federation/entities/reshare.rb index aa38b29..f194771 100644 --- a/lib/diaspora_federation/entities/reshare.rb +++ b/lib/diaspora_federation/entities/reshare.rb @@ -10,7 +10,7 @@ module DiasporaFederation # The diaspora* ID of the person who reshares the post # @see Person#author # @return [String] diaspora* ID - property :author, :string, xml_name: :diaspora_handle + property :author, :string # @!attribute [r] guid # A random string of at least 16 chars @@ -27,7 +27,7 @@ module DiasporaFederation # The diaspora* ID of the person who posted the original post # @see Person#author # @return [String] diaspora* ID - property :root_author, :string, optional: true, xml_name: :root_diaspora_id + property :root_author, :string, optional: true # @!attribute [r] root_guid # Guid of the original post diff --git a/lib/diaspora_federation/entities/retraction.rb b/lib/diaspora_federation/entities/retraction.rb index 5c53eda..e809b2f 100644 --- a/lib/diaspora_federation/entities/retraction.rb +++ b/lib/diaspora_federation/entities/retraction.rb @@ -10,17 +10,17 @@ module DiasporaFederation # The diaspora* ID of the person who deletes the entity # @see Person#author # @return [String] diaspora* ID - property :author, :string, xml_name: :diaspora_handle + property :author, :string # @!attribute [r] target_guid # Guid of the entity to be deleted # @return [String] target guid - property :target_guid, :string, xml_name: :post_guid + property :target_guid, :string # @!attribute [r] target_type # A string describing the type of the target # @return [String] target type - property :target_type, :string, xml_name: :type + property :target_type, :string # @!attribute [r] target # Target entity diff --git a/lib/diaspora_federation/entities/status_message.rb b/lib/diaspora_federation/entities/status_message.rb index 1da0d8b..e893a23 100644 --- a/lib/diaspora_federation/entities/status_message.rb +++ b/lib/diaspora_federation/entities/status_message.rb @@ -11,7 +11,7 @@ module DiasporaFederation # @!attribute [r] text # Text of the status message composed by the user # @return [String] text of the status message - property :text, :string, xml_name: :raw_message + property :text, :string # @!attribute [r] edited_at # The timestamp when the status message was edited diff --git a/lib/diaspora_federation/entity.rb b/lib/diaspora_federation/entity.rb index 26526d1..3aa042b 100644 --- a/lib/diaspora_federation/entity.rb +++ b/lib/diaspora_federation/entity.rb @@ -17,7 +17,6 @@ module DiasporaFederation # property :prop # property :optional, default: false # property :dynamic_default, default: -> { Time.now } - # property :another_prop, xml_name: :another_name # entity :nested, NestedEntity # entity :multiple, [OtherEntity] # end diff --git a/lib/diaspora_federation/parsers/json_parser.rb b/lib/diaspora_federation/parsers/json_parser.rb index 1a8642f..b91dad7 100644 --- a/lib/diaspora_federation/parsers/json_parser.rb +++ b/lib/diaspora_federation/parsers/json_parser.rb @@ -18,7 +18,7 @@ module DiasporaFederation def parse_entity_data(entity_data) hash = entity_data.map {|key, value| - property = entity_type.find_property_for_xml_name(key) + property = entity_type.class_props.keys.find {|name| name.to_s == key } if property type = entity_type.class_props[property] [property, parse_element_from_value(type, entity_data[key])] diff --git a/lib/diaspora_federation/parsers/xml_parser.rb b/lib/diaspora_federation/parsers/xml_parser.rb index 40acfda..10baf2a 100644 --- a/lib/diaspora_federation/parsers/xml_parser.rb +++ b/lib/diaspora_federation/parsers/xml_parser.rb @@ -15,14 +15,12 @@ module DiasporaFederation from_xml_sanity_validation(root_node) hash = root_node.element_children.map {|child| - xml_name = child.name - property = entity_type.find_property_for_xml_name(xml_name) + property, type = find_property_for(child.name) if property - type = class_properties[property] - value = parse_element_from_node(xml_name, type, root_node) + value = parse_element_from_node(child.name, type, root_node) [property, value] else - [xml_name, child.text] + [child.name, child.text] end }.to_h @@ -31,6 +29,18 @@ module DiasporaFederation private + def find_property_for(xml_name) + class_properties.find {|name, type| + if type.instance_of?(Symbol) + name.to_s == xml_name + elsif type.instance_of?(Array) + type.first.entity_name == xml_name + elsif type.ancestors.include?(Entity) + type.entity_name == xml_name + end + } + end + # @param [String] name property name to parse # @param [Class, Symbol] type target type to parse # @param [Nokogiri::XML::Element] root_node XML node to parse @@ -53,7 +63,6 @@ module DiasporaFederation # @return [String] data def parse_string_from_node(name, type, root_node) node = root_node.xpath(name.to_s) - node = root_node.xpath(xml_names[name].to_s) if node.empty? parse_string(type, node.first.text) if node.any? end diff --git a/lib/diaspora_federation/properties_dsl.rb b/lib/diaspora_federation/properties_dsl.rb index b473529..b420c14 100644 --- a/lib/diaspora_federation/properties_dsl.rb +++ b/lib/diaspora_federation/properties_dsl.rb @@ -8,7 +8,6 @@ module DiasporaFederation # property :prop # property :optional, default: false # property :dynamic_default, default: -> { Time.now } - # property :another_prop, xml_name: :another_name # property :original_prop, alias: :alias_prop # entity :nested, NestedEntity # entity :multiple, [OtherEntity] @@ -24,7 +23,6 @@ module DiasporaFederation # @param [Hash] opts further options # @option opts [Object, #call] :default a default value, making the # property optional - # @option opts [Symbol] :xml_name another name used for xml generation def property(name, type, opts={}) raise InvalidType unless property_type_valid?(type) @@ -79,49 +77,14 @@ module DiasporaFederation }.to_h end - # @return [Symbol] alias for the xml-generation/parsing - # @deprecated - def xml_names - @xml_names ||= {} - end - - # Finds a property by +xml_name+ or +name+ - # @param [String] xml_name name of the property from the received xml - # @return [Hash] the property data - def find_property_for_xml_name(xml_name) - class_props.keys.find {|name| [name.to_s, xml_names[name].to_s].include?(xml_name) } - end - private - # @deprecated - def determine_xml_name(name, type, opts={}) - if !type.instance_of?(Symbol) && opts.has_key?(:xml_name) - raise ArgumentError, "xml_name is not supported for nested entities" - end - - if type.instance_of?(Symbol) - if opts.has_key? :xml_name - raise InvalidName, "invalid xml_name" unless name_valid?(opts[:xml_name]) - - opts[:xml_name] - else - name - end - elsif type.instance_of?(Array) - type.first.entity_name.to_sym - elsif type.ancestors.include?(Entity) - type.entity_name.to_sym - end - end - def define_property(name, type, opts={}) raise InvalidName unless name_valid?(name) class_props[name] = type optional_props << name if opts[:optional] default_props[name] = opts[:default] if opts.has_key? :default - xml_names[name] = determine_xml_name(name, type, opts) instance_eval { attr_reader name } diff --git a/spec/entities.rb b/spec/entities.rb index 429ea8a..8eab15c 100644 --- a/spec/entities.rb +++ b/spec/entities.rb @@ -28,11 +28,6 @@ module DiasporaFederation entity :multi, [OtherEntity] end - class TestEntityWithXmlName < DiasporaFederation::Entity - property :test, :string - property :qwer, :string, xml_name: :asdf - end - class TestEntityWithRelatedEntity < DiasporaFederation::Entity property :test, :string entity :parent, RelatedEntity diff --git a/spec/lib/diaspora_federation/entities/conversation_spec.rb b/spec/lib/diaspora_federation/entities/conversation_spec.rb index ace9e08..dc3a72c 100644 --- a/spec/lib/diaspora_federation/entities/conversation_spec.rb +++ b/spec/lib/diaspora_federation/entities/conversation_spec.rb @@ -54,7 +54,7 @@ module DiasporaFederation #{parent.guid} #{data[:subject]} #{data[:created_at]} - #{data[:participants]} + #{data[:participants]} XML diff --git a/spec/lib/diaspora_federation/entities/like_spec.rb b/spec/lib/diaspora_federation/entities/like_spec.rb index 307f530..5b02a8b 100644 --- a/spec/lib/diaspora_federation/entities/like_spec.rb +++ b/spec/lib/diaspora_federation/entities/like_spec.rb @@ -75,7 +75,7 @@ module DiasporaFederation it "raises a ValidationError if the parent_guid is missing" do broken_xml = <<~XML - #{parent.entity_type} + #{parent.entity_type} XML diff --git a/spec/lib/diaspora_federation/entities/relayable_spec.rb b/spec/lib/diaspora_federation/entities/relayable_spec.rb index 4daa408..a03744c 100644 --- a/spec/lib/diaspora_federation/entities/relayable_spec.rb +++ b/spec/lib/diaspora_federation/entities/relayable_spec.rb @@ -310,7 +310,7 @@ module DiasporaFederation let(:new_signature_data) { "#{author};#{guid};#{parent_guid};#{new_property};#{property}" } let(:new_xml) { <<~XML } - #{author} + #{author} #{guid} #{parent_guid} #{new_property} diff --git a/spec/lib/diaspora_federation/entity_spec.rb b/spec/lib/diaspora_federation/entity_spec.rb index cd52a27..a349cc9 100644 --- a/spec/lib/diaspora_federation/entity_spec.rb +++ b/spec/lib/diaspora_federation/entity_spec.rb @@ -528,14 +528,5 @@ module DiasporaFederation expect(entity.multi).to be_empty end end - - context "xml_name" do - let(:hash) { {test: "test", qwer: "qwer"} } - - it "should not use the xml_name for the #to_h" do - entity = Entities::TestEntityWithXmlName.new(hash) - expect(entity.to_h).to eq(hash) - end - end end end diff --git a/spec/lib/diaspora_federation/parsers/xml_parser_spec.rb b/spec/lib/diaspora_federation/parsers/xml_parser_spec.rb index da3a661..5923234 100644 --- a/spec/lib/diaspora_federation/parsers/xml_parser_spec.rb +++ b/spec/lib/diaspora_federation/parsers/xml_parser_spec.rb @@ -33,34 +33,6 @@ module DiasporaFederation end end - it "uses xml_name for parsing" do - xml = <<~XML.strip - - asdf - qwer - - XML - - parsed = Parsers::XmlParser.new(Entities::TestEntityWithXmlName).parse(Nokogiri::XML(xml).root) - - expect(parsed[0][:test]).to eq("asdf") - expect(parsed[0][:qwer]).to eq("qwer") - end - - it "allows name for parsing even when property has a xml_name" do - xml = <<~XML.strip - - asdf - qwer - - XML - - parsed = Parsers::XmlParser.new(Entities::TestEntityWithXmlName).parse(Nokogiri::XML(xml).root) - - expect(parsed[0][:test]).to eq("asdf") - expect(parsed[0][:qwer]).to eq("qwer") - end - it "parses the string to the correct type" do xml = <<~XML.strip diff --git a/spec/lib/diaspora_federation/properties_dsl_spec.rb b/spec/lib/diaspora_federation/properties_dsl_spec.rb index f5ff9ee..b758ee2 100644 --- a/spec/lib/diaspora_federation/properties_dsl_spec.rb +++ b/spec/lib/diaspora_federation/properties_dsl_spec.rb @@ -10,7 +10,6 @@ module DiasporaFederation properties = dsl.class_props expect(properties).to have(1).item expect(properties[:test]).to eq(:string) - expect(dsl.xml_names[:test]).to eq(:test) end it "will not accept other types for names" do @@ -46,22 +45,6 @@ module DiasporaFederation expect(properties.keys).to include(:test, :asdf, :zzzz) properties.each_value {|type| expect(type).to eq(:string) } end - - it "can add an xml name to simple properties with a symbol" do - dsl.property :test, :string, xml_name: :xml_test - properties = dsl.class_props - expect(properties).to have(1).item - expect(properties[:test]).to eq(:string) - expect(dsl.xml_names[:test]).to eq(:xml_test) - end - - it "will not accept other types for xml names" do - ["test", 1234, true, {}].each do |val| - expect { - dsl.property :test, :string, xml_name: val - }.to raise_error PropertiesDSL::InvalidName, "invalid xml_name" - end - end end context "nested entities" do @@ -99,12 +82,6 @@ module DiasporaFederation }.to raise_error PropertiesDSL::InvalidType end end - - it "can not add an xml name to a nested entity" do - expect { - dsl.entity :other, Entities::TestEntity, xml_name: :other_name - }.to raise_error ArgumentError, "xml_name is not supported for nested entities" - end end describe ".optional_props" do @@ -174,22 +151,5 @@ module DiasporaFederation expect(data).not_to have_key(:test_alias) end end - - describe ".find_property_for_xml_name" do - it "finds property by xml_name" do - dsl.property :test, :string, xml_name: :xml_test - expect(dsl.find_property_for_xml_name("xml_test")).to eq(:test) - end - - it "finds property by name" do - dsl.property :test, :string, xml_name: :xml_test - expect(dsl.find_property_for_xml_name("test")).to eq(:test) - end - - it "returns nil if property is not defined" do - dsl.property :test, :string, xml_name: :xml_test - expect(dsl.find_property_for_xml_name("unknown")).to be_nil - end - end end end From 3fcea8b188f22092b43b1b57ae9a54b619f6c396 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Wed, 30 Oct 2019 17:38:58 +0100 Subject: [PATCH 6/8] Remove parent_author_signature from relayables Closes #64 --- docs/_entities/event_participation.md | 2 - lib/diaspora_federation/entities/relayable.rb | 33 +--- spec/integration/comment_integration_spec.rb | 30 ---- .../entities/comment_spec.rb | 1 - .../entities/event_participation_spec.rb | 1 - .../diaspora_federation/entities/like_spec.rb | 1 - .../entities/poll_participation_spec.rb | 1 - .../entities/relayable_spec.rb | 145 +++++------------- spec/support/helper_methods.rb | 1 - spec/support/shared_entity_specs.rb | 15 +- 10 files changed, 54 insertions(+), 176 deletions(-) diff --git a/docs/_entities/event_participation.md b/docs/_entities/event_participation.md index 29e0af7..24fc5a4 100644 --- a/docs/_entities/event_participation.md +++ b/docs/_entities/event_participation.md @@ -33,7 +33,6 @@ See also: [Relayable][relayable] bb8371f0b1c901342ebd55853a9b5d75 accepted dT6KbT7kp0bE+s3//ZErxO1wvVIqtD0lY67i81+dO43B4D2m5kjCdzW240eWt/jZmcHIsdxXf4WHNdrb6ZDnamA8I1FUVnLjHA9xexBITQsSLXrcV88UdammSmmOxl1Ac4VUXqFpdavm6a7/MwOJ7+JHP8TbUO9siN+hMfgUbtY= - ~~~ @@ -46,7 +45,6 @@ See also: [Relayable][relayable] bb8371f0b1c901342ebd55853a9b5d75 accepted dT6KbT7kp0bE+s3//ZErxO1wvVIqtD0lY67i81+dO43B4D2m5kjCdzW240eWt/jZmcHIsdxXf4WHNdrb6ZDnamA8I1FUVnLjHA9xexBITQsSLXrcV88UdammSmmOxl1Ac4VUXqFpdavm6a7/MwOJ7+JHP8TbUO9siN+hMfgUbtY= - gWasNPpSnMcKBIMWyzfoVO6sr8eRYkhUqy3PIkkh53n/ki+DM9mnh3ayotI0+6un9aq1N3XkS7Vn05ZD3+nHVby6i21XkYgPnbD8pWYuBBj7VGPyahT70BUs/vSvY8KX8V3wYfsPsaiAgJsAFg2UHYdY3r4/oWdIIbBZc21O3zk= ~~~ diff --git a/lib/diaspora_federation/entities/relayable.rb b/lib/diaspora_federation/entities/relayable.rb index 49b55cd..ef514ca 100644 --- a/lib/diaspora_federation/entities/relayable.rb +++ b/lib/diaspora_federation/entities/relayable.rb @@ -35,12 +35,6 @@ module DiasporaFederation # a target pod. # @return [String] author signature # - # @!attribute [r] parent_author_signature - # Contains a signature of the entity using the private key of the author of a parent post. - # @deprecated This signature isn't required anymore, because we can check the signature from - # the parent author in the MagicEnvelope. - # @return [String] parent author signature - # # @!attribute [r] parent # Meta information about the parent object # @return [RelatedEntity] parent entity @@ -52,7 +46,6 @@ module DiasporaFederation property :guid, :string property :parent_guid, :string property :author_signature, :string, default: nil - property :parent_author_signature, :string, default: nil entity :parent, Entities::RelatedEntity end @@ -67,7 +60,7 @@ module DiasporaFederation # @see DiasporaFederation::Entity#initialize def initialize(data, signature_order=nil, additional_data={}) self.signature_order = signature_order if signature_order - @additional_data = additional_data + self.additional_data = additional_data super(data) end @@ -104,7 +97,7 @@ module DiasporaFederation def signature_order @signature_order || self.class.class_props.keys.reject {|key| self.class.optional_props.include?(key) && public_send(key).nil? - } - %i[author_signature parent_author_signature parent] + } - %i[author_signature parent] end private @@ -121,17 +114,6 @@ module DiasporaFederation end end - # 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.root.author) - return unless privkey - - sign_with_key(privkey).tap do - logger.info "event=sign status=complete signature=parent_author_signature obj=#{self}" - end - end - # Update the signatures with the keys of the author and the parent # if the signatures are not there yet and if the keys are available. # @@ -139,7 +121,6 @@ module DiasporaFederation def enriched_properties super.merge(additional_data).tap do |hash| hash[:author_signature] = author_signature || sign_with_author - hash.delete(:parent_author_signature) end end @@ -147,10 +128,8 @@ module DiasporaFederation # # @return [Hash] sorted xml elements def xml_elements - data = super.tap do |hash| - hash[:parent_author_signature] = parent_author_signature || sign_with_parent_author_if_available.to_s - end - order = signature_order + %i[author_signature parent_author_signature] + data = super + order = signature_order + %i[author_signature] order.map {|element| [element, data[element].to_s] }.to_h end @@ -160,6 +139,10 @@ module DiasporaFederation .map {|name| prop_names.include?(name) ? name.to_sym : name } end + def additional_data=(additional_data) + @additional_data = additional_data.reject {|name, _| name =~ /signature/ } + end + # @return [String] signature data string def signature_data data = normalized_properties.merge(additional_data) diff --git a/spec/integration/comment_integration_spec.rb b/spec/integration/comment_integration_spec.rb index 2966631..2e93969 100644 --- a/spec/integration/comment_integration_spec.rb +++ b/spec/integration/comment_integration_spec.rb @@ -27,31 +27,6 @@ module DiasporaFederation # g6zpg1zxGahpmxwqFQIDAQAB # -----END PUBLIC KEY----- - let(:parent_serialized_key) { <<~KEY } - -----BEGIN RSA PRIVATE KEY----- - MIICXgIBAAKBgQDrOvW1UArKoUOg54XWXcTD3jU0zKG3Pm9IeaEzfQtApogQ3+M/ - F9nz0i3q8UhTDEPBQ3hMbqJ/4qfY+wFulxMR58DbqxFx9QcNZISUd0CPx/fJOYMx - R7bygTbiCet4FAiyMjxOX3Oei/DedUNps1RAP1bu+80iibze/Kk9BgMm0QIDAQAB - AoGAMHvikRCCaOl8SvnteBWzrLtsNAnJez9/KG0JcNdhLl4kxXWgHS0JW1wC4t4A - jj2E6ZzCet6C1+Ebv3lc/jJdV3pCK3wgX0YAt/oBW5kcuvpLHLSWusWHnHkYU+qO - 4SdC3bRhdLV9o3u/oCWzmdeKTdqIyNd2yAbb3W1TsD4EsQECQQD6w+vWVKhWbVOj - Ky3ZkLCxPcWNUt+7OIzDA1OLKhdhe44hIoRMfDT6iLK3sJTSjgOv0OFTfsdOqh5y - ZqYp/CTpAkEA8CQFKkAYt4qG1lKMPsU/Tme0/Z24VozDRnyw7r663f0De+25kXGY - PSBiOHYcAE6poYQEtR/leLTSaG3YZm7hqQJBAOLAWLg1Uwbb0v4/pDUQlgWfQsy4 - /KAx0W7hyiCTzhKTBAFIUfNLeSh2hYx+ewQt8H2B1s6GXDjwsZlm4qgiXUkCQQC9 - B12ZeIL8V2r0Yl5LOvEuQqxRx0lHt94vKhAMns5x16xabTLZrlVsKIWodDBufX1B - yq359XWooo3N7kmduEKhAkEAppzKLuVtX1XPL4VZBex/M2ewngjkSg964BvxIBwv - bFzeSqlMpnbEoOJ9hhx6CsP6Y7V19DRRXi0XgwcAjHLz8g== - -----END RSA PRIVATE KEY----- - KEY - let(:parent_key) { OpenSSL::PKey::RSA.new(parent_serialized_key) } - # -----BEGIN PUBLIC KEY----- - # MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDrOvW1UArKoUOg54XWXcTD3jU0 - # zKG3Pm9IeaEzfQtApogQ3+M/F9nz0i3q8UhTDEPBQ3hMbqJ/4qfY+wFulxMR58Db - # qxFx9QcNZISUd0CPx/fJOYMxR7bygTbiCet4FAiyMjxOX3Oei/DedUNps1RAP1bu - # +80iibze/Kk9BgMm0QIDAQAB - # -----END PUBLIC KEY----- - let(:author) { "alice@pod-a.org" } let(:guid) { "e21589b0b41101333b870f77ba60fa73" } let(:parent_guid) { "9e269ae0b41201333b8c0f77ba60fa73" } @@ -104,7 +79,6 @@ module DiasporaFederation 9e269ae0b41201333b8c0f77ba60fa73 this is a very informative comment SQbLeqsEpFmSl74G1fFJXKQcsq6jp5B2ZjmfEOF/LbBccYP2oZEyEqOq18K3Fa71OYTp6Nddb38hCmHWWHvnGUltGfxKBnQ0WHafJUi40VM4VmeRoU8cac6m+1hslwe5SNmK6oh47EV3mRCXlgGGjLIrw7iEwjKL2g9x1gkNp2s= - hWsagsczmZD6d36d6MFdTt3hKAdnRtupSIU6464G2kkMJ+WlExxMgbF6kWR+jVCBTeKipWCYK3Arnj0YkuIZM9d14bJGVMTsW/ZzNfJ69bXZhsyawI8dPnZnLVydo+hU/XmGJBEuh2TOj9Emq6/HCYiWzPTF5qhYAtyJ1oxJ4Yk= XML let(:new_data_comment_xml_bob) { <<~XML } @@ -115,7 +89,6 @@ module DiasporaFederation this is a very informative comment foobar SFYXSvCX/DhTFiOUALp2Nf1kfNkGKXrnoBPikAyhaIogGydVBm+8tIlu1U/vsnpyKO3yfC3JReJ00/UBd4J16VO1IxStntq8NUqbSv4me5A/6kdK9Xg6eYbXrqQGm8fUQ5Xuh2UzeB71p7SVySXX3OZHVe0dvHCxH/lsfSDpEjc= - NxXuEUVeXwUMR77osIbaNlp2oB3bpl8rBEFgQoO6cnoN5ewDbiGADK0x6EhcmJptjwhGVcZiNJNpq7k3/pjJtKaH++3ToCAtcuZoIKwPDsneLnjPhVjE2GXM1TiZKwoHrq41qSp/8Vl5UPbtC6sPiOzIvPKaILXUG8XCiVWuB0M= XML @@ -123,14 +96,12 @@ module DiasporaFederation context "test-data creation" do it "creates comment xml" do expect_callback(:fetch_private_key, author).and_return(author_key) - expect_callback(:fetch_private_key, parent.author).and_return(nil) comment.to_xml end it "creates relayed comment xml" do expect_callback(:fetch_public_key, author).and_return(author_key.public_key) - expect_callback(:fetch_private_key, parent.author).and_return(parent_key) expect_callback(:fetch_related_entity, "Post", parent_guid).and_return(parent) xml = Nokogiri::XML(new_data_comment_xml_alice).root @@ -141,7 +112,6 @@ module DiasporaFederation context "relaying on bobs pod" do before do expect_callback(:fetch_public_key, author).and_return(author_key.public_key) - expect_callback(:fetch_private_key, parent.author).and_return(parent_key) expect_callback(:fetch_related_entity, "Post", parent_guid).and_return(parent) end diff --git a/spec/lib/diaspora_federation/entities/comment_spec.rb b/spec/lib/diaspora_federation/entities/comment_spec.rb index 042f706..2efb00d 100644 --- a/spec/lib/diaspora_federation/entities/comment_spec.rb +++ b/spec/lib/diaspora_federation/entities/comment_spec.rb @@ -23,7 +23,6 @@ module DiasporaFederation #{data[:created_at].utc.iso8601} #{data[:edited_at].utc.iso8601} #{data[:author_signature]} - #{data[:parent_author_signature]} XML diff --git a/spec/lib/diaspora_federation/entities/event_participation_spec.rb b/spec/lib/diaspora_federation/entities/event_participation_spec.rb index b06c5d4..a4fb8f8 100644 --- a/spec/lib/diaspora_federation/entities/event_participation_spec.rb +++ b/spec/lib/diaspora_federation/entities/event_participation_spec.rb @@ -21,7 +21,6 @@ module DiasporaFederation #{data[:status]} #{data[:edited_at].utc.iso8601} #{data[:author_signature]} - #{data[:parent_author_signature]} XML diff --git a/spec/lib/diaspora_federation/entities/like_spec.rb b/spec/lib/diaspora_federation/entities/like_spec.rb index 5b02a8b..ce98b80 100644 --- a/spec/lib/diaspora_federation/entities/like_spec.rb +++ b/spec/lib/diaspora_federation/entities/like_spec.rb @@ -22,7 +22,6 @@ module DiasporaFederation #{parent.entity_type} #{data[:positive]} #{data[:author_signature]} - #{data[:parent_author_signature]} XML diff --git a/spec/lib/diaspora_federation/entities/poll_participation_spec.rb b/spec/lib/diaspora_federation/entities/poll_participation_spec.rb index 83848b0..bcbc191 100644 --- a/spec/lib/diaspora_federation/entities/poll_participation_spec.rb +++ b/spec/lib/diaspora_federation/entities/poll_participation_spec.rb @@ -20,7 +20,6 @@ module DiasporaFederation #{parent.guid} #{data[:poll_answer_guid]} #{data[:author_signature]} - #{data[:parent_author_signature]} XML diff --git a/spec/lib/diaspora_federation/entities/relayable_spec.rb b/spec/lib/diaspora_federation/entities/relayable_spec.rb index a03744c..8b5ad77 100644 --- a/spec/lib/diaspora_federation/entities/relayable_spec.rb +++ b/spec/lib/diaspora_federation/entities/relayable_spec.rb @@ -3,7 +3,6 @@ module DiasporaFederation describe Entities::Relayable do let(:author_pkey) { OpenSSL::PKey::RSA.generate(1024) } - let(:parent_pkey) { OpenSSL::PKey::RSA.generate(1024) } let(:guid) { Fabricate.sequence(:guid) } let(:parent_guid) { Fabricate.sequence(:guid) } @@ -13,24 +12,32 @@ module DiasporaFederation let(:local_parent) { Fabricate(:related_entity, author: bob.diaspora_id) } let(:remote_parent) { Fabricate(:related_entity, author: bob.diaspora_id, local: false) } 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(:hash_with_fake_signatures) { hash.merge!(author_signature: "aa") } 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 - signature_order = [:author, :guid, :parent_guid, :property, "new_property", :author_signature] + signature_order = + [:author, :guid, :parent_guid, :property, "new_property", :author_signature, "parent_author_signature"] expect(Entities::SomeRelayable.new(hash, signature_order).signature_order) .to eq([:author, :guid, :parent_guid, :property, "new_property"]) end + + it "filters signatures from additional_data" do + signature_order = [:author, :guid, :parent_guid, :property, "new_property"] + additional_data = {"new_property" => "foobar", "parent_author_signature" => "bb"} + + expect(Entities::SomeRelayable.new(hash, signature_order, additional_data).additional_data) + .to eq("new_property" => "foobar") + end end describe "#verify_signature" do - it "doesn't raise anything if correct signatures were passed" do + it "doesn't raise anything if correct author_signature was 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) @@ -38,12 +45,11 @@ module DiasporaFederation expect { Entities::SomeRelayable.new(hash, signature_order).verify_signature }.not_to raise_error end - it "doesn't raise anything if correct signatures with new property were passed" do + it "doesn't raise anything if correct author_signature with new property was 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) @@ -99,26 +105,6 @@ module DiasporaFederation Entities::SomeRelayable.new(hash, signature_order).verify_signature }.to raise_error Entities::Relayable::SignatureVerificationFailed end - - it "doesn't raise 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 - - expect_callback(:fetch_public_key, author).and_return(author_pkey.public_key) - - expect { Entities::SomeRelayable.new(hash, signature_order).verify_signature }.not_to raise_error - end - - it "doesn't raise when no parent author signature was passed and we're on upstream federation" do - 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, signature_order).verify_signature }.not_to raise_error - end end describe "#to_xml" do @@ -130,7 +116,6 @@ module DiasporaFederation #{property} #{new_property} aa - bb XML @@ -169,7 +154,6 @@ module DiasporaFederation #{new_property} aa - bb XML @@ -188,7 +172,6 @@ module DiasporaFederation #{guid} #{parent_guid} aa - bb XML @@ -197,22 +180,16 @@ module DiasporaFederation expect(xml.to_s.strip).to eq(expected_xml.strip) end - it "computes correct signatures for the entity" do + it "computes correct author_signature for the entity" do expect_callback(:fetch_private_key, author).and_return(author_pkey) - expect_callback(:fetch_private_key, local_parent.author).and_return(parent_pkey) xml = Entities::SomeRelayable.new(hash).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 + expect(verify_signature(author_pkey, xml.at_xpath("author_signature").text, signature_data)).to be_truthy end - it "computes correct signatures for the entity with invalid XML characters" do + it "computes correct author_signature for the entity with invalid XML characters" do expect_callback(:fetch_private_key, author).and_return(author_pkey) - expect_callback(:fetch_private_key, local_parent.author).and_return(parent_pkey) invalid_property = "asdfasdf asdf💩asdf\nasdf" signature_data_with_fixed_property = "#{author};#{guid};#{parent_guid};asdf�asdf asdf💩asdf\nasdf" @@ -220,31 +197,22 @@ module DiasporaFederation xml = Entities::SomeRelayable.new(hash.merge(property: invalid_property)).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_with_fixed_property)).to be_truthy - expect(verify_signature(parent_pkey, parent_author_signature, signature_data_with_fixed_property)).to be_truthy end - it "computes correct signatures for the entity when the parent is a relayable itself" do + it "computes correct author_signature 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 + expect(verify_signature(author_pkey, xml.at_xpath("author_signature").text, signature_data)).to be_truthy end - it "computes correct signatures for the entity with new unknown xml elements" do + it "computes correct author_signature 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) signature_order = [:author, :guid, :parent_guid, "new_property", :property] signature_data_with_new_property = "#{author};#{guid};#{parent_guid};#{new_property};#{property}" @@ -252,17 +220,13 @@ module DiasporaFederation xml = Entities::SomeRelayable.new(hash, signature_order, "new_property" => new_property).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_with_new_property)).to be_truthy - expect(verify_signature(parent_pkey, parent_author_signature, signature_data_with_new_property)).to be_truthy end - it "doesn't change signatures if they are already set" do + it "doesn't change author_signature if it is already set" do xml = Entities::SomeRelayable.new(hash_with_fake_signatures).to_xml expect(xml.at_xpath("author_signature").text).to eq("aa") - expect(xml.at_xpath("parent_author_signature").text).to eq("bb") end it "raises when author_signature not set and key isn't supplied" do @@ -273,15 +237,6 @@ module DiasporaFederation }.to raise_error Entities::Relayable::AuthorPrivateKeyNotFound end - it "doesn't set parent_author_signature if key isn't supplied" do - expect_callback(:fetch_private_key, author).and_return(author_pkey) - expect_callback(:fetch_private_key, local_parent.author).and_return(nil) - - xml = Entities::SomeRelayable.new(hash).to_xml - - expect(xml.at_xpath("parent_author_signature").text).to eq("") - end - it "adds 'false' booleans" do expected_xml = <<~XML @@ -290,7 +245,6 @@ module DiasporaFederation #{parent_guid} false aa - bb XML @@ -316,7 +270,6 @@ module DiasporaFederation #{new_property} #{property} #{sign_with_key(author_pkey, new_signature_data)} - #{sign_with_key(parent_pkey, new_signature_data)} XML @@ -338,7 +291,6 @@ module DiasporaFederation it "creates Entity with empty 'additional_data' if the xml has only known properties" do 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 @@ -450,13 +402,6 @@ module DiasporaFederation Entities::SomeRelayable.new(hash).to_json }.to raise_error Entities::Relayable::AuthorPrivateKeyNotFound end - - it "doesn't contain the parent_author_signature" do - expect_callback(:fetch_private_key, author).and_return(author_pkey) - - json = Entities::SomeRelayable.new(hash).to_json - expect(json[:entity_data]).not_to include(:parent_author_signature) - end end describe ".from_hash" do @@ -471,16 +416,14 @@ module DiasporaFederation context "when properties are sorted and there is an unknown property" do let(:new_signature_data) { "#{author};#{guid};#{parent_guid};#{new_property};#{property}" } let(:author_signature) { sign_with_key(author_pkey, new_signature_data) } - let(:parent_author_signature) { sign_with_key(parent_pkey, new_signature_data) } let(:entity_data) { { - :guid => guid, - :author => author, - :property => property, - :parent_guid => parent_guid, - "new_property" => new_property, - :author_signature => author_signature, - :parent_author_signature => parent_author_signature + :guid => guid, + :author => author, + :property => property, + :parent_guid => parent_guid, + "new_property" => new_property, + :author_signature => author_signature } } let(:property_order) { %w[author guid parent_guid new_property property] } @@ -493,7 +436,6 @@ module DiasporaFederation expect(entity.parent_guid).to eq(parent_guid) expect(entity.property).to eq(property) expect(entity.author_signature).to eq(author_signature) - expect(entity.parent_author_signature).to eq(parent_author_signature) end it "makes unknown properties available via #additional_data" do @@ -509,13 +451,12 @@ module DiasporaFederation it "calls a constructor of the entity of the appropriate type" do expect(Entities::SomeRelayable).to receive(:new).with( { - author: author, - guid: guid, - parent_guid: parent_guid, - property: property, - author_signature: author_signature, - parent_author_signature: parent_author_signature, - parent: remote_parent + author: author, + guid: guid, + parent_guid: parent_guid, + property: property, + author_signature: author_signature, + parent: remote_parent }.merge("new_property" => new_property), %w[author guid parent_guid new_property property], "new_property" => new_property @@ -528,12 +469,11 @@ module DiasporaFederation 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, signature_data), - parent_author_signature: sign_with_key(parent_pkey, signature_data) + guid: guid, + author: author, + property: property, + parent_guid: parent_guid, + author_signature: sign_with_key(author_pkey, signature_data) } entity = Entities::SomeRelayable.from_hash(entity_data, property_order) @@ -547,12 +487,11 @@ module DiasporaFederation it "calls signatures verification on relayable unpack" do property_order = %w[guid author property parent_guid] entity_data = { - guid: guid, - author: author, - property: property, - parent_guid: parent_guid, - author_signature: "aa", - parent_author_signature: "bb" + guid: guid, + author: author, + property: property, + parent_guid: parent_guid, + author_signature: "aa" } expect_callback(:fetch_related_entity, "Parent", parent_guid).and_return(remote_parent) diff --git a/spec/support/helper_methods.rb b/spec/support/helper_methods.rb index b2bb3a6..69f1f67 100644 --- a/spec/support/helper_methods.rb +++ b/spec/support/helper_methods.rb @@ -18,7 +18,6 @@ end def add_signatures(hash, klass=described_class) properties = klass.new(hash).send(:xml_elements) hash[:author_signature] = properties[:author_signature] - hash[:parent_author_signature] = properties[:parent_author_signature] end def sign_with_key(privkey, signature_data) diff --git a/spec/support/shared_entity_specs.rb b/spec/support/shared_entity_specs.rb index 4d1e256..3ce2f69 100644 --- a/spec/support/shared_entity_specs.rb +++ b/spec/support/shared_entity_specs.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true def entity_hash_from(hash) - hash.delete(:parent_author_signature) hash.map {|key, value| if [String, TrueClass, FalseClass, Integer, NilClass].any? {|c| value.is_a? c } [key, value] @@ -101,24 +100,19 @@ shared_examples "an XML Entity" do |ignored_props=[]| end shared_examples "a relayable Entity" do - let(:instance) { described_class.new(data.merge(author_signature: nil, parent_author_signature: nil)) } + let(:instance) { described_class.new(data.merge(author_signature: nil)) } context "signatures generation" do def verify_signature(pubkey, signature, signed_string) pubkey.verify(OpenSSL::Digest::SHA256.new, Base64.decode64(signature), signed_string) end - it "computes correct signatures for the entity" do - order = described_class.class_props.keys - %i[author_signature parent_author_signature parent] + it "computes correct author_signature for the entity" do + order = described_class.class_props.keys - %i[author_signature parent] signed_string = order.map {|name| data[name].is_a?(Time) ? data[name].iso8601 : data[name] }.join(";") - xml = instance.to_xml - - author_signature = xml.at_xpath("author_signature").text - parent_author_signature = xml.at_xpath("parent_author_signature").text - + author_signature = instance.to_xml.at_xpath("author_signature").text expect(verify_signature(alice.public_key, author_signature, signed_string)).to be_truthy - expect(verify_signature(bob.public_key, parent_author_signature, signed_string)).to be_truthy end end end @@ -182,7 +176,6 @@ shared_examples "a relayable JSON entity" do it "matches JSON schema with empty string signatures" do json = described_class.new(data).to_json json[:entity_data][:author_signature] = "" - json[:entity_data][:parent_author_signature] = "" expect(json.to_json).to match_json_schema(:entity_schema) end end From e9bf942618fbb2b0f49d106a96cb2371c5ff3e25 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Wed, 30 Jun 2021 00:11:09 +0200 Subject: [PATCH 7/8] Cleanup comment_integration_spec a little bit more --- spec/integration/comment_integration_spec.rb | 29 +++++++++----------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/spec/integration/comment_integration_spec.rb b/spec/integration/comment_integration_spec.rb index 2e93969..f61d546 100644 --- a/spec/integration/comment_integration_spec.rb +++ b/spec/integration/comment_integration_spec.rb @@ -40,17 +40,16 @@ module DiasporaFederation ) } - let(:new_format_comment_xml_alice) { <<~XML } + let(:comment_xml_alice) { <<~XML } alice@pod-a.org e21589b0b41101333b870f77ba60fa73 9e269ae0b41201333b8c0f77ba60fa73 this is a very informative comment SQbLeqsEpFmSl74G1fFJXKQcsq6jp5B2ZjmfEOF/LbBccYP2oZEyEqOq18K3Fa71OYTp6Nddb38hCmHWWHvnGUltGfxKBnQ0WHafJUi40VM4VmeRoU8cac6m+1hslwe5SNmK6oh47EV3mRCXlgGGjLIrw7iEwjKL2g9x1gkNp2s= - XML - let(:new_data_comment_xml_alice) { <<~XML } + let(:comment_xml_alice_with_new_data) { <<~XML } alice@pod-a.org e21589b0b41101333b870f77ba60fa73 @@ -58,21 +57,19 @@ module DiasporaFederation this is a very informative comment foobar SFYXSvCX/DhTFiOUALp2Nf1kfNkGKXrnoBPikAyhaIogGydVBm+8tIlu1U/vsnpyKO3yfC3JReJ00/UBd4J16VO1IxStntq8NUqbSv4me5A/6kdK9Xg6eYbXrqQGm8fUQ5Xuh2UzeB71p7SVySXX3OZHVe0dvHCxH/lsfSDpEjc= - XML - let(:legacy_order_new_format_comment_xml_bob) { <<~XML } + let(:comment_xml_bob_legacy_order) { <<~XML } e21589b0b41101333b870f77ba60fa73 9e269ae0b41201333b8c0f77ba60fa73 this is a very informative comment alice@pod-a.org XU5X1uqTh8SY6JMG9uhEVR5Rg7FURV6lpRwl/HYOu6DJ3Hd9tpA2aSFFibUxxsMgJXKNrrc5SykrrEdTiQoEei+j0QqZf3B5R7r84qgK7M46KazwIpqRPwVl2MdA/0DdQyYJLA/oavNj1nwll9vtR87M7e/C94qG6P+iQTMBQzo= - QqWSdwpb+/dcJUxuKKVe7aiz1NivXzlIdWZ71xyrxnhFxFYd+7EIittyTcp1cVehjg96pwDbn++P/rWyCffqenWu025DHvUfSmQkC93Z0dX6r3OIUlZqwEggtOdbunybiE++F3BVsGt5wC4YbAESB5ZFuhFVhBXh1X+EaZ/qoKo= XML - let(:new_format_comment_xml_bob) { <<~XML } + let(:comment_xml_bob) { <<~XML } alice@pod-a.org e21589b0b41101333b870f77ba60fa73 @@ -81,7 +78,7 @@ module DiasporaFederation SQbLeqsEpFmSl74G1fFJXKQcsq6jp5B2ZjmfEOF/LbBccYP2oZEyEqOq18K3Fa71OYTp6Nddb38hCmHWWHvnGUltGfxKBnQ0WHafJUi40VM4VmeRoU8cac6m+1hslwe5SNmK6oh47EV3mRCXlgGGjLIrw7iEwjKL2g9x1gkNp2s= XML - let(:new_data_comment_xml_bob) { <<~XML } + let(:comment_xml_bob_with_new_data) { <<~XML } alice@pod-a.org e21589b0b41101333b870f77ba60fa73 @@ -104,7 +101,7 @@ module DiasporaFederation expect_callback(:fetch_public_key, author).and_return(author_key.public_key) expect_callback(:fetch_related_entity, "Post", parent_guid).and_return(parent) - xml = Nokogiri::XML(new_data_comment_xml_alice).root + xml = Nokogiri::XML(comment_xml_alice_with_new_data).root Entity.entity_class(xml.name).from_xml(xml).to_xml end end @@ -116,15 +113,15 @@ module DiasporaFederation end it "relays new order" do - xml = Nokogiri::XML(new_format_comment_xml_alice).root + xml = Nokogiri::XML(comment_xml_alice).root entity = Entity.entity_class(xml.name).from_xml(xml) - expect(entity.to_xml.to_xml).to eq(new_format_comment_xml_bob.strip) + expect(entity.to_xml.to_xml).to eq(comment_xml_bob.strip) end it "relays new data" do - xml = Nokogiri::XML(new_data_comment_xml_alice).root + xml = Nokogiri::XML(comment_xml_alice_with_new_data).root entity = Entity.entity_class(xml.name).from_xml(xml) - expect(entity.to_xml.to_xml).to eq(new_data_comment_xml_bob.strip) + expect(entity.to_xml.to_xml).to eq(comment_xml_bob_with_new_data.strip) end end @@ -137,7 +134,7 @@ module DiasporaFederation end it "parses legacy order with new xml format" do - xml = Nokogiri::XML(legacy_order_new_format_comment_xml_bob).root + xml = Nokogiri::XML(comment_xml_bob_legacy_order).root entity = Entity.entity_class(xml.name).from_xml(xml) expect(entity.author).to eq(author) @@ -145,7 +142,7 @@ module DiasporaFederation end it "parses new xml format" do - xml = Nokogiri::XML(new_format_comment_xml_bob).root + xml = Nokogiri::XML(comment_xml_bob).root entity = Entity.entity_class(xml.name).from_xml(xml) expect(entity.author).to eq(author) @@ -153,7 +150,7 @@ module DiasporaFederation end it "parses new data with new xml format" do - xml = Nokogiri::XML(new_data_comment_xml_bob).root + xml = Nokogiri::XML(comment_xml_bob_with_new_data).root entity = Entity.entity_class(xml.name).from_xml(xml) expect(entity.author).to eq(author) From 727ccaf71b9a4321c99b9d94457f4b5824b2242a Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Wed, 30 Jun 2021 00:31:54 +0200 Subject: [PATCH 8/8] Delete legacy_helper.rb --- .../salmon/magic_envelope_spec.rb | 8 +++ spec/support/legacy_helper.rb | 68 ------------------- 2 files changed, 8 insertions(+), 68 deletions(-) delete mode 100644 spec/support/legacy_helper.rb diff --git a/spec/lib/diaspora_federation/salmon/magic_envelope_spec.rb b/spec/lib/diaspora_federation/salmon/magic_envelope_spec.rb index b3f1b27..7a9faf1 100644 --- a/spec/lib/diaspora_federation/salmon/magic_envelope_spec.rb +++ b/spec/lib/diaspora_federation/salmon/magic_envelope_spec.rb @@ -16,6 +16,14 @@ module DiasporaFederation [data, type, enc, alg].map {|i| Base64.urlsafe_encode64(i) }.join(".") end + def encrypt_magic_env(magic_env) + DiasporaFederation::Salmon::AES.generate_key_and_iv.tap do |key| + magic_env.instance_variable_set( + "@payload_data", DiasporaFederation::Salmon::AES.encrypt(magic_env.send(:payload_data), key[:key], key[:iv]) + ) + end + end + context "sanity" do it "constructs an instance" do expect { diff --git a/spec/support/legacy_helper.rb b/spec/support/legacy_helper.rb deleted file mode 100644 index d938530..0000000 --- a/spec/support/legacy_helper.rb +++ /dev/null @@ -1,68 +0,0 @@ -# frozen_string_literal: true - -# This file only exists to generate legacy XMLs to test that we can still parse it. - -def generate_legacy_salmon_slap(entity, sender, sender_privkey) - build_salmon_slap_xml do |xml| - xml.header { - xml.author_id(sender) - } - - xml.parent << DiasporaFederation::Salmon::MagicEnvelope.new(entity, sender).envelop(sender_privkey).root - end -end - -def generate_legacy_encrypted_salmon_slap(entity, sender, sender_privkey, recipient_pubkey) - magic_envelope = DiasporaFederation::Salmon::MagicEnvelope.new(entity) - cipher_params = encrypt_magic_env(magic_envelope) - - build_salmon_slap_xml do |xml| - xml.encrypted_header(encrypted_header(sender, cipher_params, recipient_pubkey)) - - xml.parent << magic_envelope.envelop(sender_privkey).root - end -end - -def build_salmon_slap_xml - Nokogiri::XML::Builder.new(encoding: "UTF-8") {|xml| - xml.diaspora("xmlns" => DiasporaFederation::Salmon::XMLNS, - "xmlns:me" => DiasporaFederation::Salmon::MagicEnvelope::XMLNS) { - yield xml - } - }.to_xml -end - -def encrypt_magic_env(magic_env) - DiasporaFederation::Salmon::AES.generate_key_and_iv.tap do |key| - magic_env.instance_variable_set( - "@payload_data", DiasporaFederation::Salmon::AES.encrypt(magic_env.send(:payload_data), key[:key], key[:iv]) - ) - end -end - -def encrypted_header(author_id, envelope_key, pubkey) - data = decrypted_header_xml(author_id, strict_base64_encode(envelope_key)) - header_key = DiasporaFederation::Salmon::AES.generate_key_and_iv - ciphertext = DiasporaFederation::Salmon::AES.encrypt(data, header_key[:key], header_key[:iv]) - - json_key = JSON.generate(strict_base64_encode(header_key)) - encrypted_key = Base64.strict_encode64(pubkey.public_encrypt(json_key)) - - json_header = JSON.generate(aes_key: encrypted_key, ciphertext: ciphertext) - - Base64.strict_encode64(json_header) -end - -def decrypted_header_xml(author_id, envelope_key) - Nokogiri::XML::Builder.new(encoding: "UTF-8") {|xml| - xml.decrypted_header { - xml.iv(envelope_key[:iv]) - xml.aes_key(envelope_key[:key]) - xml.author_id(author_id) - } - }.to_xml.strip -end - -def strict_base64_encode(hash) - hash.map {|k, v| [k, Base64.strict_encode64(v)] }.to_h -end