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

7 files changed, 33 insertions, 10 deletions

diff --git a/app/controllers/api/push_controller.rb b/app/controllers/api/push_controller.rb
index 951867140..e04d19125 100644
--- a/app/controllers/api/push_controller.rb
+++ b/app/controllers/api/push_controller.rb
@@ -1,6 +1,8 @@
 # frozen_string_literal: true
 
 class Api::PushController < Api::BaseController
+  include SignatureVerification
+
   def update
     response, status = process_push_request
     render plain: response, status: status
@@ -11,7 +13,7 @@ class Api::PushController < Api::BaseController
   def process_push_request
     case hub_mode
     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'
       Pubsubhubbub::UnsubscribeService.new.call(account_from_topic, hub_callback)
     else
@@ -57,6 +59,10 @@ class Api::PushController < Api::BaseController
     TagManager.instance.web_domain?(hub_topic_domain)
   end
 
+  def verified_domain
+    return signed_request_account.domain if signed_request_account
+  end
+
   def hub_topic_domain
     hub_topic_uri.host + (hub_topic_uri.port ? ":#{hub_topic_uri.port}" : '')
   end
diff --git a/app/models/subscription.rb b/app/models/subscription.rb
index d9d5024a9..bf643c1f9 100644
--- a/app/models/subscription.rb
+++ b/app/models/subscription.rb
@@ -1,5 +1,4 @@
 # frozen_string_literal: true
-
 # == Schema Information
 #
 # Table name: subscriptions
@@ -13,6 +12,7 @@
 #  created_at                  :datetime         not null
 #  updated_at                  :datetime         not null
 #  last_successful_delivery_at :datetime
+#  domain                      :string
 #
 
 class Subscription < ApplicationRecord
diff --git a/app/services/pubsubhubbub/subscribe_service.rb b/app/services/pubsubhubbub/subscribe_service.rb
index eeb7ab258..2dba05b12 100644
--- a/app/services/pubsubhubbub/subscribe_service.rb
+++ b/app/services/pubsubhubbub/subscribe_service.rb
@@ -3,13 +3,15 @@
 class Pubsubhubbub::SubscribeService < BaseService
   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
     @callback      = Addressable::URI.parse(callback).normalize.to_s
     @secret        = secret
     @lease_seconds = lease_seconds
+    @domain        = verified_domain
 
     process_subscribe
   end
@@ -56,6 +58,14 @@ class Pubsubhubbub::SubscribeService < BaseService
   end
 
   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
diff --git a/app/workers/pubsubhubbub/distribution_worker.rb b/app/workers/pubsubhubbub/distribution_worker.rb
index b41cec90d..7592354cc 100644
--- a/app/workers/pubsubhubbub/distribution_worker.rb
+++ b/app/workers/pubsubhubbub/distribution_worker.rb
@@ -35,16 +35,16 @@ class Pubsubhubbub::DistributionWorker
     @payload = AtomSerializer.render(AtomSerializer.new.feed(@account, stream_entries))
     @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]
     end
   end
 
   def active_subscriptions
-    Subscription.where(account: @account).active.select('id, callback_url')
+    Subscription.where(account: @account).active.select('id, callback_url, domain')
   end
 
-  def allowed_to_receive?(callback_url)
-    @domains.include?(Addressable::URI.parse(callback_url).host)
+  def allowed_to_receive?(callback_url, domain)
+    (!domain.nil? && @domains.include?(domain)) || @domains.include?(Addressable::URI.parse(callback_url).host)
   end
 end
diff --git a/db/migrate/20170714184731_add_domain_to_subscriptions.rb b/db/migrate/20170714184731_add_domain_to_subscriptions.rb
new file mode 100644
index 000000000..7c01a64f5
--- /dev/null
+++ b/db/migrate/20170714184731_add_domain_to_subscriptions.rb
@@ -0,0 +1,5 @@
+class AddDomainToSubscriptions < ActiveRecord::Migration[5.1]
+  def change
+    add_column :subscriptions, :domain, :string
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index b2c59a0f6..5ec78a7c9 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
 #
 # 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
   enable_extension "plpgsql"
@@ -326,6 +326,7 @@ ActiveRecord::Schema.define(version: 20170713190709) do
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
     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
   end
 
diff --git a/spec/controllers/api/push_controller_spec.rb b/spec/controllers/api/push_controller_spec.rb
index 18bfa70e5..647698bd1 100644
--- a/spec/controllers/api/push_controller_spec.rb
+++ b/spec/controllers/api/push_controller_spec.rb
@@ -21,6 +21,7 @@ RSpec.describe Api::PushController, type: :controller do
           'https://callback.host/api',
           'as1234df',
           '3600',
+          nil
         )
         expect(response).to have_http_status(:success)
       end