From fae32634b15a29e66d5c2a04015f2f947cf54627 Mon Sep 17 00:00:00 2001 From: Naoki Kosaka Date: Sat, 5 Jan 2019 15:17:12 +0900 Subject: Use Contact User as Relay, Report, Subscribe. (#9661) * Use Contact User as Relay, Report, Subscribe. * Use Account.representative to fetch contact user. * Use find_local. * No reason to use Account.representative in subscribe_service. * Don't required representative! * Fallback is included in Account.representative method. --- app/services/report_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/services') diff --git a/app/services/report_service.rb b/app/services/report_service.rb index 057d05ab9..1bcc1c0d5 100644 --- a/app/services/report_service.rb +++ b/app/services/report_service.rb @@ -52,6 +52,6 @@ class ReportService < BaseService end def some_local_account - @some_local_account ||= Account.local.where(suspended: false).first + @some_local_account ||= Account.representative end end -- cgit From a49d43d1121ac10f96d5a9cbf78112c707e7a59e Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sat, 5 Jan 2019 12:43:28 +0100 Subject: Add scheduled statuses (#9706) Fix #340 --- .../api/v1/scheduled_statuses_controller.rb | 77 ++++++++++ app/controllers/api/v1/statuses_controller.rb | 9 +- app/models/concerns/account_associations.rb | 1 + app/models/media_attachment.rb | 38 ++--- app/models/scheduled_status.rb | 39 +++++ .../rest/scheduled_status_serializer.rb | 11 ++ app/services/post_status_service.rb | 169 +++++++++++++++------ app/services/suspend_account_service.rb | 1 + app/workers/publish_scheduled_status_worker.rb | 24 +++ .../scheduler/scheduled_statuses_scheduler.rb | 19 +++ config/locales/en.yml | 4 + config/routes.rb | 1 + config/sidekiq.yml | 3 + .../20190103124649_create_scheduled_statuses.rb | 9 ++ ...add_scheduled_status_id_to_media_attachments.rb | 8 + db/schema.rb | 14 +- .../api/v1/conversations_controller_spec.rb | 2 +- .../api/v1/notifications_controller_spec.rb | 4 +- .../api/v1/timelines/home_controller_spec.rb | 2 +- .../api/v1/timelines/list_controller_spec.rb | 2 +- .../api/v1/timelines/public_controller_spec.rb | 4 +- .../api/v1/timelines/tag_controller_spec.rb | 2 +- spec/fabricators/scheduled_status_fabricator.rb | 4 + spec/lib/feed_manager_spec.rb | 6 +- spec/models/scheduled_status_spec.rb | 4 + .../services/batched_remove_status_service_spec.rb | 4 +- spec/services/post_status_service_spec.rb | 44 +++--- spec/services/remove_status_service_spec.rb | 2 +- .../publish_scheduled_status_worker_spec.rb | 23 +++ 29 files changed, 432 insertions(+), 98 deletions(-) create mode 100644 app/controllers/api/v1/scheduled_statuses_controller.rb create mode 100644 app/models/scheduled_status.rb create mode 100644 app/serializers/rest/scheduled_status_serializer.rb create mode 100644 app/workers/publish_scheduled_status_worker.rb create mode 100644 app/workers/scheduler/scheduled_statuses_scheduler.rb create mode 100644 db/migrate/20190103124649_create_scheduled_statuses.rb create mode 100644 db/migrate/20190103124754_add_scheduled_status_id_to_media_attachments.rb create mode 100644 spec/fabricators/scheduled_status_fabricator.rb create mode 100644 spec/models/scheduled_status_spec.rb create mode 100644 spec/workers/publish_scheduled_status_worker_spec.rb (limited to 'app/services') diff --git a/app/controllers/api/v1/scheduled_statuses_controller.rb b/app/controllers/api/v1/scheduled_statuses_controller.rb new file mode 100644 index 000000000..9950296f3 --- /dev/null +++ b/app/controllers/api/v1/scheduled_statuses_controller.rb @@ -0,0 +1,77 @@ +# frozen_string_literal: true + +class Api::V1::ScheduledStatusesController < Api::BaseController + include Authorization + + before_action -> { doorkeeper_authorize! :read, :'read:statuses' }, except: [:update, :destroy] + before_action -> { doorkeeper_authorize! :write, :'write:statuses' }, only: [:update, :destroy] + + before_action :set_statuses, only: :index + before_action :set_status, except: :index + + after_action :insert_pagination_headers, only: :index + + def index + render json: @statuses, each_serializer: REST::ScheduledStatusSerializer + end + + def show + render json: @status, serializer: REST::ScheduledStatusSerializer + end + + def update + @status.update!(scheduled_status_params) + render json: @status, serializer: REST::ScheduledStatusSerializer + end + + def destroy + @status.destroy! + render_empty + end + + private + + def set_statuses + @statuses = current_account.scheduled_statuses.paginate_by_id(limit_param(DEFAULT_STATUSES_LIMIT), params_slice(:max_id, :since_id, :min_id)) + end + + def set_status + @status = current_account.scheduled_statuses.find(params[:id]) + end + + def scheduled_status_params + params.permit(:scheduled_at) + end + + def pagination_params(core_params) + params.slice(:limit).permit(:limit).merge(core_params) + end + + def insert_pagination_headers + set_pagination_headers(next_path, prev_path) + end + + def next_path + if records_continue? + api_v1_scheduled_statuses_url pagination_params(max_id: pagination_max_id) + end + end + + def prev_path + unless @statuses.empty? + api_v1_scheduled_statuses_url pagination_params(min_id: pagination_since_id) + end + end + + def records_continue? + @statuses.size == limit_param(DEFAULT_STATUSES_LIMIT) + end + + def pagination_max_id + @statuses.last.id + end + + def pagination_since_id + @statuses.first.id + end +end diff --git a/app/controllers/api/v1/statuses_controller.rb b/app/controllers/api/v1/statuses_controller.rb index 49a52f7a6..29b420c67 100644 --- a/app/controllers/api/v1/statuses_controller.rb +++ b/app/controllers/api/v1/statuses_controller.rb @@ -45,16 +45,17 @@ class Api::V1::StatusesController < Api::BaseController def create @status = PostStatusService.new.call(current_user.account, - status_params[:status], - status_params[:in_reply_to_id].blank? ? nil : Status.find(status_params[:in_reply_to_id]), + text: status_params[:status], + thread: status_params[:in_reply_to_id].blank? ? nil : Status.find(status_params[:in_reply_to_id]), media_ids: status_params[:media_ids], sensitive: status_params[:sensitive], spoiler_text: status_params[:spoiler_text], visibility: status_params[:visibility], + scheduled_at: status_params[:scheduled_at], application: doorkeeper_token.application, idempotency: request.headers['Idempotency-Key']) - render json: @status, serializer: REST::StatusSerializer + render json: @status, serializer: @status.is_a?(ScheduledStatus) ? REST::ScheduledStatusSerializer : REST::StatusSerializer end def destroy @@ -77,7 +78,7 @@ class Api::V1::StatusesController < Api::BaseController end def status_params - params.permit(:status, :in_reply_to_id, :sensitive, :spoiler_text, :visibility, media_ids: []) + params.permit(:status, :in_reply_to_id, :sensitive, :spoiler_text, :visibility, :scheduled_at, media_ids: []) end def pagination_params(core_params) diff --git a/app/models/concerns/account_associations.rb b/app/models/concerns/account_associations.rb index a894b5eed..7dafeee34 100644 --- a/app/models/concerns/account_associations.rb +++ b/app/models/concerns/account_associations.rb @@ -14,6 +14,7 @@ module AccountAssociations has_many :mentions, inverse_of: :account, dependent: :destroy has_many :notifications, inverse_of: :account, dependent: :destroy has_many :conversations, class_name: 'AccountConversation', dependent: :destroy, inverse_of: :account + has_many :scheduled_statuses, inverse_of: :account, dependent: :destroy # Pinned statuses has_many :status_pins, inverse_of: :account, dependent: :destroy diff --git a/app/models/media_attachment.rb b/app/models/media_attachment.rb index 62a11185a..6b939124f 100644 --- a/app/models/media_attachment.rb +++ b/app/models/media_attachment.rb @@ -3,20 +3,21 @@ # # Table name: media_attachments # -# id :bigint(8) not null, primary key -# status_id :bigint(8) -# file_file_name :string -# file_content_type :string -# file_file_size :integer -# file_updated_at :datetime -# remote_url :string default(""), not null -# created_at :datetime not null -# updated_at :datetime not null -# shortcode :string -# type :integer default("image"), not null -# file_meta :json -# account_id :bigint(8) -# description :text +# id :bigint(8) not null, primary key +# status_id :bigint(8) +# file_file_name :string +# file_content_type :string +# file_file_size :integer +# file_updated_at :datetime +# remote_url :string default(""), not null +# created_at :datetime not null +# updated_at :datetime not null +# shortcode :string +# type :integer default("image"), not null +# file_meta :json +# account_id :bigint(8) +# description :text +# scheduled_status_id :bigint(8) # class MediaAttachment < ApplicationRecord @@ -76,8 +77,9 @@ class MediaAttachment < ApplicationRecord IMAGE_LIMIT = 8.megabytes VIDEO_LIMIT = 40.megabytes - belongs_to :account, inverse_of: :media_attachments, optional: true - belongs_to :status, inverse_of: :media_attachments, optional: true + belongs_to :account, inverse_of: :media_attachments, optional: true + belongs_to :status, inverse_of: :media_attachments, optional: true + belongs_to :scheduled_status, inverse_of: :media_attachments, optional: true has_attached_file :file, styles: ->(f) { file_styles f }, @@ -94,8 +96,8 @@ class MediaAttachment < ApplicationRecord validates :account, presence: true validates :description, length: { maximum: 420 }, if: :local? - scope :attached, -> { where.not(status_id: nil) } - scope :unattached, -> { where(status_id: nil) } + scope :attached, -> { where.not(status_id: nil).or(where.not(scheduled_status_id: nil)) } + scope :unattached, -> { where(status_id: nil, scheduled_status_id: nil) } scope :local, -> { where(remote_url: '') } scope :remote, -> { where.not(remote_url: '') } diff --git a/app/models/scheduled_status.rb b/app/models/scheduled_status.rb new file mode 100644 index 000000000..c95470fc8 --- /dev/null +++ b/app/models/scheduled_status.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +# == Schema Information +# +# Table name: scheduled_statuses +# +# id :bigint(8) not null, primary key +# account_id :bigint(8) +# scheduled_at :datetime +# params :jsonb +# + +class ScheduledStatus < ApplicationRecord + include Paginable + + TOTAL_LIMIT = 300 + DAILY_LIMIT = 25 + + belongs_to :account, inverse_of: :scheduled_statuses + has_many :media_attachments, inverse_of: :scheduled_status, dependent: :destroy + + validate :validate_future_date + validate :validate_total_limit + validate :validate_daily_limit + + private + + def validate_future_date + errors.add(:scheduled_at, I18n.t('scheduled_statuses.too_soon')) if scheduled_at.present? && scheduled_at <= Time.now.utc + PostStatusService::MIN_SCHEDULE_OFFSET + end + + def validate_total_limit + errors.add(:base, I18n.t('scheduled_statuses.over_total_limit', limit: TOTAL_LIMIT)) if account.scheduled_statuses.count >= TOTAL_LIMIT + end + + def validate_daily_limit + errors.add(:base, I18n.t('scheduled_statuses.over_daily_limit', limit: DAILY_LIMIT)) if account.scheduled_statuses.where('scheduled_at::date = ?::date', scheduled_at).count >= DAILY_LIMIT + end +end diff --git a/app/serializers/rest/scheduled_status_serializer.rb b/app/serializers/rest/scheduled_status_serializer.rb new file mode 100644 index 000000000..522991bcf --- /dev/null +++ b/app/serializers/rest/scheduled_status_serializer.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class REST::ScheduledStatusSerializer < ActiveModel::Serializer + attributes :id, :scheduled_at + + has_many :media_attachments, serializer: REST::MediaAttachmentSerializer + + def id + object.id.to_s + end +end diff --git a/app/services/post_status_service.rb b/app/services/post_status_service.rb index eff1b1461..07fd969e5 100644 --- a/app/services/post_status_service.rb +++ b/app/services/post_status_service.rb @@ -1,71 +1,96 @@ # frozen_string_literal: true class PostStatusService < BaseService + MIN_SCHEDULE_OFFSET = 5.minutes.freeze + # Post a text status update, fetch and notify remote users mentioned # @param [Account] account Account from which to post - # @param [String] text Message - # @param [Status] in_reply_to Optional status to reply to # @param [Hash] options + # @option [String] :text Message + # @option [Status] :thread Optional status to reply to # @option [Boolean] :sensitive # @option [String] :visibility # @option [String] :spoiler_text + # @option [String] :language + # @option [String] :scheduled_at # @option [Enumerable] :media_ids Optional array of media IDs to attach # @option [Doorkeeper::Application] :application # @option [String] :idempotency Optional idempotency key # @return [Status] - def call(account, text, in_reply_to = nil, **options) - if options[:idempotency].present? - existing_id = redis.get("idempotency:status:#{account.id}:#{options[:idempotency]}") - return Status.find(existing_id) if existing_id + def call(account, options = {}) + @account = account + @options = options + @text = @options[:text] || '' + @in_reply_to = @options[:thread] + + return idempotency_duplicate if idempotency_given? && idempotency_duplicate? + + validate_media! + preprocess_attributes! + + if scheduled? + schedule_status! + else + process_status! + postprocess_status! + bump_potential_friendship! end - media = validate_media!(options[:media_ids]) - status = nil - text = options.delete(:spoiler_text) if text.blank? && options[:spoiler_text].present? + redis.setex(idempotency_key, 3_600, @status.id) if idempotency_given? - visibility = options[:visibility] || account.user&.setting_default_privacy - visibility = :unlisted if visibility == :public && account.silenced + @status + end - ApplicationRecord.transaction do - status = account.statuses.create!(text: text, - media_attachments: media || [], - thread: in_reply_to, - sensitive: (options[:sensitive].nil? ? account.user&.setting_default_sensitive : options[:sensitive]) || options[:spoiler_text].present?, - spoiler_text: options[:spoiler_text] || '', - visibility: visibility, - language: language_from_option(options[:language]) || account.user&.setting_default_language&.presence || LanguageDetector.instance.detect(text, account), - application: options[:application]) - end + private - process_hashtags_service.call(status) - process_mentions_service.call(status) + def preprocess_attributes! + @text = @options.delete(:spoiler_text) if @text.blank? && @options[:spoiler_text].present? + @visibility = @options[:visibility] || @account.user&.setting_default_privacy + @visibility = :unlisted if @visibility == :public && @account.silenced + @scheduled_at = @options[:scheduled_at]&.to_datetime + @scheduled_at = nil if scheduled_in_the_past? + end - LinkCrawlWorker.perform_async(status.id) unless status.spoiler_text? - DistributionWorker.perform_async(status.id) - Pubsubhubbub::DistributionWorker.perform_async(status.stream_entry.id) - ActivityPub::DistributionWorker.perform_async(status.id) + def process_status! + # The following transaction block is needed to wrap the UPDATEs to + # the media attachments when the status is created - if options[:idempotency].present? - redis.setex("idempotency:status:#{account.id}:#{options[:idempotency]}", 3_600, status.id) + ApplicationRecord.transaction do + @status = @account.statuses.create!(status_attributes) end - bump_potential_friendship(account, status) - - status + process_hashtags_service.call(@status) + process_mentions_service.call(@status) end - private + def schedule_status! + if @account.statuses.build(status_attributes).valid? + # The following transaction block is needed to wrap the UPDATEs to + # the media attachments when the scheduled status is created - def validate_media!(media_ids) - return if media_ids.blank? || !media_ids.is_a?(Enumerable) + ApplicationRecord.transaction do + @status = @account.scheduled_statuses.create!(scheduled_status_attributes) + end + else + raise ActiveRecord::RecordInvalid + end + end + + def postprocess_status! + LinkCrawlWorker.perform_async(@status.id) unless @status.spoiler_text? + DistributionWorker.perform_async(@status.id) + Pubsubhubbub::DistributionWorker.perform_async(@status.stream_entry.id) + ActivityPub::DistributionWorker.perform_async(@status.id) + end - raise Mastodon::ValidationError, I18n.t('media_attachments.validations.too_many') if media_ids.size > 4 + def validate_media! + return if @options[:media_ids].blank? || !@options[:media_ids].is_a?(Enumerable) - media = MediaAttachment.where(status_id: nil).where(id: media_ids.take(4).map(&:to_i)) + raise Mastodon::ValidationError, I18n.t('media_attachments.validations.too_many') if @options[:media_ids].size > 4 - raise Mastodon::ValidationError, I18n.t('media_attachments.validations.images_and_video') if media.size > 1 && media.find(&:video?) + @media = MediaAttachment.where(status_id: nil).where(id: @options[:media_ids].take(4).map(&:to_i)) - media + raise Mastodon::ValidationError, I18n.t('media_attachments.validations.images_and_video') if @media.size > 1 && @media.find(&:video?) end def language_from_option(str) @@ -84,10 +109,68 @@ class PostStatusService < BaseService Redis.current end - def bump_potential_friendship(account, status) - return if !status.reply? || account.id == status.in_reply_to_account_id + def scheduled? + @scheduled_at.present? + end + + def idempotency_key + "idempotency:status:#{@account.id}:#{@options[:idempotency]}" + end + + def idempotency_given? + @options[:idempotency].present? + end + + def idempotency_duplicate + if scheduled? + @account.schedule_statuses.find(@idempotency_duplicate) + else + @account.statuses.find(@idempotency_duplicate) + end + end + + def idempotency_duplicate? + @idempotency_duplicate = redis.get(idempotency_key) + end + + def scheduled_in_the_past? + @scheduled_at.present? && @scheduled_at <= Time.now.utc + MIN_SCHEDULE_OFFSET + end + + def bump_potential_friendship! + return if !@status.reply? || @account.id == @status.in_reply_to_account_id ActivityTracker.increment('activity:interactions') - return if account.following?(status.in_reply_to_account_id) - PotentialFriendshipTracker.record(account.id, status.in_reply_to_account_id, :reply) + return if @account.following?(@status.in_reply_to_account_id) + PotentialFriendshipTracker.record(@account.id, @status.in_reply_to_account_id, :reply) + end + + def status_attributes + { + text: @text, + media_attachments: @media || [], + thread: @in_reply_to, + sensitive: (@options[:sensitive].nil? ? @account.user&.setting_default_sensitive : @options[:sensitive]) || @options[:spoiler_text].present?, + spoiler_text: @options[:spoiler_text] || '', + visibility: @visibility, + language: language_from_option(@options[:language]) || @account.user&.setting_default_language&.presence || LanguageDetector.instance.detect(@text, @account), + application: @options[:application], + } + end + + def scheduled_status_attributes + { + scheduled_at: @scheduled_at, + media_attachments: @media || [], + params: scheduled_options, + } + end + + def scheduled_options + @options.tap do |options_hash| + options_hash[:in_reply_to_status_id] = options_hash.delete(:thread)&.id + options_hash[:application_id] = options_hash.delete(:application)&.id + options_hash[:scheduled_at] = nil + options_hash[:idempotency] = nil + end end end diff --git a/app/services/suspend_account_service.rb b/app/services/suspend_account_service.rb index 6ab6b2901..1bc2314de 100644 --- a/app/services/suspend_account_service.rb +++ b/app/services/suspend_account_service.rb @@ -20,6 +20,7 @@ class SuspendAccountService < BaseService owned_lists passive_relationships report_notes + scheduled_statuses status_pins stream_entries subscriptions diff --git a/app/workers/publish_scheduled_status_worker.rb b/app/workers/publish_scheduled_status_worker.rb new file mode 100644 index 000000000..298a13001 --- /dev/null +++ b/app/workers/publish_scheduled_status_worker.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +class PublishScheduledStatusWorker + include Sidekiq::Worker + + def perform(scheduled_status_id) + scheduled_status = ScheduledStatus.find(scheduled_status_id) + scheduled_status.destroy! + + PostStatusService.new.call( + scheduled_status.account, + options_with_objects(scheduled_status.params.with_indifferent_access) + ) + rescue ActiveRecord::RecordNotFound, ActiveRecord::RecordInvalid + true + end + + def options_with_objects(options) + options.tap do |options_hash| + options_hash[:application] = Doorkeeper::Application.find(options_hash.delete(:application_id)) if options[:application_id] + options_hash[:thread] = Status.find(options_hash.delete(:in_reply_to_status_id)) if options_hash[:in_reply_to_status_id] + end + end +end diff --git a/app/workers/scheduler/scheduled_statuses_scheduler.rb b/app/workers/scheduler/scheduled_statuses_scheduler.rb new file mode 100644 index 000000000..70a45846b --- /dev/null +++ b/app/workers/scheduler/scheduled_statuses_scheduler.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class Scheduler::ScheduledStatusesScheduler + include Sidekiq::Worker + + sidekiq_options unique: :until_executed, retry: 0 + + def perform + due_statuses.find_each do |scheduled_status| + PublishScheduledStatusWorker.perform_at(scheduled_status.scheduled_at) + end + end + + private + + def due_statuses + ScheduledStatus.where('scheduled_at <= ?', Time.now.utc + PostStatusService::MIN_SCHEDULE_OFFSET) + end +end diff --git a/config/locales/en.yml b/config/locales/en.yml index 6c78b9fc9..7e05568f1 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -728,6 +728,10 @@ en: error: Error title: Title unfollowed: Unfollowed + scheduled_statuses: + over_daily_limit: You have exceeded the limit of %{limit} scheduled toots for that day + over_total_limit: You have exceeded the limit of %{limit} scheduled toots + too_soon: The scheduled date must be in the future sessions: activity: Last activity browser: Browser diff --git a/config/routes.rb b/config/routes.rb index 5bdd1986c..3ae2735d1 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -283,6 +283,7 @@ Rails.application.routes.draw do resources :streaming, only: [:index] resources :custom_emojis, only: [:index] resources :suggestions, only: [:index, :destroy] + resources :scheduled_statuses, only: [:index, :show, :update, :destroy] resources :conversations, only: [:index, :destroy] do member do diff --git a/config/sidekiq.yml b/config/sidekiq.yml index c44af5b6c..0ec1742ab 100644 --- a/config/sidekiq.yml +++ b/config/sidekiq.yml @@ -6,6 +6,9 @@ - [mailers, 2] - [pull] :schedule: + scheduled_statuses_scheduler: + every: '5m' + class: Scheduler::ScheduledStatusesScheduler subscriptions_scheduler: cron: '<%= Random.rand(0..59) %> <%= Random.rand(4..6) %> * * *' class: Scheduler::SubscriptionsScheduler diff --git a/db/migrate/20190103124649_create_scheduled_statuses.rb b/db/migrate/20190103124649_create_scheduled_statuses.rb new file mode 100644 index 000000000..2b78073b8 --- /dev/null +++ b/db/migrate/20190103124649_create_scheduled_statuses.rb @@ -0,0 +1,9 @@ +class CreateScheduledStatuses < ActiveRecord::Migration[5.2] + def change + create_table :scheduled_statuses do |t| + t.belongs_to :account, foreign_key: { on_delete: :cascade } + t.datetime :scheduled_at, index: true + t.jsonb :params + end + end +end diff --git a/db/migrate/20190103124754_add_scheduled_status_id_to_media_attachments.rb b/db/migrate/20190103124754_add_scheduled_status_id_to_media_attachments.rb new file mode 100644 index 000000000..6f6cf2351 --- /dev/null +++ b/db/migrate/20190103124754_add_scheduled_status_id_to_media_attachments.rb @@ -0,0 +1,8 @@ +class AddScheduledStatusIdToMediaAttachments < ActiveRecord::Migration[5.2] + disable_ddl_transaction! + + def change + add_reference :media_attachments, :scheduled_status, foreign_key: { on_delete: :nullify }, index: false + add_index :media_attachments, :scheduled_status_id, algorithm: :concurrently + end +end diff --git a/db/schema.rb b/db/schema.rb index 066a90b52..9380362e1 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: 2018_12_26_021420) do +ActiveRecord::Schema.define(version: 2019_01_03_124754) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -336,7 +336,9 @@ ActiveRecord::Schema.define(version: 2018_12_26_021420) do t.json "file_meta" t.bigint "account_id" t.text "description" + t.bigint "scheduled_status_id" t.index ["account_id"], name: "index_media_attachments_on_account_id" + t.index ["scheduled_status_id"], name: "index_media_attachments_on_scheduled_status_id" t.index ["shortcode"], name: "index_media_attachments_on_shortcode", unique: true t.index ["status_id"], name: "index_media_attachments_on_status_id" end @@ -487,6 +489,14 @@ ActiveRecord::Schema.define(version: 2018_12_26_021420) do t.index ["target_account_id"], name: "index_reports_on_target_account_id" end + create_table "scheduled_statuses", force: :cascade do |t| + t.bigint "account_id" + t.datetime "scheduled_at" + t.jsonb "params" + t.index ["account_id"], name: "index_scheduled_statuses_on_account_id" + t.index ["scheduled_at"], name: "index_scheduled_statuses_on_scheduled_at" + end + create_table "session_activations", force: :cascade do |t| t.string "session_id", null: false t.datetime "created_at", null: false @@ -700,6 +710,7 @@ ActiveRecord::Schema.define(version: 2018_12_26_021420) do add_foreign_key "list_accounts", "lists", on_delete: :cascade add_foreign_key "lists", "accounts", on_delete: :cascade add_foreign_key "media_attachments", "accounts", name: "fk_96dd81e81b", on_delete: :nullify + add_foreign_key "media_attachments", "scheduled_statuses", on_delete: :nullify add_foreign_key "media_attachments", "statuses", on_delete: :nullify add_foreign_key "mentions", "accounts", name: "fk_970d43f9d1", on_delete: :cascade add_foreign_key "mentions", "statuses", on_delete: :cascade @@ -718,6 +729,7 @@ ActiveRecord::Schema.define(version: 2018_12_26_021420) do add_foreign_key "reports", "accounts", column: "assigned_account_id", on_delete: :nullify add_foreign_key "reports", "accounts", column: "target_account_id", name: "fk_eb37af34f0", on_delete: :cascade add_foreign_key "reports", "accounts", name: "fk_4b81f7522c", on_delete: :cascade + add_foreign_key "scheduled_statuses", "accounts", on_delete: :cascade add_foreign_key "session_activations", "oauth_access_tokens", column: "access_token_id", name: "fk_957e5bda89", on_delete: :cascade add_foreign_key "session_activations", "users", name: "fk_e5fda67334", on_delete: :cascade add_foreign_key "status_pins", "accounts", name: "fk_d4cb435b62", on_delete: :cascade diff --git a/spec/controllers/api/v1/conversations_controller_spec.rb b/spec/controllers/api/v1/conversations_controller_spec.rb index 2e9525855..070f65061 100644 --- a/spec/controllers/api/v1/conversations_controller_spec.rb +++ b/spec/controllers/api/v1/conversations_controller_spec.rb @@ -15,7 +15,7 @@ RSpec.describe Api::V1::ConversationsController, type: :controller do let(:scopes) { 'read:statuses' } before do - PostStatusService.new.call(other.account, 'Hey @alice', nil, visibility: 'direct') + PostStatusService.new.call(other.account, text: 'Hey @alice', visibility: 'direct') end it 'returns http success' do diff --git a/spec/controllers/api/v1/notifications_controller_spec.rb b/spec/controllers/api/v1/notifications_controller_spec.rb index 9f679cb8a..d0f82e79f 100644 --- a/spec/controllers/api/v1/notifications_controller_spec.rb +++ b/spec/controllers/api/v1/notifications_controller_spec.rb @@ -50,9 +50,9 @@ RSpec.describe Api::V1::NotificationsController, type: :controller do let(:scopes) { 'read:notifications' } before do - first_status = PostStatusService.new.call(user.account, 'Test') + first_status = PostStatusService.new.call(user.account, text: 'Test') @reblog_of_first_status = ReblogService.new.call(other.account, first_status) - mentioning_status = PostStatusService.new.call(other.account, 'Hello @alice') + mentioning_status = PostStatusService.new.call(other.account, text: 'Hello @alice') @mention_from_status = mentioning_status.mentions.first @favourite = FavouriteService.new.call(other.account, first_status) @follow = FollowService.new.call(other.account, 'alice') diff --git a/spec/controllers/api/v1/timelines/home_controller_spec.rb b/spec/controllers/api/v1/timelines/home_controller_spec.rb index 63d624c35..e953e4649 100644 --- a/spec/controllers/api/v1/timelines/home_controller_spec.rb +++ b/spec/controllers/api/v1/timelines/home_controller_spec.rb @@ -17,7 +17,7 @@ describe Api::V1::Timelines::HomeController do describe 'GET #show' do before do follow = Fabricate(:follow, account: user.account) - PostStatusService.new.call(follow.target_account, 'New status for user home timeline.') + PostStatusService.new.call(follow.target_account, text: 'New status for user home timeline.') end it 'returns http success' do diff --git a/spec/controllers/api/v1/timelines/list_controller_spec.rb b/spec/controllers/api/v1/timelines/list_controller_spec.rb index 93a2be6e6..45e4bf34c 100644 --- a/spec/controllers/api/v1/timelines/list_controller_spec.rb +++ b/spec/controllers/api/v1/timelines/list_controller_spec.rb @@ -19,7 +19,7 @@ describe Api::V1::Timelines::ListController do before do follow = Fabricate(:follow, account: user.account) list.accounts << follow.target_account - PostStatusService.new.call(follow.target_account, 'New status for user home timeline.') + PostStatusService.new.call(follow.target_account, text: 'New status for user home timeline.') end it 'returns http success' do diff --git a/spec/controllers/api/v1/timelines/public_controller_spec.rb b/spec/controllers/api/v1/timelines/public_controller_spec.rb index a0f778cdc..737aedba6 100644 --- a/spec/controllers/api/v1/timelines/public_controller_spec.rb +++ b/spec/controllers/api/v1/timelines/public_controller_spec.rb @@ -16,7 +16,7 @@ describe Api::V1::Timelines::PublicController do describe 'GET #show' do before do - PostStatusService.new.call(user.account, 'New status from user for federated public timeline.') + PostStatusService.new.call(user.account, text: 'New status from user for federated public timeline.') end it 'returns http success' do @@ -29,7 +29,7 @@ describe Api::V1::Timelines::PublicController do describe 'GET #show with local only' do before do - PostStatusService.new.call(user.account, 'New status from user for local public timeline.') + PostStatusService.new.call(user.account, text: 'New status from user for local public timeline.') end it 'returns http success' do diff --git a/spec/controllers/api/v1/timelines/tag_controller_spec.rb b/spec/controllers/api/v1/timelines/tag_controller_spec.rb index 472779f54..f71ca2a39 100644 --- a/spec/controllers/api/v1/timelines/tag_controller_spec.rb +++ b/spec/controllers/api/v1/timelines/tag_controller_spec.rb @@ -16,7 +16,7 @@ describe Api::V1::Timelines::TagController do describe 'GET #show' do before do - PostStatusService.new.call(user.account, 'It is a #test') + PostStatusService.new.call(user.account, text: 'It is a #test') end it 'returns http success' do diff --git a/spec/fabricators/scheduled_status_fabricator.rb b/spec/fabricators/scheduled_status_fabricator.rb new file mode 100644 index 000000000..52384d137 --- /dev/null +++ b/spec/fabricators/scheduled_status_fabricator.rb @@ -0,0 +1,4 @@ +Fabricator(:scheduled_status) do + account + scheduled_at { 20.hours.from_now } +end diff --git a/spec/lib/feed_manager_spec.rb b/spec/lib/feed_manager_spec.rb index 64e109aec..c506cd87f 100644 --- a/spec/lib/feed_manager_spec.rb +++ b/spec/lib/feed_manager_spec.rb @@ -108,14 +108,14 @@ RSpec.describe FeedManager do it 'returns false for status by followee mentioning another account' do bob.follow!(alice) - status = PostStatusService.new.call(alice, 'Hey @jeff') + status = PostStatusService.new.call(alice, text: 'Hey @jeff') expect(FeedManager.instance.filter?(:home, status, bob.id)).to be false end it 'returns true for status by followee mentioning blocked account' do bob.block!(jeff) bob.follow!(alice) - status = PostStatusService.new.call(alice, 'Hey @jeff') + status = PostStatusService.new.call(alice, text: 'Hey @jeff') expect(FeedManager.instance.filter?(:home, status, bob.id)).to be true end @@ -155,7 +155,7 @@ RSpec.describe FeedManager do context 'for mentions feed' do it 'returns true for status that mentions blocked account' do bob.block!(jeff) - status = PostStatusService.new.call(alice, 'Hey @jeff') + status = PostStatusService.new.call(alice, text: 'Hey @jeff') expect(FeedManager.instance.filter?(:mentions, status, bob.id)).to be true end diff --git a/spec/models/scheduled_status_spec.rb b/spec/models/scheduled_status_spec.rb new file mode 100644 index 000000000..f8c9d8b81 --- /dev/null +++ b/spec/models/scheduled_status_spec.rb @@ -0,0 +1,4 @@ +require 'rails_helper' + +RSpec.describe ScheduledStatus, type: :model do +end diff --git a/spec/services/batched_remove_status_service_spec.rb b/spec/services/batched_remove_status_service_spec.rb index c66214555..e53623449 100644 --- a/spec/services/batched_remove_status_service_spec.rb +++ b/spec/services/batched_remove_status_service_spec.rb @@ -8,8 +8,8 @@ RSpec.describe BatchedRemoveStatusService, type: :service do let!(:jeff) { Fabricate(:user).account } let!(:hank) { Fabricate(:account, username: 'hank', protocol: :activitypub, domain: 'example.com', inbox_url: 'http://example.com/inbox') } - let(:status1) { PostStatusService.new.call(alice, 'Hello @bob@example.com') } - let(:status2) { PostStatusService.new.call(alice, 'Another status') } + let(:status1) { PostStatusService.new.call(alice, text: 'Hello @bob@example.com') } + let(:status2) { PostStatusService.new.call(alice, text: 'Another status') } before do allow(Redis.current).to receive_messages(publish: nil) diff --git a/spec/services/post_status_service_spec.rb b/spec/services/post_status_service_spec.rb index 8f3552224..3774fed6f 100644 --- a/spec/services/post_status_service_spec.rb +++ b/spec/services/post_status_service_spec.rb @@ -7,7 +7,7 @@ RSpec.describe PostStatusService, type: :service do account = Fabricate(:account) text = "test status update" - status = subject.call(account, text) + status = subject.call(account, text: text) expect(status).to be_persisted expect(status.text).to eq text @@ -18,20 +18,31 @@ RSpec.describe PostStatusService, type: :service do account = Fabricate(:account) text = "test status update" - status = subject.call(account, text, in_reply_to_status) + status = subject.call(account, text: text, thread: in_reply_to_status) expect(status).to be_persisted expect(status.text).to eq text expect(status.thread).to eq in_reply_to_status end + it 'schedules a status' do + account = Fabricate(:account) + future = Time.now.utc + 2.hours + + status = subject.call(account, text: 'Hi future!', scheduled_at: future) + + expect(status).to be_a ScheduledStatus + expect(status.scheduled_at).to eq future + expect(status.params['text']).to eq 'Hi future!' + end + it 'creates response to the original status of boost' do boosted_status = Fabricate(:status) in_reply_to_status = Fabricate(:status, reblog: boosted_status) account = Fabricate(:account) text = "test status update" - status = subject.call(account, text, in_reply_to_status) + status = subject.call(account, text: text, thread: in_reply_to_status) expect(status).to be_persisted expect(status.text).to eq text @@ -69,7 +80,7 @@ RSpec.describe PostStatusService, type: :service do end it 'creates a status with limited visibility for silenced users' do - status = subject.call(Fabricate(:account, silenced: true), 'test', nil, visibility: :public) + status = subject.call(Fabricate(:account, silenced: true), text: 'test', visibility: :public) expect(status).to be_persisted expect(status.visibility).to eq "unlisted" @@ -88,7 +99,7 @@ RSpec.describe PostStatusService, type: :service do account = Fabricate(:account) text = 'This is an English text.' - status = subject.call(account, text) + status = subject.call(account, text: text) expect(status.language).to eq 'en' end @@ -99,7 +110,7 @@ RSpec.describe PostStatusService, type: :service do allow(ProcessMentionsService).to receive(:new).and_return(mention_service) account = Fabricate(:account) - status = subject.call(account, "test status update") + status = subject.call(account, text: "test status update") expect(ProcessMentionsService).to have_received(:new) expect(mention_service).to have_received(:call).with(status) @@ -111,7 +122,7 @@ RSpec.describe PostStatusService, type: :service do allow(ProcessHashtagsService).to receive(:new).and_return(hashtags_service) account = Fabricate(:account) - status = subject.call(account, "test status update") + status = subject.call(account, text: "test status update") expect(ProcessHashtagsService).to have_received(:new) expect(hashtags_service).to have_received(:call).with(status) @@ -124,7 +135,7 @@ RSpec.describe PostStatusService, type: :service do account = Fabricate(:account) - status = subject.call(account, "test status update") + status = subject.call(account, text: "test status update") expect(DistributionWorker).to have_received(:perform_async).with(status.id) expect(Pubsubhubbub::DistributionWorker).to have_received(:perform_async).with(status.stream_entry.id) @@ -135,7 +146,7 @@ RSpec.describe PostStatusService, type: :service do allow(LinkCrawlWorker).to receive(:perform_async) account = Fabricate(:account) - status = subject.call(account, "test status update") + status = subject.call(account, text: "test status update") expect(LinkCrawlWorker).to have_received(:perform_async).with(status.id) end @@ -146,8 +157,7 @@ RSpec.describe PostStatusService, type: :service do status = subject.call( account, - "test status update", - nil, + text: "test status update", media_ids: [media.id], ) @@ -160,8 +170,7 @@ RSpec.describe PostStatusService, type: :service do expect do subject.call( account, - "test status update", - nil, + text: "test status update", media_ids: [ Fabricate(:media_attachment, account: account), Fabricate(:media_attachment, account: account), @@ -182,8 +191,7 @@ RSpec.describe PostStatusService, type: :service do expect do subject.call( account, - "test status update", - nil, + text: "test status update", media_ids: [ Fabricate(:media_attachment, type: :video, account: account), Fabricate(:media_attachment, type: :image, account: account), @@ -197,12 +205,12 @@ RSpec.describe PostStatusService, type: :service do it 'returns existing status when used twice with idempotency key' do account = Fabricate(:account) - status1 = subject.call(account, 'test', nil, idempotency: 'meepmeep') - status2 = subject.call(account, 'test', nil, idempotency: 'meepmeep') + status1 = subject.call(account, text: 'test', idempotency: 'meepmeep') + status2 = subject.call(account, text: 'test', idempotency: 'meepmeep') expect(status2.id).to eq status1.id end def create_status_with_options(**options) - subject.call(Fabricate(:account), 'test', nil, options) + subject.call(Fabricate(:account), options.merge(text: 'test')) end end diff --git a/spec/services/remove_status_service_spec.rb b/spec/services/remove_status_service_spec.rb index 2134f51fd..7bba83a60 100644 --- a/spec/services/remove_status_service_spec.rb +++ b/spec/services/remove_status_service_spec.rb @@ -19,7 +19,7 @@ RSpec.describe RemoveStatusService, type: :service do jeff.follow!(alice) hank.follow!(alice) - @status = PostStatusService.new.call(alice, 'Hello @bob@example.com') + @status = PostStatusService.new.call(alice, text: 'Hello @bob@example.com') Fabricate(:status, account: bill, reblog: @status, uri: 'hoge') subject.call(@status) end diff --git a/spec/workers/publish_scheduled_status_worker_spec.rb b/spec/workers/publish_scheduled_status_worker_spec.rb new file mode 100644 index 000000000..f8547e6fe --- /dev/null +++ b/spec/workers/publish_scheduled_status_worker_spec.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe PublishScheduledStatusWorker do + subject { described_class.new } + + let(:scheduled_status) { Fabricate(:scheduled_status, params: { text: 'Hello world, future!' }) } + + describe 'perform' do + before do + subject.perform(scheduled_status.id) + end + + it 'creates a status' do + expect(scheduled_status.account.statuses.first.text).to eq 'Hello world, future!' + end + + it 'removes the scheduled status' do + expect(ScheduledStatus.find_by(id: scheduled_status.id)).to be_nil + end + end +end -- cgit From 1cbdf8d218dcd07889d14e4f5f22a4339bddb879 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sun, 6 Jan 2019 12:03:27 +0100 Subject: Fix wrong param name in scheduled statuses and return params in API (#9725) The database column and API param are called in_reply_to_id, not in_reply_to_status_id, so it makes no sense to encode it that way --- app/serializers/rest/scheduled_status_serializer.rb | 6 +++++- app/services/post_status_service.rb | 8 ++++---- app/workers/publish_scheduled_status_worker.rb | 2 +- 3 files changed, 10 insertions(+), 6 deletions(-) (limited to 'app/services') diff --git a/app/serializers/rest/scheduled_status_serializer.rb b/app/serializers/rest/scheduled_status_serializer.rb index 522991bcf..5d6311b87 100644 --- a/app/serializers/rest/scheduled_status_serializer.rb +++ b/app/serializers/rest/scheduled_status_serializer.rb @@ -1,11 +1,15 @@ # frozen_string_literal: true class REST::ScheduledStatusSerializer < ActiveModel::Serializer - attributes :id, :scheduled_at + attributes :id, :scheduled_at, :params has_many :media_attachments, serializer: REST::MediaAttachmentSerializer def id object.id.to_s end + + def params + object.params.without(:application_id) + end end diff --git a/app/services/post_status_service.rb b/app/services/post_status_service.rb index 07fd969e5..260765edf 100644 --- a/app/services/post_status_service.rb +++ b/app/services/post_status_service.rb @@ -167,10 +167,10 @@ class PostStatusService < BaseService def scheduled_options @options.tap do |options_hash| - options_hash[:in_reply_to_status_id] = options_hash.delete(:thread)&.id - options_hash[:application_id] = options_hash.delete(:application)&.id - options_hash[:scheduled_at] = nil - options_hash[:idempotency] = nil + options_hash[:in_reply_to_id] = options_hash.delete(:thread)&.id + options_hash[:application_id] = options_hash.delete(:application)&.id + options_hash[:scheduled_at] = nil + options_hash[:idempotency] = nil end end end diff --git a/app/workers/publish_scheduled_status_worker.rb b/app/workers/publish_scheduled_status_worker.rb index 298a13001..641fcc61c 100644 --- a/app/workers/publish_scheduled_status_worker.rb +++ b/app/workers/publish_scheduled_status_worker.rb @@ -18,7 +18,7 @@ class PublishScheduledStatusWorker def options_with_objects(options) options.tap do |options_hash| options_hash[:application] = Doorkeeper::Application.find(options_hash.delete(:application_id)) if options[:application_id] - options_hash[:thread] = Status.find(options_hash.delete(:in_reply_to_status_id)) if options_hash[:in_reply_to_status_id] + options_hash[:thread] = Status.find(options_hash.delete(:in_reply_to_id)) if options_hash[:in_reply_to_id] end end end -- cgit From 43c61bca60016cad5d3fae210fd57622b40225a8 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Mon, 7 Jan 2019 14:50:20 +0100 Subject: Add locale param to sign-up API (#9747) Fix #9627 --- app/controllers/api/v1/accounts_controller.rb | 2 +- app/services/app_sign_up_service.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'app/services') diff --git a/app/controllers/api/v1/accounts_controller.rb b/app/controllers/api/v1/accounts_controller.rb index 6e4084c4e..2ccbc3cbb 100644 --- a/app/controllers/api/v1/accounts_controller.rb +++ b/app/controllers/api/v1/accounts_controller.rb @@ -76,7 +76,7 @@ class Api::V1::AccountsController < Api::BaseController end def account_params - params.permit(:username, :email, :password, :agreement) + params.permit(:username, :email, :password, :agreement, :locale) end def check_enabled_registrations diff --git a/app/services/app_sign_up_service.rb b/app/services/app_sign_up_service.rb index 1878587e8..d621cc462 100644 --- a/app/services/app_sign_up_service.rb +++ b/app/services/app_sign_up_service.rb @@ -4,7 +4,7 @@ class AppSignUpService < BaseService def call(app, params) return unless allowed_registrations? - user_params = params.slice(:email, :password, :agreement) + user_params = params.slice(:email, :password, :agreement, :locale) account_params = params.slice(:username) user = User.create!(user_params.merge(created_by_application: app, password_confirmation: user_params[:password], account_attributes: account_params)) -- cgit From 28b482874ab4393639a77fdd895658096bcbfd57 Mon Sep 17 00:00:00 2001 From: ThibG Date: Mon, 7 Jan 2019 21:45:13 +0100 Subject: Improvements to signature verification (#9667) * Refactor signature verification a bit * Rescue signature verification if recorded public key is invalid Fixes #8822 * Always re-fetch AP signing key when HTTP Signature verification fails But when the account is not marked as stale, avoid fetching collections and media, and avoid webfinger round-trip. * Apply stoplight to key/account update as well as initial key retrieval --- app/controllers/concerns/signature_verification.rb | 45 +++++++++++++++------- .../activitypub/fetch_remote_account_service.rb | 8 ++-- .../activitypub/process_account_service.rb | 10 +++-- 3 files changed, 41 insertions(+), 22 deletions(-) (limited to 'app/services') diff --git a/app/controllers/concerns/signature_verification.rb b/app/controllers/concerns/signature_verification.rb index 887096e8b..91566c4fa 100644 --- a/app/controllers/concerns/signature_verification.rb +++ b/app/controllers/concerns/signature_verification.rb @@ -60,23 +60,26 @@ module SignatureVerification signature = Base64.decode64(signature_params['signature']) compare_signed_string = build_signed_string(signature_params['headers']) - if account.keypair.public_key.verify(OpenSSL::Digest::SHA256.new, signature, compare_signed_string) - @signed_request_account = account - @signed_request_account - elsif account.possibly_stale? - account = account.refresh! + return account unless verify_signature(account, signature, compare_signed_string).nil? - if account.keypair.public_key.verify(OpenSSL::Digest::SHA256.new, signature, compare_signed_string) - @signed_request_account = account - @signed_request_account - else - @signature_verification_failure_reason = "Verification failed for #{account.username}@#{account.domain} #{account.uri}" - @signed_request_account = nil - end - else - @signature_verification_failure_reason = "Verification failed for #{account.username}@#{account.domain} #{account.uri}" + account_stoplight = Stoplight("source:#{request.ip}") { account.possibly_stale? ? account.refresh! : account_refresh_key(account) } + .with_fallback { nil } + .with_threshold(1) + .with_cool_off_time(5.minutes.seconds) + .with_error_handler { |error, handle| error.is_a?(HTTP::Error) ? handle.call(error) : raise(error) } + + account = account_stoplight.run + + if account.nil? + @signature_verification_failure_reason = "Public key not found for key #{signature_params['keyId']}" @signed_request_account = nil + return end + + return account unless verify_signature(account, signature, compare_signed_string).nil? + + @signature_verification_failure_reason = "Verification failed for #{account.username}@#{account.domain} #{account.uri}" + @signed_request_account = nil end def request_body @@ -85,6 +88,15 @@ module SignatureVerification private + def verify_signature(account, signature, compare_signed_string) + if account.keypair.public_key.verify(OpenSSL::Digest::SHA256.new, signature, compare_signed_string) + @signed_request_account = account + @signed_request_account + end + rescue OpenSSL::PKey::RSAError + nil + end + def build_signed_string(signed_headers) signed_headers = 'date' if signed_headers.blank? @@ -131,4 +143,9 @@ module SignatureVerification account end end + + def account_refresh_key(account) + return if account.local? || !account.activitypub? + ActivityPub::FetchRemoteAccountService.new.call(account.uri, only_key: true) + end end diff --git a/app/services/activitypub/fetch_remote_account_service.rb b/app/services/activitypub/fetch_remote_account_service.rb index 8430d12d5..3c2044941 100644 --- a/app/services/activitypub/fetch_remote_account_service.rb +++ b/app/services/activitypub/fetch_remote_account_service.rb @@ -5,8 +5,8 @@ class ActivityPub::FetchRemoteAccountService < BaseService SUPPORTED_TYPES = %w(Application Group Organization Person Service).freeze - # Does a WebFinger roundtrip on each call - def call(uri, id: true, prefetched_body: nil, break_on_redirect: false) + # Does a WebFinger roundtrip on each call, unless `only_key` is true + def call(uri, id: true, prefetched_body: nil, break_on_redirect: false, only_key: false) return ActivityPub::TagManager.instance.uri_to_resource(uri, Account) if ActivityPub::TagManager.instance.local_uri?(uri) @json = if prefetched_body.nil? @@ -21,9 +21,9 @@ class ActivityPub::FetchRemoteAccountService < BaseService @username = @json['preferredUsername'] @domain = Addressable::URI.parse(@uri).normalized_host - return unless verified_webfinger? + return unless only_key || verified_webfinger? - ActivityPub::ProcessAccountService.new.call(@username, @domain, @json) + ActivityPub::ProcessAccountService.new.call(@username, @domain, @json, only_key: only_key) rescue Oj::ParseError nil end diff --git a/app/services/activitypub/process_account_service.rb b/app/services/activitypub/process_account_service.rb index d6480028f..d6c791b44 100644 --- a/app/services/activitypub/process_account_service.rb +++ b/app/services/activitypub/process_account_service.rb @@ -33,8 +33,10 @@ class ActivityPub::ProcessAccountService < BaseService after_protocol_change! if protocol_changed? after_key_change! if key_changed? && !@options[:signed_with_known_key] - check_featured_collection! if @account.featured_collection_url.present? - check_links! unless @account.fields.empty? + unless @options[:only_key] + check_featured_collection! if @account.featured_collection_url.present? + check_links! unless @account.fields.empty? + end @account rescue Oj::ParseError @@ -54,11 +56,11 @@ class ActivityPub::ProcessAccountService < BaseService end def update_account - @account.last_webfingered_at = Time.now.utc + @account.last_webfingered_at = Time.now.utc unless @options[:only_key] @account.protocol = :activitypub set_immediate_attributes! - set_fetchable_attributes! + set_fetchable_attributes! unless @options[:only_keys] @account.save_with_optional_media! end -- cgit From fb0c906c717f2b21bb63610742a357850142b522 Mon Sep 17 00:00:00 2001 From: Thibaut Girka Date: Thu, 10 Jan 2019 18:46:17 +0100 Subject: Revert "Revert "Add handler for Move activity (#9629)"" This reverts commit bb96a7463758687f8187ae4483becd346c2482b3. --- app/lib/activitypub/activity.rb | 2 + app/lib/activitypub/activity/follow.rb | 2 +- app/lib/activitypub/activity/move.rb | 43 ++++++++++++++++++ app/lib/activitypub/adapter.rb | 1 + app/models/account.rb | 5 +++ app/serializers/activitypub/actor_serializer.rb | 5 +++ .../activitypub/process_account_service.rb | 1 + app/services/follow_service.rb | 2 +- app/workers/unfollow_follow_worker.rb | 18 ++++++++ ...20181226021420_add_also_known_as_to_accounts.rb | 5 +++ db/schema.rb | 3 +- spec/lib/activitypub/activity/move_spec.rb | 52 ++++++++++++++++++++++ 12 files changed, 136 insertions(+), 3 deletions(-) create mode 100644 app/lib/activitypub/activity/move.rb create mode 100644 app/workers/unfollow_follow_worker.rb create mode 100644 db/migrate/20181226021420_add_also_known_as_to_accounts.rb create mode 100644 spec/lib/activitypub/activity/move_spec.rb (limited to 'app/services') diff --git a/app/lib/activitypub/activity.rb b/app/lib/activitypub/activity.rb index 0a729011f..87318fb1c 100644 --- a/app/lib/activitypub/activity.rb +++ b/app/lib/activitypub/activity.rb @@ -50,6 +50,8 @@ class ActivityPub::Activity ActivityPub::Activity::Add when 'Remove' ActivityPub::Activity::Remove + when 'Move' + ActivityPub::Activity::Move end end end diff --git a/app/lib/activitypub/activity/follow.rb b/app/lib/activitypub/activity/follow.rb index 5e703dc61..1e805c0d1 100644 --- a/app/lib/activitypub/activity/follow.rb +++ b/app/lib/activitypub/activity/follow.rb @@ -6,7 +6,7 @@ class ActivityPub::Activity::Follow < ActivityPub::Activity return if target_account.nil? || !target_account.local? || delete_arrived_first?(@json['id']) || @account.requested?(target_account) - if target_account.blocking?(@account) || target_account.domain_blocking?(@account.domain) + if target_account.blocking?(@account) || target_account.domain_blocking?(@account.domain) || target_account.moved? reject_follow_request!(target_account) return end diff --git a/app/lib/activitypub/activity/move.rb b/app/lib/activitypub/activity/move.rb new file mode 100644 index 000000000..d7a5f595c --- /dev/null +++ b/app/lib/activitypub/activity/move.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +class ActivityPub::Activity::Move < ActivityPub::Activity + PROCESSING_COOLDOWN = 7.days.seconds + + def perform + return if origin_account.uri != object_uri || processed? + + mark_as_processing! + + target_account = ActivityPub::FetchRemoteAccountService.new.call(target_uri) + + return if target_account.nil? || !target_account.also_known_as.include?(origin_account.uri) + + # In case for some reason we didn't have a redirect for the profile already, set it + origin_account.update(moved_to_account: target_account) if origin_account.moved_to_account_id.nil? + + # Initiate a re-follow for each follower + origin_account.followers.local.select(:id).find_in_batches do |follower_accounts| + UnfollowFollowWorker.push_bulk(follower_accounts.map(&:id)) do |follower_account_id| + [follower_account_id, origin_account.id, target_account.id] + end + end + end + + private + + def origin_account + @account + end + + def target_uri + value_or_id(@json['target']) + end + + def processed? + redis.exists("move_in_progress:#{@account.id}") + end + + def mark_as_processing! + redis.setex("move_in_progress:#{@account.id}", PROCESSING_COOLDOWN, true) + end +end diff --git a/app/lib/activitypub/adapter.rb b/app/lib/activitypub/adapter.rb index d35cae889..99f4d9305 100644 --- a/app/lib/activitypub/adapter.rb +++ b/app/lib/activitypub/adapter.rb @@ -10,6 +10,7 @@ class ActivityPub::Adapter < ActiveModelSerializers::Adapter::Base 'manuallyApprovesFollowers' => 'as:manuallyApprovesFollowers', 'sensitive' => 'as:sensitive', 'movedTo' => { '@id' => 'as:movedTo', '@type' => '@id' }, + 'alsoKnownAs' => { '@id' => 'as:alsoKnownAs', '@type' => '@id' }, 'Hashtag' => 'as:Hashtag', 'ostatus' => 'http://ostatus.org#', 'atomUri' => 'ostatus:atomUri', diff --git a/app/models/account.rb b/app/models/account.rb index 722e47d65..67d9a583e 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -44,6 +44,7 @@ # fields :jsonb # actor_type :string # discoverable :boolean +# also_known_as :string is an Array # class Account < ApplicationRecord @@ -232,6 +233,10 @@ class Account < ApplicationRecord end end + def also_known_as + self[:also_known_as] || [] + end + def fields (self[:fields] || []).map { |f| Field.new(self, f) } end diff --git a/app/serializers/activitypub/actor_serializer.rb b/app/serializers/activitypub/actor_serializer.rb index 72c30dc73..6746c1782 100644 --- a/app/serializers/activitypub/actor_serializer.rb +++ b/app/serializers/activitypub/actor_serializer.rb @@ -14,6 +14,7 @@ class ActivityPub::ActorSerializer < ActiveModel::Serializer has_many :virtual_attachments, key: :attachment attribute :moved_to, if: :moved? + attribute :also_known_as, if: :also_known_as? class EndpointsSerializer < ActiveModel::Serializer include RoutingHelper @@ -116,6 +117,10 @@ class ActivityPub::ActorSerializer < ActiveModel::Serializer ActivityPub::TagManager.instance.uri_for(object.moved_to_account) end + def also_known_as? + !object.also_known_as.empty? + end + class CustomEmojiSerializer < ActivityPub::EmojiSerializer end diff --git a/app/services/activitypub/process_account_service.rb b/app/services/activitypub/process_account_service.rb index 5c865dae2..d6480028f 100644 --- a/app/services/activitypub/process_account_service.rb +++ b/app/services/activitypub/process_account_service.rb @@ -75,6 +75,7 @@ class ActivityPub::ProcessAccountService < BaseService @account.note = @json['summary'] || '' @account.locked = @json['manuallyApprovesFollowers'] || false @account.fields = property_values || {} + @account.also_known_as = as_array(@json['alsoKnownAs'] || []).map { |item| value_or_id(item) } @account.actor_type = actor_type end diff --git a/app/services/follow_service.rb b/app/services/follow_service.rb index 24b3e1f70..9d36a1449 100644 --- a/app/services/follow_service.rb +++ b/app/services/follow_service.rb @@ -10,7 +10,7 @@ class FollowService < BaseService target_account = ResolveAccountService.new.call(target_account, skip_webfinger: true) raise ActiveRecord::RecordNotFound if target_account.nil? || target_account.id == source_account.id || target_account.suspended? - raise Mastodon::NotPermittedError if target_account.blocking?(source_account) || source_account.blocking?(target_account) + raise Mastodon::NotPermittedError if target_account.blocking?(source_account) || source_account.blocking?(target_account) || target_account.moved? if source_account.following?(target_account) # We're already following this account, but we'll call follow! again to diff --git a/app/workers/unfollow_follow_worker.rb b/app/workers/unfollow_follow_worker.rb new file mode 100644 index 000000000..a2133bb8c --- /dev/null +++ b/app/workers/unfollow_follow_worker.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class UnfollowFollowWorker + include Sidekiq::Worker + + sidekiq_options queue: 'pull' + + def perform(follower_account_id, old_target_account_id, new_target_account_id) + follower_account = Account.find(follower_account_id) + old_target_account = Account.find(old_target_account_id) + new_target_account = Account.find(new_target_account_id) + + UnfollowService.new.call(follower_account, old_target_account) + FollowService.new.call(follower_account, new_target_account) + rescue ActiveRecord::RecordNotFound + true + end +end diff --git a/db/migrate/20181226021420_add_also_known_as_to_accounts.rb b/db/migrate/20181226021420_add_also_known_as_to_accounts.rb new file mode 100644 index 000000000..1fd956680 --- /dev/null +++ b/db/migrate/20181226021420_add_also_known_as_to_accounts.rb @@ -0,0 +1,5 @@ +class AddAlsoKnownAsToAccounts < ActiveRecord::Migration[5.2] + def change + add_column :accounts, :also_known_as, :string, array: true + end +end diff --git a/db/schema.rb b/db/schema.rb index 48ffea3dd..b8a9f8c58 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: 2018_12_19_235220) do +ActiveRecord::Schema.define(version: 2018_12_26_021420) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -134,6 +134,7 @@ ActiveRecord::Schema.define(version: 2018_12_19_235220) do t.jsonb "fields" t.string "actor_type" t.boolean "discoverable" + t.string "also_known_as", array: true t.index "(((setweight(to_tsvector('simple'::regconfig, (display_name)::text), 'A'::\"char\") || setweight(to_tsvector('simple'::regconfig, (username)::text), 'B'::\"char\")) || setweight(to_tsvector('simple'::regconfig, (COALESCE(domain, ''::character varying))::text), 'C'::\"char\")))", name: "search_index", using: :gin t.index "lower((username)::text), lower((domain)::text)", name: "index_accounts_on_username_and_domain_lower", unique: true t.index ["moved_to_account_id"], name: "index_accounts_on_moved_to_account_id" diff --git a/spec/lib/activitypub/activity/move_spec.rb b/spec/lib/activitypub/activity/move_spec.rb new file mode 100644 index 000000000..3574f273a --- /dev/null +++ b/spec/lib/activitypub/activity/move_spec.rb @@ -0,0 +1,52 @@ +require 'rails_helper' + +RSpec.describe ActivityPub::Activity::Move do + let(:follower) { Fabricate(:account) } + let(:old_account) { Fabricate(:account) } + let(:new_account) { Fabricate(:account) } + + before do + follower.follow!(old_account) + + old_account.update!(uri: 'https://example.org/alice', domain: 'example.org', protocol: :activitypub, inbox_url: 'https://example.org/inbox') + new_account.update!(uri: 'https://example.com/alice', domain: 'example.com', protocol: :activitypub, inbox_url: 'https://example.com/inbox', also_known_as: [old_account.uri]) + + stub_request(:post, 'https://example.org/inbox').to_return(status: 200) + stub_request(:post, 'https://example.com/inbox').to_return(status: 200) + + service_stub = double + allow(ActivityPub::FetchRemoteAccountService).to receive(:new).and_return(service_stub) + allow(service_stub).to receive(:call).and_return(new_account) + end + + let(:json) do + { + '@context': 'https://www.w3.org/ns/activitystreams', + id: 'foo', + type: 'Move', + actor: old_account.uri, + object: old_account.uri, + target: new_account.uri, + }.with_indifferent_access + end + + describe '#perform' do + subject { described_class.new(json, old_account) } + + before do + subject.perform + end + + it 'sets moved account on old account' do + expect(old_account.reload.moved_to_account_id).to eq new_account.id + end + + it 'makes followers unfollow old account' do + expect(follower.following?(old_account)).to be false + end + + it 'makes followers follow-request the new account' do + expect(follower.requested?(new_account)).to be true + end + end +end -- cgit