Make Message entity non-relayable

Related to #36

lib/diaspora_federation/entities/message.rb

@@ -4,11 +4,17 @@ module DiasporaFederation
     #
     # @see Validators::MessageValidator
     class Message < Entity
-      # Old signature order
+      # @!attribute [r] author
-      # @deprecated
+      #   The diaspora* ID of the author
-      LEGACY_SIGNATURE_ORDER = %i(guid parent_guid text created_at author conversation_guid).freeze
+      #   @see Person#author
+      #   @return [String] diaspora* ID
+      property :author, :string, xml_name: :diaspora_handle
 
-      include Relayable
+      # @!attribute [r] guid
+      #   A random string of at least 16 chars
+      #   @see Validation::Rule::Guid
+      #   @return [String] guid
+      property :guid, :string
 
       # @!attribute [r] text
       #   Text of the message composed by a user
@@ -26,55 +32,9 @@ module DiasporaFederation
       #   @return [String] conversation guid
       property :conversation_guid, :string
 
-      # It is only valid to receive a {Message} from the author itself,
+      # @return [String] string representation of this object
-      # or from the author of the parent {Conversation} if the author signature is valid.
+      def to_s
-      # @deprecated remove after {Message} doesn't include {Relayable} anymore
+        "#{super}:#{conversation_guid}"
-      def sender_valid?(sender)
-        sender == author || (sender == parent_author && verify_author_signature)
-      end
-
-      # @deprecated remove after {Message} doesn't include {Relayable} anymore
-      def to_h
-        super.tap {|hash| hash[:created_at] = created_at.utc.iso8601 }
-      end
-
-      # @deprecated remove after {Message} doesn't include {Relayable} anymore
-      private_class_method def self.xml_parser_class
-        DiasporaFederation::Parsers::XmlParser
-      end
-
-      # Default implementation, don't verify signatures for a {Message}.
-      # @see Entity.from_hash
-      # @deprecated remove after {Message} doesn't include {Relayable} anymore
-      # @param [Hash] properties hash
-      # @return [Entity] instance
-      def self.from_hash(hash)
-        new({parent_guid: nil, parent: nil}.merge(hash))
-      end
-
-      private
-
-      # @deprecated remove after {Message} doesn't include {Relayable} anymore
-      def verify_author_signature
-        verify_signature(author, :author_signature)
-        true
-      end
-
-      # @deprecated remove after {Message} doesn't include {Relayable} anymore
-      def parent_author
-        parent = DiasporaFederation.callbacks.trigger(:fetch_related_entity, "Conversation", conversation_guid)
-        raise Federation::Fetcher::NotFetchable, "parent of #{self} not found" unless parent
-        parent.author
-      end
-
-      # old timestamp format, because this signature is only used from old pods which also relay with old format
-      # @deprecated remove after {Message} doesn't include {Relayable} anymore
-      def normalize_property(name, value)
-        if name == :created_at
-          value.to_s
-        else
-          super
-        end
       end
     end
   end

lib/diaspora_federation/test/factories.rb

@@ -142,7 +142,7 @@ module DiasporaFederation
         participants { Array.new(3) { Fabricate.sequence(:diaspora_id) }.join(";") }
       end
 
-      Fabricator(:message_entity, class_name: DiasporaFederation::Entities::Message, from: :relayable_entity) do
+      Fabricator(:message_entity, class_name: DiasporaFederation::Entities::Message) do
         guid { Fabricate.sequence(:guid) }
         author { Fabricate.sequence(:diaspora_id) }
         text "this is a very informative text"

spec/lib/diaspora_federation/entities/message_spec.rb

@@ -1,108 +1,21 @@
 module DiasporaFederation
   describe Entities::Message do
-    let(:parent) { Fabricate(:conversation, author: bob) }
+    let(:data) { Fabricate.attributes_for(:message_entity, author: alice.diaspora_id) }
-    let(:parent_entity) { Fabricate(:related_entity, author: bob.diaspora_id) }
-    let(:data) {
-      Fabricate
-        .attributes_for(:message_entity, author: alice.diaspora_id, parent_guid: parent.guid, parent: parent_entity)
-        .tap {|hash| add_signatures(hash) }
-    }
 
     let(:xml) { <<-XML }
 <message>
-  <guid>#{data[:guid]}</guid>
-  <parent_guid>#{parent.guid}</parent_guid>
-  <text>#{data[:text]}</text>
-  <created_at>#{data[:created_at]}</created_at>
   <diaspora_handle>#{data[:author]}</diaspora_handle>
+  <guid>#{data[:guid]}</guid>
+  <text>#{data[:text]}</text>
+  <created_at>#{data[:created_at].utc.iso8601}</created_at>
   <conversation_guid>#{data[:conversation_guid]}</conversation_guid>
-  <author_signature>#{data[:author_signature]}</author_signature>
-  <parent_author_signature>#{data[:parent_author_signature]}</parent_author_signature>
 </message>
 XML
 
-    let(:string) { "Message:#{data[:guid]}:#{parent.guid}" }
+    let(:string) { "Message:#{data[:guid]}:#{data[:conversation_guid]}" }
 
     it_behaves_like "an Entity subclass"
 
-    it_behaves_like "an XML Entity", %i(parent parent_guid)
+    it_behaves_like "an XML Entity"
-
-    it_behaves_like "a relayable Entity"
-
-    describe "#sender_valid?" do
-      let(:entity) { Entities::Message.new(data) }
-
-      it "allows the author" do
-        expect(entity.sender_valid?(alice.diaspora_id)).to be_truthy
-      end
-
-      it "allows parent author if the signature is valid" do
-        expect_callback(:fetch_related_entity, "Conversation", entity.conversation_guid).and_return(parent_entity)
-        expect_callback(:fetch_public_key, alice.diaspora_id).and_return(alice.private_key)
-        expect(entity.sender_valid?(bob.diaspora_id)).to be_truthy
-      end
-
-      it "does not allow any other person" do
-        expect_callback(:fetch_related_entity, "Conversation", entity.conversation_guid).and_return(parent_entity)
-        invalid_sender = Fabricate.sequence(:diaspora_id)
-        expect(entity.sender_valid?(invalid_sender)).to be_falsey
-      end
-
-      it "does not allow the parent author if the signature is invalid" do
-        expect_callback(:fetch_related_entity, "Conversation", entity.conversation_guid).and_return(parent_entity)
-        expect_callback(:fetch_public_key, alice.diaspora_id).and_return(alice.private_key)
-        invalid_msg = Entities::Message.new(data.merge(author_signature: "aa"))
-        expect {
-          invalid_msg.sender_valid?(bob.diaspora_id)
-        }.to raise_error Entities::Relayable::SignatureVerificationFailed, "wrong author_signature for #{invalid_msg}"
-      end
-
-      it "raises NotFetchable if the parent Conversation can not be found" do
-        expect_callback(:fetch_related_entity, "Conversation", entity.conversation_guid).and_return(nil)
-        expect {
-          entity.sender_valid?(bob.diaspora_id)
-        }.to raise_error Federation::Fetcher::NotFetchable
-      end
-    end
-
-    context "relayable signature verification" do
-      it "does not verify the signature" do
-        data.merge!(author_signature: "aa", parent_author_signature: "bb")
-        xml = Entities::Message.new(data).to_xml
-
-        expect {
-          Entities::Message.from_xml(xml)
-        }.not_to raise_error
-      end
-    end
-
-    describe ".from_xml" do
-      it "adds a nil parent" do
-        xml = Entities::Message.new(data).to_xml
-        parsed = Entities::Message.from_xml(xml)
-        expect(parsed.parent).to be_nil
-      end
-
-      it "uses the parent_guid from the parsed xml" do
-        xml = Entities::Message.new(data).to_xml
-        parsed = Entities::Message.from_xml(xml)
-        expect(parsed.parent_guid).to eq(data[:parent_guid])
-      end
-
-      it "uses nil for parent_guid if not in the xml" do
-        xml = <<-XML
-<message>
-  <author>#{data[:author]}</author>
-  <guid>#{data[:guid]}</guid>
-  <text>#{data[:text]}</text>
-  <created_at>#{data[:created_at]}</created_at>
-  <conversation_guid>#{data[:conversation_guid]}</conversation_guid>
-</message>
-XML
-
-        parsed = Entities::Message.from_xml(Nokogiri::XML::Document.parse(xml).root)
-        expect(parsed.parent_guid).to be_nil
-      end
-    end
   end
 end