From d79c2e511b2a463d0d33ebe963b2d09fe14b0f31 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Tue, 25 Apr 2017 01:03:34 +0200 Subject: [PATCH 01/24] Bump diaspora_federation --- Gemfile | 4 ++-- Gemfile.lock | 21 +++++++++++---------- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/Gemfile b/Gemfile index 634196383..f27b8d1a0 100644 --- a/Gemfile +++ b/Gemfile @@ -13,7 +13,7 @@ gem "unicorn-worker-killer", "0.4.4" # Federation -gem "diaspora_federation-rails", "0.1.9" +gem "diaspora_federation-rails", "0.2.0" # API and JSON @@ -292,7 +292,7 @@ group :test do gem "webmock", "2.3.2", require: false gem "shoulda-matchers", "3.1.1" - gem "diaspora_federation-test", "0.1.9" + gem "diaspora_federation-test", "0.2.0" # Coverage gem 'coveralls', require: false diff --git a/Gemfile.lock b/Gemfile.lock index 50fc95720..e70cd54c9 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -163,18 +163,18 @@ GEM devise rails (>= 3.0.4) diaspora-prosody-config (0.0.7) - diaspora_federation (0.1.9) - faraday (>= 0.9.0, < 0.12.0) + diaspora_federation (0.2.0) + faraday (>= 0.9.0, < 0.13.0) faraday_middleware (>= 0.10.0, < 0.12.0) nokogiri (~> 1.6, >= 1.6.8) typhoeus (~> 1.0) valid (~> 1.0) - diaspora_federation-rails (0.1.9) - diaspora_federation (= 0.1.9) - rails (>= 4.2, < 6) - diaspora_federation-test (0.1.9) - diaspora_federation (= 0.1.9) - factory_girl (~> 4.7) + diaspora_federation-rails (0.2.0) + actionpack (>= 4.2, < 6) + diaspora_federation (= 0.2.0) + diaspora_federation-test (0.2.0) + diaspora_federation (= 0.2.0) + fabrication (~> 2.16.0) uuid (~> 2.3.8) diff-lcs (1.3) docile (1.1.5) @@ -198,6 +198,7 @@ GEM sigar (~> 0.7.3) state_machines thor + fabrication (2.16.1) factory_girl (4.8.0) activesupport (>= 3.0.0) factory_girl_rails (4.8.0) @@ -775,8 +776,8 @@ DEPENDENCIES devise (= 4.2.0) devise_lastseenable (= 0.0.6) diaspora-prosody-config (= 0.0.7) - diaspora_federation-rails (= 0.1.9) - diaspora_federation-test (= 0.1.9) + diaspora_federation-rails (= 0.2.0) + diaspora_federation-test (= 0.2.0) entypo-rails (= 3.0.0) eye (= 0.9.1) factory_girl_rails (= 4.8.0) From 78c7156e722b0453e70d56ab18305a32bfa8f6f5 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Tue, 25 Apr 2017 01:10:03 +0200 Subject: [PATCH 02/24] Replace FactoryGirl with Fabricate for federation factories --- spec/factories.rb | 3 +- spec/federation_callbacks_spec.rb | 20 +++--- spec/helper_methods.rb | 2 +- .../federation/federation_helper.rb | 2 +- .../receive_federation_messages_spec.rb | 12 ++-- .../federation/shared_receive_retraction.rb | 2 +- .../federation/shared_receive_stream_items.rb | 4 +- spec/integration/mentioning_spec.rb | 2 +- spec/lib/diaspora/federation/receive_spec.rb | 61 +++++++++---------- .../notifications/private_message_spec.rb | 2 +- 10 files changed, 52 insertions(+), 58 deletions(-) diff --git a/spec/factories.rb b/spec/factories.rb index d89eb7d95..e6ce43525 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -10,8 +10,7 @@ def r_str SecureRandom.hex(3) end -require "diaspora_federation/test" -DiasporaFederation::Test::Factories.federation_factories +require "diaspora_federation/test/factories" FactoryGirl.define do factory :profile do diff --git a/spec/federation_callbacks_spec.rb b/spec/federation_callbacks_spec.rb index 253484ded..df72b7b78 100644 --- a/spec/federation_callbacks_spec.rb +++ b/spec/federation_callbacks_spec.rb @@ -203,7 +203,7 @@ describe "diaspora federation callbacks" do it "returns nil for an unknown id" do expect( - DiasporaFederation.callbacks.trigger(:fetch_private_key, FactoryGirl.generate(:diaspora_id)) + DiasporaFederation.callbacks.trigger(:fetch_private_key, Fabricate.sequence(:diaspora_id)) ).to be_nil end end @@ -225,7 +225,7 @@ describe "diaspora federation callbacks" do end it "returns nil for an unknown person" do - diaspora_id = FactoryGirl.generate(:diaspora_id) + diaspora_id = Fabricate.sequence(:diaspora_id) expect(Person).to receive(:find_or_fetch_by_identifier).with(diaspora_id) .and_raise(DiasporaFederation::Discovery::DiscoveryError) @@ -295,7 +295,7 @@ describe "diaspora federation callbacks" do it "returns nil for a non-existing guid" do expect( - DiasporaFederation.callbacks.trigger(:fetch_related_entity, "Post", FactoryGirl.generate(:guid)) + DiasporaFederation.callbacks.trigger(:fetch_related_entity, "Post", Fabricate.sequence(:guid)) ).to be_nil end end @@ -337,7 +337,7 @@ describe "diaspora federation callbacks" do describe ":receive_entity" do it "receives an AccountDeletion" do - account_deletion = FactoryGirl.build(:account_deletion_entity, author: remote_person.diaspora_handle) + account_deletion = Fabricate(:account_deletion_entity, author: remote_person.diaspora_handle) expect(Diaspora::Federation::Receive).to receive(:account_deletion).with(account_deletion) expect(Workers::ReceiveLocal).not_to receive(:perform_async) @@ -346,7 +346,7 @@ describe "diaspora federation callbacks" do end it "receives a Retraction" do - retraction = FactoryGirl.build(:retraction_entity, author: remote_person.diaspora_handle) + retraction = Fabricate(:retraction_entity, author: remote_person.diaspora_handle) expect(Diaspora::Federation::Receive).to receive(:retraction).with(retraction, 42) expect(Workers::ReceiveLocal).not_to receive(:perform_async) @@ -355,7 +355,7 @@ describe "diaspora federation callbacks" do end it "receives a entity" do - received = FactoryGirl.build(:status_message_entity, author: remote_person.diaspora_handle) + received = Fabricate(:status_message_entity, author: remote_person.diaspora_handle) persisted = FactoryGirl.create(:status_message) expect(Diaspora::Federation::Receive).to receive(:perform).with(received).and_return(persisted) @@ -365,7 +365,7 @@ describe "diaspora federation callbacks" do end it "calls schedule_check_if_needed on the senders pod" do - received = FactoryGirl.build(:status_message_entity, author: remote_person.diaspora_handle) + received = Fabricate(:status_message_entity, author: remote_person.diaspora_handle) persisted = FactoryGirl.create(:status_message) expect(Person).to receive(:by_account_identifier).with(received.author).and_return(remote_person) @@ -377,7 +377,7 @@ describe "diaspora federation callbacks" do end it "receives a entity for a recipient" do - received = FactoryGirl.build(:status_message_entity, author: remote_person.diaspora_handle) + received = Fabricate(:status_message_entity, author: remote_person.diaspora_handle) persisted = FactoryGirl.create(:status_message) expect(Diaspora::Federation::Receive).to receive(:perform).with(received).and_return(persisted) @@ -387,7 +387,7 @@ describe "diaspora federation callbacks" do end it "does not trigger a ReceiveLocal job if Receive.perform returned nil" do - received = FactoryGirl.build(:status_message_entity, author: remote_person.diaspora_handle) + received = Fabricate(:status_message_entity, author: remote_person.diaspora_handle) expect(Diaspora::Federation::Receive).to receive(:perform).with(received).and_return(nil) expect(Workers::ReceiveLocal).not_to receive(:perform_async) @@ -460,7 +460,7 @@ describe "diaspora federation callbacks" do end it "forwards the DiscoveryError" do - diaspora_id = FactoryGirl.generate(:diaspora_id) + diaspora_id = Fabricate.sequence(:diaspora_id) expect(Person).to receive(:find_or_fetch_by_identifier).with(diaspora_id) .and_raise(DiasporaFederation::Discovery::DiscoveryError) diff --git a/spec/helper_methods.rb b/spec/helper_methods.rb index c1992dc82..c23ee2440 100644 --- a/spec/helper_methods.rb +++ b/spec/helper_methods.rb @@ -52,7 +52,7 @@ module HelperMethods end def build_relayable_federation_entity(type, data={}, additional_xml_elements={}) - attributes = FactoryGirl.attributes_for("#{type}_entity".to_sym, data) + attributes = Fabricate.attributes_for("#{type}_entity".to_sym, data) entity_class = "DiasporaFederation::Entities::#{type.capitalize}".constantize signable_fields = attributes.keys - [:author_signature] diff --git a/spec/integration/federation/federation_helper.rb b/spec/integration/federation/federation_helper.rb index afc880792..56e8a86f9 100644 --- a/spec/integration/federation/federation_helper.rb +++ b/spec/integration/federation/federation_helper.rb @@ -36,7 +36,7 @@ def create_relayable_entity(entity_name, parent, diaspora_id) ).at_least(1).times.and_return(nil) if parent == local_parent parent_guid = parent.guid - FactoryGirl.build( + Fabricate( entity_name, conversation_guid: parent_guid, parent_guid: parent_guid, diff --git a/spec/integration/federation/receive_federation_messages_spec.rb b/spec/integration/federation/receive_federation_messages_spec.rb index 85a2b142d..d16676468 100644 --- a/spec/integration/federation/receive_federation_messages_spec.rb +++ b/spec/integration/federation/receive_federation_messages_spec.rb @@ -21,7 +21,7 @@ describe "Receive federation messages feature" do end it "rejects account deletion with wrong diaspora_id" do - delete_id = FactoryGirl.generate(:diaspora_id) + delete_id = Fabricate.sequence(:diaspora_id) post_message(generate_xml(DiasporaFederation::Entities::AccountDeletion.new(diaspora_id: delete_id), sender)) expect(AccountDeletion.exists?(diaspora_handle: delete_id)).to be_falsey @@ -31,7 +31,7 @@ describe "Receive federation messages feature" do context "reshare" do it "reshare of public post passes" do post = FactoryGirl.create(:status_message, author: alice.person, public: true) - reshare = FactoryGirl.build( + reshare = Fabricate( :reshare_entity, root_author: alice.diaspora_handle, root_guid: post.guid, author: sender_id) expect(Participation::Generator).to receive(:new).with( @@ -46,7 +46,7 @@ describe "Receive federation messages feature" do it "reshare of private post fails" do post = FactoryGirl.create(:status_message, author: alice.person, public: false) - reshare = FactoryGirl.build( + reshare = Fabricate( :reshare_entity, root_author: alice.diaspora_handle, root_guid: post.guid, author: sender_id) expect { post_message(generate_xml(reshare, sender)) @@ -74,7 +74,7 @@ describe "Receive federation messages feature" do let(:recipient) { alice } it "treats sharing request recive correctly" do - entity = FactoryGirl.build(:request_entity, author: sender_id, recipient: alice.diaspora_handle) + entity = Fabricate(:request_entity, author: sender_id, recipient: alice.diaspora_handle) expect(Workers::ReceiveLocal).to receive(:perform_async).and_call_original @@ -105,7 +105,7 @@ describe "Receive federation messages feature" do it_behaves_like "messages which can't be send without sharing" it "treats profile receive correctly" do - entity = FactoryGirl.build(:profile_entity, author: sender_id) + entity = Fabricate(:profile_entity, author: sender_id) post_message(generate_xml(entity, sender, alice), alice) received_profile = sender.profile.reload @@ -115,7 +115,7 @@ describe "Receive federation messages feature" do end it "receives conversation correctly" do - entity = FactoryGirl.build( + entity = Fabricate( :conversation_entity, author: sender_id, participants: "#{sender_id};#{alice.diaspora_handle}" diff --git a/spec/integration/federation/shared_receive_retraction.rb b/spec/integration/federation/shared_receive_retraction.rb index afa415993..b54588524 100644 --- a/spec/integration/federation/shared_receive_retraction.rb +++ b/spec/integration/federation/shared_receive_retraction.rb @@ -1,5 +1,5 @@ def retraction_entity(entity_name, target_object, sender) - FactoryGirl.build( + Fabricate( entity_name, author: sender.diaspora_handle, target_guid: target_object.guid, diff --git a/spec/integration/federation/shared_receive_stream_items.rb b/spec/integration/federation/shared_receive_stream_items.rb index 391c194cf..67d5766b0 100644 --- a/spec/integration/federation/shared_receive_stream_items.rb +++ b/spec/integration/federation/shared_receive_stream_items.rb @@ -4,7 +4,7 @@ shared_examples_for "messages which are indifferent about sharing fact" do let(:public) { recipient.nil? } it "treats status message receive correctly" do - entity = FactoryGirl.build(:status_message_entity, author: sender_id, public: public) + entity = Fabricate(:status_message_entity, author: sender_id, public: public) post_message(generate_xml(entity, sender, recipient), recipient) @@ -13,7 +13,7 @@ shared_examples_for "messages which are indifferent about sharing fact" do it "doesn't accept status message with wrong signature" do allow(sender).to receive(:encryption_key).and_return(OpenSSL::PKey::RSA.new(1024)) - entity = FactoryGirl.build(:status_message_entity, author: sender_id, public: public) + entity = Fabricate(:status_message_entity, author: sender_id, public: public) post_message(generate_xml(entity, sender, recipient), recipient) diff --git a/spec/integration/mentioning_spec.rb b/spec/integration/mentioning_spec.rb index 367758bf8..94bd5f8cf 100644 --- a/spec/integration/mentioning_spec.rb +++ b/spec/integration/mentioning_spec.rb @@ -73,7 +73,7 @@ module MentioningSpecHelpers end def receive_status_message_via_federation(text, *recipients) - entity = FactoryGirl.build( + entity = Fabricate( :status_message_entity, author: remote_raphael.diaspora_handle, text: text, diff --git a/spec/lib/diaspora/federation/receive_spec.rb b/spec/lib/diaspora/federation/receive_spec.rb index c31f6332d..680a34190 100644 --- a/spec/lib/diaspora/federation/receive_spec.rb +++ b/spec/lib/diaspora/federation/receive_spec.rb @@ -3,7 +3,7 @@ describe Diaspora::Federation::Receive do let(:post) { FactoryGirl.create(:status_message, text: "hello", public: true, author: alice.person) } describe ".account_deletion" do - let(:account_deletion_entity) { FactoryGirl.build(:account_deletion_entity, author: sender.diaspora_handle) } + let(:account_deletion_entity) { Fabricate(:account_deletion_entity, author: sender.diaspora_handle) } it "saves the account deletion" do Diaspora::Federation::Receive.account_deletion(account_deletion_entity) @@ -66,7 +66,7 @@ describe Diaspora::Federation::Receive do describe ".contact" do let(:contact_entity) { - FactoryGirl.build(:contact_entity, author: sender.diaspora_handle, recipient: alice.diaspora_handle) + Fabricate(:contact_entity, author: sender.diaspora_handle, recipient: alice.diaspora_handle) } it "creates the contact if it doesn't exist" do @@ -99,7 +99,7 @@ describe Diaspora::Federation::Receive do context "sharing=false" do let(:unshare_contact_entity) { - FactoryGirl.build( + Fabricate( :contact_entity, author: sender.diaspora_handle, recipient: alice.diaspora_handle, @@ -128,9 +128,9 @@ describe Diaspora::Federation::Receive do end describe ".conversation" do - let(:conv_guid) { FactoryGirl.generate(:guid) } + let(:conv_guid) { Fabricate.sequence(:guid) } let(:message_entity) { - FactoryGirl.build( + Fabricate( :message_entity, author: alice.diaspora_handle, parent_guid: conv_guid, @@ -138,7 +138,7 @@ describe Diaspora::Federation::Receive do ) } let(:conversation_entity) { - FactoryGirl.build( + Fabricate( :conversation_entity, guid: conv_guid, author: alice.diaspora_handle, @@ -184,7 +184,7 @@ describe Diaspora::Federation::Receive do describe ".like" do let(:like_data) { - FactoryGirl.attributes_for( + Fabricate.attributes_for( :like_entity, author: sender.diaspora_handle, parent_guid: post.guid, @@ -241,7 +241,7 @@ describe Diaspora::Federation::Receive do end } let(:message_entity) { - FactoryGirl.build( + Fabricate( :message_entity, author: sender.diaspora_handle, parent_guid: conversation.guid, @@ -276,7 +276,7 @@ describe Diaspora::Federation::Receive do describe ".participation" do let(:participation_entity) { - FactoryGirl.build(:participation_entity, author: sender.diaspora_handle, parent_guid: post.guid) + Fabricate(:participation_entity, author: sender.diaspora_handle, parent_guid: post.guid) } it "saves the participation" do @@ -303,7 +303,7 @@ describe Diaspora::Federation::Receive do end describe ".photo" do - let(:photo_entity) { FactoryGirl.build(:photo_entity, author: sender.diaspora_handle) } + let(:photo_entity) { Fabricate(:photo_entity, author: sender.diaspora_handle) } it "saves the photo if it does not already exist" do received = Diaspora::Federation::Receive.perform(photo_entity) @@ -353,7 +353,7 @@ describe Diaspora::Federation::Receive do describe ".poll_participation" do let(:post_with_poll) { FactoryGirl.create(:status_message_with_poll, author: alice.person) } let(:poll_participation_data) { - FactoryGirl.attributes_for( + Fabricate.attributes_for( :poll_participation_entity, author: sender.diaspora_handle, parent_guid: post_with_poll.poll.guid, @@ -406,7 +406,7 @@ describe Diaspora::Federation::Receive do end describe ".profile" do - let(:profile_entity) { FactoryGirl.build(:profile_entity, author: sender.diaspora_handle) } + let(:profile_entity) { Fabricate(:profile_entity, author: sender.diaspora_handle) } it "updates the profile of the person" do received = Diaspora::Federation::Receive.perform(profile_entity) @@ -426,7 +426,7 @@ describe Diaspora::Federation::Receive do end describe ".reshare" do - let(:reshare_entity) { FactoryGirl.build(:reshare_entity, author: sender.diaspora_handle, root_guid: post.guid) } + let(:reshare_entity) { Fabricate(:reshare_entity, author: sender.diaspora_handle, root_guid: post.guid) } it "saves the reshare" do received = Diaspora::Federation::Receive.perform(reshare_entity) @@ -456,7 +456,7 @@ describe Diaspora::Federation::Receive do it "destroys the post" do remote_post = FactoryGirl.create(:status_message, author: sender, public: true) - retraction = FactoryGirl.build( + retraction = Fabricate( :retraction_entity, author: sender.diaspora_handle, target_guid: remote_post.guid, @@ -471,12 +471,7 @@ describe Diaspora::Federation::Receive do end it "raises when the post does not exist" do - retraction = FactoryGirl.build( - :retraction_entity, - author: sender.diaspora_handle, - target_guid: FactoryGirl.generate(:guid), - target_type: "Post" - ) + retraction = Fabricate(:retraction_entity, author: sender.diaspora_handle, target_type: "Post") expect { Diaspora::Federation::Receive.retraction(retraction, nil) @@ -486,7 +481,7 @@ describe Diaspora::Federation::Receive do it "disconnects on Person-Retraction" do alice.contacts.find_or_initialize_by(person_id: sender.id, receiving: true, sharing: true).save! - retraction = FactoryGirl.build( + retraction = Fabricate( :retraction_entity, author: sender.diaspora_handle, target_guid: sender.guid, @@ -506,7 +501,7 @@ describe Diaspora::Federation::Receive do local_post = FactoryGirl.create(:status_message, author: alice.person, public: true) remote_comment = FactoryGirl.create(:comment, author: sender, post: local_post) - retraction = FactoryGirl.build( + retraction = Fabricate( :retraction_entity, author: sender.diaspora_handle, target_guid: remote_comment.guid, @@ -529,7 +524,7 @@ describe Diaspora::Federation::Receive do remote_post = FactoryGirl.create(:status_message, author: sender, public: true) remote_comment = FactoryGirl.create(:comment, author: sender, post: remote_post) - retraction = FactoryGirl.build( + retraction = Fabricate( :retraction_entity, author: sender.diaspora_handle, target_guid: remote_comment.guid, @@ -547,7 +542,7 @@ describe Diaspora::Federation::Receive do describe ".status_message" do context "basic status message" do - let(:status_message_entity) { FactoryGirl.build(:status_message_entity, author: sender.diaspora_handle) } + let(:status_message_entity) { Fabricate(:status_message_entity, author: sender.diaspora_handle) } it "saves the status message" do received = Diaspora::Federation::Receive.perform(status_message_entity) @@ -583,7 +578,7 @@ describe Diaspora::Federation::Receive do end it "finds the correct author if the author is not lowercase" do - status_message_entity = FactoryGirl.build(:status_message_entity, author: sender.diaspora_handle.upcase) + status_message_entity = Fabricate(:status_message_entity, author: sender.diaspora_handle.upcase) received = Diaspora::Federation::Receive.perform(status_message_entity) @@ -595,9 +590,9 @@ describe Diaspora::Federation::Receive do end context "with poll" do - let(:poll_entity) { FactoryGirl.build(:poll_entity) } + let(:poll_entity) { Fabricate(:poll_entity) } let(:status_message_entity) { - FactoryGirl.build(:status_message_entity, author: sender.diaspora_handle, poll: poll_entity) + Fabricate(:status_message_entity, author: sender.diaspora_handle, poll: poll_entity) } it "saves the status message" do @@ -616,9 +611,9 @@ describe Diaspora::Federation::Receive do end context "with location" do - let(:location_entity) { FactoryGirl.build(:location_entity) } + let(:location_entity) { Fabricate(:location_entity) } let(:status_message_entity) { - FactoryGirl.build(:status_message_entity, author: sender.diaspora_handle, location: location_entity) + Fabricate(:status_message_entity, author: sender.diaspora_handle, location: location_entity) } it "saves the status message" do @@ -636,15 +631,15 @@ describe Diaspora::Federation::Receive do end context "with photos" do - let(:status_message_guid) { FactoryGirl.generate(:guid) } + let(:status_message_guid) { Fabricate.sequence(:guid) } let(:photo1) { - FactoryGirl.build(:photo_entity, author: sender.diaspora_handle, status_message_guid: status_message_guid) + Fabricate(:photo_entity, author: sender.diaspora_handle, status_message_guid: status_message_guid) } let(:photo2) { - FactoryGirl.build(:photo_entity, author: sender.diaspora_handle, status_message_guid: status_message_guid) + Fabricate(:photo_entity, author: sender.diaspora_handle, status_message_guid: status_message_guid) } let(:status_message_entity) { - FactoryGirl.build( + Fabricate( :status_message_entity, author: sender.diaspora_handle, guid: status_message_guid, diff --git a/spec/models/notifications/private_message_spec.rb b/spec/models/notifications/private_message_spec.rb index 4ab3b0d79..327312ff6 100644 --- a/spec/models/notifications/private_message_spec.rb +++ b/spec/models/notifications/private_message_spec.rb @@ -4,7 +4,7 @@ describe Notifications::PrivateMessage, type: :model do let(:conversation) { - conv_guid = FactoryGirl.generate(:guid) + conv_guid = Fabricate.sequence(:guid) Conversation.create( guid: conv_guid, From 5e3ea249a9615c1ac2b5eafe4343fcf4b66d452f Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Tue, 25 Apr 2017 01:12:57 +0200 Subject: [PATCH 03/24] Replace additional_xml_elements with additional_data for relayables --- lib/diaspora/federation/receive.rb | 2 +- spec/helper_methods.rb | 4 ++-- spec/lib/diaspora/federation/entities_spec.rb | 12 ++++++------ 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/diaspora/federation/receive.rb b/lib/diaspora/federation/receive.rb index a61566402..697ad4536 100644 --- a/lib/diaspora/federation/receive.rb +++ b/lib/diaspora/federation/receive.rb @@ -263,7 +263,7 @@ module Diaspora private_class_method def self.build_signature(klass, entity) klass.reflect_on_association(:signature).klass.new( author_signature: entity.author_signature, - additional_data: entity.additional_xml_elements, + additional_data: entity.additional_data, signature_order: SignatureOrder.find_or_create_by!(order: entity.xml_order.join(" ")) ) end diff --git a/spec/helper_methods.rb b/spec/helper_methods.rb index c23ee2440..3eb9ff3a3 100644 --- a/spec/helper_methods.rb +++ b/spec/helper_methods.rb @@ -51,11 +51,11 @@ module HelperMethods }.join(" ") end - def build_relayable_federation_entity(type, data={}, additional_xml_elements={}) + def build_relayable_federation_entity(type, data={}, additional_data={}) attributes = Fabricate.attributes_for("#{type}_entity".to_sym, data) entity_class = "DiasporaFederation::Entities::#{type.capitalize}".constantize signable_fields = attributes.keys - [:author_signature] - entity_class.new(attributes, [*signable_fields, *additional_xml_elements.keys], additional_xml_elements) + entity_class.new(attributes, [*signable_fields, *additional_data.keys], additional_data) end end diff --git a/spec/lib/diaspora/federation/entities_spec.rb b/spec/lib/diaspora/federation/entities_spec.rb index 671cbd562..bce5d7f21 100644 --- a/spec/lib/diaspora/federation/entities_spec.rb +++ b/spec/lib/diaspora/federation/entities_spec.rb @@ -19,7 +19,7 @@ describe Diaspora::Federation::Entities do expect(federation_entity.text).to eq(diaspora_entity.text) expect(federation_entity.author_signature).to be_nil expect(federation_entity.xml_order).to be_nil - expect(federation_entity.additional_xml_elements).to be_empty + expect(federation_entity.additional_data).to be_empty end it "builds a comment with signature" do @@ -33,7 +33,7 @@ describe Diaspora::Federation::Entities do expect(federation_entity.text).to eq(diaspora_entity.text) expect(federation_entity.author_signature).to eq(diaspora_entity.signature.author_signature) expect(federation_entity.xml_order).to eq(diaspora_entity.signature.signature_order.order.split) - expect(federation_entity.additional_xml_elements).to eq(diaspora_entity.signature.additional_data) + expect(federation_entity.additional_data).to eq(diaspora_entity.signature.additional_data) end it "builds a contact (request)" do @@ -86,7 +86,7 @@ describe Diaspora::Federation::Entities do expect(federation_entity.positive).to eq(diaspora_entity.positive) expect(federation_entity.author_signature).to be_nil expect(federation_entity.xml_order).to be_nil - expect(federation_entity.additional_xml_elements).to be_empty + expect(federation_entity.additional_data).to be_empty end it "builds a like with signature" do @@ -100,7 +100,7 @@ describe Diaspora::Federation::Entities do expect(federation_entity.positive).to eq(diaspora_entity.positive) expect(federation_entity.author_signature).to eq(diaspora_entity.signature.author_signature) expect(federation_entity.xml_order).to eq(diaspora_entity.signature.signature_order.order.split) - expect(federation_entity.additional_xml_elements).to eq(diaspora_entity.signature.additional_data) + expect(federation_entity.additional_data).to eq(diaspora_entity.signature.additional_data) end it "builds a message" do @@ -154,7 +154,7 @@ describe Diaspora::Federation::Entities do expect(federation_entity.poll_answer_guid).to eq(diaspora_entity.poll_answer.guid) expect(federation_entity.author_signature).to be_nil expect(federation_entity.xml_order).to be_nil - expect(federation_entity.additional_xml_elements).to be_empty + expect(federation_entity.additional_data).to be_empty end it "builds a poll participation with signature" do @@ -169,7 +169,7 @@ describe Diaspora::Federation::Entities do expect(federation_entity.poll_answer_guid).to eq(diaspora_entity.poll_answer.guid) expect(federation_entity.author_signature).to eq(signature.author_signature) expect(federation_entity.xml_order).to eq(signature.signature_order.order.split) - expect(federation_entity.additional_xml_elements).to eq(signature.additional_data) + expect(federation_entity.additional_data).to eq(signature.additional_data) end it "builds a profile" do From 49822a9af7765488cc35d1dd97b70e9c97849efb Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Tue, 25 Apr 2017 01:15:21 +0200 Subject: [PATCH 04/24] New namespace for InvalidRootNode --- app/workers/receive_base.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/workers/receive_base.rb b/app/workers/receive_base.rb index aab2082e4..c49dd7c8c 100644 --- a/app/workers/receive_base.rb +++ b/app/workers/receive_base.rb @@ -8,7 +8,7 @@ module Workers def filter_errors_for_retry yield rescue DiasporaFederation::Entity::ValidationError, - DiasporaFederation::Entity::InvalidRootNode, + DiasporaFederation::Parsers::BaseParser::InvalidRootNode, DiasporaFederation::Entity::InvalidEntityName, DiasporaFederation::Entity::UnknownEntity, DiasporaFederation::Entities::Relayable::SignatureVerificationFailed, From 189c22322401ee559091433ab7325767a7400b43 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Tue, 25 Apr 2017 01:32:35 +0200 Subject: [PATCH 05/24] Fix notification factory --- spec/factories.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/spec/factories.rb b/spec/factories.rb index e6ce43525..fadc1d172 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -245,10 +245,9 @@ FactoryGirl.define do association(:post, factory: :status_message) end - factory(:notification) do + factory(:notification, class: Notifications::AlsoCommented) do association :recipient, :factory => :user association :target, :factory => :comment - type 'Notifications::AlsoCommented' after(:build) do |note| note.actors << FactoryGirl.build(:person) From 5e2d063c4906eced7a65757247e6776cb31085db Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Tue, 25 Apr 2017 01:39:52 +0200 Subject: [PATCH 06/24] Hash from federation entity now contains the correct data-type --- lib/diaspora/federated/retraction.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/diaspora/federated/retraction.rb b/lib/diaspora/federated/retraction.rb index f925bbb9a..642c93898 100644 --- a/lib/diaspora/federated/retraction.rb +++ b/lib/diaspora/federated/retraction.rb @@ -41,7 +41,7 @@ class Retraction def public? # TODO: backward compatibility for pre 0.6 pods, they don't relay public retractions - data[:target][:public] == "true" && (!data[:target][:parent] || data[:target][:parent][:local] == "true") + data[:target][:public] == true && (!data[:target][:parent] || data[:target][:parent][:local] == true) end private From 87d0778086c0f262ee31d5e6000345f0582cd19d Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Sun, 30 Apr 2017 04:13:30 +0200 Subject: [PATCH 07/24] Remove guid and public_key from WebFinger Related to diaspora/diaspora_federation#39 --- config/initializers/diaspora_federation.rb | 4 +--- spec/federation_callbacks_spec.rb | 2 -- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/config/initializers/diaspora_federation.rb b/config/initializers/diaspora_federation.rb index aff729ec4..5acd33085 100644 --- a/config/initializers/diaspora_federation.rb +++ b/config/initializers/diaspora_federation.rb @@ -20,9 +20,7 @@ DiasporaFederation.configure do |config| profile_url: person.profile_url, atom_url: person.atom_url, salmon_url: person.receive_url, - subscribe_url: AppConfig.url_to("/people?q={uri}"), - guid: person.guid, - public_key: person.serialized_public_key + subscribe_url: AppConfig.url_to("/people?q={uri}") ) end end diff --git a/spec/federation_callbacks_spec.rb b/spec/federation_callbacks_spec.rb index df72b7b78..33040b30c 100644 --- a/spec/federation_callbacks_spec.rb +++ b/spec/federation_callbacks_spec.rb @@ -13,8 +13,6 @@ describe "diaspora federation callbacks" do expect(wf.atom_url).to eq(person.atom_url) expect(wf.salmon_url).to eq(person.receive_url) expect(wf.subscribe_url).to eq(AppConfig.url_to("/people?q={uri}")) - expect(wf.guid).to eq(person.guid) - expect(wf.public_key).to eq(person.serialized_public_key) end it "returns nil if the person was not found" do From 95def40c5532a130ce30dd00426d55e6028c06a2 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Sun, 30 Apr 2017 04:28:46 +0200 Subject: [PATCH 08/24] Make Message entity non-relayable Related to #36 --- app/controllers/messages_controller.rb | 8 --- app/models/message.rb | 9 +-- ...20170430022507_remove_message_signature.rb | 5 ++ db/schema.rb | 15 ++-- lib/diaspora/federation/entities.rb | 5 +- lib/diaspora/federation/receive.rb | 13 +--- spec/controllers/messages_controller_spec.rb | 13 ---- .../federation/federation_helper.rb | 10 ++- .../receive_federation_messages_spec.rb | 69 ++++++++++++++----- spec/lib/diaspora/federation/entities_spec.rb | 3 +- spec/lib/diaspora/federation/receive_spec.rb | 1 - spec/models/message_spec.rb | 5 -- 12 files changed, 74 insertions(+), 82 deletions(-) create mode 100644 db/migrate/20170430022507_remove_message_signature.rb diff --git a/app/controllers/messages_controller.rb b/app/controllers/messages_controller.rb index c5387799b..a217aab87 100644 --- a/app/controllers/messages_controller.rb +++ b/app/controllers/messages_controller.rb @@ -18,14 +18,6 @@ class MessagesController < ApplicationController logger.info "event=create type=message user=#{current_user.diaspora_handle} status=success " \ "message=#{message.id} chars=#{params[:message][:text].length}" Diaspora::Federation::Dispatcher.defer_dispatch(current_user, message) - - # TODO: can be removed when messages are not relayed anymore - conversation_owner = conversation.author.owner - if conversation_owner && conversation_owner != current_user - remote_subs = conversation.participants.remote.ids - opts = {subscriber_ids: remote_subs} - Diaspora::Federation::Dispatcher.defer_dispatch(conversation_owner, message, opts) unless remote_subs.empty? - end else flash[:error] = I18n.t('conversations.new_conversation.fail') end diff --git a/app/models/message.rb b/app/models/message.rb index 82c551262..2a56ea47a 100644 --- a/app/models/message.rb +++ b/app/models/message.rb @@ -7,9 +7,6 @@ class Message < ActiveRecord::Base delegate :name, to: :author, prefix: true - # TODO: can be removed when messages are not relayed anymore - alias_attribute :parent, :conversation - validates :conversation, presence: true validates :text, presence: true validate :participant_of_parent_conversation @@ -31,11 +28,7 @@ class Message < ActiveRecord::Base # @return [Array] def subscribers - if author.local? - conversation.participants - else # for relaying, TODO: can be removed when messages are not relayed anymore - conversation.participants.remote - end + conversation.participants end private diff --git a/db/migrate/20170430022507_remove_message_signature.rb b/db/migrate/20170430022507_remove_message_signature.rb new file mode 100644 index 000000000..f2519dba5 --- /dev/null +++ b/db/migrate/20170430022507_remove_message_signature.rb @@ -0,0 +1,5 @@ +class RemoveMessageSignature < ActiveRecord::Migration + def change + remove_column :messages, :author_signature, :text + end +end diff --git a/db/schema.rb b/db/schema.rb index 7e0e55ec0..48db06ecd 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20161107100840) do +ActiveRecord::Schema.define(version: 20170430022507) do create_table "account_deletions", force: :cascade do |t| t.string "diaspora_handle", limit: 255 @@ -214,13 +214,12 @@ ActiveRecord::Schema.define(version: 20161107100840) do add_index "mentions", ["person_id"], name: "index_mentions_on_person_id", using: :btree create_table "messages", force: :cascade do |t| - t.integer "conversation_id", limit: 4, null: false - t.integer "author_id", limit: 4, null: false - t.string "guid", limit: 255, null: false - t.text "text", limit: 65535, null: false - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.text "author_signature", limit: 65535 + t.integer "conversation_id", limit: 4, null: false + t.integer "author_id", limit: 4, null: false + t.string "guid", limit: 255, null: false + t.text "text", limit: 65535, null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false end add_index "messages", ["author_id"], name: "index_messages_on_author_id", using: :btree diff --git a/lib/diaspora/federation/entities.rb b/lib/diaspora/federation/entities.rb index f806ec158..da8e0bb33 100644 --- a/lib/diaspora/federation/entities.rb +++ b/lib/diaspora/federation/entities.rb @@ -98,10 +98,7 @@ module Diaspora guid: message.guid, text: message.text, created_at: message.created_at, - parent_guid: message.conversation.guid, - conversation_guid: message.conversation.guid, - author_signature: message.author_signature, - parent: related_entity(message.conversation) + conversation_guid: message.conversation.guid ) end diff --git a/lib/diaspora/federation/receive.rb b/lib/diaspora/federation/receive.rb index 697ad4536..18c642fd7 100644 --- a/lib/diaspora/federation/receive.rb +++ b/lib/diaspora/federation/receive.rb @@ -59,7 +59,9 @@ module Diaspora end def self.message(entity) - save_message(entity).tap {|message| relay_relayable(message) if message } + ignore_existing_guid(Message, entity.guid, author_of(entity)) do + build_message(entity).tap(&:save!) + end end def self.participation(entity) @@ -215,15 +217,6 @@ module Diaspora end end - private_class_method def self.save_message(entity) - ignore_existing_guid(Message, entity.guid, author_of(entity)) do - build_message(entity).tap do |message| - message.author_signature = entity.author_signature if message.conversation.author.local? - message.save! - end - end - end - private_class_method def self.save_photo(entity) Photo.create!( author: author_of(entity), diff --git a/spec/controllers/messages_controller_spec.rb b/spec/controllers/messages_controller_spec.rb index 7e753c4f0..6e7bea228 100644 --- a/spec/controllers/messages_controller_spec.rb +++ b/spec/controllers/messages_controller_spec.rb @@ -102,19 +102,6 @@ describe MessagesController, :type => :controller do expect(Diaspora::Federation::Dispatcher).to receive(:defer_dispatch).with(alice, instance_of(Message)) post :create, @message_params end - - it "dispatches the message twice if the conversation author is local and it has remote users" do - @conversation_params[:participant_ids] = [bob.person.id, alice.person.id, remote_raphael.id] - conversation = Conversation.create!(@conversation_params) - @message_params[:conversation_id] = conversation.id - - expect(Diaspora::Federation::Dispatcher).to receive(:defer_dispatch).with(alice, instance_of(Message)) - expect(Diaspora::Federation::Dispatcher).to receive(:defer_dispatch).with( - bob, instance_of(Message), subscriber_ids: [remote_raphael.id] - ) - - post :create, @message_params - end end context 'on a post from a stranger' do diff --git a/spec/integration/federation/federation_helper.rb b/spec/integration/federation/federation_helper.rb index 56e8a86f9..f16cd604c 100644 --- a/spec/integration/federation/federation_helper.rb +++ b/spec/integration/federation/federation_helper.rb @@ -35,14 +35,12 @@ def create_relayable_entity(entity_name, parent, diaspora_id) :fetch_private_key, alice.diaspora_handle ).at_least(1).times.and_return(nil) if parent == local_parent - parent_guid = parent.guid Fabricate( entity_name, - conversation_guid: parent_guid, - parent_guid: parent_guid, - author: diaspora_id, - poll_answer_guid: parent.respond_to?(:poll_answers) ? parent.poll_answers.first.guid : nil, - parent: Diaspora::Federation::Entities.related_entity(parent) + parent_guid: parent.guid, + author: diaspora_id, + poll_answer_guid: parent.respond_to?(:poll_answers) ? parent.poll_answers.first.guid : nil, + parent: Diaspora::Federation::Entities.related_entity(parent) ) end diff --git a/spec/integration/federation/receive_federation_messages_spec.rb b/spec/integration/federation/receive_federation_messages_spec.rb index d16676468..4a017cedf 100644 --- a/spec/integration/federation/receive_federation_messages_spec.rb +++ b/spec/integration/federation/receive_federation_messages_spec.rb @@ -126,24 +126,59 @@ describe "Receive federation messages feature" do end context "with message" do - let(:local_parent) { - FactoryGirl.build(:conversation, author: alice.person).tap do |target| - target.participants << remote_user_on_pod_b.person - target.participants << remote_user_on_pod_c.person - target.save - end - } - let(:remote_parent) { - FactoryGirl.build(:conversation, author: remote_user_on_pod_b.person).tap do |target| - target.participants << alice.person - target.participants << remote_user_on_pod_c.person - target.save - end - } - let(:entity_name) { :message_entity } - let(:klass) { Message } + context "local" do + let(:parent) { + FactoryGirl.build(:conversation, author: alice.person).tap do |target| + target.participants << remote_user_on_pod_b.person + target.participants << remote_user_on_pod_c.person + target.save + end + } + let(:message) { + Fabricate( + :message_entity, + conversation_guid: parent.guid, + author: sender_id, + parent: Diaspora::Federation::Entities.related_entity(parent) + ) + } - it_behaves_like "it deals correctly with a relayable" + it "receives the message correctly" do + expect(Workers::ReceiveLocal).to receive(:perform_async) + post_message(generate_payload(message, sender, recipient), recipient) + + received_message = Message.find_by(guid: message.guid) + expect(received_message).not_to be_nil + expect(received_message.author.diaspora_handle).to eq(sender_id) + end + end + + context "remote" do + let(:parent) { + FactoryGirl.build(:conversation, author: remote_user_on_pod_b.person).tap do |target| + target.participants << alice.person + target.participants << remote_user_on_pod_c.person + target.save + end + } + let(:message) { + Fabricate( + :message_entity, + conversation_guid: parent.guid, + author: remote_user_on_pod_c.diaspora_handle, + parent: Diaspora::Federation::Entities.related_entity(parent) + ) + } + + it "receives the message correctly" do + expect(Workers::ReceiveLocal).to receive(:perform_async) + post_message(generate_payload(message, remote_user_on_pod_c, recipient), recipient) + + received_message = Message.find_by(guid: message.guid) + expect(received_message).not_to be_nil + expect(received_message.author.diaspora_handle).to eq(remote_user_on_pod_c.diaspora_handle) + end + end end end end diff --git a/spec/lib/diaspora/federation/entities_spec.rb b/spec/lib/diaspora/federation/entities_spec.rb index bce5d7f21..428990bcd 100644 --- a/spec/lib/diaspora/federation/entities_spec.rb +++ b/spec/lib/diaspora/federation/entities_spec.rb @@ -104,7 +104,7 @@ describe Diaspora::Federation::Entities do end it "builds a message" do - diaspora_entity = FactoryGirl.create(:message, author_signature: "abc") + diaspora_entity = FactoryGirl.create(:message) federation_entity = described_class.build(diaspora_entity) expect(federation_entity).to be_instance_of(DiasporaFederation::Entities::Message) @@ -113,7 +113,6 @@ describe Diaspora::Federation::Entities do expect(federation_entity.conversation_guid).to eq(diaspora_entity.conversation.guid) expect(federation_entity.text).to eq(diaspora_entity.text) expect(federation_entity.created_at).to eq(diaspora_entity.created_at) - expect(federation_entity.author_signature).to eq(diaspora_entity.author_signature) end it "builds a participation" do diff --git a/spec/lib/diaspora/federation/receive_spec.rb b/spec/lib/diaspora/federation/receive_spec.rb index 680a34190..f871dad6c 100644 --- a/spec/lib/diaspora/federation/receive_spec.rb +++ b/spec/lib/diaspora/federation/receive_spec.rb @@ -271,7 +271,6 @@ describe Diaspora::Federation::Receive do let(:entity) { message_entity } it_behaves_like "it ignores existing object received twice", Message - it_behaves_like "it relays relayables", Message end describe ".participation" do diff --git a/spec/models/message_spec.rb b/spec/models/message_spec.rb index 051b384ce..e2588a502 100644 --- a/spec/models/message_spec.rb +++ b/spec/models/message_spec.rb @@ -39,11 +39,6 @@ describe Message, type: :model do message = Message.create(author: local_luke.person, text: "yo", conversation: remote_conv) expect(message.subscribers).to match_array([local_luke.person, local_leia.person, remote_raphael]) end - - it "returns only remote participants, if the conversation is local, but the author is remote" do - message = Message.create(author: remote_raphael, text: "yo", conversation: local_conv) - expect(message.subscribers).to match_array([remote_raphael]) - end end describe "#increase_unread" do From f6dc809e29cd50bab115d13fe6d446cd17d1f361 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Sun, 30 Apr 2017 04:43:55 +0200 Subject: [PATCH 09/24] Send new MagicEnvelope and EncryptedMagicEnvelope Related to diaspora/diaspora_federation#30 --- lib/diaspora/federation/dispatcher.rb | 10 ++++++++++ lib/diaspora/federation/dispatcher/private.rb | 15 +++++---------- lib/diaspora/federation/dispatcher/public.rb | 11 +---------- 3 files changed, 16 insertions(+), 20 deletions(-) diff --git a/lib/diaspora/federation/dispatcher.rb b/lib/diaspora/federation/dispatcher.rb index fc79e1092..64b16dae7 100644 --- a/lib/diaspora/federation/dispatcher.rb +++ b/lib/diaspora/federation/dispatcher.rb @@ -31,6 +31,16 @@ module Diaspora attr_reader :sender, :object, :opts + def entity + @entity ||= Entities.build(object) + end + + def magic_envelope + @magic_envelope ||= DiasporaFederation::Salmon::MagicEnvelope.new( + entity, sender.diaspora_handle + ).envelop(sender.encryption_key) + end + def deliver_to_services deliver_to_user_services if opts[:service_types] end diff --git a/lib/diaspora/federation/dispatcher/private.rb b/lib/diaspora/federation/dispatcher/private.rb index ab13756fd..456e1dcec 100644 --- a/lib/diaspora/federation/dispatcher/private.rb +++ b/lib/diaspora/federation/dispatcher/private.rb @@ -7,22 +7,17 @@ module Diaspora def deliver_to_remote(people) return if people.empty? - entity = Entities.build(object) - Workers::SendPrivate.perform_async(sender.id, entity.to_s, targets(people, salmon_slap(entity))) + Workers::SendPrivate.perform_async(sender.id, entity.to_s, targets(people)) end - def targets(people, salmon_slap) + def targets(people) active, inactive = people.partition {|person| person.pod.active? } logger.info "ignoring inactive pods: #{inactive.map(&:diaspora_handle).join(', ')}" if inactive.any? - active.map {|person| [person.receive_url, salmon_slap.generate_xml(person.public_key)] }.to_h + active.map {|person| [person.receive_url, encrypted_magic_envelope(person)] }.to_h end - def salmon_slap(entity) - DiasporaFederation::Salmon::EncryptedSlap.prepare( - sender.diaspora_handle, - sender.encryption_key, - entity - ) + def encrypted_magic_envelope(person) + DiasporaFederation::Salmon::EncryptedMagicEnvelope.encrypt(magic_envelope, person.public_key) end end end diff --git a/lib/diaspora/federation/dispatcher/public.rb b/lib/diaspora/federation/dispatcher/public.rb index c91979c20..b8ad30bff 100644 --- a/lib/diaspora/federation/dispatcher/public.rb +++ b/lib/diaspora/federation/dispatcher/public.rb @@ -14,8 +14,7 @@ module Diaspora return if targets.empty? - entity = Entities.build(object) - Workers::SendPublic.perform_async(sender.id, entity.to_s, targets, salmon_xml(entity)) + Workers::SendPublic.perform_async(sender.id, entity.to_s, targets, magic_envelope.to_xml) end def target_urls(people) @@ -29,14 +28,6 @@ module Diaspora [AppConfig.relay.outbound.url] end - def salmon_xml(entity) - DiasporaFederation::Salmon::Slap.generate_xml( - sender.diaspora_handle, - sender.encryption_key, - entity - ) - end - def deliver_to_hub logger.debug "deliver to pubsubhubbub sender: #{sender.diaspora_handle}" Workers::PublishToHub.perform_async(sender.atom_url) From e907b3eb18ede79988b0741d0b645956f853e01f Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Sun, 30 Apr 2017 14:15:21 +0200 Subject: [PATCH 10/24] Send Contact entity for start/stop sharing Related to diaspora/diaspora_federation#32 --- app/models/user/connecting.rb | 1 + lib/diaspora/federated/retraction.rb | 4 +- lib/diaspora/federation/entities.rb | 48 +++---- lib/diaspora/federation/mappings.rb | 2 +- lib/diaspora/federation/receive.rb | 2 +- .../federation/attack_vectors_spec.rb | 5 +- .../receive_federation_messages_spec.rb | 4 +- .../lib/diaspora/federated/retraction_spec.rb | 2 +- spec/lib/diaspora/federation/entities_spec.rb | 126 +++++++----------- spec/lib/diaspora/federation/receive_spec.rb | 2 +- spec/models/user/connecting_spec.rb | 1 + 11 files changed, 80 insertions(+), 117 deletions(-) diff --git a/app/models/user/connecting.rb b/app/models/user/connecting.rb index 0781745f4..558cb7de4 100644 --- a/app/models/user/connecting.rb +++ b/app/models/user/connecting.rb @@ -34,6 +34,7 @@ class User if contact.person.local? contact.person.owner.disconnected_by(contact.user.person) else + contact.receiving = false Retraction.for(contact).defer_dispatch(self) end diff --git a/lib/diaspora/federated/retraction.rb b/lib/diaspora/federated/retraction.rb index 642c93898..a68fcccfa 100644 --- a/lib/diaspora/federated/retraction.rb +++ b/lib/diaspora/federated/retraction.rb @@ -21,10 +21,10 @@ class Retraction when Post Diaspora::Federation::Entities.signed_retraction(target, sender) else - Diaspora::Federation::Entities.retraction(target) + Diaspora::Federation::Entities.retraction_data_for(target) end - new(federation_retraction.to_h, target.subscribers.select(&:remote?), target) + new(federation_retraction, target.subscribers.select(&:remote?), target) end def defer_dispatch(user, include_target_author=true) diff --git a/lib/diaspora/federation/entities.rb b/lib/diaspora/federation/entities.rb index da8e0bb33..bab9624b7 100644 --- a/lib/diaspora/federation/entities.rb +++ b/lib/diaspora/federation/entities.rb @@ -5,17 +5,6 @@ module Diaspora public_send(Mappings.builder_for(entity.class), entity) end - def self.build_retraction(retraction) - case retraction.data[:target_type] - when "Comment", "Like", "PollParticipation" - DiasporaFederation::Entities::RelayableRetraction.new(retraction.data) - when "Post" - DiasporaFederation::Entities::SignedRetraction.new(retraction.data) - else - DiasporaFederation::Entities::Retraction.new(retraction.data) - end - end - def self.post(post) case post when StatusMessage @@ -50,10 +39,11 @@ module Diaspora end def self.contact(contact) - # TODO: use DiasporaFederation::Entities::Contact - DiasporaFederation::Entities::Request.new( + DiasporaFederation::Entities::Contact.new( author: contact.user.diaspora_handle, - recipient: contact.person.diaspora_handle + recipient: contact.person.diaspora_handle, + sharing: contact.receiving, + following: contact.receiving ) end @@ -182,7 +172,7 @@ module Diaspora target_type: Mappings.entity_name_for(target), target: related_entity(target), author: sender.diaspora_handle - ) + ).to_h end def self.reshare(reshare) @@ -197,24 +187,30 @@ module Diaspora ) end - def self.retraction(target) + def self.retraction(retraction) + case retraction.data[:target_type] + when "Comment", "Like", "PollParticipation" + DiasporaFederation::Entities::RelayableRetraction.new(retraction.data) + when "Post" + DiasporaFederation::Entities::SignedRetraction.new(retraction.data) + when "Contact" + DiasporaFederation::Entities::Contact.new(retraction.data) + else + DiasporaFederation::Entities::Retraction.new(retraction.data) + end + end + + def self.retraction_data_for(target) case target when Contact - # TODO: deprecated - author = target.user.diaspora_handle - DiasporaFederation::Entities::Retraction.new( - target_guid: target.user.guid, - target_type: "Person", - target: DiasporaFederation::Entities::RelatedEntity.new(author: author, local: true), - author: author - ) + contact(target).to_h.tap {|data| data[:target_type] = "Contact" } else DiasporaFederation::Entities::Retraction.new( target_guid: target.guid, target_type: Mappings.entity_name_for(target), target: related_entity(target), author: target.diaspora_handle - ) + ).to_h end end @@ -225,7 +221,7 @@ module Diaspora target_type: Mappings.entity_name_for(target), target: related_entity(target), author: sender.diaspora_handle - ) + ).to_h end def self.status_message(status_message) diff --git a/lib/diaspora/federation/mappings.rb b/lib/diaspora/federation/mappings.rb index 2b761b697..46e0ff969 100644 --- a/lib/diaspora/federation/mappings.rb +++ b/lib/diaspora/federation/mappings.rb @@ -49,7 +49,7 @@ module Diaspora PollParticipation => :poll_participation, Profile => :profile, Reshare => :reshare, - Retraction => :build_retraction, + Retraction => :retraction, StatusMessage => :status_message }.freeze diff --git a/lib/diaspora/federation/receive.rb b/lib/diaspora/federation/receive.rb index 18c642fd7..15c3d2caf 100644 --- a/lib/diaspora/federation/receive.rb +++ b/lib/diaspora/federation/receive.rb @@ -25,7 +25,7 @@ module Diaspora def self.contact(entity) recipient = Person.find_by(diaspora_handle: entity.recipient).owner - if entity.sharing.to_s == "true" + if entity.sharing Contact.create_or_update_sharing_contact(recipient, author_of(entity)) else recipient.disconnected_by(author_of(entity)) diff --git a/spec/integration/federation/attack_vectors_spec.rb b/spec/integration/federation/attack_vectors_spec.rb index e345420ef..c5a802647 100644 --- a/spec/integration/federation/attack_vectors_spec.rb +++ b/spec/integration/federation/attack_vectors_spec.rb @@ -49,9 +49,10 @@ describe "attack vectors", type: :request do it "should not receive retractions where the retractor and the salmon author do not match" do original_message = eve.post(:status_message, text: "store this!", to: eves_aspect.id) + retraction = Retraction.for(original_message) expect { - post_message(generate_xml(Diaspora::Federation::Entities.retraction(original_message), alice, bob), bob) + post_message(generate_xml(Diaspora::Federation::Entities.retraction(retraction), alice, bob), bob) }.to_not change { bob.visible_shareables(Post).count(:all) } end @@ -61,7 +62,7 @@ describe "attack vectors", type: :request do contact = bob.contacts(true).find_by(person_id: eve.person.id) expect(contact).to be_sharing - post_message(generate_xml(Diaspora::Federation::Entities.retraction(contact), alice, bob), bob) + post_message(generate_xml(Diaspora::Federation::Entities.retraction(Retraction.for(contact)), alice, bob), bob) expect(bob.contacts(true).find_by(person_id: eve.person.id)).to be_sharing end diff --git a/spec/integration/federation/receive_federation_messages_spec.rb b/spec/integration/federation/receive_federation_messages_spec.rb index 4a017cedf..d532b3c71 100644 --- a/spec/integration/federation/receive_federation_messages_spec.rb +++ b/spec/integration/federation/receive_federation_messages_spec.rb @@ -73,8 +73,8 @@ describe "Receive federation messages feature" do context "with private receive" do let(:recipient) { alice } - it "treats sharing request recive correctly" do - entity = Fabricate(:request_entity, author: sender_id, recipient: alice.diaspora_handle) + it "treats sharing request receive correctly" do + entity = Fabricate(:contact_entity, author: sender_id, recipient: alice.diaspora_handle) expect(Workers::ReceiveLocal).to receive(:perform_async).and_call_original diff --git a/spec/lib/diaspora/federated/retraction_spec.rb b/spec/lib/diaspora/federated/retraction_spec.rb index 3f7b9f39b..ee5e0f485 100644 --- a/spec/lib/diaspora/federated/retraction_spec.rb +++ b/spec/lib/diaspora/federated/retraction_spec.rb @@ -42,7 +42,7 @@ describe Retraction do it "creates a retraction for a contact" do contact = FactoryGirl.create(:contact) - expect(Diaspora::Federation::Entities).to receive(:retraction).with(contact) + expect(Diaspora::Federation::Entities).to receive(:retraction_data_for).with(contact) Retraction.for(contact, contact.user) end diff --git a/spec/lib/diaspora/federation/entities_spec.rb b/spec/lib/diaspora/federation/entities_spec.rb index 428990bcd..5263ff599 100644 --- a/spec/lib/diaspora/federation/entities_spec.rb +++ b/spec/lib/diaspora/federation/entities_spec.rb @@ -36,13 +36,15 @@ describe Diaspora::Federation::Entities do expect(federation_entity.additional_data).to eq(diaspora_entity.signature.additional_data) end - it "builds a contact (request)" do - diaspora_entity = FactoryGirl.build(:contact) + it "builds a contact" do + diaspora_entity = FactoryGirl.build(:contact, receiving: true) federation_entity = described_class.build(diaspora_entity) - expect(federation_entity).to be_instance_of(DiasporaFederation::Entities::Request) + expect(federation_entity).to be_instance_of(DiasporaFederation::Entities::Contact) expect(federation_entity.author).to eq(diaspora_entity.user.diaspora_handle) expect(federation_entity.recipient).to eq(diaspora_entity.person.diaspora_handle) + expect(federation_entity.sharing).to be_truthy + expect(federation_entity.following).to be_truthy end context "Conversation" do @@ -205,6 +207,31 @@ describe Diaspora::Federation::Entities do expect(federation_entity.provider_display_name).to eq(diaspora_entity.provider_display_name) end + context "Retraction" do + it "builds a Retraction entity for a Photo retraction" do + target = FactoryGirl.create(:photo, author: alice.person) + retraction = Retraction.for(target) + federation_entity = described_class.build(retraction) + + expect(federation_entity).to be_instance_of(DiasporaFederation::Entities::Retraction) + expect(federation_entity.author).to eq(target.author.diaspora_handle) + expect(federation_entity.target_guid).to eq(target.guid) + expect(federation_entity.target_type).to eq("Photo") + end + + it "builds a Contact for a Contact retraction" do + target = FactoryGirl.create(:contact, receiving: false) + retraction = Retraction.for(target) + federation_entity = described_class.build(retraction) + + expect(federation_entity).to be_instance_of(DiasporaFederation::Entities::Contact) + expect(federation_entity.author).to eq(target.user.diaspora_handle) + expect(federation_entity.recipient).to eq(target.person.diaspora_handle) + expect(federation_entity.sharing).to be_falsey + expect(federation_entity.following).to be_falsey + end + end + context "StatusMessage" do it "builds a status message" do diaspora_entity = FactoryGirl.create(:status_message) @@ -274,88 +301,25 @@ describe Diaspora::Federation::Entities do end end - describe ".build_retraction" do - context "Retraction" do - it "builds a Retraction for a Photo" do - target = FactoryGirl.create(:photo, author: alice.person) - retraction = Retraction.for(target, alice) - federation_retraction = described_class.build_retraction(retraction) + describe ".retraction_data_for" do + it "returns the data for a Photo retraction" do + target = FactoryGirl.create(:photo, author: alice.person) + retraction_data = described_class.retraction_data_for(target) - expect(federation_retraction).to be_instance_of(DiasporaFederation::Entities::Retraction) - expect(federation_retraction.author).to eq(target.author.diaspora_handle) - expect(federation_retraction.target_guid).to eq(target.guid) - expect(federation_retraction.target_type).to eq("Photo") - end - - it "builds a Retraction for a Contact" do - target = FactoryGirl.create(:contact) - retraction = Retraction.for(target, target.user) - federation_retraction = described_class.build_retraction(retraction) - - expect(federation_retraction).to be_instance_of(DiasporaFederation::Entities::Retraction) - expect(federation_retraction.author).to eq(target.user.diaspora_handle) - expect(federation_retraction.target_guid).to eq(target.user.guid) - expect(federation_retraction.target_type).to eq("Person") - end + expect(retraction_data[:author]).to eq(target.author.diaspora_handle) + expect(retraction_data[:target_guid]).to eq(target.guid) + expect(retraction_data[:target_type]).to eq("Photo") end - context "SignedRetraction" do - it "builds a SignedRetraction for a StatusMessage" do - target = FactoryGirl.create(:status_message, author: alice.person) - retraction = Retraction.for(target, alice) - federation_retraction = described_class.build_retraction(retraction) + it "returns the data for a Contact entity" do + target = FactoryGirl.create(:contact, receiving: false) + retraction_data = described_class.retraction_data_for(target) - expect(federation_retraction).to be_instance_of(DiasporaFederation::Entities::SignedRetraction) - expect(federation_retraction.author).to eq(target.author.diaspora_handle) - expect(federation_retraction.target_guid).to eq(target.guid) - expect(federation_retraction.target_type).to eq("Post") - end - - it "builds a SignedRetraction for a Reshare" do - target = FactoryGirl.create(:reshare, author: alice.person) - retraction = Retraction.for(target, alice) - federation_retraction = described_class.build_retraction(retraction) - - expect(federation_retraction).to be_instance_of(DiasporaFederation::Entities::SignedRetraction) - expect(federation_retraction.author).to eq(target.author.diaspora_handle) - expect(federation_retraction.target_guid).to eq(target.guid) - expect(federation_retraction.target_type).to eq("Post") - end - end - - context "RelayableRetraction" do - it "builds a RelayableRetraction for a Comment" do - target = FactoryGirl.create(:comment, author: alice.person) - retraction = Retraction.for(target, alice) - federation_retraction = described_class.build_retraction(retraction) - - expect(federation_retraction).to be_instance_of(DiasporaFederation::Entities::RelayableRetraction) - expect(federation_retraction.author).to eq(alice.diaspora_handle) - expect(federation_retraction.target_guid).to eq(target.guid) - expect(federation_retraction.target_type).to eq("Comment") - end - - it "builds a RelayableRetraction for a Like" do - target = FactoryGirl.create(:like, author: alice.person) - retraction = Retraction.for(target, alice) - federation_retraction = described_class.build_retraction(retraction) - - expect(federation_retraction).to be_instance_of(DiasporaFederation::Entities::RelayableRetraction) - expect(federation_retraction.author).to eq(alice.diaspora_handle) - expect(federation_retraction.target_guid).to eq(target.guid) - expect(federation_retraction.target_type).to eq("Like") - end - - it "builds a RelayableRetraction for a PollParticipation" do - target = FactoryGirl.create(:poll_participation, author: alice.person) - retraction = Retraction.for(target, alice) - federation_retraction = described_class.build_retraction(retraction) - - expect(federation_retraction).to be_instance_of(DiasporaFederation::Entities::RelayableRetraction) - expect(federation_retraction.author).to eq(alice.diaspora_handle) - expect(federation_retraction.target_guid).to eq(target.guid) - expect(federation_retraction.target_type).to eq("PollParticipation") - end + expect(retraction_data[:author]).to eq(target.user.diaspora_handle) + expect(retraction_data[:recipient]).to eq(target.person.diaspora_handle) + expect(retraction_data[:sharing]).to be_falsey + expect(retraction_data[:following]).to be_falsey + expect(retraction_data[:target_type]).to eq("Contact") end end end diff --git a/spec/lib/diaspora/federation/receive_spec.rb b/spec/lib/diaspora/federation/receive_spec.rb index f871dad6c..3da798df1 100644 --- a/spec/lib/diaspora/federation/receive_spec.rb +++ b/spec/lib/diaspora/federation/receive_spec.rb @@ -103,7 +103,7 @@ describe Diaspora::Federation::Receive do :contact_entity, author: sender.diaspora_handle, recipient: alice.diaspora_handle, - sharing: "false" + sharing: false ) } diff --git a/spec/models/user/connecting_spec.rb b/spec/models/user/connecting_spec.rb index 455ce2143..40ef55ce6 100644 --- a/spec/models/user/connecting_spec.rb +++ b/spec/models/user/connecting_spec.rb @@ -87,6 +87,7 @@ describe User::Connecting, type: :model do contact = local_leia.contact_for(remote_raphael) retraction = double + expect(contact).to receive(:receiving=).with(false) expect(Retraction).to receive(:for).with(contact).and_return(retraction) expect(retraction).to receive(:defer_dispatch).with(local_leia) From b6b0aac969c10efb9de09e6a307957d0285cb2e0 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Sun, 30 Apr 2017 04:59:42 +0200 Subject: [PATCH 11/24] Send only Retractions Related to diaspora/diaspora_federation#27 --- app/models/user.rb | 2 +- lib/diaspora/federated/retraction.rb | 14 +---- lib/diaspora/federation/entities.rb | 24 ------- lib/diaspora/federation/receive.rb | 4 +- .../lib/diaspora/federated/retraction_spec.rb | 62 ++++++++++--------- spec/lib/diaspora/federation/receive_spec.rb | 4 +- spec/models/user_spec.rb | 2 +- spec/shared_behaviors/dispatcher.rb | 4 +- 8 files changed, 45 insertions(+), 71 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 556a14641..2332f08d2 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -347,7 +347,7 @@ class User < ActiveRecord::Base ######### Posts and Such ############### def retract(target) - retraction = Retraction.for(target, self) + retraction = Retraction.for(target) retraction.defer_dispatch(self) retraction.perform end diff --git a/lib/diaspora/federated/retraction.rb b/lib/diaspora/federated/retraction.rb index a68fcccfa..caa494964 100644 --- a/lib/diaspora/federated/retraction.rb +++ b/lib/diaspora/federated/retraction.rb @@ -14,17 +14,9 @@ class Retraction @target = target end - def self.for(target, sender=nil) - federation_retraction = case target - when Diaspora::Relayable - Diaspora::Federation::Entities.relayable_retraction(target, sender) - when Post - Diaspora::Federation::Entities.signed_retraction(target, sender) - else - Diaspora::Federation::Entities.retraction_data_for(target) - end - - new(federation_retraction, target.subscribers.select(&:remote?), target) + def self.for(target) + federation_retraction_data = Diaspora::Federation::Entities.retraction_data_for(target) + new(federation_retraction_data, target.subscribers.select(&:remote?), target) end def defer_dispatch(user, include_target_author=true) diff --git a/lib/diaspora/federation/entities.rb b/lib/diaspora/federation/entities.rb index bab9624b7..8286fef62 100644 --- a/lib/diaspora/federation/entities.rb +++ b/lib/diaspora/federation/entities.rb @@ -165,16 +165,6 @@ module Diaspora ) end - # @deprecated - def self.relayable_retraction(target, sender) - DiasporaFederation::Entities::RelayableRetraction.new( - target_guid: target.guid, - target_type: Mappings.entity_name_for(target), - target: related_entity(target), - author: sender.diaspora_handle - ).to_h - end - def self.reshare(reshare) DiasporaFederation::Entities::Reshare.new( root_author: reshare.root_diaspora_id, @@ -189,10 +179,6 @@ module Diaspora def self.retraction(retraction) case retraction.data[:target_type] - when "Comment", "Like", "PollParticipation" - DiasporaFederation::Entities::RelayableRetraction.new(retraction.data) - when "Post" - DiasporaFederation::Entities::SignedRetraction.new(retraction.data) when "Contact" DiasporaFederation::Entities::Contact.new(retraction.data) else @@ -214,16 +200,6 @@ module Diaspora end end - # @deprecated - def self.signed_retraction(target, sender) - DiasporaFederation::Entities::SignedRetraction.new( - target_guid: target.guid, - target_type: Mappings.entity_name_for(target), - target: related_entity(target), - author: sender.diaspora_handle - ).to_h - end - def self.status_message(status_message) DiasporaFederation::Entities::StatusMessage.new( author: status_message.diaspora_handle, diff --git a/lib/diaspora/federation/receive.rb b/lib/diaspora/federation/receive.rb index 15c3d2caf..6fd7f34bc 100644 --- a/lib/diaspora/federation/receive.rb +++ b/lib/diaspora/federation/receive.rb @@ -151,7 +151,7 @@ module Diaspora when Diaspora::Relayable if object.parent.author.local? parent_author = object.parent.author.owner - retraction = Retraction.for(object, parent_author) + retraction = Retraction.for(object) retraction.defer_dispatch(parent_author, false) retraction.perform else @@ -265,7 +265,7 @@ module Diaspora parent_author = relayable.parent.author.owner return unless parent_author && parent_author.ignored_people.include?(relayable.author) - retraction = Retraction.for(relayable, parent_author) + retraction = Retraction.for(relayable) Diaspora::Federation::Dispatcher.build(parent_author, retraction, subscribers: [relayable.author]).dispatch raise Diaspora::Federation::AuthorIgnored diff --git a/spec/lib/diaspora/federated/retraction_spec.rb b/spec/lib/diaspora/federated/retraction_spec.rb index ee5e0f485..ff5d11ab8 100644 --- a/spec/lib/diaspora/federated/retraction_spec.rb +++ b/spec/lib/diaspora/federated/retraction_spec.rb @@ -4,13 +4,13 @@ describe Retraction do let(:post) { alice.post(:status_message, text: "destroy!", public: true) } - let(:retraction) { Retraction.for(post, alice) } + let(:retraction) { Retraction.for(post) } describe "#subscribers" do it "contains all remote-subscribers of target object" do post = local_luke.post(:status_message, text: "destroy!", public: true) - retraction = Retraction.for(post, local_luke) + retraction = Retraction.for(post) expect(retraction.subscribers).to eq([remote_raphael]) end @@ -18,25 +18,25 @@ describe Retraction do describe "#data" do it "contains the hash with all data from the federation-retraction" do - federation_retraction = Diaspora::Federation::Entities.signed_retraction(post, alice) + federation_retraction_data = Diaspora::Federation::Entities.retraction_data_for(post) - expect(retraction.data).to eq(federation_retraction.to_h) + expect(retraction.data).to eq(federation_retraction_data) end end describe ".for" do it "creates a retraction for a post" do - expect(Diaspora::Federation::Entities).to receive(:signed_retraction).with(post, alice) + expect(Diaspora::Federation::Entities).to receive(:retraction_data_for).with(post) - Retraction.for(post, alice) + Retraction.for(post) end it "creates a retraction for a relayable" do comment = FactoryGirl.create(:comment, author: alice.person, post: post) - expect(Diaspora::Federation::Entities).to receive(:relayable_retraction).with(comment, alice) + expect(Diaspora::Federation::Entities).to receive(:retraction_data_for).with(comment) - Retraction.for(comment, alice) + Retraction.for(comment) end it "creates a retraction for a contact" do @@ -44,20 +44,21 @@ describe Retraction do expect(Diaspora::Federation::Entities).to receive(:retraction_data_for).with(contact) - Retraction.for(contact, contact.user) + Retraction.for(contact) end end describe ".defer_dispatch" do it "queues a job to send the retraction later" do post = local_luke.post(:status_message, text: "destroy!", public: true) - federation_retraction = Diaspora::Federation::Entities.signed_retraction(post, local_luke) + retraction = Retraction.for(post) + federation_retraction = Diaspora::Federation::Entities.retraction(retraction) expect(Workers::DeferredRetraction).to receive(:perform_async).with( local_luke.id, federation_retraction.to_h, [remote_raphael.id], service_types: [] ) - Retraction.for(post, local_luke).defer_dispatch(local_luke) + retraction.defer_dispatch(local_luke) end it "adds service metadata to queued job for deletion" do @@ -66,47 +67,50 @@ describe Retraction do facebook = Services::Facebook.new(access_token: "facebook") alice.services << twitter << facebook - federation_retraction = Diaspora::Federation::Entities.signed_retraction(post, alice) + retraction = Retraction.for(post) + federation_retraction = Diaspora::Federation::Entities.retraction(retraction) expect(Workers::DeferredRetraction).to receive(:perform_async).with( alice.id, federation_retraction.to_h, [], service_types: ["Services::Twitter"], tweet_id: "123" ) - Retraction.for(post, alice).defer_dispatch(alice) + retraction.defer_dispatch(alice) end it "queues also a job if subscribers is empty" do - federation_retraction = Diaspora::Federation::Entities.signed_retraction(post, alice) + retraction = Retraction.for(post) + federation_retraction = Diaspora::Federation::Entities.retraction(retraction) expect(Workers::DeferredRetraction).to receive(:perform_async).with( alice.id, federation_retraction.to_h, [], service_types: [] ) - Retraction.for(post, alice).defer_dispatch(alice) + retraction.defer_dispatch(alice) end it "queues a job with empty opts for non-StatusMessage" do post = local_luke.post(:status_message, text: "hello", public: true) comment = local_luke.comment!(post, "destroy!") - federation_retraction = Diaspora::Federation::Entities.relayable_retraction(comment, local_luke) + retraction = Retraction.for(comment) + federation_retraction = Diaspora::Federation::Entities.retraction(retraction) expect(Workers::DeferredRetraction).to receive(:perform_async).with( local_luke.id, federation_retraction.to_h, [remote_raphael.id], {} ) - Retraction.for(comment, local_luke).defer_dispatch(local_luke) + retraction.defer_dispatch(local_luke) end it "uses the author of the target parent as sender for a comment-retraction if the parent is local" do post = local_luke.post(:status_message, text: "hello", public: true) comment = local_leia.comment!(post, "destroy!") - federation_retraction = Diaspora::Federation::Entities.relayable_retraction(comment, local_leia) + federation_retraction = Diaspora::Federation::Entities.retraction(comment) expect(Workers::DeferredRetraction).to receive(:perform_async).with( local_luke.id, federation_retraction.to_h, [remote_raphael.id], {} ) - Retraction.for(comment, local_leia).defer_dispatch(local_leia) + Retraction.for(comment).defer_dispatch(local_leia) end context "relayable" do @@ -114,23 +118,25 @@ describe Retraction do let(:comment) { FactoryGirl.create(:comment, post: post, author: remote_raphael) } it "sends retraction to target author if deleted by parent author" do - federation_retraction = Diaspora::Federation::Entities.relayable_retraction(comment, local_luke) + retraction = Retraction.for(comment) + federation_retraction = Diaspora::Federation::Entities.retraction(retraction) expect(Workers::DeferredRetraction).to receive(:perform_async).with( local_luke.id, federation_retraction.to_h, [remote_raphael.id], {} ) - Retraction.for(comment, local_luke).defer_dispatch(local_luke) + retraction.defer_dispatch(local_luke) end it "don't sends retraction back to target author if relayed by parent author" do - federation_retraction = Diaspora::Federation::Entities.relayable_retraction(comment, local_luke) + retraction = Retraction.for(comment) + federation_retraction = Diaspora::Federation::Entities.retraction(retraction) expect(Workers::DeferredRetraction).to receive(:perform_async).with( local_luke.id, federation_retraction.to_h, [], {} ) - Retraction.for(comment, local_luke).defer_dispatch(local_luke, false) + retraction.defer_dispatch(local_luke, false) end end end @@ -138,29 +144,29 @@ describe Retraction do describe "#perform" do it "destroys the target object" do expect(post).to receive(:destroy!) - Retraction.for(post, alice).perform + Retraction.for(post).perform end end describe "#public?" do it "returns true for a public post" do - expect(Retraction.for(post, alice).public?).to be_truthy + expect(Retraction.for(post).public?).to be_truthy end it "returns true for a public comment if parent post is local" do comment = bob.comment!(post, "destroy!") - expect(Retraction.for(comment, bob).public?).to be_truthy + expect(Retraction.for(comment).public?).to be_truthy end it "returns false for a public comment if parent post is not local" do remote_post = FactoryGirl.create(:status_message, author: remote_raphael) comment = alice.comment!(remote_post, "destroy!") - expect(Retraction.for(comment, alice).public?).to be_falsey + expect(Retraction.for(comment).public?).to be_falsey end it "returns false for a private target" do private_post = alice.post(:status_message, text: "destroy!", to: alice.aspects.first.id) - expect(Retraction.for(private_post, alice).public?).to be_falsey + expect(Retraction.for(private_post).public?).to be_falsey end end end diff --git a/spec/lib/diaspora/federation/receive_spec.rb b/spec/lib/diaspora/federation/receive_spec.rb index 3da798df1..1bff8ddad 100644 --- a/spec/lib/diaspora/federation/receive_spec.rb +++ b/spec/lib/diaspora/federation/receive_spec.rb @@ -507,9 +507,9 @@ describe Diaspora::Federation::Receive do target_type: "Comment" ) - comment_retraction = Retraction.for(remote_comment, alice) + comment_retraction = Retraction.for(remote_comment) - expect(Retraction).to receive(:for).with(instance_of(Comment), alice).and_return(comment_retraction) + expect(Retraction).to receive(:for).with(instance_of(Comment)).and_return(comment_retraction) expect(comment_retraction).to receive(:defer_dispatch).with(alice, false) expect(comment_retraction).to receive(:perform).and_call_original expect_any_instance_of(Comment).to receive(:destroy!).and_call_original diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index f12f0390c..719129c78 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -778,7 +778,7 @@ describe User, :type => :model do context "posts" do it "sends a retraction" do - expect(Retraction).to receive(:for).with(post, bob).and_return(retraction) + expect(Retraction).to receive(:for).with(post).and_return(retraction) expect(retraction).to receive(:defer_dispatch).with(bob) expect(retraction).to receive(:perform) diff --git a/spec/shared_behaviors/dispatcher.rb b/spec/shared_behaviors/dispatcher.rb index 1d87d8c24..b168465d8 100644 --- a/spec/shared_behaviors/dispatcher.rb +++ b/spec/shared_behaviors/dispatcher.rb @@ -17,7 +17,7 @@ shared_examples "a dispatcher" do opts = {service_types: "Services::Twitter", tweet_id: "123"} expect(Workers::DeletePostFromService).to receive(:perform_async).with(twitter.id, opts) - retraction = Retraction.for(post, alice) + retraction = Retraction.for(post) Diaspora::Federation::Dispatcher.build(alice, retraction, opts).dispatch end @@ -35,7 +35,7 @@ shared_examples "a dispatcher" do it "does not deliver a Retraction of a Comment to services" do expect(Workers::DeletePostFromService).not_to receive(:perform_async) - retraction = Retraction.for(comment, alice) + retraction = Retraction.for(comment) Diaspora::Federation::Dispatcher.build(alice, retraction).dispatch end end From 381c03cfd7d9356170a5a45a144dd0b68af613fc Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Sun, 30 Apr 2017 05:05:32 +0200 Subject: [PATCH 12/24] Remove backward-compatibility for Retractions --- lib/diaspora/federated/retraction.rb | 11 ++--------- spec/lib/diaspora/federated/retraction_spec.rb | 18 +++--------------- 2 files changed, 5 insertions(+), 24 deletions(-) diff --git a/lib/diaspora/federated/retraction.rb b/lib/diaspora/federated/retraction.rb index caa494964..b9c73dd5e 100644 --- a/lib/diaspora/federated/retraction.rb +++ b/lib/diaspora/federated/retraction.rb @@ -21,8 +21,7 @@ class Retraction def defer_dispatch(user, include_target_author=true) subscribers = dispatch_subscribers(include_target_author) - sender = dispatch_sender(user) - Workers::DeferredRetraction.perform_async(sender.id, data, subscribers.map(&:id), service_opts(user)) + Workers::DeferredRetraction.perform_async(user.id, data, subscribers.map(&:id), service_opts(user)) end def perform @@ -32,8 +31,7 @@ class Retraction end def public? - # TODO: backward compatibility for pre 0.6 pods, they don't relay public retractions - data[:target][:public] == true && (!data[:target][:parent] || data[:target][:parent][:local] == true) + data[:target][:public] end private @@ -45,11 +43,6 @@ class Retraction subscribers end - # @deprecated This is only needed for pre 0.6 pods - def dispatch_sender(user) - target.try(:sender_for_dispatch) || user - end - def service_opts(user) return {} unless target.is_a?(StatusMessage) diff --git a/spec/lib/diaspora/federated/retraction_spec.rb b/spec/lib/diaspora/federated/retraction_spec.rb index ff5d11ab8..3dad77b2f 100644 --- a/spec/lib/diaspora/federated/retraction_spec.rb +++ b/spec/lib/diaspora/federated/retraction_spec.rb @@ -101,18 +101,6 @@ describe Retraction do retraction.defer_dispatch(local_luke) end - it "uses the author of the target parent as sender for a comment-retraction if the parent is local" do - post = local_luke.post(:status_message, text: "hello", public: true) - comment = local_leia.comment!(post, "destroy!") - federation_retraction = Diaspora::Federation::Entities.retraction(comment) - - expect(Workers::DeferredRetraction).to receive(:perform_async).with( - local_luke.id, federation_retraction.to_h, [remote_raphael.id], {} - ) - - Retraction.for(comment).defer_dispatch(local_leia) - end - context "relayable" do let(:post) { local_luke.post(:status_message, text: "hello", public: true) } let(:comment) { FactoryGirl.create(:comment, post: post, author: remote_raphael) } @@ -158,10 +146,10 @@ describe Retraction do expect(Retraction.for(comment).public?).to be_truthy end - it "returns false for a public comment if parent post is not local" do - remote_post = FactoryGirl.create(:status_message, author: remote_raphael) + it "returns true for a public comment if parent post is not local" do + remote_post = FactoryGirl.create(:status_message, author: remote_raphael, public: true) comment = alice.comment!(remote_post, "destroy!") - expect(Retraction.for(comment).public?).to be_falsey + expect(Retraction.for(comment).public?).to be_truthy end it "returns false for a private target" do From a6d7dbf1ddc2d569fe9240e8dfed523d493c5206 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Sun, 30 Apr 2017 16:41:01 +0200 Subject: [PATCH 13/24] Send MagicEnvelope as body with correct Content-Type in specs Related to diaspora/diaspora_federation#30 --- .../federation/attack_vectors_spec.rb | 20 ++-- .../federation/federation_helper.rb | 27 +++--- .../receive_federation_messages_spec.rb | 14 +-- .../federation/shared_receive_relayable.rb | 10 +- .../federation/shared_receive_retraction.rb | 24 ++--- .../federation/shared_receive_stream_items.rb | 96 ++++++++----------- .../federation/dispatcher/private_spec.rb | 51 +++++++--- .../federation/dispatcher/public_spec.rb | 29 ++++-- 8 files changed, 149 insertions(+), 122 deletions(-) diff --git a/spec/integration/federation/attack_vectors_spec.rb b/spec/integration/federation/attack_vectors_spec.rb index c5a802647..25fdaecb1 100644 --- a/spec/integration/federation/attack_vectors_spec.rb +++ b/spec/integration/federation/attack_vectors_spec.rb @@ -23,7 +23,7 @@ describe "attack vectors", type: :request do alice.share_with(eve.person, alices_aspect) - post_message(generate_xml(Diaspora::Federation::Entities.post(original_message), bob, alice), alice) + post_message(generate_payload(Diaspora::Federation::Entities.post(original_message), bob, alice), alice) # alice still should not see eves original post, even though bob sent it to her expect(alice.reload.visible_shareables(Post).where(guid: original_message.guid)).to be_blank @@ -34,7 +34,7 @@ describe "attack vectors", type: :request do profile = eve.profile.clone profile.first_name = "Not BOB" - post_message(generate_xml(Diaspora::Federation::Entities.profile(profile), alice, bob), bob) + post_message(generate_payload(Diaspora::Federation::Entities.profile(profile), alice, bob), bob) expect(eve.profile(true).first_name).not_to eq("Not BOB") end @@ -42,7 +42,7 @@ describe "attack vectors", type: :request do it "public post should not be spoofed from another author" do post = FactoryGirl.build(:status_message, public: true, author: eve.person) - post_message(generate_xml(Diaspora::Federation::Entities.post(post), alice)) + post_message(generate_payload(Diaspora::Federation::Entities.post(post), alice)) expect(StatusMessage.exists?(guid: post.guid)).to be_falsey end @@ -52,7 +52,7 @@ describe "attack vectors", type: :request do retraction = Retraction.for(original_message) expect { - post_message(generate_xml(Diaspora::Federation::Entities.retraction(retraction), alice, bob), bob) + post_message(generate_payload(Diaspora::Federation::Entities.retraction(retraction), alice, bob), bob) }.to_not change { bob.visible_shareables(Post).count(:all) } end @@ -62,7 +62,9 @@ describe "attack vectors", type: :request do contact = bob.contacts(true).find_by(person_id: eve.person.id) expect(contact).to be_sharing - post_message(generate_xml(Diaspora::Federation::Entities.retraction(Retraction.for(contact)), alice, bob), bob) + post_message( + generate_payload(Diaspora::Federation::Entities.retraction(Retraction.for(contact)), alice, bob), bob + ) expect(bob.contacts(true).find_by(person_id: eve.person.id)).to be_sharing end @@ -80,7 +82,7 @@ describe "attack vectors", type: :request do author: alice.person ) - post_message(generate_xml(Diaspora::Federation::Entities.post(malicious_message), alice, bob), bob) + post_message(generate_payload(Diaspora::Federation::Entities.post(malicious_message), alice, bob), bob) expect(original_message.reload.author_id).to eq(eve.person.id) end @@ -93,7 +95,7 @@ describe "attack vectors", type: :request do # eve tries to send me another message with the same ID malicious_message = FactoryGirl.build(:status_message, id: original_message.id, text: "BAD!!!", author: eve.person) - post_message(generate_xml(Diaspora::Federation::Entities.post(malicious_message), eve, bob), bob) + post_message(generate_payload(Diaspora::Federation::Entities.post(malicious_message), eve, bob), bob) expect(original_message.reload.text).to eq("store this!") end @@ -109,7 +111,7 @@ describe "attack vectors", type: :request do ) expect { - post_message(generate_xml(retraction, alice, bob), bob) + post_message(generate_payload(retraction, alice, bob), bob) }.to_not change(StatusMessage, :count) end @@ -122,7 +124,7 @@ describe "attack vectors", type: :request do new_message.height = 23 new_message.width = 42 - post_message(generate_xml(Diaspora::Federation::Entities.photo(new_message), alice, bob), bob) + post_message(generate_payload(Diaspora::Federation::Entities.photo(new_message), alice, bob), bob) expect(original_message.reload.text).to eq("store this!") end diff --git a/spec/integration/federation/federation_helper.rb b/spec/integration/federation/federation_helper.rb index f16cd604c..3525c95fe 100644 --- a/spec/integration/federation/federation_helper.rb +++ b/spec/integration/federation/federation_helper.rb @@ -44,30 +44,29 @@ def create_relayable_entity(entity_name, parent, diaspora_id) ) end -def generate_xml(entity, remote_user, recipient=nil) +def generate_payload(entity, remote_user, recipient=nil) + magic_env = DiasporaFederation::Salmon::MagicEnvelope.new( + entity, + remote_user.diaspora_handle + ).envelop(remote_user.encryption_key) + if recipient - DiasporaFederation::Salmon::EncryptedSlap.prepare( - remote_user.diaspora_handle, - remote_user.encryption_key, - entity - ).generate_xml(recipient.encryption_key) + DiasporaFederation::Salmon::EncryptedMagicEnvelope.encrypt(magic_env, recipient.encryption_key) else - DiasporaFederation::Salmon::Slap.generate_xml( - remote_user.diaspora_handle, - remote_user.encryption_key, - entity - ) + magic_env.to_xml end end -def post_message(xml, recipient=nil) +def post_message(payload, recipient=nil) if recipient inlined_jobs do - post "/receive/users/#{recipient.guid}", guid: recipient.guid, xml: xml + headers = {"CONTENT_TYPE" => "application/json"} + post "/receive/users/#{recipient.guid}", payload, headers end else inlined_jobs do - post "/receive/public", xml: xml + headers = {"CONTENT_TYPE" => "application/magic-envelope+xml"} + post "/receive/public", payload, headers end end end diff --git a/spec/integration/federation/receive_federation_messages_spec.rb b/spec/integration/federation/receive_federation_messages_spec.rb index d532b3c71..651c65150 100644 --- a/spec/integration/federation/receive_federation_messages_spec.rb +++ b/spec/integration/federation/receive_federation_messages_spec.rb @@ -15,14 +15,14 @@ describe "Receive federation messages feature" do let(:recipient) { nil } it "receives account deletion correctly" do - post_message(generate_xml(DiasporaFederation::Entities::AccountDeletion.new(diaspora_id: sender_id), sender)) + post_message(generate_payload(DiasporaFederation::Entities::AccountDeletion.new(diaspora_id: sender_id), sender)) expect(AccountDeletion.exists?(diaspora_handle: sender_id)).to be_truthy end it "rejects account deletion with wrong diaspora_id" do delete_id = Fabricate.sequence(:diaspora_id) - post_message(generate_xml(DiasporaFederation::Entities::AccountDeletion.new(diaspora_id: delete_id), sender)) + post_message(generate_payload(DiasporaFederation::Entities::AccountDeletion.new(diaspora_id: delete_id), sender)) expect(AccountDeletion.exists?(diaspora_handle: delete_id)).to be_falsey expect(AccountDeletion.exists?(diaspora_handle: sender_id)).to be_falsey @@ -38,7 +38,7 @@ describe "Receive federation messages feature" do alice, instance_of(Reshare) ).and_return(double(create!: true)) - post_message(generate_xml(reshare, sender)) + post_message(generate_payload(reshare, sender)) expect(Reshare.exists?(root_guid: post.guid)).to be_truthy expect(Reshare.where(root_guid: post.guid).last.diaspora_handle).to eq(sender_id) @@ -49,7 +49,7 @@ describe "Receive federation messages feature" do reshare = Fabricate( :reshare_entity, root_author: alice.diaspora_handle, root_guid: post.guid, author: sender_id) expect { - post_message(generate_xml(reshare, sender)) + post_message(generate_payload(reshare, sender)) }.to raise_error ActiveRecord::RecordInvalid, "Validation failed: Only posts which are public may be reshared." expect(Reshare.exists?(root_guid: post.guid)).to be_falsey @@ -78,7 +78,7 @@ describe "Receive federation messages feature" do expect(Workers::ReceiveLocal).to receive(:perform_async).and_call_original - post_message(generate_xml(entity, sender, alice), alice) + post_message(generate_payload(entity, sender, alice), alice) expect(alice.contacts.count).to eq(2) new_contact = alice.contacts.find {|c| c.person.diaspora_handle == sender_id } @@ -106,7 +106,7 @@ describe "Receive federation messages feature" do it "treats profile receive correctly" do entity = Fabricate(:profile_entity, author: sender_id) - post_message(generate_xml(entity, sender, alice), alice) + post_message(generate_payload(entity, sender, alice), alice) received_profile = sender.profile.reload @@ -120,7 +120,7 @@ describe "Receive federation messages feature" do author: sender_id, participants: "#{sender_id};#{alice.diaspora_handle}" ) - post_message(generate_xml(entity, sender, alice), alice) + post_message(generate_payload(entity, sender, alice), alice) expect(Conversation.exists?(guid: entity.guid)).to be_truthy end diff --git a/spec/integration/federation/shared_receive_relayable.rb b/spec/integration/federation/shared_receive_relayable.rb index 06e67c247..2d0e150ba 100644 --- a/spec/integration/federation/shared_receive_relayable.rb +++ b/spec/integration/federation/shared_receive_relayable.rb @@ -4,7 +4,7 @@ shared_examples_for "it deals correctly with a relayable" do it "treats upstream receive correctly" do expect(Workers::ReceiveLocal).to receive(:perform_async) - post_message(generate_xml(entity, sender, recipient), recipient) + post_message(generate_payload(entity, sender, recipient), recipient) received_entity = klass.find_by(guid: entity.guid) expect(received_entity).not_to be_nil @@ -15,7 +15,7 @@ shared_examples_for "it deals correctly with a relayable" do it "rejects an upstream entity with a malformed author signature" do expect(Workers::ReceiveLocal).not_to receive(:perform_async) allow(remote_user_on_pod_b).to receive(:encryption_key).and_return(OpenSSL::PKey::RSA.new(1024)) - post_message(generate_xml(entity, sender, recipient), recipient) + post_message(generate_payload(entity, sender, recipient), recipient) expect(klass.exists?(guid: entity.guid)).to be_falsey end @@ -28,7 +28,7 @@ shared_examples_for "it deals correctly with a relayable" do it "treats downstream receive correctly" do expect(Workers::ReceiveLocal).to receive(:perform_async) - post_message(generate_xml(entity, sender, recipient), recipient) + post_message(generate_payload(entity, sender, recipient), recipient) received_entity = klass.find_by(guid: entity.guid) expect(received_entity).not_to be_nil @@ -40,7 +40,7 @@ shared_examples_for "it deals correctly with a relayable" do it "rejects a downstream entity with a malformed author signature" do expect(Workers::ReceiveLocal).not_to receive(:perform_async) allow(remote_user_on_pod_c).to receive(:encryption_key).and_return(OpenSSL::PKey::RSA.new(1024)) - post_message(generate_xml(entity, sender, recipient), recipient) + post_message(generate_payload(entity, sender, recipient), recipient) expect(klass.exists?(guid: entity.guid)).to be_falsey end @@ -50,7 +50,7 @@ shared_examples_for "it deals correctly with a relayable" do it "declines downstream receive when sender signed with a wrong key" do expect(Workers::ReceiveLocal).not_to receive(:perform_async) allow(sender).to receive(:encryption_key).and_return(OpenSSL::PKey::RSA.new(1024)) - post_message(generate_xml(entity, sender, recipient), recipient) + post_message(generate_payload(entity, sender, recipient), recipient) expect(klass.exists?(guid: entity.guid)).to be_falsey end diff --git a/spec/integration/federation/shared_receive_retraction.rb b/spec/integration/federation/shared_receive_retraction.rb index b54588524..adc8112c0 100644 --- a/spec/integration/federation/shared_receive_retraction.rb +++ b/spec/integration/federation/shared_receive_retraction.rb @@ -1,6 +1,6 @@ -def retraction_entity(entity_name, target_object, sender) +def retraction_entity(target_object, sender) Fabricate( - entity_name, + :retraction_entity, author: sender.diaspora_handle, target_guid: target_object.guid, target_type: target_object.class.to_s, @@ -10,23 +10,23 @@ end shared_examples_for "it retracts non-relayable object" do it "retracts object by a correct retraction message" do - entity = retraction_entity(entity_name, target_object, sender) - post_message(generate_xml(entity, sender, recipient), recipient) + entity = retraction_entity(target_object, sender) + post_message(generate_payload(entity, sender, recipient), recipient) expect(target_object.class.exists?(guid: target_object.guid)).to be_falsey end it "doesn't retract object when retraction has wrong signatures" do allow(sender).to receive(:encryption_key).and_return(OpenSSL::PKey::RSA.new(1024)) - entity = retraction_entity(entity_name, target_object, sender) - post_message(generate_xml(entity, sender, recipient), recipient) + entity = retraction_entity(target_object, sender) + post_message(generate_payload(entity, sender, recipient), recipient) expect(target_object.class.exists?(guid: target_object.guid)).to be_truthy end it "doesn't retract object when sender is different from target object" do - entity = retraction_entity(entity_name, target_object, remote_user_on_pod_c) - post_message(generate_xml(entity, remote_user_on_pod_c, recipient), recipient) + entity = retraction_entity(target_object, remote_user_on_pod_c) + post_message(generate_payload(entity, remote_user_on_pod_c, recipient), recipient) expect(target_object.class.exists?(guid: target_object.guid)).to be_truthy end @@ -34,16 +34,16 @@ end shared_examples_for "it retracts relayable object" do it "retracts object by a correct message" do - entity = retraction_entity(entity_name, target_object, sender) - post_message(generate_xml(entity, sender, recipient), recipient) + entity = retraction_entity(target_object, sender) + post_message(generate_payload(entity, sender, recipient), recipient) expect(target_object.class.exists?(guid: target_object.guid)).to be_falsey end it "doesn't retract object when retraction has wrong signatures" do allow(sender).to receive(:encryption_key).and_return(OpenSSL::PKey::RSA.new(1024)) - entity = retraction_entity(entity_name, target_object, sender) - post_message(generate_xml(entity, sender, recipient), recipient) + entity = retraction_entity(target_object, sender) + post_message(generate_payload(entity, sender, recipient), recipient) expect(target_object.class.exists?(guid: target_object.guid)).to be_truthy end diff --git a/spec/integration/federation/shared_receive_stream_items.rb b/spec/integration/federation/shared_receive_stream_items.rb index 67d5766b0..e025a138d 100644 --- a/spec/integration/federation/shared_receive_stream_items.rb +++ b/spec/integration/federation/shared_receive_stream_items.rb @@ -6,7 +6,7 @@ shared_examples_for "messages which are indifferent about sharing fact" do it "treats status message receive correctly" do entity = Fabricate(:status_message_entity, author: sender_id, public: public) - post_message(generate_xml(entity, sender, recipient), recipient) + post_message(generate_payload(entity, sender, recipient), recipient) expect(StatusMessage.exists?(guid: entity.guid)).to be_truthy end @@ -15,7 +15,7 @@ shared_examples_for "messages which are indifferent about sharing fact" do allow(sender).to receive(:encryption_key).and_return(OpenSSL::PKey::RSA.new(1024)) entity = Fabricate(:status_message_entity, author: sender_id, public: public) - post_message(generate_xml(entity, sender, recipient), recipient) + post_message(generate_payload(entity, sender, recipient), recipient) expect(StatusMessage.exists?(guid: entity.guid)).to be_falsey end @@ -27,7 +27,7 @@ shared_examples_for "messages which are indifferent about sharing fact" do describe "notifications are sent where required" do it "for comment on local post" do entity = create_relayable_entity(:comment_entity, local_parent, remote_user_on_pod_b.diaspora_handle) - post_message(generate_xml(entity, sender, recipient), recipient) + post_message(generate_payload(entity, sender, recipient), recipient) expect( Notifications::CommentOnPost.exists?( @@ -40,7 +40,7 @@ shared_examples_for "messages which are indifferent about sharing fact" do it "for like on local post" do entity = create_relayable_entity(:like_entity, local_parent, remote_user_on_pod_b.diaspora_handle) - post_message(generate_xml(entity, sender, recipient), recipient) + post_message(generate_payload(entity, sender, recipient), recipient) expect( Notifications::Liked.exists?( @@ -66,7 +66,7 @@ shared_examples_for "messages which are indifferent about sharing fact" do it "treats participation receive correctly" do expect(Workers::ReceiveLocal).to receive(:perform_async) - post_message(generate_xml(entity, sender, recipient), recipient) + post_message(generate_payload(entity, sender, recipient), recipient) received_entity = Participation.find_by(guid: entity.guid) expect(received_entity).not_to be_nil @@ -77,7 +77,7 @@ shared_examples_for "messages which are indifferent about sharing fact" do expect(Workers::ReceiveLocal).not_to receive(:perform_async) entity = create_relayable_entity(:participation_entity, remote_parent, sender_id) - post_message(generate_xml(entity, sender, recipient), recipient) + post_message(generate_payload(entity, sender, recipient), recipient) expect(Participation.exists?(guid: entity.guid)).to be_falsey end @@ -107,17 +107,11 @@ end shared_examples_for "messages which can't be send without sharing" do # retractions shouldn't depend on sharing fact describe "retractions for non-relayable objects" do - %w(retraction signed_retraction).each do |retraction_entity_name| - context "with #{retraction_entity_name}" do - let(:entity_name) { "#{retraction_entity_name}_entity".to_sym } + %w[status_message photo].each do |target| + context "with #{target}" do + let(:target_object) { FactoryGirl.create(target.to_sym, author: remote_user_on_pod_b.person) } - %w(status_message photo).each do |target| - context "with #{target}" do - let(:target_object) { FactoryGirl.create(target.to_sym, author: remote_user_on_pod_b.person) } - - it_behaves_like "it retracts non-relayable object" - end - end + it_behaves_like "it retracts non-relayable object" end end end @@ -133,7 +127,7 @@ shared_examples_for "messages which can't be send without sharing" do alice.participate!(remote_parent) author_id = remote_user_on_pod_c.diaspora_handle entity = create_relayable_entity(:comment_entity, remote_parent, author_id) - post_message(generate_xml(entity, sender, recipient), recipient) + post_message(generate_payload(entity, sender, recipient), recipient) expect( Notifications::AlsoCommented.exists?( @@ -146,47 +140,41 @@ shared_examples_for "messages which can't be send without sharing" do end describe "retractions for relayable objects" do - %w(retraction signed_retraction relayable_retraction).each do |retraction_entity_name| - context "with #{retraction_entity_name}" do - let(:entity_name) { "#{retraction_entity_name}_entity".to_sym } + before do + allow(DiasporaFederation.callbacks).to receive(:trigger).with( + :fetch_private_key, alice.diaspora_handle + ) { alice.encryption_key } + end - before do - allow(DiasporaFederation.callbacks).to receive(:trigger).with( - :fetch_private_key, alice.diaspora_handle - ) { alice.encryption_key } - end + context "with comment" do + it_behaves_like "it retracts relayable object" do + # case for to-upstream federation + let(:target_object) { + FactoryGirl.create(:comment, author: remote_user_on_pod_b.person, post: local_parent) + } + end - context "with comment" do - it_behaves_like "it retracts relayable object" do - # case for to-upstream federation - let(:target_object) { - FactoryGirl.create(:comment, author: remote_user_on_pod_b.person, post: local_parent) - } - end + it_behaves_like "it retracts relayable object" do + # case for to-downsteam federation + let(:target_object) { + FactoryGirl.create(:comment, author: remote_user_on_pod_c.person, post: remote_parent) + } + end + end - it_behaves_like "it retracts relayable object" do - # case for to-downsteam federation - let(:target_object) { - FactoryGirl.create(:comment, author: remote_user_on_pod_c.person, post: remote_parent) - } - end - end + context "with like" do + it_behaves_like "it retracts relayable object" do + # case for to-upstream federation + let(:target_object) { + FactoryGirl.create(:like, author: remote_user_on_pod_b.person, target: local_parent) + } + end - context "with like" do - it_behaves_like "it retracts relayable object" do - # case for to-upstream federation - let(:target_object) { - FactoryGirl.create(:like, author: remote_user_on_pod_b.person, target: local_parent) - } - end - - it_behaves_like "it retracts relayable object" do - # case for to-downsteam federation - let(:target_object) { - FactoryGirl.create(:like, author: remote_user_on_pod_c.person, target: remote_parent) - } - end - end + it_behaves_like "it retracts relayable object" do + # case for to-downsteam federation + let(:target_object) { + FactoryGirl.create(:like, author: remote_user_on_pod_c.person, target: remote_parent) + } end end end diff --git a/spec/lib/diaspora/federation/dispatcher/private_spec.rb b/spec/lib/diaspora/federation/dispatcher/private_spec.rb index 9f20ece51..b1fd867a6 100644 --- a/spec/lib/diaspora/federation/dispatcher/private_spec.rb +++ b/spec/lib/diaspora/federation/dispatcher/private_spec.rb @@ -28,18 +28,29 @@ describe Diaspora::Federation::Dispatcher::Private do end context "deliver to remote user" do - let(:xml) { "" } + let(:encryption_key) { double } + let(:magic_env) { double } + let(:magic_env_xml) { double } + let(:json) { "{\"aes_key\": \"...\", \"encrypted_magic_envelope\": \"...\"}" } + it "queues a private send job" do expect(Workers::SendPrivate).to receive(:perform_async) do |user_id, _entity_string, targets| expect(user_id).to eq(alice.id) expect(targets.size).to eq(1) expect(targets).to have_key(remote_raphael.receive_url) - expect(targets[remote_raphael.receive_url]).to eq(xml) + expect(targets[remote_raphael.receive_url]).to eq(json) end - salmon = double - expect(DiasporaFederation::Salmon::EncryptedSlap).to receive(:prepare).and_return(salmon) - expect(salmon).to receive(:generate_xml).and_return(xml) + expect(alice).to receive(:encryption_key).and_return(encryption_key) + expect(DiasporaFederation::Salmon::MagicEnvelope).to receive(:new).with( + instance_of(DiasporaFederation::Entities::StatusMessage), alice.diaspora_handle + ).and_return(magic_env) + expect(magic_env).to receive(:envelop).with(encryption_key).and_return(magic_env_xml) + expect(DiasporaFederation::Salmon::EncryptedMagicEnvelope).to receive(:encrypt) do |magic_env, public_key| + expect(magic_env).to eq(magic_env_xml) + expect(public_key.to_s).to eq(remote_raphael.public_key.to_s) + json + end Diaspora::Federation::Dispatcher.build(alice, post).dispatch end @@ -60,12 +71,19 @@ describe Diaspora::Federation::Dispatcher::Private do expect(user_id).to eq(alice.id) expect(targets.size).to eq(1) expect(targets).to have_key(remote_person.receive_url) - expect(targets[remote_person.receive_url]).to eq(xml) + expect(targets[remote_person.receive_url]).to eq(json) end - salmon = double - expect(DiasporaFederation::Salmon::EncryptedSlap).to receive(:prepare).and_return(salmon) - expect(salmon).to receive(:generate_xml).and_return(xml) + expect(alice).to receive(:encryption_key).and_return(encryption_key) + expect(DiasporaFederation::Salmon::MagicEnvelope).to receive(:new).with( + instance_of(DiasporaFederation::Entities::StatusMessage), alice.diaspora_handle + ).and_return(magic_env) + expect(magic_env).to receive(:envelop).with(encryption_key).and_return(magic_env_xml) + expect(DiasporaFederation::Salmon::EncryptedMagicEnvelope).to receive(:encrypt) do |magic_env, public_key| + expect(magic_env).to eq(magic_env_xml) + expect(public_key.to_s).to eq(remote_raphael.public_key.to_s) + json + end Diaspora::Federation::Dispatcher.build(alice, post, subscribers: [remote_person]).dispatch end @@ -79,12 +97,19 @@ describe Diaspora::Federation::Dispatcher::Private do expect(user_id).to eq(alice.id) expect(targets.size).to eq(1) expect(targets).to have_key(remote_person.receive_url) - expect(targets[remote_person.receive_url]).to eq(xml) + expect(targets[remote_person.receive_url]).to eq(json) end - salmon = double - expect(DiasporaFederation::Salmon::EncryptedSlap).to receive(:prepare).and_return(salmon) - expect(salmon).to receive(:generate_xml).and_return(xml) + expect(alice).to receive(:encryption_key).and_return(encryption_key) + expect(DiasporaFederation::Salmon::MagicEnvelope).to receive(:new).with( + instance_of(DiasporaFederation::Entities::StatusMessage), alice.diaspora_handle + ).and_return(magic_env) + expect(magic_env).to receive(:envelop).with(encryption_key).and_return(magic_env_xml) + expect(DiasporaFederation::Salmon::EncryptedMagicEnvelope).to receive(:encrypt) do |magic_env, public_key| + expect(magic_env).to eq(magic_env_xml) + expect(public_key.to_s).to eq(remote_raphael.public_key.to_s) + json + end Diaspora::Federation::Dispatcher.build(alice, post, subscribers: [remote_person, offline_person]).dispatch end diff --git a/spec/lib/diaspora/federation/dispatcher/public_spec.rb b/spec/lib/diaspora/federation/dispatcher/public_spec.rb index bb5bac652..f893c7a57 100644 --- a/spec/lib/diaspora/federation/dispatcher/public_spec.rb +++ b/spec/lib/diaspora/federation/dispatcher/public_spec.rb @@ -48,7 +48,9 @@ describe Diaspora::Federation::Dispatcher::Public do end context "deliver to remote user" do - let(:salmon_xml) { "" } + let(:encryption_key) { double } + let(:magic_env) { double } + let(:magic_env_xml) { double(to_xml: "") } it "queues a public send job" do alice.share_with(remote_raphael, alice.aspects.first) @@ -57,10 +59,14 @@ describe Diaspora::Federation::Dispatcher::Public do expect(user_id).to eq(alice.id) expect(urls.size).to eq(1) expect(urls[0]).to eq(remote_raphael.pod.url_to("/receive/public")) - expect(xml).to eq(salmon_xml) + expect(xml).to eq(magic_env_xml.to_xml) end - expect(DiasporaFederation::Salmon::Slap).to receive(:generate_xml).and_return(salmon_xml) + expect(alice).to receive(:encryption_key).and_return(encryption_key) + expect(DiasporaFederation::Salmon::MagicEnvelope).to receive(:new).with( + instance_of(DiasporaFederation::Entities::StatusMessage), alice.diaspora_handle + ).and_return(magic_env) + expect(magic_env).to receive(:envelop).with(encryption_key).and_return(magic_env_xml) Diaspora::Federation::Dispatcher.build(alice, post).dispatch end @@ -76,11 +82,14 @@ describe Diaspora::Federation::Dispatcher::Public do expect(user_id).to eq(alice.id) expect(urls.size).to eq(1) expect(urls[0]).to eq(remote_raphael.pod.url_to("/receive/public")) - expect(xml).to eq(salmon_xml) + expect(xml).to eq(magic_env_xml.to_xml) end - expect(DiasporaFederation::Salmon::Slap).to receive(:generate_xml).and_return(salmon_xml) - + expect(alice).to receive(:encryption_key).and_return(encryption_key) + expect(DiasporaFederation::Salmon::MagicEnvelope).to receive(:new).with( + instance_of(DiasporaFederation::Entities::StatusMessage), alice.diaspora_handle + ).and_return(magic_env) + expect(magic_env).to receive(:envelop).with(encryption_key).and_return(magic_env_xml) Diaspora::Federation::Dispatcher.build(alice, post, subscribers: [remote_raphael]).dispatch end @@ -92,10 +101,14 @@ describe Diaspora::Federation::Dispatcher::Public do expect(user_id).to eq(alice.id) expect(urls.size).to eq(1) expect(urls[0]).to eq(remote_raphael.pod.url_to("/receive/public")) - expect(xml).to eq(salmon_xml) + expect(xml).to eq(magic_env_xml.to_xml) end - expect(DiasporaFederation::Salmon::Slap).to receive(:generate_xml).and_return(salmon_xml) + expect(alice).to receive(:encryption_key).and_return(encryption_key) + expect(DiasporaFederation::Salmon::MagicEnvelope).to receive(:new).with( + instance_of(DiasporaFederation::Entities::StatusMessage), alice.diaspora_handle + ).and_return(magic_env) + expect(magic_env).to receive(:envelop).with(encryption_key).and_return(magic_env_xml) Diaspora::Federation::Dispatcher.build(alice, post, subscribers: [remote_raphael, offline_person]).dispatch end From 246d1ebbdf9efdb6663081179fb85b44ce78fd38 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Sun, 30 Apr 2017 17:07:06 +0200 Subject: [PATCH 14/24] Remove legacy post xml This was used for the old post fetching Related to diaspora/diaspora_federation#31 --- app/controllers/posts_controller.rb | 1 - spec/controllers/posts_controller_spec.rb | 6 ------ 2 files changed, 7 deletions(-) diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index 2441173bc..6435ed3ad 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -26,7 +26,6 @@ class PostsController < ApplicationController render locals: {post: presenter} end format.mobile { render locals: {post: post} } - format.xml { render xml: DiasporaFederation::Salmon::XmlPayload.pack(Diaspora::Federation::Entities.post(post)) } format.json { render json: presenter } end end diff --git a/spec/controllers/posts_controller_spec.rb b/spec/controllers/posts_controller_spec.rb index 2ce6b677c..efbc1d8dd 100644 --- a/spec/controllers/posts_controller_spec.rb +++ b/spec/controllers/posts_controller_spec.rb @@ -76,12 +76,6 @@ describe PostsController, type: :controller do expect(response.body).to match "hello" end - it "responds with diaspora xml if format is xml" do - get :show, id: public.guid, format: :xml - expected_xml = DiasporaFederation::Salmon::XmlPayload.pack(Diaspora::Federation::Entities.post(public)).to_xml - expect(response.body).to eq(expected_xml) - end - it "includes the correct uniques meta tags" do presenter = PostPresenter.new(public) methods_properties = { From 07e9bf8be885609c1d40e16d7e100fe4146f8573 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Sun, 30 Apr 2017 17:42:19 +0200 Subject: [PATCH 15/24] Remove old unused salmon method --- app/models/user.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 2332f08d2..940a48f25 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -268,10 +268,6 @@ class User < ActiveRecord::Base end end - def salmon(post) - Salmon::EncryptedSlap.create_by_user_and_activity(self, post.to_diaspora_xml) - end - # Check whether the user has liked a post. # @param [Post] post def liked?(target) From 4244f2a5dfe6a37b814cc9cfadfecc8504a73468 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Sun, 30 Apr 2017 17:57:05 +0200 Subject: [PATCH 16/24] Make Participation entity non-relayable Related to diaspora/diaspora_federation#35 --- .../federation/shared_receive_stream_items.rb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/spec/integration/federation/shared_receive_stream_items.rb b/spec/integration/federation/shared_receive_stream_items.rb index e025a138d..86cb4125b 100644 --- a/spec/integration/federation/shared_receive_stream_items.rb +++ b/spec/integration/federation/shared_receive_stream_items.rb @@ -62,7 +62,14 @@ shared_examples_for "messages which are indifferent about sharing fact" do end context "with participations" do - let(:entity) { create_relayable_entity(:participation_entity, local_parent, sender_id) } + let(:entity) { + Fabricate( + :participation_entity, + author: sender_id, + parent_guid: local_parent.guid, + parent: Diaspora::Federation::Entities.related_entity(local_parent) + ) + } it "treats participation receive correctly" do expect(Workers::ReceiveLocal).to receive(:perform_async) From 0f551c7b190220f0e3b4218ebebbe18b28fd3ea1 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Mon, 1 May 2017 03:34:37 +0200 Subject: [PATCH 17/24] Fix public flag for contact retractions --- lib/diaspora/federated/retraction.rb | 2 +- spec/lib/diaspora/federated/retraction_spec.rb | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/diaspora/federated/retraction.rb b/lib/diaspora/federated/retraction.rb index b9c73dd5e..756b94f32 100644 --- a/lib/diaspora/federated/retraction.rb +++ b/lib/diaspora/federated/retraction.rb @@ -31,7 +31,7 @@ class Retraction end def public? - data[:target][:public] + data[:target] && data[:target][:public] end private diff --git a/spec/lib/diaspora/federated/retraction_spec.rb b/spec/lib/diaspora/federated/retraction_spec.rb index 3dad77b2f..864b03203 100644 --- a/spec/lib/diaspora/federated/retraction_spec.rb +++ b/spec/lib/diaspora/federated/retraction_spec.rb @@ -40,7 +40,7 @@ describe Retraction do end it "creates a retraction for a contact" do - contact = FactoryGirl.create(:contact) + contact = FactoryGirl.build(:contact, receiving: false) expect(Diaspora::Federation::Entities).to receive(:retraction_data_for).with(contact) @@ -156,5 +156,10 @@ describe Retraction do private_post = alice.post(:status_message, text: "destroy!", to: alice.aspects.first.id) expect(Retraction.for(private_post).public?).to be_falsey end + + it "returns false for a contact retraction" do + contact = FactoryGirl.create(:contact, receiving: false) + expect(Retraction.for(contact).public?).to be_falsey + end end end From 3ab674552fc1ff041c0b39dd401b99d765b681f5 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Mon, 1 May 2017 21:38:55 +0200 Subject: [PATCH 18/24] Rename xml_order to signature_order --- lib/diaspora/federation/receive.rb | 2 +- spec/lib/diaspora/federation/entities_spec.rb | 9 +++------ spec/lib/diaspora/federation/receive_spec.rb | 2 +- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/lib/diaspora/federation/receive.rb b/lib/diaspora/federation/receive.rb index 6fd7f34bc..3a33a0d74 100644 --- a/lib/diaspora/federation/receive.rb +++ b/lib/diaspora/federation/receive.rb @@ -257,7 +257,7 @@ module Diaspora klass.reflect_on_association(:signature).klass.new( author_signature: entity.author_signature, additional_data: entity.additional_data, - signature_order: SignatureOrder.find_or_create_by!(order: entity.xml_order.join(" ")) + signature_order: SignatureOrder.find_or_create_by!(order: entity.signature_order.join(" ")) ) end diff --git a/spec/lib/diaspora/federation/entities_spec.rb b/spec/lib/diaspora/federation/entities_spec.rb index 5263ff599..c94f023e1 100644 --- a/spec/lib/diaspora/federation/entities_spec.rb +++ b/spec/lib/diaspora/federation/entities_spec.rb @@ -18,7 +18,6 @@ describe Diaspora::Federation::Entities do expect(federation_entity.parent_guid).to eq(diaspora_entity.post.guid) expect(federation_entity.text).to eq(diaspora_entity.text) expect(federation_entity.author_signature).to be_nil - expect(federation_entity.xml_order).to be_nil expect(federation_entity.additional_data).to be_empty end @@ -32,7 +31,7 @@ describe Diaspora::Federation::Entities do expect(federation_entity.parent_guid).to eq(diaspora_entity.post.guid) expect(federation_entity.text).to eq(diaspora_entity.text) expect(federation_entity.author_signature).to eq(diaspora_entity.signature.author_signature) - expect(federation_entity.xml_order).to eq(diaspora_entity.signature.signature_order.order.split) + expect(federation_entity.signature_order.map(&:to_s)).to eq(diaspora_entity.signature.signature_order.order.split) expect(federation_entity.additional_data).to eq(diaspora_entity.signature.additional_data) end @@ -87,7 +86,6 @@ describe Diaspora::Federation::Entities do expect(federation_entity.parent_guid).to eq(diaspora_entity.target.guid) expect(federation_entity.positive).to eq(diaspora_entity.positive) expect(federation_entity.author_signature).to be_nil - expect(federation_entity.xml_order).to be_nil expect(federation_entity.additional_data).to be_empty end @@ -101,7 +99,7 @@ describe Diaspora::Federation::Entities do expect(federation_entity.parent_guid).to eq(diaspora_entity.target.guid) expect(federation_entity.positive).to eq(diaspora_entity.positive) expect(federation_entity.author_signature).to eq(diaspora_entity.signature.author_signature) - expect(federation_entity.xml_order).to eq(diaspora_entity.signature.signature_order.order.split) + expect(federation_entity.signature_order.map(&:to_s)).to eq(diaspora_entity.signature.signature_order.order.split) expect(federation_entity.additional_data).to eq(diaspora_entity.signature.additional_data) end @@ -154,7 +152,6 @@ describe Diaspora::Federation::Entities do expect(federation_entity.parent_guid).to eq(diaspora_entity.poll_answer.poll.guid) expect(federation_entity.poll_answer_guid).to eq(diaspora_entity.poll_answer.guid) expect(federation_entity.author_signature).to be_nil - expect(federation_entity.xml_order).to be_nil expect(federation_entity.additional_data).to be_empty end @@ -169,7 +166,7 @@ describe Diaspora::Federation::Entities do expect(federation_entity.parent_guid).to eq(diaspora_entity.poll_answer.poll.guid) expect(federation_entity.poll_answer_guid).to eq(diaspora_entity.poll_answer.guid) expect(federation_entity.author_signature).to eq(signature.author_signature) - expect(federation_entity.xml_order).to eq(signature.signature_order.order.split) + expect(federation_entity.signature_order.map(&:to_s)).to eq(signature.signature_order.order.split) expect(federation_entity.additional_data).to eq(signature.additional_data) end diff --git a/spec/lib/diaspora/federation/receive_spec.rb b/spec/lib/diaspora/federation/receive_spec.rb index 1bff8ddad..06b3bef92 100644 --- a/spec/lib/diaspora/federation/receive_spec.rb +++ b/spec/lib/diaspora/federation/receive_spec.rb @@ -55,7 +55,7 @@ describe Diaspora::Federation::Receive do expect(comment.signature).not_to be_nil expect(comment.signature.author_signature).to eq("aa") expect(comment.signature.additional_data).to eq("new_property" => "data") - expect(comment.signature.order).to eq(comment_entity.xml_order.map(&:to_s)) + expect(comment.signature.order).to eq(comment_entity.signature_order.map(&:to_s)) end let(:entity) { comment_entity } From 283722a6939e8fa662cc36f31467b4ff63f53947 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Tue, 2 May 2017 02:27:24 +0200 Subject: [PATCH 19/24] Use build_relayable_federation_entity helper for receive specs --- spec/helper_methods.rb | 4 +- spec/lib/diaspora/federation/receive_spec.rb | 42 +++++++++----------- 2 files changed, 20 insertions(+), 26 deletions(-) diff --git a/spec/helper_methods.rb b/spec/helper_methods.rb index 3eb9ff3a3..f7406a3ef 100644 --- a/spec/helper_methods.rb +++ b/spec/helper_methods.rb @@ -53,8 +53,8 @@ module HelperMethods def build_relayable_federation_entity(type, data={}, additional_data={}) attributes = Fabricate.attributes_for("#{type}_entity".to_sym, data) - entity_class = "DiasporaFederation::Entities::#{type.capitalize}".constantize - signable_fields = attributes.keys - [:author_signature] + entity_class = "DiasporaFederation::Entities::#{type.to_s.camelize}".constantize + signable_fields = attributes.keys - %i[author_signature parent] entity_class.new(attributes, [*signable_fields, *additional_data.keys], additional_data) end diff --git a/spec/lib/diaspora/federation/receive_spec.rb b/spec/lib/diaspora/federation/receive_spec.rb index 06b3bef92..fd943b06d 100644 --- a/spec/lib/diaspora/federation/receive_spec.rb +++ b/spec/lib/diaspora/federation/receive_spec.rb @@ -183,17 +183,15 @@ describe Diaspora::Federation::Receive do end describe ".like" do - let(:like_data) { - Fabricate.attributes_for( - :like_entity, - author: sender.diaspora_handle, - parent_guid: post.guid, - author_signature: "aa" - ) - } let(:like_entity) { - DiasporaFederation::Entities::Like.new( - like_data, [:author, :guid, :parent_guid, :parent_type, :positive, "new_property"], "new_property" => "data" + build_relayable_federation_entity( + :like, + { + author: sender.diaspora_handle, + parent_guid: post.guid, + author_signature: "aa" + }, + "new_property" => "data" ) } @@ -224,7 +222,7 @@ describe Diaspora::Federation::Receive do expect(like.signature).not_to be_nil expect(like.signature.author_signature).to eq("aa") expect(like.signature.additional_data).to eq("new_property" => "data") - expect(like.signature.order).to eq(%w(author guid parent_guid parent_type positive new_property)) + expect(like.signature.order).to eq(like_entity.signature_order.map(&:to_s)) end let(:entity) { like_entity } @@ -351,19 +349,15 @@ describe Diaspora::Federation::Receive do describe ".poll_participation" do let(:post_with_poll) { FactoryGirl.create(:status_message_with_poll, author: alice.person) } - let(:poll_participation_data) { - Fabricate.attributes_for( - :poll_participation_entity, - author: sender.diaspora_handle, - parent_guid: post_with_poll.poll.guid, - poll_answer_guid: post_with_poll.poll.poll_answers.first.guid, - author_signature: "aa" - ) - } let(:poll_participation_entity) { - DiasporaFederation::Entities::PollParticipation.new( - poll_participation_data, - [:author, :guid, :parent_guid, :poll_answer_guid, "new_property"], + build_relayable_federation_entity( + :poll_participation, + { + author: sender.diaspora_handle, + parent_guid: post_with_poll.poll.guid, + poll_answer_guid: post_with_poll.poll.poll_answers.first.guid, + author_signature: "aa" + }, "new_property" => "data" ) } @@ -395,7 +389,7 @@ describe Diaspora::Federation::Receive do expect(poll_participation.signature).not_to be_nil expect(poll_participation.signature.author_signature).to eq("aa") expect(poll_participation.signature.additional_data).to eq("new_property" => "data") - expect(poll_participation.signature.order).to eq(%w(author guid parent_guid poll_answer_guid new_property)) + expect(poll_participation.signature.order).to eq(poll_participation_entity.signature_order.map(&:to_s)) end let(:entity) { poll_participation_entity } From 4f9e560ab3194e46b91bc49faf81bb11de29c41e Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Sat, 13 May 2017 23:54:55 +0200 Subject: [PATCH 20/24] Use RFC 7033 webfinger from diaspora_federation gem --- .../openid_connect/discovery_controller.rb | 11 --------- config/initializers/diaspora_federation.rb | 24 ++++++++++++------- config/routes.rb | 1 - .../discovery_controller_spec.rb | 16 ------------- spec/federation_callbacks_spec.rb | 9 ++++++- .../api/openid_connect/id_token_spec.rb | 14 ++++------- 6 files changed, 28 insertions(+), 47 deletions(-) diff --git a/app/controllers/api/openid_connect/discovery_controller.rb b/app/controllers/api/openid_connect/discovery_controller.rb index 19c9001b4..f648e3566 100644 --- a/app/controllers/api/openid_connect/discovery_controller.rb +++ b/app/controllers/api/openid_connect/discovery_controller.rb @@ -24,17 +24,6 @@ module Api module OpenidConnect class DiscoveryController < ApplicationController - def webfinger - jrd = { - links: [{ - rel: OpenIDConnect::Discovery::Provider::Issuer::REL_VALUE, - href: root_url - }] - } - jrd[:subject] = params[:resource] if params[:resource].present? - render json: jrd, content_type: "application/jrd+json" - end - def configuration render json: OpenIDConnect::Discovery::Provider::Config::Response.new( issuer: root_url, diff --git a/config/initializers/diaspora_federation.rb b/config/initializers/diaspora_federation.rb index 5acd33085..e9270d469 100644 --- a/config/initializers/diaspora_federation.rb +++ b/config/initializers/diaspora_federation.rb @@ -13,14 +13,22 @@ DiasporaFederation.configure do |config| person = Person.where(diaspora_handle: diaspora_id, closed_account: false).where.not(owner: nil).first if person DiasporaFederation::Discovery::WebFinger.new( - acct_uri: "acct:#{person.diaspora_handle}", - alias_url: AppConfig.url_to("/people/#{person.guid}"), - hcard_url: AppConfig.url_to(DiasporaFederation::Engine.routes.url_helpers.hcard_path(person.guid)), - seed_url: AppConfig.pod_uri, - profile_url: person.profile_url, - atom_url: person.atom_url, - salmon_url: person.receive_url, - subscribe_url: AppConfig.url_to("/people?q={uri}") + { + acct_uri: "acct:#{person.diaspora_handle}", + hcard_url: AppConfig.url_to(DiasporaFederation::Engine.routes.url_helpers.hcard_path(person.guid)), + seed_url: AppConfig.pod_uri, + profile_url: person.profile_url, + atom_url: person.atom_url, + salmon_url: person.receive_url, + subscribe_url: AppConfig.url_to("/people?q={uri}") + }, + aliases: [AppConfig.url_to("/people/#{person.guid}")], + links: [ + { + rel: OpenIDConnect::Discovery::Provider::Issuer::REL_VALUE, + href: Rails.application.routes.url_helpers.root_url + } + ] ) end end diff --git a/config/routes.rb b/config/routes.rb index a4313011d..355154a6f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -230,6 +230,5 @@ Diaspora::Application.routes.draw do end end - get ".well-known/webfinger", to: "api/openid_connect/discovery#webfinger" get ".well-known/openid-configuration", to: "api/openid_connect/discovery#configuration" end diff --git a/spec/controllers/api/openid_connect/discovery_controller_spec.rb b/spec/controllers/api/openid_connect/discovery_controller_spec.rb index 9d2ff4526..73d6e51fd 100644 --- a/spec/controllers/api/openid_connect/discovery_controller_spec.rb +++ b/spec/controllers/api/openid_connect/discovery_controller_spec.rb @@ -1,20 +1,4 @@ describe Api::OpenidConnect::DiscoveryController, type: :controller do - describe "#webfinger" do - before do - get :webfinger, resource: "http://example.com/bob" - end - - it "should return a url to the openid-configuration" do - json_body = JSON.parse(response.body) - expect(json_body["links"].first["href"]).to eq(root_url) - end - - it "should return the resource in the subject" do - json_body = JSON.parse(response.body) - expect(json_body["subject"]).to eq("http://example.com/bob") - end - end - describe "#configuration" do before do get :configuration diff --git a/spec/federation_callbacks_spec.rb b/spec/federation_callbacks_spec.rb index 33040b30c..7a5ddb58e 100644 --- a/spec/federation_callbacks_spec.rb +++ b/spec/federation_callbacks_spec.rb @@ -6,7 +6,6 @@ describe "diaspora federation callbacks" do person = alice.person wf = DiasporaFederation.callbacks.trigger(:fetch_person_for_webfinger, alice.diaspora_handle) expect(wf.acct_uri).to eq("acct:#{person.diaspora_handle}") - expect(wf.alias_url).to eq(AppConfig.url_to("/people/#{person.guid}")) expect(wf.hcard_url).to eq(AppConfig.url_to("/hcard/users/#{person.guid}")) expect(wf.seed_url).to eq(AppConfig.pod_uri) expect(wf.profile_url).to eq(person.profile_url) @@ -15,6 +14,14 @@ describe "diaspora federation callbacks" do expect(wf.subscribe_url).to eq(AppConfig.url_to("/people?q={uri}")) end + it "contains the OpenID issuer" do + wf = DiasporaFederation.callbacks.trigger(:fetch_person_for_webfinger, alice.diaspora_handle) + links = wf.additional_data[:links] + openid_issuer = links.find {|l| l[:rel] == OpenIDConnect::Discovery::Provider::Issuer::REL_VALUE } + expect(openid_issuer).not_to be_nil + expect(openid_issuer[:href]).to eq(Rails.application.routes.url_helpers.root_url) + end + it "returns nil if the person was not found" do wf = DiasporaFederation.callbacks.trigger(:fetch_person_for_webfinger, "unknown@example.com") expect(wf).to be_nil diff --git a/spec/models/api/openid_connect/id_token_spec.rb b/spec/models/api/openid_connect/id_token_spec.rb index 7c85c3cb4..154bbd90f 100644 --- a/spec/models/api/openid_connect/id_token_spec.rb +++ b/spec/models/api/openid_connect/id_token_spec.rb @@ -7,19 +7,13 @@ describe Api::OpenidConnect::IdToken, type: :model do let(:decoded_hash) { JSON::JWT.decode(id_token.to_jwt, Api::OpenidConnect::IdTokenConfig::PRIVATE_KEY) } - let(:discovery_controller) { - Api::OpenidConnect::DiscoveryController.new.tap {|controller| - controller.request = ActionController::TestRequest.new - controller.request.host = AppConfig.pod_uri.authority - controller.response = ActionController::TestResponse.new - } - } - let(:openid_webfinger) { - JSON.parse(discovery_controller.webfinger[0]) + let(:webfinger) { + DiasporaFederation.callbacks.trigger(:fetch_person_for_webfinger, alice.diaspora_handle).to_json } it "issuer value must much the one we provided in OpenID discovery routine" do - expect(decoded_hash["iss"]).to eq(openid_webfinger["links"][0]["href"]) + openid_issuer = webfinger[:links].find {|l| l[:rel] == OpenIDConnect::Discovery::Provider::Issuer::REL_VALUE } + expect(decoded_hash["iss"]).to eq(openid_issuer[:href]) end end end From e2a40bb643dd870ba69de74ab2cac62cfb5aac27 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Sun, 14 May 2017 22:16:20 +0200 Subject: [PATCH 21/24] Send Relayables with parent author when the parent is local Needed for diaspora/diaspora_federation#64 --- lib/diaspora/relayable.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/diaspora/relayable.rb b/lib/diaspora/relayable.rb index c471636db..f7c023291 100644 --- a/lib/diaspora/relayable.rb +++ b/lib/diaspora/relayable.rb @@ -36,7 +36,6 @@ module Diaspora end end - # @deprecated This is only needed for pre 0.6 pods def sender_for_dispatch parent.author.owner if parent.author.local? end From c2b9b8ab54dea4df540ea4e5761cc29c90990cd5 Mon Sep 17 00:00:00 2001 From: cmrd Senya Date: Sun, 4 Jun 2017 16:43:20 +0200 Subject: [PATCH 22/24] Use ContactRetraction for stop sharing with someone --- app/models/user/connecting.rb | 3 +-- app/workers/deferred_retraction.rb | 4 ++-- lib/diaspora/federated.rb | 1 + lib/diaspora/federated/contact_retraction.rb | 18 +++++++++++++++++ lib/diaspora/federated/retraction.rb | 19 +++++++++++++++--- lib/diaspora/federation/entities.rb | 21 +------------------- lib/diaspora/federation/mappings.rb | 1 + 7 files changed, 40 insertions(+), 27 deletions(-) create mode 100644 lib/diaspora/federated/contact_retraction.rb diff --git a/app/models/user/connecting.rb b/app/models/user/connecting.rb index 558cb7de4..161fb429a 100644 --- a/app/models/user/connecting.rb +++ b/app/models/user/connecting.rb @@ -34,8 +34,7 @@ class User if contact.person.local? contact.person.owner.disconnected_by(contact.user.person) else - contact.receiving = false - Retraction.for(contact).defer_dispatch(self) + ContactRetraction.for(contact).defer_dispatch(self) end contact.aspect_memberships.delete_all diff --git a/app/workers/deferred_retraction.rb b/app/workers/deferred_retraction.rb index 5f3b8834a..f55e9c405 100644 --- a/app/workers/deferred_retraction.rb +++ b/app/workers/deferred_retraction.rb @@ -6,10 +6,10 @@ module Workers class DeferredRetraction < Base sidekiq_options queue: :high - def perform(user_id, retraction_data, recipient_ids, opts) + def perform(user_id, retraction_class, retraction_data, recipient_ids, opts) user = User.find(user_id) subscribers = Person.where(id: recipient_ids) - object = Retraction.new(retraction_data.deep_symbolize_keys, subscribers) + object = retraction_class.constantize.new(retraction_data.deep_symbolize_keys, subscribers) opts = HashWithIndifferentAccess.new(opts) Diaspora::Federation::Dispatcher.build(user, object, opts).dispatch diff --git a/lib/diaspora/federated.rb b/lib/diaspora/federated.rb index 348364fa0..e00fbd86a 100644 --- a/lib/diaspora/federated.rb +++ b/lib/diaspora/federated.rb @@ -6,5 +6,6 @@ module Diaspora module Federated require "diaspora/federated/base" require "diaspora/federated/retraction" + require "diaspora/federated/contact_retraction" end end diff --git a/lib/diaspora/federated/contact_retraction.rb b/lib/diaspora/federated/contact_retraction.rb new file mode 100644 index 000000000..7ff7cc6f5 --- /dev/null +++ b/lib/diaspora/federated/contact_retraction.rb @@ -0,0 +1,18 @@ +class ContactRetraction < Retraction + def self.entity_class + DiasporaFederation::Entities::Contact + end + + def self.retraction_data_for(target) + Diaspora::Federation::Entities.contact(target).to_h + end + + def self.for(target) + target.receiving = false + super + end + + def public? + false + end +end diff --git a/lib/diaspora/federated/retraction.rb b/lib/diaspora/federated/retraction.rb index 756b94f32..d6343c6fc 100644 --- a/lib/diaspora/federated/retraction.rb +++ b/lib/diaspora/federated/retraction.rb @@ -14,14 +14,27 @@ class Retraction @target = target end + def self.entity_class + DiasporaFederation::Entities::Retraction + end + + def self.retraction_data_for(target) + DiasporaFederation::Entities::Retraction.new( + target_guid: target.guid, + target: Diaspora::Federation::Entities.related_entity(target), + target_type: Diaspora::Federation::Mappings.entity_name_for(target), + author: target.diaspora_handle + ).to_h + end + def self.for(target) - federation_retraction_data = Diaspora::Federation::Entities.retraction_data_for(target) + federation_retraction_data = retraction_data_for(target) new(federation_retraction_data, target.subscribers.select(&:remote?), target) end def defer_dispatch(user, include_target_author=true) subscribers = dispatch_subscribers(include_target_author) - Workers::DeferredRetraction.perform_async(user.id, data, subscribers.map(&:id), service_opts(user)) + Workers::DeferredRetraction.perform_async(user.id, self.class.to_s, data, subscribers.map(&:id), service_opts(user)) end def perform @@ -31,7 +44,7 @@ class Retraction end def public? - data[:target] && data[:target][:public] + data[:target][:public] end private diff --git a/lib/diaspora/federation/entities.rb b/lib/diaspora/federation/entities.rb index 8286fef62..dd1da2fb8 100644 --- a/lib/diaspora/federation/entities.rb +++ b/lib/diaspora/federation/entities.rb @@ -178,26 +178,7 @@ module Diaspora end def self.retraction(retraction) - case retraction.data[:target_type] - when "Contact" - DiasporaFederation::Entities::Contact.new(retraction.data) - else - DiasporaFederation::Entities::Retraction.new(retraction.data) - end - end - - def self.retraction_data_for(target) - case target - when Contact - contact(target).to_h.tap {|data| data[:target_type] = "Contact" } - else - DiasporaFederation::Entities::Retraction.new( - target_guid: target.guid, - target_type: Mappings.entity_name_for(target), - target: related_entity(target), - author: target.diaspora_handle - ).to_h - end + retraction.class.entity_class.new(retraction.data) end def self.status_message(status_message) diff --git a/lib/diaspora/federation/mappings.rb b/lib/diaspora/federation/mappings.rb index 46e0ff969..0d19af545 100644 --- a/lib/diaspora/federation/mappings.rb +++ b/lib/diaspora/federation/mappings.rb @@ -50,6 +50,7 @@ module Diaspora Profile => :profile, Reshare => :reshare, Retraction => :retraction, + ContactRetraction => :retraction, StatusMessage => :status_message }.freeze From be8a1dfef42f7ba6e02cd9469947cde2ff005a49 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Sun, 4 Jun 2017 18:36:45 +0200 Subject: [PATCH 23/24] Add tests for ContactRetraction --- .../federation/attack_vectors_spec.rb | 2 +- .../federated/contact_retraction_spec.rb | 66 +++++++++++++++++++ .../lib/diaspora/federated/retraction_spec.rb | 54 ++++++++------- spec/lib/diaspora/federation/entities_spec.rb | 24 +------ 4 files changed, 99 insertions(+), 47 deletions(-) create mode 100644 spec/lib/diaspora/federated/contact_retraction_spec.rb diff --git a/spec/integration/federation/attack_vectors_spec.rb b/spec/integration/federation/attack_vectors_spec.rb index 25fdaecb1..b7dcbbe5b 100644 --- a/spec/integration/federation/attack_vectors_spec.rb +++ b/spec/integration/federation/attack_vectors_spec.rb @@ -63,7 +63,7 @@ describe "attack vectors", type: :request do expect(contact).to be_sharing post_message( - generate_payload(Diaspora::Federation::Entities.retraction(Retraction.for(contact)), alice, bob), bob + generate_payload(Diaspora::Federation::Entities.retraction(ContactRetraction.for(contact)), alice, bob), bob ) expect(bob.contacts(true).find_by(person_id: eve.person.id)).to be_sharing diff --git a/spec/lib/diaspora/federated/contact_retraction_spec.rb b/spec/lib/diaspora/federated/contact_retraction_spec.rb new file mode 100644 index 000000000..bd5c7c3dd --- /dev/null +++ b/spec/lib/diaspora/federated/contact_retraction_spec.rb @@ -0,0 +1,66 @@ +describe ContactRetraction do + let(:contact) { FactoryGirl.build(:contact, sharing: true, receiving: true) } + let(:retraction) { ContactRetraction.for(contact) } + + describe "#subscribers" do + it "contains the contact person" do + expect(retraction.subscribers).to eq([contact.person]) + end + end + + describe "#data" do + it "contains the hash with all data from the federation-retraction" do + federation_retraction_data = Diaspora::Federation::Entities.contact(contact).to_h + federation_retraction_data[:sharing] = false + federation_retraction_data[:following] = false + + expect(retraction.data).to eq(federation_retraction_data) + end + end + + describe ".retraction_data_for" do + it "creates a retraction for a contact" do + contact = FactoryGirl.build(:contact, sharing: false, receiving: false) + expect(Diaspora::Federation::Entities).to receive(:contact).with(contact).and_call_original + data = ContactRetraction.retraction_data_for(contact) + + expect(data[:author]).to eq(contact.user.diaspora_handle) + expect(data[:recipient]).to eq(contact.person.diaspora_handle) + expect(data[:sharing]).to be_falsey + expect(data[:following]).to be_falsey + end + end + + describe ".for" do + it "creates a retraction for a contact" do + expect(ContactRetraction).to receive(:retraction_data_for).with(contact) + ContactRetraction.for(contact) + end + + it "create contact entity with 'sharing' and 'following' set to false" do + data = ContactRetraction.for(contact).data + expect(data[:sharing]).to be_falsey + expect(data[:following]).to be_falsey + end + end + + describe ".defer_dispatch" do + it "queues a job to send the retraction later" do + contact = FactoryGirl.build(:contact, user: local_luke, person: remote_raphael) + retraction = ContactRetraction.for(contact) + federation_retraction_data = Diaspora::Federation::Entities.contact(contact).to_h + + expect(Workers::DeferredRetraction).to receive(:perform_async).with( + local_luke.id, "ContactRetraction", federation_retraction_data, [remote_raphael.id], {} + ) + + retraction.defer_dispatch(local_luke) + end + end + + describe "#public?" do + it "returns false for a contact retraction" do + expect(retraction.public?).to be_falsey + end + end +end diff --git a/spec/lib/diaspora/federated/retraction_spec.rb b/spec/lib/diaspora/federated/retraction_spec.rb index 864b03203..565f22387 100644 --- a/spec/lib/diaspora/federated/retraction_spec.rb +++ b/spec/lib/diaspora/federated/retraction_spec.rb @@ -18,15 +18,36 @@ describe Retraction do describe "#data" do it "contains the hash with all data from the federation-retraction" do - federation_retraction_data = Diaspora::Federation::Entities.retraction_data_for(post) + expect(retraction.data[:target_guid]).to eq(post.guid) + expect(retraction.data[:target]).to eq(Diaspora::Federation::Entities.related_entity(post).to_h) + expect(retraction.data[:target_type]).to eq("Post") + expect(retraction.data[:author]).to eq(alice.diaspora_handle) + end + end - expect(retraction.data).to eq(federation_retraction_data) + describe ".retraction_data_for" do + it "creates the retraction data for a post" do + data = Retraction.retraction_data_for(post) + expect(data[:target_guid]).to eq(post.guid) + expect(data[:target]).to eq(Diaspora::Federation::Entities.related_entity(post).to_h) + expect(data[:target_type]).to eq("Post") + expect(data[:author]).to eq(alice.diaspora_handle) + end + + it "creates the retraction data for a relayable" do + comment = FactoryGirl.create(:comment, author: alice.person, post: post) + + data = Retraction.retraction_data_for(comment) + expect(data[:target_guid]).to eq(comment.guid) + expect(data[:target]).to eq(Diaspora::Federation::Entities.related_entity(comment).to_h) + expect(data[:target_type]).to eq("Comment") + expect(data[:author]).to eq(alice.diaspora_handle) end end describe ".for" do it "creates a retraction for a post" do - expect(Diaspora::Federation::Entities).to receive(:retraction_data_for).with(post) + expect(Retraction).to receive(:retraction_data_for).with(post) Retraction.for(post) end @@ -34,18 +55,10 @@ describe Retraction do it "creates a retraction for a relayable" do comment = FactoryGirl.create(:comment, author: alice.person, post: post) - expect(Diaspora::Federation::Entities).to receive(:retraction_data_for).with(comment) + expect(Retraction).to receive(:retraction_data_for).with(comment) Retraction.for(comment) end - - it "creates a retraction for a contact" do - contact = FactoryGirl.build(:contact, receiving: false) - - expect(Diaspora::Federation::Entities).to receive(:retraction_data_for).with(contact) - - Retraction.for(contact) - end end describe ".defer_dispatch" do @@ -55,7 +68,7 @@ describe Retraction do federation_retraction = Diaspora::Federation::Entities.retraction(retraction) expect(Workers::DeferredRetraction).to receive(:perform_async).with( - local_luke.id, federation_retraction.to_h, [remote_raphael.id], service_types: [] + local_luke.id, "Retraction", federation_retraction.to_h, [remote_raphael.id], service_types: [] ) retraction.defer_dispatch(local_luke) @@ -71,7 +84,7 @@ describe Retraction do federation_retraction = Diaspora::Federation::Entities.retraction(retraction) expect(Workers::DeferredRetraction).to receive(:perform_async).with( - alice.id, federation_retraction.to_h, [], service_types: ["Services::Twitter"], tweet_id: "123" + alice.id, "Retraction", federation_retraction.to_h, [], service_types: ["Services::Twitter"], tweet_id: "123" ) retraction.defer_dispatch(alice) @@ -82,7 +95,7 @@ describe Retraction do federation_retraction = Diaspora::Federation::Entities.retraction(retraction) expect(Workers::DeferredRetraction).to receive(:perform_async).with( - alice.id, federation_retraction.to_h, [], service_types: [] + alice.id, "Retraction", federation_retraction.to_h, [], service_types: [] ) retraction.defer_dispatch(alice) @@ -95,7 +108,7 @@ describe Retraction do federation_retraction = Diaspora::Federation::Entities.retraction(retraction) expect(Workers::DeferredRetraction).to receive(:perform_async).with( - local_luke.id, federation_retraction.to_h, [remote_raphael.id], {} + local_luke.id, "Retraction", federation_retraction.to_h, [remote_raphael.id], {} ) retraction.defer_dispatch(local_luke) @@ -110,7 +123,7 @@ describe Retraction do federation_retraction = Diaspora::Federation::Entities.retraction(retraction) expect(Workers::DeferredRetraction).to receive(:perform_async).with( - local_luke.id, federation_retraction.to_h, [remote_raphael.id], {} + local_luke.id, "Retraction", federation_retraction.to_h, [remote_raphael.id], {} ) retraction.defer_dispatch(local_luke) @@ -121,7 +134,7 @@ describe Retraction do federation_retraction = Diaspora::Federation::Entities.retraction(retraction) expect(Workers::DeferredRetraction).to receive(:perform_async).with( - local_luke.id, federation_retraction.to_h, [], {} + local_luke.id, "Retraction", federation_retraction.to_h, [], {} ) retraction.defer_dispatch(local_luke, false) @@ -156,10 +169,5 @@ describe Retraction do private_post = alice.post(:status_message, text: "destroy!", to: alice.aspects.first.id) expect(Retraction.for(private_post).public?).to be_falsey end - - it "returns false for a contact retraction" do - contact = FactoryGirl.create(:contact, receiving: false) - expect(Retraction.for(contact).public?).to be_falsey - end end end diff --git a/spec/lib/diaspora/federation/entities_spec.rb b/spec/lib/diaspora/federation/entities_spec.rb index c94f023e1..3a1e680f4 100644 --- a/spec/lib/diaspora/federation/entities_spec.rb +++ b/spec/lib/diaspora/federation/entities_spec.rb @@ -218,7 +218,7 @@ describe Diaspora::Federation::Entities do it "builds a Contact for a Contact retraction" do target = FactoryGirl.create(:contact, receiving: false) - retraction = Retraction.for(target) + retraction = ContactRetraction.for(target) federation_entity = described_class.build(retraction) expect(federation_entity).to be_instance_of(DiasporaFederation::Entities::Contact) @@ -297,26 +297,4 @@ describe Diaspora::Federation::Entities do end end end - - describe ".retraction_data_for" do - it "returns the data for a Photo retraction" do - target = FactoryGirl.create(:photo, author: alice.person) - retraction_data = described_class.retraction_data_for(target) - - expect(retraction_data[:author]).to eq(target.author.diaspora_handle) - expect(retraction_data[:target_guid]).to eq(target.guid) - expect(retraction_data[:target_type]).to eq("Photo") - end - - it "returns the data for a Contact entity" do - target = FactoryGirl.create(:contact, receiving: false) - retraction_data = described_class.retraction_data_for(target) - - expect(retraction_data[:author]).to eq(target.user.diaspora_handle) - expect(retraction_data[:recipient]).to eq(target.person.diaspora_handle) - expect(retraction_data[:sharing]).to be_falsey - expect(retraction_data[:following]).to be_falsey - expect(retraction_data[:target_type]).to eq("Contact") - end - end end From a931bee319046c5547e9a55d0aa267aac35fec5f Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Mon, 3 Jul 2017 00:01:08 +0200 Subject: [PATCH 24/24] Make Participation entity non-relayable --- lib/diaspora/federation/entities.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/diaspora/federation/entities.rb b/lib/diaspora/federation/entities.rb index dd1da2fb8..2e18a57b6 100644 --- a/lib/diaspora/federation/entities.rb +++ b/lib/diaspora/federation/entities.rb @@ -97,8 +97,7 @@ module Diaspora author: participation.diaspora_handle, guid: participation.guid, parent_guid: participation.target.guid, - parent_type: Mappings.entity_name_for(participation.target), - parent: related_entity(participation.target) + parent_type: Mappings.entity_name_for(participation.target) ) end