From ba444797d20f744250fe2670a8c1f0424c909a84 Mon Sep 17 00:00:00 2001 From: ThibG Date: Wed, 3 Oct 2018 23:44:13 +0200 Subject: Fix handling of ActivityPub activities lacking some attributes (#8864) --- app/lib/activitypub/activity/accept.rb | 2 +- app/lib/activitypub/activity/delete.rb | 2 ++ app/lib/activitypub/activity/reject.rb | 2 +- app/lib/activitypub/activity/undo.rb | 2 ++ 4 files changed, 6 insertions(+), 2 deletions(-) (limited to 'app') diff --git a/app/lib/activitypub/activity/accept.rb b/app/lib/activitypub/activity/accept.rb index 7e60b2c00..348ee0d1c 100644 --- a/app/lib/activitypub/activity/accept.rb +++ b/app/lib/activitypub/activity/accept.rb @@ -26,7 +26,7 @@ class ActivityPub::Activity::Accept < ActivityPub::Activity end def relay - @relay ||= Relay.find_by(follow_activity_id: object_uri) + @relay ||= Relay.find_by(follow_activity_id: object_uri) unless object_uri.nil? end def relay_follow? diff --git a/app/lib/activitypub/activity/delete.rb b/app/lib/activitypub/activity/delete.rb index 3474d55d9..457047ac0 100644 --- a/app/lib/activitypub/activity/delete.rb +++ b/app/lib/activitypub/activity/delete.rb @@ -17,6 +17,8 @@ class ActivityPub::Activity::Delete < ActivityPub::Activity end def delete_note + return if object_uri.nil? + @status = Status.find_by(uri: object_uri, account: @account) @status ||= Status.find_by(uri: @object['atomUri'], account: @account) if @object.is_a?(Hash) && @object['atomUri'].present? diff --git a/app/lib/activitypub/activity/reject.rb b/app/lib/activitypub/activity/reject.rb index d81b157de..dba21fb9a 100644 --- a/app/lib/activitypub/activity/reject.rb +++ b/app/lib/activitypub/activity/reject.rb @@ -28,7 +28,7 @@ class ActivityPub::Activity::Reject < ActivityPub::Activity end def relay - @relay ||= Relay.find_by(follow_activity_id: object_uri) + @relay ||= Relay.find_by(follow_activity_id: object_uri) unless object_uri.nil? end def relay_follow? diff --git a/app/lib/activitypub/activity/undo.rb b/app/lib/activitypub/activity/undo.rb index 64c2be7d9..599823c6e 100644 --- a/app/lib/activitypub/activity/undo.rb +++ b/app/lib/activitypub/activity/undo.rb @@ -19,6 +19,8 @@ class ActivityPub::Activity::Undo < ActivityPub::Activity private def undo_announce + return if object_uri.nil? + status = Status.find_by(uri: object_uri, account: @account) status ||= Status.find_by(uri: @object['atomUri'], account: @account) if @object.is_a?(Hash) && @object['atomUri'].present? -- cgit From 7fe137d2f7792ed735be11eaca6d87fbc114043a Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Thu, 4 Oct 2018 15:47:03 +0200 Subject: Fix link verification for remote accounts (#8868) --- app/models/account.rb | 26 +++++- app/serializers/rest/account_serializer.rb | 6 +- app/services/verify_link_service.rb | 2 +- spec/services/verify_link_service_spec.rb | 139 +++++++++++++++++------------ 4 files changed, 108 insertions(+), 65 deletions(-) (limited to 'app') diff --git a/app/models/account.rb b/app/models/account.rb index d8e5c7340..44963f3e6 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -312,8 +312,8 @@ class Account < ApplicationRecord def initialize(account, attributes) @account = account @attributes = attributes - @name = attributes['name'].strip[0, 255] - @value = attributes['value'].strip[0, 255] + @name = attributes['name'].strip[0, string_limit] + @value = attributes['value'].strip[0, string_limit] @verified_at = attributes['verified_at']&.to_datetime @errors = {} end @@ -322,8 +322,18 @@ class Account < ApplicationRecord verified_at.present? end + def value_for_verification + @value_for_verification ||= begin + if account.local? + value + else + ActionController::Base.helpers.strip_tags(value) + end + end + end + def verifiable? - value.present? && value.start_with?('http://', 'https://') + value_for_verification.present? && value_for_verification.start_with?('http://', 'https://') end def mark_verified! @@ -334,6 +344,16 @@ class Account < ApplicationRecord def to_h { name: @name, value: @value, verified_at: @verified_at } end + + private + + def string_limit + if account.local? + 255 + else + 2047 + end + end end class << self diff --git a/app/serializers/rest/account_serializer.rb b/app/serializers/rest/account_serializer.rb index d84b48afb..12adc971c 100644 --- a/app/serializers/rest/account_serializer.rb +++ b/app/serializers/rest/account_serializer.rb @@ -11,11 +11,7 @@ class REST::AccountSerializer < ActiveModel::Serializer has_many :emojis, serializer: REST::CustomEmojiSerializer class FieldSerializer < ActiveModel::Serializer - attributes :name, :value - - attribute :verified_at, if: :verifiable? - - delegate :verifiable?, to: :object + attributes :name, :value, :verified_at def value Formatter.instance.format_field(object.account, object.value) diff --git a/app/services/verify_link_service.rb b/app/services/verify_link_service.rb index 7d53bc255..3453b54c5 100644 --- a/app/services/verify_link_service.rb +++ b/app/services/verify_link_service.rb @@ -3,7 +3,7 @@ class VerifyLinkService < BaseService def call(field) @link_back = ActivityPub::TagManager.instance.url_for(field.account) - @url = field.value + @url = field.value_for_verification perform_request! diff --git a/spec/services/verify_link_service_spec.rb b/spec/services/verify_link_service_spec.rb index 9b04d6136..2edcdb75f 100644 --- a/spec/services/verify_link_service_spec.rb +++ b/spec/services/verify_link_service_spec.rb @@ -3,80 +3,107 @@ require 'rails_helper' RSpec.describe VerifyLinkService, type: :service do subject { described_class.new } - let(:account) { Fabricate(:account, username: 'alice') } - let(:field) { Account::Field.new(account, 'name' => 'Website', 'value' => 'http://example.com') } + context 'given a local account' do + let(:account) { Fabricate(:account, username: 'alice') } + let(:field) { Account::Field.new(account, 'name' => 'Website', 'value' => 'http://example.com') } - before do - stub_request(:head, 'https://redirect.me/abc').to_return(status: 301, headers: { 'Location' => ActivityPub::TagManager.instance.url_for(account) }) - stub_request(:get, 'http://example.com').to_return(status: 200, body: html) - subject.call(field) - end - - context 'when a link contains an back' do - let(:html) do - <<-HTML - - - Follow me on Mastodon - - HTML + before do + stub_request(:head, 'https://redirect.me/abc').to_return(status: 301, headers: { 'Location' => ActivityPub::TagManager.instance.url_for(account) }) + stub_request(:get, 'http://example.com').to_return(status: 200, body: html) + subject.call(field) end - it 'marks the field as verified' do - expect(field.verified?).to be true + context 'when a link contains an back' do + let(:html) do + <<-HTML + + + Follow me on Mastodon + + HTML + end + + it 'marks the field as verified' do + expect(field.verified?).to be true + end end - end - context 'when a link contains an back' do - let(:html) do - <<-HTML - - - Follow me on Mastodon - - HTML + context 'when a link contains an back' do + let(:html) do + <<-HTML + + + Follow me on Mastodon + + HTML + end + + it 'marks the field as verified' do + expect(field.verified?).to be true + end end - it 'marks the field as verified' do - expect(field.verified?).to be true + context 'when a link contains a back' do + let(:html) do + <<-HTML + + + + + HTML + end + + it 'marks the field as verified' do + expect(field.verified?).to be true + end end - end - context 'when a link contains a back' do - let(:html) do - <<-HTML - - - - - HTML + context 'when a link goes through a redirect back' do + let(:html) do + <<-HTML + + + + + HTML + end + + it 'marks the field as verified' do + expect(field.verified?).to be true + end end - it 'marks the field as verified' do - expect(field.verified?).to be true + context 'when a link does not contain a link back' do + let(:html) { '' } + + it 'marks the field as verified' do + expect(field.verified?).to be false + end end end - context 'when a link goes through a redirect back' do - let(:html) do - <<-HTML - - - - - HTML - end + context 'given a remote account' do + let(:account) { Fabricate(:account, username: 'alice', domain: 'example.com', url: 'https://profile.example.com/alice') } + let(:field) { Account::Field.new(account, 'name' => 'Website', 'value' => 'example.com') } - it 'marks the field as verified' do - expect(field.verified?).to be true + before do + stub_request(:get, 'http://example.com').to_return(status: 200, body: html) + subject.call(field) end - end - context 'when a link does not contain a link back' do - let(:html) { '' } + context 'when a link contains an back' do + let(:html) do + <<-HTML + + + Follow me on Mastodon + + HTML + end - it 'marks the field as verified' do - expect(field.verified?).to be false + it 'marks the field as verified' do + expect(field.verified?).to be true + end end end end -- cgit From e645ae95610fcb69d15366bc32ab60014d2ebdcf Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Thu, 4 Oct 2018 16:05:38 +0200 Subject: Change admin accounts default sort to most recent (#8813) --- app/controllers/admin/accounts_controller.rb | 2 +- app/helpers/admin/filter_helper.rb | 2 +- app/models/account_filter.rb | 6 +++--- app/views/admin/accounts/index.html.haml | 4 ++-- spec/controllers/admin/accounts_controller_spec.rb | 4 ++-- spec/models/account_filter_spec.rb | 6 +++--- 6 files changed, 12 insertions(+), 12 deletions(-) (limited to 'app') diff --git a/app/controllers/admin/accounts_controller.rb b/app/controllers/admin/accounts_controller.rb index e7ca6b907..5d57fe361 100644 --- a/app/controllers/admin/accounts_controller.rb +++ b/app/controllers/admin/accounts_controller.rb @@ -95,7 +95,7 @@ module Admin :remote, :by_domain, :silenced, - :recent, + :alphabetic, :suspended, :username, :display_name, diff --git a/app/helpers/admin/filter_helper.rb b/app/helpers/admin/filter_helper.rb index 359c43d0e..60e5142e3 100644 --- a/app/helpers/admin/filter_helper.rb +++ b/app/helpers/admin/filter_helper.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module Admin::FilterHelper - ACCOUNT_FILTERS = %i(local remote by_domain silenced suspended recent username display_name email ip staff).freeze + ACCOUNT_FILTERS = %i(local remote by_domain silenced suspended alphabetic username display_name email ip staff).freeze REPORT_FILTERS = %i(resolved account_id target_account_id).freeze INVITE_FILTER = %i(available expired).freeze CUSTOM_EMOJI_FILTERS = %i(local remote by_domain shortcode).freeze diff --git a/app/models/account_filter.rb b/app/models/account_filter.rb index dc7a03039..84364bf1b 100644 --- a/app/models/account_filter.rb +++ b/app/models/account_filter.rb @@ -8,7 +8,7 @@ class AccountFilter end def results - scope = Account.alphabetic + scope = Account.recent params.each do |key, value| scope.merge!(scope_for(key, value)) if value.present? @@ -29,8 +29,8 @@ class AccountFilter Account.where(domain: value) when 'silenced' Account.silenced - when 'recent' - Account.recent + when 'alphabetic' + Account.reorder(nil).alphabetic when 'suspended' Account.suspended when 'username' diff --git a/app/views/admin/accounts/index.html.haml b/app/views/admin/accounts/index.html.haml index 6aa39a80a..4bee73adc 100644 --- a/app/views/admin/accounts/index.html.haml +++ b/app/views/admin/accounts/index.html.haml @@ -38,8 +38,8 @@ .filter-subset %strong= t('admin.accounts.order.title') %ul - %li= filter_link_to t('admin.accounts.order.alphabetic'), recent: nil - %li= filter_link_to t('admin.accounts.order.most_recent'), recent: '1' + %li= filter_link_to t('admin.accounts.order.most_recent'), alphabetic: nil + %li= filter_link_to t('admin.accounts.order.alphabetic'), alphabetic: '1' = form_tag admin_accounts_url, method: 'GET', class: 'simple_form' do .fields-group diff --git a/spec/controllers/admin/accounts_controller_spec.rb b/spec/controllers/admin/accounts_controller_spec.rb index fa2786c9b..ae9e058c8 100644 --- a/spec/controllers/admin/accounts_controller_spec.rb +++ b/spec/controllers/admin/accounts_controller_spec.rb @@ -25,7 +25,7 @@ RSpec.describe Admin::AccountsController, type: :controller do expect(h[:remote]).to eq '1' expect(h[:by_domain]).to eq 'domain' expect(h[:silenced]).to eq '1' - expect(h[:recent]).to eq '1' + expect(h[:alphabetic]).to eq '1' expect(h[:suspended]).to eq '1' expect(h[:username]).to eq 'username' expect(h[:display_name]).to eq 'display name' @@ -40,7 +40,7 @@ RSpec.describe Admin::AccountsController, type: :controller do remote: '1', by_domain: 'domain', silenced: '1', - recent: '1', + alphabetic: '1', suspended: '1', username: 'username', display_name: 'display name', diff --git a/spec/models/account_filter_spec.rb b/spec/models/account_filter_spec.rb index 8441939c5..0a0252642 100644 --- a/spec/models/account_filter_spec.rb +++ b/spec/models/account_filter_spec.rb @@ -2,10 +2,10 @@ require 'rails_helper' describe AccountFilter do describe 'with empty params' do - it 'defaults to alphabetic account list' do + it 'defaults to recent account list' do filter = described_class.new({}) - expect(filter.results).to eq Account.alphabetic + expect(filter.results).to eq Account.recent end end @@ -60,7 +60,7 @@ describe AccountFilter do end describe 'that call account methods' do - %i(local remote silenced recent suspended).each do |option| + %i(local remote silenced alphabetic suspended).each do |option| it "delegates the #{option} option" do allow(Account).to receive(option).and_return(Account.none) filter = described_class.new({ option => true }) -- cgit From a46ab86adfc9e4ea182af9a555237f17071e194c Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Thu, 4 Oct 2018 17:36:11 +0200 Subject: Limit the number of people that can be followed from one account (#8807) Configurable soft limit of 7,500, and above that, configurable ratio of 1.1 * followers, controlled by: - MAX_FOLLOWS_THRESHOLD - MAX_FOLLOWS_RATIO Fix #2311 --- app/models/follow.rb | 1 + app/models/follow_request.rb | 1 + app/validators/follow_limit_validator.rb | 27 +++++++++++++++++++++++++++ app/workers/import_worker.rb | 4 +++- config/locales/en.yml | 1 + spec/models/follow_spec.rb | 14 ++++++++++++++ 6 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 app/validators/follow_limit_validator.rb (limited to 'app') diff --git a/app/models/follow.rb b/app/models/follow.rb index 714f4e898..7ad56eb78 100644 --- a/app/models/follow.rb +++ b/app/models/follow.rb @@ -25,6 +25,7 @@ class Follow < ApplicationRecord has_one :notification, as: :activity, dependent: :destroy validates :account_id, uniqueness: { scope: :target_account_id } + validates_with FollowLimitValidator, on: :create scope :recent, -> { reorder(id: :desc) } diff --git a/app/models/follow_request.rb b/app/models/follow_request.rb index 9c4875564..c5451a050 100644 --- a/app/models/follow_request.rb +++ b/app/models/follow_request.rb @@ -22,6 +22,7 @@ class FollowRequest < ApplicationRecord has_one :notification, as: :activity, dependent: :destroy validates :account_id, uniqueness: { scope: :target_account_id } + validates_with FollowLimitValidator, on: :create def authorize! account.follow!(target_account, reblogs: show_reblogs, uri: uri) diff --git a/app/validators/follow_limit_validator.rb b/app/validators/follow_limit_validator.rb new file mode 100644 index 000000000..eb083ed85 --- /dev/null +++ b/app/validators/follow_limit_validator.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +class FollowLimitValidator < ActiveModel::Validator + LIMIT = ENV.fetch('MAX_FOLLOWS_THRESHOLD', 7_500).to_i + RATIO = ENV.fetch('MAX_FOLLOWS_RATIO', 1.1).to_f + + def validate(follow) + return if follow.account.nil? || !follow.account.local? + follow.errors.add(:base, I18n.t('users.follow_limit_reached', limit: self.class.limit_for_account(follow.account))) if limit_reached?(follow.account) + end + + class << self + def limit_for_account(account) + if account.following_count < LIMIT + LIMIT + else + account.followers_count * RATIO + end + end + end + + private + + def limit_reached?(account) + account.following_count >= self.class.limit_for_account(account) + end +end diff --git a/app/workers/import_worker.rb b/app/workers/import_worker.rb index d7c126f75..aeb221cf6 100644 --- a/app/workers/import_worker.rb +++ b/app/workers/import_worker.rb @@ -37,6 +37,8 @@ class ImportWorker end def import_rows - CSV.new(import_contents).reject(&:blank?) + rows = CSV.new(import_contents).reject(&:blank?) + rows = rows.take(FollowLimitValidator.limit_for_account(@import.account)) if @import.type == 'following' + rows end end diff --git a/config/locales/en.yml b/config/locales/en.yml index f883b17a1..439c5627b 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -917,6 +917,7 @@ en: tips: Tips title: Welcome aboard, %{name}! users: + follow_limit_reached: You cannot follow more than %{limit} people invalid_email: The e-mail address is invalid invalid_otp_token: Invalid two-factor code otp_lost_help_html: If you lost access to both, you may get in touch with %{email} diff --git a/spec/models/follow_spec.rb b/spec/models/follow_spec.rb index f221973b6..0c84e5e7b 100644 --- a/spec/models/follow_spec.rb +++ b/spec/models/follow_spec.rb @@ -23,6 +23,20 @@ RSpec.describe Follow, type: :model do follow.valid? expect(follow).to model_have_error_on_field(:target_account) end + + it 'is invalid if account already follows too many people' do + alice.update(following_count: FollowLimitValidator::LIMIT) + + expect(subject).to_not be_valid + expect(subject).to model_have_error_on_field(:base) + end + + it 'is valid if account is only on the brink of following too many people' do + alice.update(following_count: FollowLimitValidator::LIMIT - 1) + + expect(subject).to be_valid + expect(subject).to_not model_have_error_on_field(:base) + end end describe 'recent' do -- cgit