From f29918e7071160f277ac5834d83e409d8fa20063 Mon Sep 17 00:00:00 2001 From: ThibG Date: Tue, 12 Sep 2017 23:10:40 +0200 Subject: [WiP] Whenever a remote keypair changes, unfollow them and re-subscribe to … (#4907) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Whenever a remote keypair changes, unfollow them and re-subscribe to them In Mastodon (it could be different for other OStatus or AP-enabled software), a keypair change is indicative of whole user (or instance) data loss. In this situation, the “new” user might be different, and almost certainly has an empty followers list. In this case, Mastodon instances will disagree on follower lists, leading to unreliable delivery and “shadow followers”, that is users believed by a remote instance to be followers, without the affected user knowing. Drawbacks of this change are: 1. If an user legitimately changes public key for some reason without losing data (not possible in Mastodon at the moment), they will have their remote followers unsubscribed/re-subscribed needlessly. 2. Depending of the number of remote followers, this may generate quite some traffic. 3. If the user change is an attempt at usurpation, the remote followers will unknowingly follow the usurper. Note that this is *not* a change of behavior, Mastodon already behaves like that, although delivery might be unreliable, and the usurper would not have known the former user's followers. * Rename ResubscribeWorker to RefollowWorker * Process followers in batches --- app/services/resolve_remote_account_service.rb | 2 ++ 1 file changed, 2 insertions(+) (limited to 'app/services/resolve_remote_account_service.rb') diff --git a/app/services/resolve_remote_account_service.rb b/app/services/resolve_remote_account_service.rb index 7031c98f5..753601501 100644 --- a/app/services/resolve_remote_account_service.rb +++ b/app/services/resolve_remote_account_service.rb @@ -85,8 +85,10 @@ class ResolveRemoteAccountService < BaseService def handle_ostatus create_account if @account.nil? + old_public_key = @account.public_key update_account update_account_profile if update_profile? + RefollowWorker.perform_async(@account.id) if old_public_key != @account.public_key end def update_profile? -- cgit From af00220d795670e10bc8c7378837c4a5a287b556 Mon Sep 17 00:00:00 2001 From: ThibG Date: Thu, 14 Sep 2017 00:05:25 +0200 Subject: Fix refollowing (#4931) * Make RefollowWorker ActivityPub-only to avoid potential identifier mismatches * Don't call RefollowWorker on new accounts --- app/services/activitypub/process_account_service.rb | 4 ++-- app/services/resolve_remote_account_service.rb | 2 -- app/workers/refollow_worker.rb | 1 + 3 files changed, 3 insertions(+), 4 deletions(-) (limited to 'app/services/resolve_remote_account_service.rb') diff --git a/app/services/activitypub/process_account_service.rb b/app/services/activitypub/process_account_service.rb index badb26720..a45681078 100644 --- a/app/services/activitypub/process_account_service.rb +++ b/app/services/activitypub/process_account_service.rb @@ -15,11 +15,11 @@ class ActivityPub::ProcessAccountService < BaseService @account = Account.find_by(uri: @uri) @collections = {} + old_public_key = @account&.public_key create_account if @account.nil? upgrade_account if @account.ostatus? - old_public_key = @account.public_key update_account - RefollowWorker.perform_async(@account.id) if old_public_key != @account.public_key + RefollowWorker.perform_async(@account.id) if !old_public_key.nil? && old_public_key != @account.public_key @account rescue Oj::ParseError diff --git a/app/services/resolve_remote_account_service.rb b/app/services/resolve_remote_account_service.rb index 753601501..7031c98f5 100644 --- a/app/services/resolve_remote_account_service.rb +++ b/app/services/resolve_remote_account_service.rb @@ -85,10 +85,8 @@ class ResolveRemoteAccountService < BaseService def handle_ostatus create_account if @account.nil? - old_public_key = @account.public_key update_account update_account_profile if update_profile? - RefollowWorker.perform_async(@account.id) if old_public_key != @account.public_key end def update_profile? diff --git a/app/workers/refollow_worker.rb b/app/workers/refollow_worker.rb index 9c42d4271..66bcd27c3 100644 --- a/app/workers/refollow_worker.rb +++ b/app/workers/refollow_worker.rb @@ -7,6 +7,7 @@ class RefollowWorker def perform(target_account_id) target_account = Account.find(target_account_id) + return unless target_account.protocol == :activitypub target_account.followers.where(domain: nil).find_each do |follower| # Locally unfollow remote account -- cgit From 1eab53ee1030542a5c4c56203a61eecae9768131 Mon Sep 17 00:00:00 2001 From: unarist Date: Sun, 17 Sep 2017 18:54:23 +0900 Subject: Fix an error when actor json couldn't be fetched in ResolveRemoteAccountService (#4979) * Fix an error when actor json couldn't be fetched in ResolveRemoteAccountService * Add specs --- app/services/resolve_remote_account_service.rb | 1 + .../requests/activitypub-actor-noinbox.txt | 9 +++++ spec/fixtures/requests/activitypub-actor.txt | 9 +++++ spec/fixtures/requests/activitypub-feed.txt | 47 ++++++++++++++++++++++ spec/fixtures/requests/activitypub-webfinger.txt | 7 ++++ .../resolve_remote_account_service_spec.rb | 33 +++++++++++++++ 6 files changed, 106 insertions(+) create mode 100644 spec/fixtures/requests/activitypub-actor-noinbox.txt create mode 100644 spec/fixtures/requests/activitypub-actor.txt create mode 100644 spec/fixtures/requests/activitypub-feed.txt create mode 100644 spec/fixtures/requests/activitypub-webfinger.txt (limited to 'app/services/resolve_remote_account_service.rb') diff --git a/app/services/resolve_remote_account_service.rb b/app/services/resolve_remote_account_service.rb index 7031c98f5..57c80fc82 100644 --- a/app/services/resolve_remote_account_service.rb +++ b/app/services/resolve_remote_account_service.rb @@ -80,6 +80,7 @@ class ResolveRemoteAccountService < BaseService def activitypub_ready? !@webfinger.link('self').nil? && ['application/activity+json', 'application/ld+json; profile="https://www.w3.org/ns/activitystreams"'].include?(@webfinger.link('self').type) && + !actor_json.nil? && actor_json['inbox'].present? end diff --git a/spec/fixtures/requests/activitypub-actor-noinbox.txt b/spec/fixtures/requests/activitypub-actor-noinbox.txt new file mode 100644 index 000000000..95b4650e0 --- /dev/null +++ b/spec/fixtures/requests/activitypub-actor-noinbox.txt @@ -0,0 +1,9 @@ +HTTP/1.1 200 OK +Date: Sun, 17 Sep 2017 06:51:23 GMT +Content-Type: application/json; charset=utf-8 +X-XSS-Protection: 1; mode=block +Link: ; rel="lrdd"; type="application/xrd+xml", ; rel="alternate"; type="application/atom+xml" +Vary: Accept-Encoding +Cache-Control: max-age=0, private, must-revalidate + +{"@context":"https://www.w3.org/ns/activitystreams","id":"https://ap.example.com/users/foo","type":"Person","following":"https://ap.example.com/users/foo/following","followers":"https://ap.example.com/users/foo/followers","inbox":null,"outbox":"https://ap.example.com/users/foo/outbox","preferredUsername":"foo","name":"","summary":"\u003cp\u003etest\u003c/p\u003e","icon":"https://quitter.no/avatar/7477-300-20160211190340.png","image":"/headers/original/missing.png","publicKey":{"id":"https://ap.example.com/users/foo#main-key","owner":"https://ap.example.com/users/foo","publicKeyPem":"-----BEGIN PUBLIC KEY-----\nMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAu3L4vnpNLzVH31MeWI39\n4F0wKeJFsLDAsNXGeOu0QF2x+h1zLWZw/agqD2R3JPU9/kaDJGPIV2Sn5zLyUA9S\n6swCCMOtn7BBR9g9sucgXJmUFB0tACH2QSgHywMAybGfmSb3LsEMNKsGJ9VsvYoh\n8lDET6X4Pyw+ZJU0/OLo/41q9w+OrGtlsTm/PuPIeXnxa6BLqnDaxC+4IcjG/FiP\nahNCTINl/1F/TgSSDZ4Taf4U9XFEIFw8wmgploELozzIzKq+t8nhQYkgAkt64euW\npva3qL5KD1mTIZQEP+LZvh3s2WHrLi3fhbdRuwQ2c0KkJA2oSTFPDpqqbPGZ3Qvu\nHQIDAQAB\n-----END PUBLIC KEY-----\n"}} \ No newline at end of file diff --git a/spec/fixtures/requests/activitypub-actor.txt b/spec/fixtures/requests/activitypub-actor.txt new file mode 100644 index 000000000..6514241cb --- /dev/null +++ b/spec/fixtures/requests/activitypub-actor.txt @@ -0,0 +1,9 @@ +HTTP/1.1 200 OK +Cache-Control: max-age=0, private, must-revalidate +Content-Type: application/activity+json; charset=utf-8 +Link: ; rel="lrdd"; type="application/xrd+xml", ; rel="alternate"; type="application/atom+xml", ; rel="alternate"; type="application/activity+json" +Vary: Accept-Encoding +X-Content-Type-Options: nosniff +X-Xss-Protection: 1; mode=block + +{"@context":["https://www.w3.org/ns/activitystreams","https://w3id.org/security/v1",{"manuallyApprovesFollowers":"as:manuallyApprovesFollowers","sensitive":"as:sensitive","Hashtag":"as:Hashtag","ostatus":"http://ostatus.org#","atomUri":"ostatus:atomUri","inReplyToAtomUri":"ostatus:inReplyToAtomUri","conversation":"ostatus:conversation"}],"id":"https://ap.example.com/users/foo","type":"Person","following":"https://ap.example.com/users/foo/following","followers":"https://ap.example.com/users/foo/followers","inbox":"https://ap.example.com/users/foo/inbox","outbox":"https://ap.example.com/users/foo/outbox","preferredUsername":"foo","name":"","summary":"\u003cp\u003etest\u003c/p\u003e","url":"https://ap.example.com/@foo","manuallyApprovesFollowers":false,"publicKey":{"id":"https://ap.example.com/users/foo#main-key","owner":"https://ap.example.com/users/foo","publicKeyPem":"-----BEGIN PUBLIC KEY-----\nMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAu3L4vnpNLzVH31MeWI39\n4F0wKeJFsLDAsNXGeOu0QF2x+h1zLWZw/agqD2R3JPU9/kaDJGPIV2Sn5zLyUA9S\n6swCCMOtn7BBR9g9sucgXJmUFB0tACH2QSgHywMAybGfmSb3LsEMNKsGJ9VsvYoh\n8lDET6X4Pyw+ZJU0/OLo/41q9w+OrGtlsTm/PuPIeXnxa6BLqnDaxC+4IcjG/FiP\nahNCTINl/1F/TgSSDZ4Taf4U9XFEIFw8wmgploELozzIzKq+t8nhQYkgAkt64euW\npva3qL5KD1mTIZQEP+LZvh3s2WHrLi3fhbdRuwQ2c0KkJA2oSTFPDpqqbPGZ3Qvu\nHQIDAQAB\n-----END PUBLIC KEY-----\n"},"endpoints":{"sharedInbox":"https://ap.example.com/inbox"},"icon":{"type":"Image","url":"https://quitter.no/avatar/7477-300-20160211190340.png"}} \ No newline at end of file diff --git a/spec/fixtures/requests/activitypub-feed.txt b/spec/fixtures/requests/activitypub-feed.txt new file mode 100644 index 000000000..84fd414c3 --- /dev/null +++ b/spec/fixtures/requests/activitypub-feed.txt @@ -0,0 +1,47 @@ +HTTP/1.1 200 OK +Cache-Control: max-age=0, private, must-revalidate +Content-Type: application/atom+xml; charset=utf-8 +Link: ; rel="lrdd"; type="application/xrd+xml", ; rel="alternate"; type="application/atom+xml", ; rel="alternate"; type="application/activity+json" +Vary: Accept-Encoding +Date: Sun, 17 Sep 2017 06:33:53 GMT + + + + https://ap.example.com/users/foo.atom + foo + test + 2017-09-16T18:50:09Z + https://ap.example.com/system/accounts/avatars/000/000/001/original/141ee5846d159cba.png?1505587809 + + https://ap.example.com/users/foo + http://activitystrea.ms/schema/1.0/person + https://ap.example.com/users/foo + foo + foo@ap.example.com + <p>test</p> + + + foo + test + public + + + + + + + https://ap.example.com/users/foo/statuses/11076 + 2017-09-13T01:23:19Z + 2017-09-13T01:23:19Z + New status by foo + http://activitystrea.ms/schema/1.0/note + http://activitystrea.ms/schema/1.0/post + + <p>test</p> + + public + + + + + diff --git a/spec/fixtures/requests/activitypub-webfinger.txt b/spec/fixtures/requests/activitypub-webfinger.txt new file mode 100644 index 000000000..465066d84 --- /dev/null +++ b/spec/fixtures/requests/activitypub-webfinger.txt @@ -0,0 +1,7 @@ +HTTP/1.1 200 OK +Cache-Control: max-age=0, private, must-revalidate +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 diff --git a/spec/services/resolve_remote_account_service_spec.rb b/spec/services/resolve_remote_account_service_spec.rb index d0eab2310..d0bb6a137 100644 --- a/spec/services/resolve_remote_account_service_spec.rb +++ b/spec/services/resolve_remote_account_service_spec.rb @@ -72,6 +72,39 @@ RSpec.describe ResolveRemoteAccountService do end context 'with an ActivityPub account' do + before do + stub_request(:get, "https://ap.example.com/.well-known/webfinger?resource=acct:foo@ap.example.com").to_return(request_fixture('activitypub-webfinger.txt')) + stub_request(:get, "https://ap.example.com/users/foo").to_return(request_fixture('activitypub-actor.txt')) + stub_request(:get, "https://ap.example.com/users/foo.atom").to_return(request_fixture('activitypub-feed.txt')) + stub_request(:get, %r{https://ap.example.com/users/foo/\w+}).to_return(status: 404) + end + + it 'fallback to OStatus if actor json could not be fetched' do + stub_request(:get, "https://ap.example.com/users/foo").to_return(status: 404) + + account = subject.call('foo@ap.example.com') + + expect(account.ostatus?).to eq true + expect(account.remote_url).to eq 'https://ap.example.com/users/foo.atom' + end + + it 'fallback to OStatus if actor json did not have inbox_url' do + stub_request(:get, "https://ap.example.com/users/foo").to_return(request_fixture('activitypub-actor-noinbox.txt')) + + account = subject.call('foo@ap.example.com') + + expect(account.ostatus?).to eq true + expect(account.remote_url).to eq 'https://ap.example.com/users/foo.atom' + end + + it 'returns new remote account' do + account = subject.call('foo@ap.example.com') + + expect(account.activitypub?).to eq true + expect(account.domain).to eq 'ap.example.com' + expect(account.inbox_url).to eq 'https://ap.example.com/users/foo/inbox' + end + pending end -- cgit