Skip to content

Commit

Permalink
Select correct self link when parsing Webfinger response (mastodon#31110
Browse files Browse the repository at this point in the history
)
  • Loading branch information
adamniedzielski authored Jul 23, 2024
1 parent a8330be commit cd0ca4b
Show file tree
Hide file tree
Showing 10 changed files with 73 additions and 26 deletions.
17 changes: 15 additions & 2 deletions app/lib/webfinger.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ class GoneError < Error; end
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)
Expand All @@ -20,17 +22,28 @@ def subject
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

Expand Down
5 changes: 2 additions & 3 deletions app/services/activitypub/fetch_remote_actor_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ def check_webfinger!
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
Expand All @@ -58,8 +58,7 @@ def check_webfinger!
@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
Expand Down
8 changes: 1 addition & 7 deletions app/services/resolve_account_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,6 @@ def split_acct(acct)
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
Expand All @@ -122,12 +120,8 @@ def webfinger_update_due?
@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?
Expand Down
2 changes: 1 addition & 1 deletion spec/fixtures/requests/activitypub-webfinger.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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:[email protected]","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}"}]}
{"subject":"acct:[email protected]","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}"}]}
2 changes: 1 addition & 1 deletion spec/fixtures/requests/webfinger.txt
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,4 @@ Access-Control-Allow-Origin: *
Vary: Accept-Encoding,Cookie
Strict-Transport-Security: max-age=31536000; includeSubdomains;

{"subject":"acct:[email protected]","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:[email protected]","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}"}]}
41 changes: 41 additions & 0 deletions spec/lib/webfinger_spec.rb
Original file line number Diff line number Diff line change
@@ -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:[email protected]', 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:[email protected]').to_return(body: Oj.dump(webfinger), headers: { 'Content-Type': 'application/jrd+json' })

response = described_class.new('acct:[email protected]').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:[email protected]', 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:[email protected]').to_return(body: Oj.dump(webfinger), headers: { 'Content-Type': 'application/jrd+json' })

response = described_class.new('acct:[email protected]').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:[email protected]', 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:[email protected]').to_return(body: Oj.dump(webfinger), headers: { 'Content-Type': 'application/jrd+json' })

expect { described_class.new('acct:[email protected]').perform }.to raise_error(Webfinger::Error)
end
end
end
end
10 changes: 5 additions & 5 deletions spec/services/activitypub/fetch_remote_account_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
end

context 'when the account does not have a inbox' do
let!(:webfinger) { { subject: 'acct:[email protected]', links: [{ rel: 'self', href: 'https://example.com/alice' }] } }
let!(:webfinger) { { subject: 'acct:[email protected]', links: [{ rel: 'self', href: 'https://example.com/alice', type: 'application/activity+json' }] } }

before do
actor[:inbox] = nil
Expand All @@ -51,7 +51,7 @@
end

context 'when URI and WebFinger share the same host' do
let!(:webfinger) { { subject: 'acct:[email protected]', links: [{ rel: 'self', href: 'https://example.com/alice' }] } }
let!(:webfinger) { { subject: 'acct:[email protected]', 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' })
Expand All @@ -72,7 +72,7 @@
end

context 'when WebFinger presents different domain than URI' do
let!(:webfinger) { { subject: 'acct:[email protected]', links: [{ rel: 'self', href: 'https://example.com/alice' }] } }
let!(:webfinger) { { subject: 'acct:[email protected]', 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' })
Expand All @@ -95,7 +95,7 @@
end

context 'when WebFinger returns a different URI' do
let!(:webfinger) { { subject: 'acct:[email protected]', links: [{ rel: 'self', href: 'https://example.com/bob' }] } }
let!(:webfinger) { { subject: 'acct:[email protected]', 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' })
Expand All @@ -111,7 +111,7 @@
end

context 'when WebFinger returns a different URI after a redirection' do
let!(:webfinger) { { subject: 'acct:[email protected]', links: [{ rel: 'self', href: 'https://example.com/bob' }] } }
let!(:webfinger) { { subject: 'acct:[email protected]', 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' })
Expand Down
10 changes: 5 additions & 5 deletions spec/services/activitypub/fetch_remote_actor_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
end

context 'when the account does not have a inbox' do
let!(:webfinger) { { subject: 'acct:[email protected]', links: [{ rel: 'self', href: 'https://example.com/alice' }] } }
let!(:webfinger) { { subject: 'acct:[email protected]', links: [{ rel: 'self', href: 'https://example.com/alice', type: 'application/activity+json' }] } }

before do
actor[:inbox] = nil
Expand All @@ -51,7 +51,7 @@
end

context 'when URI and WebFinger share the same host' do
let!(:webfinger) { { subject: 'acct:[email protected]', links: [{ rel: 'self', href: 'https://example.com/alice' }] } }
let!(:webfinger) { { subject: 'acct:[email protected]', 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' })
Expand All @@ -72,7 +72,7 @@
end

context 'when WebFinger presents different domain than URI' do
let!(:webfinger) { { subject: 'acct:[email protected]', links: [{ rel: 'self', href: 'https://example.com/alice' }] } }
let!(:webfinger) { { subject: 'acct:[email protected]', 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' })
Expand All @@ -95,7 +95,7 @@
end

context 'when WebFinger returns a different URI' do
let!(:webfinger) { { subject: 'acct:[email protected]', links: [{ rel: 'self', href: 'https://example.com/bob' }] } }
let!(:webfinger) { { subject: 'acct:[email protected]', 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' })
Expand All @@ -111,7 +111,7 @@
end

context 'when WebFinger returns a different URI after a redirection' do
let!(:webfinger) { { subject: 'acct:[email protected]', links: [{ rel: 'self', href: 'https://example.com/bob' }] } }
let!(:webfinger) { { subject: 'acct:[email protected]', 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' })
Expand Down
2 changes: 1 addition & 1 deletion spec/services/activitypub/fetch_remote_key_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
RSpec.describe ActivityPub::FetchRemoteKeyService do
subject { described_class.new }

let(:webfinger) { { subject: 'acct:[email protected]', links: [{ rel: 'self', href: 'https://example.com/alice' }] } }
let(:webfinger) { { subject: 'acct:[email protected]', links: [{ rel: 'self', href: 'https://example.com/alice', type: 'application/activity+json' }] } }

let(:public_key_pem) do
<<~TEXT
Expand Down
2 changes: 1 addition & 1 deletion spec/services/activitypub/process_account_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@
}.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' })
Expand Down

0 comments on commit cd0ca4b

Please sign in to comment.