diff --git a/app/models/person.rb b/app/models/person.rb index 4182c7a4b..45ba1f5fe 100644 --- a/app/models/person.rb +++ b/app/models/person.rb @@ -236,30 +236,20 @@ class Person < ActiveRecord::Base end # discovery (webfinger) - def self.find_or_fetch_by_identifier(account) + def self.find_or_fetch_by_identifier(diaspora_id) # exiting person? - person = by_account_identifier(account) + person = by_account_identifier(diaspora_id) return person if person.present? && person.profile.present? # create or update person from webfinger - logger.info "webfingering #{account}, it is not known or needs updating" - DiasporaFederation::Discovery::Discovery.new(account).fetch_and_save + logger.info "webfingering #{diaspora_id}, it is not known or needs updating" + DiasporaFederation::Discovery::Discovery.new(diaspora_id).fetch_and_save - by_account_identifier(account) + by_account_identifier(diaspora_id) end - # database calls - def self.by_account_identifier(identifier) - identifier = identifier.strip.downcase.sub("acct:", "") - find_by(diaspora_handle: identifier) - end - - def self.find_local_by_diaspora_handle(handle) - where(diaspora_handle: handle, closed_account: false).where.not(owner: nil).take - end - - def self.find_local_by_guid(guid) - where(guid: guid, closed_account: false).where.not(owner: nil).take + def self.by_account_identifier(diaspora_id) + find_by(diaspora_handle: diaspora_id.strip.downcase) end def remote? diff --git a/config/initializers/diaspora_federation.rb b/config/initializers/diaspora_federation.rb index adf29e600..c4a8ce274 100644 --- a/config/initializers/diaspora_federation.rb +++ b/config/initializers/diaspora_federation.rb @@ -6,8 +6,8 @@ DiasporaFederation.configure do |config| config.certificate_authorities = AppConfig.environment.certificate_authorities.get config.define_callbacks do - on :fetch_person_for_webfinger do |handle| - person = Person.find_local_by_diaspora_handle(handle) + on :fetch_person_for_webfinger do |diaspora_id| + 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}", @@ -25,7 +25,7 @@ DiasporaFederation.configure do |config| end on :fetch_person_for_hcard do |guid| - person = Person.find_local_by_guid(guid) + person = Person.where(guid: guid, closed_account: false).where.not(owner: nil).take if person DiasporaFederation::Discovery::HCard.new( guid: person.guid, @@ -74,7 +74,7 @@ DiasporaFederation.configure do |config| end on :fetch_public_key_by_diaspora_id do |diaspora_id| - key = Person.where(diaspora_handle: diaspora_id).pluck(:serialized_public_key).first + key = Person.find_or_fetch_by_identifier(diaspora_id).serialized_public_key OpenSSL::PKey::RSA.new key unless key.nil? end diff --git a/spec/federation_callbacks_spec.rb b/spec/federation_callbacks_spec.rb index 0de40cf49..9037e2a3f 100644 --- a/spec/federation_callbacks_spec.rb +++ b/spec/federation_callbacks_spec.rb @@ -22,6 +22,19 @@ describe "diaspora federation callbacks" do wf = DiasporaFederation.callbacks.trigger(:fetch_person_for_webfinger, "unknown@example.com") expect(wf).to be_nil end + + it "returns nil for a remote person" do + person = FactoryGirl.create(:person) + wf = DiasporaFederation.callbacks.trigger(:fetch_person_for_webfinger, person.diaspora_handle) + expect(wf).to be_nil + end + + it "returns nil for a closed account" do + user = FactoryGirl.create(:user) + user.person.lock_access! + wf = DiasporaFederation.callbacks.trigger(:fetch_person_for_webfinger, user.diaspora_handle) + expect(wf).to be_nil + end end describe ":fetch_person_for_hcard" do @@ -54,6 +67,19 @@ describe "diaspora federation callbacks" do hcard = DiasporaFederation.callbacks.trigger(:fetch_person_for_hcard, "1234567890abcdef") expect(hcard).to be_nil end + + it "returns nil for a remote person" do + person = FactoryGirl.create(:person) + hcard = DiasporaFederation.callbacks.trigger(:fetch_person_for_hcard, person.guid) + expect(hcard).to be_nil + end + + it "returns nil for a closed account" do + user = FactoryGirl.create(:user) + user.person.lock_access! + hcard = DiasporaFederation.callbacks.trigger(:fetch_person_for_hcard, user.guid) + expect(hcard).to be_nil + end end describe ":save_person_after_webfinger" do @@ -205,10 +231,23 @@ describe "diaspora federation callbacks" do expect(key.to_s).to eq(remote_person.serialized_public_key) end + it "fetches an unknown user" do + person = FactoryGirl.build(:person) + expect(Person).to receive(:find_or_fetch_by_identifier).with(person.diaspora_handle).and_return(person) + + key = DiasporaFederation.callbacks.trigger(:fetch_public_key_by_diaspora_id, person.diaspora_handle) + expect(key).to be_a(OpenSSL::PKey::RSA) + expect(key.to_s).to eq(person.serialized_public_key) + end + it "returns nil for an unknown person" do - expect( - DiasporaFederation.callbacks.trigger(:fetch_public_key_by_diaspora_id, FactoryGirl.generate(:diaspora_id)) - ).to be_nil + diaspora_id = FactoryGirl.generate(:diaspora_id) + expect(Person).to receive(:find_or_fetch_by_identifier).with(diaspora_id) + .and_raise(DiasporaFederation::Discovery::DiscoveryError) + + expect { + DiasporaFederation.callbacks.trigger(:fetch_public_key_by_diaspora_id, diaspora_id) + }.to raise_error DiasporaFederation::Discovery::DiscoveryError end end diff --git a/spec/models/person_spec.rb b/spec/models/person_spec.rb index 54d9ec223..9215d699c 100644 --- a/spec/models/person_spec.rb +++ b/spec/models/person_spec.rb @@ -460,42 +460,6 @@ describe Person, :type => :model do expect(f).to be nil end end - - describe ".find_local_by_diaspora_handle" do - it "should find local users person" do - person = Person.find_local_by_diaspora_handle(user.diaspora_handle) - expect(person).to eq(user.person) - end - - it "should not find a remote person" do - person = Person.find_local_by_diaspora_handle(@person.diaspora_handle) - expect(person).to be nil - end - - it "should not find a person with closed account" do - user.person.lock_access! - person = Person.find_local_by_diaspora_handle(user.diaspora_handle) - expect(person).to be nil - end - end - - describe ".find_local_by_guid" do - it "should find local users person" do - person = Person.find_local_by_guid(user.guid) - expect(person).to eq(user.person) - end - - it "should not find a remote person" do - person = Person.find_local_by_guid(@person.guid) - expect(person).to be nil - end - - it "should not find a person with closed account" do - user.person.lock_access! - person = Person.find_local_by_guid(user.guid) - expect(person).to be nil - end - end end describe '#has_photos?' do