diff options
| -rw-r--r-- | app/helpers/jsonld_helper.rb | 13 | ||||
| -rw-r--r-- | app/lib/request.rb | 4 | ||||
| -rw-r--r-- | app/services/fetch_resource_service.rb | 2 | ||||
| -rw-r--r-- | spec/controllers/concerns/signature_verification_spec.rb | 2 | ||||
| -rw-r--r-- | spec/services/fetch_remote_account_service_spec.rb | 1 | ||||
| -rw-r--r-- | spec/services/fetch_resource_service_spec.rb | 61 |
6 files changed, 43 insertions, 40 deletions
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' => '<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>' } |