From cd0ca4b99473f54464e5134f91b7b1c8d5544011 Mon Sep 17 00:00:00 2001 From: Adam Niedzielski Date: Tue, 23 Jul 2024 16:42:31 +0200 Subject: [PATCH] Select correct self link when parsing Webfinger response (#31110) --- app/lib/webfinger.rb | 17 +++++++- .../activitypub/fetch_remote_actor_service.rb | 5 +-- app/services/resolve_account_service.rb | 8 +--- .../requests/activitypub-webfinger.txt | 2 +- spec/fixtures/requests/webfinger.txt | 2 +- spec/lib/webfinger_spec.rb | 41 +++++++++++++++++++ .../fetch_remote_account_service_spec.rb | 10 ++--- .../fetch_remote_actor_service_spec.rb | 10 ++--- .../fetch_remote_key_service_spec.rb | 2 +- .../process_account_service_spec.rb | 2 +- 10 files changed, 73 insertions(+), 26 deletions(-) create mode 100644 spec/lib/webfinger_spec.rb diff --git a/app/lib/webfinger.rb b/app/lib/webfinger.rb index aeafe19702..01a5dbc21d 100644 --- a/app/lib/webfinger.rb +++ b/app/lib/webfinger.rb @@ -6,6 +6,8 @@ class Webfinger class RedirectError < Error; end class Response + ACTIVITYPUB_READY_TYPE = ['application/activity+json', 'application/ld+json; profile="https://www.w3.org/ns/activitystreams"'].freeze + attr_reader :uri def initialize(uri, body) @@ -20,17 +22,28 @@ class Webfinger end def link(rel, attribute) - links.dig(rel, attribute) + links.dig(rel, 0, attribute) + end + + def self_link_href + self_link.fetch('href') end private def links - @links ||= @json.fetch('links', []).index_by { |link| link['rel'] } + @links ||= @json.fetch('links', []).group_by { |link| link['rel'] } + end + + def self_link + links.fetch('self', []).find do |link| + ACTIVITYPUB_READY_TYPE.include?(link['type']) + end end def validate_response! raise Webfinger::Error, "Missing subject in response for #{@uri}" if subject.blank? + raise Webfinger::Error, "Missing self link in response for #{@uri}" if self_link.blank? end end diff --git a/app/services/activitypub/fetch_remote_actor_service.rb b/app/services/activitypub/fetch_remote_actor_service.rb index 86a134bb4e..2c372c2ec3 100644 --- a/app/services/activitypub/fetch_remote_actor_service.rb +++ b/app/services/activitypub/fetch_remote_actor_service.rb @@ -49,7 +49,7 @@ class ActivityPub::FetchRemoteActorService < BaseService confirmed_username, confirmed_domain = split_acct(webfinger.subject) if @username.casecmp(confirmed_username).zero? && @domain.casecmp(confirmed_domain).zero? - raise Error, "Webfinger response for #{@username}@#{@domain} does not loop back to #{@uri}" if webfinger.link('self', 'href') != @uri + raise Error, "Webfinger response for #{@username}@#{@domain} does not loop back to #{@uri}" if webfinger.self_link_href != @uri return end @@ -58,8 +58,7 @@ class ActivityPub::FetchRemoteActorService < BaseService @username, @domain = split_acct(webfinger.subject) raise Webfinger::RedirectError, "Too many webfinger redirects for URI #{@uri} (stopped at #{@username}@#{@domain})" unless confirmed_username.casecmp(@username).zero? && confirmed_domain.casecmp(@domain).zero? - - raise Error, "Webfinger response for #{@username}@#{@domain} does not loop back to #{@uri}" if webfinger.link('self', 'href') != @uri + raise Error, "Webfinger response for #{@username}@#{@domain} does not loop back to #{@uri}" if webfinger.self_link_href != @uri rescue Webfinger::RedirectError => e raise Error, e.message rescue Webfinger::Error => e diff --git a/app/services/resolve_account_service.rb b/app/services/resolve_account_service.rb index 078a0423f2..8a5863baba 100644 --- a/app/services/resolve_account_service.rb +++ b/app/services/resolve_account_service.rb @@ -106,8 +106,6 @@ class ResolveAccountService < BaseService end def fetch_account! - return unless activitypub_ready? - with_redis_lock("resolve:#{@username}@#{@domain}") do @account = ActivityPub::FetchRemoteAccountService.new.call(actor_url, suppress_errors: @options[:suppress_errors]) end @@ -122,12 +120,8 @@ class ResolveAccountService < BaseService @options[:skip_cache] || @account.nil? || @account.possibly_stale? end - def activitypub_ready? - ['application/activity+json', 'application/ld+json; profile="https://www.w3.org/ns/activitystreams"'].include?(@webfinger.link('self', 'type')) - end - def actor_url - @actor_url ||= @webfinger.link('self', 'href') + @actor_url ||= @webfinger.self_link_href end def gone_from_origin? diff --git a/spec/fixtures/requests/activitypub-webfinger.txt b/spec/fixtures/requests/activitypub-webfinger.txt index 465066d84e..733b1693dc 100644 --- a/spec/fixtures/requests/activitypub-webfinger.txt +++ b/spec/fixtures/requests/activitypub-webfinger.txt @@ -4,4 +4,4 @@ Content-Type: application/jrd+json; charset=utf-8 X-Content-Type-Options: nosniff Date: Sun, 17 Sep 2017 06:22:50 GMT -{"subject":"acct:foo@ap.example.com","aliases":["https://ap.example.com/@foo","https://ap.example.com/users/foo"],"links":[{"rel":"http://webfinger.net/rel/profile-page","type":"text/html","href":"https://ap.example.com/@foo"},{"rel":"http://schemas.google.com/g/2010#updates-from","type":"application/atom+xml","href":"https://ap.example.com/users/foo.atom"},{"rel":"self","type":"application/activity+json","href":"https://ap.example.com/users/foo"},{"rel":"salmon","href":"https://ap.example.com/api/salmon/1"},{"rel":"magic-public-key","href":"data:application/magic-public-key,RSA.u3L4vnpNLzVH31MeWI394F0wKeJFsLDAsNXGeOu0QF2x-h1zLWZw_agqD2R3JPU9_kaDJGPIV2Sn5zLyUA9S6swCCMOtn7BBR9g9sucgXJmUFB0tACH2QSgHywMAybGfmSb3LsEMNKsGJ9VsvYoh8lDET6X4Pyw-ZJU0_OLo_41q9w-OrGtlsTm_PuPIeXnxa6BLqnDaxC-4IcjG_FiPahNCTINl_1F_TgSSDZ4Taf4U9XFEIFw8wmgploELozzIzKq-t8nhQYkgAkt64euWpva3qL5KD1mTIZQEP-LZvh3s2WHrLi3fhbdRuwQ2c0KkJA2oSTFPDpqqbPGZ3QvuHQ==.AQAB"},{"rel":"http://ostatus.org/schema/1.0/subscribe","template":"https://ap.example.com/authorize_follow?acct={uri}"}]} \ No newline at end of file +{"subject":"acct:foo@ap.example.com","aliases":["https://ap.example.com/@foo","https://ap.example.com/users/foo"],"links":[{"rel":"http://webfinger.net/rel/profile-page","type":"text/html","href":"https://ap.example.com/@foo"},{"rel":"http://schemas.google.com/g/2010#updates-from","type":"application/atom+xml","href":"https://ap.example.com/users/foo.atom"},{"rel":"self","type":"application/html","href":"https://ap.example.com/users/foo.html"},{"rel":"self","type":"application/activity+json","href":"https://ap.example.com/users/foo"},{"rel":"self","type":"application/json","href":"https://ap.example.com/users/foo.json"},{"rel":"salmon","href":"https://ap.example.com/api/salmon/1"},{"rel":"magic-public-key","href":"data:application/magic-public-key,RSA.u3L4vnpNLzVH31MeWI394F0wKeJFsLDAsNXGeOu0QF2x-h1zLWZw_agqD2R3JPU9_kaDJGPIV2Sn5zLyUA9S6swCCMOtn7BBR9g9sucgXJmUFB0tACH2QSgHywMAybGfmSb3LsEMNKsGJ9VsvYoh8lDET6X4Pyw-ZJU0_OLo_41q9w-OrGtlsTm_PuPIeXnxa6BLqnDaxC-4IcjG_FiPahNCTINl_1F_TgSSDZ4Taf4U9XFEIFw8wmgploELozzIzKq-t8nhQYkgAkt64euWpva3qL5KD1mTIZQEP-LZvh3s2WHrLi3fhbdRuwQ2c0KkJA2oSTFPDpqqbPGZ3QvuHQ==.AQAB"},{"rel":"http://ostatus.org/schema/1.0/subscribe","template":"https://ap.example.com/authorize_follow?acct={uri}"}]} \ No newline at end of file diff --git a/spec/fixtures/requests/webfinger.txt b/spec/fixtures/requests/webfinger.txt index f337ecae6f..fce821bddb 100644 --- a/spec/fixtures/requests/webfinger.txt +++ b/spec/fixtures/requests/webfinger.txt @@ -8,4 +8,4 @@ Access-Control-Allow-Origin: * Vary: Accept-Encoding,Cookie Strict-Transport-Security: max-age=31536000; includeSubdomains; -{"subject":"acct:gargron@quitter.no","aliases":["https:\/\/quitter.no\/user\/7477","https:\/\/quitter.no\/gargron","https:\/\/quitter.no\/index.php\/user\/7477","https:\/\/quitter.no\/index.php\/gargron"],"links":[{"rel":"http:\/\/webfinger.net\/rel\/profile-page","type":"text\/html","href":"https:\/\/quitter.no\/gargron"},{"rel":"http:\/\/gmpg.org\/xfn\/11","type":"text\/html","href":"https:\/\/quitter.no\/gargron"},{"rel":"describedby","type":"application\/rdf+xml","href":"https:\/\/quitter.no\/gargron\/foaf"},{"rel":"http:\/\/apinamespace.org\/atom","type":"application\/atomsvc+xml","href":"https:\/\/quitter.no\/api\/statusnet\/app\/service\/gargron.xml"},{"rel":"http:\/\/apinamespace.org\/twitter","href":"https:\/\/quitter.no\/api\/"},{"rel":"http:\/\/specs.openid.net\/auth\/2.0\/provider","href":"https:\/\/quitter.no\/gargron"},{"rel":"http:\/\/schemas.google.com\/g\/2010#updates-from","type":"application\/atom+xml","href":"https:\/\/quitter.no\/api\/statuses\/user_timeline\/7477.atom"},{"rel":"magic-public-key","href":"data:application\/magic-public-key,RSA.1ZBkHTavLvxH3FzlKv4O6WtlILKRFfNami3_Rcu8EuogtXSYiS-bB6hElZfUCSHbC4uLemOA34PEhz__CDMozax1iI_t8dzjDnh1x0iFSup7pSfW9iXk_WU3Dm74yWWW2jildY41vWgrEstuQ1dJ8vVFfSJ9T_tO4c-T9y8vDI8=.AQAB"},{"rel":"salmon","href":"https:\/\/quitter.no\/main\/salmon\/user\/7477"},{"rel":"http:\/\/salmon-protocol.org\/ns\/salmon-replies","href":"https:\/\/quitter.no\/main\/salmon\/user\/7477"},{"rel":"http:\/\/salmon-protocol.org\/ns\/salmon-mention","href":"https:\/\/quitter.no\/main\/salmon\/user\/7477"},{"rel":"http:\/\/ostatus.org\/schema\/1.0\/subscribe","template":"https:\/\/quitter.no\/main\/ostatussub?profile={uri}"}]} +{"subject":"acct:gargron@quitter.no","aliases":["https:\/\/quitter.no\/user\/7477","https:\/\/quitter.no\/gargron","https:\/\/quitter.no\/index.php\/user\/7477","https:\/\/quitter.no\/index.php\/gargron"],"links":[{"rel":"http:\/\/webfinger.net\/rel\/profile-page","type":"text\/html","href":"https:\/\/quitter.no\/gargron"},{"rel":"http:\/\/gmpg.org\/xfn\/11","type":"text\/html","href":"https:\/\/quitter.no\/gargron"},{"rel":"describedby","type":"application\/rdf+xml","href":"https:\/\/quitter.no\/gargron\/foaf"},{"rel":"self","type":"application/activity+json","href":"https://ap.example.com/users/foo"},{"rel":"http:\/\/apinamespace.org\/atom","type":"application\/atomsvc+xml","href":"https:\/\/quitter.no\/api\/statusnet\/app\/service\/gargron.xml"},{"rel":"http:\/\/apinamespace.org\/twitter","href":"https:\/\/quitter.no\/api\/"},{"rel":"http:\/\/specs.openid.net\/auth\/2.0\/provider","href":"https:\/\/quitter.no\/gargron"},{"rel":"http:\/\/schemas.google.com\/g\/2010#updates-from","type":"application\/atom+xml","href":"https:\/\/quitter.no\/api\/statuses\/user_timeline\/7477.atom"},{"rel":"magic-public-key","href":"data:application\/magic-public-key,RSA.1ZBkHTavLvxH3FzlKv4O6WtlILKRFfNami3_Rcu8EuogtXSYiS-bB6hElZfUCSHbC4uLemOA34PEhz__CDMozax1iI_t8dzjDnh1x0iFSup7pSfW9iXk_WU3Dm74yWWW2jildY41vWgrEstuQ1dJ8vVFfSJ9T_tO4c-T9y8vDI8=.AQAB"},{"rel":"salmon","href":"https:\/\/quitter.no\/main\/salmon\/user\/7477"},{"rel":"http:\/\/salmon-protocol.org\/ns\/salmon-replies","href":"https:\/\/quitter.no\/main\/salmon\/user\/7477"},{"rel":"http:\/\/salmon-protocol.org\/ns\/salmon-mention","href":"https:\/\/quitter.no\/main\/salmon\/user\/7477"},{"rel":"http:\/\/ostatus.org\/schema\/1.0\/subscribe","template":"https:\/\/quitter.no\/main\/ostatussub?profile={uri}"}]} diff --git a/spec/lib/webfinger_spec.rb b/spec/lib/webfinger_spec.rb new file mode 100644 index 0000000000..5015deac7f --- /dev/null +++ b/spec/lib/webfinger_spec.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Webfinger do + describe 'self link' do + context 'when self link is specified with type application/activity+json' do + let!(:webfinger) { { subject: 'acct:alice@example.com', links: [{ rel: 'self', href: 'https://example.com/alice', type: 'application/activity+json' }] } } + + it 'correctly parses the response' do + stub_request(:get, 'https://example.com/.well-known/webfinger?resource=acct:alice@example.com').to_return(body: Oj.dump(webfinger), headers: { 'Content-Type': 'application/jrd+json' }) + + response = described_class.new('acct:alice@example.com').perform + + expect(response.self_link_href).to eq 'https://example.com/alice' + end + end + + context 'when self link is specified with type application/ld+json' do + let!(:webfinger) { { subject: 'acct:alice@example.com', links: [{ rel: 'self', href: 'https://example.com/alice', type: 'application/ld+json; profile="https://www.w3.org/ns/activitystreams"' }] } } + + it 'correctly parses the response' do + stub_request(:get, 'https://example.com/.well-known/webfinger?resource=acct:alice@example.com').to_return(body: Oj.dump(webfinger), headers: { 'Content-Type': 'application/jrd+json' }) + + response = described_class.new('acct:alice@example.com').perform + + expect(response.self_link_href).to eq 'https://example.com/alice' + end + end + + context 'when self link is specified with incorrect type' do + let!(:webfinger) { { subject: 'acct:alice@example.com', links: [{ rel: 'self', href: 'https://example.com/alice', type: 'application/json"' }] } } + + it 'raises an error' do + stub_request(:get, 'https://example.com/.well-known/webfinger?resource=acct:alice@example.com').to_return(body: Oj.dump(webfinger), headers: { 'Content-Type': 'application/jrd+json' }) + + expect { described_class.new('acct:alice@example.com').perform }.to raise_error(Webfinger::Error) + end + end + end +end diff --git a/spec/services/activitypub/fetch_remote_account_service_spec.rb b/spec/services/activitypub/fetch_remote_account_service_spec.rb index 789a705c41..175ac9cb61 100644 --- a/spec/services/activitypub/fetch_remote_account_service_spec.rb +++ b/spec/services/activitypub/fetch_remote_account_service_spec.rb @@ -33,7 +33,7 @@ RSpec.describe ActivityPub::FetchRemoteAccountService do end context 'when the account does not have a inbox' do - let!(:webfinger) { { subject: 'acct:alice@example.com', links: [{ rel: 'self', href: 'https://example.com/alice' }] } } + let!(:webfinger) { { subject: 'acct:alice@example.com', links: [{ rel: 'self', href: 'https://example.com/alice', type: 'application/activity+json' }] } } before do actor[:inbox] = nil @@ -51,7 +51,7 @@ RSpec.describe ActivityPub::FetchRemoteAccountService do end context 'when URI and WebFinger share the same host' do - let!(:webfinger) { { subject: 'acct:alice@example.com', links: [{ rel: 'self', href: 'https://example.com/alice' }] } } + let!(:webfinger) { { subject: 'acct:alice@example.com', links: [{ rel: 'self', href: 'https://example.com/alice', type: 'application/activity+json' }] } } before do stub_request(:get, 'https://example.com/alice').to_return(body: Oj.dump(actor), headers: { 'Content-Type': 'application/activity+json' }) @@ -72,7 +72,7 @@ RSpec.describe ActivityPub::FetchRemoteAccountService do end context 'when WebFinger presents different domain than URI' do - let!(:webfinger) { { subject: 'acct:alice@iscool.af', links: [{ rel: 'self', href: 'https://example.com/alice' }] } } + let!(:webfinger) { { subject: 'acct:alice@iscool.af', links: [{ rel: 'self', href: 'https://example.com/alice', type: 'application/activity+json' }] } } before do stub_request(:get, 'https://example.com/alice').to_return(body: Oj.dump(actor), headers: { 'Content-Type': 'application/activity+json' }) @@ -95,7 +95,7 @@ RSpec.describe ActivityPub::FetchRemoteAccountService do end context 'when WebFinger returns a different URI' do - let!(:webfinger) { { subject: 'acct:alice@example.com', links: [{ rel: 'self', href: 'https://example.com/bob' }] } } + let!(:webfinger) { { subject: 'acct:alice@example.com', links: [{ rel: 'self', href: 'https://example.com/bob', type: 'application/activity+json' }] } } before do stub_request(:get, 'https://example.com/alice').to_return(body: Oj.dump(actor), headers: { 'Content-Type': 'application/activity+json' }) @@ -111,7 +111,7 @@ RSpec.describe ActivityPub::FetchRemoteAccountService do end context 'when WebFinger returns a different URI after a redirection' do - let!(:webfinger) { { subject: 'acct:alice@iscool.af', links: [{ rel: 'self', href: 'https://example.com/bob' }] } } + let!(:webfinger) { { subject: 'acct:alice@iscool.af', links: [{ rel: 'self', href: 'https://example.com/bob', type: 'application/activity+json' }] } } before do stub_request(:get, 'https://example.com/alice').to_return(body: Oj.dump(actor), headers: { 'Content-Type': 'application/activity+json' }) diff --git a/spec/services/activitypub/fetch_remote_actor_service_spec.rb b/spec/services/activitypub/fetch_remote_actor_service_spec.rb index 025051e9fa..9d031cb89b 100644 --- a/spec/services/activitypub/fetch_remote_actor_service_spec.rb +++ b/spec/services/activitypub/fetch_remote_actor_service_spec.rb @@ -33,7 +33,7 @@ RSpec.describe ActivityPub::FetchRemoteActorService do end context 'when the account does not have a inbox' do - let!(:webfinger) { { subject: 'acct:alice@example.com', links: [{ rel: 'self', href: 'https://example.com/alice' }] } } + let!(:webfinger) { { subject: 'acct:alice@example.com', links: [{ rel: 'self', href: 'https://example.com/alice', type: 'application/activity+json' }] } } before do actor[:inbox] = nil @@ -51,7 +51,7 @@ RSpec.describe ActivityPub::FetchRemoteActorService do end context 'when URI and WebFinger share the same host' do - let!(:webfinger) { { subject: 'acct:alice@example.com', links: [{ rel: 'self', href: 'https://example.com/alice' }] } } + let!(:webfinger) { { subject: 'acct:alice@example.com', links: [{ rel: 'self', href: 'https://example.com/alice', type: 'application/activity+json' }] } } before do stub_request(:get, 'https://example.com/alice').to_return(body: Oj.dump(actor), headers: { 'Content-Type': 'application/activity+json' }) @@ -72,7 +72,7 @@ RSpec.describe ActivityPub::FetchRemoteActorService do end context 'when WebFinger presents different domain than URI' do - let!(:webfinger) { { subject: 'acct:alice@iscool.af', links: [{ rel: 'self', href: 'https://example.com/alice' }] } } + let!(:webfinger) { { subject: 'acct:alice@iscool.af', links: [{ rel: 'self', href: 'https://example.com/alice', type: 'application/activity+json' }] } } before do stub_request(:get, 'https://example.com/alice').to_return(body: Oj.dump(actor), headers: { 'Content-Type': 'application/activity+json' }) @@ -95,7 +95,7 @@ RSpec.describe ActivityPub::FetchRemoteActorService do end context 'when WebFinger returns a different URI' do - let!(:webfinger) { { subject: 'acct:alice@example.com', links: [{ rel: 'self', href: 'https://example.com/bob' }] } } + let!(:webfinger) { { subject: 'acct:alice@example.com', links: [{ rel: 'self', href: 'https://example.com/bob', type: 'application/activity+json' }] } } before do stub_request(:get, 'https://example.com/alice').to_return(body: Oj.dump(actor), headers: { 'Content-Type': 'application/activity+json' }) @@ -111,7 +111,7 @@ RSpec.describe ActivityPub::FetchRemoteActorService do end context 'when WebFinger returns a different URI after a redirection' do - let!(:webfinger) { { subject: 'acct:alice@iscool.af', links: [{ rel: 'self', href: 'https://example.com/bob' }] } } + let!(:webfinger) { { subject: 'acct:alice@iscool.af', links: [{ rel: 'self', href: 'https://example.com/bob', type: 'application/activity+json' }] } } before do stub_request(:get, 'https://example.com/alice').to_return(body: Oj.dump(actor), headers: { 'Content-Type': 'application/activity+json' }) diff --git a/spec/services/activitypub/fetch_remote_key_service_spec.rb b/spec/services/activitypub/fetch_remote_key_service_spec.rb index b6fcf3f479..847a154108 100644 --- a/spec/services/activitypub/fetch_remote_key_service_spec.rb +++ b/spec/services/activitypub/fetch_remote_key_service_spec.rb @@ -5,7 +5,7 @@ require 'rails_helper' RSpec.describe ActivityPub::FetchRemoteKeyService do subject { described_class.new } - let(:webfinger) { { subject: 'acct:alice@example.com', links: [{ rel: 'self', href: 'https://example.com/alice' }] } } + let(:webfinger) { { subject: 'acct:alice@example.com', links: [{ rel: 'self', href: 'https://example.com/alice', type: 'application/activity+json' }] } } let(:public_key_pem) do <<~TEXT diff --git a/spec/services/activitypub/process_account_service_spec.rb b/spec/services/activitypub/process_account_service_spec.rb index 4fbb527b39..86314e6b48 100644 --- a/spec/services/activitypub/process_account_service_spec.rb +++ b/spec/services/activitypub/process_account_service_spec.rb @@ -215,7 +215,7 @@ RSpec.describe ActivityPub::ProcessAccountService do }.with_indifferent_access webfinger = { subject: "acct:user#{i}@foo.test", - links: [{ rel: 'self', href: "https://foo.test/users/#{i}" }], + links: [{ rel: 'self', href: "https://foo.test/users/#{i}", type: 'application/activity+json' }], }.with_indifferent_access stub_request(:get, "https://foo.test/users/#{i}").to_return(status: 200, body: actor_json.to_json, headers: { 'Content-Type': 'application/activity+json' }) stub_request(:get, "https://foo.test/users/#{i}/featured").to_return(status: 200, body: featured_json.to_json, headers: { 'Content-Type': 'application/activity+json' })