diff --git a/app/models/account.rb b/app/models/account.rb index a86778f5971..9ee0f5312bc 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -237,7 +237,17 @@ class Account < ApplicationRecord local? ? username : "#{username}@#{domain}" end + def pretty_username + # Return special username for user-facing invalid handle accounts + return id.to_s if invalidated_username? + + username + end + def pretty_acct + # Return special handle for user-facing invalid handle accounts + return "#{id}@handle.invalid" if invalidated_username? + local? ? username : "#{username}@#{Addressable::IDNA.to_unicode(domain)}" end @@ -254,17 +264,20 @@ class Account < ApplicationRecord end def invalidate_username! + raise ArgumentError if local? return if invalidated_username? - # This uses some characters (`{`, `}`, `!`) that we are extremely unlikely to allow - # i nthe future, in order to ensure this will never match a webfinger handle. + # It is very unlikely that we will allow `!` in usernames in the future, + # and we will never allow ` ` in them either, so this ensure this will never + # match a valid username on a remote server. + # Using the local ID ensures we won't have any conflict. - update_attribute(:username, "{invalid!#{id}}") + update_attribute(:username, "! #{id}") end def invalidated_username? - username.start_with?('{invalid!') + username.start_with?('! ') end def possibly_stale? diff --git a/app/models/concerns/account/finder_concern.rb b/app/models/concerns/account/finder_concern.rb index 249a7b5fd17..e075c2ed04c 100644 --- a/app/models/concerns/account/finder_concern.rb +++ b/app/models/concerns/account/finder_concern.rb @@ -25,6 +25,8 @@ module Account::FinderConcern end def find_remote(username, domain) + return if domain == 'handle.invalid' + Account .with_username(username) .with_domain(domain) diff --git a/app/serializers/rest/account_serializer.rb b/app/serializers/rest/account_serializer.rb index 34c99669ce0..b6c15979905 100644 --- a/app/serializers/rest/account_serializer.rb +++ b/app/serializers/rest/account_serializer.rb @@ -154,6 +154,10 @@ class REST::AccountSerializer < ActiveModel::Serializer object.memorial? end + def username + object.pretty_username + end + def invalid_handle object.invalidated_username? end diff --git a/app/serializers/rest/announcement_serializer.rb b/app/serializers/rest/announcement_serializer.rb index d1a011425dd..5735f93da08 100644 --- a/app/serializers/rest/announcement_serializer.rb +++ b/app/serializers/rest/announcement_serializer.rb @@ -41,6 +41,10 @@ class REST::AnnouncementSerializer < ActiveModel::Serializer object.id.to_s end + def username + object.pretty_username + end + def url ActivityPub::TagManager.instance.url_for(object) end diff --git a/app/serializers/rest/status_serializer.rb b/app/serializers/rest/status_serializer.rb index 8468029f7db..c06cdc1c9ff 100644 --- a/app/serializers/rest/status_serializer.rb +++ b/app/serializers/rest/status_serializer.rb @@ -201,7 +201,7 @@ class REST::StatusSerializer < ActiveModel::Serializer end def username - object.account_username + object.account.pretty_username end def url diff --git a/app/services/resolve_account_service.rb b/app/services/resolve_account_service.rb index 3da9a8ab772..da844ab149b 100644 --- a/app/services/resolve_account_service.rb +++ b/app/services/resolve_account_service.rb @@ -25,7 +25,7 @@ class ResolveAccountService < BaseService # First of all we want to check if we've got the account # record with the URI already, and if so, we can exit early - return if domain_not_allowed?(@domain) + return if domain_not_allowed?(@domain) || @domain == 'handle.invalid' @account ||= Account.find_remote(@username, @domain) diff --git a/spec/services/activitypub/process_account_service_spec.rb b/spec/services/activitypub/process_account_service_spec.rb index ebfbbcd0168..3fced86c96c 100644 --- a/spec/services/activitypub/process_account_service_spec.rb +++ b/spec/services/activitypub/process_account_service_spec.rb @@ -501,7 +501,7 @@ RSpec.describe ActivityPub::ProcessAccountService do .to change { account.reload.username }.from('bob').to('alice') .and change { account.reload.domain }.from('foo.test').to('example.com') .and not_change { account.reload.uri } - .and change { conflicting_account.reload.username }.from('alice').to("{invalid!#{conflicting_account.id}}") + .and change { conflicting_account.reload.username }.from('alice').to("! #{conflicting_account.id}") .and(not_change { Account.count }) expect(AccountRefreshWorker) @@ -583,7 +583,7 @@ RSpec.describe ActivityPub::ProcessAccountService do .to change { account.reload.username }.from('bob').to('alice') .and not_change { account.reload.domain } .and not_change { account.reload.uri } - .and change { conflicting_account.reload.username }.from('alice').to("{invalid!#{conflicting_account.id}}") + .and change { conflicting_account.reload.username }.from('alice').to("! #{conflicting_account.id}") .and(not_change { Account.count }) expect(AccountRefreshWorker)