Fix #2706 - Always respond with 200 to PuSH payloads (#2733)

Fix #2196 - Respond with 201 when Salmon accepted, 400 when unverified
Fix #2629 - Correctly handle confirm_domain? for local accounts
Unify rules for extracting author acct from XML, prefer <email>, fall back
to <name> + <uri> (see also #2017, #2172)
This commit is contained in:
Eugen Rochko 2017-05-03 17:02:18 +02:00 committed by GitHub
parent dd9d57300b
commit bafd22ecf4
9 changed files with 77 additions and 85 deletions

View File

@ -5,13 +5,13 @@ class Api::SalmonController < ApiController
respond_to :txt respond_to :txt
def update def update
body = request.body.read payload = request.body.read
if body.nil? if !payload.nil? && verify?(payload)
head 200 SalmonWorker.perform_async(@account.id, payload.force_encoding('UTF-8'))
else
SalmonWorker.perform_async(@account.id, body.force_encoding('UTF-8'))
head 201 head 201
else
head 202
end end
end end
@ -20,4 +20,8 @@ class Api::SalmonController < ApiController
def set_account def set_account
@account = Account.find(params[:id]) @account = Account.find(params[:id])
end end
def verify?(payload)
VerifySalmonService.new.call(payload)
end
end end

View File

@ -19,10 +19,9 @@ class Api::SubscriptionsController < ApiController
if subscription.verify(body, request.headers['HTTP_X_HUB_SIGNATURE']) if subscription.verify(body, request.headers['HTTP_X_HUB_SIGNATURE'])
ProcessingWorker.perform_async(@account.id, body.force_encoding('UTF-8')) ProcessingWorker.perform_async(@account.id, body.force_encoding('UTF-8'))
head 201
else
head 202
end end
head 200
end end
private private

View File

@ -0,0 +1,21 @@
# frozen_string_literal: true
module AuthorExtractor
def author_from_xml(xml)
# Try <email> for acct
acct = xml.at_xpath('./xmlns:author/xmlns:email', xmlns: TagManager::XMLNS)&.content
# Try <name> + <uri>
if acct.blank?
username = xml.at_xpath('./xmlns:author/xmlns:name', xmlns: TagManager::XMLNS)&.content
uri = xml.at_xpath('./xmlns:author/xmlns:uri', xmlns: TagManager::XMLNS)&.content
return nil if username.blank? || uri.blank?
domain = Addressable::URI.parse(uri).normalize.host
acct = "#{username}@#{domain}"
end
FollowRemoteAccountService.new.call(acct)
end
end

View File

@ -1,6 +1,8 @@
# frozen_string_literal: true # frozen_string_literal: true
class FetchRemoteAccountService < BaseService class FetchRemoteAccountService < BaseService
include AuthorExtractor
def call(url, prefetched_body = nil) def call(url, prefetched_body = nil)
if prefetched_body.nil? if prefetched_body.nil?
atom_url, body = FetchAtomService.new.call(url) atom_url, body = FetchAtomService.new.call(url)
@ -19,21 +21,10 @@ class FetchRemoteAccountService < BaseService
xml = Nokogiri::XML(body) xml = Nokogiri::XML(body)
xml.encoding = 'utf-8' xml.encoding = 'utf-8'
email = xml.at_xpath('//xmlns:author/xmlns:email').try(:content) account = author_from_xml(xml.at_xpath('/xmlns:feed', xmlns: TagManager::XMLNS))
if email.nil?
url_parts = Addressable::URI.parse(url).normalize
username = xml.at_xpath('//xmlns:author/xmlns:name').try(:content)
domain = url_parts.host
else
username, domain = email.split('@')
end
return nil if username.nil? || domain.nil?
Rails.logger.debug "Going to webfinger #{username}@#{domain}"
account = FollowRemoteAccountService.new.call("#{username}@#{domain}")
UpdateRemoteProfileService.new.call(xml, account) unless account.nil? UpdateRemoteProfileService.new.call(xml, account) unless account.nil?
account account
rescue TypeError rescue TypeError
Rails.logger.debug "Unparseable URL given: #{url}" Rails.logger.debug "Unparseable URL given: #{url}"

View File

@ -1,6 +1,8 @@
# frozen_string_literal: true # frozen_string_literal: true
class FetchRemoteStatusService < BaseService class FetchRemoteStatusService < BaseService
include AuthorExtractor
def call(url, prefetched_body = nil) def call(url, prefetched_body = nil)
if prefetched_body.nil? if prefetched_body.nil?
atom_url, body = FetchAtomService.new.call(url) atom_url, body = FetchAtomService.new.call(url)
@ -21,37 +23,19 @@ class FetchRemoteStatusService < BaseService
xml = Nokogiri::XML(body) xml = Nokogiri::XML(body)
xml.encoding = 'utf-8' xml.encoding = 'utf-8'
account = extract_author(url, xml) account = author_from_xml(xml.at_xpath('/xmlns:entry', xmlns: TagManager::XMLNS))
domain = Addressable::URI.parse(url).normalize.host
return nil if account.nil? return nil unless !account.nil? && confirmed_domain?(domain, account)
statuses = ProcessFeedService.new.call(body, account) statuses = ProcessFeedService.new.call(body, account)
statuses.first statuses.first
end
def extract_author(url, xml)
url_parts = Addressable::URI.parse(url).normalize
username = xml.at_xpath('//xmlns:author/xmlns:name').try(:content)
domain = url_parts.host
return nil if username.nil?
Rails.logger.debug "Going to webfinger #{username}@#{domain}"
account = FollowRemoteAccountService.new.call("#{username}@#{domain}")
# If the author's confirmed URLs do not match the domain of the URL
# we are reading this from, abort
return nil unless confirmed_domain?(domain, account)
account
rescue Nokogiri::XML::XPath::SyntaxError rescue Nokogiri::XML::XPath::SyntaxError
Rails.logger.debug 'Invalid XML or missing namespace' Rails.logger.debug 'Invalid XML or missing namespace'
nil nil
end end
def confirmed_domain?(domain, account) def confirmed_domain?(domain, account)
domain.casecmp(account.domain).zero? || domain.casecmp(Addressable::URI.parse(account.remote_url).normalize.host).zero? account.domain.nil? || domain.casecmp(account.domain).zero? || domain.casecmp(Addressable::URI.parse(account.remote_url).normalize.host).zero?
end end
end end

View File

@ -20,6 +20,8 @@ class ProcessFeedService < BaseService
end end
class ProcessEntry class ProcessEntry
include AuthorExtractor
def call(xml, account) def call(xml, account)
@account = account @account = account
@xml = xml @xml = xml
@ -108,7 +110,7 @@ class ProcessFeedService < BaseService
# If that author cannot be found, don't record the status (do not misattribute) # If that author cannot be found, don't record the status (do not misattribute)
if account?(entry) if account?(entry)
begin begin
account = find_or_resolve_account(acct(entry)) account = author_from_xml(entry)
return [nil, false] if account.nil? return [nil, false] if account.nil?
rescue Goldfinger::Error rescue Goldfinger::Error
return [nil, false] return [nil, false]
@ -143,10 +145,6 @@ class ProcessFeedService < BaseService
[status, true] [status, true]
end end
def find_or_resolve_account(acct)
FollowRemoteAccountService.new.call(acct)
end
def find_or_resolve_status(parent, uri, url) def find_or_resolve_status(parent, uri, url)
status = find_status(uri) status = find_status(uri)
@ -275,13 +273,5 @@ class ProcessFeedService < BaseService
def account?(xml = @xml) def account?(xml = @xml)
!xml.at_xpath('./xmlns:author', xmlns: TagManager::XMLNS).nil? !xml.at_xpath('./xmlns:author', xmlns: TagManager::XMLNS).nil?
end end
def acct(xml = @xml)
username = xml.at_xpath('./xmlns:author/xmlns:name', xmlns: TagManager::XMLNS).content
url = xml.at_xpath('./xmlns:author/xmlns:uri', xmlns: TagManager::XMLNS).content
domain = Addressable::URI.parse(url).normalize.host
"#{username}@#{domain}"
end
end end
end end

View File

@ -1,6 +1,8 @@
# frozen_string_literal: true # frozen_string_literal: true
class ProcessInteractionService < BaseService class ProcessInteractionService < BaseService
include AuthorExtractor
# Record locally the remote interaction with our user # Record locally the remote interaction with our user
# @param [String] envelope Salmon envelope # @param [String] envelope Salmon envelope
# @param [Account] target_account Account the Salmon was addressed to # @param [Account] target_account Account the Salmon was addressed to
@ -10,18 +12,9 @@ class ProcessInteractionService < BaseService
xml = Nokogiri::XML(body) xml = Nokogiri::XML(body)
xml.encoding = 'utf-8' xml.encoding = 'utf-8'
return unless contains_author?(xml) account = author_from_xml(xml.at_xpath('/xmlns:entry', xmlns: TagManager::XMLNS))
username = xml.at_xpath('/xmlns:entry/xmlns:author/xmlns:name', xmlns: TagManager::XMLNS).content return if account.nil? || account.suspended?
url = xml.at_xpath('/xmlns:entry/xmlns:author/xmlns:uri', xmlns: TagManager::XMLNS).content
domain = Addressable::URI.parse(url).normalize.host
account = Account.find_by(username: username, domain: domain)
if account.nil?
account = follow_remote_account_service.call("#{username}@#{domain}")
end
return if account.suspended?
if salmon.verify(envelope, account.keypair) if salmon.verify(envelope, account.keypair)
RemoteProfileUpdateWorker.perform_async(account.id, body.force_encoding('UTF-8'), true) RemoteProfileUpdateWorker.perform_async(account.id, body.force_encoding('UTF-8'), true)
@ -59,10 +52,6 @@ class ProcessInteractionService < BaseService
private private
def contains_author?(xml)
!(xml.at_xpath('/xmlns:entry/xmlns:author/xmlns:name', xmlns: TagManager::XMLNS).nil? || xml.at_xpath('/xmlns:entry/xmlns:author/xmlns:uri', xmlns: TagManager::XMLNS).nil?)
end
def mentions_account?(xml, account) def mentions_account?(xml, account)
xml.xpath('/xmlns:entry/xmlns:link[@rel="mentioned"]', xmlns: TagManager::XMLNS).each { |mention_link| return true if [TagManager.instance.uri_for(account), TagManager.instance.url_for(account)].include?(mention_link.attribute('href').value) } xml.xpath('/xmlns:entry/xmlns:link[@rel="mentioned"]', xmlns: TagManager::XMLNS).each { |mention_link| return true if [TagManager.instance.uri_for(account), TagManager.instance.url_for(account)].include?(mention_link.attribute('href').value) }
false false
@ -144,16 +133,4 @@ class ProcessInteractionService < BaseService
def salmon def salmon
@salmon ||= OStatus2::Salmon.new @salmon ||= OStatus2::Salmon.new
end end
def follow_remote_account_service
@follow_remote_account_service ||= FollowRemoteAccountService.new
end
def process_feed_service
@process_feed_service ||= ProcessFeedService.new
end
def remove_status_service
@remove_status_service ||= RemoveStatusService.new
end
end end

View File

@ -0,0 +1,26 @@
# frozen_string_literal: true
class VerifySalmonService < BaseService
include AuthorExtractor
def call(payload)
body = salmon.unpack(payload)
xml = Nokogiri::XML(body)
xml.encoding = 'utf-8'
account = author_from_xml(xml.at_xpath('/xmlns:entry', xmlns: TagManager::XMLNS))
if account.nil?
false
else
salmon.verify(payload, account.keypair)
end
end
private
def salmon
@salmon ||= OStatus2::Salmon.new
end
end

View File

@ -45,8 +45,8 @@ RSpec.describe Api::SubscriptionsController, type: :controller do
post :update, params: { id: account.id } post :update, params: { id: account.id }
end end
it 'returns http created' do it 'returns http success' do
expect(response).to have_http_status(:created) expect(response).to have_http_status(:success)
end end
it 'creates statuses for feed' do it 'creates statuses for feed' do