Fix #2672 - Connect signed PuSH subscription requests to instance domain (#4205)

* Fix #2672 - Connect signed PuSH subscription requests to instance domain

Resolves #2739

* Fix return of locate_subscription

* Fix tests
This commit is contained in:
Eugen Rochko 2017-07-14 23:01:20 +02:00 committed by GitHub
parent de397f3bc1
commit cd9b2ab2f7
7 changed files with 33 additions and 10 deletions

View file

@ -1,6 +1,8 @@
# frozen_string_literal: true # frozen_string_literal: true
class Api::PushController < Api::BaseController class Api::PushController < Api::BaseController
include SignatureVerification
def update def update
response, status = process_push_request response, status = process_push_request
render plain: response, status: status render plain: response, status: status
@ -11,7 +13,7 @@ class Api::PushController < Api::BaseController
def process_push_request def process_push_request
case hub_mode case hub_mode
when 'subscribe' when 'subscribe'
Pubsubhubbub::SubscribeService.new.call(account_from_topic, hub_callback, hub_secret, hub_lease_seconds) Pubsubhubbub::SubscribeService.new.call(account_from_topic, hub_callback, hub_secret, hub_lease_seconds, verified_domain)
when 'unsubscribe' when 'unsubscribe'
Pubsubhubbub::UnsubscribeService.new.call(account_from_topic, hub_callback) Pubsubhubbub::UnsubscribeService.new.call(account_from_topic, hub_callback)
else else
@ -57,6 +59,10 @@ class Api::PushController < Api::BaseController
TagManager.instance.web_domain?(hub_topic_domain) TagManager.instance.web_domain?(hub_topic_domain)
end end
def verified_domain
return signed_request_account.domain if signed_request_account
end
def hub_topic_domain def hub_topic_domain
hub_topic_uri.host + (hub_topic_uri.port ? ":#{hub_topic_uri.port}" : '') hub_topic_uri.host + (hub_topic_uri.port ? ":#{hub_topic_uri.port}" : '')
end end

View file

@ -1,5 +1,4 @@
# frozen_string_literal: true # frozen_string_literal: true
# == Schema Information # == Schema Information
# #
# Table name: subscriptions # Table name: subscriptions
@ -13,6 +12,7 @@
# created_at :datetime not null # created_at :datetime not null
# updated_at :datetime not null # updated_at :datetime not null
# last_successful_delivery_at :datetime # last_successful_delivery_at :datetime
# domain :string
# #
class Subscription < ApplicationRecord class Subscription < ApplicationRecord

View file

@ -3,13 +3,15 @@
class Pubsubhubbub::SubscribeService < BaseService class Pubsubhubbub::SubscribeService < BaseService
URL_PATTERN = /\A#{URI.regexp(%w(http https))}\z/ URL_PATTERN = /\A#{URI.regexp(%w(http https))}\z/
attr_reader :account, :callback, :secret, :lease_seconds attr_reader :account, :callback, :secret,
:lease_seconds, :domain
def call(account, callback, secret, lease_seconds) def call(account, callback, secret, lease_seconds, verified_domain = nil)
@account = account @account = account
@callback = Addressable::URI.parse(callback).normalize.to_s @callback = Addressable::URI.parse(callback).normalize.to_s
@secret = secret @secret = secret
@lease_seconds = lease_seconds @lease_seconds = lease_seconds
@domain = verified_domain
process_subscribe process_subscribe
end end
@ -56,6 +58,14 @@ class Pubsubhubbub::SubscribeService < BaseService
end end
def locate_subscription def locate_subscription
Subscription.where(account: account, callback_url: callback).first_or_create!(account: account, callback_url: callback) subscription = Subscription.find_by(account: account, callback_url: callback)
if subscription.nil?
subscription = Subscription.new(account: account, callback_url: callback)
end
subscription.domain = domain
subscription.save!
subscription
end end
end end

View file

@ -35,16 +35,16 @@ class Pubsubhubbub::DistributionWorker
@payload = AtomSerializer.render(AtomSerializer.new.feed(@account, stream_entries)) @payload = AtomSerializer.render(AtomSerializer.new.feed(@account, stream_entries))
@domains = @account.followers.domains @domains = @account.followers.domains
Pubsubhubbub::DeliveryWorker.push_bulk(@subscriptions.reject { |s| !allowed_to_receive?(s.callback_url) }) do |subscription| Pubsubhubbub::DeliveryWorker.push_bulk(@subscriptions.reject { |s| !allowed_to_receive?(s.callback_url, s.domain) }) do |subscription|
[subscription.id, @payload] [subscription.id, @payload]
end end
end end
def active_subscriptions def active_subscriptions
Subscription.where(account: @account).active.select('id, callback_url') Subscription.where(account: @account).active.select('id, callback_url, domain')
end end
def allowed_to_receive?(callback_url) def allowed_to_receive?(callback_url, domain)
@domains.include?(Addressable::URI.parse(callback_url).host) (!domain.nil? && @domains.include?(domain)) || @domains.include?(Addressable::URI.parse(callback_url).host)
end end
end end

View file

@ -0,0 +1,5 @@
class AddDomainToSubscriptions < ActiveRecord::Migration[5.1]
def change
add_column :subscriptions, :domain, :string
end
end

View file

@ -10,7 +10,7 @@
# #
# It's strongly recommended that you check this file into your version control system. # It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 20170713190709) do ActiveRecord::Schema.define(version: 20170714184731) do
# These are extensions that must be enabled in order to support this database # These are extensions that must be enabled in order to support this database
enable_extension "plpgsql" enable_extension "plpgsql"
@ -326,6 +326,7 @@ ActiveRecord::Schema.define(version: 20170713190709) do
t.datetime "created_at", null: false t.datetime "created_at", null: false
t.datetime "updated_at", null: false t.datetime "updated_at", null: false
t.datetime "last_successful_delivery_at" t.datetime "last_successful_delivery_at"
t.string "domain"
t.index ["account_id", "callback_url"], name: "index_subscriptions_on_account_id_and_callback_url", unique: true t.index ["account_id", "callback_url"], name: "index_subscriptions_on_account_id_and_callback_url", unique: true
end end

View file

@ -21,6 +21,7 @@ RSpec.describe Api::PushController, type: :controller do
'https://callback.host/api', 'https://callback.host/api',
'as1234df', 'as1234df',
'3600', '3600',
nil
) )
expect(response).to have_http_status(:success) expect(response).to have_http_status(:success)
end end