From 7e2321d6c92f00cf20140375d4f85d94dfed68ec Mon Sep 17 00:00:00 2001 From: cmrd Senya Date: Mon, 30 Oct 2017 19:52:54 +0200 Subject: [PATCH] Introduce alternative form of the account migration message In the alternative form author can be the new diaspora user --- docs/_entities/account_migration.md | 14 +- .../entities/account_migration.rb | 43 ++- lib/diaspora_federation/test/factories.rb | 1 + .../validators/account_migration_validator.rb | 2 + .../entities/account_migration_spec.rb | 259 ++++++++++++------ .../account_migration_validator_spec.rb | 8 +- 6 files changed, 231 insertions(+), 96 deletions(-) diff --git a/docs/_entities/account_migration.md b/docs/_entities/account_migration.md index 072355a..a418c4c 100644 --- a/docs/_entities/account_migration.md +++ b/docs/_entities/account_migration.md @@ -8,9 +8,16 @@ This entity is sent when a person changes their diaspora* ID (e.g. when a user m | Property | Type | Description | | ----------- | ---------------------------- | ------------------------------------------------------------------------------------ | -| `author` | [diaspora\* ID][diaspora-id] | The diaspora\* ID of the closed account. | -| `person` | [Profile][profile] | New profile of a person | -| `signature` | [Signature][signature] | Signature that validates original and target diaspora* IDs with the new key of person | +| `author` | [diaspora\* ID][diaspora-id] | The diaspora\* ID of the sender of the entity. The entity may be sent by either old user identity or new user identity. | +| `person` | [Profile][profile] | New profile of a person. | +| `signature` | [Signature][signature] | Signature that validates original and target diaspora* IDs with the private key of the second identity, other than the entity author. So if the author is the old identity then this signature is made with the new identity key, and vice versa. | + +## Optional Properties + +| Property | Type | Description | +| ----------- | ---------------------------- | ------------------------------------------------------------------------------------ | +| `old_identity` | [diaspora\* ID][diaspora-id] | The diaspora\* ID of the closed account. This field is mandatory if the author of the entity is the new identity. | + ### Signature @@ -51,6 +58,7 @@ AccountMigration:old-diaspora-id@example.org:new-diaspora-id@example.com 07b1OIY6sTUQwV5pbpgFK0uz6W4cu+oQnlg410Q4uISUOdNOlBdYqhZJm62VFhgvzt4TZXfiJgoupFkRjP0BsaVaZuP2zKMNvO3ngWOeJRf2oRK4Ub5cEA/g7yijkRc+7y8r1iLJ31MFb1czyeCsLxw9Ol8SvAJddogGiLHDhjE= + alice@example.org ~~~ diff --git a/lib/diaspora_federation/entities/account_migration.rb b/lib/diaspora_federation/entities/account_migration.rb index 6fd8d07..89e19c0 100644 --- a/lib/diaspora_federation/entities/account_migration.rb +++ b/lib/diaspora_federation/entities/account_migration.rb @@ -8,7 +8,9 @@ module DiasporaFederation include AccountMigration::Signable # @!attribute [r] author - # The old diaspora* ID of the person who changes their ID + # Sender of the AccountMigration message. Usually it is the old diaspora* ID of the person who changes their ID. + # This property is also allowed to be the new diaspora* ID, which is equal to the author of the included + # profile. # @see Person#author # @return [String] author diaspora* ID property :author, :string @@ -19,14 +21,31 @@ module DiasporaFederation entity :profile, Entities::Profile # @!attribute [r] signature - # Signature that validates original and target diaspora* IDs with the new key of person + # Signature that validates original and target diaspora* IDs with the private key of the second identity, other + # than the entity author. So if the author is the old identity then this signature is made with the new identity + # key, and vice versa. # @return [String] signature property :signature, :string, default: nil - def old_user_id + # @!attribute [r] old_identity + # Optional attribute which keeps old diaspora* ID. Must be present when author attribute contains new diaspora* + # ID. + # @return [String] old identity + property :old_identity, :string, default: nil + + # Returns diaspora* ID of the old person identity. + # @return [String] diaspora* ID of the old person identity + def old_identity + return @old_identity if author_is_new_id? author end + # Returns diaspora* ID of the new person identity. + # @return [String] diaspora* ID of the new person identity + def new_identity + profile.author + end + # @return [String] string representation of this object alias to_s unique_migration_descriptor @@ -46,33 +65,33 @@ module DiasporaFederation private - def new_user_id - profile.author + def author_is_new_id? + author == new_identity end def signer_id - new_user_id + author_is_new_id? ? @old_identity : new_identity end def enriched_properties super.tap do |hash| - hash[:signature] = signature || sign_with_new_key + hash[:signature] = signature || sign_with_respective_key end end - # Sign with new user's key - # @raise [NewPrivateKeyNotFound] if the new user's private key is not found + # Sign with the key of the #signer_id identity + # @raise [PrivateKeyNotFound] if the signer's private key is not found # @return [String] A Base64 encoded signature of #signature_data with key - def sign_with_new_key + def sign_with_respective_key privkey = DiasporaFederation.callbacks.trigger(:fetch_private_key, signer_id) - raise NewPrivateKeyNotFound, "signer=#{signer_id} obj=#{self}" if privkey.nil? + raise PrivateKeyNotFound, "signer=#{signer_id} obj=#{self}" if privkey.nil? sign_with_key(privkey).tap do logger.info "event=sign status=complete signature=signature signer=#{signer_id} obj=#{self}" end end # Raised, if creating the signature fails, because the new private key of a user was not found - class NewPrivateKeyNotFound < RuntimeError + class PrivateKeyNotFound < RuntimeError end end end diff --git a/lib/diaspora_federation/test/factories.rb b/lib/diaspora_federation/test/factories.rb index 9c4d83f..ebca70d 100644 --- a/lib/diaspora_federation/test/factories.rb +++ b/lib/diaspora_federation/test/factories.rb @@ -42,6 +42,7 @@ module DiasporaFederation Fabricator(:account_migration_entity, class_name: DiasporaFederation::Entities::AccountMigration) do author { Fabricate.sequence(:diaspora_id) } profile { Fabricate(:profile_entity) } + old_identity { Fabricate.sequence(:diaspora_id) } end Fabricator(:person_entity, class_name: DiasporaFederation::Entities::Person) do diff --git a/lib/diaspora_federation/validators/account_migration_validator.rb b/lib/diaspora_federation/validators/account_migration_validator.rb index 1c11b25..36c38be 100644 --- a/lib/diaspora_federation/validators/account_migration_validator.rb +++ b/lib/diaspora_federation/validators/account_migration_validator.rb @@ -7,6 +7,8 @@ module DiasporaFederation rule :author, :diaspora_id rule :profile, :not_nil + + rule :old_identity, :diaspora_id end end end diff --git a/spec/lib/diaspora_federation/entities/account_migration_spec.rb b/spec/lib/diaspora_federation/entities/account_migration_spec.rb index debc3d0..97d08b2 100644 --- a/spec/lib/diaspora_federation/entities/account_migration_spec.rb +++ b/spec/lib/diaspora_federation/entities/account_migration_spec.rb @@ -1,21 +1,185 @@ module DiasporaFederation describe Entities::AccountMigration do - let(:new_diaspora_id) { alice.diaspora_id } - let(:new_author_pkey) { alice.private_key } - let(:hash) { - Fabricate.attributes_for(:account_deletion_entity).merge( - profile: Fabricate(:profile_entity, author: new_diaspora_id) - ) - } + let(:old_user) { Fabricate(:user) } + let(:new_user) { Fabricate(:user) } + let(:old_diaspora_id) { old_user.diaspora_id } + let(:new_diaspora_id) { new_user.diaspora_id } + let(:data) { hash.tap {|hash| properties = described_class.new(hash).send(:enriched_properties) hash[:signature] = properties[:signature] } } - let(:signature_data) { "AccountMigration:#{hash[:author]}:#{new_diaspora_id}" } + let(:signature_data) { "AccountMigration:#{old_diaspora_id}:#{new_diaspora_id}" } + let(:string) { signature_data } - let(:xml) { <<-XML } + shared_examples_for "an account migration entity" do + it_behaves_like "an Entity subclass" + + it_behaves_like "an XML Entity" + + describe "#to_xml" do + it "computes signature when no signature was provided" do + expect_callback(:fetch_private_key, signer_id).and_return(signer_pkey) + + entity = Entities::AccountMigration.new(hash) + xml = entity.to_xml + + signature = xml.at_xpath("signature").text + expect(verify_signature(signer_pkey, signature, entity.to_s)).to be_truthy + end + + it "doesn't change signature if it is already set" do + hash[:signature] = "aa" + + xml = Entities::AccountMigration.new(hash).to_xml + + expect(xml.at_xpath("signature").text).to eq("aa") + end + + it "raises when signature isn't set and key isn't supplied" do + expect_callback(:fetch_private_key, signer_id).and_return(nil) + + expect { + Entities::AccountMigration.new(hash).to_xml + }.to raise_error Entities::AccountMigration::PrivateKeyNotFound + end + end + + describe "#verify_signature" do + it "doesn't raise anything if correct signature was passed" do + hash[:signature] = sign_with_key(signer_pkey, signature_data) + expect_callback(:fetch_public_key, signer_id).and_return(signer_pkey) + expect { Entities::AccountMigration.new(hash).verify_signature }.not_to raise_error + end + + it "raises when no public key for author was fetched" do + expect_callback(:fetch_public_key, anything).and_return(nil) + + expect { + Entities::AccountMigration.new(hash).verify_signature + }.to raise_error Entities::AccountMigration::PublicKeyNotFound + end + + it "raises when bad author signature was passed" do + hash[:signature] = "abcdef" + + expect_callback(:fetch_public_key, signer_id).and_return(signer_pkey.public_key) + + expect { + Entities::AccountMigration.new(hash).verify_signature + }.to raise_error Entities::AccountMigration::SignatureVerificationFailed + end + end + + describe ".from_hash" do + it "calls #verify_signature" do + expect_any_instance_of(Entities::AccountMigration).to receive(:freeze) + expect_any_instance_of(Entities::AccountMigration).to receive(:verify_signature) + Entities::AccountMigration.from_hash(hash) + end + + it "raises when bad author signature was passed" do + hash[:signature] = "abcdef" + + expect_callback(:fetch_public_key, signer_id).and_return(signer_pkey.public_key) + + expect { + Entities::AccountMigration.from_hash(hash) + }.to raise_error Entities::AccountMigration::SignatureVerificationFailed + end + end + end + + context "with old identity as author" do + let(:signer_id) { new_diaspora_id } + let(:signer_pkey) { new_user.private_key } + + let(:hash) { + { + author: old_diaspora_id, + profile: Fabricate(:profile_entity, author: new_diaspora_id), + old_identity: old_diaspora_id + } + } + + let(:xml) { <<-XML } + + #{data[:author]} + + #{data[:profile].author} + #{data[:profile].first_name} + #{data[:profile].image_url} + #{data[:profile].image_url} + #{data[:profile].image_url} + #{data[:profile].bio} + #{data[:profile].birthday} + #{data[:profile].gender} + #{data[:profile].location} + #{data[:profile].searchable} + #{data[:profile].public} + #{data[:profile].nsfw} + #{data[:profile].tag_string} + + #{data[:signature]} + #{data[:old_identity]} + +XML + + it_behaves_like "an account migration entity" + end + + context "with new identity as author" do + let(:signer_id) { old_diaspora_id } + let(:signer_pkey) { old_user.private_key } + + let(:hash) { + { + author: new_diaspora_id, + profile: Fabricate(:profile_entity, author: new_diaspora_id), + old_identity: old_diaspora_id + } + } + + let(:xml) { <<-XML } + + #{data[:author]} + + #{data[:profile].author} + #{data[:profile].first_name} + #{data[:profile].image_url} + #{data[:profile].image_url} + #{data[:profile].image_url} + #{data[:profile].bio} + #{data[:profile].birthday} + #{data[:profile].gender} + #{data[:profile].location} + #{data[:profile].searchable} + #{data[:profile].public} + #{data[:profile].nsfw} + #{data[:profile].tag_string} + + #{data[:signature]} + #{data[:old_identity]} + +XML + + it_behaves_like "an account migration entity" + end + + context "when author is the new identity and old_identity prop is missing" do + let(:signer_id) { old_diaspora_id } + let(:signer_pkey) { old_user.private_key } + + let(:hash) { + { + author: new_diaspora_id, + profile: Fabricate(:profile_entity, author: new_diaspora_id) + } + } + + let(:xml) { <<-XML } #{data[:author]} @@ -37,81 +201,16 @@ module DiasporaFederation XML - let(:string) { "AccountMigration:#{data[:author]}:#{data[:profile].author}" } - - it_behaves_like "an Entity subclass" - - it_behaves_like "an XML Entity" - - describe "#to_xml" do - it "computes signature when no signature was provided" do - expect_callback(:fetch_private_key, new_diaspora_id).and_return(new_author_pkey) - - entity = Entities::AccountMigration.new(hash) - xml = entity.to_xml - - signature = xml.at_xpath("signature").text - expect(verify_signature(new_author_pkey, signature, entity.to_s)).to be_truthy - end - - it "doesn't change signature if it is already set" do - hash[:signature] = "aa" - - xml = Entities::AccountMigration.new(hash).to_xml - - expect(xml.at_xpath("signature").text).to eq("aa") - end - - it "raises when signature isn't set and key isn't supplied" do - expect_callback(:fetch_private_key, new_diaspora_id).and_return(nil) - + it "fails validation on construction" do expect { - Entities::AccountMigration.new(hash).to_xml - }.to raise_error Entities::AccountMigration::NewPrivateKeyNotFound - end - end - - describe "#verify_signature" do - it "doesn't raise anything if correct signatures were passed" do - hash[:signature] = sign_with_key(new_author_pkey, signature_data) - expect_callback(:fetch_public_key, new_diaspora_id).and_return(new_author_pkey) - expect { Entities::AccountMigration.new(hash).verify_signature }.not_to raise_error + described_class.new(hash) + }.to raise_error Entity::ValidationError end - it "raises when no public key for author was fetched" do - expect_callback(:fetch_public_key, anything).and_return(nil) - + it "fails validation on parsing" do expect { - Entities::AccountMigration.new(hash).verify_signature - }.to raise_error Entities::AccountMigration::PublicKeyNotFound - end - - it "raises when bad author signature was passed" do - hash[:signature] = "abcdef" - - expect_callback(:fetch_public_key, new_diaspora_id).and_return(new_author_pkey.public_key) - - expect { - Entities::AccountMigration.new(hash).verify_signature - }.to raise_error Entities::AccountMigration::SignatureVerificationFailed - end - end - - describe ".from_hash" do - it "calls #verify_signature" do - expect_any_instance_of(Entities::AccountMigration).to receive(:freeze) - expect_any_instance_of(Entities::AccountMigration).to receive(:verify_signature) - Entities::AccountMigration.from_hash(hash) - end - - it "raises when bad author signature was passed" do - hash[:signature] = "abcdef" - - expect_callback(:fetch_public_key, new_diaspora_id).and_return(new_author_pkey.public_key) - - expect { - Entities::AccountMigration.from_hash(hash) - }.to raise_error Entities::AccountMigration::SignatureVerificationFailed + DiasporaFederation::Salmon::XmlPayload.unpack(Nokogiri::XML(xml).root) + }.to raise_error Entity::ValidationError end end end diff --git a/spec/lib/diaspora_federation/validators/account_migration_validator_spec.rb b/spec/lib/diaspora_federation/validators/account_migration_validator_spec.rb index 7d95768..e8c4ea5 100644 --- a/spec/lib/diaspora_federation/validators/account_migration_validator_spec.rb +++ b/spec/lib/diaspora_federation/validators/account_migration_validator_spec.rb @@ -8,12 +8,18 @@ module DiasporaFederation let(:property) { :author } end - describe "#person" do + describe "#profile" do it_behaves_like "a property with a value validation/restriction" do let(:property) { :profile } let(:wrong_values) { [nil] } let(:correct_values) { [] } end end + + describe "#old_identity" do + it_behaves_like "a diaspora* ID validator" do + let(:property) { :old_identity } + end + end end end