From cd3a7abf4d778f7e3139bcb73a42a9dc4cbcb835 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Sun, 30 Apr 2017 04:00:28 +0200 Subject: [PATCH] Remove sign-code from SignedRetraction and RelayableRetraction Second step of #27 --- .../entities/relayable_retraction.rb | 43 +------ .../entities/signed_retraction.rb | 44 +------ lib/diaspora_federation/test/factories.rb | 14 --- lib/diaspora_federation/validators.rb | 2 - .../relayable_retraction_validator.rb | 15 --- .../validators/signed_retraction_validator.rb | 15 --- .../entities/relayable_retraction_spec.rb | 109 +++--------------- .../entities/retraction_spec.rb | 11 +- .../entities/signed_retraction_spec.rb | 73 +++--------- .../relayable_retraction_validator_spec.rb | 29 ----- .../signed_retraction_validator_spec.rb | 29 ----- spec/support/shared_entity_specs.rb | 15 --- 12 files changed, 44 insertions(+), 355 deletions(-) delete mode 100644 lib/diaspora_federation/validators/relayable_retraction_validator.rb delete mode 100644 lib/diaspora_federation/validators/signed_retraction_validator.rb delete mode 100644 spec/lib/diaspora_federation/validators/relayable_retraction_validator_spec.rb delete mode 100644 spec/lib/diaspora_federation/validators/signed_retraction_validator_spec.rb diff --git a/lib/diaspora_federation/entities/relayable_retraction.rb b/lib/diaspora_federation/entities/relayable_retraction.rb index 85570ca..aa7b03f 100644 --- a/lib/diaspora_federation/entities/relayable_retraction.rb +++ b/lib/diaspora_federation/entities/relayable_retraction.rb @@ -53,50 +53,13 @@ module DiasporaFederation # @return [String] target author signature property :target_author_signature, :string, default: nil - # @!attribute [r] target - # Target entity - # @return [RelatedEntity] target entity - entity :target, Entities::RelatedEntity - - # Use only {Retraction} for receive - # @return [Retraction] instance as normal retraction - def to_retraction - Retraction.new(author: author, target_guid: target_guid, target_type: target_type, target: target) - end - - # @return [String] string representation of this object - def to_s - "RelayableRetraction:#{target_type}:#{target_guid}" + def initialize(*) + raise "Sending RelayableRetraction is not supported anymore! Use Retraction instead!" end # @return [Retraction] instance def self.from_hash(hash) - hash[:target] = Retraction.send(:fetch_target, hash[:target_type], hash[:target_guid]) - new(hash).to_retraction - end - - private - - # It updates also the signatures with the keys of the author and the parent - # if the signatures are not there yet and if the keys are available. - # - # @return [Hash] xml elements with updated signatures - def enriched_properties - privkey = DiasporaFederation.callbacks.trigger(:fetch_private_key, author) - - super.tap do |hash| - fill_required_signature(privkey, hash) unless privkey.nil? - end - end - - # @param [OpenSSL::PKey::RSA] privkey private key of sender - # @param [Hash] hash hash given for a signing - def fill_required_signature(privkey, hash) - if target.parent.author == author && parent_author_signature.nil? - hash[:parent_author_signature] = SignedRetraction.sign_with_key(privkey, self) - elsif target.author == author && target_author_signature.nil? - hash[:target_author_signature] = SignedRetraction.sign_with_key(privkey, self) - end + Retraction.from_hash(hash) end end end diff --git a/lib/diaspora_federation/entities/signed_retraction.rb b/lib/diaspora_federation/entities/signed_retraction.rb index 389543e..af850b9 100644 --- a/lib/diaspora_federation/entities/signed_retraction.rb +++ b/lib/diaspora_federation/entities/signed_retraction.rb @@ -30,51 +30,13 @@ module DiasporaFederation # @return [String] author signature property :target_author_signature, :string, default: nil - # @!attribute [r] target - # Target entity - # @return [RelatedEntity] target entity - entity :target, Entities::RelatedEntity - - # Use only {Retraction} for receive - # @return [Retraction] instance as normal retraction - def to_retraction - Retraction.new(author: author, target_guid: target_guid, target_type: target_type, target: target) - end - - # Create signature for a retraction - # @param [OpenSSL::PKey::RSA] privkey private key of sender - # @param [SignedRetraction, RelayableRetraction] ret the retraction to sign - # @return [String] a Base64 encoded signature of the retraction with the key - def self.sign_with_key(privkey, ret) - Base64.strict_encode64(privkey.sign(Relayable::DIGEST, [ret.target_guid, ret.target_type].join(";"))) - end - - # @return [String] string representation of this object - def to_s - "SignedRetraction:#{target_type}:#{target_guid}" + def initialize(*) + raise "Sending SignedRetraction is not supported anymore! Use Retraction instead!" end # @return [Retraction] instance def self.from_hash(hash) - hash[:target] = Retraction.send(:fetch_target, hash[:target_type], hash[:target_guid]) - new(hash).to_retraction - end - - private - - # It updates also the signatures with the keys of the author and the parent - # if the signatures are not there yet and if the keys are available. - # - # @return [Hash] xml elements with updated signatures - def enriched_properties - super.tap do |hash| - hash[:target_author_signature] = target_author_signature || sign_with_author.to_s - end - end - - def sign_with_author - privkey = DiasporaFederation.callbacks.trigger(:fetch_private_key, author) - SignedRetraction.sign_with_key(privkey, self) unless privkey.nil? + Retraction.from_hash(hash) end end end diff --git a/lib/diaspora_federation/test/factories.rb b/lib/diaspora_federation/test/factories.rb index 8e6fd41..0a3453e 100644 --- a/lib/diaspora_federation/test/factories.rb +++ b/lib/diaspora_federation/test/factories.rb @@ -150,13 +150,6 @@ module DiasporaFederation conversation_guid { Fabricate.sequence(:guid) } end - Fabricator(:relayable_retraction_entity, class_name: DiasporaFederation::Entities::RelayableRetraction) do - author { Fabricate.sequence(:diaspora_id) } - target_guid { Fabricate.sequence(:guid) } - target_type "Comment" - target {|attrs| Fabricate(:related_entity, author: attrs[:author]) } - end - Fabricator(:reshare_entity, class_name: DiasporaFederation::Entities::Reshare) do root_author { Fabricate.sequence(:diaspora_id) } root_guid { Fabricate.sequence(:guid) } @@ -174,13 +167,6 @@ module DiasporaFederation target {|attrs| Fabricate(:related_entity, author: attrs[:author]) } end - Fabricator(:signed_retraction_entity, class_name: DiasporaFederation::Entities::SignedRetraction) do - author { Fabricate.sequence(:diaspora_id) } - target_guid { Fabricate.sequence(:guid) } - target_type "Post" - target {|attrs| Fabricate(:related_entity, author: attrs[:author]) } - end - Fabricator(:poll_answer_entity, class_name: DiasporaFederation::Entities::PollAnswer) do guid { Fabricate.sequence(:guid) } answer { "Obama is a bicycle" } diff --git a/lib/diaspora_federation/validators.rb b/lib/diaspora_federation/validators.rb index 5b0d3ed..63db3f8 100644 --- a/lib/diaspora_federation/validators.rb +++ b/lib/diaspora_federation/validators.rb @@ -64,6 +64,4 @@ require "diaspora_federation/validators/status_message_validator" require "diaspora_federation/validators/web_finger_validator" # deprecated -require "diaspora_federation/validators/relayable_retraction_validator" require "diaspora_federation/validators/request_validator" -require "diaspora_federation/validators/signed_retraction_validator" diff --git a/lib/diaspora_federation/validators/relayable_retraction_validator.rb b/lib/diaspora_federation/validators/relayable_retraction_validator.rb deleted file mode 100644 index be5e4ad..0000000 --- a/lib/diaspora_federation/validators/relayable_retraction_validator.rb +++ /dev/null @@ -1,15 +0,0 @@ -module DiasporaFederation - module Validators - # This validates a {Entities::RelayableRetraction}. - # @deprecated The {Entities::RelayableRetraction} will be replaced with {Entities::Retraction}. - class RelayableRetractionValidator < Validation::Validator - include Validation - - rule :target_guid, :guid - rule :target_type, :not_empty - rule :target, :not_nil - - rule :author, %i(not_empty diaspora_id) - end - end -end diff --git a/lib/diaspora_federation/validators/signed_retraction_validator.rb b/lib/diaspora_federation/validators/signed_retraction_validator.rb deleted file mode 100644 index 414784f..0000000 --- a/lib/diaspora_federation/validators/signed_retraction_validator.rb +++ /dev/null @@ -1,15 +0,0 @@ -module DiasporaFederation - module Validators - # This validates a {Entities::SignedRetraction}. - # @deprecated The {Entities::RelayableRetraction} will be replaced with {Entities::Retraction}. - class SignedRetractionValidator < Validation::Validator - include Validation - - rule :target_guid, :guid - rule :target_type, :not_empty - rule :target, :not_nil - - rule :author, %i(not_empty diaspora_id) - 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 index 1b41c7a..67bf69d 100644 --- a/spec/lib/diaspora_federation/entities/relayable_retraction_spec.rb +++ b/spec/lib/diaspora_federation/entities/relayable_retraction_spec.rb @@ -8,117 +8,36 @@ module DiasporaFederation parent: Fabricate(:related_entity, author: alice.diaspora_id) ) } - let(:data) { - Fabricate( - :relayable_retraction_entity, - author: alice.diaspora_id, - target_guid: target.guid, - target_type: target.entity_type, - target: target_entity - ).send(:enriched_properties).tap do |data| - data[:target_author_signature] = "" - data[:target] = target_entity - end - } + let(:data) { {author: alice.diaspora_id, target_guid: target.guid, target_type: target.entity_type} } let(:xml) { <<-XML } - #{data[:parent_author_signature]} + #{data[:target_guid]} #{data[:target_type]} - #{data[:author]} + #{data[:author]} XML - let(:string) { "RelayableRetraction:#{data[:target_type]}:#{data[:target_guid]}" } - - it_behaves_like "an Entity subclass" - - it_behaves_like "an XML Entity", %i(parent_author_signature target_author_signature target) - - it_behaves_like "a retraction" - - describe "#to_xml" do - let(:author_pkey) { OpenSSL::PKey::RSA.generate(1024) } - let(:hash) { Fabricate.attributes_for(:relayable_retraction_entity) } - - it "updates author signature when it was nil and key was supplied and author is not parent author" do - parent = Fabricate(:related_entity, author: bob.diaspora_id) - hash[:target] = Fabricate(:related_entity, author: hash[:author], parent: parent) - - expect_callback(:fetch_private_key, hash[:author]).and_return(author_pkey) - - signed_string = "#{hash[:target_guid]};#{hash[:target_type]}" - - xml = Entities::RelayableRetraction.new(hash).to_xml - - signature = Base64.decode64(xml.at_xpath("target_author_signature").text) - expect(author_pkey.verify(OpenSSL::Digest::SHA256.new, signature, signed_string)).to be_truthy - end - - it "sets parent author signature when author is parent author" do - parent = Fabricate(:related_entity, author: hash[:author]) - hash[:target] = Fabricate(:related_entity, author: hash[:author], parent: parent) - - expect_callback(:fetch_private_key, hash[:author]).and_return(author_pkey) - - signed_string = "#{hash[:target_guid]};#{hash[:target_type]}" - - xml = Entities::RelayableRetraction.new(hash).to_xml - - signature = Base64.decode64(xml.at_xpath("parent_author_signature").text) - expect(author_pkey.verify(OpenSSL::Digest::SHA256.new, signature, signed_string)).to be_truthy - end - - it "updates parent author signature when it was nil, key was supplied and sender is author of the parent" do - parent = Fabricate(:related_entity, author: hash[:author]) - hash[:target] = Fabricate(:related_entity, author: bob.diaspora_id, parent: parent) - - expect_callback(:fetch_private_key, hash[:author]).and_return(author_pkey) - - signed_string = "#{hash[:target_guid]};#{hash[:target_type]}" - - xml = Entities::RelayableRetraction.new(hash).to_xml - - signature = Base64.decode64(xml.at_xpath("parent_author_signature").text) - expect(author_pkey.verify(OpenSSL::Digest::SHA256.new, signature, signed_string)).to be_truthy - end - - it "doesn't change signatures if they are already set" do - hash.merge!(target_author_signature: "aa", parent_author_signature: "bb") - - xml = Entities::RelayableRetraction.new(hash).to_xml - - expect(xml.at_xpath("target_author_signature").text).to eq("aa") - expect(xml.at_xpath("parent_author_signature").text).to eq("bb") - end - - it "doesn't change signatures if keys weren't supplied" do - expect_callback(:fetch_private_key, hash[:author]).and_return(nil) - - xml = Entities::RelayableRetraction.new(hash).to_xml - expect(xml.at_xpath("target_author_signature").text).to eq("") - expect(xml.at_xpath("parent_author_signature").text).to eq("") - end - end - - describe "#to_retraction" do - it "copies the attributes to a Retraction" do - relayable_retraction = Fabricate(:relayable_retraction_entity) - retraction = relayable_retraction.to_retraction - - expect(retraction).to be_a(Entities::Retraction) - expect(retraction.author).to eq(relayable_retraction.author) - expect(retraction.target_guid).to eq(relayable_retraction.target_guid) - expect(retraction.target_type).to eq(relayable_retraction.target_type) + 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::Document.parse(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 diff --git a/spec/lib/diaspora_federation/entities/retraction_spec.rb b/spec/lib/diaspora_federation/entities/retraction_spec.rb index 09b7287..aad6821 100644 --- a/spec/lib/diaspora_federation/entities/retraction_spec.rb +++ b/spec/lib/diaspora_federation/entities/retraction_spec.rb @@ -25,7 +25,16 @@ XML it_behaves_like "an XML Entity" - it_behaves_like "a retraction" + context "receive with no target found" do + it "raises when no target is found" do + unknown_guid = Fabricate.sequence(:guid) + retraction = Entities::Retraction.new(data.merge(target_guid: unknown_guid)) + expect { + described_class.from_xml(retraction.to_xml) + }.to raise_error DiasporaFederation::Entities::Retraction::TargetNotFound, + "not found: #{data[:target_type]}:#{unknown_guid}" + end + end describe "#sender_valid?" do context "unrelayable target" do diff --git a/spec/lib/diaspora_federation/entities/signed_retraction_spec.rb b/spec/lib/diaspora_federation/entities/signed_retraction_spec.rb index 20e038e..b1582d4 100644 --- a/spec/lib/diaspora_federation/entities/signed_retraction_spec.rb +++ b/spec/lib/diaspora_federation/entities/signed_retraction_spec.rb @@ -2,80 +2,35 @@ module DiasporaFederation describe Entities::SignedRetraction do let(:target) { Fabricate(:post, author: alice) } let(:target_entity) { Fabricate(:related_entity, author: alice.diaspora_id) } - let(:data) { - Fabricate( - :signed_retraction_entity, - author: alice.diaspora_id, - target_guid: target.guid, - target_type: target.entity_type, - target: target_entity - ).send(:enriched_properties).merge(target: target_entity) - } + 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]} - #{data[:target_author_signature]} + #{data[:author]} + XML - let(:string) { "SignedRetraction:#{data[:target_type]}:#{data[:target_guid]}" } - - it_behaves_like "an Entity subclass" - - it_behaves_like "an XML Entity", [:target_author_signature] - - it_behaves_like "a retraction" - - describe "#to_xml" do - let(:author_pkey) { OpenSSL::PKey::RSA.generate(1024) } - let(:hash) { Fabricate.attributes_for(:signed_retraction_entity) } - - it "updates author signature when it was nil and key was supplied" do - expect_callback(:fetch_private_key, hash[:author]).and_return(author_pkey) - - signed_string = "#{hash[:target_guid]};#{hash[:target_type]}" - - xml = Entities::SignedRetraction.new(hash).to_xml - - signature = Base64.decode64(xml.at_xpath("target_author_signature").text) - expect(author_pkey.verify(OpenSSL::Digest::SHA256.new, signature, signed_string)).to be_truthy - end - - it "doesn't change signature if it is already set" do - hash[:target_author_signature] = "aa" - - xml = Entities::SignedRetraction.new(hash).to_xml - - expect(xml.at_xpath("target_author_signature").text).to eq("aa") - end - - it "doesn't change signature if a key wasn't supplied" do - expect_callback(:fetch_private_key, hash[:author]).and_return(nil) - - xml = Entities::SignedRetraction.new(hash).to_xml - expect(xml.at_xpath("target_author_signature").text).to eq("") - end - end - - describe "#to_retraction" do - it "copies the attributes to a Retraction" do - signed_retraction = Fabricate(:signed_retraction_entity) - retraction = signed_retraction.to_retraction - - expect(retraction).to be_a(Entities::Retraction) - expect(retraction.author).to eq(signed_retraction.author) - expect(retraction.target_guid).to eq(signed_retraction.target_guid) - expect(retraction.target_type).to eq(signed_retraction.target_type) + 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::Document.parse(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 diff --git a/spec/lib/diaspora_federation/validators/relayable_retraction_validator_spec.rb b/spec/lib/diaspora_federation/validators/relayable_retraction_validator_spec.rb deleted file mode 100644 index 6d1495c..0000000 --- a/spec/lib/diaspora_federation/validators/relayable_retraction_validator_spec.rb +++ /dev/null @@ -1,29 +0,0 @@ -module DiasporaFederation - describe Validators::RelayableRetractionValidator do - let(:entity) { :relayable_retraction_entity } - it_behaves_like "a common validator" - - it_behaves_like "a diaspora* ID validator" do - let(:property) { :author } - let(:mandatory) { true } - end - - it_behaves_like "a guid validator" do - let(:property) { :target_guid } - end - - describe "#target_type" do - it_behaves_like "a property that mustn't be empty" do - let(:property) { :target_type } - end - end - - describe "#target" do - it_behaves_like "a property with a value validation/restriction" do - let(:property) { :target } - let(:wrong_values) { [nil] } - let(:correct_values) { [Fabricate(:related_entity)] } - end - end - end -end diff --git a/spec/lib/diaspora_federation/validators/signed_retraction_validator_spec.rb b/spec/lib/diaspora_federation/validators/signed_retraction_validator_spec.rb deleted file mode 100644 index 27e9fcf..0000000 --- a/spec/lib/diaspora_federation/validators/signed_retraction_validator_spec.rb +++ /dev/null @@ -1,29 +0,0 @@ -module DiasporaFederation - describe Validators::SignedRetractionValidator do - let(:entity) { :signed_retraction_entity } - it_behaves_like "a common validator" - - it_behaves_like "a diaspora* ID validator" do - let(:property) { :author } - let(:mandatory) { true } - end - - it_behaves_like "a guid validator" do - let(:property) { :target_guid } - end - - describe "#target_type" do - it_behaves_like "a property that mustn't be empty" do - let(:property) { :target_type } - end - end - - describe "#target" do - it_behaves_like "a property with a value validation/restriction" do - let(:property) { :target } - let(:wrong_values) { [nil] } - let(:correct_values) { [Fabricate(:related_entity)] } - end - end - end -end diff --git a/spec/support/shared_entity_specs.rb b/spec/support/shared_entity_specs.rb index 1b609ff..66c97f3 100644 --- a/spec/support/shared_entity_specs.rb +++ b/spec/support/shared_entity_specs.rb @@ -120,21 +120,6 @@ shared_examples "a relayable Entity" do end end -shared_examples "a retraction" do - context "receive with no target found" do - let(:unknown_guid) { Fabricate.sequence(:guid) } - let(:instance) { described_class.new(data.merge(target_guid: unknown_guid)) } - - it "raises when no target is found" do - xml = instance.to_xml - expect { - described_class.from_xml(xml) - }.to raise_error DiasporaFederation::Entities::Retraction::TargetNotFound, - "not found: #{data[:target_type]}:#{unknown_guid}" - end - end -end - shared_examples "a JSON Entity" do describe "#to_json" do it "#to_json output matches JSON schema" do