diff --git a/CHANGELOG.md b/CHANGELOG.md index 636b3c1611b786..8e579e14899463 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,41 @@ All notable changes to this project will be documented in this file. +## [4.2.9] - 2024-05-30 + +### Security + +- Update dependencies +- Fix private mention filtering ([GHSA-5fq7-3p3j-9vrf](https://github.com/mastodon/mastodon/security/advisories/GHSA-5fq7-3p3j-9vrf)) +- Fix password change endpoint not being rate-limited ([GHSA-q3rg-xx5v-4mxh](https://github.com/mastodon/mastodon/security/advisories/GHSA-q3rg-xx5v-4mxh)) +- Add hardening around rate-limit bypass ([GHSA-c2r5-cfqr-c553](https://github.com/mastodon/mastodon/security/advisories/GHSA-c2r5-cfqr-c553)) + +### Added + +- Add rate-limit on OAuth application registration ([ThisIsMissEm](https://github.com/mastodon/mastodon/pull/30316)) +- Add fallback redirection when getting a webfinger query `WEB_DOMAIN@WEB_DOMAIN` ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/28592)) +- Add `digest` attribute to `Admin::DomainBlock` entity in REST API ([ThisIsMissEm](https://github.com/mastodon/mastodon/pull/29092)) + +### Removed + +- Remove superfluous application-level caching in some controllers ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/29862)) +- Remove aggressive OAuth application vacuuming ([ThisIsMissEm](https://github.com/mastodon/mastodon/pull/30316)) + +### Fixed + +- Fix leaking Elasticsearch connections in Sidekiq processes ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/30450)) +- Fix language of remote posts not being recognized when using unusual casing ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/30403)) +- Fix off-by-one in `tootctl media` commands ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/30306)) +- Fix removal of allowed domains (in `LIMITED_FEDERATION_MODE`) not being recorded in the audit log ([ThisIsMissEm](https://github.com/mastodon/mastodon/pull/30125)) +- Fix not being able to block a subdomain of an already-blocked domain through the API ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/30119)) +- Fix `Idempotency-Key` being ignored when scheduling a post ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/30084)) +- Fix crash when supplying the `FFMPEG_BINARY` environment variable ([timothyjrogers](https://github.com/mastodon/mastodon/pull/30022)) +- Fix improper email address validation ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/29838)) +- Fix results/query in `api/v1/featured_tags/suggestions` ([mjankowski](https://github.com/mastodon/mastodon/pull/29597)) +- Fix unblocking internationalized domain names under certain conditions ([tribela](https://github.com/mastodon/mastodon/pull/29530)) +- Fix admin account created by `mastodon:setup` not being auto-approved ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/29379)) +- Fix reference to non-existent var in CLI maintenance command ([mjankowski](https://github.com/mastodon/mastodon/pull/28363)) + ## [4.2.8] - 2024-02-23 ### Added diff --git a/Gemfile b/Gemfile index d273ece62e4c38..de65e0555db148 100644 --- a/Gemfile +++ b/Gemfile @@ -206,3 +206,5 @@ gem 'net-http', '~> 0.3.2' gem 'rubyzip', '~> 2.3' gem 'hcaptcha', '~> 7.1' + +gem 'mail', '~> 2.8' diff --git a/Gemfile.lock b/Gemfile.lock index 3b6e68f65beb64..9f29d85ff9903d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -288,7 +288,7 @@ GEM faraday_middleware (1.2.0) faraday (~> 1.0) fast_blank (1.0.1) - fastimage (2.2.7) + fastimage (2.3.1) ffi (1.15.5) ffi-compiler (1.0.1) ffi (>= 1.0.0) @@ -368,7 +368,7 @@ GEM jmespath (1.6.2) json (2.6.3) json-canonicalization (1.0.0) - json-jwt (1.15.3) + json-jwt (1.15.3.1) activesupport (>= 4.2) aes_key_wrap bindata @@ -469,7 +469,7 @@ GEM net-protocol net-ssh (7.1.0) nio4r (2.7.0) - nokogiri (1.16.2) + nokogiri (1.16.5) mini_portile2 (~> 2.8.2) racc (~> 1.4) nsa (0.3.0) @@ -537,7 +537,7 @@ GEM rack (2.2.8.1) rack-attack (6.7.0) rack (>= 1.0, < 4) - rack-cors (2.0.1) + rack-cors (2.0.2) rack (>= 2.0.0) rack-oauth2 (1.21.3) activesupport @@ -605,8 +605,9 @@ GEM responders (3.1.0) actionpack (>= 5.2) railties (>= 5.2) - rexml (3.2.6) - rotp (6.2.2) + rexml (3.2.8) + strscan (>= 3.0.9) + rotp (6.3.0) rouge (4.1.2) rpam2 (4.0.2) rqrcode (2.2.0) @@ -733,6 +734,7 @@ GEM redlock (~> 1.0) strong_migrations (0.8.0) activerecord (>= 5.2) + strscan (3.0.9) swd (1.3.0) activesupport (>= 3) attr_required (>= 0.0.5) @@ -873,6 +875,7 @@ DEPENDENCIES letter_opener_web (~> 2.0) link_header (~> 0.0) lograge (~> 0.12) + mail (~> 2.8) mario-redis-lock (~> 1.2) md-paperclip-azure (~> 2.2) memory_profiler diff --git a/app/controllers/admin/domain_allows_controller.rb b/app/controllers/admin/domain_allows_controller.rb index 31be1978bbbb10..b0f139e3a8284e 100644 --- a/app/controllers/admin/domain_allows_controller.rb +++ b/app/controllers/admin/domain_allows_controller.rb @@ -25,6 +25,8 @@ def create def destroy authorize @domain_allow, :destroy? UnallowDomainService.new.call(@domain_allow) + log_action :destroy, @domain_allow + redirect_to admin_instances_path, notice: I18n.t('admin.domain_allows.destroyed_msg') end diff --git a/app/controllers/api/v1/admin/domain_blocks_controller.rb b/app/controllers/api/v1/admin/domain_blocks_controller.rb index 6dbf58d2da7d0c..765ea9572b1518 100644 --- a/app/controllers/api/v1/admin/domain_blocks_controller.rb +++ b/app/controllers/api/v1/admin/domain_blocks_controller.rb @@ -29,10 +29,11 @@ def show def create authorize :domain_block, :create? + @domain_block = DomainBlock.new(resource_params) existing_domain_block = resource_params[:domain].present? ? DomainBlock.rule_for(resource_params[:domain]) : nil - return render json: existing_domain_block, serializer: REST::Admin::ExistingDomainBlockErrorSerializer, status: 422 if existing_domain_block.present? + return render json: existing_domain_block, serializer: REST::Admin::ExistingDomainBlockErrorSerializer, status: 422 if conflicts_with_existing_block?(@domain_block, existing_domain_block) - @domain_block = DomainBlock.create!(resource_params) + @domain_block.save! DomainBlockWorker.perform_async(@domain_block.id) log_action :create, @domain_block render json: @domain_block, serializer: REST::Admin::DomainBlockSerializer @@ -55,6 +56,10 @@ def destroy private + def conflicts_with_existing_block?(domain_block, existing_domain_block) + existing_domain_block.present? && (existing_domain_block.domain == TagManager.instance.normalize_domain(domain_block.domain) || !domain_block.stricter_than?(existing_domain_block)) + end + def set_domain_blocks @domain_blocks = filtered_domain_blocks.order(id: :desc).to_a_paginated_by_id(limit_param(LIMIT), params_slice(:max_id, :since_id, :min_id)) end diff --git a/app/controllers/api/v1/featured_tags/suggestions_controller.rb b/app/controllers/api/v1/featured_tags/suggestions_controller.rb index 76633210a1dd9c..4f732ed2d5a474 100644 --- a/app/controllers/api/v1/featured_tags/suggestions_controller.rb +++ b/app/controllers/api/v1/featured_tags/suggestions_controller.rb @@ -12,6 +12,10 @@ def index private def set_recently_used_tags - @recently_used_tags = Tag.recently_used(current_account).where.not(id: current_account.featured_tags).limit(10) + @recently_used_tags = Tag.recently_used(current_account).where.not(id: featured_tag_ids).limit(10) + end + + def featured_tag_ids + current_account.featured_tags.pluck(:tag_id) end end diff --git a/app/controllers/concerns/cache_concern.rb b/app/controllers/concerns/cache_concern.rb index 088b8db56a5eef..e4e32cdad9dc8c 100644 --- a/app/controllers/concerns/cache_concern.rb +++ b/app/controllers/concerns/cache_concern.rb @@ -208,34 +208,19 @@ def render_with_cache(**options) end end + # TODO: Rename this method, as it does not perform any caching anymore. def cache_collection(raw, klass) - return raw unless klass.respond_to?(:with_includes) + return raw unless klass.respond_to?(:preload_cacheable_associations) - raw = raw.cache_ids.to_a if raw.is_a?(ActiveRecord::Relation) - return [] if raw.empty? + records = raw.to_a - cached_keys_with_value = begin - Rails.cache.read_multi(*raw).transform_keys(&:id).transform_values { |r| ActiveRecordCoder.load(r) } - rescue ActiveRecordCoder::Error - {} # The serialization format may have changed, let's pretend it's a cache miss. - end - - uncached_ids = raw.map(&:id) - cached_keys_with_value.keys - - klass.reload_stale_associations!(cached_keys_with_value.values) if klass.respond_to?(:reload_stale_associations!) - - unless uncached_ids.empty? - uncached = klass.where(id: uncached_ids).with_includes.index_by(&:id) - - uncached.each_value do |item| - Rails.cache.write(item, ActiveRecordCoder.dump(item)) - end - end + klass.preload_cacheable_associations(records) - raw.filter_map { |item| cached_keys_with_value[item.id] || uncached[item.id] } + records end + # TODO: Rename this method, as it does not perform any caching anymore. def cache_collection_paginated_by_id(raw, klass, limit, options) - cache_collection raw.cache_ids.to_a_paginated_by_id(limit, options), klass + cache_collection raw.to_a_paginated_by_id(limit, options), klass end end diff --git a/app/controllers/well_known/webfinger_controller.rb b/app/controllers/well_known/webfinger_controller.rb index 4748940f7c2422..6cf37c2ff04b5a 100644 --- a/app/controllers/well_known/webfinger_controller.rb +++ b/app/controllers/well_known/webfinger_controller.rb @@ -21,7 +21,7 @@ def set_account username = username_from_resource @account = begin - if username == Rails.configuration.x.local_domain + if username == Rails.configuration.x.local_domain || username == Rails.configuration.x.web_domain Account.representative else Account.find_local!(username) diff --git a/app/lib/activitypub/parser/status_parser.rb b/app/lib/activitypub/parser/status_parser.rb index 3db137ee737515..322e372c137b63 100644 --- a/app/lib/activitypub/parser/status_parser.rb +++ b/app/lib/activitypub/parser/status_parser.rb @@ -3,6 +3,8 @@ class ActivityPub::Parser::StatusParser include JsonLdHelper + NORMALIZED_LOCALE_NAMES = LanguagesHelper::SUPPORTED_LOCALES.keys.index_by(&:downcase).freeze + # @param [Hash] json # @param [Hash] magic_values # @option magic_values [String] :followers_collection @@ -99,6 +101,13 @@ def limited_scope end def language + lang = raw_language_code + lang.presence && NORMALIZED_LOCALE_NAMES.fetch(lang.downcase.to_sym, lang) + end + + private + + def raw_language_code if content_language_map? @object['contentMap'].keys.first elsif name_language_map? @@ -108,8 +117,6 @@ def language end end - private - def audience_to as_array(@object['to'] || @json['to']).map { |x| value_or_id(x) } end diff --git a/app/lib/vacuum/applications_vacuum.rb b/app/lib/vacuum/applications_vacuum.rb deleted file mode 100644 index ba88655f167c25..00000000000000 --- a/app/lib/vacuum/applications_vacuum.rb +++ /dev/null @@ -1,10 +0,0 @@ -# frozen_string_literal: true - -class Vacuum::ApplicationsVacuum - def perform - Doorkeeper::Application.where(owner_id: nil) - .where.missing(:created_users, :access_tokens, :access_grants) - .where(created_at: ...1.day.ago) - .in_batches.delete_all - end -end diff --git a/app/lib/video_metadata_extractor.rb b/app/lib/video_metadata_extractor.rb index f27d34868a2798..df5409375f16ef 100644 --- a/app/lib/video_metadata_extractor.rb +++ b/app/lib/video_metadata_extractor.rb @@ -22,7 +22,7 @@ def valid? private def ffmpeg_command_output - command = Terrapin::CommandLine.new('ffprobe', '-i :path -print_format :format -show_format -show_streams -show_error -loglevel :loglevel') + command = Terrapin::CommandLine.new(Rails.configuration.x.ffprobe_binary, '-i :path -print_format :format -show_format -show_streams -show_error -loglevel :loglevel') command.run(path: @path, format: 'json', loglevel: 'fatal') end diff --git a/app/models/concerns/account_interactions.rb b/app/models/concerns/account_interactions.rb index 0252242d68c245..fd849c9af29d42 100644 --- a/app/models/concerns/account_interactions.rb +++ b/app/models/concerns/account_interactions.rb @@ -185,7 +185,7 @@ def unmute_conversation!(conversation) end def unblock_domain!(other_domain) - block = domain_blocks.find_by(domain: other_domain) + block = domain_blocks.find_by(domain: normalized_domain(other_domain)) block&.destroy end @@ -333,4 +333,8 @@ def relations_map(account_ids, domains = nil, **options) def remove_potential_friendship(other_account) PotentialFriendshipTracker.remove(id, other_account.id) end + + def normalized_domain(domain) + TagManager.instance.normalize_domain(domain) + end end diff --git a/app/models/concerns/cacheable.rb b/app/models/concerns/cacheable.rb index d7524cdfd011b0..0633f20c779681 100644 --- a/app/models/concerns/cacheable.rb +++ b/app/models/concerns/cacheable.rb @@ -14,6 +14,10 @@ def with_includes includes(@cache_associated) end + def preload_cacheable_associations(records) + ActiveRecord::Associations::Preloader.new(records: records, associations: @cache_associated).call + end + def cache_ids select(:id, :updated_at) end diff --git a/app/models/feed.rb b/app/models/feed.rb index f51dcfab1dad66..30073fed4b4f38 100644 --- a/app/models/feed.rb +++ b/app/models/feed.rb @@ -28,7 +28,7 @@ def from_redis(limit, max_id, since_id, min_id) unhydrated = redis.zrangebyscore(key, "(#{min_id}", "(#{max_id}", limit: [0, limit], with_scores: true).map(&:first).map(&:to_i) end - Status.where(id: unhydrated).cache_ids + Status.where(id: unhydrated) end def key diff --git a/app/models/public_feed.rb b/app/models/public_feed.rb index a641e77039194d..fcd006d5b870f7 100644 --- a/app/models/public_feed.rb +++ b/app/models/public_feed.rb @@ -31,7 +31,7 @@ def get(limit, max_id = nil, since_id = nil, min_id = nil) # scope.merge!(anonymous_scope) unless account? scope = to_anonymous_scope(scope) unless account? - scope.cache_ids.to_a_paginated_by_id(limit, max_id: max_id, since_id: since_id, min_id: min_id) + scope.to_a_paginated_by_id(limit, max_id: max_id, since_id: since_id, min_id: min_id) end private diff --git a/app/models/status.rb b/app/models/status.rb index 69779eccb38557..244247b978ed22 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -490,38 +490,6 @@ def emoji_reaction_allows_map(status_ids, account_id) Status.where(id: status_ids).pluck(:account_id).uniq.index_with { |a| Account.find_by(id: a).show_emoji_reaction?(my_account) } end - def reload_stale_associations!(cached_items) - account_ids = [] - - cached_items.each do |item| - account_ids << item.account_id - account_ids << item.reblog.account_id if item.reblog? - end - - account_ids.uniq! - - status_ids = cached_items.map { |item| item.reblog? ? item.reblog_of_id : item.id }.uniq - - return if account_ids.empty? - - accounts = Account.where(id: account_ids).includes(:account_stat, :user).index_by(&:id) - - status_stats = StatusStat.where(status_id: status_ids).index_by(&:status_id) - - cached_items.each do |item| - item.account = accounts[item.account_id] - item.reblog.account = accounts[item.reblog.account_id] if item.reblog? - - if item.reblog? - status_stat = status_stats[item.reblog.id] - item.reblog.status_stat = status_stat if status_stat.present? - else - status_stat = status_stats[item.id] - item.status_stat = status_stat if status_stat.present? - end - end - end - def from_text(text) return [] if text.blank? diff --git a/app/models/tag_feed.rb b/app/models/tag_feed.rb index 9c679a02495453..28c0b2ee45f250 100644 --- a/app/models/tag_feed.rb +++ b/app/models/tag_feed.rb @@ -35,7 +35,7 @@ def get(limit, max_id = nil, since_id = nil, min_id = nil) # scope.merge!(anonymous_scope) unless account? scope = to_anonymous_scope(scope) unless account? - scope.cache_ids.to_a_paginated_by_id(limit, max_id: max_id, since_id: since_id, min_id: min_id) + scope.to_a_paginated_by_id(limit, max_id: max_id, since_id: since_id, min_id: min_id) end private diff --git a/app/models/user.rb b/app/models/user.rb index b3003b25b8c058..82d95b996c4a58 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -98,6 +98,8 @@ class User < ApplicationRecord accepts_nested_attributes_for :invite_request, reject_if: ->(attributes) { attributes['text'].blank? && !Setting.require_invite_text } validates :invite_request, presence: true, on: :create, if: :invite_text_required? + validates :email, presence: true, email_address: true + validates_with BlacklistedEmailValidator, if: -> { ENV['EMAIL_DOMAIN_LISTS_APPLY_AFTER_CONFIRMATION'] == 'true' || !confirmed? } validates_with EmailMxValidator, if: :validate_email_dns? validates :agreement, acceptance: { allow_nil: false, accept: [true, 'true', '1'] }, on: :create diff --git a/app/serializers/rest/admin/domain_block_serializer.rb b/app/serializers/rest/admin/domain_block_serializer.rb index e86fd2fe2def51..d7ec7230c6d2d9 100644 --- a/app/serializers/rest/admin/domain_block_serializer.rb +++ b/app/serializers/rest/admin/domain_block_serializer.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class REST::Admin::DomainBlockSerializer < ActiveModel::Serializer - attributes :id, :domain, :created_at, :severity, + attributes :id, :domain, :digest, :created_at, :severity, :reject_media, :reject_favourite, :reject_reply, :reject_reports, :reject_reply_exclude_followers, :reject_send_sensitive, @@ -11,4 +11,8 @@ class REST::Admin::DomainBlockSerializer < ActiveModel::Serializer def id object.id.to_s end + + def digest + object.domain_digest + end end diff --git a/app/services/notify_service.rb b/app/services/notify_service.rb index eaa5ce233b4cda..993ab33c1742b3 100644 --- a/app/services/notify_service.rb +++ b/app/services/notify_service.rb @@ -73,16 +73,17 @@ def response_to_recipient? LEFT JOIN mentions m ON m.silent = FALSE AND m.account_id = :sender_id AND m.status_id = s.id WHERE s.id = :id UNION ALL - SELECT s.id, s.in_reply_to_id, m.id, st.path || s.id, st.depth + 1 - FROM ancestors st - JOIN statuses s ON s.id = st.in_reply_to_id - LEFT JOIN mentions m ON m.silent = FALSE AND m.account_id = :sender_id AND m.status_id = s.id - WHERE st.mention_id IS NULL AND NOT s.id = ANY(path) AND st.depth < :depth_limit + SELECT s.id, s.in_reply_to_id, m.id, ancestors.path || s.id, ancestors.depth + 1 + FROM ancestors + JOIN statuses s ON s.id = ancestors.in_reply_to_id + /* early exit if we already have a mention matching our requirements */ + LEFT JOIN mentions m ON m.silent = FALSE AND m.account_id = :sender_id AND m.status_id = s.id AND s.account_id = :recipient_id + WHERE ancestors.mention_id IS NULL AND NOT s.id = ANY(path) AND ancestors.depth < :depth_limit ) SELECT COUNT(*) - FROM ancestors st - JOIN statuses s ON s.id = st.id - WHERE st.mention_id IS NOT NULL AND s.visibility = 3 + FROM ancestors + JOIN statuses s ON s.id = ancestors.id + WHERE ancestors.mention_id IS NOT NULL AND s.account_id = :recipient_id AND s.visibility = 3 SQL end diff --git a/app/services/post_status_service.rb b/app/services/post_status_service.rb index f48555245ef20d..9026ec739f7b23 100644 --- a/app/services/post_status_service.rb +++ b/app/services/post_status_service.rb @@ -240,7 +240,7 @@ def idempotency_given? def idempotency_duplicate if scheduled? - @account.schedule_statuses.find(@idempotency_duplicate) + @account.scheduled_statuses.find(@idempotency_duplicate) else @account.statuses.find(@idempotency_duplicate) end @@ -298,7 +298,7 @@ def poll_attributes end def scheduled_options - @options.tap do |options_hash| + @options.dup.tap do |options_hash| 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 diff --git a/app/validators/email_address_validator.rb b/app/validators/email_address_validator.rb new file mode 100644 index 00000000000000..ed0bb116524aec --- /dev/null +++ b/app/validators/email_address_validator.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +# NOTE: I initially wrote this as `EmailValidator` but it ended up clashing +# with an indirect dependency of ours, `validate_email`, which, turns out, +# has the same approach as we do, but with an extra check disallowing +# single-label domains. Decided to not switch to `validate_email` because +# we do want to allow at least `localhost`. + +class EmailAddressValidator < ActiveModel::EachValidator + def validate_each(record, attribute, value) + value = value.strip + + address = Mail::Address.new(value) + record.errors.add(attribute, :invalid) if address.address != value + rescue Mail::Field::FieldError + record.errors.add(attribute, :invalid) + end +end diff --git a/app/workers/scheduler/vacuum_scheduler.rb b/app/workers/scheduler/vacuum_scheduler.rb index 1c9a2aabe3be57..c22d6f5f80fc98 100644 --- a/app/workers/scheduler/vacuum_scheduler.rb +++ b/app/workers/scheduler/vacuum_scheduler.rb @@ -22,7 +22,6 @@ def vacuum_operations preview_cards_vacuum, backups_vacuum, access_tokens_vacuum, - applications_vacuum, feeds_vacuum, imports_vacuum, ] @@ -56,10 +55,6 @@ def imports_vacuum Vacuum::ImportsVacuum.new end - def applications_vacuum - Vacuum::ApplicationsVacuum.new - end - def content_retention_policy ContentRetentionPolicy.current end diff --git a/config/application.rb b/config/application.rb index 2a62c37e8be4e5..c2a63c869786f8 100644 --- a/config/application.rb +++ b/config/application.rb @@ -48,6 +48,7 @@ require_relative '../lib/webpacker/manifest_extensions' require_relative '../lib/webpacker/helper_extensions' require_relative '../lib/rails/engine_extensions' +require_relative '../lib/action_dispatch/remote_ip_extensions' require_relative '../lib/active_record/database_tasks_extensions' require_relative '../lib/active_record/batches' require_relative '../lib/simple_navigation/item_extensions' diff --git a/config/initializers/ffmpeg.rb b/config/initializers/ffmpeg.rb index 30ea617fcd2a86..87f85eeec70007 100644 --- a/config/initializers/ffmpeg.rb +++ b/config/initializers/ffmpeg.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true -if ENV['FFMPEG_BINARY'].present? - FFMPEG.ffmpeg_binary = ENV['FFMPEG_BINARY'] +Rails.application.configure do + config.x.ffmpeg_binary = ENV['FFMPEG_BINARY'] || 'ffmpeg' + config.x.ffprobe_binary = ENV['FFPROBE_BINARY'] || 'ffprobe' end diff --git a/config/initializers/rack_attack.rb b/config/initializers/rack_attack.rb index d0af0fe9409fb0..6d8284e2b45b40 100644 --- a/config/initializers/rack_attack.rb +++ b/config/initializers/rack_attack.rb @@ -37,6 +37,10 @@ def authenticated_token_id authenticated_token&.id end + def warden_user_id + @env['warden']&.user&.id + end + def unauthenticated? !authenticated_user_id end @@ -58,10 +62,6 @@ def paging_request? end end - Rack::Attack.safelist('allow from localhost') do |req| - req.remote_ip == '127.0.0.1' || req.remote_ip == '::1' - end - Rack::Attack.blocklist('deny from blocklist') do |req| IpBlock.blocked?(req.remote_ip) end @@ -105,6 +105,10 @@ def paging_request? req.authenticated_user_id if (req.post? && req.path.match?(API_DELETE_REBLOG_REGEX)) || (req.delete? && req.path.match?(API_DELETE_STATUS_REGEX)) end + throttle('throttle_oauth_application_registrations/ip', limit: 5, period: 10.minutes) do |req| + req.throttleable_remote_ip if req.post? && req.path == '/api/v1/apps' + end + throttle('throttle_sign_up_attempts/ip', limit: 25, period: 5.minutes) do |req| req.throttleable_remote_ip if req.post? && req.path_matches?('/auth') end @@ -137,6 +141,10 @@ def paging_request? req.session[:attempt_user_id] || req.params.dig('user', 'email').presence if req.post? && req.path_matches?('/auth/sign_in') end + throttle('throttle_password_change/account', limit: 10, period: 10.minutes) do |req| + req.warden_user_id if req.put? || (req.patch? && req.path_matches?('/auth')) + end + self.throttled_responder = lambda do |request| now = Time.now.utc match_data = request.env['rack.attack.match_data'] diff --git a/docker-compose.yml b/docker-compose.yml index 004f5dbb0f8b04..10a57e31e3e7bd 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -56,7 +56,7 @@ services: web: build: . - image: ghcr.io/mastodon/mastodon:v4.2.8 + image: ghcr.io/mastodon/mastodon:v4.2.9 restart: always env_file: .env.production command: bash -c "rm -f /mastodon/tmp/pids/server.pid; bundle exec rails s -p 3000" @@ -77,7 +77,7 @@ services: streaming: build: . - image: ghcr.io/mastodon/mastodon:v4.2.8 + image: ghcr.io/mastodon/mastodon:v4.2.9 restart: always env_file: .env.production command: node ./streaming @@ -95,7 +95,7 @@ services: sidekiq: build: . - image: ghcr.io/mastodon/mastodon:v4.2.8 + image: ghcr.io/mastodon/mastodon:v4.2.9 restart: always env_file: .env.production command: bundle exec sidekiq diff --git a/lib/action_dispatch/remote_ip_extensions.rb b/lib/action_dispatch/remote_ip_extensions.rb new file mode 100644 index 00000000000000..e5c48bf3c5b0ee --- /dev/null +++ b/lib/action_dispatch/remote_ip_extensions.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +# Mastodon is not made to be directly accessed without a reverse proxy. +# This monkey-patch prevents remote IP address spoofing when being accessed +# directly. +# +# See PR: https://github.com/rails/rails/pull/51610 + +# In addition to the PR above, it also raises an error if a request with +# `X-Forwarded-For` or `Client-Ip` comes directly from a client without +# going through a trusted proxy. + +# rubocop:disable all -- This is a mostly vendored file + +module ActionDispatch + class RemoteIp + module GetIpExtensions + def calculate_ip + # Set by the Rack web server, this is a single value. + remote_addr = ips_from(@req.remote_addr).last + + # Could be a CSV list and/or repeated headers that were concatenated. + client_ips = ips_from(@req.client_ip).reverse! + forwarded_ips = ips_from(@req.x_forwarded_for).reverse! + + # `Client-Ip` and `X-Forwarded-For` should not, generally, both be set. If they + # are both set, it means that either: + # + # 1) This request passed through two proxies with incompatible IP header + # conventions. + # + # 2) The client passed one of `Client-Ip` or `X-Forwarded-For` + # (whichever the proxy servers weren't using) themselves. + # + # Either way, there is no way for us to determine which header is the right one + # after the fact. Since we have no idea, if we are concerned about IP spoofing + # we need to give up and explode. (If you're not concerned about IP spoofing you + # can turn the `ip_spoofing_check` option off.) + should_check_ip = @check_ip && client_ips.last && forwarded_ips.last + if should_check_ip && !forwarded_ips.include?(client_ips.last) + # We don't know which came from the proxy, and which from the user + raise IpSpoofAttackError, "IP spoofing attack?! " \ + "HTTP_CLIENT_IP=#{@req.client_ip.inspect} " \ + "HTTP_X_FORWARDED_FOR=#{@req.x_forwarded_for.inspect}" + end + + # NOTE: Mastodon addition to make sure we don't get requests from a non-trusted client + if @check_ip && (forwarded_ips.last || client_ips.last) && !@proxies.any? { |proxy| proxy === remote_addr } + raise IpSpoofAttackError, "IP spoofing attack?! client #{remote_addr} is not a trusted proxy " \ + "HTTP_CLIENT_IP=#{@req.client_ip.inspect} " \ + "HTTP_X_FORWARDED_FOR=#{@req.x_forwarded_for.inspect}" + end + + # We assume these things about the IP headers: + # + # - X-Forwarded-For will be a list of IPs, one per proxy, or blank + # - Client-Ip is propagated from the outermost proxy, or is blank + # - REMOTE_ADDR will be the IP that made the request to Rack + ips = forwarded_ips + client_ips + ips.compact! + + # If every single IP option is in the trusted list, return the IP that's + # furthest away + filter_proxies([remote_addr] + ips).first || ips.last || remote_addr + end + end + end +end + +ActionDispatch::RemoteIp::GetIp.prepend(ActionDispatch::RemoteIp::GetIpExtensions) + +# rubocop:enable all diff --git a/lib/mastodon/cli/maintenance.rb b/lib/mastodon/cli/maintenance.rb index e73bcbf86a0ab5..c2a6802e1d73a6 100644 --- a/lib/mastodon/cli/maintenance.rb +++ b/lib/mastodon/cli/maintenance.rb @@ -224,7 +224,7 @@ def deduplicate_users! users = User.where(id: row['ids'].split(',')).sort_by(&:updated_at).reverse ref_user = users.shift say "Multiple users registered with e-mail address #{ref_user.email}.", :yellow - say "e-mail will be disabled for the following accounts: #{user.map(&:account).map(&:acct).join(', ')}", :yellow + say "e-mail will be disabled for the following accounts: #{users.map { |user| user.account.acct }.join(', ')}", :yellow say 'Please reach out to them and set another address with `tootctl account modify` or delete them.', :yellow users.each_with_index do |user, index| diff --git a/lib/mastodon/cli/media.rb b/lib/mastodon/cli/media.rb index c906161770a6ea..5879c532e80a99 100644 --- a/lib/mastodon/cli/media.rb +++ b/lib/mastodon/cli/media.rb @@ -134,7 +134,7 @@ def remove_orphans model_name = path_segments.first.classify attachment_name = path_segments[1].singularize - record_id = path_segments[2..-2].join.to_i + record_id = path_segments[2...-2].join.to_i file_name = path_segments.last record = record_map.dig(model_name, record_id) attachment = record&.public_send(attachment_name) @@ -180,7 +180,7 @@ def remove_orphans end model_name = path_segments.first.classify - record_id = path_segments[2..-2].join.to_i + record_id = path_segments[2...-2].join.to_i attachment_name = path_segments[1].singularize file_name = path_segments.last @@ -311,7 +311,7 @@ def lookup(url) end model_name = path_segments.first.classify - record_id = path_segments[2..-2].join.to_i + record_id = path_segments[2...-2].join.to_i unless PRELOAD_MODEL_WHITELIST.include?(model_name) say("Cannot find corresponding model: #{model_name}", :red) @@ -361,7 +361,7 @@ def preload_records_from_mixed_objects(objects) next unless VALID_PATH_SEGMENTS_SIZE.include?(segments.size) model_name = segments.first.classify - record_id = segments[2..-2].join.to_i + record_id = segments[2...-2].join.to_i next unless PRELOAD_MODEL_WHITELIST.include?(model_name) diff --git a/lib/mastodon/sidekiq_middleware.rb b/lib/mastodon/sidekiq_middleware.rb index 3a747afb63c260..ac63d4d38ce60a 100644 --- a/lib/mastodon/sidekiq_middleware.rb +++ b/lib/mastodon/sidekiq_middleware.rb @@ -8,6 +8,7 @@ def call(*, &block) rescue Mastodon::HostValidationError # Do not retry rescue => e + clean_up_elasticsearch_connections! limit_backtrace_and_raise(e) ensure clean_up_sockets! @@ -25,6 +26,32 @@ def clean_up_sockets! clean_up_statsd_socket! end + # This is a hack to immediately free up unused Elasticsearch connections. + # + # Indeed, Chewy creates one `Elasticsearch::Client` instance per thread, + # and each such client manages its long-lasting connection to + # Elasticsearch. + # + # As far as I know, neither `chewy`, `elasticsearch-transport` or even + # `faraday` provide a reliable way to immediately close a connection, and + # rely on the underlying object to be garbage-collected instead. + # + # Furthermore, `sidekiq` creates a new thread each time a job throws an + # exception, meaning that each failure will create a new connection, and + # the old one will only be closed on full garbage collection. + def clean_up_elasticsearch_connections! + return unless Chewy.enabled? && Chewy.current[:chewy_client].present? + + Chewy.client.transport.connections.each do |connection| + # NOTE: This bit of code is tailored for the HTTPClient Faraday adapter + connection.connection.app.instance_variable_get(:@client)&.reset_all + end + + Chewy.current.delete(:chewy_client) + rescue + nil + end + def clean_up_redis_socket! RedisConfiguration.pool.checkin if Thread.current[:redis] Thread.current[:redis] = nil diff --git a/lib/mastodon/version.rb b/lib/mastodon/version.rb index 104e1140b946c1..4071ff3e81a042 100644 --- a/lib/mastodon/version.rb +++ b/lib/mastodon/version.rb @@ -9,7 +9,7 @@ def kmyblue_major end def kmyblue_minor - 18 + 19 end def kmyblue_flag @@ -25,7 +25,7 @@ def minor end def patch - 8 + 9 end def default_prerelease diff --git a/lib/paperclip/image_extractor.rb b/lib/paperclip/image_extractor.rb index 17fe4326fdbf08..8a565d0469e1f5 100644 --- a/lib/paperclip/image_extractor.rb +++ b/lib/paperclip/image_extractor.rb @@ -35,7 +35,7 @@ def extract_image_from_file! dst.binmode begin - command = Terrapin::CommandLine.new('ffmpeg', '-i :source -loglevel :loglevel -y :destination', logger: Paperclip.logger) + command = Terrapin::CommandLine.new(Rails.configuration.x.ffmpeg_binary, '-i :source -loglevel :loglevel -y :destination', logger: Paperclip.logger) command.run(source: @file.path, destination: dst.path, loglevel: 'fatal') rescue Terrapin::ExitStatusError dst.close(true) diff --git a/lib/paperclip/transcoder.rb b/lib/paperclip/transcoder.rb index d2d946d3ade491..3efffa355a52d1 100644 --- a/lib/paperclip/transcoder.rb +++ b/lib/paperclip/transcoder.rb @@ -61,7 +61,7 @@ def make command_arguments, interpolations = prepare_command(destination) begin - command = Terrapin::CommandLine.new('ffmpeg', command_arguments.join(' '), logger: Paperclip.logger) + command = Terrapin::CommandLine.new(Rails.configuration.x.ffmpeg_binary, command_arguments.join(' '), logger: Paperclip.logger) command.run(interpolations) rescue Terrapin::ExitStatusError => e raise Paperclip::Error, "Error while transcoding #{@basename}: #{e}" diff --git a/lib/tasks/mastodon.rake b/lib/tasks/mastodon.rake index dfa2c5a8e1283c..aa612fe4869794 100644 --- a/lib/tasks/mastodon.rake +++ b/lib/tasks/mastodon.rake @@ -515,6 +515,7 @@ namespace :mastodon do owner_role = UserRole.find_by(name: 'Owner') user = User.new(email: email, password: password, confirmed_at: Time.now.utc, account_attributes: { username: username }, bypass_invite_request_check: true, role: owner_role) user.save(validate: false) + user.approve! Setting.site_contact_username = username diff --git a/spec/config/initializers/rack_attack_spec.rb b/spec/config/initializers/rack_attack_spec.rb index 7cd4ac76bbb3ab..78c4bf03a549f9 100644 --- a/spec/config/initializers/rack_attack_spec.rb +++ b/spec/config/initializers/rack_attack_spec.rb @@ -103,4 +103,46 @@ def app it_behaves_like 'throttled endpoint' end end + + describe 'throttle excessive oauth application registration requests by IP address' do + let(:throttle) { 'throttle_oauth_application_registrations/ip' } + let(:limit) { 5 } + let(:period) { 10.minutes } + let(:path) { '/api/v1/apps' } + let(:params) do + { + client_name: 'Throttle Test', + redirect_uris: 'urn:ietf:wg:oauth:2.0:oob', + scopes: 'read', + } + end + + let(:request) { -> { post path, params: params, headers: { 'REMOTE_ADDR' => remote_ip } } } + + it_behaves_like 'throttled endpoint' + end + + describe 'throttle excessive password change requests by account' do + let(:user) { Fabricate(:user, email: 'user@host.example') } + let(:limit) { 10 } + let(:period) { 10.minutes } + let(:request) { -> { put path, headers: { 'REMOTE_ADDR' => remote_ip } } } + let(:path) { '/auth' } + + before do + sign_in user, scope: :user + + # Unfortunately, devise's `sign_in` helper causes the `session` to be + # loaded in the next request regardless of whether it's actually accessed + # by the client code. + # + # So, we make an extra query to clear issue a session cookie instead. + # + # A less resource-intensive way to deal with that would be to generate the + # session cookie manually, but this seems pretty involved. + get '/' + end + + it_behaves_like 'throttled endpoint' + end end diff --git a/spec/controllers/api/v1/featured_tags/suggestions_controller_spec.rb b/spec/controllers/api/v1/featured_tags/suggestions_controller_spec.rb index 54c63dcc6f19b5..8cb928ea2c5e3e 100644 --- a/spec/controllers/api/v1/featured_tags/suggestions_controller_spec.rb +++ b/spec/controllers/api/v1/featured_tags/suggestions_controller_spec.rb @@ -7,17 +7,39 @@ let(:user) { Fabricate(:user) } let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: 'read:accounts') } - let(:account) { Fabricate(:account) } + let(:account) { Fabricate(:account, user: user) } before do allow(controller).to receive(:doorkeeper_token) { token } end describe 'GET #index' do - it 'returns http success' do + let!(:unused_featured_tag) { Fabricate(:tag, name: 'unused_featured_tag') } + let!(:used_tag) { Fabricate(:tag, name: 'used_tag') } + let!(:used_featured_tag) { Fabricate(:tag, name: 'used_featured_tag') } + + before do + _unused_tag = Fabricate(:tag, name: 'unused_tag') + + # Make relevant tags used by account + status = Fabricate(:status, account: account) + status.tags << used_tag + status.tags << used_featured_tag + + # Feature the relevant tags + Fabricate :featured_tag, account: account, name: unused_featured_tag.name + Fabricate :featured_tag, account: account, name: used_featured_tag.name + end + + it 'returns http success and recently used but not featured tags', :aggregate_failures do get :index, params: { account_id: account.id, limit: 2 } - expect(response).to have_http_status(200) + expect(response) + .to have_http_status(200) + expect(body_as_json) + .to contain_exactly( + include(name: used_tag.name) + ) end end end diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index e9d47960353fdb..5cd45e991914b3 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -221,39 +221,4 @@ def route_unprocessable_entity include_examples 'respond_with_error', 422 end - - describe 'cache_collection' do - subject do - Class.new(ApplicationController) do - public :cache_collection - end - end - - shared_examples 'receives :with_includes' do |fabricator, klass| - it 'uses raw if it is not an ActiveRecord::Relation' do - record = Fabricate(fabricator) - expect(subject.new.cache_collection([record], klass)).to eq [record] - end - end - - shared_examples 'cacheable' do |fabricator, klass| - include_examples 'receives :with_includes', fabricator, klass - - it 'calls cache_ids of raw if it is an ActiveRecord::Relation' do - record = Fabricate(fabricator) - relation = klass.none - allow(relation).to receive(:cache_ids).and_return([record]) - expect(subject.new.cache_collection(relation, klass)).to eq [record] - end - end - - it 'returns raw unless class responds to :with_includes' do - raw = Object.new - expect(subject.new.cache_collection(raw, Object)).to eq raw - end - - context 'with a Status' do - include_examples 'cacheable', :status, Status - end - end end diff --git a/spec/fabricators/featured_tag_fabricator.rb b/spec/fabricators/featured_tag_fabricator.rb index 0803dc43a72e34..6003099dbdb3dc 100644 --- a/spec/fabricators/featured_tag_fabricator.rb +++ b/spec/fabricators/featured_tag_fabricator.rb @@ -2,6 +2,6 @@ Fabricator(:featured_tag) do account { Fabricate.build(:account) } - tag { Fabricate.build(:tag) } + tag { nil } name { sequence(:name) { |i| "Tag#{i}" } } end diff --git a/spec/lib/activitypub/parser/status_parser_spec.rb b/spec/lib/activitypub/parser/status_parser_spec.rb new file mode 100644 index 00000000000000..5d9f008db158d5 --- /dev/null +++ b/spec/lib/activitypub/parser/status_parser_spec.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe ActivityPub::Parser::StatusParser do + subject { described_class.new(json) } + + let(:sender) { Fabricate(:account, followers_url: 'http://example.com/followers', domain: 'example.com', uri: 'https://example.com/actor') } + let(:follower) { Fabricate(:account, username: 'bob') } + + let(:json) do + { + '@context': 'https://www.w3.org/ns/activitystreams', + id: [ActivityPub::TagManager.instance.uri_for(sender), '#foo'].join, + type: 'Create', + actor: ActivityPub::TagManager.instance.uri_for(sender), + object: object_json, + }.with_indifferent_access + end + + let(:object_json) do + { + id: [ActivityPub::TagManager.instance.uri_for(sender), 'post1'].join('/'), + type: 'Note', + to: [ + 'https://www.w3.org/ns/activitystreams#Public', + ActivityPub::TagManager.instance.uri_for(follower), + ], + content: '@bob lorem ipsum', + contentMap: { + EN: '@bob lorem ipsum', + }, + published: 1.hour.ago.utc.iso8601, + updated: 1.hour.ago.utc.iso8601, + tag: { + type: 'Mention', + href: ActivityPub::TagManager.instance.uri_for(follower), + }, + } + end + + it 'correctly parses status' do + expect(subject).to have_attributes( + text: '@bob lorem ipsum', + uri: [ActivityPub::TagManager.instance.uri_for(sender), 'post1'].join('/'), + reply: false, + language: :en + ) + end +end diff --git a/spec/lib/vacuum/applications_vacuum_spec.rb b/spec/lib/vacuum/applications_vacuum_spec.rb deleted file mode 100644 index 57a222aafc8589..00000000000000 --- a/spec/lib/vacuum/applications_vacuum_spec.rb +++ /dev/null @@ -1,48 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe Vacuum::ApplicationsVacuum do - subject { described_class.new } - - describe '#perform' do - let!(:app_with_token) { Fabricate(:application, created_at: 1.month.ago) } - let!(:app_with_grant) { Fabricate(:application, created_at: 1.month.ago) } - let!(:app_with_signup) { Fabricate(:application, created_at: 1.month.ago) } - let!(:app_with_owner) { Fabricate(:application, created_at: 1.month.ago, owner: Fabricate(:user)) } - let!(:unused_app) { Fabricate(:application, created_at: 1.month.ago) } - let!(:recent_app) { Fabricate(:application, created_at: 1.hour.ago) } - - let!(:active_access_token) { Fabricate(:access_token, application: app_with_token) } - let!(:active_access_grant) { Fabricate(:access_grant, application: app_with_grant) } - let!(:user) { Fabricate(:user, created_by_application: app_with_signup) } - - before do - subject.perform - end - - it 'does not delete applications with valid access tokens' do - expect { app_with_token.reload }.to_not raise_error - end - - it 'does not delete applications with valid access grants' do - expect { app_with_grant.reload }.to_not raise_error - end - - it 'does not delete applications that were used to create users' do - expect { app_with_signup.reload }.to_not raise_error - end - - it 'does not delete owned applications' do - expect { app_with_owner.reload }.to_not raise_error - end - - it 'does not delete applications registered less than a day ago' do - expect { recent_app.reload }.to_not raise_error - end - - it 'deletes unused applications' do - expect { unused_app.reload }.to raise_error ActiveRecord::RecordNotFound - end - end -end diff --git a/spec/models/concerns/account_interactions_spec.rb b/spec/models/concerns/account_interactions_spec.rb index 0b8825135d1f68..7519b5b09496ac 100644 --- a/spec/models/concerns/account_interactions_spec.rb +++ b/spec/models/concerns/account_interactions_spec.rb @@ -252,6 +252,24 @@ end end + describe '#block_idna_domain!' do + subject do + [ + account.block_domain!(idna_domain), + account.block_domain!(punycode_domain), + ] + end + + let(:idna_domain) { '대한민국.한국' } + let(:punycode_domain) { 'xn--3e0bs9hfvinn1a.xn--3e0b707e' } + + it 'creates single AccountDomainBlock' do + expect do + expect(subject).to all(be_a AccountDomainBlock) + end.to change { account.domain_blocks.count }.by 1 + end + end + describe '#unfollow!' do subject { account.unfollow!(target_account) } @@ -347,6 +365,28 @@ end end + describe '#unblock_idna_domain!' do + subject { account.unblock_domain!(punycode_domain) } + + let(:idna_domain) { '대한민국.한국' } + let(:punycode_domain) { 'xn--3e0bs9hfvinn1a.xn--3e0b707e' } + + context 'when blocking the domain' do + it 'returns destroyed AccountDomainBlock' do + account_domain_block = Fabricate(:account_domain_block, domain: idna_domain) + account.domain_blocks << account_domain_block + expect(subject).to be_a AccountDomainBlock + expect(subject).to be_destroyed + end + end + + context 'when unblocking idna domain' do + it 'returns nil' do + expect(subject).to be_nil + end + end + end + describe '#following?' do subject { account.following?(target_account) } diff --git a/spec/models/home_feed_spec.rb b/spec/models/home_feed_spec.rb index bd649d82693ce0..06bb63b1a42be8 100644 --- a/spec/models/home_feed_spec.rb +++ b/spec/models/home_feed_spec.rb @@ -27,7 +27,6 @@ results = subject.get(3) expect(results.map(&:id)).to eq [3, 2] - expect(results.first.attributes.keys).to eq %w(id updated_at) end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index ffd1889cbd93c3..f06150f02c612d 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -39,6 +39,12 @@ expect(user.valid?).to be true end + it 'is valid with a localhost e-mail address' do + user = Fabricate.build(:user, email: 'admin@localhost') + user.valid? + expect(user.valid?).to be true + end + it 'cleans out invalid locale' do user = Fabricate.build(:user, locale: 'toto') expect(user.valid?).to be true diff --git a/spec/requests/api/v1/admin/domain_blocks_spec.rb b/spec/requests/api/v1/admin/domain_blocks_spec.rb index 86675b2cc761f5..900fc84c936a71 100644 --- a/spec/requests/api/v1/admin/domain_blocks_spec.rb +++ b/spec/requests/api/v1/admin/domain_blocks_spec.rb @@ -49,6 +49,7 @@ { id: domain_block.id.to_s, domain: domain_block.domain, + digest: domain_block.domain_digest, created_at: domain_block.created_at.strftime('%Y-%m-%dT%H:%M:%S.%LZ'), severity: domain_block.severity.to_s, reject_media: domain_block.reject_media, @@ -65,6 +66,7 @@ { id: domain_block.id.to_s, domain: domain_block.domain, + digest: domain_block.domain_digest, created_at: domain_block.created_at.strftime('%Y-%m-%dT%H:%M:%S.%LZ'), severity: domain_block.severity.to_s, reject_media: domain_block.reject_media, @@ -109,28 +111,6 @@ let!(:domain_block) { Fabricate(:domain_block) } - let(:expected_response) do - { - id: domain_block.id.to_s, - domain: domain_block.domain, - created_at: domain_block.created_at.strftime('%Y-%m-%dT%H:%M:%S.%LZ'), - severity: domain_block.severity.to_s, - reject_media: domain_block.reject_media, - reject_reports: domain_block.reject_reports, - private_comment: domain_block.private_comment, - public_comment: domain_block.public_comment, - obfuscate: domain_block.obfuscate, - reject_favourite: domain_block.reject_favourite, - reject_hashtag: domain_block.reject_hashtag, - detect_invalid_subscription: domain_block.detect_invalid_subscription, - reject_new_follow: domain_block.reject_new_follow, - reject_reply: domain_block.reject_reply, - reject_reply_exclude_followers: domain_block.reject_reply_exclude_followers, - reject_send_sensitive: domain_block.reject_send_sensitive, - reject_straight_follow: domain_block.reject_straight_follow, - } - end - it_behaves_like 'forbidden for wrong scope', 'write:statuses' it_behaves_like 'forbidden for wrong role', '' it_behaves_like 'forbidden for wrong role', 'Moderator' @@ -141,10 +121,31 @@ expect(response).to have_http_status(200) end - it 'returns the expected domain block content' do + it 'returns the expected domain block content' do # rubocop:disable RSpec/ExampleLength subject - expect(body_as_json).to eq(expected_response) + expect(body_as_json).to eq( + { + id: domain_block.id.to_s, + domain: domain_block.domain, + digest: domain_block.domain_digest, + created_at: domain_block.created_at.strftime('%Y-%m-%dT%H:%M:%S.%LZ'), + severity: domain_block.severity.to_s, + reject_media: domain_block.reject_media, + reject_reports: domain_block.reject_reports, + private_comment: domain_block.private_comment, + public_comment: domain_block.public_comment, + obfuscate: domain_block.obfuscate, + reject_favourite: domain_block.reject_favourite, + reject_hashtag: domain_block.reject_hashtag, + detect_invalid_subscription: domain_block.detect_invalid_subscription, + reject_new_follow: domain_block.reject_new_follow, + reject_reply: domain_block.reject_reply, + reject_reply_exclude_followers: domain_block.reject_reply_exclude_followers, + reject_send_sensitive: domain_block.reject_send_sensitive, + reject_straight_follow: domain_block.reject_straight_follow, + } + ) end context 'when the requested domain block does not exist' do @@ -167,14 +168,10 @@ it_behaves_like 'forbidden for wrong role', '' it_behaves_like 'forbidden for wrong role', 'Moderator' - it 'returns http success' do + it 'creates a domain block with the expected domain name and severity', :aggregate_failures do subject expect(response).to have_http_status(200) - end - - it 'returns expected domain name and severity' do - subject body = body_as_json @@ -192,7 +189,44 @@ expect(DomainBlock.find_by(domain: 'foo.bar.com')).to be_present end - context 'when a stricter domain block already exists' do + context 'when a looser domain block already exists on a higher level domain' do + let(:params) { { domain: 'foo.bar.com', severity: :suspend } } + + before do + Fabricate(:domain_block, domain: 'bar.com', severity: :silence) + end + + it 'creates a domain block with the expected domain name and severity', :aggregate_failures do + subject + + body = body_as_json + + expect(response).to have_http_status(200) + expect(body).to match a_hash_including( + { + domain: 'foo.bar.com', + severity: 'suspend', + } + ) + + expect(DomainBlock.find_by(domain: 'foo.bar.com')).to be_present + end + end + + context 'when a domain block already exists on the same domain' do + before do + Fabricate(:domain_block, domain: 'foo.bar.com', severity: :silence) + end + + it 'returns existing domain block in error', :aggregate_failures do + subject + + expect(response).to have_http_status(422) + expect(body_as_json[:existing_domain_block][:domain]).to eq('foo.bar.com') + end + end + + context 'when a stricter domain block already exists on a higher level domain' do before do Fabricate(:domain_block, domain: 'bar.com', severity: :suspend) end @@ -246,6 +280,7 @@ { id: domain_block.id.to_s, domain: domain_block.domain, + digest: domain_block.domain_digest, severity: 'suspend', } ) diff --git a/spec/services/notify_service_spec.rb b/spec/services/notify_service_spec.rb index 8fcb5865804a6f..c2664e79c23bb5 100644 --- a/spec/services/notify_service_spec.rb +++ b/spec/services/notify_service_spec.rb @@ -76,10 +76,10 @@ end context 'when the message chain is initiated by recipient, but without a mention to the sender, even if the sender sends multiple messages in a row' do - let(:reply_to) { Fabricate(:status, account: recipient) } - let!(:mention) { Fabricate(:mention, account: sender, status: reply_to) } - let(:dummy_reply) { Fabricate(:status, account: sender, visibility: :direct, thread: reply_to) } - let(:activity) { Fabricate(:mention, account: recipient, status: Fabricate(:status, account: sender, visibility: :direct, thread: dummy_reply)) } + let(:public_status) { Fabricate(:status, account: recipient) } + let(:intermediate_reply) { Fabricate(:status, account: sender, thread: public_status, visibility: :direct) } + let!(:intermediate_mention) { Fabricate(:mention, account: sender, status: intermediate_reply) } + let(:activity) { Fabricate(:mention, account: recipient, status: Fabricate(:status, account: sender, visibility: :direct, thread: intermediate_reply)) } it 'does not notify' do expect { subject }.to_not change(Notification, :count) diff --git a/spec/services/post_status_service_spec.rb b/spec/services/post_status_service_spec.rb index a2cf6fbfb07348..0caaba2f8d9d6f 100644 --- a/spec/services/post_status_service_spec.rb +++ b/spec/services/post_status_service_spec.rb @@ -54,6 +54,13 @@ it 'does not change statuses count' do expect { subject.call(account, text: 'Hi future!', scheduled_at: future, thread: previous_status) }.to_not(change { [account.statuses_count, previous_status.replies_count] }) end + + it 'returns existing status when used twice with idempotency key' do + account = Fabricate(:account) + status1 = subject.call(account, text: 'test', idempotency: 'meepmeep', scheduled_at: future) + status2 = subject.call(account, text: 'test', idempotency: 'meepmeep', scheduled_at: future) + expect(status2.id).to eq status1.id + end end it 'creates response to the original status of boost' do