From 608a2bfffc9186e71d52e0916b54b0f513994db2 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Tue, 20 Sep 2016 02:43:20 +0200 Subject: [PATCH] Upgrade to PubSubHubbub 0.4 (removing verify_token) --- Gemfile.lock | 2 +- app/controllers/api/subscriptions_controller.rb | 2 +- app/models/account.rb | 4 ++-- app/services/subscribe_service.rb | 7 ++----- .../20160920003904_remove_verify_token_from_accounts.rb | 5 +++++ db/schema.rb | 3 +-- lib/tasks/mastodon.rake | 3 ++- spec/controllers/api/subscriptions_controller_spec.rb | 4 ++-- spec/models/account_spec.rb | 8 +++----- 9 files changed, 19 insertions(+), 19 deletions(-) create mode 100644 db/migrate/20160920003904_remove_verify_token_from_accounts.rb diff --git a/Gemfile.lock b/Gemfile.lock index f7b51fed7a..6a567adec0 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -171,7 +171,7 @@ GEM pkg-config (~> 1.1.7) oj (2.17.3) orm_adapter (0.5.0) - ostatus2 (0.2.1) + ostatus2 (0.3) addressable (~> 2.4) http (~> 1.0) nokogiri (~> 1.6) diff --git a/app/controllers/api/subscriptions_controller.rb b/app/controllers/api/subscriptions_controller.rb index 84b88765a4..04d99b8280 100644 --- a/app/controllers/api/subscriptions_controller.rb +++ b/app/controllers/api/subscriptions_controller.rb @@ -3,7 +3,7 @@ class Api::SubscriptionsController < ApiController respond_to :txt def show - if @account.subscription(api_subscription_url(@account.id)).valid?(params['hub.topic'], params['hub.verify_token']) + if @account.subscription(api_subscription_url(@account.id)).valid?(params['hub.topic']) @account.update(subscription_expires_at: Time.now + (params['hub.lease_seconds'].to_i).seconds) render plain: HTMLEntities.new.encode(params['hub.challenge']), status: 200 else diff --git a/app/models/account.rb b/app/models/account.rb index a7f31440f1..bfb10ae516 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -66,7 +66,7 @@ class Account < ApplicationRecord end def subscribed? - !(self.secret.blank? || self.verify_token.blank?) + !self.subscription_expires_at.nil? end def favourited?(status) @@ -82,7 +82,7 @@ class Account < ApplicationRecord end def subscription(webhook_url) - OStatus2::Subscription.new(self.remote_url, secret: self.secret, token: self.verify_token, webhook: webhook_url, hub: self.hub_url) + OStatus2::Subscription.new(self.remote_url, secret: self.secret, lease_seconds: 86400 * 30, webhook: webhook_url, hub: self.hub_url) end def ping!(atom_url, hubs) diff --git a/app/services/subscribe_service.rb b/app/services/subscribe_service.rb index 7ead559d5c..427a5e1986 100644 --- a/app/services/subscribe_service.rb +++ b/app/services/subscribe_service.rb @@ -1,15 +1,12 @@ class SubscribeService < BaseService def call(account) - account.secret = SecureRandom.hex - account.verify_token = SecureRandom.hex + account.secret = SecureRandom.hex subscription = account.subscription(api_subscription_url(account.id)) response = subscription.subscribe unless response.successful? - account.secret = '' - account.verify_token = '' - + account.secret = '' Rails.logger.debug "PuSH subscription request for #{account.acct} failed: #{response.message}" end diff --git a/db/migrate/20160920003904_remove_verify_token_from_accounts.rb b/db/migrate/20160920003904_remove_verify_token_from_accounts.rb new file mode 100644 index 0000000000..ab6a6c84cb --- /dev/null +++ b/db/migrate/20160920003904_remove_verify_token_from_accounts.rb @@ -0,0 +1,5 @@ +class RemoveVerifyTokenFromAccounts < ActiveRecord::Migration[5.0] + def change + remove_column :accounts, :verify_token, :string, null: false, default: '' + end +end diff --git a/db/schema.rb b/db/schema.rb index 3179942c01..7127762152 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: 20160919221059) do +ActiveRecord::Schema.define(version: 20160920003904) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -18,7 +18,6 @@ ActiveRecord::Schema.define(version: 20160919221059) do create_table "accounts", force: :cascade do |t| t.string "username", default: "", null: false t.string "domain" - t.string "verify_token", default: "", null: false t.string "secret", default: "", null: false t.text "private_key" t.text "public_key", default: "", null: false diff --git a/lib/tasks/mastodon.rake b/lib/tasks/mastodon.rake index c75028ecb1..872b33cdd2 100644 --- a/lib/tasks/mastodon.rake +++ b/lib/tasks/mastodon.rake @@ -13,12 +13,13 @@ namespace :mastodon do task clear: :environment do Account.remote.without_followers.find_each do |a| Rails.logger.debug "PuSH unsubscribing from #{a.acct}" + begin a.subscription('').unsubscribe rescue HTTP::Error, OpenSSL::SSL::SSLError Rails.logger.debug "PuSH unsubscribing from #{a.acct} failed due to an HTTP or SSL error" ensure - a.update!(verify_token: '', secret: '', subscription_expires_at: nil) + a.update!(secret: '', subscription_expires_at: nil) end end end diff --git a/spec/controllers/api/subscriptions_controller_spec.rb b/spec/controllers/api/subscriptions_controller_spec.rb index a0b0b44135..ad0d0bc055 100644 --- a/spec/controllers/api/subscriptions_controller_spec.rb +++ b/spec/controllers/api/subscriptions_controller_spec.rb @@ -3,11 +3,11 @@ require 'rails_helper' RSpec.describe Api::SubscriptionsController, type: :controller do render_views - let(:account) { Fabricate(:account, username: 'gargron', domain: 'quitter.no', verify_token: '123', remote_url: 'topic_url', secret: 'abc') } + let(:account) { Fabricate(:account, username: 'gargron', domain: 'quitter.no', remote_url: 'topic_url', secret: 'abc') } describe 'GET #show' do before do - get :show, params: { :id => account.id, 'hub.topic' => 'topic_url', 'hub.verify_token' => 123, 'hub.challenge' => '456' } + get :show, params: { :id => account.id, 'hub.topic' => 'topic_url', 'hub.challenge' => '456', 'hub.lease_seconds' => "#{86400 * 30}" } end it 'returns http success' do diff --git a/spec/models/account_spec.rb b/spec/models/account_spec.rb index 93731d1e4c..0939ecdd01 100644 --- a/spec/models/account_spec.rb +++ b/spec/models/account_spec.rb @@ -66,14 +66,12 @@ RSpec.describe Account, type: :model do end describe '#subscribed?' do - it 'returns false when no secrets and tokens have been set' do + it 'returns false when no subscription expiration information is present' do expect(subject.subscribed?).to be false end - it 'returns true when the secret and token have been set' do - subject.secret = 'a' - subject.verify_token = 'b' - + it 'returns true when subscription expiration has been set' do + subject.subscription_expires_at = 30.days.from_now expect(subject.subscribed?).to be true end end