From 5d3feed191bcbe2769512119752b426108152fe9 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Wed, 10 Jul 2019 18:59:28 +0200 Subject: Refactor fetching of remote resources (#11251) --- app/services/fetch_resource_service.rb | 68 ++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 app/services/fetch_resource_service.rb (limited to 'app/services/fetch_resource_service.rb') diff --git a/app/services/fetch_resource_service.rb b/app/services/fetch_resource_service.rb new file mode 100644 index 000000000..c0473f3ad --- /dev/null +++ b/app/services/fetch_resource_service.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +class FetchResourceService < BaseService + include JsonLdHelper + + ACCEPT_HEADER = 'application/activity+json, application/ld+json; profile="https://www.w3.org/ns/activitystreams", text/html' + + def call(url) + return if url.blank? + + process(url) + rescue HTTP::Error, OpenSSL::SSL::SSLError, Addressable::URI::InvalidURIError, Mastodon::HostValidationError, Mastodon::LengthValidationError => e + Rails.logger.debug "Error fetching resource #{@url}: #{e}" + nil + end + + private + + def process(url, terminal = false) + @url = url + + perform_request { |response| process_response(response, terminal) } + end + + def perform_request(&block) + Request.new(:get, @url).add_headers('Accept' => ACCEPT_HEADER).perform(&block) + end + + def process_response(response, terminal = false) + return nil if response.code != 200 + + if ['application/activity+json', 'application/ld+json'].include?(response.mime_type) + body = response.body_with_limit + json = body_to_json(body) + + [json['id'], { prefetched_body: body, id: true }, :activitypub] if supported_context?(json) && (equals_or_includes_any?(json['type'], ActivityPub::FetchRemoteAccountService::SUPPORTED_TYPES) || expected_type?(json)) + elsif !terminal + link_header = response['Link'] && parse_link_header(response) + + if link_header&.find_link(%w(rel alternate)) + process_link_headers(link_header) + elsif response.mime_type == 'text/html' + process_html(response) + end + end + end + + def expected_type?(json) + equals_or_includes_any?(json['type'], ActivityPub::Activity::Create::SUPPORTED_TYPES + ActivityPub::Activity::Create::CONVERTED_TYPES) + end + + def process_html(response) + page = Nokogiri::HTML(response.body_with_limit) + json_link = page.xpath('//link[@rel="alternate"]').find { |link| ['application/activity+json', 'application/ld+json; profile="https://www.w3.org/ns/activitystreams"'].include?(link['type']) } + + process(json_link['href'], terminal: true) unless json_link.nil? + end + + def process_link_headers(link_header) + json_link = link_header.find_link(%w(rel alternate), %w(type application/activity+json)) || link_header.find_link(%w(rel alternate), ['type', 'application/ld+json; profile="https://www.w3.org/ns/activitystreams"']) + + process(json_link.href, terminal: true) unless json_link.nil? + end + + def parse_link_header(response) + LinkHeader.parse(response['Link'].is_a?(Array) ? response['Link'].first : response['Link']) + end +end -- cgit From 4e8dcc5dbbf625b7268ed10d36122de985da6bdc Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Thu, 11 Jul 2019 14:49:55 +0200 Subject: Add HTTP signatures to all outgoing ActivityPub GET requests (#11284) --- app/helpers/jsonld_helper.rb | 13 ++--- app/lib/request.rb | 4 +- app/services/fetch_resource_service.rb | 2 +- .../concerns/signature_verification_spec.rb | 2 +- spec/services/fetch_remote_account_service_spec.rb | 1 + spec/services/fetch_resource_service_spec.rb | 61 +++++++++++++--------- 6 files changed, 43 insertions(+), 40 deletions(-) (limited to 'app/services/fetch_resource_service.rb') diff --git a/app/helpers/jsonld_helper.rb b/app/helpers/jsonld_helper.rb index 34a657e06..83a5b2462 100644 --- a/app/helpers/jsonld_helper.rb +++ b/app/helpers/jsonld_helper.rb @@ -77,19 +77,12 @@ module JsonLdHelper 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 diff --git a/app/lib/request.rb b/app/lib/request.rb index 1fd3f5190..9d874fe2c 100644 --- a/app/lib/request.rb +++ b/app/lib/request.rb @@ -40,8 +40,8 @@ class Request 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 diff --git a/app/services/fetch_resource_service.rb b/app/services/fetch_resource_service.rb index c0473f3ad..3676d899d 100644 --- a/app/services/fetch_resource_service.rb +++ b/app/services/fetch_resource_service.rb @@ -23,7 +23,7 @@ class FetchResourceService < BaseService 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) diff --git a/spec/controllers/concerns/signature_verification_spec.rb b/spec/controllers/concerns/signature_verification_spec.rb index 720690097..1fa19f54d 100644 --- a/spec/controllers/concerns/signature_verification_spec.rb +++ b/spec/controllers/concerns/signature_verification_spec.rb @@ -38,7 +38,7 @@ describe ApplicationController, type: :controller do 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 diff --git a/spec/services/fetch_remote_account_service_spec.rb b/spec/services/fetch_remote_account_service_spec.rb index ee7325be2..b37445861 100644 --- a/spec/services/fetch_remote_account_service_spec.rb +++ b/spec/services/fetch_remote_account_service_spec.rb @@ -4,6 +4,7 @@ RSpec.describe FetchRemoteAccountService, type: :service do 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) } diff --git a/spec/services/fetch_resource_service_spec.rb b/spec/services/fetch_resource_service_spec.rb index 17c192c44..98630966b 100644 --- a/spec/services/fetch_resource_service_spec.rb +++ b/spec/services/fetch_resource_service_spec.rb @@ -5,69 +5,78 @@ RSpec.describe FetchResourceService, type: :service do 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 } @@ -75,17 +84,17 @@ RSpec.describe FetchResourceService, type: :service do 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' => '; 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) { '' } -- cgit