fetch public key for unknown person

and do some refactorings

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?

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
 

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
 

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