Add HTTP signatures to all outgoing ActivityPub GET requests (mastodon#11284)

Author: Gargron

app/helpers/jsonld_helper.rb

@@ -77,19 +77,12 @@ def fetch_resource(uri, id, on_behalf_of = nil)
   end
 
   def fetch_resource_without_id_validation(uri, on_behalf_of = nil, raise_on_temporary_error = false)
-    build_request(uri, on_behalf_of).perform do |response|
-      raise Mastodon::UnexpectedResponseError, response unless response_successful?(response) || response_error_unsalvageable?(response) || !raise_on_temporary_error
-
-      return body_to_json(response.body_with_limit) if response.code == 200
-    end
-
-    # If request failed, retry without doing it on behalf of a user
-    return if on_behalf_of.nil?
+    on_behalf_of ||= Account.representative
 
-    build_request(uri).perform do |response|
+    build_request(uri, on_behalf_of).perform do |response|
       raise Mastodon::UnexpectedResponseError, response unless response_successful?(response) || response_error_unsalvageable?(response) || !raise_on_temporary_error
 
-      response.code == 200 ? body_to_json(response.body_with_limit) : nil
+      body_to_json(response.body_with_limit) if response.code == 200
     end
   end
 

app/lib/request.rb

@@ -40,8 +40,8 @@ def initialize(verb, url, **options)
     set_digest! if options.key?(:body)
   end
 
-  def on_behalf_of(account, key_id_format = :acct, sign_with: nil)
-    raise ArgumentError, 'account must be local' unless account&.local?
+  def on_behalf_of(account, key_id_format = :uri, sign_with: nil)
+    raise ArgumentError, 'account must not be nil' if account.nil?
 
     @account       = account
     @keypair       = sign_with.present? ? OpenSSL::PKey::RSA.new(sign_with) : @account.keypair

app/services/fetch_resource_service.rb

@@ -23,7 +23,7 @@ def process(url, terminal = false)
   end
 
   def perform_request(&block)
-    Request.new(:get, @url).add_headers('Accept' => ACCEPT_HEADER).perform(&block)
+    Request.new(:get, @url).add_headers('Accept' => ACCEPT_HEADER).on_behalf_of(Account.representative).perform(&block)
   end
 
   def process_response(response, terminal = false)

spec/controllers/concerns/signature_verification_spec.rb

@@ -38,7 +38,7 @@ def alternative_success
   end
 
   context 'with signature header' do
-    let!(:author) { Fabricate(:account) }
+    let!(:author) { Fabricate(:account, domain: 'example.com', uri: 'https://example.com/actor') }
 
     context 'without body' do
       before do

spec/services/fetch_remote_account_service_spec.rb

@@ -4,6 +4,7 @@
   let(:url) { 'https://example.com/alice' }
   let(:prefetched_body) { nil }
   let(:protocol) { :ostatus }
+  let!(:representative) { Fabricate(:account) }
 
   subject { FetchRemoteAccountService.new.call(url, prefetched_body, protocol) }
 

spec/services/fetch_resource_service_spec.rb

@@ -5,87 +5,96 @@
 
   describe '#call' do
     let(:url) { 'http://example.com' }
+
     subject { described_class.new.call(url) }
 
-    context 'url is blank' do
+    context 'with blank url' do
       let(:url) { '' }
       it { is_expected.to be_nil }
     end
 
-    context 'request failed' do
+    context 'when request fails' do
       before do
-        WebMock.stub_request(:get, url).to_return(status: 500, body: '', headers: {})
+        stub_request(:get, url).to_return(status: 500, body: '', headers: {})
       end
 
       it { is_expected.to be_nil }
     end
 
-    context 'raise OpenSSL::SSL::SSLError' do
+    context 'when OpenSSL::SSL::SSLError is raised' do
       before do
-        allow(Request).to receive_message_chain(:new, :add_headers, :perform).and_raise(OpenSSL::SSL::SSLError)
+        allow(Request).to receive_message_chain(:new, :add_headers, :on_behalf_of, :perform).and_raise(OpenSSL::SSL::SSLError)
       end
 
-      it 'return nil' do
-        is_expected.to be_nil
-      end
+      it { is_expected.to be_nil }
     end
 
-    context 'raise HTTP::ConnectionError' do
+    context 'when HTTP::ConnectionError is raised' do
       before do
-        allow(Request).to receive_message_chain(:new, :add_headers, :perform).and_raise(HTTP::ConnectionError)
+        allow(Request).to receive_message_chain(:new, :add_headers, :on_behalf_of, :perform).and_raise(HTTP::ConnectionError)
       end
 
-      it 'return nil' do
-        is_expected.to be_nil
-      end
+      it { is_expected.to be_nil }
     end
 
-    context 'response success' do
+    context 'when request succeeds' do
       let(:body) { '' }
-      let(:headers) { { 'Content-Type' => content_type } }
-      let(:json) {
-        { id: 1,
+
+      let(:content_type) { 'application/json' }
+
+      let(:headers) do
+        { 'Content-Type' => content_type }
+      end
+
+      let(:json) do
+        {
+          id: 1,
           '@context': ActivityPub::TagManager::CONTEXT,
           type: 'Note',
         }.to_json
-      }
+      end
 
       before do
-        WebMock.stub_request(:get, url).to_return(status: 200, body: body, headers: headers)
+        stub_request(:get, url).to_return(status: 200, body: body, headers: headers)
+      end
+
+      it 'signs request' do
+        subject
+        expect(a_request(:get, url).with(headers: { 'Signature' => /keyId="#{Regexp.escape(ActivityPub::TagManager.instance.uri_for(representative) + '#main-key')}"/ })).to have_been_made
       end
 
-      context 'content type is application/atom+xml' do
+      context 'when content type is application/atom+xml' do
         let(:content_type) { 'application/atom+xml' }
 
         it { is_expected.to eq nil }
       end
 
-      context 'content_type is activity+json' do
+      context 'when content type is activity+json' do
         let(:content_type) { 'application/activity+json; charset=utf-8' }
         let(:body) { json }
 
         it { is_expected.to eq [1, { prefetched_body: body, id: true }, :activitypub] }
       end
 
-      context 'content_type is ld+json with profile' do
+      context 'when content type is ld+json with profile' do
         let(:content_type) { 'application/ld+json; profile="https://www.w3.org/ns/activitystreams"' }
         let(:body) { json }
 
         it { is_expected.to eq [1, { prefetched_body: body, id: true }, :activitypub] }
       end
 
       before do
-        WebMock.stub_request(:get, url).to_return(status: 200, body: body, headers: headers)
-        WebMock.stub_request(:get, 'http://example.com/foo').to_return(status: 200, body: json, headers: { 'Content-Type' => 'application/activity+json' })
+        stub_request(:get, url).to_return(status: 200, body: body, headers: headers)
+        stub_request(:get, 'http://example.com/foo').to_return(status: 200, body: json, headers: { 'Content-Type' => 'application/activity+json' })
       end
 
-      context 'has link header' do
+      context 'when link header is present' do
         let(:headers) { { 'Link' => '<http://example.com/foo>; rel="alternate"; type="application/activity+json"', } }
 
         it { is_expected.to eq [1, { prefetched_body: json, id: true }, :activitypub] }
       end
 
-      context 'content type is text/html' do
+      context 'when content type is text/html' do
         let(:content_type) { 'text/html' }
         let(:body) { '<html><head><link rel="alternate" href="http://example.com/foo" type="application/activity+json"/></head></html>' }