From 7b0fe4aef97c6a5f73a03146b669a415f396799c Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 29 Apr 2022 22:43:07 +0200 Subject: Fix opening and closing Redis connections instead of using a pool (#18171) * Fix opening and closing Redis connections instead of using a pool * Fix Redis connections not being returned to the pool in CLI commands --- spec/services/resolve_account_service_spec.rb | 2 ++ 1 file changed, 2 insertions(+) (limited to 'spec') diff --git a/spec/services/resolve_account_service_spec.rb b/spec/services/resolve_account_service_spec.rb index 7b1e8885c..8c302e1d8 100644 --- a/spec/services/resolve_account_service_spec.rb +++ b/spec/services/resolve_account_service_spec.rb @@ -220,6 +220,8 @@ RSpec.describe ResolveAccountService, type: :service do return_values << described_class.new.call('foo@ap.example.com') rescue ActiveRecord::RecordNotUnique fail_occurred = true + ensure + RedisConfiguration.pool.checkin if Thread.current[:redis] end end end -- cgit From f6d35ed57d156f4225338a89372c8e83721e46c9 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 29 Apr 2022 23:27:03 +0200 Subject: Remove IP matching from e-mail domain blocks (#18190) Clear out e-mail domain blocks created from automatically resolved DNS records --- app/models/email_domain_block.rb | 24 ++++----- app/validators/email_mx_validator.rb | 6 +-- .../email_domain_block_refresh_scheduler.rb | 31 ----------- config/sidekiq.yml | 4 -- .../20180615122121_add_autofollow_to_invites.rb | 2 +- ...29101025_remove_ips_from_email_domain_blocks.rb | 12 +++++ .../20220429101850_clear_email_domain_blocks.rb | 14 +++++ db/schema.rb | 4 +- spec/validators/email_mx_validator_spec.rb | 60 ---------------------- 9 files changed, 43 insertions(+), 114 deletions(-) delete mode 100644 app/workers/scheduler/email_domain_block_refresh_scheduler.rb create mode 100644 db/post_migrate/20220429101025_remove_ips_from_email_domain_blocks.rb create mode 100644 db/post_migrate/20220429101850_clear_email_domain_blocks.rb (limited to 'spec') diff --git a/app/models/email_domain_block.rb b/app/models/email_domain_block.rb index 36e7e62ab..0e1e663c1 100644 --- a/app/models/email_domain_block.rb +++ b/app/models/email_domain_block.rb @@ -3,16 +3,19 @@ # # Table name: email_domain_blocks # -# id :bigint(8) not null, primary key -# domain :string default(""), not null -# created_at :datetime not null -# updated_at :datetime not null -# parent_id :bigint(8) -# ips :inet is an Array -# last_refresh_at :datetime +# id :bigint(8) not null, primary key +# domain :string default(""), not null +# created_at :datetime not null +# updated_at :datetime not null +# parent_id :bigint(8) # class EmailDomainBlock < ApplicationRecord + self.ignored_columns = %w( + ips + last_refresh_at + ) + include DomainNormalizable belongs_to :parent, class_name: 'EmailDomainBlock', optional: true @@ -27,7 +30,7 @@ class EmailDomainBlock < ApplicationRecord @history ||= Trends::History.new('email_domain_blocks', id) end - def self.block?(domain_or_domains, ips: [], attempt_ip: nil) + def self.block?(domain_or_domains, attempt_ip: nil) domains = Array(domain_or_domains).map do |str| domain = begin if str.include?('@') @@ -48,10 +51,7 @@ class EmailDomainBlock < ApplicationRecord blocked = domains.any?(&:nil?) - scope = where(domain: domains) - scope = scope.or(where('ips && ARRAY[?]::inet[]', ips)) if ips.any? - - scope.find_each do |block| + where(domain: domains).find_each do |block| blocked = true block.history.add(attempt_ip) if attempt_ip.present? end diff --git a/app/validators/email_mx_validator.rb b/app/validators/email_mx_validator.rb index 237ca4c7b..20f2fd37c 100644 --- a/app/validators/email_mx_validator.rb +++ b/app/validators/email_mx_validator.rb @@ -15,7 +15,7 @@ class EmailMxValidator < ActiveModel::Validator if resolved_ips.empty? user.errors.add(:email, :unreachable) - elsif on_blacklist?(resolved_domains, resolved_ips, user.sign_up_ip) + elsif on_blacklist?(resolved_domains, user.sign_up_ip) user.errors.add(:email, :blocked) end end @@ -57,7 +57,7 @@ class EmailMxValidator < ActiveModel::Validator [ips, records] end - def on_blacklist?(domains, resolved_ips, attempt_ip) - EmailDomainBlock.block?(domains, ips: resolved_ips, attempt_ip: attempt_ip) + def on_blacklist?(domains, attempt_ip) + EmailDomainBlock.block?(domains, attempt_ip: attempt_ip) end end diff --git a/app/workers/scheduler/email_domain_block_refresh_scheduler.rb b/app/workers/scheduler/email_domain_block_refresh_scheduler.rb deleted file mode 100644 index e0ad89866..000000000 --- a/app/workers/scheduler/email_domain_block_refresh_scheduler.rb +++ /dev/null @@ -1,31 +0,0 @@ -# frozen_string_literal: true - -class Scheduler::EmailDomainBlockRefreshScheduler - include Sidekiq::Worker - include Redisable - - sidekiq_options retry: 0 - - def perform - Resolv::DNS.open do |dns| - dns.timeouts = 5 - - EmailDomainBlock.find_each do |email_domain_block| - ips = begin - if ip?(email_domain_block.domain) - [email_domain_block.domain] - else - resources = dns.getresources(email_domain_block.domain, Resolv::DNS::Resource::IN::A).to_a + dns.getresources(email_domain_block.domain, Resolv::DNS::Resource::IN::AAAA).to_a - resources.map { |resource| resource.address.to_s } - end - end - - email_domain_block.update(ips: ips, last_refresh_at: Time.now.utc) - end - end - end - - def ip?(str) - str =~ Regexp.union([Resolv::IPv4::Regex, Resolv::IPv6::Regex]) - end -end diff --git a/config/sidekiq.yml b/config/sidekiq.yml index f2ae9279b..26be26326 100644 --- a/config/sidekiq.yml +++ b/config/sidekiq.yml @@ -17,10 +17,6 @@ every: '5m' class: Scheduler::Trends::RefreshScheduler queue: scheduler - email_domain_block_refresh_scheduler: - every: '1h' - class: Scheduler::EmailDomainBlockRefreshScheduler - queue: scheduler trends_review_notifications_scheduler: every: '6h' class: Scheduler::Trends::ReviewNotificationsScheduler diff --git a/db/migrate/20180615122121_add_autofollow_to_invites.rb b/db/migrate/20180615122121_add_autofollow_to_invites.rb index 850b1d693..8c5fb7410 100644 --- a/db/migrate/20180615122121_add_autofollow_to_invites.rb +++ b/db/migrate/20180615122121_add_autofollow_to_invites.rb @@ -5,7 +5,7 @@ class AddAutofollowToInvites < ActiveRecord::Migration[5.2] disable_ddl_transaction! - def change + def up safety_assured do add_column_with_default :invites, :autofollow, :bool, default: false, allow_null: false end diff --git a/db/post_migrate/20220429101025_remove_ips_from_email_domain_blocks.rb b/db/post_migrate/20220429101025_remove_ips_from_email_domain_blocks.rb new file mode 100644 index 000000000..fbb74d99e --- /dev/null +++ b/db/post_migrate/20220429101025_remove_ips_from_email_domain_blocks.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +class RemoveIpsFromEmailDomainBlocks < ActiveRecord::Migration[5.2] + disable_ddl_transaction! + + def change + safety_assured do + remove_column :email_domain_blocks, :ips, :inet, array: true + remove_column :email_domain_blocks, :last_refresh_at, :datetime + end + end +end diff --git a/db/post_migrate/20220429101850_clear_email_domain_blocks.rb b/db/post_migrate/20220429101850_clear_email_domain_blocks.rb new file mode 100644 index 000000000..ff525b650 --- /dev/null +++ b/db/post_migrate/20220429101850_clear_email_domain_blocks.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class ClearEmailDomainBlocks < ActiveRecord::Migration[5.2] + disable_ddl_transaction! + + class EmailDomainBlock < ApplicationRecord + end + + def up + EmailDomainBlock.where.not(parent_id: nil).in_batches.delete_all + end + + def down; end +end diff --git a/db/schema.rb b/db/schema.rb index a58f42400..726989bef 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: 2022_04_28_114902) do +ActiveRecord::Schema.define(version: 2022_04_29_101850) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -389,8 +389,6 @@ ActiveRecord::Schema.define(version: 2022_04_28_114902) do t.datetime "created_at", null: false t.datetime "updated_at", null: false t.bigint "parent_id" - t.inet "ips", array: true - t.datetime "last_refresh_at" t.index ["domain"], name: "index_email_domain_blocks_on_domain", unique: true end diff --git a/spec/validators/email_mx_validator_spec.rb b/spec/validators/email_mx_validator_spec.rb index af0eb98f5..4feedd0c7 100644 --- a/spec/validators/email_mx_validator_spec.rb +++ b/spec/validators/email_mx_validator_spec.rb @@ -56,66 +56,6 @@ describe EmailMxValidator do expect(user.errors).to have_received(:add) end - it 'adds an error if the A record is blacklisted' do - EmailDomainBlock.create!(domain: 'alternate-example.com', ips: ['1.2.3.4']) - resolver = double - - allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::MX).and_return([]) - allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::A).and_return([double(address: '1.2.3.4')]) - allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::AAAA).and_return([]) - allow(resolver).to receive(:timeouts=).and_return(nil) - allow(Resolv::DNS).to receive(:open).and_yield(resolver) - - subject.validate(user) - expect(user.errors).to have_received(:add) - end - - it 'adds an error if the AAAA record is blacklisted' do - EmailDomainBlock.create!(domain: 'alternate-example.com', ips: ['fd00::1']) - resolver = double - - allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::MX).and_return([]) - allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::A).and_return([]) - allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::AAAA).and_return([double(address: 'fd00::1')]) - allow(resolver).to receive(:timeouts=).and_return(nil) - allow(Resolv::DNS).to receive(:open).and_yield(resolver) - - subject.validate(user) - expect(user.errors).to have_received(:add) - end - - it 'adds an error if the A record of the MX record is blacklisted' do - EmailDomainBlock.create!(domain: 'mail.other-domain.com', ips: ['2.3.4.5']) - resolver = double - - allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::MX).and_return([double(exchange: 'mail.example.com')]) - allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::A).and_return([]) - allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::AAAA).and_return([]) - allow(resolver).to receive(:getresources).with('mail.example.com', Resolv::DNS::Resource::IN::A).and_return([double(address: '2.3.4.5')]) - allow(resolver).to receive(:getresources).with('mail.example.com', Resolv::DNS::Resource::IN::AAAA).and_return([]) - allow(resolver).to receive(:timeouts=).and_return(nil) - allow(Resolv::DNS).to receive(:open).and_yield(resolver) - - subject.validate(user) - expect(user.errors).to have_received(:add) - end - - it 'adds an error if the AAAA record of the MX record is blacklisted' do - EmailDomainBlock.create!(domain: 'mail.other-domain.com', ips: ['fd00::2']) - resolver = double - - allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::MX).and_return([double(exchange: 'mail.example.com')]) - allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::A).and_return([]) - allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::AAAA).and_return([]) - allow(resolver).to receive(:getresources).with('mail.example.com', Resolv::DNS::Resource::IN::A).and_return([]) - allow(resolver).to receive(:getresources).with('mail.example.com', Resolv::DNS::Resource::IN::AAAA).and_return([double(address: 'fd00::2')]) - allow(resolver).to receive(:timeouts=).and_return(nil) - allow(Resolv::DNS).to receive(:open).and_yield(resolver) - - subject.validate(user) - expect(user.errors).to have_received(:add) - end - it 'adds an error if the MX record is blacklisted' do EmailDomainBlock.create!(domain: 'mail.example.com') resolver = double -- cgit From 71d02ffcf3a79dfc1c413dcc7ff45c77ce9cb94c Mon Sep 17 00:00:00 2001 From: Claire Date: Mon, 2 May 2022 17:41:01 +0200 Subject: Fix compatibility with Friendica regarding pinned posts (#18254) * Fix multiple database queries when fetching pinned posts for remote account * Fix compatibility with Friendica regarding pinned posts Fixes #18066 * Add tests --- .../fetch_featured_collection_service.rb | 34 ++++-- .../fetch_featured_collection_service_spec.rb | 123 +++++++++++++++++++++ 2 files changed, 146 insertions(+), 11 deletions(-) create mode 100644 spec/services/activitypub/fetch_featured_collection_service_spec.rb (limited to 'spec') diff --git a/app/services/activitypub/fetch_featured_collection_service.rb b/app/services/activitypub/fetch_featured_collection_service.rb index 07a9fe039..e62470f70 100644 --- a/app/services/activitypub/fetch_featured_collection_service.rb +++ b/app/services/activitypub/fetch_featured_collection_service.rb @@ -7,19 +7,33 @@ class ActivityPub::FetchFeaturedCollectionService < BaseService return if account.featured_collection_url.blank? || account.suspended? || account.local? @account = account - @json = fetch_resource(@account.featured_collection_url, true) + @json = fetch_resource(@account.featured_collection_url, true, local_follower) - return unless supported_context? + return unless supported_context?(@json) - case @json['type'] + process_items(collection_items(@json)) + end + + private + + def collection_items(collection) + collection = fetch_collection(collection['first']) if collection['first'].present? + return unless collection.is_a?(Hash) + + case collection['type'] when 'Collection', 'CollectionPage' - process_items @json['items'] + collection['items'] when 'OrderedCollection', 'OrderedCollectionPage' - process_items @json['orderedItems'] + collection['orderedItems'] end end - private + def fetch_collection(collection_or_uri) + return collection_or_uri if collection_or_uri.is_a?(Hash) + return if invalid_origin?(collection_or_uri) + + fetch_resource_without_id_validation(collection_or_uri, nil, true, local_follower) + end def process_items(items) status_ids = items.filter_map do |item| @@ -53,11 +67,9 @@ class ActivityPub::FetchFeaturedCollectionService < BaseService end end - def supported_context? - super(@json) - end - def local_follower - @local_follower ||= @account.followers.local.without_suspended.first + return @local_follower if defined?(@local_follower) + + @local_follower = @account.followers.local.without_suspended.first end end diff --git a/spec/services/activitypub/fetch_featured_collection_service_spec.rb b/spec/services/activitypub/fetch_featured_collection_service_spec.rb new file mode 100644 index 000000000..f552b9dc0 --- /dev/null +++ b/spec/services/activitypub/fetch_featured_collection_service_spec.rb @@ -0,0 +1,123 @@ +require 'rails_helper' + +RSpec.describe ActivityPub::FetchFeaturedCollectionService, type: :service do + let(:actor) { Fabricate(:account, domain: 'example.com', uri: 'https://example.com/account', featured_collection_url: 'https://example.com/account/pinned') } + + let!(:known_status) { Fabricate(:status, account: actor, uri: 'https://example.com/account/pinned/1') } + + let(:status_json_1) do + { + '@context': 'https://www.w3.org/ns/activitystreams', + type: 'Note', + id: 'https://example.com/account/pinned/1', + content: 'foo', + attributedTo: actor.uri, + to: 'https://www.w3.org/ns/activitystreams#Public', + } + end + + let(:status_json_2) do + { + '@context': 'https://www.w3.org/ns/activitystreams', + type: 'Note', + id: 'https://example.com/account/pinned/2', + content: 'foo', + attributedTo: actor.uri, + to: 'https://www.w3.org/ns/activitystreams#Public', + } + end + + let(:status_json_4) do + { + '@context': 'https://www.w3.org/ns/activitystreams', + type: 'Note', + id: 'https://example.com/account/pinned/4', + content: 'foo', + attributedTo: actor.uri, + to: 'https://www.w3.org/ns/activitystreams#Public', + } + end + + let(:items) do + [ + 'https://example.com/account/pinned/1', # known + status_json_2, # unknown inlined + 'https://example.com/account/pinned/3', # unknown unreachable + 'https://example.com/account/pinned/4', # unknown reachable + ] + end + + let(:payload) do + { + '@context': 'https://www.w3.org/ns/activitystreams', + type: 'Collection', + id: actor.featured_collection_url, + items: items, + }.with_indifferent_access + end + + subject { described_class.new } + + shared_examples 'sets pinned posts' do + before do + stub_request(:get, 'https://example.com/account/pinned/1').to_return(status: 200, body: Oj.dump(status_json_1)) + stub_request(:get, 'https://example.com/account/pinned/2').to_return(status: 200, body: Oj.dump(status_json_2)) + stub_request(:get, 'https://example.com/account/pinned/3').to_return(status: 404) + stub_request(:get, 'https://example.com/account/pinned/4').to_return(status: 200, body: Oj.dump(status_json_4)) + + subject.call(actor) + end + + it 'sets expected posts as pinned posts' do + expect(actor.pinned_statuses.pluck(:uri)).to match_array ['https://example.com/account/pinned/1', 'https://example.com/account/pinned/2', 'https://example.com/account/pinned/4'] + end + end + + describe '#call' do + context 'when the endpoint is a Collection' do + before do + stub_request(:get, actor.featured_collection_url).to_return(status: 200, body: Oj.dump(payload)) + end + + it_behaves_like 'sets pinned posts' + end + + context 'when the endpoint is an OrderedCollection' do + let(:payload) do + { + '@context': 'https://www.w3.org/ns/activitystreams', + type: 'OrderedCollection', + id: actor.featured_collection_url, + orderedItems: items, + }.with_indifferent_access + end + + before do + stub_request(:get, actor.featured_collection_url).to_return(status: 200, body: Oj.dump(payload)) + end + + it_behaves_like 'sets pinned posts' + end + + context 'when the endpoint is a paginated Collection' do + let(:payload) do + { + '@context': 'https://www.w3.org/ns/activitystreams', + type: 'Collection', + id: actor.featured_collection_url, + first: { + type: 'CollectionPage', + partOf: actor.featured_collection_url, + items: items, + } + }.with_indifferent_access + end + + before do + stub_request(:get, actor.featured_collection_url).to_return(status: 200, body: Oj.dump(payload)) + end + + it_behaves_like 'sets pinned posts' + end + end +end -- cgit