diff options
| -rw-r--r-- | Gemfile | 1 | ||||
| -rw-r--r-- | Gemfile.lock | 3 | ||||
| -rw-r--r-- | app/services/resolve_remote_account_service.rb | 166 | ||||
| -rw-r--r-- | spec/services/resolve_remote_account_service_spec.rb | 23 |
4 files changed, 138 insertions, 55 deletions
diff --git a/Gemfile b/Gemfile index a6c2b2d65..f4d889165 100644 --- a/Gemfile +++ b/Gemfile @@ -52,6 +52,7 @@ gem 'rack-timeout', '~> 0.4' gem 'rails-i18n', '~> 5.0' gem 'rails-settings-cached', '~> 0.6' gem 'redis', '~> 3.3', require: ['redis', 'redis/connection/hiredis'] +gem 'mario-redis-lock', '~> 1.2', require: 'redis_lock' gem 'rqrcode', '~> 0.10' gem 'ruby-oembed', '~> 0.12', require: 'oembed' gem 'sanitize', '~> 4.4' diff --git a/Gemfile.lock b/Gemfile.lock index f637c9bbe..6f0621f5f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -242,6 +242,8 @@ GEM nokogiri (>= 1.5.9) mail (2.6.6) mime-types (>= 1.16, < 4) + mario-redis-lock (1.2.0) + redis (~> 3, >= 3.0.5) method_source (0.8.2) microformats (4.0.7) json @@ -535,6 +537,7 @@ DEPENDENCIES letter_opener_web (~> 1.3) link_header (~> 0.0) lograge (~> 0.5) + mario-redis-lock (~> 1.2) microformats (~> 4.0) mime-types (~> 3.1) nokogiri (~> 1.7) diff --git a/app/services/resolve_remote_account_service.rb b/app/services/resolve_remote_account_service.rb index d2dfda824..c948243ec 100644 --- a/app/services/resolve_remote_account_service.rb +++ b/app/services/resolve_remote_account_service.rb @@ -11,97 +11,153 @@ class ResolveRemoteAccountService < BaseService # @param [String] uri User URI in the form of username@domain # @return [Account] def call(uri, update_profile = true, redirected = nil) - username, domain = uri.split('@') + @username, @domain = uri.split('@') - return Account.find_local(username) if TagManager.instance.local_domain?(domain) + return Account.find_local(@username) if TagManager.instance.local_domain?(@domain) - account = Account.find_remote(username, domain) - return account unless account_needs_webfinger_update?(account) + @account = Account.find_remote(@username, @domain) + + return @account unless webfinger_update_due? Rails.logger.debug "Looking up webfinger for #{uri}" - data = Goldfinger.finger("acct:#{uri}") + @webfinger = Goldfinger.finger("acct:#{uri}") - raise Goldfinger::Error, 'Missing resource links' if data.link('http://schemas.google.com/g/2010#updates-from').nil? || data.link('salmon').nil? || data.link('http://webfinger.net/rel/profile-page').nil? || data.link('magic-public-key').nil? + raise Goldfinger::Error, 'Missing resource links' if links_missing? - # Disallow account hijacking - confirmed_username, confirmed_domain = data.subject.gsub(/\Aacct:/, '').split('@') + confirmed_username, confirmed_domain = @webfinger.subject.gsub(/\Aacct:/, '').split('@') - unless confirmed_username.casecmp(username).zero? && confirmed_domain.casecmp(domain).zero? + if confirmed_username.casecmp(@username).zero? && confirmed_domain.casecmp(@domain).zero? + @username = confirmed_username + @domain = confirmed_domain + else return call("#{confirmed_username}@#{confirmed_domain}", update_profile, true) if redirected.nil? - raise Goldfinger::Error, 'Requested and returned acct URI do not match' + raise Goldfinger::Error, 'Requested and returned acct URIs do not match' end - return Account.find_local(confirmed_username) if TagManager.instance.local_domain?(confirmed_domain) + return Account.find_local(@username) if TagManager.instance.local_domain?(@domain) - confirmed_account = Account.find_remote(confirmed_username, confirmed_domain) - if confirmed_account.nil? - Rails.logger.debug "Creating new remote account for #{uri}" + RedisLock.acquire(lock_options) do |lock| + if lock.acquired? + @account = Account.find_remote(@username, @domain) - domain_block = DomainBlock.find_by(domain: domain) - account = Account.new(username: confirmed_username, domain: confirmed_domain) - account.suspended = true if domain_block && domain_block.suspend? - account.silenced = true if domain_block && domain_block.silence? - account.private_key = nil - else - account = confirmed_account + create_account if @account.nil? + update_account + + update_account_profile if update_profile + end end - account.last_webfingered_at = Time.now.utc + @account + end - account.remote_url = data.link('http://schemas.google.com/g/2010#updates-from').href - account.salmon_url = data.link('salmon').href - account.url = data.link('http://webfinger.net/rel/profile-page').href - account.public_key = magic_key_to_pem(data.link('magic-public-key').href) + private - body, xml = get_feed(account.remote_url) - hubs = get_hubs(xml) + def links_missing? + @webfinger.link('http://schemas.google.com/g/2010#updates-from').nil? || + @webfinger.link('salmon').nil? || + @webfinger.link('http://webfinger.net/rel/profile-page').nil? || + @webfinger.link('magic-public-key').nil? + end - account.uri = get_account_uri(xml) - account.hub_url = hubs.first.attribute('href').value + def webfinger_update_due? + @account.nil? || @account.last_webfingered_at.nil? || @account.last_webfingered_at <= 1.day.ago + end - begin - account.save! - get_profile(body, account) if update_profile - rescue ActiveRecord::RecordNotUnique - # The account has been added by another worker! - return Account.find_remote(confirmed_username, confirmed_domain) - end + def create_account + Rails.logger.debug "Creating new remote account for #{@username}@#{@domain}" - account + @account = Account.new(username: @username, domain: @domain) + @account.suspended = true if auto_suspend? + @account.silenced = true if auto_silence? + @account.private_key = nil end - private + def update_account + @account.last_webfingered_at = Time.now.utc + @account.remote_url = atom_url + @account.salmon_url = salmon_url + @account.url = url + @account.public_key = public_key + @account.uri = canonical_uri + @account.hub_url = hub_url + @account.save! + end - def account_needs_webfinger_update?(account) - account&.last_webfingered_at.nil? || account.last_webfingered_at <= 1.day.ago + def auto_suspend? + domain_block && domain_block.suspend? end - def get_feed(url) - response = Request.new(:get, url).perform - raise Goldfinger::Error, "Feed attempt failed for #{url}: HTTP #{response.code}" unless response.code == 200 - [response.to_s, Nokogiri::XML(response)] + def auto_silence? + domain_block && domain_block.silence? end - def get_hubs(xml) - hubs = xml.xpath('//xmlns:link[@rel="hub"]') - raise Goldfinger::Error, 'No PubSubHubbub hubs found' if hubs.empty? || hubs.first.attribute('href').nil? - hubs + def domain_block + return @domain_block if defined?(@domain_block) + @domain_block = DomainBlock.find_by(domain: @domain) end - def get_account_uri(xml) - author_uri = xml.at_xpath('/xmlns:feed/xmlns:author/xmlns:uri') + def atom_url + @atom_url ||= @webfinger.link('http://schemas.google.com/g/2010#updates-from').href + end + + def salmon_url + @salmon_url ||= @webfinger.link('salmon').href + end + + def url + @url ||= @webfinger.link('http://webfinger.net/rel/profile-page').href + end + + def public_key + @public_key ||= magic_key_to_pem(@webfinger.link('magic-public-key').href) + end + + def canonical_uri + return @canonical_uri if defined?(@canonical_uri) + + author_uri = atom.at_xpath('/xmlns:feed/xmlns:author/xmlns:uri') if author_uri.nil? - owner = xml.at_xpath('/xmlns:feed').at_xpath('./dfrn:owner', dfrn: DFRN_NS) + owner = atom.at_xpath('/xmlns:feed').at_xpath('./dfrn:owner', dfrn: DFRN_NS) author_uri = owner.at_xpath('./xmlns:uri') unless owner.nil? end raise Goldfinger::Error, 'Author URI could not be found' if author_uri.nil? - author_uri.content + + @canonical_uri = author_uri.content + end + + def hub_url + return @hub_url if defined?(@hub_url) + + hubs = atom.xpath('//xmlns:link[@rel="hub"]') + + raise Goldfinger::Error, 'No PubSubHubbub hubs found' if hubs.empty? || hubs.first['href'].nil? + + @hub_url = hubs.first['href'] + end + + def atom_body + return @atom_body if defined?(@atom_body) + + response = Request.new(:get, atom_url).perform + + raise Goldfinger::Error, "Feed attempt failed for #{atom_url}: HTTP #{response.code}" unless response.code == 200 + + @atom_body = response.to_s + end + + def atom + return @atom if defined?(@atom) + @atom = Nokogiri::XML(atom_body) + end + + def update_account_profile + RemoteProfileUpdateWorker.perform_async(@account.id, atom_body.force_encoding('UTF-8'), false) end - def get_profile(body, account) - RemoteProfileUpdateWorker.perform_async(account.id, body.force_encoding('UTF-8'), false) + def lock_options + { redis: Redis.current, key: "resolve:#{@username}@#{@domain}" } end end diff --git a/spec/services/resolve_remote_account_service_spec.rb b/spec/services/resolve_remote_account_service_spec.rb index ad4d436ba..c51588210 100644 --- a/spec/services/resolve_remote_account_service_spec.rb +++ b/spec/services/resolve_remote_account_service_spec.rb @@ -68,4 +68,27 @@ RSpec.describe ResolveRemoteAccountService do expect(account.domain).to eq 'localdomain.com' expect(account.remote_url).to eq 'https://webdomain.com/users/foo.atom' end + + it 'processes one remote account at a time using locks' do + wait_for_start = true + fail_occurred = false + return_values = [] + + threads = Array.new(5) do + Thread.new do + true while wait_for_start + begin + return_values << subject.call('foo@localdomain.com') + rescue ActiveRecord::RecordNotUnique + fail_occurred = true + end + end + end + + wait_for_start = false + threads.each(&:join) + + expect(fail_occurred).to be false + expect(return_values).to_not include(nil) + end end |