From 1060666c583670bb3b89ed5154e61038331e30c3 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Wed, 19 Jan 2022 22:37:27 +0100 Subject: [PATCH 01/16] Add support for editing for published statuses (#16697) * Add support for editing for published statuses * Fix references to stripped-out code * Various fixes and improvements * Further fixes and improvements * Fix updates being potentially sent to unauthorized recipients * Various fixes and improvements * Fix wrong words in test * Fix notifying accounts that were tagged but were not in the audience * Fix mistake --- .../api/v1/statuses/histories_controller.rb | 21 ++ .../api/v1/statuses/sources_controller.rb | 21 ++ app/helpers/jsonld_helper.rb | 8 +- .../mastodon/actions/importer/normalizer.js | 7 +- app/javascript/mastodon/actions/statuses.js | 3 + app/javascript/mastodon/actions/streaming.js | 4 + app/javascript/mastodon/components/status.js | 3 +- .../status/components/detailed_status.js | 14 +- .../styles/mastodon/components.scss | 11 + app/lib/activitypub/activity.rb | 43 --- app/lib/activitypub/activity/announce.rb | 18 +- app/lib/activitypub/activity/create.rb | 249 ++++++---------- app/lib/activitypub/activity/update.rb | 17 +- .../activitypub/parser/custom_emoji_parser.rb | 27 ++ .../parser/media_attachment_parser.rb | 58 ++++ app/lib/activitypub/parser/poll_parser.rb | 53 ++++ app/lib/activitypub/parser/status_parser.rb | 118 ++++++++ app/lib/feed_manager.rb | 20 +- app/lib/status_reach_finder.rb | 31 +- app/models/poll.rb | 1 + app/models/status.rb | 7 + app/models/status_edit.rb | 23 ++ .../activitypub/note_serializer.rb | 7 + .../rest/status_edit_serializer.rb | 6 + app/serializers/rest/status_serializer.rb | 2 +- .../rest/status_source_serializer.rb | 9 + .../activitypub/fetch_remote_poll_service.rb | 2 +- .../activitypub/process_poll_service.rb | 64 ---- .../process_status_update_service.rb | 275 ++++++++++++++++++ app/services/fan_out_on_write_service.rb | 157 +++++----- app/services/process_mentions_service.rb | 69 +++-- app/services/remove_status_service.rb | 2 +- .../activitypub/distribution_worker.rb | 48 +-- .../activitypub/raw_distribution_worker.rb | 37 ++- .../activitypub/reply_distribution_worker.rb | 34 --- .../activitypub/update_distribution_worker.rb | 25 +- app/workers/distribution_worker.rb | 4 +- app/workers/feed_insert_worker.rb | 34 ++- app/workers/local_notification_worker.rb | 2 + app/workers/poll_expiration_notify_worker.rb | 45 ++- app/workers/push_update_worker.rb | 35 ++- config/routes.rb | 3 + ...0210904215403_add_edited_at_to_statuses.rb | 5 + .../20210908220918_create_status_edits.rb | 13 + db/schema.rb | 15 + .../v1/statuses/histories_controller_spec.rb | 29 ++ .../v1/statuses/sources_controller_spec.rb | 29 ++ spec/fabricators/preview_card_fabricator.rb | 6 + spec/fabricators/status_edit_fabricator.rb | 7 + spec/lib/status_reach_finder_spec.rb | 109 +++++++ spec/models/status_edit_spec.rb | 5 + .../fetch_remote_status_service_spec.rb | 6 +- .../services/fan_out_on_write_service_spec.rb | 107 ++++++- .../services/process_mentions_service_spec.rb | 32 +- .../activitypub/distribution_worker_spec.rb | 7 +- spec/workers/feed_insert_worker_spec.rb | 2 +- 56 files changed, 1415 insertions(+), 574 deletions(-) create mode 100644 app/controllers/api/v1/statuses/histories_controller.rb create mode 100644 app/controllers/api/v1/statuses/sources_controller.rb create mode 100644 app/lib/activitypub/parser/custom_emoji_parser.rb create mode 100644 app/lib/activitypub/parser/media_attachment_parser.rb create mode 100644 app/lib/activitypub/parser/poll_parser.rb create mode 100644 app/lib/activitypub/parser/status_parser.rb create mode 100644 app/models/status_edit.rb create mode 100644 app/serializers/rest/status_edit_serializer.rb create mode 100644 app/serializers/rest/status_source_serializer.rb delete mode 100644 app/services/activitypub/process_poll_service.rb create mode 100644 app/services/activitypub/process_status_update_service.rb delete mode 100644 app/workers/activitypub/reply_distribution_worker.rb create mode 100644 db/migrate/20210904215403_add_edited_at_to_statuses.rb create mode 100644 db/migrate/20210908220918_create_status_edits.rb create mode 100644 spec/controllers/api/v1/statuses/histories_controller_spec.rb create mode 100644 spec/controllers/api/v1/statuses/sources_controller_spec.rb create mode 100644 spec/fabricators/preview_card_fabricator.rb create mode 100644 spec/fabricators/status_edit_fabricator.rb create mode 100644 spec/lib/status_reach_finder_spec.rb create mode 100644 spec/models/status_edit_spec.rb diff --git a/app/controllers/api/v1/statuses/histories_controller.rb b/app/controllers/api/v1/statuses/histories_controller.rb new file mode 100644 index 0000000000..c2c1fac5d5 --- /dev/null +++ b/app/controllers/api/v1/statuses/histories_controller.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +class Api::V1::Statuses::HistoriesController < Api::BaseController + include Authorization + + before_action -> { authorize_if_got_token! :read, :'read:statuses' } + before_action :set_status + + def show + render json: @status.edits, each_serializer: REST::StatusEditSerializer + end + + private + + def set_status + @status = Status.find(params[:status_id]) + authorize @status, :show? + rescue Mastodon::NotPermittedError + not_found + end +end diff --git a/app/controllers/api/v1/statuses/sources_controller.rb b/app/controllers/api/v1/statuses/sources_controller.rb new file mode 100644 index 0000000000..4340864513 --- /dev/null +++ b/app/controllers/api/v1/statuses/sources_controller.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +class Api::V1::Statuses::SourcesController < Api::BaseController + include Authorization + + before_action -> { doorkeeper_authorize! :read, :'read:statuses' } + before_action :set_status + + def show + render json: @status, serializer: REST::StatusSourceSerializer + end + + private + + def set_status + @status = Status.find(params[:status_id]) + authorize @status, :show? + rescue Mastodon::NotPermittedError + not_found + end +end diff --git a/app/helpers/jsonld_helper.rb b/app/helpers/jsonld_helper.rb index 62eb50f786..c24d2ddf10 100644 --- a/app/helpers/jsonld_helper.rb +++ b/app/helpers/jsonld_helper.rb @@ -34,7 +34,13 @@ module JsonLdHelper end def as_array(value) - value.is_a?(Array) ? value : [value] + if value.nil? + [] + elsif value.is_a?(Array) + value + else + [value] + end end def value_or_id(value) diff --git a/app/javascript/mastodon/actions/importer/normalizer.js b/app/javascript/mastodon/actions/importer/normalizer.js index 6b79e1f16d..ca76e3494d 100644 --- a/app/javascript/mastodon/actions/importer/normalizer.js +++ b/app/javascript/mastodon/actions/importer/normalizer.js @@ -54,9 +54,10 @@ export function normalizeStatus(status, normalOldStatus) { normalStatus.poll = status.poll.id; } - // Only calculate these values when status first encountered - // Otherwise keep the ones already in the reducer - if (normalOldStatus) { + // Only calculate these values when status first encountered and + // when the underlying values change. Otherwise keep the ones + // already in the reducer + if (normalOldStatus && normalOldStatus.get('content') === normalStatus.content && normalOldStatus.get('spoiler_text') === normalStatus.spoiler_text) { normalStatus.search_index = normalOldStatus.get('search_index'); normalStatus.contentHtml = normalOldStatus.get('contentHtml'); normalStatus.spoilerHtml = normalOldStatus.get('spoilerHtml'); diff --git a/app/javascript/mastodon/actions/statuses.js b/app/javascript/mastodon/actions/statuses.js index 3fc7c07023..20d71362e9 100644 --- a/app/javascript/mastodon/actions/statuses.js +++ b/app/javascript/mastodon/actions/statuses.js @@ -131,6 +131,9 @@ export function deleteStatusFail(id, error) { }; }; +export const updateStatus = status => dispatch => + dispatch(importFetchedStatus(status)); + export function fetchContext(id) { return (dispatch, getState) => { dispatch(fetchContextRequest(id)); diff --git a/app/javascript/mastodon/actions/streaming.js b/app/javascript/mastodon/actions/streaming.js index beb5c6a4a9..8fbb22271a 100644 --- a/app/javascript/mastodon/actions/streaming.js +++ b/app/javascript/mastodon/actions/streaming.js @@ -10,6 +10,7 @@ import { } from './timelines'; import { updateNotifications, expandNotifications } from './notifications'; import { updateConversations } from './conversations'; +import { updateStatus } from './statuses'; import { fetchAnnouncements, updateAnnouncements, @@ -75,6 +76,9 @@ export const connectTimelineStream = (timelineId, channelName, params = {}, opti case 'update': dispatch(updateTimeline(timelineId, JSON.parse(data.payload), options.accept)); break; + case 'status.update': + dispatch(updateStatus(JSON.parse(data.payload))); + break; case 'delete': dispatch(deleteFromTimelines(data.payload)); break; diff --git a/app/javascript/mastodon/components/status.js b/app/javascript/mastodon/components/status.js index 9955046c04..fb370ca71f 100644 --- a/app/javascript/mastodon/components/status.js +++ b/app/javascript/mastodon/components/status.js @@ -57,6 +57,7 @@ const messages = defineMessages({ unlisted_short: { id: 'privacy.unlisted.short', defaultMessage: 'Unlisted' }, private_short: { id: 'privacy.private.short', defaultMessage: 'Followers-only' }, direct_short: { id: 'privacy.direct.short', defaultMessage: 'Direct' }, + edited: { id: 'status.edited', defaultMessage: 'Edited {date}' }, }); export default @injectIntl @@ -483,7 +484,7 @@ class Status extends ImmutablePureComponent {
- + {status.get('edited_at') && *} diff --git a/app/javascript/mastodon/features/status/components/detailed_status.js b/app/javascript/mastodon/features/status/components/detailed_status.js index 72ddeb2b24..ee4a6b9898 100644 --- a/app/javascript/mastodon/features/status/components/detailed_status.js +++ b/app/javascript/mastodon/features/status/components/detailed_status.js @@ -6,7 +6,7 @@ import DisplayName from '../../../components/display_name'; import StatusContent from '../../../components/status_content'; import MediaGallery from '../../../components/media_gallery'; import { Link } from 'react-router-dom'; -import { injectIntl, defineMessages, FormattedDate } from 'react-intl'; +import { injectIntl, defineMessages, FormattedDate, FormattedMessage } from 'react-intl'; import Card from './card'; import ImmutablePureComponent from 'react-immutable-pure-component'; import Video from '../../video'; @@ -116,6 +116,7 @@ class DetailedStatus extends ImmutablePureComponent { let reblogLink = ''; let reblogIcon = 'retweet'; let favouriteLink = ''; + let edited = ''; if (this.props.measureHeight) { outerStyle.height = `${this.state.height}px`; @@ -237,6 +238,15 @@ class DetailedStatus extends ImmutablePureComponent { ); } + if (status.get('edited_at')) { + edited = ( + + · + + + ); + } + return (
@@ -252,7 +262,7 @@ class DetailedStatus extends ImmutablePureComponent {
- {visibilityLink}{applicationLink}{reblogLink} · {favouriteLink} + {edited}{visibilityLink}{applicationLink}{reblogLink} · {favouriteLink}
diff --git a/app/javascript/styles/mastodon/components.scss b/app/javascript/styles/mastodon/components.scss index 0a62e6b829..02b3473a92 100644 --- a/app/javascript/styles/mastodon/components.scss +++ b/app/javascript/styles/mastodon/components.scss @@ -967,6 +967,17 @@ } } +.status__content__edited-label { + display: block; + cursor: default; + font-size: 15px; + line-height: 20px; + padding: 0; + padding-top: 8px; + color: $dark-text-color; + font-weight: 500; +} + .status__content__spoiler-link { display: inline-block; border-radius: 2px; diff --git a/app/lib/activitypub/activity.rb b/app/lib/activitypub/activity.rb index 3aeecb4ec0..706960f929 100644 --- a/app/lib/activitypub/activity.rb +++ b/app/lib/activitypub/activity.rb @@ -94,49 +94,6 @@ class ActivityPub::Activity equals_or_includes_any?(@object['type'], CONVERTED_TYPES) end - def distribute(status) - crawl_links(status) - - notify_about_reblog(status) if reblog_of_local_account?(status) && !reblog_by_following_group_account?(status) - notify_about_mentions(status) - - # Only continue if the status is supposed to have arrived in real-time. - # Note that if @options[:override_timestamps] isn't set, the status - # may have a lower snowflake id than other existing statuses, potentially - # "hiding" it from paginated API calls - return unless @options[:override_timestamps] || status.within_realtime_window? - - distribute_to_followers(status) - end - - def reblog_of_local_account?(status) - status.reblog? && status.reblog.account.local? - end - - def reblog_by_following_group_account?(status) - status.reblog? && status.account.group? && status.reblog.account.following?(status.account) - end - - def notify_about_reblog(status) - NotifyService.new.call(status.reblog.account, :reblog, status) - end - - def notify_about_mentions(status) - status.active_mentions.includes(:account).each do |mention| - next unless mention.account.local? && audience_includes?(mention.account) - NotifyService.new.call(mention.account, :mention, mention) - end - end - - def crawl_links(status) - # Spread out crawling randomly to avoid DDoSing the link - LinkCrawlWorker.perform_in(rand(1..59).seconds, status.id) - end - - def distribute_to_followers(status) - ::DistributionWorker.perform_async(status.id) - end - def delete_arrived_first?(uri) redis.exists?("delete_upon_arrival:#{@account.id}:#{uri}") end diff --git a/app/lib/activitypub/activity/announce.rb b/app/lib/activitypub/activity/announce.rb index 6c5d88d185..1f93192905 100644 --- a/app/lib/activitypub/activity/announce.rb +++ b/app/lib/activitypub/activity/announce.rb @@ -25,7 +25,7 @@ class ActivityPub::Activity::Announce < ActivityPub::Activity Trends.tags.register(@status) Trends.links.register(@status) - distribute(@status) + distribute end @status @@ -33,6 +33,22 @@ class ActivityPub::Activity::Announce < ActivityPub::Activity private + def distribute + # Notify the author of the original status if that status is local + NotifyService.new.call(@status.reblog.account, :reblog, @status) if reblog_of_local_account?(@status) && !reblog_by_following_group_account?(@status) + + # Distribute into home and list feeds + ::DistributionWorker.perform_async(@status.id) if @options[:override_timestamps] || @status.within_realtime_window? + end + + def reblog_of_local_account?(status) + status.reblog? && status.reblog.account.local? + end + + def reblog_by_following_group_account?(status) + status.reblog? && status.account.group? && status.reblog.account.following?(status.account) + end + def audience_to as_array(@json['to']).map { |x| value_or_id(x) } end diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb index 8a0dc9d33d..a861c34bc3 100644 --- a/app/lib/activitypub/activity/create.rb +++ b/app/lib/activitypub/activity/create.rb @@ -69,9 +69,10 @@ class ActivityPub::Activity::Create < ActivityPub::Activity end def process_status - @tags = [] - @mentions = [] - @params = {} + @tags = [] + @mentions = [] + @silenced_account_ids = [] + @params = {} process_status_params process_tags @@ -84,10 +85,18 @@ class ActivityPub::Activity::Create < ActivityPub::Activity resolve_thread(@status) fetch_replies(@status) - distribute(@status) + distribute forward_for_reply end + def distribute + # Spread out crawling randomly to avoid DDoSing the link + LinkCrawlWorker.perform_in(rand(1..59).seconds, @status.id) + + # Distribute into home and list feeds and notify mentioned accounts + ::DistributionWorker.perform_async(@status.id, silenced_account_ids: @silenced_account_ids) if @options[:override_timestamps] || @status.within_realtime_window? + end + def find_existing_status status = status_from_uri(object_uri) status ||= Status.find_by(uri: @object['atomUri']) if @object['atomUri'].present? @@ -95,19 +104,22 @@ class ActivityPub::Activity::Create < ActivityPub::Activity end def process_status_params + @status_parser = ActivityPub::Parser::StatusParser.new(@json, followers_collection: @account.followers_url) + @params = begin { - uri: object_uri, - url: object_url || object_uri, + uri: @status_parser.uri, + url: @status_parser.url || @status_parser.uri, account: @account, - text: text_from_content || '', - language: detected_language, - spoiler_text: converted_object_type? ? '' : (text_from_summary || ''), - created_at: @object['published'], + text: converted_object_type? ? converted_text : (@status_parser.text || ''), + language: @status_parser.language || detected_language, + spoiler_text: converted_object_type? ? '' : (@status_parser.spoiler_text || ''), + created_at: @status_parser.created_at, + edited_at: @status_parser.edited_at, override_timestamps: @options[:override_timestamps], - reply: @object['inReplyTo'].present?, - sensitive: @account.sensitized? || @object['sensitive'] || false, - visibility: visibility_from_audience, + reply: @status_parser.reply, + sensitive: @account.sensitized? || @status_parser.sensitive || false, + visibility: @status_parser.visibility, thread: replied_to_status, conversation: conversation_from_uri(@object['conversation']), media_attachment_ids: process_attachments.take(4).map(&:id), @@ -117,42 +129,40 @@ class ActivityPub::Activity::Create < ActivityPub::Activity end def process_audience - (audience_to + audience_cc).uniq.each do |audience| - next if ActivityPub::TagManager.instance.public_collection?(audience) + # Unlike with tags, there is no point in resolving accounts we don't already + # know here, because silent mentions would only be used for local access control anyway + accounts_in_audience = (audience_to + audience_cc).uniq.filter_map do |audience| + account_from_uri(audience) unless ActivityPub::TagManager.instance.public_collection?(audience) + end - # Unlike with tags, there is no point in resolving accounts we don't already - # know here, because silent mentions would only be used for local access - # control anyway - account = account_from_uri(audience) + # If the payload was delivered to a specific inbox, the inbox owner must have + # access to it, unless they already have access to it anyway + if @options[:delivered_to_account_id] + accounts_in_audience << delivered_to_account + accounts_in_audience.uniq! + end - next if account.nil? || @mentions.any? { |mention| mention.account_id == account.id } + accounts_in_audience.each do |account| + # This runs after tags are processed, and those translate into non-silent + # mentions, which take precedence + next if @mentions.any? { |mention| mention.account_id == account.id } @mentions << Mention.new(account: account, silent: true) # If there is at least one silent mention, then the status can be considered # as a limited-audience status, and not strictly a direct message, but only # if we considered a direct message in the first place - next unless @params[:visibility] == :direct - - @params[:visibility] = :limited + @params[:visibility] = :limited if @params[:visibility] == :direct end - # If the payload was delivered to a specific inbox, the inbox owner must have - # access to it, unless they already have access to it anyway - return if @options[:delivered_to_account_id].nil? || @mentions.any? { |mention| mention.account_id == @options[:delivered_to_account_id] } - - @mentions << Mention.new(account_id: @options[:delivered_to_account_id], silent: true) - - return unless @params[:visibility] == :direct - - @params[:visibility] = :limited + # Accounts that are tagged but are not in the audience are not + # supposed to be notified explicitly + @silenced_account_ids = @mentions.map(&:account_id) - accounts_in_audience.map(&:id) end def postprocess_audience_and_deliver return if @status.mentions.find_by(account_id: @options[:delivered_to_account_id]) - delivered_to_account = Account.find(@options[:delivered_to_account_id]) - @status.mentions.create(account: delivered_to_account, silent: true) @status.update(visibility: :limited) if @status.direct_visibility? @@ -161,6 +171,10 @@ class ActivityPub::Activity::Create < ActivityPub::Activity FeedInsertWorker.perform_async(@status.id, delivered_to_account.id, :home) end + def delivered_to_account + @delivered_to_account ||= Account.find(@options[:delivered_to_account_id]) + end + def attach_tags(status) @tags.each do |tag| status.tags << tag @@ -215,21 +229,22 @@ class ActivityPub::Activity::Create < ActivityPub::Activity def process_emoji(tag) return if skip_download? - return if tag['name'].blank? || tag['icon'].blank? || tag['icon']['url'].blank? - shortcode = tag['name'].delete(':') - image_url = tag['icon']['url'] - uri = tag['id'] - updated = tag['updated'] - emoji = CustomEmoji.find_by(shortcode: shortcode, domain: @account.domain) + custom_emoji_parser = ActivityPub::Parser::CustomEmojiParser.new(tag) - return unless emoji.nil? || image_url != emoji.image_remote_url || (updated && updated >= emoji.updated_at) + return if custom_emoji_parser.shortcode.blank? || custom_emoji_parser.image_remote_url.blank? - emoji ||= CustomEmoji.new(domain: @account.domain, shortcode: shortcode, uri: uri) - emoji.image_remote_url = image_url - emoji.save - rescue Seahorse::Client::NetworkingError => e - Rails.logger.warn "Error storing emoji: #{e}" + emoji = CustomEmoji.find_by(shortcode: custom_emoji_parser.shortcode, domain: @account.domain) + + return unless emoji.nil? || custom_emoji_parser.image_remote_url != emoji.image_remote_url || (custom_emoji_parser.updated_at && custom_emoji_parser.updated_at >= emoji.updated_at) + + begin + emoji ||= CustomEmoji.new(domain: @account.domain, shortcode: custom_emoji_parser.shortcode, uri: custom_emoji_parser.uri) + emoji.image_remote_url = custom_emoji_parser.image_remote_url + emoji.save + rescue Seahorse::Client::NetworkingError => e + Rails.logger.warn "Error storing emoji: #{e}" + end end def process_attachments @@ -238,14 +253,23 @@ class ActivityPub::Activity::Create < ActivityPub::Activity media_attachments = [] as_array(@object['attachment']).each do |attachment| - next if attachment['url'].blank? || media_attachments.size >= 4 + media_attachment_parser = ActivityPub::Parser::MediaAttachmentParser.new(attachment) + + next if media_attachment_parser.remote_url.blank? || media_attachments.size >= 4 begin - href = Addressable::URI.parse(attachment['url']).normalize.to_s - media_attachment = MediaAttachment.create(account: @account, remote_url: href, thumbnail_remote_url: icon_url_from_attachment(attachment), description: attachment['summary'].presence || attachment['name'].presence, focus: attachment['focalPoint'], blurhash: supported_blurhash?(attachment['blurhash']) ? attachment['blurhash'] : nil) + media_attachment = MediaAttachment.create( + account: @account, + remote_url: media_attachment_parser.remote_url, + thumbnail_remote_url: media_attachment_parser.thumbnail_remote_url, + description: media_attachment_parser.description, + focus: media_attachment_parser.focus, + blurhash: media_attachment_parser.blurhash + ) + media_attachments << media_attachment - next if unsupported_media_type?(attachment['mediaType']) || skip_download? + next if unsupported_media_type?(media_attachment_parser.file_content_type) || skip_download? media_attachment.download_file! media_attachment.download_thumbnail! @@ -263,42 +287,17 @@ class ActivityPub::Activity::Create < ActivityPub::Activity media_attachments end - def icon_url_from_attachment(attachment) - url = attachment['icon'].is_a?(Hash) ? attachment['icon']['url'] : attachment['icon'] - Addressable::URI.parse(url).normalize.to_s if url.present? - rescue Addressable::URI::InvalidURIError - nil - end - def process_poll - return unless @object['type'] == 'Question' && (@object['anyOf'].is_a?(Array) || @object['oneOf'].is_a?(Array)) + poll_parser = ActivityPub::Parser::PollParser.new(@object) - expires_at = begin - if @object['closed'].is_a?(String) - @object['closed'] - elsif !@object['closed'].nil? && !@object['closed'].is_a?(FalseClass) - Time.now.utc - else - @object['endTime'] - end - end - - if @object['anyOf'].is_a?(Array) - multiple = true - items = @object['anyOf'] - else - multiple = false - items = @object['oneOf'] - end - - voters_count = @object['votersCount'] + return unless poll_parser.valid? @account.polls.new( - multiple: multiple, - expires_at: expires_at, - options: items.map { |item| item['name'].presence || item['content'] }.compact, - cached_tallies: items.map { |item| item.dig('replies', 'totalItems') || 0 }, - voters_count: voters_count + multiple: poll_parser.multiple, + expires_at: poll_parser.expires_at, + options: poll_parser.options, + cached_tallies: poll_parser.cached_tallies, + voters_count: poll_parser.voters_count ) end @@ -351,23 +350,6 @@ class ActivityPub::Activity::Create < ActivityPub::Activity end end - def visibility_from_audience - if audience_to.any? { |to| ActivityPub::TagManager.instance.public_collection?(to) } - :public - elsif audience_cc.any? { |cc| ActivityPub::TagManager.instance.public_collection?(cc) } - :unlisted - elsif audience_to.include?(@account.followers_url) - :private - else - :direct - end - end - - def audience_includes?(account) - uri = ActivityPub::TagManager.instance.uri_for(account) - audience_to.include?(uri) || audience_cc.include?(uri) - end - def replied_to_status return @replied_to_status if defined?(@replied_to_status) @@ -384,81 +366,18 @@ class ActivityPub::Activity::Create < ActivityPub::Activity value_or_id(@object['inReplyTo']) end - def text_from_content - return Formatter.instance.linkify([[text_from_name, text_from_summary.presence].compact.join("\n\n"), object_url || object_uri].join(' ')) if converted_object_type? - - if @object['content'].present? - @object['content'] - elsif content_language_map? - @object['contentMap'].values.first - end - end - - def text_from_summary - if @object['summary'].present? - @object['summary'] - elsif summary_language_map? - @object['summaryMap'].values.first - end - end - - def text_from_name - if @object['name'].present? - @object['name'] - elsif name_language_map? - @object['nameMap'].values.first - end + def converted_text + Formatter.instance.linkify([@status_parser.title.presence, @status_parser.spoiler_text.presence, @status_parser.url || @status_parser.uri].compact.join("\n\n")) end def detected_language - if content_language_map? - @object['contentMap'].keys.first - elsif name_language_map? - @object['nameMap'].keys.first - elsif summary_language_map? - @object['summaryMap'].keys.first - elsif supported_object_type? - LanguageDetector.instance.detect(text_from_content, @account) - end - end - - def object_url - return if @object['url'].blank? - - url_candidate = url_to_href(@object['url'], 'text/html') - - if invalid_origin?(url_candidate) - nil - else - url_candidate - end - end - - def summary_language_map? - @object['summaryMap'].is_a?(Hash) && !@object['summaryMap'].empty? - end - - def content_language_map? - @object['contentMap'].is_a?(Hash) && !@object['contentMap'].empty? - end - - def name_language_map? - @object['nameMap'].is_a?(Hash) && !@object['nameMap'].empty? + LanguageDetector.instance.detect(@status_parser.text, @account) if supported_object_type? end def unsupported_media_type?(mime_type) mime_type.present? && !MediaAttachment.supported_mime_types.include?(mime_type) end - def supported_blurhash?(blurhash) - components = blurhash.blank? || !blurhash_valid_chars?(blurhash) ? nil : Blurhash.components(blurhash) - components.present? && components.none? { |comp| comp > 5 } - end - - def blurhash_valid_chars?(blurhash) - /^[\w#$%*+-.:;=?@\[\]^{|}~]+$/.match?(blurhash) - end - def skip_download? return @skip_download if defined?(@skip_download) diff --git a/app/lib/activitypub/activity/update.rb b/app/lib/activitypub/activity/update.rb index 018e2df549..f04ad321b2 100644 --- a/app/lib/activitypub/activity/update.rb +++ b/app/lib/activitypub/activity/update.rb @@ -1,32 +1,31 @@ # frozen_string_literal: true class ActivityPub::Activity::Update < ActivityPub::Activity - SUPPORTED_TYPES = %w(Application Group Organization Person Service).freeze - def perform dereference_object! - if equals_or_includes_any?(@object['type'], SUPPORTED_TYPES) + if equals_or_includes_any?(@object['type'], %w(Application Group Organization Person Service)) update_account - elsif equals_or_includes_any?(@object['type'], %w(Question)) - update_poll + elsif equals_or_includes_any?(@object['type'], %w(Note Question)) + update_status end end private def update_account - return if @account.uri != object_uri + return reject_payload! if @account.uri != object_uri ActivityPub::ProcessAccountService.new.call(@account.username, @account.domain, @object, signed_with_known_key: true) end - def update_poll + def update_status return reject_payload! if invalid_origin?(@object['id']) status = Status.find_by(uri: object_uri, account_id: @account.id) - return if status.nil? || status.preloadable_poll.nil? - ActivityPub::ProcessPollService.new.call(status.preloadable_poll, @object) + return if status.nil? + + ActivityPub::ProcessStatusUpdateService.new.call(status, @object) end end diff --git a/app/lib/activitypub/parser/custom_emoji_parser.rb b/app/lib/activitypub/parser/custom_emoji_parser.rb new file mode 100644 index 0000000000..724c602150 --- /dev/null +++ b/app/lib/activitypub/parser/custom_emoji_parser.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +class ActivityPub::Parser::CustomEmojiParser + include JsonLdHelper + + def initialize(json) + @json = json + end + + def uri + @json['id'] + end + + def shortcode + @json['name']&.delete(':') + end + + def image_remote_url + @json.dig('icon', 'url') + end + + def updated_at + @json['updated']&.to_datetime + rescue ArgumentError + nil + end +end diff --git a/app/lib/activitypub/parser/media_attachment_parser.rb b/app/lib/activitypub/parser/media_attachment_parser.rb new file mode 100644 index 0000000000..1798e58a4b --- /dev/null +++ b/app/lib/activitypub/parser/media_attachment_parser.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +class ActivityPub::Parser::MediaAttachmentParser + include JsonLdHelper + + def initialize(json) + @json = json + end + + # @param [MediaAttachment] previous_record + def significantly_changes?(previous_record) + remote_url != previous_record.remote_url || + thumbnail_remote_url != previous_record.thumbnail_remote_url || + description != previous_record.description + end + + def remote_url + Addressable::URI.parse(@json['url'])&.normalize&.to_s + rescue Addressable::URI::InvalidURIError + nil + end + + def thumbnail_remote_url + Addressable::URI.parse(@json['icon'].is_a?(Hash) ? @json['icon']['url'] : @json['icon'])&.normalize&.to_s + rescue Addressable::URI::InvalidURIError + nil + end + + def description + @json['summary'].presence || @json['name'].presence + end + + def focus + @json['focalPoint'] + end + + def blurhash + supported_blurhash? ? @json['blurhash'] : nil + end + + def file_content_type + @json['mediaType'] + end + + private + + def supported_blurhash? + components = begin + blurhash = @json['blurhash'] + + if blurhash.present? && /^[\w#$%*+-.:;=?@\[\]^{|}~]+$/.match?(blurhash) + Blurhash.components(blurhash) + end + end + + components.present? && components.none? { |comp| comp > 5 } + end +end diff --git a/app/lib/activitypub/parser/poll_parser.rb b/app/lib/activitypub/parser/poll_parser.rb new file mode 100644 index 0000000000..758c03f07e --- /dev/null +++ b/app/lib/activitypub/parser/poll_parser.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +class ActivityPub::Parser::PollParser + include JsonLdHelper + + def initialize(json) + @json = json + end + + def valid? + equals_or_includes?(@json['type'], 'Question') && items.is_a?(Array) + end + + # @param [Poll] previous_record + def significantly_changes?(previous_record) + options != previous_record.options || + multiple != previous_record.multiple + end + + def options + items.filter_map { |item| item['name'].presence || item['content'] } + end + + def multiple + @json['anyOf'].is_a?(Array) + end + + def expires_at + if @json['closed'].is_a?(String) + @json['closed'].to_datetime + elsif !@json['closed'].nil? && !@json['closed'].is_a?(FalseClass) + Time.now.utc + else + @json['endTime']&.to_datetime + end + rescue ArgumentError + nil + end + + def voters_count + @json['votersCount'] + end + + def cached_tallies + items.map { |item| item.dig('replies', 'totalItems') || 0 } + end + + private + + def items + @json['anyOf'] || @json['oneOf'] + end +end diff --git a/app/lib/activitypub/parser/status_parser.rb b/app/lib/activitypub/parser/status_parser.rb new file mode 100644 index 0000000000..3ba154d015 --- /dev/null +++ b/app/lib/activitypub/parser/status_parser.rb @@ -0,0 +1,118 @@ +# frozen_string_literal: true + +class ActivityPub::Parser::StatusParser + include JsonLdHelper + + # @param [Hash] json + # @param [Hash] magic_values + # @option magic_values [String] :followers_collection + def initialize(json, magic_values = {}) + @json = json + @object = json['object'] || json + @magic_values = magic_values + end + + def uri + id = @object['id'] + + if id&.start_with?('bear:') + Addressable::URI.parse(id).query_values['u'] + else + id + end + rescue Addressable::URI::InvalidURIError + id + end + + def url + url_to_href(@object['url'], 'text/html') if @object['url'].present? + end + + def text + if @object['content'].present? + @object['content'] + elsif content_language_map? + @object['contentMap'].values.first + end + end + + def spoiler_text + if @object['summary'].present? + @object['summary'] + elsif summary_language_map? + @object['summaryMap'].values.first + end + end + + def title + if @object['name'].present? + @object['name'] + elsif name_language_map? + @object['nameMap'].values.first + end + end + + def created_at + @object['published']&.to_datetime + rescue ArgumentError + nil + end + + def edited_at + @object['updated']&.to_datetime + rescue ArgumentError + nil + end + + def reply + @object['inReplyTo'].present? + end + + def sensitive + @object['sensitive'] + end + + def visibility + if audience_to.any? { |to| ActivityPub::TagManager.instance.public_collection?(to) } + :public + elsif audience_cc.any? { |cc| ActivityPub::TagManager.instance.public_collection?(cc) } + :unlisted + elsif audience_to.include?(@magic_values[:followers_collection]) + :private + else + :direct + end + end + + def language + if content_language_map? + @object['contentMap'].keys.first + elsif name_language_map? + @object['nameMap'].keys.first + elsif summary_language_map? + @object['summaryMap'].keys.first + end + end + + private + + def audience_to + as_array(@object['to'] || @json['to']).map { |x| value_or_id(x) } + end + + def audience_cc + as_array(@object['cc'] || @json['cc']).map { |x| value_or_id(x) } + end + + def summary_language_map? + @object['summaryMap'].is_a?(Hash) && !@object['summaryMap'].empty? + end + + def content_language_map? + @object['contentMap'].is_a?(Hash) && !@object['contentMap'].empty? + end + + def name_language_map? + @object['nameMap'].is_a?(Hash) && !@object['nameMap'].empty? + end +end diff --git a/app/lib/feed_manager.rb b/app/lib/feed_manager.rb index d5e435216a..c4dd9d00ff 100644 --- a/app/lib/feed_manager.rb +++ b/app/lib/feed_manager.rb @@ -53,46 +53,50 @@ class FeedManager # Add a status to a home feed and send a streaming API update # @param [Account] account # @param [Status] status + # @param [Boolean] update # @return [Boolean] - def push_to_home(account, status) + def push_to_home(account, status, update: false) return false unless add_to_feed(:home, account.id, status, account.user&.aggregates_reblogs?) trim(:home, account.id) - PushUpdateWorker.perform_async(account.id, status.id, "timeline:#{account.id}") if push_update_required?("timeline:#{account.id}") + PushUpdateWorker.perform_async(account.id, status.id, "timeline:#{account.id}", update: update) if push_update_required?("timeline:#{account.id}") true end # Remove a status from a home feed and send a streaming API update # @param [Account] account # @param [Status] status + # @param [Boolean] update # @return [Boolean] - def unpush_from_home(account, status) + def unpush_from_home(account, status, update: false) return false unless remove_from_feed(:home, account.id, status, account.user&.aggregates_reblogs?) - redis.publish("timeline:#{account.id}", Oj.dump(event: :delete, payload: status.id.to_s)) + redis.publish("timeline:#{account.id}", Oj.dump(event: :delete, payload: status.id.to_s)) unless update true end # Add a status to a list feed and send a streaming API update # @param [List] list # @param [Status] status + # @param [Boolean] update # @return [Boolean] - def push_to_list(list, status) + def push_to_list(list, status, update: false) return false if filter_from_list?(status, list) || !add_to_feed(:list, list.id, status, list.account.user&.aggregates_reblogs?) trim(:list, list.id) - PushUpdateWorker.perform_async(list.account_id, status.id, "timeline:list:#{list.id}") if push_update_required?("timeline:list:#{list.id}") + PushUpdateWorker.perform_async(list.account_id, status.id, "timeline:list:#{list.id}", update: update) if push_update_required?("timeline:list:#{list.id}") true end # Remove a status from a list feed and send a streaming API update # @param [List] list # @param [Status] status + # @param [Boolean] update # @return [Boolean] - def unpush_from_list(list, status) + def unpush_from_list(list, status, update: false) return false unless remove_from_feed(:list, list.id, status, list.account.user&.aggregates_reblogs?) - redis.publish("timeline:list:#{list.id}", Oj.dump(event: :delete, payload: status.id.to_s)) + redis.publish("timeline:list:#{list.id}", Oj.dump(event: :delete, payload: status.id.to_s)) unless update true end diff --git a/app/lib/status_reach_finder.rb b/app/lib/status_reach_finder.rb index 735d66a4f7..98e502bb68 100644 --- a/app/lib/status_reach_finder.rb +++ b/app/lib/status_reach_finder.rb @@ -1,8 +1,12 @@ # frozen_string_literal: true class StatusReachFinder - def initialize(status) - @status = status + # @param [Status] status + # @param [Hash] options + # @option options [Boolean] :unsafe + def initialize(status, options = {}) + @status = status + @options = options end def inboxes @@ -38,7 +42,7 @@ class StatusReachFinder end def replied_to_account_id - @status.in_reply_to_account_id + @status.in_reply_to_account_id if distributable? end def reblog_of_account_id @@ -49,21 +53,26 @@ class StatusReachFinder @status.mentions.pluck(:account_id) end + # Beware: Reblogs can be created without the author having had access to the status def reblogs_account_ids - @status.reblogs.pluck(:account_id) + @status.reblogs.pluck(:account_id) if distributable? || unsafe? end + # Beware: Favourites can be created without the author having had access to the status def favourites_account_ids - @status.favourites.pluck(:account_id) + @status.favourites.pluck(:account_id) if distributable? || unsafe? end + # Beware: Replies can be created without the author having had access to the status def replies_account_ids - @status.replies.pluck(:account_id) + @status.replies.pluck(:account_id) if distributable? || unsafe? end def followers_inboxes - if @status.in_reply_to_local_account? && @status.distributable? + if @status.in_reply_to_local_account? && distributable? @status.account.followers.or(@status.thread.account.followers).inboxes + elsif @status.direct_visibility? || @status.limited_visibility? + [] else @status.account.followers.inboxes end @@ -76,4 +85,12 @@ class StatusReachFinder [] end end + + def distributable? + @status.public_visibility? || @status.unlisted_visibility? + end + + def unsafe? + @options[:unsafe] + end end diff --git a/app/models/poll.rb b/app/models/poll.rb index d2a17277b2..71b5e191f8 100644 --- a/app/models/poll.rb +++ b/app/models/poll.rb @@ -26,6 +26,7 @@ class Poll < ApplicationRecord belongs_to :status has_many :votes, class_name: 'PollVote', inverse_of: :poll, dependent: :delete_all + has_many :voters, -> { group('accounts.id') }, through: :votes, class_name: 'Account', source: :account has_many :notifications, as: :activity, dependent: :destroy diff --git a/app/models/status.rb b/app/models/status.rb index 749a23718f..3358d6891b 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -23,6 +23,7 @@ # in_reply_to_account_id :bigint(8) # poll_id :bigint(8) # deleted_at :datetime +# edited_at :datetime # class Status < ApplicationRecord @@ -56,6 +57,8 @@ class Status < ApplicationRecord belongs_to :thread, foreign_key: 'in_reply_to_id', class_name: 'Status', inverse_of: :replies, optional: true belongs_to :reblog, foreign_key: 'reblog_of_id', class_name: 'Status', inverse_of: :reblogs, optional: true + has_many :edits, class_name: 'StatusEdit', inverse_of: :status, dependent: :destroy + has_many :favourites, inverse_of: :status, dependent: :destroy has_many :bookmarks, inverse_of: :status, dependent: :destroy has_many :reblogs, foreign_key: 'reblog_of_id', class_name: 'Status', inverse_of: :reblog, dependent: :destroy @@ -209,6 +212,10 @@ class Status < ApplicationRecord public_visibility? || unlisted_visibility? end + def edited? + edited_at.present? + end + alias sign? distributable? def with_media? diff --git a/app/models/status_edit.rb b/app/models/status_edit.rb new file mode 100644 index 0000000000..a89df86c5f --- /dev/null +++ b/app/models/status_edit.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true +# == Schema Information +# +# Table name: status_edits +# +# id :bigint(8) not null, primary key +# status_id :bigint(8) not null +# account_id :bigint(8) +# text :text default(""), not null +# spoiler_text :text default(""), not null +# media_attachments_changed :boolean default(FALSE), not null +# created_at :datetime not null +# updated_at :datetime not null +# + +class StatusEdit < ApplicationRecord + belongs_to :status + belongs_to :account, optional: true + + default_scope { order(id: :asc) } + + delegate :local?, to: :status +end diff --git a/app/serializers/activitypub/note_serializer.rb b/app/serializers/activitypub/note_serializer.rb index 7c52b634dd..12dabc65a0 100644 --- a/app/serializers/activitypub/note_serializer.rb +++ b/app/serializers/activitypub/note_serializer.rb @@ -11,6 +11,7 @@ class ActivityPub::NoteSerializer < ActivityPub::Serializer attribute :content attribute :content_map, if: :language? + attribute :updated, if: :edited? has_many :media_attachments, key: :attachment has_many :virtual_tags, key: :tag @@ -65,6 +66,8 @@ class ActivityPub::NoteSerializer < ActivityPub::Serializer object.language.present? end + delegate :edited?, to: :object + def in_reply_to return unless object.reply? && !object.thread.nil? @@ -79,6 +82,10 @@ class ActivityPub::NoteSerializer < ActivityPub::Serializer object.created_at.iso8601 end + def updated + object.edited_at.iso8601 + end + def url ActivityPub::TagManager.instance.url_for(object) end diff --git a/app/serializers/rest/status_edit_serializer.rb b/app/serializers/rest/status_edit_serializer.rb new file mode 100644 index 0000000000..b123b4e09e --- /dev/null +++ b/app/serializers/rest/status_edit_serializer.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true + +class REST::StatusEditSerializer < ActiveModel::Serializer + attributes :text, :spoiler_text, :media_attachments_changed, + :created_at +end diff --git a/app/serializers/rest/status_serializer.rb b/app/serializers/rest/status_serializer.rb index e84f3bd614..aef51e0f76 100644 --- a/app/serializers/rest/status_serializer.rb +++ b/app/serializers/rest/status_serializer.rb @@ -4,7 +4,7 @@ class REST::StatusSerializer < ActiveModel::Serializer attributes :id, :created_at, :in_reply_to_id, :in_reply_to_account_id, :sensitive, :spoiler_text, :visibility, :language, :uri, :url, :replies_count, :reblogs_count, - :favourites_count + :favourites_count, :edited_at attribute :favourited, if: :current_user? attribute :reblogged, if: :current_user? diff --git a/app/serializers/rest/status_source_serializer.rb b/app/serializers/rest/status_source_serializer.rb new file mode 100644 index 0000000000..cd3c740847 --- /dev/null +++ b/app/serializers/rest/status_source_serializer.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class REST::StatusSourceSerializer < ActiveModel::Serializer + attributes :id, :text, :spoiler_text + + def id + object.id.to_s + end +end diff --git a/app/services/activitypub/fetch_remote_poll_service.rb b/app/services/activitypub/fetch_remote_poll_service.rb index 1c79ecf116..1829e791ce 100644 --- a/app/services/activitypub/fetch_remote_poll_service.rb +++ b/app/services/activitypub/fetch_remote_poll_service.rb @@ -8,6 +8,6 @@ class ActivityPub::FetchRemotePollService < BaseService return unless supported_context?(json) - ActivityPub::ProcessPollService.new.call(poll, json) + ActivityPub::ProcessStatusUpdateService.new.call(poll.status, json) end end diff --git a/app/services/activitypub/process_poll_service.rb b/app/services/activitypub/process_poll_service.rb deleted file mode 100644 index d83e614d88..0000000000 --- a/app/services/activitypub/process_poll_service.rb +++ /dev/null @@ -1,64 +0,0 @@ -# frozen_string_literal: true - -class ActivityPub::ProcessPollService < BaseService - include JsonLdHelper - - def call(poll, json) - @json = json - - return unless expected_type? - - previous_expires_at = poll.expires_at - - expires_at = begin - if @json['closed'].is_a?(String) - @json['closed'] - elsif !@json['closed'].nil? && !@json['closed'].is_a?(FalseClass) - Time.now.utc - else - @json['endTime'] - end - end - - items = begin - if @json['anyOf'].is_a?(Array) - @json['anyOf'] - else - @json['oneOf'] - end - end - - voters_count = @json['votersCount'] - - latest_options = items.filter_map { |item| item['name'].presence || item['content'] } - - # If for some reasons the options were changed, it invalidates all previous - # votes, so we need to remove them - poll.votes.delete_all if latest_options != poll.options - - begin - poll.update!( - last_fetched_at: Time.now.utc, - expires_at: expires_at, - options: latest_options, - cached_tallies: items.map { |item| item.dig('replies', 'totalItems') || 0 }, - voters_count: voters_count - ) - rescue ActiveRecord::StaleObjectError - poll.reload - retry - end - - # If the poll had no expiration date set but now has, and people have voted, - # schedule a notification. - if previous_expires_at.nil? && poll.expires_at.present? && poll.votes.exists? - PollExpirationNotifyWorker.perform_at(poll.expires_at + 5.minutes, poll.id) - end - end - - private - - def expected_type? - equals_or_includes_any?(@json['type'], %w(Question)) - end -end diff --git a/app/services/activitypub/process_status_update_service.rb b/app/services/activitypub/process_status_update_service.rb new file mode 100644 index 0000000000..e3e9b9d6ae --- /dev/null +++ b/app/services/activitypub/process_status_update_service.rb @@ -0,0 +1,275 @@ +# frozen_string_literal: true + +class ActivityPub::ProcessStatusUpdateService < BaseService + include JsonLdHelper + + def call(status, json) + @json = json + @status_parser = ActivityPub::Parser::StatusParser.new(@json) + @uri = @status_parser.uri + @status = status + @account = status.account + @media_attachments_changed = false + + # Only native types can be updated at the moment + return if !expected_type? || already_updated_more_recently? + + # Only allow processing one create/update per status at a time + RedisLock.acquire(lock_options) do |lock| + if lock.acquired? + Status.transaction do + create_previous_edit! + update_media_attachments! + update_poll! + update_immediate_attributes! + update_metadata! + create_edit! + end + + queue_poll_notifications! + reset_preview_card! + broadcast_updates! + else + raise Mastodon::RaceConditionError + end + end + end + + private + + def update_media_attachments! + previous_media_attachments = @status.media_attachments.to_a + next_media_attachments = [] + + as_array(@json['attachment']).each do |attachment| + media_attachment_parser = ActivityPub::Parser::MediaAttachmentParser.new(attachment) + + next if media_attachment_parser.remote_url.blank? || next_media_attachments.size > 4 + + begin + media_attachment = previous_media_attachments.find { |previous_media_attachment| previous_media_attachment.remote_url == media_attachment_parser.remote_url } + media_attachment ||= MediaAttachment.new(account: @account, remote_url: media_attachment_parser.remote_url) + + # If a previously existing media attachment was significantly updated, mark + # media attachments as changed even if none were added or removed + if media_attachment_parser.significantly_changes?(media_attachment) + @media_attachments_changed = true + end + + media_attachment.description = media_attachment_parser.description + media_attachment.focus = media_attachment_parser.focus + media_attachment.thumbnail_remote_url = media_attachment_parser.thumbnail_remote_url + media_attachment.blurhash = media_attachment_parser.blurhash + media_attachment.save! + + next_media_attachments << media_attachment + + next if unsupported_media_type?(media_attachment_parser.file_content_type) || skip_download? + + RedownloadMediaWorker.perform_async(media_attachment.id) if media_attachment.remote_url_previously_changed? || media_attachment.thumbnail_remote_url_previously_changed? + rescue Addressable::URI::InvalidURIError => e + Rails.logger.debug "Invalid URL in attachment: #{e}" + end + end + + removed_media_attachments = previous_media_attachments - next_media_attachments + added_media_attachments = next_media_attachments - previous_media_attachments + + MediaAttachment.where(id: removed_media_attachments.map(&:id)).update_all(status_id: nil) + MediaAttachment.where(id: added_media_attachments.map(&:id)).update_all(status_id: @status.id) + + @media_attachments_changed = true if removed_media_attachments.positive? || added_media_attachments.positive? + end + + def update_poll! + previous_poll = @status.preloadable_poll + @previous_expires_at = previous_poll&.expires_at + poll_parser = ActivityPub::Parser::PollParser.new(@json) + + if poll_parser.valid? + poll = previous_poll || @account.polls.new(status: @status) + + # If for some reasons the options were changed, it invalidates all previous + # votes, so we need to remove them + if poll_parser.significantly_changes?(poll) + @media_attachments_changed = true + poll.votes.delete_all unless poll.new_record? + end + + poll.last_fetched_at = Time.now.utc + poll.options = poll_parser.options + poll.multiple = poll_parser.multiple + poll.expires_at = poll_parser.expires_at + poll.voters_count = poll_parser.voters_count + poll.cached_tallies = poll_parser.cached_tallies + poll.save! + + @status.poll_id = poll.id + elsif previous_poll.present? + previous_poll.destroy! + @media_attachments_changed = true + @status.poll_id = nil + end + end + + def update_immediate_attributes! + @status.text = @status_parser.text || '' + @status.spoiler_text = @status_parser.spoiler_text || '' + @status.sensitive = @account.sensitized? || @status_parser.sensitive || false + @status.language = @status_parser.language || detected_language + @status.edited_at = @status_parser.edited_at || Time.now.utc + + @status.save! + end + + def update_metadata! + @raw_tags = [] + @raw_mentions = [] + @raw_emojis = [] + + as_array(@json['tag']).each do |tag| + if equals_or_includes?(tag['type'], 'Hashtag') + @raw_tags << tag['name'] + elsif equals_or_includes?(tag['type'], 'Mention') + @raw_mentions << tag['href'] + elsif equals_or_includes?(tag['type'], 'Emoji') + @raw_emojis << tag + end + end + + update_tags! + update_mentions! + update_emojis! + end + + def update_tags! + @status.tags = Tag.find_or_create_by_names(@raw_tags) + end + + def update_mentions! + previous_mentions = @status.active_mentions.includes(:account).to_a + current_mentions = [] + + @raw_mentions.each do |href| + next if href.blank? + + account = ActivityPub::TagManager.instance.uri_to_resource(href, Account) + account ||= ActivityPub::FetchRemoteAccountService.new.call(href) + + next if account.nil? + + mention = previous_mentions.find { |x| x.account_id == account.id } + mention ||= account.mentions.new(status: @status) + + current_mentions << mention + end + + current_mentions.each do |mention| + mention.save if mention.new_record? + end + + # If previous mentions are no longer contained in the text, convert them + # to silent mentions, since withdrawing access from someone who already + # received a notification might be more confusing + removed_mentions = previous_mentions - current_mentions + + Mention.where(id: removed_mentions.map(&:id)).update_all(silent: true) unless removed_mentions.empty? + end + + def update_emojis! + return if skip_download? + + @raw_emojis.each do |raw_emoji| + custom_emoji_parser = ActivityPub::Parser::CustomEmojiParser.new(raw_emoji) + + next if custom_emoji_parser.shortcode.blank? || custom_emoji_parser.image_remote_url.blank? + + emoji = CustomEmoji.find_by(shortcode: custom_emoji_parser.shortcode, domain: @account.domain) + + next unless emoji.nil? || custom_emoji_parser.image_remote_url != emoji.image_remote_url || (custom_emoji_parser.updated_at && custom_emoji_parser.updated_at >= emoji.updated_at) + + begin + emoji ||= CustomEmoji.new(domain: @account.domain, shortcode: custom_emoji_parser.shortcode, uri: custom_emoji_parser.uri) + emoji.image_remote_url = custom_emoji_parser.image_remote_url + emoji.save + rescue Seahorse::Client::NetworkingError => e + Rails.logger.warn "Error storing emoji: #{e}" + end + end + end + + def expected_type? + equals_or_includes_any?(@json['type'], %w(Note Question)) + end + + def lock_options + { redis: Redis.current, key: "create:#{@uri}", autorelease: 15.minutes.seconds } + end + + def detected_language + LanguageDetector.instance.detect(@status_parser.text, @account) + end + + def create_previous_edit! + # We only need to create a previous edit when no previous edits exist, e.g. + # when the status has never been edited. For other cases, we always create + # an edit, so the step can be skipped + + return if @status.edits.any? + + @status.edits.create( + text: @status.text, + spoiler_text: @status.spoiler_text, + media_attachments_changed: false, + account_id: @account.id, + created_at: @status.created_at + ) + end + + def create_edit! + return unless @status.text_previously_changed? || @status.spoiler_text_previously_changed? || @media_attachments_changed + + @status_edit = @status.edits.create( + text: @status.text, + spoiler_text: @status.spoiler_text, + media_attachments_changed: @media_attachments_changed, + account_id: @account.id, + created_at: @status.edited_at + ) + end + + def skip_download? + return @skip_download if defined?(@skip_download) + + @skip_download ||= DomainBlock.reject_media?(@account.domain) + end + + def unsupported_media_type?(mime_type) + mime_type.present? && !MediaAttachment.supported_mime_types.include?(mime_type) + end + + def already_updated_more_recently? + @status.edited_at.present? && @status_parser.edited_at.present? && @status.edited_at > @status_parser.edited_at + end + + def reset_preview_card! + @status.preview_cards.clear if @status.text_previously_changed? || @status.spoiler_text.present? + LinkCrawlWorker.perform_in(rand(1..59).seconds, @status.id) if @status.spoiler_text.blank? + end + + def broadcast_updates! + ::DistributionWorker.perform_async(@status.id, update: true) + end + + def queue_poll_notifications! + poll = @status.preloadable_poll + + # If the poll had no expiration date set but now has, or now has a sooner + # expiration date, and people have voted, schedule a notification + + return unless poll.present? && poll.expires_at.present? && poll.votes.exists? + + PollExpirationNotifyWorker.remove_from_scheduled(poll.id) if @previous_expires_at.present? && @previous_expires_at > poll.expires_at + PollExpirationNotifyWorker.perform_at(poll.expires_at + 5.minutes, poll.id) + end +end diff --git a/app/services/fan_out_on_write_service.rb b/app/services/fan_out_on_write_service.rb index b72bb82d3f..f62f78a790 100644 --- a/app/services/fan_out_on_write_service.rb +++ b/app/services/fan_out_on_write_service.rb @@ -3,107 +3,126 @@ class FanOutOnWriteService < BaseService # Push a status into home and mentions feeds # @param [Status] status - def call(status) - raise Mastodon::RaceConditionError if status.visibility.nil? + # @param [Hash] options + # @option options [Boolean] update + # @option options [Array] silenced_account_ids + def call(status, options = {}) + @status = status + @account = status.account + @options = options - deliver_to_self(status) if status.account.local? + check_race_condition! - if status.direct_visibility? - deliver_to_mentioned_followers(status) - deliver_to_own_conversation(status) - elsif status.limited_visibility? - deliver_to_mentioned_followers(status) - else - deliver_to_followers(status) - deliver_to_lists(status) - end - - return if status.account.silenced? || !status.public_visibility? || status.reblog? - - render_anonymous_payload(status) - - deliver_to_hashtags(status) - - return if status.reply? && status.in_reply_to_account_id != status.account_id - - deliver_to_public(status) - deliver_to_media(status) if status.media_attachments.any? + fan_out_to_local_recipients! + fan_out_to_public_streams! if broadcastable? end private - def deliver_to_self(status) - Rails.logger.debug "Delivering status #{status.id} to author" - FeedManager.instance.push_to_home(status.account, status) + def check_race_condition! + # I don't know why but at some point we had an issue where + # this service was being executed with status objects + # that had a null visibility - which should not be possible + # since the column in the database is not nullable. + # + # This check re-queues the service to be run at a later time + # with the full object, if something like it occurs + + raise Mastodon::RaceConditionError if @status.visibility.nil? end - def deliver_to_followers(status) - Rails.logger.debug "Delivering status #{status.id} to followers" + def fan_out_to_local_recipients! + deliver_to_self! + notify_mentioned_accounts! - status.account.followers_for_local_distribution.select(:id).reorder(nil).find_in_batches do |followers| + case @status.visibility.to_sym + when :public, :unlisted, :private + deliver_to_all_followers! + deliver_to_lists! + when :limited + deliver_to_mentioned_followers! + else + deliver_to_mentioned_followers! + deliver_to_conversation! + end + end + + def fan_out_to_public_streams! + broadcast_to_hashtag_streams! + broadcast_to_public_streams! + end + + def deliver_to_self! + FeedManager.instance.push_to_home(@account, @status, update: update?) if @account.local? + end + + def notify_mentioned_accounts! + @status.active_mentions.where.not(id: @options[:silenced_account_ids] || []).joins(:account).merge(Account.local).select(:id, :account_id).reorder(nil).find_in_batches do |mentions| + LocalNotificationWorker.push_bulk(mentions) do |mention| + [mention.account_id, mention.id, 'Mention', :mention] + end + end + end + + def deliver_to_all_followers! + @account.followers_for_local_distribution.select(:id).reorder(nil).find_in_batches do |followers| FeedInsertWorker.push_bulk(followers) do |follower| - [status.id, follower.id, :home] + [@status.id, follower.id, :home, update: update?] end end end - def deliver_to_lists(status) - Rails.logger.debug "Delivering status #{status.id} to lists" - - status.account.lists_for_local_distribution.select(:id).reorder(nil).find_in_batches do |lists| + def deliver_to_lists! + @account.lists_for_local_distribution.select(:id).reorder(nil).find_in_batches do |lists| FeedInsertWorker.push_bulk(lists) do |list| - [status.id, list.id, :list] + [@status.id, list.id, :list, update: update?] end end end - def deliver_to_mentioned_followers(status) - Rails.logger.debug "Delivering status #{status.id} to limited followers" - - status.mentions.joins(:account).merge(status.account.followers_for_local_distribution).select(:id, :account_id).reorder(nil).find_in_batches do |mentions| + def deliver_to_mentioned_followers! + @status.mentions.joins(:account).merge(@account.followers_for_local_distribution).select(:id, :account_id).reorder(nil).find_in_batches do |mentions| FeedInsertWorker.push_bulk(mentions) do |mention| - [status.id, mention.account_id, :home] + [@status.id, mention.account_id, :home, update: update?] end end end - def render_anonymous_payload(status) - @payload = InlineRenderer.render(status, nil, :status) - @payload = Oj.dump(event: :update, payload: @payload) - end - - def deliver_to_hashtags(status) - Rails.logger.debug "Delivering status #{status.id} to hashtags" - - status.tags.pluck(:name).each do |hashtag| - Redis.current.publish("timeline:hashtag:#{hashtag.mb_chars.downcase}", @payload) - Redis.current.publish("timeline:hashtag:#{hashtag.mb_chars.downcase}:local", @payload) if status.local? + def broadcast_to_hashtag_streams! + @status.tags.pluck(:name).each do |hashtag| + Redis.current.publish("timeline:hashtag:#{hashtag.mb_chars.downcase}", anonymous_payload) + Redis.current.publish("timeline:hashtag:#{hashtag.mb_chars.downcase}:local", anonymous_payload) if @status.local? end end - def deliver_to_public(status) - Rails.logger.debug "Delivering status #{status.id} to public timeline" + def broadcast_to_public_streams! + return if @status.reply? && @status.in_reply_to_account_id != @account.id - Redis.current.publish('timeline:public', @payload) - if status.local? - Redis.current.publish('timeline:public:local', @payload) - else - Redis.current.publish('timeline:public:remote', @payload) + Redis.current.publish('timeline:public', anonymous_payload) + Redis.current.publish(@status.local? ? 'timeline:public:local' : 'timeline:public:remote', anonymous_payload) + + if @status.media_attachments.any? + Redis.current.publish('timeline:public:media', anonymous_payload) + Redis.current.publish(@status.local? ? 'timeline:public:local:media' : 'timeline:public:remote:media', anonymous_payload) end end - def deliver_to_media(status) - Rails.logger.debug "Delivering status #{status.id} to media timeline" - - Redis.current.publish('timeline:public:media', @payload) - if status.local? - Redis.current.publish('timeline:public:local:media', @payload) - else - Redis.current.publish('timeline:public:remote:media', @payload) - end + def deliver_to_conversation! + AccountConversation.add_status(@account, @status) unless update? end - def deliver_to_own_conversation(status) - AccountConversation.add_status(status.account, status) + def anonymous_payload + @anonymous_payload ||= Oj.dump( + event: update? ? :'status.update' : :update, + payload: InlineRenderer.render(@status, nil, :status) + ) + end + + def update? + @is_update + end + + def broadcastable? + @status.public_visibility? && !@status.reblog? && !@account.silenced? end end diff --git a/app/services/process_mentions_service.rb b/app/services/process_mentions_service.rb index 73dbb18345..9d239fc657 100644 --- a/app/services/process_mentions_service.rb +++ b/app/services/process_mentions_service.rb @@ -8,12 +8,23 @@ class ProcessMentionsService < BaseService # remote users # @param [Status] status def call(status) - return unless status.local? + @status = status - @status = status - mentions = [] + return unless @status.local? - status.text = status.text.gsub(Account::MENTION_RE) do |match| + @previous_mentions = @status.active_mentions.includes(:account).to_a + @current_mentions = [] + + Status.transaction do + scan_text! + assign_mentions! + end + end + + private + + def scan_text! + @status.text = @status.text.gsub(Account::MENTION_RE) do |match| username, domain = Regexp.last_match(1).split('@') domain = begin @@ -26,49 +37,45 @@ class ProcessMentionsService < BaseService mentioned_account = Account.find_remote(username, domain) + # If the account cannot be found or isn't the right protocol, + # first try to resolve it if mention_undeliverable?(mentioned_account) begin - mentioned_account = resolve_account_service.call(Regexp.last_match(1)) + mentioned_account = ResolveAccountService.new.call(Regexp.last_match(1)) rescue Webfinger::Error, HTTP::Error, OpenSSL::SSL::SSLError, Mastodon::UnexpectedResponseError mentioned_account = nil end end + # If after resolving it still isn't found or isn't the right + # protocol, then give up next match if mention_undeliverable?(mentioned_account) || mentioned_account&.suspended? - mention = mentioned_account.mentions.new(status: status) - mentions << mention if mention.save + mention = @previous_mentions.find { |x| x.account_id == mentioned_account.id } + mention ||= mentioned_account.mentions.new(status: @status) + + @current_mentions << mention "@#{mentioned_account.acct}" end - status.save! - - mentions.each { |mention| create_notification(mention) } + @status.save! end - private + def assign_mentions! + @current_mentions.each do |mention| + mention.save if mention.new_record? + end + + # If previous mentions are no longer contained in the text, convert them + # to silent mentions, since withdrawing access from someone who already + # received a notification might be more confusing + removed_mentions = @previous_mentions - @current_mentions + + Mention.where(id: removed_mentions.map(&:id)).update_all(silent: true) unless removed_mentions.empty? + end def mention_undeliverable?(mentioned_account) - mentioned_account.nil? || (!mentioned_account.local? && mentioned_account.ostatus?) - end - - def create_notification(mention) - mentioned_account = mention.account - - if mentioned_account.local? - LocalNotificationWorker.perform_async(mentioned_account.id, mention.id, mention.class.name, :mention) - elsif mentioned_account.activitypub? - ActivityPub::DeliveryWorker.perform_async(activitypub_json, mention.status.account_id, mentioned_account.inbox_url, { synchronize_followers: !mention.status.distributable? }) - end - end - - def activitypub_json - return @activitypub_json if defined?(@activitypub_json) - @activitypub_json = Oj.dump(serialize_payload(ActivityPub::ActivityPresenter.from_status(@status), ActivityPub::ActivitySerializer, signer: @status.account)) - end - - def resolve_account_service - ResolveAccountService.new + mentioned_account.nil? || (!mentioned_account.local? && !mentioned_account.activitypub?) end end diff --git a/app/services/remove_status_service.rb b/app/services/remove_status_service.rb index 3535b503be..bec95bb1bc 100644 --- a/app/services/remove_status_service.rb +++ b/app/services/remove_status_service.rb @@ -87,7 +87,7 @@ class RemoveStatusService < BaseService # the author and wouldn't normally receive the delete # notification - so here, we explicitly send it to them - status_reach_finder = StatusReachFinder.new(@status) + status_reach_finder = StatusReachFinder.new(@status, unsafe: true) ActivityPub::DeliveryWorker.push_bulk(status_reach_finder.inboxes) do |inbox_url| [signed_activity_json, @account.id, inbox_url] diff --git a/app/workers/activitypub/distribution_worker.rb b/app/workers/activitypub/distribution_worker.rb index 09898ca49e..17c108461e 100644 --- a/app/workers/activitypub/distribution_worker.rb +++ b/app/workers/activitypub/distribution_worker.rb @@ -1,54 +1,32 @@ # frozen_string_literal: true -class ActivityPub::DistributionWorker - include Sidekiq::Worker - include Payloadable - - sidekiq_options queue: 'push' - +class ActivityPub::DistributionWorker < ActivityPub::RawDistributionWorker + # Distribute a new status or an edit of a status to all the places + # where the status is supposed to go or where it was interacted with def perform(status_id) @status = Status.find(status_id) @account = @status.account - return if skip_distribution? - - ActivityPub::DeliveryWorker.push_bulk(inboxes) do |inbox_url| - [payload, @account.id, inbox_url, { synchronize_followers: !@status.distributable? }] - end - - relay! if relayable? + distribute! rescue ActiveRecord::RecordNotFound true end - private - - def skip_distribution? - @status.direct_visibility? || @status.limited_visibility? - end - - def relayable? - @status.public_visibility? - end + protected def inboxes - # Deliver the status to all followers. - # If the status is a reply to another local status, also forward it to that - # status' authors' followers. - @inboxes ||= if @status.in_reply_to_local_account? && @status.distributable? - @account.followers.or(@status.thread.account.followers).inboxes - else - @account.followers.inboxes - end + @inboxes ||= StatusReachFinder.new(@status).inboxes end def payload - @payload ||= Oj.dump(serialize_payload(ActivityPub::ActivityPresenter.from_status(@status), ActivityPub::ActivitySerializer, signer: @account)) + @payload ||= Oj.dump(serialize_payload(activity, ActivityPub::ActivitySerializer, signer: @account)) end - def relay! - ActivityPub::DeliveryWorker.push_bulk(Relay.enabled.pluck(:inbox_url)) do |inbox_url| - [payload, @account.id, inbox_url] - end + def activity + ActivityPub::ActivityPresenter.from_status(@status) + end + + def options + { synchronize_followers: @status.private_visibility? } end end diff --git a/app/workers/activitypub/raw_distribution_worker.rb b/app/workers/activitypub/raw_distribution_worker.rb index 41e61132fb..ac5eda4af7 100644 --- a/app/workers/activitypub/raw_distribution_worker.rb +++ b/app/workers/activitypub/raw_distribution_worker.rb @@ -2,22 +2,47 @@ class ActivityPub::RawDistributionWorker include Sidekiq::Worker + include Payloadable sidekiq_options queue: 'push' + # Base worker for when you want to queue up a bunch of deliveries of + # some payload. In this case, we have already generated JSON and + # we are going to distribute it to the account's followers minus + # the explicitly provided inboxes def perform(json, source_account_id, exclude_inboxes = []) - @account = Account.find(source_account_id) + @account = Account.find(source_account_id) + @json = json + @exclude_inboxes = exclude_inboxes - ActivityPub::DeliveryWorker.push_bulk(inboxes - exclude_inboxes) do |inbox_url| - [json, @account.id, inbox_url] - end + distribute! rescue ActiveRecord::RecordNotFound true end - private + protected + + def distribute! + return if inboxes.empty? + + ActivityPub::DeliveryWorker.push_bulk(inboxes) do |inbox_url| + [payload, source_account_id, inbox_url, options] + end + end + + def payload + @json + end + + def source_account_id + @account.id + end def inboxes - @inboxes ||= @account.followers.inboxes + @inboxes ||= @account.followers.inboxes - @exclude_inboxes + end + + def options + nil end end diff --git a/app/workers/activitypub/reply_distribution_worker.rb b/app/workers/activitypub/reply_distribution_worker.rb deleted file mode 100644 index d4d0148ac3..0000000000 --- a/app/workers/activitypub/reply_distribution_worker.rb +++ /dev/null @@ -1,34 +0,0 @@ -# frozen_string_literal: true - -# Obsolete but kept around to make sure existing jobs do not fail after upgrade. -# Should be removed in a subsequent release. - -class ActivityPub::ReplyDistributionWorker - include Sidekiq::Worker - include Payloadable - - sidekiq_options queue: 'push' - - def perform(status_id) - @status = Status.find(status_id) - @account = @status.thread&.account - - return unless @account.present? && @status.distributable? - - ActivityPub::DeliveryWorker.push_bulk(inboxes) do |inbox_url| - [payload, @status.account_id, inbox_url] - end - rescue ActiveRecord::RecordNotFound - true - end - - private - - def inboxes - @inboxes ||= @account.followers.inboxes - end - - def payload - @payload ||= Oj.dump(serialize_payload(ActivityPub::ActivityPresenter.from_status(@status), ActivityPub::ActivitySerializer, signer: @status.account)) - end -end diff --git a/app/workers/activitypub/update_distribution_worker.rb b/app/workers/activitypub/update_distribution_worker.rb index 3a207f0719..81fde63b84 100644 --- a/app/workers/activitypub/update_distribution_worker.rb +++ b/app/workers/activitypub/update_distribution_worker.rb @@ -1,33 +1,24 @@ # frozen_string_literal: true -class ActivityPub::UpdateDistributionWorker - include Sidekiq::Worker - include Payloadable - - sidekiq_options queue: 'push' - +class ActivityPub::UpdateDistributionWorker < ActivityPub::RawDistributionWorker + # Distribute an profile update to servers that might have a copy + # of the account in question def perform(account_id, options = {}) @options = options.with_indifferent_access @account = Account.find(account_id) - ActivityPub::DeliveryWorker.push_bulk(inboxes) do |inbox_url| - [signed_payload, @account.id, inbox_url] - end - - ActivityPub::DeliveryWorker.push_bulk(Relay.enabled.pluck(:inbox_url)) do |inbox_url| - [signed_payload, @account.id, inbox_url] - end + distribute! rescue ActiveRecord::RecordNotFound true end - private + protected def inboxes - @inboxes ||= @account.followers.inboxes + @inboxes ||= AccountReachFinder.new(@account).inboxes end - def signed_payload - @signed_payload ||= Oj.dump(serialize_payload(@account, ActivityPub::UpdateSerializer, signer: @account, sign_with: @options[:sign_with])) + def payload + @payload ||= Oj.dump(serialize_payload(@account, ActivityPub::UpdateSerializer, signer: @account, sign_with: @options[:sign_with])) end end diff --git a/app/workers/distribution_worker.rb b/app/workers/distribution_worker.rb index e85cd7e95a..770325ccf3 100644 --- a/app/workers/distribution_worker.rb +++ b/app/workers/distribution_worker.rb @@ -3,10 +3,10 @@ class DistributionWorker include Sidekiq::Worker - def perform(status_id) + def perform(status_id, options = {}) RedisLock.acquire(redis: Redis.current, key: "distribute:#{status_id}", autorelease: 5.minutes.seconds) do |lock| if lock.acquired? - FanOutOnWriteService.new.call(Status.find(status_id)) + FanOutOnWriteService.new.call(Status.find(status_id), **options.symbolize_keys) else raise Mastodon::RaceConditionError end diff --git a/app/workers/feed_insert_worker.rb b/app/workers/feed_insert_worker.rb index b70c7e3895..0122be95d9 100644 --- a/app/workers/feed_insert_worker.rb +++ b/app/workers/feed_insert_worker.rb @@ -3,9 +3,10 @@ class FeedInsertWorker include Sidekiq::Worker - def perform(status_id, id, type = :home) - @type = type.to_sym - @status = Status.find(status_id) + def perform(status_id, id, type = :home, options = {}) + @type = type.to_sym + @status = Status.find(status_id) + @options = options.symbolize_keys case @type when :home @@ -23,10 +24,12 @@ class FeedInsertWorker private def check_and_insert - return if feed_filtered? - - perform_push - perform_notify if notify? + if feed_filtered? + perform_unpush if update? + else + perform_push + perform_notify if notify? + end end def feed_filtered? @@ -47,13 +50,26 @@ class FeedInsertWorker def perform_push case @type when :home - FeedManager.instance.push_to_home(@follower, @status) + FeedManager.instance.push_to_home(@follower, @status, update: update?) when :list - FeedManager.instance.push_to_list(@list, @status) + FeedManager.instance.push_to_list(@list, @status, update: update?) + end + end + + def perform_unpush + case @type + when :home + FeedManager.instance.unpush_from_home(@follower, @status, update: true) + when :list + FeedManager.instance.unpush_from_list(@list, @status, update: true) end end def perform_notify NotifyService.new.call(@follower, :status, @status) end + + def update? + @options[:update] + end end diff --git a/app/workers/local_notification_worker.rb b/app/workers/local_notification_worker.rb index 6b08ca6fcf..a22e2834de 100644 --- a/app/workers/local_notification_worker.rb +++ b/app/workers/local_notification_worker.rb @@ -12,6 +12,8 @@ class LocalNotificationWorker activity = activity_class_name.constantize.find(activity_id) end + return if Notification.where(account: receiver, activity: activity).any? + NotifyService.new.call(receiver, type || activity_class_name.underscore, activity) rescue ActiveRecord::RecordNotFound true diff --git a/app/workers/poll_expiration_notify_worker.rb b/app/workers/poll_expiration_notify_worker.rb index f0191d4799..7613ed5f15 100644 --- a/app/workers/poll_expiration_notify_worker.rb +++ b/app/workers/poll_expiration_notify_worker.rb @@ -6,19 +6,44 @@ class PollExpirationNotifyWorker sidekiq_options lock: :until_executed def perform(poll_id) - poll = Poll.find(poll_id) + @poll = Poll.find(poll_id) - # Notify poll owner and remote voters - if poll.local? - ActivityPub::DistributePollUpdateWorker.perform_async(poll.status.id) - NotifyService.new.call(poll.account, :poll, poll) - end + return if does_not_expire? + requeue! && return if not_due_yet? - # Notify local voters - poll.votes.includes(:account).group(:account_id).select(:account_id).map(&:account).select(&:local?).each do |account| - NotifyService.new.call(account, :poll, poll) - end + notify_remote_voters_and_owner! if @poll.local? + notify_local_voters! rescue ActiveRecord::RecordNotFound true end + + def self.remove_from_scheduled(poll_id) + queue = Sidekiq::ScheduledSet.new + queue.select { |scheduled| scheduled.klass == name && scheduled.args[0] == poll_id }.map(&:delete) + end + + private + + def does_not_expire? + @poll.expires_at.nil? + end + + def not_due_yet? + @poll.expires_at.present? && !@poll.expired? + end + + def requeue! + PollExpirationNotifyWorker.perform_at(@poll.expires_at + 5.minutes, @poll.id) + end + + def notify_remote_voters_and_owner! + ActivityPub::DistributePollUpdateWorker.perform_async(@poll.status.id) + NotifyService.new.call(@poll.account, :poll, @poll) + end + + def notify_local_voters! + @poll.voters.merge(Account.local).find_each do |account| + NotifyService.new.call(account, :poll, @poll) + end + end end diff --git a/app/workers/push_update_worker.rb b/app/workers/push_update_worker.rb index d76d73d964..ae444cfde9 100644 --- a/app/workers/push_update_worker.rb +++ b/app/workers/push_update_worker.rb @@ -2,15 +2,38 @@ class PushUpdateWorker include Sidekiq::Worker + include Redisable - def perform(account_id, status_id, timeline_id = nil) - account = Account.find(account_id) - status = Status.find(status_id) - message = InlineRenderer.render(status, account, :status) - timeline_id = "timeline:#{account.id}" if timeline_id.nil? + def perform(account_id, status_id, timeline_id = nil, options = {}) + @account = Account.find(account_id) + @status = Status.find(status_id) + @timeline_id = timeline_id || "timeline:#{account.id}" + @options = options.symbolize_keys - Redis.current.publish(timeline_id, Oj.dump(event: :update, payload: message, queued_at: (Time.now.to_f * 1000.0).to_i)) + publish! rescue ActiveRecord::RecordNotFound true end + + private + + def payload + InlineRenderer.render(@status, @account, :status) + end + + def message + Oj.dump( + event: update? ? :'status.update' : :update, + payload: payload, + queued_at: (Time.now.to_f * 1000.0).to_i + ) + end + + def publish! + redis.publish(@timeline_id, message) + end + + def update? + @options[:update] + end end diff --git a/config/routes.rb b/config/routes.rb index 41ba453791..121587819a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -350,6 +350,9 @@ Rails.application.routes.draw do resource :pin, only: :create post :unpin, to: 'pins#destroy' + + resource :history, only: :show + resource :source, only: :show end member do diff --git a/db/migrate/20210904215403_add_edited_at_to_statuses.rb b/db/migrate/20210904215403_add_edited_at_to_statuses.rb new file mode 100644 index 0000000000..216ad8e138 --- /dev/null +++ b/db/migrate/20210904215403_add_edited_at_to_statuses.rb @@ -0,0 +1,5 @@ +class AddEditedAtToStatuses < ActiveRecord::Migration[6.1] + def change + add_column :statuses, :edited_at, :datetime + end +end diff --git a/db/migrate/20210908220918_create_status_edits.rb b/db/migrate/20210908220918_create_status_edits.rb new file mode 100644 index 0000000000..6c90149d00 --- /dev/null +++ b/db/migrate/20210908220918_create_status_edits.rb @@ -0,0 +1,13 @@ +class CreateStatusEdits < ActiveRecord::Migration[6.1] + def change + create_table :status_edits do |t| + t.belongs_to :status, null: false, foreign_key: { on_delete: :cascade } + t.belongs_to :account, null: true, foreign_key: { on_delete: :nullify } + t.text :text, null: false, default: '' + t.text :spoiler_text, null: false, default: '' + t.boolean :media_attachments_changed, null: false, default: false + + t.timestamps + end + end +end diff --git a/db/schema.rb b/db/schema.rb index ed615a1ee0..4e0f76dcdb 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -816,6 +816,18 @@ ActiveRecord::Schema.define(version: 2022_01_16_202951) do t.index ["var"], name: "index_site_uploads_on_var", unique: true end + create_table "status_edits", force: :cascade do |t| + t.bigint "status_id", null: false + t.bigint "account_id" + t.text "text", default: "", null: false + t.text "spoiler_text", default: "", null: false + t.boolean "media_attachments_changed", default: false, null: false + t.datetime "created_at", precision: 6, null: false + t.datetime "updated_at", precision: 6, null: false + t.index ["account_id"], name: "index_status_edits_on_account_id" + t.index ["status_id"], name: "index_status_edits_on_status_id" + end + create_table "status_pins", force: :cascade do |t| t.bigint "account_id", null: false t.bigint "status_id", null: false @@ -854,6 +866,7 @@ ActiveRecord::Schema.define(version: 2022_01_16_202951) do t.bigint "in_reply_to_account_id" t.bigint "poll_id" t.datetime "deleted_at" + t.datetime "edited_at" t.index ["account_id", "id", "visibility", "updated_at"], name: "index_statuses_20190820", order: { id: :desc }, where: "(deleted_at IS NULL)" t.index ["deleted_at"], name: "index_statuses_on_deleted_at", where: "(deleted_at IS NOT NULL)" t.index ["id", "account_id"], name: "index_statuses_local_20190824", order: { id: :desc }, where: "((local OR (uri IS NULL)) AND (deleted_at IS NULL) AND (visibility = 0) AND (reblog_of_id IS NULL) AND ((NOT reply) OR (in_reply_to_account_id = account_id)))" @@ -1081,6 +1094,8 @@ ActiveRecord::Schema.define(version: 2022_01_16_202951) do add_foreign_key "scheduled_statuses", "accounts", on_delete: :cascade add_foreign_key "session_activations", "oauth_access_tokens", column: "access_token_id", name: "fk_957e5bda89", on_delete: :cascade add_foreign_key "session_activations", "users", name: "fk_e5fda67334", on_delete: :cascade + add_foreign_key "status_edits", "accounts", on_delete: :nullify + add_foreign_key "status_edits", "statuses", on_delete: :cascade add_foreign_key "status_pins", "accounts", name: "fk_d4cb435b62", on_delete: :cascade add_foreign_key "status_pins", "statuses", on_delete: :cascade add_foreign_key "status_stats", "statuses", on_delete: :cascade diff --git a/spec/controllers/api/v1/statuses/histories_controller_spec.rb b/spec/controllers/api/v1/statuses/histories_controller_spec.rb new file mode 100644 index 0000000000..8d9d6a3591 --- /dev/null +++ b/spec/controllers/api/v1/statuses/histories_controller_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe Api::V1::Statuses::HistoriesController do + render_views + + let(:user) { Fabricate(:user, account: Fabricate(:account, username: 'alice')) } + let(:app) { Fabricate(:application, name: 'Test app', website: 'http://testapp.com') } + let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: 'read:statuses', application: app) } + + context 'with an oauth token' do + before do + allow(controller).to receive(:doorkeeper_token) { token } + end + + describe 'GET #show' do + let(:status) { Fabricate(:status, account: user.account) } + + before do + get :show, params: { status_id: status.id } + end + + it 'returns http success' do + expect(response).to have_http_status(200) + end + end + end +end diff --git a/spec/controllers/api/v1/statuses/sources_controller_spec.rb b/spec/controllers/api/v1/statuses/sources_controller_spec.rb new file mode 100644 index 0000000000..293c90ec9c --- /dev/null +++ b/spec/controllers/api/v1/statuses/sources_controller_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe Api::V1::Statuses::SourcesController do + render_views + + let(:user) { Fabricate(:user, account: Fabricate(:account, username: 'alice')) } + let(:app) { Fabricate(:application, name: 'Test app', website: 'http://testapp.com') } + let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: 'read:statuses', application: app) } + + context 'with an oauth token' do + before do + allow(controller).to receive(:doorkeeper_token) { token } + end + + describe 'GET #show' do + let(:status) { Fabricate(:status, account: user.account) } + + before do + get :show, params: { status_id: status.id } + end + + it 'returns http success' do + expect(response).to have_http_status(200) + end + end + end +end diff --git a/spec/fabricators/preview_card_fabricator.rb b/spec/fabricators/preview_card_fabricator.rb new file mode 100644 index 0000000000..f119c117d9 --- /dev/null +++ b/spec/fabricators/preview_card_fabricator.rb @@ -0,0 +1,6 @@ +Fabricator(:preview_card) do + url { Faker::Internet.url } + title { Faker::Lorem.sentence } + description { Faker::Lorem.paragraph } + type 'link' +end diff --git a/spec/fabricators/status_edit_fabricator.rb b/spec/fabricators/status_edit_fabricator.rb new file mode 100644 index 0000000000..21b793747e --- /dev/null +++ b/spec/fabricators/status_edit_fabricator.rb @@ -0,0 +1,7 @@ +Fabricator(:status_edit) do + status nil + account nil + text "MyText" + spoiler_text "MyText" + media_attachments_changed false +end \ No newline at end of file diff --git a/spec/lib/status_reach_finder_spec.rb b/spec/lib/status_reach_finder_spec.rb new file mode 100644 index 0000000000..f0c22b1651 --- /dev/null +++ b/spec/lib/status_reach_finder_spec.rb @@ -0,0 +1,109 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe StatusReachFinder do + describe '#inboxes' do + context 'for a local status' do + let(:parent_status) { nil } + let(:visibility) { :public } + let(:alice) { Fabricate(:account, username: 'alice') } + let(:status) { Fabricate(:status, account: alice, thread: parent_status, visibility: visibility) } + + subject { described_class.new(status) } + + context 'when it contains mentions of remote accounts' do + let(:bob) { Fabricate(:account, username: 'bob', domain: 'foo.bar', protocol: :activitypub, inbox_url: 'https://foo.bar/inbox') } + + before do + status.mentions.create!(account: bob) + end + + it 'includes the inbox of the mentioned account' do + expect(subject.inboxes).to include 'https://foo.bar/inbox' + end + end + + context 'when it has been reblogged by a remote account' do + let(:bob) { Fabricate(:account, username: 'bob', domain: 'foo.bar', protocol: :activitypub, inbox_url: 'https://foo.bar/inbox') } + + before do + bob.statuses.create!(reblog: status) + end + + it 'includes the inbox of the reblogger' do + expect(subject.inboxes).to include 'https://foo.bar/inbox' + end + + context 'when status is not public' do + let(:visibility) { :private } + + it 'does not include the inbox of the reblogger' do + expect(subject.inboxes).to_not include 'https://foo.bar/inbox' + end + end + end + + context 'when it has been favourited by a remote account' do + let(:bob) { Fabricate(:account, username: 'bob', domain: 'foo.bar', protocol: :activitypub, inbox_url: 'https://foo.bar/inbox') } + + before do + bob.favourites.create!(status: status) + end + + it 'includes the inbox of the favouriter' do + expect(subject.inboxes).to include 'https://foo.bar/inbox' + end + + context 'when status is not public' do + let(:visibility) { :private } + + it 'does not include the inbox of the favouriter' do + expect(subject.inboxes).to_not include 'https://foo.bar/inbox' + end + end + end + + context 'when it has been replied to by a remote account' do + let(:bob) { Fabricate(:account, username: 'bob', domain: 'foo.bar', protocol: :activitypub, inbox_url: 'https://foo.bar/inbox') } + + before do + bob.statuses.create!(thread: status, text: 'Hoge') + end + + context do + it 'includes the inbox of the replier' do + expect(subject.inboxes).to include 'https://foo.bar/inbox' + end + end + + context 'when status is not public' do + let(:visibility) { :private } + + it 'does not include the inbox of the replier' do + expect(subject.inboxes).to_not include 'https://foo.bar/inbox' + end + end + end + + context 'when it is a reply to a remote account' do + let(:bob) { Fabricate(:account, username: 'bob', domain: 'foo.bar', protocol: :activitypub, inbox_url: 'https://foo.bar/inbox') } + let(:parent_status) { Fabricate(:status, account: bob) } + + context do + it 'includes the inbox of the replied-to account' do + expect(subject.inboxes).to include 'https://foo.bar/inbox' + end + end + + context 'when status is not public and replied-to account is not mentioned' do + let(:visibility) { :private } + + it 'does not include the inbox of the replied-to account' do + expect(subject.inboxes).to_not include 'https://foo.bar/inbox' + end + end + end + end + end +end diff --git a/spec/models/status_edit_spec.rb b/spec/models/status_edit_spec.rb new file mode 100644 index 0000000000..2ecafef734 --- /dev/null +++ b/spec/models/status_edit_spec.rb @@ -0,0 +1,5 @@ +require 'rails_helper' + +RSpec.describe StatusEdit, type: :model do + pending "add some examples to (or delete) #{__FILE__}" +end diff --git a/spec/services/activitypub/fetch_remote_status_service_spec.rb b/spec/services/activitypub/fetch_remote_status_service_spec.rb index ceba5f2106..94574aa7f4 100644 --- a/spec/services/activitypub/fetch_remote_status_service_spec.rb +++ b/spec/services/activitypub/fetch_remote_status_service_spec.rb @@ -67,7 +67,7 @@ RSpec.describe ActivityPub::FetchRemoteStatusService, type: :service do expect(status).to_not be_nil expect(status.url).to eq "https://#{valid_domain}/watch?v=12345" - expect(strip_tags(status.text)).to eq "Nyan Cat 10 hours remix https://#{valid_domain}/watch?v=12345" + expect(strip_tags(status.text)).to eq "Nyan Cat 10 hours remixhttps://#{valid_domain}/watch?v=12345" end end @@ -100,7 +100,7 @@ RSpec.describe ActivityPub::FetchRemoteStatusService, type: :service do expect(status).to_not be_nil expect(status.url).to eq "https://#{valid_domain}/watch?v=12345" - expect(strip_tags(status.text)).to eq "Nyan Cat 10 hours remix https://#{valid_domain}/watch?v=12345" + expect(strip_tags(status.text)).to eq "Nyan Cat 10 hours remixhttps://#{valid_domain}/watch?v=12345" end end @@ -120,7 +120,7 @@ RSpec.describe ActivityPub::FetchRemoteStatusService, type: :service do expect(status).to_not be_nil expect(status.url).to eq "https://#{valid_domain}/@foo/1234" - expect(strip_tags(status.text)).to eq "Let's change the world https://#{valid_domain}/@foo/1234" + expect(strip_tags(status.text)).to eq "Let's change the worldhttps://#{valid_domain}/@foo/1234" end end diff --git a/spec/services/fan_out_on_write_service_spec.rb b/spec/services/fan_out_on_write_service_spec.rb index 538dc25922..4ce110e457 100644 --- a/spec/services/fan_out_on_write_service_spec.rb +++ b/spec/services/fan_out_on_write_service_spec.rb @@ -1,37 +1,112 @@ require 'rails_helper' RSpec.describe FanOutOnWriteService, type: :service do - let(:author) { Fabricate(:account, username: 'tom') } - let(:status) { Fabricate(:status, text: 'Hello @alice #test', account: author) } - let(:alice) { Fabricate(:user, account: Fabricate(:account, username: 'alice')).account } - let(:follower) { Fabricate(:account, username: 'bob') } + let(:last_active_at) { Time.now.utc } - subject { FanOutOnWriteService.new } + let!(:alice) { Fabricate(:user, current_sign_in_at: last_active_at, account: Fabricate(:account, username: 'alice')).account } + let!(:bob) { Fabricate(:user, current_sign_in_at: last_active_at, account: Fabricate(:account, username: 'bob')).account } + let!(:tom) { Fabricate(:user, current_sign_in_at: last_active_at, account: Fabricate(:account, username: 'tom')).account } + + subject { described_class.new } + + let(:status) { Fabricate(:status, account: alice, visibility: visibility, text: 'Hello @bob #hoge') } before do - alice - follower.follow!(author) + bob.follow!(alice) + tom.follow!(alice) ProcessMentionsService.new.call(status) ProcessHashtagsService.new.call(status) + allow(Redis.current).to receive(:publish) + subject.call(status) end - it 'delivers status to home timeline' do - expect(HomeFeed.new(author).get(10).map(&:id)).to include status.id + def home_feed_of(account) + HomeFeed.new(account).get(10).map(&:id) end - it 'delivers status to local followers' do - pending 'some sort of problem in test environment causes this to sometimes fail' - expect(HomeFeed.new(follower).get(10).map(&:id)).to include status.id + context 'when status is public' do + let(:visibility) { 'public' } + + it 'is added to the home feed of its author' do + expect(home_feed_of(alice)).to include status.id + end + + it 'is added to the home feed of a follower' do + expect(home_feed_of(bob)).to include status.id + expect(home_feed_of(tom)).to include status.id + end + + it 'is broadcast to the hashtag stream' do + expect(Redis.current).to have_received(:publish).with('timeline:hashtag:hoge', anything) + expect(Redis.current).to have_received(:publish).with('timeline:hashtag:hoge:local', anything) + end + + it 'is broadcast to the public stream' do + expect(Redis.current).to have_received(:publish).with('timeline:public', anything) + expect(Redis.current).to have_received(:publish).with('timeline:public:local', anything) + end end - it 'delivers status to hashtag' do - expect(TagFeed.new(Tag.find_by(name: 'test'), alice).get(20).map(&:id)).to include status.id + context 'when status is limited' do + let(:visibility) { 'limited' } + + it 'is added to the home feed of its author' do + expect(home_feed_of(alice)).to include status.id + end + + it 'is added to the home feed of the mentioned follower' do + expect(home_feed_of(bob)).to include status.id + end + + it 'is not added to the home feed of the other follower' do + expect(home_feed_of(tom)).to_not include status.id + end + + it 'is not broadcast publicly' do + expect(Redis.current).to_not have_received(:publish).with('timeline:hashtag:hoge', anything) + expect(Redis.current).to_not have_received(:publish).with('timeline:public', anything) + end end - it 'delivers status to public timeline' do - expect(PublicFeed.new(alice).get(20).map(&:id)).to include status.id + context 'when status is private' do + let(:visibility) { 'private' } + + it 'is added to the home feed of its author' do + expect(home_feed_of(alice)).to include status.id + end + + it 'is added to the home feed of a follower' do + expect(home_feed_of(bob)).to include status.id + expect(home_feed_of(tom)).to include status.id + end + + it 'is not broadcast publicly' do + expect(Redis.current).to_not have_received(:publish).with('timeline:hashtag:hoge', anything) + expect(Redis.current).to_not have_received(:publish).with('timeline:public', anything) + end + end + + context 'when status is direct' do + let(:visibility) { 'direct' } + + it 'is added to the home feed of its author' do + expect(home_feed_of(alice)).to include status.id + end + + it 'is added to the home feed of the mentioned follower' do + expect(home_feed_of(bob)).to include status.id + end + + it 'is not added to the home feed of the other follower' do + expect(home_feed_of(tom)).to_not include status.id + end + + it 'is not broadcast publicly' do + expect(Redis.current).to_not have_received(:publish).with('timeline:hashtag:hoge', anything) + expect(Redis.current).to_not have_received(:publish).with('timeline:public', anything) + end end end diff --git a/spec/services/process_mentions_service_spec.rb b/spec/services/process_mentions_service_spec.rb index d74e8dc624..89b265e9a0 100644 --- a/spec/services/process_mentions_service_spec.rb +++ b/spec/services/process_mentions_service_spec.rb @@ -9,75 +9,55 @@ RSpec.describe ProcessMentionsService, type: :service do context 'ActivityPub' do context do - let(:remote_user) { Fabricate(:account, username: 'remote_user', protocol: :activitypub, domain: 'example.com', inbox_url: 'http://example.com/inbox') } + let!(:remote_user) { Fabricate(:account, username: 'remote_user', protocol: :activitypub, domain: 'example.com', inbox_url: 'http://example.com/inbox') } before do - stub_request(:post, remote_user.inbox_url) subject.call(status) end it 'creates a mention' do expect(remote_user.mentions.where(status: status).count).to eq 1 end - - it 'sends activity to the inbox' do - expect(a_request(:post, remote_user.inbox_url)).to have_been_made.once - end end context 'with an IDN domain' do - let(:remote_user) { Fabricate(:account, username: 'sneak', protocol: :activitypub, domain: 'xn--hresiar-mxa.ch', inbox_url: 'http://example.com/inbox') } - let(:status) { Fabricate(:status, account: account, text: "Hello @sneak@hæresiar.ch") } + let!(:remote_user) { Fabricate(:account, username: 'sneak', protocol: :activitypub, domain: 'xn--hresiar-mxa.ch', inbox_url: 'http://example.com/inbox') } + let!(:status) { Fabricate(:status, account: account, text: "Hello @sneak@hæresiar.ch") } before do - stub_request(:post, remote_user.inbox_url) subject.call(status) end it 'creates a mention' do expect(remote_user.mentions.where(status: status).count).to eq 1 end - - it 'sends activity to the inbox' do - expect(a_request(:post, remote_user.inbox_url)).to have_been_made.once - end end context 'with an IDN TLD' do - let(:remote_user) { Fabricate(:account, username: 'foo', protocol: :activitypub, domain: 'xn--y9a3aq.xn--y9a3aq', inbox_url: 'http://example.com/inbox') } - let(:status) { Fabricate(:status, account: account, text: "Hello @foo@հայ.հայ") } + let!(:remote_user) { Fabricate(:account, username: 'foo', protocol: :activitypub, domain: 'xn--y9a3aq.xn--y9a3aq', inbox_url: 'http://example.com/inbox') } + let!(:status) { Fabricate(:status, account: account, text: "Hello @foo@հայ.հայ") } before do - stub_request(:post, remote_user.inbox_url) subject.call(status) end it 'creates a mention' do expect(remote_user.mentions.where(status: status).count).to eq 1 end - - it 'sends activity to the inbox' do - expect(a_request(:post, remote_user.inbox_url)).to have_been_made.once - end end end context 'Temporarily-unreachable ActivityPub user' do - let(:remote_user) { Fabricate(:account, username: 'remote_user', protocol: :activitypub, domain: 'example.com', inbox_url: 'http://example.com/inbox', last_webfingered_at: nil) } + let!(:remote_user) { Fabricate(:account, username: 'remote_user', protocol: :activitypub, domain: 'example.com', inbox_url: 'http://example.com/inbox', last_webfingered_at: nil) } before do stub_request(:get, "https://example.com/.well-known/host-meta").to_return(status: 404) stub_request(:get, "https://example.com/.well-known/webfinger?resource=acct:remote_user@example.com").to_return(status: 500) - stub_request(:post, remote_user.inbox_url) subject.call(status) end it 'creates a mention' do expect(remote_user.mentions.where(status: status).count).to eq 1 end - - it 'sends activity to the inbox' do - expect(a_request(:post, remote_user.inbox_url)).to have_been_made.once - end end end diff --git a/spec/workers/activitypub/distribution_worker_spec.rb b/spec/workers/activitypub/distribution_worker_spec.rb index 368ca025a0..c017b4da1a 100644 --- a/spec/workers/activitypub/distribution_worker_spec.rb +++ b/spec/workers/activitypub/distribution_worker_spec.rb @@ -35,13 +35,16 @@ describe ActivityPub::DistributionWorker do end context 'with direct status' do + let(:mentioned_account) { Fabricate(:account, protocol: :activitypub, inbox_url: 'https://foo.bar/inbox')} + before do status.update(visibility: :direct) + status.mentions.create!(account: mentioned_account) end - it 'does nothing' do + it 'delivers to mentioned accounts' do subject.perform(status.id) - expect(ActivityPub::DeliveryWorker).to_not have_received(:push_bulk) + expect(ActivityPub::DeliveryWorker).to have_received(:push_bulk).with(['https://foo.bar/inbox']) end end end diff --git a/spec/workers/feed_insert_worker_spec.rb b/spec/workers/feed_insert_worker_spec.rb index 3509f1f50e..fb34970fc3 100644 --- a/spec/workers/feed_insert_worker_spec.rb +++ b/spec/workers/feed_insert_worker_spec.rb @@ -45,7 +45,7 @@ describe FeedInsertWorker do result = subject.perform(status.id, follower.id) expect(result).to be_nil - expect(instance).to have_received(:push_to_home).with(follower, status) + expect(instance).to have_received(:push_to_home).with(follower, status, update: nil) end end end From d412a8d1f239aa93a92f420127cb3183a8bb6449 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Wed, 19 Jan 2022 22:50:01 +0100 Subject: [PATCH 02/16] Fix error when processing poll updates (#17333) Regression from #16697 --- app/services/activitypub/process_status_update_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/activitypub/process_status_update_service.rb b/app/services/activitypub/process_status_update_service.rb index e3e9b9d6ae..ed10a00635 100644 --- a/app/services/activitypub/process_status_update_service.rb +++ b/app/services/activitypub/process_status_update_service.rb @@ -78,7 +78,7 @@ class ActivityPub::ProcessStatusUpdateService < BaseService MediaAttachment.where(id: removed_media_attachments.map(&:id)).update_all(status_id: nil) MediaAttachment.where(id: added_media_attachments.map(&:id)).update_all(status_id: @status.id) - @media_attachments_changed = true if removed_media_attachments.positive? || added_media_attachments.positive? + @media_attachments_changed = true if removed_media_attachments.any? || added_media_attachments.any? end def update_poll! From 9eb775a9d1fd39bc2d255679bb12b2ff6f98080d Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Wed, 19 Jan 2022 23:05:59 +0100 Subject: [PATCH 03/16] Fix error when using raw distribution worker (#17334) Regression from #16697 --- app/workers/activitypub/raw_distribution_worker.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/workers/activitypub/raw_distribution_worker.rb b/app/workers/activitypub/raw_distribution_worker.rb index ac5eda4af7..8ecc17db9a 100644 --- a/app/workers/activitypub/raw_distribution_worker.rb +++ b/app/workers/activitypub/raw_distribution_worker.rb @@ -43,6 +43,6 @@ class ActivityPub::RawDistributionWorker end def options - nil + {} end end From d4654dc8929e4ded74194c95f8ee45daf6dc6516 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Wed, 19 Jan 2022 22:37:27 +0100 Subject: [PATCH 04/16] [Glitch] Add support for editing for published statuses Port front-end changes from 1060666c583670bb3b89ed5154e61038331e30c3 to glitch-soc Signed-off-by: Claire --- .../glitch/actions/importer/normalizer.js | 7 ++++--- .../flavours/glitch/actions/statuses.js | 3 +++ .../flavours/glitch/actions/streaming.js | 4 ++++ .../glitch/components/status_action_bar.js | 5 ++++- .../status/components/detailed_status.js | 20 +++++++++++++++---- .../glitch/styles/components/status.scss | 11 ++++++++++ 6 files changed, 42 insertions(+), 8 deletions(-) diff --git a/app/javascript/flavours/glitch/actions/importer/normalizer.js b/app/javascript/flavours/glitch/actions/importer/normalizer.js index 3995585f6c..bda15a9b08 100644 --- a/app/javascript/flavours/glitch/actions/importer/normalizer.js +++ b/app/javascript/flavours/glitch/actions/importer/normalizer.js @@ -54,9 +54,10 @@ export function normalizeStatus(status, normalOldStatus) { normalStatus.poll = status.poll.id; } - // Only calculate these values when status first encountered - // Otherwise keep the ones already in the reducer - if (normalOldStatus) { + // Only calculate these values when status first encountered and + // when the underlying values change. Otherwise keep the ones + // already in the reducer + if (normalOldStatus && normalOldStatus.get('content') === normalStatus.content && normalOldStatus.get('spoiler_text') === normalStatus.spoiler_text) { normalStatus.search_index = normalOldStatus.get('search_index'); normalStatus.contentHtml = normalOldStatus.get('contentHtml'); normalStatus.spoilerHtml = normalOldStatus.get('spoilerHtml'); diff --git a/app/javascript/flavours/glitch/actions/statuses.js b/app/javascript/flavours/glitch/actions/statuses.js index 4d2bda78be..7db357df1b 100644 --- a/app/javascript/flavours/glitch/actions/statuses.js +++ b/app/javascript/flavours/glitch/actions/statuses.js @@ -128,6 +128,9 @@ export function deleteStatusFail(id, error) { }; }; +export const updateStatus = status => dispatch => + dispatch(importFetchedStatus(status)); + export function fetchContext(id) { return (dispatch, getState) => { dispatch(fetchContextRequest(id)); diff --git a/app/javascript/flavours/glitch/actions/streaming.js b/app/javascript/flavours/glitch/actions/streaming.js index 35db5dcc92..223924534e 100644 --- a/app/javascript/flavours/glitch/actions/streaming.js +++ b/app/javascript/flavours/glitch/actions/streaming.js @@ -10,6 +10,7 @@ import { } from './timelines'; import { updateNotifications, expandNotifications } from './notifications'; import { updateConversations } from './conversations'; +import { updateStatus } from './statuses'; import { fetchAnnouncements, updateAnnouncements, @@ -75,6 +76,9 @@ export const connectTimelineStream = (timelineId, channelName, params = {}, opti case 'update': dispatch(updateTimeline(timelineId, JSON.parse(data.payload), options.accept)); break; + case 'status.update': + dispatch(updateStatus(JSON.parse(data.payload))); + break; case 'delete': dispatch(deleteFromTimelines(data.payload)); break; diff --git a/app/javascript/flavours/glitch/components/status_action_bar.js b/app/javascript/flavours/glitch/components/status_action_bar.js index 650b33b629..ae67c6116d 100644 --- a/app/javascript/flavours/glitch/components/status_action_bar.js +++ b/app/javascript/flavours/glitch/components/status_action_bar.js @@ -38,6 +38,7 @@ const messages = defineMessages({ admin_status: { id: 'status.admin_status', defaultMessage: 'Open this status in the moderation interface' }, copy: { id: 'status.copy', defaultMessage: 'Copy link to status' }, hide: { id: 'status.hide', defaultMessage: 'Hide toot' }, + edited: { id: 'status.edited', defaultMessage: 'Edited {date}' }, }); export default @injectIntl @@ -324,7 +325,9 @@ class StatusActionBar extends ImmutablePureComponent {
, ]} - + + {status.get('edited_at') && *} + ); } diff --git a/app/javascript/flavours/glitch/features/status/components/detailed_status.js b/app/javascript/flavours/glitch/features/status/components/detailed_status.js index a1f9814846..4b3a6aaaa3 100644 --- a/app/javascript/flavours/glitch/features/status/components/detailed_status.js +++ b/app/javascript/flavours/glitch/features/status/components/detailed_status.js @@ -7,7 +7,7 @@ import StatusContent from 'flavours/glitch/components/status_content'; import MediaGallery from 'flavours/glitch/components/media_gallery'; import AttachmentList from 'flavours/glitch/components/attachment_list'; import { Link } from 'react-router-dom'; -import { FormattedDate } from 'react-intl'; +import { injectIntl, FormattedDate, FormattedMessage } from 'react-intl'; import Card from './card'; import ImmutablePureComponent from 'react-immutable-pure-component'; import Video from 'flavours/glitch/features/video'; @@ -20,7 +20,8 @@ import Icon from 'flavours/glitch/components/icon'; import AnimatedNumber from 'flavours/glitch/components/animated_number'; import PictureInPicturePlaceholder from 'flavours/glitch/components/picture_in_picture_placeholder'; -export default class DetailedStatus extends ImmutablePureComponent { +export default @injectIntl +class DetailedStatus extends ImmutablePureComponent { static contextTypes = { router: PropTypes.object, @@ -40,6 +41,7 @@ export default class DetailedStatus extends ImmutablePureComponent { showMedia: PropTypes.bool, usingPiP: PropTypes.bool, onToggleMediaVisibility: PropTypes.func, + intl: PropTypes.object.isRequired, }; state = { @@ -111,7 +113,7 @@ export default class DetailedStatus extends ImmutablePureComponent { render () { const status = (this.props.status && this.props.status.get('reblog')) ? this.props.status.get('reblog') : this.props.status; - const { expanded, onToggleHidden, settings, usingPiP } = this.props; + const { expanded, onToggleHidden, settings, usingPiP, intl } = this.props; const outerStyle = { boxSizing: 'border-box' }; const { compact } = this.props; @@ -125,6 +127,7 @@ export default class DetailedStatus extends ImmutablePureComponent { let reblogLink = ''; let reblogIcon = 'retweet'; let favouriteLink = ''; + let edited = ''; if (this.props.measureHeight) { outerStyle.height = `${this.state.height}px`; @@ -258,6 +261,15 @@ export default class DetailedStatus extends ImmutablePureComponent { ); } + if (status.get('edited_at')) { + edited = ( + + · + + + ); + } + return (
@@ -283,7 +295,7 @@ export default class DetailedStatus extends ImmutablePureComponent {
- {visibilityLink}{applicationLink}{reblogLink} · {favouriteLink} + {edited}{visibilityLink}{applicationLink}{reblogLink} · {favouriteLink}
diff --git a/app/javascript/flavours/glitch/styles/components/status.scss b/app/javascript/flavours/glitch/styles/components/status.scss index e9d30544fc..013b1bd258 100644 --- a/app/javascript/flavours/glitch/styles/components/status.scss +++ b/app/javascript/flavours/glitch/styles/components/status.scss @@ -205,6 +205,17 @@ } } +.status__content__edited-label { + display: block; + cursor: default; + font-size: 15px; + line-height: 20px; + padding: 0; + padding-top: 8px; + color: $dark-text-color; + font-weight: 500; +} + .status__content__spoiler-link { display: inline-block; border-radius: 2px; From 4d0383d75ac606f8c9c2b8ecb0ea2dddf28213cb Mon Sep 17 00:00:00 2001 From: Claire Date: Thu, 20 Jan 2022 00:02:17 +0100 Subject: [PATCH 05/16] Add content-type to status source in glitch-soc --- app/serializers/rest/status_source_serializer.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/serializers/rest/status_source_serializer.rb b/app/serializers/rest/status_source_serializer.rb index cd3c740847..c03cbd20d3 100644 --- a/app/serializers/rest/status_source_serializer.rb +++ b/app/serializers/rest/status_source_serializer.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class REST::StatusSourceSerializer < ActiveModel::Serializer - attributes :id, :text, :spoiler_text + attributes :id, :text, :spoiler_text, :content_type def id object.id.to_s From 6eea3f8f9c41114002782d7a44993004bdd64832 Mon Sep 17 00:00:00 2001 From: Claire Date: Thu, 20 Jan 2022 13:37:31 +0100 Subject: [PATCH 06/16] Add post edited notice in admin and public UIs (#17335) * Add edited toot flag on public pages * Add toot edit flag to admin pages --- app/views/admin/reports/_status.html.haml | 3 +++ app/views/statuses/_detailed_status.html.haml | 5 +++++ app/views/statuses/_simple_status.html.haml | 3 +++ config/locales/en.yml | 1 + 4 files changed, 12 insertions(+) diff --git a/app/views/admin/reports/_status.html.haml b/app/views/admin/reports/_status.html.haml index 924b0e9c25..4e06d4bbf0 100644 --- a/app/views/admin/reports/_status.html.haml +++ b/app/views/admin/reports/_status.html.haml @@ -27,6 +27,9 @@ · = link_to ActivityPub::TagManager.instance.url_for(status), class: 'detailed-status__datetime', target: stream_link_target, rel: 'noopener noreferrer' do %time.formatted{ datetime: status.created_at.iso8601, title: l(status.created_at) }= l(status.created_at) + - if status.edited? + · + = t('statuses.edited_at', date: l(status.edited_at)) - if status.discarded? · %span.negative-hint= t('admin.statuses.deleted') diff --git a/app/views/statuses/_detailed_status.html.haml b/app/views/statuses/_detailed_status.html.haml index 6b3b813067..cd5ed52af4 100644 --- a/app/views/statuses/_detailed_status.html.haml +++ b/app/views/statuses/_detailed_status.html.haml @@ -37,10 +37,15 @@ .detailed-status__meta %data.dt-published{ value: status.created_at.to_time.iso8601 } + - if status.edited? + %data.dt-updated{ value: status.edited_at.to_time.iso8601 } = link_to ActivityPub::TagManager.instance.url_for(status), class: 'detailed-status__datetime u-url u-uid', target: stream_link_target, rel: 'noopener noreferrer' do %time.formatted{ datetime: status.created_at.iso8601, title: l(status.created_at) }= l(status.created_at) · + - if status.edited? + = t('statuses.edited_at', date: l(status.edited_at)) + · %span.detailed-status__visibility-icon = visibility_icon status · diff --git a/app/views/statuses/_simple_status.html.haml b/app/views/statuses/_simple_status.html.haml index 728e6b9b09..0139d2016c 100644 --- a/app/views/statuses/_simple_status.html.haml +++ b/app/views/statuses/_simple_status.html.haml @@ -7,6 +7,9 @@ %span.status__visibility-icon>< = visibility_icon status %time.time-ago{ datetime: status.created_at.iso8601, title: l(status.created_at) }= l(status.created_at) + - if status.edited? + %abbr{ title: t('statuses.edited_at', date: l(status.edited_at.to_date)) } + * %data.dt-published{ value: status.created_at.to_time.iso8601 } .p-author.h-card diff --git a/config/locales/en.yml b/config/locales/en.yml index 36ac896643..13a252a47b 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1309,6 +1309,7 @@ en: disallowed_hashtags: one: 'contained a disallowed hashtag: %{tags}' other: 'contained the disallowed hashtags: %{tags}' + edited_at: Edited %{date} errors: in_reply_not_found: The post you are trying to reply to does not appear to exist. language_detection: Automatically detect language From 1e8c885e5a5a3456785578440168983ce21266ab Mon Sep 17 00:00:00 2001 From: Claire Date: Thu, 20 Jan 2022 14:51:23 +0100 Subject: [PATCH 07/16] Change mastodon:webpush:generate_vapid_key task to not require functional env (#17338) Fixes #17297 --- lib/tasks/mastodon.rake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/tasks/mastodon.rake b/lib/tasks/mastodon.rake index d905a07dad..a89af67780 100644 --- a/lib/tasks/mastodon.rake +++ b/lib/tasks/mastodon.rake @@ -441,7 +441,7 @@ namespace :mastodon do namespace :webpush do desc 'Generate VAPID key' - task generate_vapid_key: :environment do + task :generate_vapid_key do vapid_key = Webpush.generate_key puts "VAPID_PRIVATE_KEY=#{vapid_key.private_key}" puts "VAPID_PUBLIC_KEY=#{vapid_key.public_key}" From 3a103cd317fd56aca27fca01e03647df44e3ffd2 Mon Sep 17 00:00:00 2001 From: Claire Date: Thu, 20 Jan 2022 20:56:21 +0100 Subject: [PATCH 08/16] Fix text being incorrectly pre-selected in composer textarea on /share (#17339) Fixes #17295 --- .../mastodon/features/compose/components/compose_form.js | 3 ++- .../features/compose/containers/compose_form_container.js | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/app/javascript/mastodon/features/compose/components/compose_form.js b/app/javascript/mastodon/features/compose/components/compose_form.js index ba2d20cc7e..d224222ace 100644 --- a/app/javascript/mastodon/features/compose/components/compose_form.js +++ b/app/javascript/mastodon/features/compose/components/compose_form.js @@ -60,6 +60,7 @@ class ComposeForm extends ImmutablePureComponent { onPickEmoji: PropTypes.func.isRequired, showSearch: PropTypes.bool, anyMedia: PropTypes.bool, + isInReply: PropTypes.bool, singleColumn: PropTypes.bool, }; @@ -149,7 +150,7 @@ class ComposeForm extends ImmutablePureComponent { if (this.props.focusDate !== prevProps.focusDate) { let selectionEnd, selectionStart; - if (this.props.preselectDate !== prevProps.preselectDate) { + if (this.props.preselectDate !== prevProps.preselectDate && this.props.isInReply) { selectionEnd = this.props.text.length; selectionStart = this.props.text.search(/\s/) + 1; } else if (typeof this.props.caretPosition === 'number') { diff --git a/app/javascript/mastodon/features/compose/containers/compose_form_container.js b/app/javascript/mastodon/features/compose/containers/compose_form_container.js index 37a0e8845b..c44850294d 100644 --- a/app/javascript/mastodon/features/compose/containers/compose_form_container.js +++ b/app/javascript/mastodon/features/compose/containers/compose_form_container.js @@ -25,6 +25,7 @@ const mapStateToProps = state => ({ isUploading: state.getIn(['compose', 'is_uploading']), showSearch: state.getIn(['search', 'submitted']) && !state.getIn(['search', 'hidden']), anyMedia: state.getIn(['compose', 'media_attachments']).size > 0, + isInReply: state.getIn(['compose', 'in_reply_to']) !== null, }); const mapDispatchToProps = (dispatch) => ({ From 96f0b33c8b994440199bccda14123d0569c6bcc5 Mon Sep 17 00:00:00 2001 From: Claire Date: Sun, 23 Jan 2022 13:53:58 +0100 Subject: [PATCH 09/16] Remove old duplicate index (#17245) Some Mastodon versions (v1.1 and v1.2) had a duplicate index in `db/schema.rb` without any migration script creating it. #2224 (included in v1.3) removed the duplicate index from the file but did not provide a migration script to remove it. This means that any instance that was installed from v1.1 or v1.2's source code has a duplicate index and a corresponding warning in PgHero. Instances set up using an earlier or later Mastodon version do not have this issue. This PR removes the duplicate index if it is present. --- .../20220105163928_remove_mentions_status_id_index.rb | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 db/migrate/20220105163928_remove_mentions_status_id_index.rb diff --git a/db/migrate/20220105163928_remove_mentions_status_id_index.rb b/db/migrate/20220105163928_remove_mentions_status_id_index.rb new file mode 100644 index 0000000000..56e9037192 --- /dev/null +++ b/db/migrate/20220105163928_remove_mentions_status_id_index.rb @@ -0,0 +1,9 @@ +class RemoveMentionsStatusIdIndex < ActiveRecord::Migration[6.1] + def up + remove_index :mentions, name: :mentions_status_id_index if index_exists?(:mentions, :status_id, name: :mentions_status_id_index) + end + + def down + # As this index should not exist and is a duplicate of another index, do not re-create it + end +end From 8a07ecd3773b1beae607bfe1edde62104654d64f Mon Sep 17 00:00:00 2001 From: Claire Date: Sun, 23 Jan 2022 15:46:30 +0100 Subject: [PATCH 10/16] Remove leftover database columns from Devise::Models::Rememberable (#17191) * Remove leftover database columns from Devise::Models::Rememberable * Update fix-duplication maintenance script * Improve errors/warnings in the fix-duplicates maintenance script --- app/models/user.rb | 12 ++++--- ...10_remove_index_users_on_remember_token.rb | 13 +++++++ ...18183123_remove_rememberable_from_users.rb | 8 +++++ db/schema.rb | 5 +-- lib/mastodon/maintenance_cli.rb | 34 ++++++++++++------- 5 files changed, 51 insertions(+), 21 deletions(-) create mode 100644 db/post_migrate/20220118183010_remove_index_users_on_remember_token.rb create mode 100644 db/post_migrate/20220118183123_remove_rememberable_from_users.rb diff --git a/app/models/user.rb b/app/models/user.rb index 49dcb81560..c2bc5b5901 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -10,7 +10,6 @@ # encrypted_password :string default(""), not null # reset_password_token :string # reset_password_sent_at :datetime -# remember_created_at :datetime # sign_in_count :integer default(0), not null # current_sign_in_at :datetime # last_sign_in_at :datetime @@ -32,7 +31,6 @@ # disabled :boolean default(FALSE), not null # moderator :boolean default(FALSE), not null # invite_id :bigint(8) -# remember_token :string # chosen_languages :string is an Array # created_by_application_id :bigint(8) # approved :boolean default(TRUE), not null @@ -44,6 +42,11 @@ # class User < ApplicationRecord + self.ignored_columns = %w( + remember_created_at + remember_token + ) + include Settings::Extend include UserRoles @@ -329,10 +332,9 @@ class User < ApplicationRecord end def reset_password! - # First, change password to something random, invalidate the remember-me token, - # and deactivate all sessions + # First, change password to something random and deactivate all sessions transaction do - update(remember_token: nil, remember_created_at: nil, password: SecureRandom.hex) + update(password: SecureRandom.hex) session_activations.destroy_all end diff --git a/db/post_migrate/20220118183010_remove_index_users_on_remember_token.rb b/db/post_migrate/20220118183010_remove_index_users_on_remember_token.rb new file mode 100644 index 0000000000..367d489de9 --- /dev/null +++ b/db/post_migrate/20220118183010_remove_index_users_on_remember_token.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class RemoveIndexUsersOnRememberToken < ActiveRecord::Migration[6.1] + disable_ddl_transaction! + + def up + remove_index :users, name: :index_users_on_remember_token + end + + def down + add_index :users, :remember_token, algorithm: :concurrently, unique: true, name: :index_users_on_remember_token + end +end diff --git a/db/post_migrate/20220118183123_remove_rememberable_from_users.rb b/db/post_migrate/20220118183123_remove_rememberable_from_users.rb new file mode 100644 index 0000000000..1e274c6e0b --- /dev/null +++ b/db/post_migrate/20220118183123_remove_rememberable_from_users.rb @@ -0,0 +1,8 @@ +class RemoveRememberableFromUsers < ActiveRecord::Migration[6.1] + def change + safety_assured do + remove_column :users, :remember_token, :string, null: true, default: nil + remove_column :users, :remember_created_at, :datetime, null: true, default: nil + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 4e0f76dcdb..fd4633d695 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2022_01_16_202951) do +ActiveRecord::Schema.define(version: 2022_01_18_183123) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -937,7 +937,6 @@ ActiveRecord::Schema.define(version: 2022_01_16_202951) do t.string "encrypted_password", default: "", null: false t.string "reset_password_token" t.datetime "reset_password_sent_at" - t.datetime "remember_created_at" t.integer "sign_in_count", default: 0, null: false t.datetime "current_sign_in_at" t.datetime "last_sign_in_at" @@ -959,7 +958,6 @@ ActiveRecord::Schema.define(version: 2022_01_16_202951) do t.boolean "disabled", default: false, null: false t.boolean "moderator", default: false, null: false t.bigint "invite_id" - t.string "remember_token" t.string "chosen_languages", array: true t.bigint "created_by_application_id" t.boolean "approved", default: true, null: false @@ -972,7 +970,6 @@ ActiveRecord::Schema.define(version: 2022_01_16_202951) do t.index ["confirmation_token"], name: "index_users_on_confirmation_token", unique: true t.index ["created_by_application_id"], name: "index_users_on_created_by_application_id" t.index ["email"], name: "index_users_on_email", unique: true - t.index ["remember_token"], name: "index_users_on_remember_token", unique: true t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true end diff --git a/lib/mastodon/maintenance_cli.rb b/lib/mastodon/maintenance_cli.rb index 47e2d78bb1..00861df774 100644 --- a/lib/mastodon/maintenance_cli.rb +++ b/lib/mastodon/maintenance_cli.rb @@ -14,7 +14,7 @@ module Mastodon end MIN_SUPPORTED_VERSION = 2019_10_01_213028 - MAX_SUPPORTED_VERSION = 2021_05_26_193025 + MAX_SUPPORTED_VERSION = 2022_01_18_183123 # Stubs to enjoy ActiveRecord queries while not depending on a particular # version of the code/database @@ -84,13 +84,14 @@ module Mastodon owned_classes = [ Status, StatusPin, MediaAttachment, Poll, Report, Tombstone, Favourite, - Follow, FollowRequest, Block, Mute, AccountIdentityProof, + Follow, FollowRequest, Block, Mute, AccountModerationNote, AccountPin, AccountStat, ListAccount, PollVote, Mention ] owned_classes << AccountDeletionRequest if ActiveRecord::Base.connection.table_exists?(:account_deletion_requests) owned_classes << AccountNote if ActiveRecord::Base.connection.table_exists?(:account_notes) owned_classes << FollowRecommendationSuppression if ActiveRecord::Base.connection.table_exists?(:follow_recommendation_suppressions) + owned_classes << AccountIdentityProof if ActiveRecord::Base.connection.table_exists?(:account_identity_proofs) owned_classes.each do |klass| klass.where(account_id: other_account.id).find_each do |record| @@ -139,17 +140,22 @@ module Mastodon @prompt = TTY::Prompt.new if ActiveRecord::Migrator.current_version < MIN_SUPPORTED_VERSION - @prompt.warn 'Your version of the database schema is too old and is not supported by this script.' - @prompt.warn 'Please update to at least Mastodon 3.0.0 before running this script.' + @prompt.error 'Your version of the database schema is too old and is not supported by this script.' + @prompt.error 'Please update to at least Mastodon 3.0.0 before running this script.' exit(1) elsif ActiveRecord::Migrator.current_version > MAX_SUPPORTED_VERSION @prompt.warn 'Your version of the database schema is more recent than this script, this may cause unexpected errors.' - exit(1) unless @prompt.yes?('Continue anyway?') + exit(1) unless @prompt.yes?('Continue anyway? (Yes/No)') + end + + if Sidekiq::ProcessSet.new.any? + @prompt.error 'It seems Sidekiq is running. All Mastodon processes need to be stopped when using this script.' + exit(1) end @prompt.warn 'This task will take a long time to run and is potentially destructive.' @prompt.warn 'Please make sure to stop Mastodon and have a backup.' - exit(1) unless @prompt.yes?('Continue?') + exit(1) unless @prompt.yes?('Continue? (Yes/No)') deduplicate_users! deduplicate_account_domain_blocks! @@ -236,12 +242,14 @@ module Mastodon end end - ActiveRecord::Base.connection.select_all("SELECT string_agg(id::text, ',') AS ids FROM users WHERE remember_token IS NOT NULL GROUP BY remember_token HAVING count(*) > 1").each do |row| - users = User.where(id: row['ids'].split(',')).sort_by(&:updated_at).reverse.drop(1) - @prompt.warn "Unsetting remember token for those accounts: #{users.map(&:account).map(&:acct).join(', ')}" + if ActiveRecord::Migrator.current_version < 20220118183010 + ActiveRecord::Base.connection.select_all("SELECT string_agg(id::text, ',') AS ids FROM users WHERE remember_token IS NOT NULL GROUP BY remember_token HAVING count(*) > 1").each do |row| + users = User.where(id: row['ids'].split(',')).sort_by(&:updated_at).reverse.drop(1) + @prompt.warn "Unsetting remember token for those accounts: #{users.map(&:account).map(&:acct).join(', ')}" - users.each do |user| - user.update!(remember_token: nil) + users.each do |user| + user.update!(remember_token: nil) + end end end @@ -257,7 +265,7 @@ module Mastodon @prompt.say 'Restoring users indexes…' ActiveRecord::Base.connection.add_index :users, ['confirmation_token'], name: 'index_users_on_confirmation_token', unique: true ActiveRecord::Base.connection.add_index :users, ['email'], name: 'index_users_on_email', unique: true - ActiveRecord::Base.connection.add_index :users, ['remember_token'], name: 'index_users_on_remember_token', unique: true + ActiveRecord::Base.connection.add_index :users, ['remember_token'], name: 'index_users_on_remember_token', unique: true if ActiveRecord::Migrator.current_version < 20220118183010 ActiveRecord::Base.connection.add_index :users, ['reset_password_token'], name: 'index_users_on_reset_password_token', unique: true end @@ -274,6 +282,8 @@ module Mastodon end def deduplicate_account_identity_proofs! + return unless ActiveRecord::Base.connection.table_exists?(:account_identity_proofs) + remove_index_if_exists!(:account_identity_proofs, 'index_account_proofs_on_account_and_provider_and_username') @prompt.say 'Removing duplicate account identity proofs…' From cfa583fa7111cfc16b9ce548f9d9b58963f154bd Mon Sep 17 00:00:00 2001 From: Claire Date: Sun, 23 Jan 2022 15:50:41 +0100 Subject: [PATCH 11/16] Remove support for OAUTH_REDIRECT_AT_SIGN_IN (#17287) Fixes #15959 Introduced in #6540, OAUTH_REDIRECT_AT_SIGN_IN allowed skipping the log-in form to instead redirect to the external OmniAuth login provider. However, it did not prevent the log-in form on /about introduced by #10232 from appearing, and completely broke with the introduction of #15228. As I restoring that previous log-in flow without introducing a security vulnerability may require extensive care and knowledge of how OmniAuth works, this commit removes support for OAUTH_REDIRECT_AT_SIGN_IN instead for the time being. --- .env.nanobox | 4 ---- app/controllers/auth/sessions_controller.rb | 16 ---------------- config/initializers/omniauth.rb | 1 - 3 files changed, 21 deletions(-) diff --git a/.env.nanobox b/.env.nanobox index ad941c947c..51dfdbd58f 100644 --- a/.env.nanobox +++ b/.env.nanobox @@ -202,10 +202,6 @@ SMTP_FROM_ADDRESS=notifications@${APP_NAME}.nanoapp.io # Name of the pam service used for checking if an user can register (pam "account" section is evaluated) (nil (disabled) by default) # PAM_CONTROLLED_SERVICE=rpam -# Global OAuth settings (optional) : -# If you have only one strategy, you may want to enable this -# OAUTH_REDIRECT_AT_SIGN_IN=true - # Optional CAS authentication (cf. omniauth-cas) : # CAS_ENABLED=true # CAS_URL=https://sso.myserver.com/ diff --git a/app/controllers/auth/sessions_controller.rb b/app/controllers/auth/sessions_controller.rb index 3337a43c45..4d2695bf57 100644 --- a/app/controllers/auth/sessions_controller.rb +++ b/app/controllers/auth/sessions_controller.rb @@ -13,14 +13,6 @@ class Auth::SessionsController < Devise::SessionsController before_action :set_instance_presenter, only: [:new] before_action :set_body_classes - def new - Devise.omniauth_configs.each do |provider, config| - return redirect_to(omniauth_authorize_path(resource_name, provider)) if config.strategy.redirect_at_sign_in - end - - super - end - def create super do |resource| # We only need to call this if this hasn't already been @@ -87,14 +79,6 @@ class Auth::SessionsController < Devise::SessionsController end end - def after_sign_out_path_for(_resource_or_scope) - Devise.omniauth_configs.each_value do |config| - return root_path if config.strategy.redirect_at_sign_in - end - - super - end - def require_no_authentication super diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb index 5039b4c1f0..19d59f1551 100644 --- a/config/initializers/omniauth.rb +++ b/config/initializers/omniauth.rb @@ -5,7 +5,6 @@ end Devise.setup do |config| # Devise omniauth strategies options = {} - options[:redirect_at_sign_in] = ENV['OAUTH_REDIRECT_AT_SIGN_IN'] == 'true' # CAS strategy if ENV['CAS_ENABLED'] == 'true' From bddd9ba36d9f4e86e2a4bbea77f967c143afa2cc Mon Sep 17 00:00:00 2001 From: Claire Date: Sun, 23 Jan 2022 15:52:58 +0100 Subject: [PATCH 12/16] Add OMNIAUTH_ONLY environment variable to enforce externa log-in (#17288) * Remove support for OAUTH_REDIRECT_AT_SIGN_IN Fixes #15959 Introduced in #6540, OAUTH_REDIRECT_AT_SIGN_IN allowed skipping the log-in form to instead redirect to the external OmniAuth login provider. However, it did not prevent the log-in form on /about introduced by #10232 from appearing, and completely broke with the introduction of #15228. As I restoring that previous log-in flow without introducing a security vulnerability may require extensive care and knowledge of how OmniAuth works, this commit removes support for OAUTH_REDIRECT_AT_SIGN_IN instead for the time being. * Add OMNIAUTH_ONLY environment variable to enforce external log-in only * Disable user registration when OMNIAUTH_ONLY is set to true * Replace log-in links When OMNIAUTH_ONLY is set with exactly one OmniAuth provider --- app/controllers/api/v1/accounts_controller.rb | 6 +++- .../auth/registrations_controller.rb | 6 +++- app/helpers/application_helper.rb | 26 ++++++++++++++++- app/views/about/_login.html.haml | 29 ++++++++++++------- app/views/auth/sessions/new.html.haml | 25 ++++++++-------- app/views/auth/shared/_links.html.haml | 2 +- app/views/layouts/public.html.haml | 2 +- app/views/statuses/_status.html.haml | 2 +- config/locales/en.yml | 1 + 9 files changed, 71 insertions(+), 28 deletions(-) diff --git a/app/controllers/api/v1/accounts_controller.rb b/app/controllers/api/v1/accounts_controller.rb index cbccd64f37..5c47158e02 100644 --- a/app/controllers/api/v1/accounts_controller.rb +++ b/app/controllers/api/v1/accounts_controller.rb @@ -83,10 +83,14 @@ class Api::V1::AccountsController < Api::BaseController end def check_enabled_registrations - forbidden if single_user_mode? || !allowed_registrations? + forbidden if single_user_mode? || omniauth_only? || !allowed_registrations? end def allowed_registrations? Setting.registrations_mode != 'none' end + + def omniauth_only? + ENV['OMNIAUTH_ONLY'] == 'true' + end end diff --git a/app/controllers/auth/registrations_controller.rb b/app/controllers/auth/registrations_controller.rb index 3c1730f25f..f37e906fde 100644 --- a/app/controllers/auth/registrations_controller.rb +++ b/app/controllers/auth/registrations_controller.rb @@ -81,13 +81,17 @@ class Auth::RegistrationsController < Devise::RegistrationsController end def check_enabled_registrations - redirect_to root_path if single_user_mode? || !allowed_registrations? + redirect_to root_path if single_user_mode? || omniauth_only? || !allowed_registrations? end def allowed_registrations? Setting.registrations_mode != 'none' || @invite&.valid_for_use? end + def omniauth_only? + ENV['OMNIAUTH_ONLY'] == 'true' + end + def invite_code if params[:user] params[:user][:invite_code] diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 34fc46615f..9e16de5b5b 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -50,13 +50,37 @@ module ApplicationHelper end def available_sign_up_path - if closed_registrations? + if closed_registrations? || omniauth_only? 'https://joinmastodon.org/#getting-started' else new_user_registration_path end end + def omniauth_only? + ENV['OMNIAUTH_ONLY'] == 'true' + end + + def link_to_login(name = nil, html_options = nil, &block) + target = new_user_session_path + + if omniauth_only? && Devise.mappings[:user].omniauthable? && User.omniauth_providers.size == 1 + target = omniauth_authorize_path(:user, User.omniauth_providers[0]) + html_options ||= {} + html_options[:method] = :post + end + + if block_given? + link_to(target, html_options, &block) + else + link_to(name, target, html_options) + end + end + + def provider_sign_in_link(provider) + link_to I18n.t("auth.providers.#{provider}", default: provider.to_s.chomp('_oauth2').capitalize), omniauth_authorize_path(:user, provider), class: "button button-#{provider}", method: :post + end + def open_deletion? Setting.open_deletion end diff --git a/app/views/about/_login.html.haml b/app/views/about/_login.html.haml index fa58f04d73..0f19e81643 100644 --- a/app/views/about/_login.html.haml +++ b/app/views/about/_login.html.haml @@ -1,13 +1,22 @@ -= simple_form_for(new_user, url: user_session_path, namespace: 'login') do |f| - .fields-group - - if use_seamless_external_login? - = f.input :email, placeholder: t('simple_form.labels.defaults.username_or_email'), input_html: { 'aria-label' => t('simple_form.labels.defaults.username_or_email') }, hint: false - - else - = f.input :email, placeholder: t('simple_form.labels.defaults.email'), input_html: { 'aria-label' => t('simple_form.labels.defaults.email') }, hint: false +- unless omniauth_only? + = simple_form_for(new_user, url: user_session_path, namespace: 'login') do |f| + .fields-group + - if use_seamless_external_login? + = f.input :email, placeholder: t('simple_form.labels.defaults.username_or_email'), input_html: { 'aria-label' => t('simple_form.labels.defaults.username_or_email') }, hint: false + - else + = f.input :email, placeholder: t('simple_form.labels.defaults.email'), input_html: { 'aria-label' => t('simple_form.labels.defaults.email') }, hint: false - = f.input :password, placeholder: t('simple_form.labels.defaults.password'), input_html: { 'aria-label' => t('simple_form.labels.defaults.password') }, hint: false + = f.input :password, placeholder: t('simple_form.labels.defaults.password'), input_html: { 'aria-label' => t('simple_form.labels.defaults.password') }, hint: false - .actions - = f.button :button, t('auth.login'), type: :submit, class: 'button button-primary' + .actions + = f.button :button, t('auth.login'), type: :submit, class: 'button button-primary' - %p.hint.subtle-hint= link_to t('auth.trouble_logging_in'), new_user_password_path + %p.hint.subtle-hint= link_to t('auth.trouble_logging_in'), new_user_password_path + +- if Devise.mappings[:user].omniauthable? and User.omniauth_providers.any? + .simple_form.alternative-login + %h4= omniauth_only? ? t('auth.log_in_with') : t('auth.or_log_in_with') + + .actions + - User.omniauth_providers.each do |provider| + = provider_sign_in_link(provider) diff --git a/app/views/auth/sessions/new.html.haml b/app/views/auth/sessions/new.html.haml index 9713bdaebf..a4323d1d9a 100644 --- a/app/views/auth/sessions/new.html.haml +++ b/app/views/auth/sessions/new.html.haml @@ -4,24 +4,25 @@ - content_for :header_tags do = render partial: 'shared/og' -= simple_form_for(resource, as: resource_name, url: session_path(resource_name)) do |f| - .fields-group - - if use_seamless_external_login? - = f.input :email, autofocus: true, wrapper: :with_label, label: t('simple_form.labels.defaults.username_or_email'), input_html: { 'aria-label' => t('simple_form.labels.defaults.username_or_email') }, hint: false - - else - = f.input :email, autofocus: true, wrapper: :with_label, label: t('simple_form.labels.defaults.email'), input_html: { 'aria-label' => t('simple_form.labels.defaults.email') }, hint: false - .fields-group - = f.input :password, wrapper: :with_label, label: t('simple_form.labels.defaults.password'), input_html: { 'aria-label' => t('simple_form.labels.defaults.password'), :autocomplete => 'off' }, hint: false +- unless omniauth_only? + = simple_form_for(resource, as: resource_name, url: session_path(resource_name)) do |f| + .fields-group + - if use_seamless_external_login? + = f.input :email, autofocus: true, wrapper: :with_label, label: t('simple_form.labels.defaults.username_or_email'), input_html: { 'aria-label' => t('simple_form.labels.defaults.username_or_email') }, hint: false + - else + = f.input :email, autofocus: true, wrapper: :with_label, label: t('simple_form.labels.defaults.email'), input_html: { 'aria-label' => t('simple_form.labels.defaults.email') }, hint: false + .fields-group + = f.input :password, wrapper: :with_label, label: t('simple_form.labels.defaults.password'), input_html: { 'aria-label' => t('simple_form.labels.defaults.password'), :autocomplete => 'off' }, hint: false - .actions - = f.button :button, t('auth.login'), type: :submit + .actions + = f.button :button, t('auth.login'), type: :submit - if devise_mapping.omniauthable? and resource_class.omniauth_providers.any? .simple_form.alternative-login - %h4= t('auth.or_log_in_with') + %h4= omniauth_only? ? t('auth.log_in_with') : t('auth.or_log_in_with') .actions - resource_class.omniauth_providers.each do |provider| - = link_to t("auth.providers.#{provider}", default: provider.to_s.chomp("_oauth2").capitalize), omniauth_authorize_path(resource_name, provider), class: "button button-#{provider}", method: :post + = provider_sign_in_link(provider) .form-footer= render 'auth/shared/links' diff --git a/app/views/auth/shared/_links.html.haml b/app/views/auth/shared/_links.html.haml index 66ed5b93f3..f078e2f7ec 100644 --- a/app/views/auth/shared/_links.html.haml +++ b/app/views/auth/shared/_links.html.haml @@ -3,7 +3,7 @@ %li= link_to t('settings.account_settings'), edit_user_registration_path - else - if controller_name != 'sessions' - %li= link_to t('auth.login'), new_user_session_path + %li= link_to_login t('auth.login') - if controller_name != 'registrations' %li= link_to t('auth.register'), available_sign_up_path diff --git a/app/views/layouts/public.html.haml b/app/views/layouts/public.html.haml index bdb8a3a8ec..069931cfd6 100644 --- a/app/views/layouts/public.html.haml +++ b/app/views/layouts/public.html.haml @@ -22,7 +22,7 @@ - if user_signed_in? = link_to t('settings.back'), root_url, class: 'nav-link nav-button webapp-btn' - else - = link_to t('auth.login'), new_user_session_path, class: 'webapp-btn nav-link nav-button' + = link_to_login t('auth.login'), class: 'webapp-btn nav-link nav-button' = link_to t('auth.register'), available_sign_up_path, class: 'webapp-btn nav-link nav-button' .container= yield diff --git a/app/views/statuses/_status.html.haml b/app/views/statuses/_status.html.haml index 9f3197d0db..3b7152753a 100644 --- a/app/views/statuses/_status.html.haml +++ b/app/views/statuses/_status.html.haml @@ -56,6 +56,6 @@ - if include_threads && !embedded_view? && !user_signed_in? .entry{ class: entry_classes } - = link_to new_user_session_path, class: 'load-more load-gap' do + = link_to_login class: 'load-more load-gap' do = fa_icon 'comments' = t('statuses.sign_in_to_participate') diff --git a/config/locales/en.yml b/config/locales/en.yml index 13a252a47b..85aa87c7a6 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -844,6 +844,7 @@ en: invalid_reset_password_token: Password reset token is invalid or expired. Please request a new one. link_to_otp: Enter a two-factor code from your phone or a recovery code link_to_webauth: Use your security key device + log_in_with: Log in with login: Log in logout: Logout migrate_account: Move to a different account From a63495230a3a28e022504f36356cd75b17b635ba Mon Sep 17 00:00:00 2001 From: Claire Date: Sun, 23 Jan 2022 16:01:25 +0100 Subject: [PATCH 13/16] Change `percent` to `rate` in retention metrics API (#16910) --- app/javascript/mastodon/components/admin/Retention.js | 6 +++--- app/lib/admin/metrics/retention.rb | 4 ++-- app/serializers/rest/admin/cohort_serializer.rb | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/javascript/mastodon/components/admin/Retention.js b/app/javascript/mastodon/components/admin/Retention.js index 3a7aaed9d8..47c9e71513 100644 --- a/app/javascript/mastodon/components/admin/Retention.js +++ b/app/javascript/mastodon/components/admin/Retention.js @@ -88,7 +88,7 @@ export default class Retention extends React.PureComponent { {data[0].data.slice(1).map((retention, i) => { - const average = data.reduce((sum, cohort, k) => cohort.data[i + 1] ? sum + (cohort.data[i + 1].percent - sum)/(k + 1) : sum, 0); + const average = data.reduce((sum, cohort, k) => cohort.data[i + 1] ? sum + (cohort.data[i + 1].rate - sum)/(k + 1) : sum, 0); return ( @@ -118,8 +118,8 @@ export default class Retention extends React.PureComponent { {cohort.data.slice(1).map(retention => ( -
- +
+
))} diff --git a/app/lib/admin/metrics/retention.rb b/app/lib/admin/metrics/retention.rb index 6b9dfde499..0179a6e282 100644 --- a/app/lib/admin/metrics/retention.rb +++ b/app/lib/admin/metrics/retention.rb @@ -6,7 +6,7 @@ class Admin::Metrics::Retention end class CohortData < ActiveModelSerializers::Model - attributes :date, :percent, :value + attributes :date, :rate, :value end def initialize(start_at, end_at, frequency) @@ -59,7 +59,7 @@ class Admin::Metrics::Retention current_cohort.data << CohortData.new( date: row['retention_period'], - percent: rate.to_f, + rate: rate.to_f, value: value.to_s ) end diff --git a/app/serializers/rest/admin/cohort_serializer.rb b/app/serializers/rest/admin/cohort_serializer.rb index 56b35c6991..f681736165 100644 --- a/app/serializers/rest/admin/cohort_serializer.rb +++ b/app/serializers/rest/admin/cohort_serializer.rb @@ -4,7 +4,7 @@ class REST::Admin::CohortSerializer < ActiveModel::Serializer attributes :period, :frequency class CohortDataSerializer < ActiveModel::Serializer - attributes :date, :percent, :value + attributes :date, :rate, :value def date object.date.iso8601 From 0a120d86d28e3f2e20455f56c1656f5d5f2f4af6 Mon Sep 17 00:00:00 2001 From: Claire Date: Sun, 23 Jan 2022 18:10:10 +0100 Subject: [PATCH 14/16] Fix error-prone SQL queries (#15828) * Fix error-prone SQL queries in Account search While this code seems to not present an actual vulnerability, one could easily be introduced by mistake due to how the query is built. This PR parameterises the `to_tsquery` input to make the query more robust. * Harden code for Status#tagged_with_all and Status#tagged_with_none Those two scopes aren't used in a way that could be vulnerable to an SQL injection, but keeping them unchanged might be a hazard. * Remove unneeded spaces surrounding tsquery term * Please CodeClimate * Move advanced_search_for SQL template to its own function This avoids one level of indentation while making clearer that the SQL template isn't build from all the dynamic parameters of advanced_search_for. * Add tests covering tagged_with, tagged_with_all and tagged_with_none * Rewrite tagged_with_none to avoid multiple joins and make it more robust * Remove obsolete brakeman warnings * Revert "Remove unneeded spaces surrounding tsquery term" The two queries are not strictly equivalent. This reverts commit 86f16c537e06c6ba4a8b250f25dcce9f049023ff. --- app/models/account.rb | 113 +++++++++++++++++++------------------ app/models/status.rb | 7 +-- config/brakeman.ignore | 80 -------------------------- spec/models/status_spec.rb | 81 ++++++++++++++++++++++++++ 4 files changed, 142 insertions(+), 139 deletions(-) diff --git a/app/models/account.rb b/app/models/account.rb index c459125c7b..771cc0b1ba 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -427,6 +427,9 @@ class Account < ApplicationRecord end class << self + DISALLOWED_TSQUERY_CHARACTERS = /['?\\:‘’]/.freeze + TEXTSEARCH = "(setweight(to_tsvector('simple', accounts.display_name), 'A') || setweight(to_tsvector('simple', accounts.username), 'B') || setweight(to_tsvector('simple', coalesce(accounts.domain, '')), 'C'))" + def readonly_attributes super - %w(statuses_count following_count followers_count) end @@ -437,70 +440,29 @@ class Account < ApplicationRecord end def search_for(terms, limit = 10, offset = 0) - textsearch, query = generate_query_for_search(terms) + tsquery = generate_query_for_search(terms) sql = <<-SQL.squish SELECT accounts.*, - ts_rank_cd(#{textsearch}, #{query}, 32) AS rank + ts_rank_cd(#{TEXTSEARCH}, to_tsquery('simple', :tsquery), 32) AS rank FROM accounts - WHERE #{query} @@ #{textsearch} + WHERE to_tsquery('simple', :tsquery) @@ #{TEXTSEARCH} AND accounts.suspended_at IS NULL AND accounts.moved_to_account_id IS NULL ORDER BY rank DESC - LIMIT ? OFFSET ? + LIMIT :limit OFFSET :offset SQL - records = find_by_sql([sql, limit, offset]) + records = find_by_sql([sql, limit: limit, offset: offset, tsquery: tsquery]) ActiveRecord::Associations::Preloader.new.preload(records, :account_stat) records end def advanced_search_for(terms, account, limit = 10, following = false, offset = 0) - textsearch, query = generate_query_for_search(terms) - - if following - sql = <<-SQL.squish - WITH first_degree AS ( - SELECT target_account_id - FROM follows - WHERE account_id = ? - UNION ALL - SELECT ? - ) - SELECT - accounts.*, - (count(f.id) + 1) * ts_rank_cd(#{textsearch}, #{query}, 32) AS rank - FROM accounts - LEFT OUTER JOIN follows AS f ON (accounts.id = f.account_id AND f.target_account_id = ?) - WHERE accounts.id IN (SELECT * FROM first_degree) - AND #{query} @@ #{textsearch} - AND accounts.suspended_at IS NULL - AND accounts.moved_to_account_id IS NULL - GROUP BY accounts.id - ORDER BY rank DESC - LIMIT ? OFFSET ? - SQL - - records = find_by_sql([sql, account.id, account.id, account.id, limit, offset]) - else - sql = <<-SQL.squish - SELECT - accounts.*, - (count(f.id) + 1) * ts_rank_cd(#{textsearch}, #{query}, 32) AS rank - FROM accounts - LEFT OUTER JOIN follows AS f ON (accounts.id = f.account_id AND f.target_account_id = ?) OR (accounts.id = f.target_account_id AND f.account_id = ?) - WHERE #{query} @@ #{textsearch} - AND accounts.suspended_at IS NULL - AND accounts.moved_to_account_id IS NULL - GROUP BY accounts.id - ORDER BY rank DESC - LIMIT ? OFFSET ? - SQL - - records = find_by_sql([sql, account.id, account.id, limit, offset]) - end - + tsquery = generate_query_for_search(terms) + sql = advanced_search_for_sql_template(following) + records = find_by_sql([sql, id: account.id, limit: limit, offset: offset, tsquery: tsquery]) ActiveRecord::Associations::Preloader.new.preload(records, :account_stat) records end @@ -522,12 +484,55 @@ class Account < ApplicationRecord private - def generate_query_for_search(terms) - terms = Arel.sql(connection.quote(terms.gsub(/['?\\:]/, ' '))) - textsearch = "(setweight(to_tsvector('simple', accounts.display_name), 'A') || setweight(to_tsvector('simple', accounts.username), 'B') || setweight(to_tsvector('simple', coalesce(accounts.domain, '')), 'C'))" - query = "to_tsquery('simple', ''' ' || #{terms} || ' ''' || ':*')" + def generate_query_for_search(unsanitized_terms) + terms = unsanitized_terms.gsub(DISALLOWED_TSQUERY_CHARACTERS, ' ') - [textsearch, query] + # The final ":*" is for prefix search. + # The trailing space does not seem to fit any purpose, but `to_tsquery` + # behaves differently with and without a leading space if the terms start + # with `./`, `../`, or `.. `. I don't understand why, so, in doubt, keep + # the same query. + "' #{terms} ':*" + end + + def advanced_search_for_sql_template(following) + if following + <<-SQL.squish + WITH first_degree AS ( + SELECT target_account_id + FROM follows + WHERE account_id = :id + UNION ALL + SELECT :id + ) + SELECT + accounts.*, + (count(f.id) + 1) * ts_rank_cd(#{TEXTSEARCH}, to_tsquery('simple', :tsquery), 32) AS rank + FROM accounts + LEFT OUTER JOIN follows AS f ON (accounts.id = f.account_id AND f.target_account_id = :id) + WHERE accounts.id IN (SELECT * FROM first_degree) + AND to_tsquery('simple', :tsquery) @@ #{TEXTSEARCH} + AND accounts.suspended_at IS NULL + AND accounts.moved_to_account_id IS NULL + GROUP BY accounts.id + ORDER BY rank DESC + LIMIT :limit OFFSET :offset + SQL + else + <<-SQL.squish + SELECT + accounts.*, + (count(f.id) + 1) * ts_rank_cd(#{TEXTSEARCH}, to_tsquery('simple', :tsquery), 32) AS rank + FROM accounts + LEFT OUTER JOIN follows AS f ON (accounts.id = f.account_id AND f.target_account_id = :id) OR (accounts.id = f.target_account_id AND f.account_id = :id) + WHERE to_tsquery('simple', :tsquery) @@ #{TEXTSEARCH} + AND accounts.suspended_at IS NULL + AND accounts.moved_to_account_id IS NULL + GROUP BY accounts.id + ORDER BY rank DESC + LIMIT :limit OFFSET :offset + SQL + end end end diff --git a/app/models/status.rb b/app/models/status.rb index 3358d6891b..47671c0f52 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -99,15 +99,12 @@ class Status < ApplicationRecord scope :not_excluded_by_account, ->(account) { where.not(account_id: account.excluded_from_timeline_account_ids) } scope :not_domain_blocked_by_account, ->(account) { account.excluded_from_timeline_domains.blank? ? left_outer_joins(:account) : left_outer_joins(:account).where('accounts.domain IS NULL OR accounts.domain NOT IN (?)', account.excluded_from_timeline_domains) } scope :tagged_with_all, ->(tag_ids) { - Array(tag_ids).reduce(self) do |result, id| + Array(tag_ids).map(&:to_i).reduce(self) do |result, id| result.joins("INNER JOIN statuses_tags t#{id} ON t#{id}.status_id = statuses.id AND t#{id}.tag_id = #{id}") end } scope :tagged_with_none, ->(tag_ids) { - Array(tag_ids).reduce(self) do |result, id| - result.joins("LEFT OUTER JOIN statuses_tags t#{id} ON t#{id}.status_id = statuses.id AND t#{id}.tag_id = #{id}") - .where("t#{id}.tag_id IS NULL") - end + where('NOT EXISTS (SELECT * FROM statuses_tags forbidden WHERE forbidden.status_id = statuses.id AND forbidden.tag_id IN (?))', tag_ids) } cache_associated :application, diff --git a/config/brakeman.ignore b/config/brakeman.ignore index c032e5412a..4245b71924 100644 --- a/config/brakeman.ignore +++ b/config/brakeman.ignore @@ -60,46 +60,6 @@ "confidence": "High", "note": "" }, - { - "warning_type": "SQL Injection", - "warning_code": 0, - "fingerprint": "6e4051854bb62e2ddbc671f82d6c2328892e1134b8b28105ecba9b0122540714", - "check_name": "SQL", - "message": "Possible SQL injection", - "file": "app/models/account.rb", - "line": 484, - "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", - "code": "find_by_sql([\" WITH first_degree AS (\\n SELECT target_account_id\\n FROM follows\\n WHERE account_id = ?\\n UNION ALL\\n SELECT ?\\n )\\n SELECT\\n accounts.*,\\n (count(f.id) + 1) * ts_rank_cd(#{textsearch}, #{query}, 32) AS rank\\n FROM accounts\\n LEFT OUTER JOIN follows AS f ON (accounts.id = f.account_id AND f.target_account_id = ?)\\n WHERE accounts.id IN (SELECT * FROM first_degree)\\n AND #{query} @@ #{textsearch}\\n AND accounts.suspended_at IS NULL\\n AND accounts.moved_to_account_id IS NULL\\n GROUP BY accounts.id\\n ORDER BY rank DESC\\n LIMIT ? OFFSET ?\\n\".squish, account.id, account.id, account.id, limit, offset])", - "render_path": null, - "location": { - "type": "method", - "class": "Account", - "method": "advanced_search_for" - }, - "user_input": "textsearch", - "confidence": "Medium", - "note": "" - }, - { - "warning_type": "SQL Injection", - "warning_code": 0, - "fingerprint": "6f075c1484908e3ec9bed21ab7cf3c7866be8da3881485d1c82e13093aefcbd7", - "check_name": "SQL", - "message": "Possible SQL injection", - "file": "app/models/status.rb", - "line": 105, - "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", - "code": "result.joins(\"LEFT OUTER JOIN statuses_tags t#{id} ON t#{id}.status_id = statuses.id AND t#{id}.tag_id = #{id}\")", - "render_path": null, - "location": { - "type": "method", - "class": "Status", - "method": null - }, - "user_input": "id", - "confidence": "Weak", - "note": "" - }, { "warning_type": "SQL Injection", "warning_code": 0, @@ -180,26 +140,6 @@ "confidence": "Medium", "note": "" }, - { - "warning_type": "SQL Injection", - "warning_code": 0, - "fingerprint": "9251d682c4e2840e1b2fea91e7d758efe2097ecb7f6255c065e3750d25eb178c", - "check_name": "SQL", - "message": "Possible SQL injection", - "file": "app/models/account.rb", - "line": 453, - "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", - "code": "find_by_sql([\" SELECT\\n accounts.*,\\n ts_rank_cd(#{textsearch}, #{query}, 32) AS rank\\n FROM accounts\\n WHERE #{query} @@ #{textsearch}\\n AND accounts.suspended_at IS NULL\\n AND accounts.moved_to_account_id IS NULL\\n ORDER BY rank DESC\\n LIMIT ? OFFSET ?\\n\".squish, limit, offset])", - "render_path": null, - "location": { - "type": "method", - "class": "Account", - "method": "search_for" - }, - "user_input": "textsearch", - "confidence": "Medium", - "note": "" - }, { "warning_type": "Redirect", "warning_code": 18, @@ -270,26 +210,6 @@ "confidence": "Weak", "note": "" }, - { - "warning_type": "SQL Injection", - "warning_code": 0, - "fingerprint": "e21d8fee7a5805761679877ca35ed1029c64c45ef3b4012a30262623e1ba8bb9", - "check_name": "SQL", - "message": "Possible SQL injection", - "file": "app/models/account.rb", - "line": 500, - "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", - "code": "find_by_sql([\" SELECT\\n accounts.*,\\n (count(f.id) + 1) * ts_rank_cd(#{textsearch}, #{query}, 32) AS rank\\n FROM accounts\\n LEFT OUTER JOIN follows AS f ON (accounts.id = f.account_id AND f.target_account_id = ?) OR (accounts.id = f.target_account_id AND f.account_id = ?)\\n WHERE #{query} @@ #{textsearch}\\n AND accounts.suspended_at IS NULL\\n AND accounts.moved_to_account_id IS NULL\\n GROUP BY accounts.id\\n ORDER BY rank DESC\\n LIMIT ? OFFSET ?\\n\".squish, account.id, account.id, limit, offset])", - "render_path": null, - "location": { - "type": "method", - "class": "Account", - "method": "advanced_search_for" - }, - "user_input": "textsearch", - "confidence": "Medium", - "note": "" - }, { "warning_type": "Mass Assignment", "warning_code": 105, diff --git a/spec/models/status_spec.rb b/spec/models/status_spec.rb index 20fb894e77..653575778b 100644 --- a/spec/models/status_spec.rb +++ b/spec/models/status_spec.rb @@ -267,6 +267,87 @@ RSpec.describe Status, type: :model do end end + describe '.tagged_with' do + let(:tag1) { Fabricate(:tag) } + let(:tag2) { Fabricate(:tag) } + let(:tag3) { Fabricate(:tag) } + let!(:status1) { Fabricate(:status, tags: [tag1]) } + let!(:status2) { Fabricate(:status, tags: [tag2]) } + let!(:status3) { Fabricate(:status, tags: [tag3]) } + let!(:status4) { Fabricate(:status, tags: []) } + let!(:status5) { Fabricate(:status, tags: [tag1, tag2, tag3]) } + + context 'when given one tag' do + it 'returns the expected statuses' do + expect(Status.tagged_with([tag1.id]).reorder(:id).pluck(:id).uniq).to eq [status1.id, status5.id] + expect(Status.tagged_with([tag2.id]).reorder(:id).pluck(:id).uniq).to eq [status2.id, status5.id] + expect(Status.tagged_with([tag3.id]).reorder(:id).pluck(:id).uniq).to eq [status3.id, status5.id] + end + end + + context 'when given multiple tags' do + it 'returns the expected statuses' do + expect(Status.tagged_with([tag1.id, tag2.id]).reorder(:id).pluck(:id).uniq).to eq [status1.id, status2.id, status5.id] + expect(Status.tagged_with([tag1.id, tag3.id]).reorder(:id).pluck(:id).uniq).to eq [status1.id, status3.id, status5.id] + expect(Status.tagged_with([tag2.id, tag3.id]).reorder(:id).pluck(:id).uniq).to eq [status2.id, status3.id, status5.id] + end + end + end + + describe '.tagged_with_all' do + let(:tag1) { Fabricate(:tag) } + let(:tag2) { Fabricate(:tag) } + let(:tag3) { Fabricate(:tag) } + let!(:status1) { Fabricate(:status, tags: [tag1]) } + let!(:status2) { Fabricate(:status, tags: [tag2]) } + let!(:status3) { Fabricate(:status, tags: [tag3]) } + let!(:status4) { Fabricate(:status, tags: []) } + let!(:status5) { Fabricate(:status, tags: [tag1, tag2]) } + + context 'when given one tag' do + it 'returns the expected statuses' do + expect(Status.tagged_with_all([tag1.id]).reorder(:id).pluck(:id).uniq).to eq [status1.id, status5.id] + expect(Status.tagged_with_all([tag2.id]).reorder(:id).pluck(:id).uniq).to eq [status2.id, status5.id] + expect(Status.tagged_with_all([tag3.id]).reorder(:id).pluck(:id).uniq).to eq [status3.id] + end + end + + context 'when given multiple tags' do + it 'returns the expected statuses' do + expect(Status.tagged_with_all([tag1.id, tag2.id]).reorder(:id).pluck(:id).uniq).to eq [status5.id] + expect(Status.tagged_with_all([tag1.id, tag3.id]).reorder(:id).pluck(:id).uniq).to eq [] + expect(Status.tagged_with_all([tag2.id, tag3.id]).reorder(:id).pluck(:id).uniq).to eq [] + end + end + end + + describe '.tagged_with_none' do + let(:tag1) { Fabricate(:tag) } + let(:tag2) { Fabricate(:tag) } + let(:tag3) { Fabricate(:tag) } + let!(:status1) { Fabricate(:status, tags: [tag1]) } + let!(:status2) { Fabricate(:status, tags: [tag2]) } + let!(:status3) { Fabricate(:status, tags: [tag3]) } + let!(:status4) { Fabricate(:status, tags: []) } + let!(:status5) { Fabricate(:status, tags: [tag1, tag2, tag3]) } + + context 'when given one tag' do + it 'returns the expected statuses' do + expect(Status.tagged_with_none([tag1.id]).reorder(:id).pluck(:id).uniq).to eq [status2.id, status3.id, status4.id] + expect(Status.tagged_with_none([tag2.id]).reorder(:id).pluck(:id).uniq).to eq [status1.id, status3.id, status4.id] + expect(Status.tagged_with_none([tag3.id]).reorder(:id).pluck(:id).uniq).to eq [status1.id, status2.id, status4.id] + end + end + + context 'when given multiple tags' do + it 'returns the expected statuses' do + expect(Status.tagged_with_none([tag1.id, tag2.id]).reorder(:id).pluck(:id).uniq).to eq [status3.id, status4.id] + expect(Status.tagged_with_none([tag1.id, tag3.id]).reorder(:id).pluck(:id).uniq).to eq [status2.id, status4.id] + expect(Status.tagged_with_none([tag2.id, tag3.id]).reorder(:id).pluck(:id).uniq).to eq [status1.id, status4.id] + end + end + end + describe '.permitted_for' do subject { described_class.permitted_for(target_account, account).pluck(:visibility) } From 4dd4fc2e5e6f835c6820a2b9f677090405a67976 Mon Sep 17 00:00:00 2001 From: Claire Date: Sun, 23 Jan 2022 18:24:34 +0100 Subject: [PATCH 15/16] [Glitch] Fix text being incorrectly pre-selected in composer textarea on /share Port 3a103cd317fd56aca27fca01e03647df44e3ffd2 to glitch-soc Signed-off-by: Claire --- .../glitch/features/compose/components/compose_form.js | 3 ++- .../features/compose/containers/compose_form_container.js | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/app/javascript/flavours/glitch/features/compose/components/compose_form.js b/app/javascript/flavours/glitch/features/compose/components/compose_form.js index d4804a3c28..5dfc119c1b 100644 --- a/app/javascript/flavours/glitch/features/compose/components/compose_form.js +++ b/app/javascript/flavours/glitch/features/compose/components/compose_form.js @@ -58,6 +58,7 @@ class ComposeForm extends ImmutablePureComponent { onPickEmoji: PropTypes.func, showSearch: PropTypes.bool, anyMedia: PropTypes.bool, + isInReply: PropTypes.bool, singleColumn: PropTypes.bool, advancedOptions: ImmutablePropTypes.map, @@ -233,7 +234,7 @@ class ComposeForm extends ImmutablePureComponent { // Caret/selection handling. if (focusDate !== prevProps.focusDate) { switch (true) { - case preselectDate !== prevProps.preselectDate && preselectOnReply: + case preselectDate !== prevProps.preselectDate && this.props.isInReply && preselectOnReply: selectionStart = text.search(/\s/) + 1; selectionEnd = text.length; break; diff --git a/app/javascript/flavours/glitch/features/compose/containers/compose_form_container.js b/app/javascript/flavours/glitch/features/compose/containers/compose_form_container.js index fcd2caf1bf..8eff8a36b8 100644 --- a/app/javascript/flavours/glitch/features/compose/containers/compose_form_container.js +++ b/app/javascript/flavours/glitch/features/compose/containers/compose_form_container.js @@ -68,6 +68,7 @@ function mapStateToProps (state) { spoilersAlwaysOn: spoilersAlwaysOn, mediaDescriptionConfirmation: state.getIn(['local_settings', 'confirm_missing_media_description']), preselectOnReply: state.getIn(['local_settings', 'preselect_on_reply']), + isInReply: state.getIn(['compose', 'in_reply_to']) !== null, }; }; From 9483d0c6b2b3eaffca8e02e8be4b9a21c5f9e767 Mon Sep 17 00:00:00 2001 From: Claire Date: Sun, 23 Jan 2022 16:01:25 +0100 Subject: [PATCH 16/16] [Glitch] Change `percent` to `rate` in retention metrics API Port a63495230a3a28e022504f36356cd75b17b635ba to glitch-soc Signed-off-by: Claire --- .../flavours/glitch/components/admin/Retention.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/javascript/flavours/glitch/components/admin/Retention.js b/app/javascript/flavours/glitch/components/admin/Retention.js index 9127839f6a..6d7e4b2798 100644 --- a/app/javascript/flavours/glitch/components/admin/Retention.js +++ b/app/javascript/flavours/glitch/components/admin/Retention.js @@ -88,7 +88,7 @@ export default class Retention extends React.PureComponent { {data[0].data.slice(1).map((retention, i) => { - const average = data.reduce((sum, cohort, k) => cohort.data[i + 1] ? sum + (cohort.data[i + 1].percent - sum)/(k + 1) : sum, 0); + const average = data.reduce((sum, cohort, k) => cohort.data[i + 1] ? sum + (cohort.data[i + 1].rate - sum)/(k + 1) : sum, 0); return ( @@ -118,8 +118,8 @@ export default class Retention extends React.PureComponent { {cohort.data.slice(1).map(retention => ( -
- +
+
))}