diff --git a/.github/workflows/test-ruby.yml b/.github/workflows/test-ruby.yml index 90467e46d3f50a..fb956f3eb619e5 100644 --- a/.github/workflows/test-ruby.yml +++ b/.github/workflows/test-ruby.yml @@ -149,3 +149,5 @@ jobs: - name: Load database schema run: './bin/rails db:create db:schema:load db:seed' + + - run: bin/rspec diff --git a/CHANGELOG.md b/CHANGELOG.md index e65ebd8f96247b..b0254d4676bfe5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,39 @@ Changelog All notable changes to this project will be documented in this file. +## [4.1.17] - 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 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)) + +### 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.1.16] - 2024-02-23 ### Added diff --git a/Gemfile b/Gemfile index 6e5292d82486f3..f5b43934a9525d 100644 --- a/Gemfile +++ b/Gemfile @@ -158,3 +158,4 @@ gem 'concurrent-ruby', require: false gem 'connection_pool', require: false gem 'xorcist', '~> 1.1' gem 'cocoon', '~> 1.2' +gem 'mail', '~> 2.8' diff --git a/Gemfile.lock b/Gemfile.lock index 17700ba1e49876..4a1876dea4e6a6 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -330,7 +330,7 @@ GEM jmespath (1.6.2) json (2.6.3) json-canonicalization (0.3.0) - json-jwt (1.15.3) + json-jwt (1.15.3.1) activesupport (>= 4.2) aes_key_wrap bindata @@ -424,7 +424,7 @@ GEM net-protocol net-ssh (7.0.1) nio4r (2.5.9) - nokogiri (1.16.2) + nokogiri (1.16.5) mini_portile2 (~> 2.8.2) racc (~> 1.4) nsa (0.2.8) @@ -491,7 +491,7 @@ GEM pry-rails (0.3.9) pry (>= 0.10.4) public_suffix (5.0.1) - puma (5.6.7) + puma (5.6.8) nio4r (~> 2.0) pundit (2.3.0) activesupport (>= 3.0.0) @@ -565,7 +565,8 @@ GEM responders (3.0.1) actionpack (>= 5.0) railties (>= 5.0) - rexml (3.2.5) + rexml (3.2.8) + strscan (>= 3.0.9) rotp (6.2.0) rpam2 (4.0.2) rqrcode (2.1.2) @@ -679,6 +680,7 @@ GEM redlock (~> 1.0) strong_migrations (0.7.9) activerecord (>= 5) + strscan (3.1.0) swd (1.3.0) activesupport (>= 3) attr_required (>= 0.0.5) @@ -816,6 +818,7 @@ DEPENDENCIES letter_opener_web (~> 2.0) link_header (~> 0.0) lograge (~> 0.12) + mail (~> 2.8) makara (~> 0.5) mario-redis-lock (~> 1.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 8b77e9717da47b..dacab4d191a626 100644 --- a/app/controllers/api/v1/admin/domain_blocks_controller.rb +++ b/app/controllers/api/v1/admin/domain_blocks_controller.rb @@ -19,10 +19,11 @@ class Api::V1::Admin::DomainBlocksController < Api::BaseController 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 05e431b19ac56f..d12efe3a9fa985 100644 --- a/app/controllers/concerns/cache_concern.rb +++ b/app/controllers/concerns/cache_concern.rb @@ -28,29 +28,19 @@ def set_cache_headers response.headers['Vary'] = public_fetch_mode? ? 'Accept' : 'Accept, Signature' 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 = Rails.cache.read_multi(*raw).transform_keys(&:id) - uncached_ids = raw.map(&:id) - cached_keys_with_value.keys + klass.preload_cacheable_associations(records) - 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, item) - end - end - - 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 f83a62a1f4d703..6dae6a44382def 100644 --- a/app/controllers/well_known/webfinger_controller.rb +++ b/app/controllers/well_known/webfinger_controller.rb @@ -20,7 +20,7 @@ def show 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 45f5fc5bf2d549..6699dc21bd7049 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 @@ -86,6 +88,13 @@ def visibility 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? @@ -95,8 +104,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/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 de8bf338f2d5b4..7e2e2bda46fcea 100644 --- a/app/models/concerns/account_interactions.rb +++ b/app/models/concerns/account_interactions.rb @@ -187,7 +187,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 @@ -299,4 +299,8 @@ def local_followers_hash 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..0d082623227a72 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.preload(records, @cache_associated) + 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 fe789faaa93666..b29d321312d731 100644 --- a/app/models/public_feed.rb +++ b/app/models/public_feed.rb @@ -29,7 +29,7 @@ def get(limit, max_id = nil, since_id = nil, min_id = nil) scope.merge!(media_only_scope) if media_only? scope.merge!(language_scope) if account&.chosen_languages.present? - 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 5b67817968f2e7..d368c07efe1f9f 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -345,38 +345,6 @@ def pins_map(status_ids, account_id) StatusPin.select('status_id').where(status_id: status_ids).where(account_id: account_id).each_with_object({}) { |p, h| h[p.status_id] = true } 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 ecd4cafea687b3..decaaff5a02129 100644 --- a/app/models/tag_feed.rb +++ b/app/models/tag_feed.rb @@ -33,7 +33,7 @@ def get(limit, max_id = nil, since_id = nil, min_id = nil) scope.merge!(account_filters_scope) if account? scope.merge!(media_only_scope) if media_only? - 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 810ced84a877f7..e2355915510a60 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -94,6 +94,9 @@ class User < ApplicationRecord validates :invite_request, presence: true, on: :create, if: :invite_text_required? validates :locale, inclusion: I18n.available_locales.map(&:to_s), if: :locale? + + 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 b955d008a6e62b..e94a337cb8d9f9 100644 --- a/app/serializers/rest/admin/domain_block_serializer.rb +++ b/app/serializers/rest/admin/domain_block_serializer.rb @@ -1,11 +1,15 @@ # 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_reports, :private_comment, :public_comment, :obfuscate 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 c7454fc609d2de..a0dd9bed3f984a 100644 --- a/app/services/notify_service.rb +++ b/app/services/notify_service.rb @@ -62,16 +62,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 bd3b6963292331..7b57c05f58bcf4 100644 --- a/app/services/post_status_service.rb +++ b/app/services/post_status_service.rb @@ -137,7 +137,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 @@ -189,7 +189,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/auto_close_registrations_scheduler.rb b/app/workers/scheduler/auto_close_registrations_scheduler.rb index f774dda117880f..f44c9e30425bff 100644 --- a/app/workers/scheduler/auto_close_registrations_scheduler.rb +++ b/app/workers/scheduler/auto_close_registrations_scheduler.rb @@ -27,7 +27,7 @@ def switch_to_approval_mode! Setting.registrations_mode = 'approved' User.those_who_can(:manage_settings).includes(:account).find_each do |user| - AdminMailer.auto_close_registrations(recipient: user.account).deliver_later + AdminMailer.auto_close_registrations(user.account).deliver_later end end end diff --git a/config/application.rb b/config/application.rb index 513df0fe4b0908..53b044ea2896a1 100644 --- a/config/application.rb +++ b/config/application.rb @@ -44,6 +44,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 4c0bf779d88992..87f85eeec70007 100644 --- a/config/initializers/ffmpeg.rb +++ b/config/initializers/ffmpeg.rb @@ -1,3 +1,6 @@ -if ENV['FFMPEG_BINARY'].present? - FFMPEG.ffmpeg_binary = ENV['FFMPEG_BINARY'] +# frozen_string_literal: true + +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 3857e3055519c8..e60f719854d14e 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 @@ -137,6 +137,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 acaeafa633576f..6dde096d22f6eb 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -56,7 +56,7 @@ services: web: build: . - image: ghcr.io/mastodon/mastodon:v4.1.16 + image: ghcr.io/mastodon/mastodon:v4.1.17 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.1.16 + image: ghcr.io/mastodon/mastodon:v4.1.17 restart: always env_file: .env.production command: node ./streaming @@ -95,7 +95,7 @@ services: sidekiq: build: . - image: ghcr.io/mastodon/mastodon:v4.1.16 + image: ghcr.io/mastodon/mastodon:v4.1.17 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/maintenance_cli.rb b/lib/mastodon/maintenance_cli.rb index 85937da8126cc1..f9af3c77f905ee 100644 --- a/lib/mastodon/maintenance_cli.rb +++ b/lib/mastodon/maintenance_cli.rb @@ -234,7 +234,7 @@ def deduplicate_users! users = User.where(id: row['ids'].split(',')).sort_by(&:updated_at).reverse ref_user = users.shift @prompt.warn "Multiple users registered with e-mail address #{ref_user.email}." - @prompt.warn "e-mail will be disabled for the following accounts: #{user.map(&:account).map(&:acct).join(', ')}" + @prompt.warn "e-mail will be disabled for the following accounts: #{users.map(&:account).map(&:acct).join(', ')}" @prompt.warn 'Please reach out to them and set another address with `tootctl account modify` or delete them.' i = 0 diff --git a/lib/mastodon/media_cli.rb b/lib/mastodon/media_cli.rb index 24cc989646640e..3fe5bf420c9369 100644 --- a/lib/mastodon/media_cli.rb +++ b/lib/mastodon/media_cli.rb @@ -143,7 +143,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) @@ -186,7 +186,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 @@ -322,7 +322,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) @@ -372,7 +372,7 @@ def preload_records_from_mixed_objects(objects) next unless [7, 10].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 9832e1a27c96ca..c6bfa492fe93a4 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 d5babbe1d8d35e..3e2a11f14bf92e 100644 --- a/lib/mastodon/version.rb +++ b/lib/mastodon/version.rb @@ -13,7 +13,7 @@ def minor end def patch - 16 + 17 end def flags 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 0f2e30f7d5e45e..e7cf78f8c8a1cc 100644 --- a/lib/paperclip/transcoder.rb +++ b/lib/paperclip/transcoder.rb @@ -51,7 +51,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 1184e5273961b7..8c6c4be897c9e3 100644 --- a/lib/tasks/mastodon.rake +++ b/lib/tasks/mastodon.rake @@ -516,6 +516,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 581021cb90e321..50e5d5052a44c1 100644 --- a/spec/config/initializers/rack_attack_spec.rb +++ b/spec/config/initializers/rack_attack_spec.rb @@ -1,8 +1,6 @@ require 'rails_helper' -describe Rack::Attack do - include Rack::Test::Methods - +describe Rack::Attack, type: :request do def app Rails.application end @@ -12,7 +10,7 @@ def app it 'does not change the request status' do limit.times do request.call - expect(last_response.status).to_not eq(429) + expect(response.status).to_not eq(429) end end end @@ -21,7 +19,7 @@ def app it 'returns http too many requests' do (limit * 2).times do |i| request.call - expect(last_response.status).to eq(429) if i > limit + expect(response.status).to eq(429) if i > limit end end end @@ -32,7 +30,7 @@ def app describe 'throttle excessive sign-up requests by IP address' do context 'through the website' do let(:limit) { 25 } - let(:request) { ->() { post path, {}, 'REMOTE_ADDR' => remote_ip } } + let(:request) { ->() { post path, headers: { 'REMOTE_ADDR' => remote_ip } } } context 'for exact path' do let(:path) { '/auth' } @@ -47,7 +45,7 @@ def app context 'through the API' do let(:limit) { 5 } - let(:request) { ->() { post path, {}, 'REMOTE_ADDR' => remote_ip } } + let(:request) { ->() { post path, headers: { 'REMOTE_ADDR' => remote_ip } } } context 'for exact path' do let(:path) { '/api/v1/accounts' } @@ -59,7 +57,7 @@ def app it 'returns http not found' do request.call - expect(last_response.status).to eq(404) + expect(response.status).to eq(404) end end end @@ -67,7 +65,7 @@ def app describe 'throttle excessive sign-in requests by IP address' do let(:limit) { 25 } - let(:request) { ->() { post path, {}, 'REMOTE_ADDR' => remote_ip } } + let(:request) { ->() { post path, headers: { 'REMOTE_ADDR' => remote_ip } } } context 'for exact path' do let(:path) { '/auth/sign_in' } @@ -79,4 +77,28 @@ def app it_behaves_like 'throttled endpoint' end 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/admin/domain_blocks_controller_spec.rb b/spec/controllers/api/v1/admin/domain_blocks_controller_spec.rb index 606def602fc0d3..dc65aef5dcd108 100644 --- a/spec/controllers/api/v1/admin/domain_blocks_controller_spec.rb +++ b/spec/controllers/api/v1/admin/domain_blocks_controller_spec.rb @@ -16,6 +16,8 @@ let(:scopes) { wrong_scope } it 'returns http forbidden' do + subject + expect(response).to have_http_status(403) end end @@ -24,6 +26,8 @@ let(:role) { UserRole.find_by(name: wrong_role) } it 'returns http forbidden' do + subject + expect(response).to have_http_status(403) end end @@ -140,39 +144,70 @@ describe 'POST #create' do let(:existing_block_domain) { 'example.com' } + let(:params) { { domain: 'foo.bar.com', severity: :silence } } let!(:block) { Fabricate(:domain_block, domain: existing_block_domain, severity: :suspend) } - before do - post :create, params: { domain: 'foo.bar.com', severity: :silence } + subject do + post :create, params: params 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' - it 'returns http success' do + it 'creates a domain block and returns expected domain name', :aggregate_failures do + subject + expect(response).to have_http_status(200) + expect(body_as_json[:domain]).to eq 'foo.bar.com' + expect(DomainBlock.find_by(domain: 'foo.bar.com')).to_not be_nil end - it 'returns expected domain name' do - json = body_as_json - expect(json[:domain]).to eq 'foo.bar.com' - end + context 'when a looser domain block already exists on a higher level domain' do + let(:params) { { domain: 'foo.bar.com', severity: :suspend } } - it 'creates a domain block' do - expect(DomainBlock.find_by(domain: 'foo.bar.com')).to_not be_nil + 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 stricter domain block already exists' do - let(:existing_block_domain) { 'bar.com' } + 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 - it 'returns http unprocessable entity' do 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 + let(:existing_block_domain) { 'bar.com' } + + it 'returns http unprocessable entity with existing domain block in error', :aggregate_reblogs do + subject - it 'renders existing domain block in error' do - json = body_as_json - expect(json[:existing_domain_block][:domain]).to eq existing_block_domain + expect(response).to have_http_status(422) + expect(body_as_json[:existing_domain_block][:domain]).to eq existing_block_domain end 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 new file mode 100644 index 00000000000000..8cb928ea2c5e3e --- /dev/null +++ b/spec/controllers/api/v1/featured_tags/suggestions_controller_spec.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe Api::V1::FeaturedTags::SuggestionsController do + render_views + + let(:user) { Fabricate(:user) } + let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: 'read:accounts') } + let(:account) { Fabricate(:account, user: user) } + + before do + allow(controller).to receive(:doorkeeper_token) { token } + end + + describe 'GET #index' 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(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 1b002e01cb58a2..bfab2818ed877c 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -242,37 +242,4 @@ def route_unprocessable_entity include_examples 'respond_with_error', 422 end - - describe 'cache_collection' do - class C < ApplicationController - public :cache_collection - end - - shared_examples 'receives :with_includes' do |fabricator, klass| - it 'uses raw if it is not an ActiveRecord::Relation' do - record = Fabricate(fabricator) - expect(C.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(C.new.cache_collection(relation, klass)).to eq [record] - end - end - - it 'returns raw unless class responds to :with_includes' do - raw = Object.new - expect(C.new.cache_collection(raw, Object)).to eq raw - end - - context 'Status' do - include_examples 'cacheable', :status, Status - end - end 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/models/concerns/account_interactions_spec.rb b/spec/models/concerns/account_interactions_spec.rb index 1d1898ab0cac4b..dc7d5e972e0a24 100644 --- a/spec/models/concerns/account_interactions_spec.rb +++ b/spec/models/concerns/account_interactions_spec.rb @@ -248,6 +248,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) } @@ -343,6 +361,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 80f6edbff6aaa8..cdc3f5ad4c4708 100644 --- a/spec/models/home_feed_spec.rb +++ b/spec/models/home_feed_spec.rb @@ -25,7 +25,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 6414917a096ad3..79cd43f6d891bc 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -40,6 +40,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 empty string from languages' do user = Fabricate.build(:user, chosen_languages: ['']) user.valid? diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index c197bc00759317..759303e3934951 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -41,6 +41,7 @@ def sign_in(resource, _deprecated = nil, scope: nil) config.include Devise::Test::ControllerHelpers, type: :controller config.include Devise::Test::ControllerHelpers, type: :view + config.include Devise::Test::IntegrationHelpers, type: :request config.include Paperclip::Shoulda::Matchers config.include ActiveSupport::Testing::TimeHelpers config.include Redisable diff --git a/spec/services/notify_service_spec.rb b/spec/services/notify_service_spec.rb index 294c31b0445451..82770d4fe78c95 100644 --- a/spec/services/notify_service_spec.rb +++ b/spec/services/notify_service_spec.rb @@ -72,11 +72,11 @@ end end - context 'if 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)) } + 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(: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 d21270c7938946..c56f9efdb0942a 100644 --- a/spec/services/post_status_service_spec.rb +++ b/spec/services/post_status_service_spec.rb @@ -52,6 +52,13 @@ it 'does not change statuses count' do expect { subject.call(account, text: 'Hi future!', scheduled_at: future, thread: previous_status) }.not_to 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