From 619fad6cf8078ea997554081febe850404bee73c Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sun, 11 Apr 2021 11:22:50 +0200 Subject: Remove spam check and dependency on nilsimsa gem (#16011) --- app/models/form/admin_settings.rb | 2 -- 1 file changed, 2 deletions(-) (limited to 'app/models') diff --git a/app/models/form/admin_settings.rb b/app/models/form/admin_settings.rb index e9f78da21..b5c3dcdbe 100644 --- a/app/models/form/admin_settings.rb +++ b/app/models/form/admin_settings.rb @@ -29,7 +29,6 @@ class Form::AdminSettings thumbnail hero mascot - spam_check_enabled trends trendable_by_default show_domain_blocks @@ -48,7 +47,6 @@ class Form::AdminSettings show_known_fediverse_at_about_page preview_sensitive_media profile_directory - spam_check_enabled trends trendable_by_default noindex -- cgit From f7117646afddb2676e9275d8efe90c3a20c59021 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Mon, 12 Apr 2021 12:37:14 +0200 Subject: Add cold-start follow recommendations (#15945) --- .../admin/follow_recommendations_controller.rb | 53 ++++++++++++++++++ app/controllers/api/v1/suggestions_controller.rb | 2 +- app/controllers/api/v2/suggestions_controller.rb | 19 +++++++ app/javascript/mastodon/actions/suggestions.js | 8 +-- .../features/compose/components/search_results.js | 12 ++--- app/javascript/mastodon/reducers/suggestions.js | 8 +-- app/lib/potential_friendship_tracker.rb | 12 +++-- app/models/account.rb | 3 +- app/models/account_suggestions.rb | 17 ++++++ app/models/account_summary.rb | 25 +++++++++ app/models/concerns/account_associations.rb | 3 ++ app/models/follow_recommendation.rb | 39 ++++++++++++++ app/models/follow_recommendation_filter.rb | 26 +++++++++ app/models/follow_recommendation_suppression.rb | 28 ++++++++++ app/models/form/account_batch.rb | 18 +++++++ app/policies/follow_recommendation_policy.rb | 15 ++++++ app/serializers/rest/suggestion_serializer.rb | 7 +++ .../follow_recommendations/_account.html.haml | 20 +++++++ .../admin/follow_recommendations/show.html.haml | 42 +++++++++++++++ .../scheduler/follow_recommendations_scheduler.rb | 61 +++++++++++++++++++++ config/locales/en.yml | 8 +++ config/navigation.rb | 1 + config/routes.rb | 2 + config/sidekiq.yml | 4 ++ .../20210322164601_create_account_summaries.rb | 9 ++++ ...20210323114347_create_follow_recommendations.rb | 5 ++ ...13_create_follow_recommendation_suppressions.rb | 9 ++++ db/schema.rb | 63 +++++++++++++++++++--- db/views/account_summaries_v01.sql | 22 ++++++++ db/views/follow_recommendations_v01.sql | 38 +++++++++++++ ...follow_recommendation_suppression_fabricator.rb | 3 ++ .../follow_recommendation_suppression_spec.rb | 4 ++ 32 files changed, 560 insertions(+), 26 deletions(-) create mode 100644 app/controllers/admin/follow_recommendations_controller.rb create mode 100644 app/controllers/api/v2/suggestions_controller.rb create mode 100644 app/models/account_suggestions.rb create mode 100644 app/models/account_summary.rb create mode 100644 app/models/follow_recommendation.rb create mode 100644 app/models/follow_recommendation_filter.rb create mode 100644 app/models/follow_recommendation_suppression.rb create mode 100644 app/policies/follow_recommendation_policy.rb create mode 100644 app/serializers/rest/suggestion_serializer.rb create mode 100644 app/views/admin/follow_recommendations/_account.html.haml create mode 100644 app/views/admin/follow_recommendations/show.html.haml create mode 100644 app/workers/scheduler/follow_recommendations_scheduler.rb create mode 100644 db/migrate/20210322164601_create_account_summaries.rb create mode 100644 db/migrate/20210323114347_create_follow_recommendations.rb create mode 100644 db/migrate/20210324171613_create_follow_recommendation_suppressions.rb create mode 100644 db/views/account_summaries_v01.sql create mode 100644 db/views/follow_recommendations_v01.sql create mode 100644 spec/fabricators/follow_recommendation_suppression_fabricator.rb create mode 100644 spec/models/follow_recommendation_suppression_spec.rb (limited to 'app/models') diff --git a/app/controllers/admin/follow_recommendations_controller.rb b/app/controllers/admin/follow_recommendations_controller.rb new file mode 100644 index 000000000..e3eac62b3 --- /dev/null +++ b/app/controllers/admin/follow_recommendations_controller.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +module Admin + class FollowRecommendationsController < BaseController + before_action :set_language + + def show + authorize :follow_recommendation, :show? + + @form = Form::AccountBatch.new + @accounts = filtered_follow_recommendations + end + + def update + @form = Form::AccountBatch.new(form_account_batch_params.merge(current_account: current_account, action: action_from_button)) + @form.save + rescue ActionController::ParameterMissing + # Do nothing + ensure + redirect_to admin_follow_recommendations_path(filter_params) + end + + private + + def set_language + @language = follow_recommendation_filter.language + end + + def filtered_follow_recommendations + follow_recommendation_filter.results + end + + def follow_recommendation_filter + @follow_recommendation_filter ||= FollowRecommendationFilter.new(filter_params) + end + + def form_account_batch_params + params.require(:form_account_batch).permit(:action, account_ids: []) + end + + def filter_params + params.slice(*FollowRecommendationFilter::KEYS).permit(*FollowRecommendationFilter::KEYS) + end + + def action_from_button + if params[:suppress] + 'suppress_follow_recommendation' + elsif params[:unsuppress] + 'unsuppress_follow_recommendation' + end + end + end +end diff --git a/app/controllers/api/v1/suggestions_controller.rb b/app/controllers/api/v1/suggestions_controller.rb index 52054160d..b2788cc76 100644 --- a/app/controllers/api/v1/suggestions_controller.rb +++ b/app/controllers/api/v1/suggestions_controller.rb @@ -19,6 +19,6 @@ class Api::V1::SuggestionsController < Api::BaseController private def set_accounts - @accounts = PotentialFriendshipTracker.get(current_account.id, limit: limit_param(DEFAULT_ACCOUNTS_LIMIT)) + @accounts = PotentialFriendshipTracker.get(current_account, limit_param(DEFAULT_ACCOUNTS_LIMIT)) end end diff --git a/app/controllers/api/v2/suggestions_controller.rb b/app/controllers/api/v2/suggestions_controller.rb new file mode 100644 index 000000000..35eb276c0 --- /dev/null +++ b/app/controllers/api/v2/suggestions_controller.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class Api::V2::SuggestionsController < Api::BaseController + include Authorization + + before_action -> { doorkeeper_authorize! :read } + before_action :require_user! + before_action :set_suggestions + + def index + render json: @suggestions, each_serializer: REST::SuggestionSerializer + end + + private + + def set_suggestions + @suggestions = AccountSuggestions.get(current_account, limit_param(DEFAULT_ACCOUNTS_LIMIT)) + end +end diff --git a/app/javascript/mastodon/actions/suggestions.js b/app/javascript/mastodon/actions/suggestions.js index b15bd916b..0bf959017 100644 --- a/app/javascript/mastodon/actions/suggestions.js +++ b/app/javascript/mastodon/actions/suggestions.js @@ -11,8 +11,8 @@ export function fetchSuggestions() { return (dispatch, getState) => { dispatch(fetchSuggestionsRequest()); - api(getState).get('/api/v1/suggestions').then(response => { - dispatch(importFetchedAccounts(response.data)); + api(getState).get('/api/v2/suggestions').then(response => { + dispatch(importFetchedAccounts(response.data.map(x => x.account))); dispatch(fetchSuggestionsSuccess(response.data)); }).catch(error => dispatch(fetchSuggestionsFail(error))); }; @@ -25,10 +25,10 @@ export function fetchSuggestionsRequest() { }; }; -export function fetchSuggestionsSuccess(accounts) { +export function fetchSuggestionsSuccess(suggestions) { return { type: SUGGESTIONS_FETCH_SUCCESS, - accounts, + suggestions, skipLoading: true, }; }; diff --git a/app/javascript/mastodon/features/compose/components/search_results.js b/app/javascript/mastodon/features/compose/components/search_results.js index 4b4cdff74..c4e160b8a 100644 --- a/app/javascript/mastodon/features/compose/components/search_results.js +++ b/app/javascript/mastodon/features/compose/components/search_results.js @@ -51,13 +51,13 @@ class SearchResults extends ImmutablePureComponent { - {suggestions && suggestions.map(accountId => ( + {suggestions && suggestions.map(suggestion => ( ))} diff --git a/app/javascript/mastodon/reducers/suggestions.js b/app/javascript/mastodon/reducers/suggestions.js index 834be728f..1a6e66ee7 100644 --- a/app/javascript/mastodon/reducers/suggestions.js +++ b/app/javascript/mastodon/reducers/suggestions.js @@ -19,18 +19,18 @@ export default function suggestionsReducer(state = initialState, action) { return state.set('isLoading', true); case SUGGESTIONS_FETCH_SUCCESS: return state.withMutations(map => { - map.set('items', fromJS(action.accounts.map(x => x.id))); + map.set('items', fromJS(action.suggestions.map(x => ({ ...x, account: x.account.id })))); map.set('isLoading', false); }); case SUGGESTIONS_FETCH_FAIL: return state.set('isLoading', false); case SUGGESTIONS_DISMISS: - return state.update('items', list => list.filterNot(id => id === action.id)); + return state.update('items', list => list.filterNot(x => x.account === action.id)); case ACCOUNT_BLOCK_SUCCESS: case ACCOUNT_MUTE_SUCCESS: - return state.update('items', list => list.filterNot(id => id === action.relationship.id)); + return state.update('items', list => list.filterNot(x => x.account === action.relationship.id)); case DOMAIN_BLOCK_SUCCESS: - return state.update('items', list => list.filterNot(id => action.accounts.includes(id))); + return state.update('items', list => list.filterNot(x => action.accounts.includes(x.account))); default: return state; } diff --git a/app/lib/potential_friendship_tracker.rb b/app/lib/potential_friendship_tracker.rb index 188aa4a27..e72d454b6 100644 --- a/app/lib/potential_friendship_tracker.rb +++ b/app/lib/potential_friendship_tracker.rb @@ -28,10 +28,14 @@ class PotentialFriendshipTracker redis.zrem("interactions:#{account_id}", target_account_id) end - def get(account_id, limit: 20, offset: 0) - account_ids = redis.zrevrange("interactions:#{account_id}", offset, limit) - return [] if account_ids.empty? - Account.searchable.where(id: account_ids) + def get(account, limit) + account_ids = redis.zrevrange("interactions:#{account.id}", 0, limit) + + return [] if account_ids.empty? || limit < 1 + + accounts = Account.searchable.where(id: account_ids).index_by(&:id) + + account_ids.map { |id| accounts[id.to_i] }.compact end end end diff --git a/app/models/account.rb b/app/models/account.rb index d85fd1f6e..80689d4aa 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -110,6 +110,7 @@ class Account < ApplicationRecord scope :matches_domain, ->(value) { where(arel_table[:domain].matches("%#{value}%")) } scope :searchable, -> { without_suspended.where(moved_to_account_id: nil) } scope :discoverable, -> { searchable.without_silenced.where(discoverable: true).left_outer_joins(:account_stat) } + scope :followable_by, ->(account) { joins(arel_table.join(Follow.arel_table, Arel::Nodes::OuterJoin).on(arel_table[:id].eq(Follow.arel_table[:target_account_id]).and(Follow.arel_table[:account_id].eq(account.id))).join_sources).where(Follow.arel_table[:id].eq(nil)).joins(arel_table.join(FollowRequest.arel_table, Arel::Nodes::OuterJoin).on(arel_table[:id].eq(FollowRequest.arel_table[:target_account_id]).and(FollowRequest.arel_table[:account_id].eq(account.id))).join_sources).where(FollowRequest.arel_table[:id].eq(nil)) } scope :tagged_with, ->(tag) { joins(:accounts_tags).where(accounts_tags: { tag_id: tag }) } scope :by_recent_status, -> { order(Arel.sql('(case when account_stats.last_status_at is null then 1 else 0 end) asc, account_stats.last_status_at desc, accounts.id desc')) } scope :by_recent_sign_in, -> { order(Arel.sql('(case when users.current_sign_in_at is null then 1 else 0 end) asc, users.current_sign_in_at desc, accounts.id desc')) } @@ -363,7 +364,7 @@ class Account < ApplicationRecord end def excluded_from_timeline_account_ids - Rails.cache.fetch("exclude_account_ids_for:#{id}") { blocking.pluck(:target_account_id) + blocked_by.pluck(:account_id) + muting.pluck(:target_account_id) } + Rails.cache.fetch("exclude_account_ids_for:#{id}") { block_relationships.pluck(:target_account_id) + blocked_by_relationships.pluck(:account_id) + mute_relationships.pluck(:target_account_id) } end def excluded_from_timeline_domains diff --git a/app/models/account_suggestions.rb b/app/models/account_suggestions.rb new file mode 100644 index 000000000..7fe9d618e --- /dev/null +++ b/app/models/account_suggestions.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AccountSuggestions + class Suggestion < ActiveModelSerializers::Model + attributes :account, :source + end + + def self.get(account, limit) + suggestions = PotentialFriendshipTracker.get(account, limit).map { |target_account| Suggestion.new(account: target_account, source: :past_interaction) } + suggestions.concat(FollowRecommendation.get(account, limit - suggestions.size, suggestions.map { |suggestion| suggestion.account.id }).map { |target_account| Suggestion.new(account: target_account, source: :global) }) if suggestions.size < limit + suggestions + end + + def self.remove(account, target_account_id) + PotentialFriendshipTracker.remove(account.id, target_account_id) + end +end diff --git a/app/models/account_summary.rb b/app/models/account_summary.rb new file mode 100644 index 000000000..6a7e17c6c --- /dev/null +++ b/app/models/account_summary.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true +# == Schema Information +# +# Table name: account_summaries +# +# account_id :bigint(8) primary key +# language :string +# sensitive :boolean +# + +class AccountSummary < ApplicationRecord + self.primary_key = :account_id + + scope :safe, -> { where(sensitive: false) } + scope :localized, ->(locale) { where(language: locale) } + scope :filtered, -> { joins(arel_table.join(FollowRecommendationSuppression.arel_table, Arel::Nodes::OuterJoin).on(arel_table[:account_id].eq(FollowRecommendationSuppression.arel_table[:account_id])).join_sources).where(FollowRecommendationSuppression.arel_table[:id].eq(nil)) } + + def self.refresh + Scenic.database.refresh_materialized_view(table_name, concurrently: true, cascade: false) + end + + def readonly? + true + end +end diff --git a/app/models/concerns/account_associations.rb b/app/models/concerns/account_associations.rb index 98849f8fc..aaf371ebd 100644 --- a/app/models/concerns/account_associations.rb +++ b/app/models/concerns/account_associations.rb @@ -63,5 +63,8 @@ module AccountAssociations # Account deletion requests has_one :deletion_request, class_name: 'AccountDeletionRequest', inverse_of: :account, dependent: :destroy + + # Follow recommendations + has_one :follow_recommendation_suppression, inverse_of: :account, dependent: :destroy end end diff --git a/app/models/follow_recommendation.rb b/app/models/follow_recommendation.rb new file mode 100644 index 000000000..c4355224d --- /dev/null +++ b/app/models/follow_recommendation.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true +# == Schema Information +# +# Table name: follow_recommendations +# +# account_id :bigint(8) primary key +# rank :decimal(, ) +# reason :text is an Array +# + +class FollowRecommendation < ApplicationRecord + self.primary_key = :account_id + + belongs_to :account_summary, foreign_key: :account_id + belongs_to :account, foreign_key: :account_id + + scope :safe, -> { joins(:account_summary).merge(AccountSummary.safe) } + scope :localized, ->(locale) { joins(:account_summary).merge(AccountSummary.localized(locale)) } + scope :filtered, -> { joins(:account_summary).merge(AccountSummary.filtered) } + + def readonly? + true + end + + def self.get(account, limit, exclude_account_ids = []) + account_ids = Redis.current.zrevrange("follow_recommendations:#{account.user_locale}", 0, -1).map(&:to_i) - exclude_account_ids - [account.id] + + return [] if account_ids.empty? || limit < 1 + + accounts = Account.followable_by(account) + .not_excluded_by_account(account) + .not_domain_blocked_by_account(account) + .where(id: account_ids) + .limit(limit) + .index_by(&:id) + + account_ids.map { |id| accounts[id] }.compact + end +end diff --git a/app/models/follow_recommendation_filter.rb b/app/models/follow_recommendation_filter.rb new file mode 100644 index 000000000..acf03cd84 --- /dev/null +++ b/app/models/follow_recommendation_filter.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +class FollowRecommendationFilter + KEYS = %i( + language + status + ).freeze + + attr_reader :params, :language + + def initialize(params) + @language = params.delete('language') || I18n.locale + @params = params + end + + def results + if params['status'] == 'suppressed' + Account.joins(:follow_recommendation_suppression).order(FollowRecommendationSuppression.arel_table[:id].desc).to_a + else + account_ids = Redis.current.zrevrange("follow_recommendations:#{@language}", 0, -1).map(&:to_i) + accounts = Account.where(id: account_ids).index_by(&:id) + + account_ids.map { |id| accounts[id] }.compact + end + end +end diff --git a/app/models/follow_recommendation_suppression.rb b/app/models/follow_recommendation_suppression.rb new file mode 100644 index 000000000..170506b85 --- /dev/null +++ b/app/models/follow_recommendation_suppression.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true +# == Schema Information +# +# Table name: follow_recommendation_suppressions +# +# id :bigint(8) not null, primary key +# account_id :bigint(8) not null +# created_at :datetime not null +# updated_at :datetime not null +# + +class FollowRecommendationSuppression < ApplicationRecord + include Redisable + + belongs_to :account + + after_commit :remove_follow_recommendations, on: :create + + private + + def remove_follow_recommendations + redis.pipelined do + I18n.available_locales.each do |locale| + redis.zrem("follow_recommendations:#{locale}", account_id) + end + end + end +end diff --git a/app/models/form/account_batch.rb b/app/models/form/account_batch.rb index 26d6d3abf..698933c9f 100644 --- a/app/models/form/account_batch.rb +++ b/app/models/form/account_batch.rb @@ -21,6 +21,10 @@ class Form::AccountBatch approve! when 'reject' reject! + when 'suppress_follow_recommendation' + suppress_follow_recommendation! + when 'unsuppress_follow_recommendation' + unsuppress_follow_recommendation! end end @@ -79,4 +83,18 @@ class Form::AccountBatch records.each { |account| authorize(account.user, :reject?) } .each { |account| DeleteAccountService.new.call(account, reserve_email: false, reserve_username: false) } end + + def suppress_follow_recommendation! + authorize(:follow_recommendation, :suppress?) + + accounts.each do |account| + FollowRecommendationSuppression.create(account: account) + end + end + + def unsuppress_follow_recommendation! + authorize(:follow_recommendation, :unsuppress?) + + FollowRecommendationSuppression.where(account_id: account_ids).destroy_all + end end diff --git a/app/policies/follow_recommendation_policy.rb b/app/policies/follow_recommendation_policy.rb new file mode 100644 index 000000000..68cd0e547 --- /dev/null +++ b/app/policies/follow_recommendation_policy.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class FollowRecommendationPolicy < ApplicationPolicy + def show? + staff? + end + + def suppress? + staff? + end + + def unsuppress? + staff? + end +end diff --git a/app/serializers/rest/suggestion_serializer.rb b/app/serializers/rest/suggestion_serializer.rb new file mode 100644 index 000000000..3d697fd9f --- /dev/null +++ b/app/serializers/rest/suggestion_serializer.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class REST::SuggestionSerializer < ActiveModel::Serializer + attributes :source + + has_one :account, serializer: REST::AccountSerializer +end diff --git a/app/views/admin/follow_recommendations/_account.html.haml b/app/views/admin/follow_recommendations/_account.html.haml new file mode 100644 index 000000000..af5a4aaf7 --- /dev/null +++ b/app/views/admin/follow_recommendations/_account.html.haml @@ -0,0 +1,20 @@ +.batch-table__row + %label.batch-table__row__select.batch-table__row__select--aligned.batch-checkbox + = f.check_box :account_ids, { multiple: true, include_hidden: false }, account.id + .batch-table__row__content.batch-table__row__content--unpadded + %table.accounts-table + %tbody + %tr + %td= account_link_to account + %td.accounts-table__count.optional + = number_to_human account.statuses_count, strip_insignificant_zeros: true + %small= t('accounts.posts', count: account.statuses_count).downcase + %td.accounts-table__count.optional + = number_to_human account.followers_count, strip_insignificant_zeros: true + %small= t('accounts.followers', count: account.followers_count).downcase + %td.accounts-table__count + - if account.last_status_at.present? + %time.time-ago{ datetime: account.last_status_at.to_date.iso8601, title: l(account.last_status_at.to_date) }= l account.last_status_at + - else + \- + %small= t('accounts.last_active') diff --git a/app/views/admin/follow_recommendations/show.html.haml b/app/views/admin/follow_recommendations/show.html.haml new file mode 100644 index 000000000..1f050329a --- /dev/null +++ b/app/views/admin/follow_recommendations/show.html.haml @@ -0,0 +1,42 @@ +- content_for :page_title do + = t('admin.follow_recommendations.title') + +- content_for :header_tags do + = javascript_pack_tag 'admin', async: true, crossorigin: 'anonymous' + +.simple_form + %p.hint= t('admin.follow_recommendations.description_html') + +%hr.spacer/ + += form_tag admin_follow_recommendations_path, method: 'GET', class: 'simple_form' do + .filters + .filter-subset.filter-subset--with-select + %strong= t('admin.follow_recommendations.language') + .input.select.optional + = select_tag :language, options_for_select(I18n.available_locales.map { |key| [human_locale(key), key]}, @language) + + .filter-subset + %strong= t('admin.follow_recommendations.status') + %ul + %li= filter_link_to t('admin.accounts.moderation.active'), status: nil + %li= filter_link_to t('admin.follow_recommendations.suppressed'), status: 'suppressed' + += form_for(@form, url: admin_follow_recommendations_path, method: :patch) do |f| + - RelationshipFilter::KEYS.each do |key| + = hidden_field_tag key, params[key] if params[key].present? + + .batch-table + .batch-table__toolbar + %label.batch-table__toolbar__select.batch-checkbox-all + = check_box_tag :batch_checkbox_all, nil, false + .batch-table__toolbar__actions + - if params[:status].blank? && can?(:suppress, :follow_recommendation) + = f.button safe_join([fa_icon('times'), t('admin.follow_recommendations.suppress')]), name: :suppress, class: 'table-action-link', type: :submit, data: { confirm: t('admin.reports.are_you_sure') } + - if params[:status] == 'suppressed' && can?(:unsuppress, :follow_recommendation) + = f.button safe_join([fa_icon('plus'), t('admin.follow_recommendations.unsuppress')]), name: :unsuppress, class: 'table-action-link', type: :submit + .batch-table__body + - if @accounts.empty? + = nothing_here 'nothing-here--under-tabs' + - else + = render partial: 'account', collection: @accounts, locals: { f: f } diff --git a/app/workers/scheduler/follow_recommendations_scheduler.rb b/app/workers/scheduler/follow_recommendations_scheduler.rb new file mode 100644 index 000000000..0a0286496 --- /dev/null +++ b/app/workers/scheduler/follow_recommendations_scheduler.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +class Scheduler::FollowRecommendationsScheduler + include Sidekiq::Worker + include Redisable + + sidekiq_options retry: 0 + + # The maximum number of accounts that can be requested in one page from the + # API is 80, and the suggestions API does not allow pagination. This number + # leaves some room for accounts being filtered during live access + SET_SIZE = 100 + + def perform + # Maintaining a materialized view speeds-up subsequent queries significantly + AccountSummary.refresh + + fallback_recommendations = FollowRecommendation.safe.filtered.limit(SET_SIZE).index_by(&:account_id) + + I18n.available_locales.each do |locale| + recommendations = begin + if AccountSummary.safe.filtered.localized(locale).exists? # We can skip the work if no accounts with that language exist + FollowRecommendation.safe.filtered.localized(locale).limit(SET_SIZE).index_by(&:account_id) + else + {} + end + end + + # Use language-agnostic results if there are not enough language-specific ones + missing = SET_SIZE - recommendations.keys.size + + if missing.positive? + added = 0 + + # Avoid duplicate results + fallback_recommendations.each_value do |recommendation| + next if recommendations.key?(recommendation.account_id) + + recommendations[recommendation.account_id] = recommendation + added += 1 + + break if added >= missing + end + end + + redis.pipelined do + redis.del(key(locale)) + + recommendations.each_value do |recommendation| + redis.zadd(key(locale), recommendation.rank, recommendation.account_id) + end + end + end + end + + private + + def key(locale) + "follow_recommendations:#{locale}" + end +end diff --git a/config/locales/en.yml b/config/locales/en.yml index 3387b4df6..afab6d9b5 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -440,6 +440,14 @@ en: create: Add domain title: Block new e-mail domain title: Blocked e-mail domains + follow_recommendations: + description_html: "Follow recommendations help new users quickly find interesting content. When a user has not interacted with others enough to form personalized follow recommendations, these accounts are recommended instead. They are re-calculated on a daily basis from a mix of accounts with the highest recent engagements and highest local follower counts for a given language." + language: For language + status: Status + suppress: Suppress follow recommendation + suppressed: Suppressed + title: Follow recommendations + unsuppress: Restore follow recommendation instances: by_domain: Domain delivery_available: Delivery is available diff --git a/config/navigation.rb b/config/navigation.rb index 3a82c7971..b3462c48d 100644 --- a/config/navigation.rb +++ b/config/navigation.rb @@ -39,6 +39,7 @@ SimpleNavigation::Configuration.run do |navigation| s.item :accounts, safe_join([fa_icon('users fw'), t('admin.accounts.title')]), admin_accounts_url, highlights_on: %r{/admin/accounts|/admin/pending_accounts} s.item :invites, safe_join([fa_icon('user-plus fw'), t('admin.invites.title')]), admin_invites_path s.item :tags, safe_join([fa_icon('hashtag fw'), t('admin.tags.title')]), admin_tags_path, highlights_on: %r{/admin/tags} + s.item :follow_recommendations, safe_join([fa_icon('user-plus fw'), t('admin.follow_recommendations.title')]), admin_follow_recommendations_path, highlights_on: %r{/admin/follow_recommendations} s.item :instances, safe_join([fa_icon('cloud fw'), t('admin.instances.title')]), admin_instances_url(limited: whitelist_mode? ? nil : '1'), highlights_on: %r{/admin/instances|/admin/domain_blocks|/admin/domain_allows}, if: -> { current_user.admin? } s.item :email_domain_blocks, safe_join([fa_icon('envelope fw'), t('admin.email_domain_blocks.title')]), admin_email_domain_blocks_url, highlights_on: %r{/admin/email_domain_blocks}, if: -> { current_user.admin? } s.item :ip_blocks, safe_join([fa_icon('ban fw'), t('admin.ip_blocks.title')]), admin_ip_blocks_url, highlights_on: %r{/admin/ip_blocks}, if: -> { current_user.admin? } diff --git a/config/routes.rb b/config/routes.rb index eedd0de69..4661a7c11 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -292,6 +292,7 @@ Rails.application.routes.draw do end resources :account_moderation_notes, only: [:create, :destroy] + resource :follow_recommendations, only: [:show, :update] resources :tags, only: [:index, :show, :update] do collection do @@ -507,6 +508,7 @@ Rails.application.routes.draw do namespace :v2 do resources :media, only: [:create] get '/search', to: 'search#index', as: :search + resources :suggestions, only: [:index] end namespace :web do diff --git a/config/sidekiq.yml b/config/sidekiq.yml index 010923717..a8e4c7feb 100644 --- a/config/sidekiq.yml +++ b/config/sidekiq.yml @@ -25,6 +25,10 @@ cron: '<%= Random.rand(0..59) %> <%= Random.rand(0..2) %> * * *' class: Scheduler::FeedCleanupScheduler queue: scheduler + follow_recommendations_scheduler: + cron: '<%= Random.rand(0..59) %> <%= Random.rand(6..9) %> * * *' + class: Scheduler::FollowRecommendationsScheduler + queue: scheduler doorkeeper_cleanup_scheduler: cron: '<%= Random.rand(0..59) %> <%= Random.rand(0..2) %> * * 0' class: Scheduler::DoorkeeperCleanupScheduler diff --git a/db/migrate/20210322164601_create_account_summaries.rb b/db/migrate/20210322164601_create_account_summaries.rb new file mode 100644 index 000000000..b9faf180d --- /dev/null +++ b/db/migrate/20210322164601_create_account_summaries.rb @@ -0,0 +1,9 @@ +class CreateAccountSummaries < ActiveRecord::Migration[5.2] + def change + create_view :account_summaries, materialized: true + + # To be able to refresh the view concurrently, + # at least one unique index is required + safety_assured { add_index :account_summaries, :account_id, unique: true } + end +end diff --git a/db/migrate/20210323114347_create_follow_recommendations.rb b/db/migrate/20210323114347_create_follow_recommendations.rb new file mode 100644 index 000000000..77e729032 --- /dev/null +++ b/db/migrate/20210323114347_create_follow_recommendations.rb @@ -0,0 +1,5 @@ +class CreateFollowRecommendations < ActiveRecord::Migration[5.2] + def change + create_view :follow_recommendations + end +end diff --git a/db/migrate/20210324171613_create_follow_recommendation_suppressions.rb b/db/migrate/20210324171613_create_follow_recommendation_suppressions.rb new file mode 100644 index 000000000..c17a0be63 --- /dev/null +++ b/db/migrate/20210324171613_create_follow_recommendation_suppressions.rb @@ -0,0 +1,9 @@ +class CreateFollowRecommendationSuppressions < ActiveRecord::Migration[6.1] + def change + create_table :follow_recommendation_suppressions do |t| + t.references :account, null: false, foreign_key: { on_delete: :cascade }, index: { unique: true } + + t.timestamps + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 4edaf5651..28f36abb1 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -2,15 +2,15 @@ # of editing this file, please use the migrations feature of Active Record to # incrementally modify your database, and then regenerate this schema definition. # -# Note that this schema.rb definition is the authoritative source for your -# database schema. If you need to create the application database on another -# system, you should be using db:schema:load, not running all the migrations -# from scratch. The latter is a flawed and unsustainable approach (the more migrations -# you'll amass, the slower it'll run and the greater likelihood for issues). +# This file is the source Rails uses to define your schema when running `bin/rails +# db:schema:load`. When creating a new database, `bin/rails db:schema:load` tends to +# be faster and is potentially less error prone than running all of your +# migrations from scratch. Old migrations may fail to apply correctly if those +# migrations use external dependencies or application code. # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2021_03_08_133107) do +ActiveRecord::Schema.define(version: 2021_03_24_171613) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -406,6 +406,13 @@ ActiveRecord::Schema.define(version: 2021_03_08_133107) do t.index ["tag_id"], name: "index_featured_tags_on_tag_id" end + create_table "follow_recommendation_suppressions", force: :cascade do |t| + t.bigint "account_id", null: false + t.datetime "created_at", precision: 6, null: false + t.datetime "updated_at", precision: 6, null: false + t.index ["account_id"], name: "index_follow_recommendation_suppressions_on_account_id", unique: true + end + create_table "follow_requests", force: :cascade do |t| t.datetime "created_at", null: false t.datetime "updated_at", null: false @@ -996,6 +1003,7 @@ ActiveRecord::Schema.define(version: 2021_03_08_133107) do add_foreign_key "favourites", "statuses", name: "fk_b0e856845e", on_delete: :cascade add_foreign_key "featured_tags", "accounts", on_delete: :cascade add_foreign_key "featured_tags", "tags", on_delete: :cascade + add_foreign_key "follow_recommendation_suppressions", "accounts", on_delete: :cascade add_foreign_key "follow_requests", "accounts", column: "target_account_id", name: "fk_9291ec025d", on_delete: :cascade add_foreign_key "follow_requests", "accounts", name: "fk_76d644b0e7", on_delete: :cascade add_foreign_key "follows", "accounts", column: "target_account_id", name: "fk_745ca29eac", on_delete: :cascade @@ -1079,4 +1087,47 @@ ActiveRecord::Schema.define(version: 2021_03_08_133107) do SQL add_index "instances", ["domain"], name: "index_instances_on_domain", unique: true + create_view "account_summaries", materialized: true, sql_definition: <<-SQL + SELECT accounts.id AS account_id, + mode() WITHIN GROUP (ORDER BY t0.language) AS language, + mode() WITHIN GROUP (ORDER BY t0.sensitive) AS sensitive + FROM (accounts + CROSS JOIN LATERAL ( SELECT statuses.account_id, + statuses.language, + statuses.sensitive + FROM statuses + WHERE ((statuses.account_id = accounts.id) AND (statuses.deleted_at IS NULL)) + ORDER BY statuses.id DESC + LIMIT 20) t0) + WHERE ((accounts.suspended_at IS NULL) AND (accounts.silenced_at IS NULL) AND (accounts.moved_to_account_id IS NULL) AND (accounts.discoverable = true) AND (accounts.locked = false)) + GROUP BY accounts.id; + SQL + add_index "account_summaries", ["account_id"], name: "index_account_summaries_on_account_id", unique: true + + create_view "follow_recommendations", sql_definition: <<-SQL + SELECT t0.account_id, + sum(t0.rank) AS rank, + array_agg(t0.reason) AS reason + FROM ( SELECT accounts.id AS account_id, + ((count(follows.id))::numeric / (1.0 + (count(follows.id))::numeric)) AS rank, + 'most_followed'::text AS reason + FROM ((follows + JOIN accounts ON ((accounts.id = follows.target_account_id))) + JOIN users ON ((users.account_id = follows.account_id))) + WHERE ((users.current_sign_in_at >= (now() - 'P30D'::interval)) AND (accounts.suspended_at IS NULL) AND (accounts.moved_to_account_id IS NULL) AND (accounts.silenced_at IS NULL) AND (accounts.locked = false) AND (accounts.discoverable = true)) + GROUP BY accounts.id + HAVING (count(follows.id) >= 5) + UNION ALL + SELECT accounts.id AS account_id, + (sum((status_stats.reblogs_count + status_stats.favourites_count)) / (1.0 + sum((status_stats.reblogs_count + status_stats.favourites_count)))) AS rank, + 'most_interactions'::text AS reason + FROM ((status_stats + JOIN statuses ON ((statuses.id = status_stats.status_id))) + JOIN accounts ON ((accounts.id = statuses.account_id))) + WHERE ((statuses.id >= (((date_part('epoch'::text, (now() - 'P30D'::interval)) * (1000)::double precision))::bigint << 16)) AND (accounts.suspended_at IS NULL) AND (accounts.moved_to_account_id IS NULL) AND (accounts.silenced_at IS NULL) AND (accounts.locked = false) AND (accounts.discoverable = true)) + GROUP BY accounts.id + HAVING (sum((status_stats.reblogs_count + status_stats.favourites_count)) >= (5)::numeric)) t0 + GROUP BY t0.account_id + ORDER BY (sum(t0.rank)) DESC; + SQL end diff --git a/db/views/account_summaries_v01.sql b/db/views/account_summaries_v01.sql new file mode 100644 index 000000000..5a632b622 --- /dev/null +++ b/db/views/account_summaries_v01.sql @@ -0,0 +1,22 @@ +SELECT + accounts.id AS account_id, + mode() WITHIN GROUP (ORDER BY language ASC) AS language, + mode() WITHIN GROUP (ORDER BY sensitive ASC) AS sensitive +FROM accounts +CROSS JOIN LATERAL ( + SELECT + statuses.account_id, + statuses.language, + statuses.sensitive + FROM statuses + WHERE statuses.account_id = accounts.id + AND statuses.deleted_at IS NULL + ORDER BY statuses.id DESC + LIMIT 20 +) t0 +WHERE accounts.suspended_at IS NULL + AND accounts.silenced_at IS NULL + AND accounts.moved_to_account_id IS NULL + AND accounts.discoverable = 't' + AND accounts.locked = 'f' +GROUP BY accounts.id diff --git a/db/views/follow_recommendations_v01.sql b/db/views/follow_recommendations_v01.sql new file mode 100644 index 000000000..799abeaee --- /dev/null +++ b/db/views/follow_recommendations_v01.sql @@ -0,0 +1,38 @@ +SELECT + account_id, + sum(rank) AS rank, + array_agg(reason) AS reason +FROM ( + SELECT + accounts.id AS account_id, + count(follows.id) / (1.0 + count(follows.id)) AS rank, + 'most_followed' AS reason + FROM follows + INNER JOIN accounts ON accounts.id = follows.target_account_id + INNER JOIN users ON users.account_id = follows.account_id + WHERE users.current_sign_in_at >= (now() - interval '30 days') + AND accounts.suspended_at IS NULL + AND accounts.moved_to_account_id IS NULL + AND accounts.silenced_at IS NULL + AND accounts.locked = 'f' + AND accounts.discoverable = 't' + GROUP BY accounts.id + HAVING count(follows.id) >= 5 + UNION ALL + SELECT accounts.id AS account_id, + sum(reblogs_count + favourites_count) / (1.0 + sum(reblogs_count + favourites_count)) AS rank, + 'most_interactions' AS reason + FROM status_stats + INNER JOIN statuses ON statuses.id = status_stats.status_id + INNER JOIN accounts ON accounts.id = statuses.account_id + WHERE statuses.id >= ((date_part('epoch', now() - interval '30 days') * 1000)::bigint << 16) + AND accounts.suspended_at IS NULL + AND accounts.moved_to_account_id IS NULL + AND accounts.silenced_at IS NULL + AND accounts.locked = 'f' + AND accounts.discoverable = 't' + GROUP BY accounts.id + HAVING sum(reblogs_count + favourites_count) >= 5 +) t0 +GROUP BY account_id +ORDER BY rank DESC diff --git a/spec/fabricators/follow_recommendation_suppression_fabricator.rb b/spec/fabricators/follow_recommendation_suppression_fabricator.rb new file mode 100644 index 000000000..4a6a07a66 --- /dev/null +++ b/spec/fabricators/follow_recommendation_suppression_fabricator.rb @@ -0,0 +1,3 @@ +Fabricator(:follow_recommendation_suppression) do + account +end diff --git a/spec/models/follow_recommendation_suppression_spec.rb b/spec/models/follow_recommendation_suppression_spec.rb new file mode 100644 index 000000000..39107a2b0 --- /dev/null +++ b/spec/models/follow_recommendation_suppression_spec.rb @@ -0,0 +1,4 @@ +require 'rails_helper' + +RSpec.describe FollowRecommendationSuppression, type: :model do +end -- cgit From 120965eb0b1b0da6906bb242da50a77367defd96 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Mon, 12 Apr 2021 14:25:34 +0200 Subject: Change Web Push API deliveries to use request pooling (#16014) --- Gemfile | 2 +- Gemfile.lock | 2 +- app/models/web/push_subscription.rb | 91 +++++++++++------------ app/workers/web/push_notification_worker.rb | 65 +++++++++++++--- spec/workers/web/push_notification_worker_spec.rb | 48 ++++++++++++ 5 files changed, 150 insertions(+), 58 deletions(-) create mode 100644 spec/workers/web/push_notification_worker_spec.rb (limited to 'app/models') diff --git a/Gemfile b/Gemfile index d4385f014..cc77ee618 100644 --- a/Gemfile +++ b/Gemfile @@ -94,7 +94,7 @@ gem 'tty-prompt', '~> 0.23', require: false gem 'twitter-text', '~> 3.1.0' gem 'tzinfo-data', '~> 1.2021' gem 'webpacker', '~> 5.2' -gem 'webpush' +gem 'webpush', '~> 0.3' gem 'webauthn', '~> 3.0.0.alpha1' gem 'json-ld' diff --git a/Gemfile.lock b/Gemfile.lock index b1fb350d4..1d3c24595 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -808,5 +808,5 @@ DEPENDENCIES webauthn (~> 3.0.0.alpha1) webmock (~> 3.12) webpacker (~> 5.2) - webpush + webpush (~> 0.3) xorcist (~> 1.1) diff --git a/app/models/web/push_subscription.rb b/app/models/web/push_subscription.rb index c407a7789..7609b1bfc 100644 --- a/app/models/web/push_subscription.rb +++ b/app/models/web/push_subscription.rb @@ -24,81 +24,80 @@ class Web::PushSubscription < ApplicationRecord validates :key_p256dh, presence: true validates :key_auth, presence: true - def push(notification) - I18n.with_locale(associated_user&.locale || I18n.default_locale) do - push_payload(payload_for_notification(notification), 48.hours.seconds) - end + delegate :locale, to: :associated_user + + def encrypt(payload) + Webpush::Encryption.encrypt(payload, key_p256dh, key_auth) + end + + def audience + @audience ||= Addressable::URI.parse(endpoint).normalized_site + end + + def crypto_key_header + p256ecdsa = vapid_key.public_key_for_push_header + + "p256ecdsa=#{p256ecdsa}" + end + + def authorization_header + jwt = JWT.encode({ aud: audience, exp: 24.hours.from_now.to_i, sub: "mailto:#{contact_email}" }, vapid_key.curve, 'ES256', typ: 'JWT') + + "WebPush #{jwt}" end def pushable?(notification) - data&.key?('alerts') && ActiveModel::Type::Boolean.new.cast(data['alerts'][notification.type.to_s]) + ActiveModel::Type::Boolean.new.cast(data&.dig('alerts', notification.type.to_s)) end def associated_user return @associated_user if defined?(@associated_user) - @associated_user = if user_id.nil? - session_activation.user - else - user - end + @associated_user = begin + if user_id.nil? + session_activation.user + else + user + end + end end def associated_access_token return @associated_access_token if defined?(@associated_access_token) - @associated_access_token = if access_token_id.nil? - find_or_create_access_token.token - else - access_token.token - end + @associated_access_token = begin + if access_token_id.nil? + find_or_create_access_token.token + else + access_token.token + end + end end class << self def unsubscribe_for(application_id, resource_owner) - access_token_ids = Doorkeeper::AccessToken.where(application_id: application_id, resource_owner_id: resource_owner.id, revoked_at: nil) - .pluck(:id) - + access_token_ids = Doorkeeper::AccessToken.where(application_id: application_id, resource_owner_id: resource_owner.id, revoked_at: nil).pluck(:id) where(access_token_id: access_token_ids).delete_all end end private - def push_payload(message, ttl = 5.minutes.seconds) - Webpush.payload_send( - message: Oj.dump(message), - endpoint: endpoint, - p256dh: key_p256dh, - auth: key_auth, - ttl: ttl, - ssl_timeout: 10, - open_timeout: 10, - read_timeout: 10, - vapid: { - subject: "mailto:#{::Setting.site_contact_email}", - private_key: Rails.configuration.x.vapid_private_key, - public_key: Rails.configuration.x.vapid_public_key, - } - ) - end - - def payload_for_notification(notification) - ActiveModelSerializers::SerializableResource.new( - notification, - serializer: Web::NotificationSerializer, - scope: self, - scope_name: :current_push_subscription - ).as_json - end - def find_or_create_access_token Doorkeeper::AccessToken.find_or_create_for( application: Doorkeeper::Application.find_by(superapp: true), - resource_owner: session_activation.user_id, + resource_owner: user_id || session_activation.user_id, scopes: Doorkeeper::OAuth::Scopes.from_string('read write follow push'), expires_in: Doorkeeper.configuration.access_token_expires_in, use_refresh_token: Doorkeeper.configuration.refresh_token_enabled? ) end + + def vapid_key + @vapid_key ||= Webpush::VapidKey.from_keys(Rails.configuration.x.vapid_public_key, Rails.configuration.x.vapid_private_key) + end + + def contact_email + @contact_email ||= ::Setting.site_contact_email + end end diff --git a/app/workers/web/push_notification_worker.rb b/app/workers/web/push_notification_worker.rb index 46aeaa30b..57f5b5c22 100644 --- a/app/workers/web/push_notification_worker.rb +++ b/app/workers/web/push_notification_worker.rb @@ -3,22 +3,67 @@ class Web::PushNotificationWorker include Sidekiq::Worker - sidekiq_options backtrace: true, retry: 5 + sidekiq_options queue: 'push', retry: 5 + + TTL = 48.hours.to_s + URGENCY = 'normal' def perform(subscription_id, notification_id) - subscription = ::Web::PushSubscription.find(subscription_id) - notification = Notification.find(notification_id) + @subscription = Web::PushSubscription.find(subscription_id) + @notification = Notification.find(notification_id) + + # Polymorphically associated activity could have been deleted + # in the meantime, so we have to double-check before proceeding + return unless @notification.activity.present? && @subscription.pushable?(@notification) + + payload = @subscription.encrypt(push_notification_json) - subscription.push(notification) unless notification.activity.nil? - rescue Webpush::ResponseError => e - code = e.response.code.to_i + request_pool.with(@subscription.audience) do |http_client| + request = Request.new(:post, @subscription.endpoint, body: payload.fetch(:ciphertext), http_client: http_client) - if (400..499).cover?(code) && ![408, 429].include?(code) - subscription.destroy! - else - raise e + request.add_headers( + 'Content-Type' => 'application/octet-stream', + 'Ttl' => TTL, + 'Urgency' => URGENCY, + 'Content-Encoding' => 'aesgcm', + 'Encryption' => "salt=#{Webpush.encode64(payload.fetch(:salt)).delete('=')}", + 'Crypto-Key' => "dh=#{Webpush.encode64(payload.fetch(:server_public_key)).delete('=')};#{@subscription.crypto_key_header}", + 'Authorization' => @subscription.authorization_header + ) + + request.perform do |response| + # If the server responds with an error in the 4xx range + # that isn't about rate-limiting or timeouts, we can + # assume that the subscription is invalid or expired + # and must be removed + + if (400..499).cover?(response.code) && ![408, 429].include?(response.code) + @subscription.destroy! + elsif !(200...300).cover?(response.code) + raise Mastodon::UnexpectedResponseError, response + end + end end rescue ActiveRecord::RecordNotFound true end + + private + + def push_notification_json + json = I18n.with_locale(@subscription.locale || I18n.default_locale) do + ActiveModelSerializers::SerializableResource.new( + @notification, + serializer: Web::NotificationSerializer, + scope: @subscription, + scope_name: :current_push_subscription + ).as_json + end + + Oj.dump(json) + end + + def request_pool + RequestPool.current + end end diff --git a/spec/workers/web/push_notification_worker_spec.rb b/spec/workers/web/push_notification_worker_spec.rb new file mode 100644 index 000000000..5bc24f888 --- /dev/null +++ b/spec/workers/web/push_notification_worker_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe Web::PushNotificationWorker do + subject { described_class.new } + + let(:p256dh) { 'BN4GvZtEZiZuqFxSKVZfSfluwKBD7UxHNBmWkfiZfCtgDE8Bwh-_MtLXbBxTBAWH9r7IPKL0lhdcaqtL1dfxU5E=' } + let(:auth) { 'Q2BoAjC09xH3ywDLNJr-dA==' } + let(:endpoint) { 'https://updates.push.services.mozilla.com/push/v1/subscription-id' } + let(:user) { Fabricate(:user) } + let(:notification) { Fabricate(:notification) } + let(:subscription) { Fabricate(:web_push_subscription, user_id: user.id, key_p256dh: p256dh, key_auth: auth, endpoint: endpoint, data: { alerts: { notification.type => true } }) } + let(:vapid_public_key) { 'BB37UCyc8LLX4PNQSe-04vSFvpUWGrENubUaslVFM_l5TxcGVMY0C3RXPeUJAQHKYlcOM2P4vTYmkoo0VZGZTM4=' } + let(:vapid_private_key) { 'OPrw1Sum3gRoL4-DXfSCC266r-qfFSRZrnj8MgIhRHg=' } + let(:vapid_key) { Webpush::VapidKey.from_keys(vapid_public_key, vapid_private_key) } + let(:contact_email) { 'sender@example.com' } + let(:ciphertext) { "+\xB8\xDBT}\x13\xB6\xDD.\xF9\xB0\xA7\xC8\xD2\x80\xFD\x99#\xF7\xAC\x83\xA4\xDB,\x1F\xB5\xB9w\x85>\xF7\xADr" } + let(:salt) { "X\x97\x953\xE4X\xF8_w\xE7T\x95\xC51q\xFE" } + let(:server_public_key) { "\x04\b-RK9w\xDD$\x16lFz\xF9=\xB4~\xC6\x12k\xF3\xF40t\xA9\xC1\fR\xC3\x81\x80\xAC\f\x7F\xE4\xCC\x8E\xC2\x88 n\x8BB\xF1\x9C\x14\a\xFA\x8D\xC9\x80\xA1\xDDyU\\&c\x01\x88#\x118Ua" } + let(:shared_secret) { "\t\xA7&\x85\t\xC5m\b\xA8\xA7\xF8B{1\xADk\xE1y'm\xEDE\xEC\xDD\xEDj\xB3$s\xA9\xDA\xF0" } + let(:payload) { { ciphertext: ciphertext, salt: salt, server_public_key: server_public_key, shared_secret: shared_secret } } + + describe 'perform' do + before do + allow_any_instance_of(subscription.class).to receive(:contact_email).and_return(contact_email) + allow_any_instance_of(subscription.class).to receive(:vapid_key).and_return(vapid_key) + allow(Webpush::Encryption).to receive(:encrypt).and_return(payload) + allow(JWT).to receive(:encode).and_return('jwt.encoded.payload') + + stub_request(:post, endpoint).to_return(status: 201, body: '') + + subject.perform(subscription.id, notification.id) + end + + it 'calls the relevant service with the correct headers' do + expect(a_request(:post, endpoint).with(headers: { + 'Content-Encoding' => 'aesgcm', + 'Content-Type' => 'application/octet-stream', + 'Crypto-Key' => 'dh=BAgtUks5d90kFmxGevk9tH7GEmvz9DB0qcEMUsOBgKwMf-TMjsKIIG6LQvGcFAf6jcmAod15VVwmYwGIIxE4VWE;p256ecdsa=' + vapid_public_key.delete('='), + 'Encryption' => 'salt=WJeVM-RY-F9351SVxTFx_g', + 'Ttl' => '172800', + 'Urgency' => 'normal', + 'Authorization' => 'WebPush jwt.encoded.payload', + }, body: "+\xB8\xDBT}\u0013\xB6\xDD.\xF9\xB0\xA7\xC8Ҁ\xFD\x99#\xF7\xAC\x83\xA4\xDB,\u001F\xB5\xB9w\x85>\xF7\xADr")).to have_been_made + end + end +end -- cgit From ce2148c57111981be455a9953d3bb589cf53967f Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Thu, 15 Apr 2021 05:00:25 +0200 Subject: Add `policy` param to `POST /api/v1/push/subscriptions` (#16040) With possible values `all`, `followed`, `follower`, and `none`, control from whom notifications will generate a Web Push alert --- .../api/v1/push/subscriptions_controller.rb | 28 +++---- .../api/web/push_subscriptions_controller.rb | 25 +++--- app/models/web/push_subscription.rb | 23 +++++- .../api/v1/push/subscriptions_controller_spec.rb | 28 ++++--- .../api/web/push_subscriptions_controller_spec.rb | 23 ++++-- spec/models/web/push_subscription_spec.rb | 94 ++++++++++++++++++++-- 6 files changed, 170 insertions(+), 51 deletions(-) (limited to 'app/models') diff --git a/app/controllers/api/v1/push/subscriptions_controller.rb b/app/controllers/api/v1/push/subscriptions_controller.rb index 0918c61e9..47f2e6440 100644 --- a/app/controllers/api/v1/push/subscriptions_controller.rb +++ b/app/controllers/api/v1/push/subscriptions_controller.rb @@ -3,13 +3,13 @@ class Api::V1::Push::SubscriptionsController < Api::BaseController before_action -> { doorkeeper_authorize! :push } before_action :require_user! - before_action :set_web_push_subscription - before_action :check_web_push_subscription, only: [:show, :update] + before_action :set_push_subscription + before_action :check_push_subscription, only: [:show, :update] def create - @web_subscription&.destroy! + @push_subscription&.destroy! - @web_subscription = ::Web::PushSubscription.create!( + @push_subscription = Web::PushSubscription.create!( endpoint: subscription_params[:endpoint], key_p256dh: subscription_params[:keys][:p256dh], key_auth: subscription_params[:keys][:auth], @@ -18,31 +18,31 @@ class Api::V1::Push::SubscriptionsController < Api::BaseController access_token_id: doorkeeper_token.id ) - render json: @web_subscription, serializer: REST::WebPushSubscriptionSerializer + render json: @push_subscription, serializer: REST::WebPushSubscriptionSerializer end def show - render json: @web_subscription, serializer: REST::WebPushSubscriptionSerializer + render json: @push_subscription, serializer: REST::WebPushSubscriptionSerializer end def update - @web_subscription.update!(data: data_params) - render json: @web_subscription, serializer: REST::WebPushSubscriptionSerializer + @push_subscription.update!(data: data_params) + render json: @push_subscription, serializer: REST::WebPushSubscriptionSerializer end def destroy - @web_subscription&.destroy! + @push_subscription&.destroy! render_empty end private - def set_web_push_subscription - @web_subscription = ::Web::PushSubscription.find_by(access_token_id: doorkeeper_token.id) + def set_push_subscription + @push_subscription = Web::PushSubscription.find_by(access_token_id: doorkeeper_token.id) end - def check_web_push_subscription - not_found if @web_subscription.nil? + def check_push_subscription + not_found if @push_subscription.nil? end def subscription_params @@ -52,6 +52,6 @@ class Api::V1::Push::SubscriptionsController < Api::BaseController def data_params return {} if params[:data].blank? - params.require(:data).permit(alerts: [:follow, :follow_request, :favourite, :reblog, :mention, :poll, :status]) + params.require(:data).permit(:policy, alerts: [:follow, :follow_request, :favourite, :reblog, :mention, :poll, :status]) end end diff --git a/app/controllers/api/web/push_subscriptions_controller.rb b/app/controllers/api/web/push_subscriptions_controller.rb index 1dce3e70f..bed57fc54 100644 --- a/app/controllers/api/web/push_subscriptions_controller.rb +++ b/app/controllers/api/web/push_subscriptions_controller.rb @@ -2,6 +2,7 @@ class Api::Web::PushSubscriptionsController < Api::Web::BaseController before_action :require_user! + before_action :set_push_subscription, only: :update def create active_session = current_session @@ -15,9 +16,11 @@ class Api::Web::PushSubscriptionsController < Api::Web::BaseController alerts_enabled = active_session.detection.device.mobile? || active_session.detection.device.tablet? data = { + policy: 'all', + alerts: { follow: alerts_enabled, - follow_request: false, + follow_request: alerts_enabled, favourite: alerts_enabled, reblog: alerts_enabled, mention: alerts_enabled, @@ -28,7 +31,7 @@ class Api::Web::PushSubscriptionsController < Api::Web::BaseController data.deep_merge!(data_params) if params[:data] - web_subscription = ::Web::PushSubscription.create!( + push_subscription = ::Web::PushSubscription.create!( endpoint: subscription_params[:endpoint], key_p256dh: subscription_params[:keys][:p256dh], key_auth: subscription_params[:keys][:auth], @@ -37,27 +40,27 @@ class Api::Web::PushSubscriptionsController < Api::Web::BaseController access_token_id: active_session.access_token_id ) - active_session.update!(web_push_subscription: web_subscription) + active_session.update!(web_push_subscription: push_subscription) - render json: web_subscription, serializer: REST::WebPushSubscriptionSerializer + render json: push_subscription, serializer: REST::WebPushSubscriptionSerializer end def update - params.require([:id]) - - web_subscription = ::Web::PushSubscription.find(params[:id]) - web_subscription.update!(data: data_params) - - render json: web_subscription, serializer: REST::WebPushSubscriptionSerializer + @push_subscription.update!(data: data_params) + render json: @push_subscription, serializer: REST::WebPushSubscriptionSerializer end private + def set_push_subscription + @push_subscription = ::Web::PushSubscription.find(params[:id]) + end + def subscription_params @subscription_params ||= params.require(:subscription).permit(:endpoint, keys: [:auth, :p256dh]) end def data_params - @data_params ||= params.require(:data).permit(alerts: [:follow, :follow_request, :favourite, :reblog, :mention, :poll, :status]) + @data_params ||= params.require(:data).permit(:policy, alerts: [:follow, :follow_request, :favourite, :reblog, :mention, :poll, :status]) end end diff --git a/app/models/web/push_subscription.rb b/app/models/web/push_subscription.rb index 7609b1bfc..6e46573ae 100644 --- a/app/models/web/push_subscription.rb +++ b/app/models/web/push_subscription.rb @@ -47,7 +47,7 @@ class Web::PushSubscription < ApplicationRecord end def pushable?(notification) - ActiveModel::Type::Boolean.new.cast(data&.dig('alerts', notification.type.to_s)) + policy_allows_notification?(notification) && alert_enabled_for_notification_type?(notification) end def associated_user @@ -100,4 +100,25 @@ class Web::PushSubscription < ApplicationRecord def contact_email @contact_email ||= ::Setting.site_contact_email end + + def alert_enabled_for_notification_type?(notification) + truthy?(data&.dig('alerts', notification.type.to_s)) + end + + def policy_allows_notification?(notification) + case data&.dig('policy') + when nil, 'all' + true + when 'none' + false + when 'followed' + notification.account.following?(notification.from_account) + when 'follower' + notification.from_account.following?(notification.account) + end + end + + def truthy?(val) + ActiveModel::Type::Boolean.new.cast(val) + end end diff --git a/spec/controllers/api/v1/push/subscriptions_controller_spec.rb b/spec/controllers/api/v1/push/subscriptions_controller_spec.rb index 01146294f..534d02879 100644 --- a/spec/controllers/api/v1/push/subscriptions_controller_spec.rb +++ b/spec/controllers/api/v1/push/subscriptions_controller_spec.rb @@ -27,20 +27,27 @@ describe Api::V1::Push::SubscriptionsController do let(:alerts_payload) do { data: { + policy: 'all', + alerts: { follow: true, + follow_request: true, favourite: false, reblog: true, mention: false, + poll: true, + status: false, } } }.with_indifferent_access end describe 'POST #create' do - it 'saves push subscriptions' do + before do post :create, params: create_payload + end + it 'saves push subscriptions' do push_subscription = Web::PushSubscription.find_by(endpoint: create_payload[:subscription][:endpoint]) expect(push_subscription.endpoint).to eq(create_payload[:subscription][:endpoint]) @@ -52,31 +59,34 @@ describe Api::V1::Push::SubscriptionsController do it 'replaces old subscription on repeat calls' do post :create, params: create_payload - post :create, params: create_payload - expect(Web::PushSubscription.where(endpoint: create_payload[:subscription][:endpoint]).count).to eq 1 end end describe 'PUT #update' do - it 'changes alert settings' do + before do post :create, params: create_payload put :update, params: alerts_payload + end + it 'changes alert settings' do push_subscription = Web::PushSubscription.find_by(endpoint: create_payload[:subscription][:endpoint]) - expect(push_subscription.data.dig('alerts', 'follow')).to eq(alerts_payload[:data][:alerts][:follow].to_s) - expect(push_subscription.data.dig('alerts', 'favourite')).to eq(alerts_payload[:data][:alerts][:favourite].to_s) - expect(push_subscription.data.dig('alerts', 'reblog')).to eq(alerts_payload[:data][:alerts][:reblog].to_s) - expect(push_subscription.data.dig('alerts', 'mention')).to eq(alerts_payload[:data][:alerts][:mention].to_s) + expect(push_subscription.data['policy']).to eq(alerts_payload[:data][:policy]) + + %w(follow follow_request favourite reblog mention poll status).each do |type| + expect(push_subscription.data['alerts'][type]).to eq(alerts_payload[:data][:alerts][type.to_sym].to_s) + end end end describe 'DELETE #destroy' do - it 'removes the subscription' do + before do post :create, params: create_payload delete :destroy + end + it 'removes the subscription' do expect(Web::PushSubscription.find_by(endpoint: create_payload[:subscription][:endpoint])).to be_nil end end diff --git a/spec/controllers/api/web/push_subscriptions_controller_spec.rb b/spec/controllers/api/web/push_subscriptions_controller_spec.rb index 381cdeab9..bda4a7661 100644 --- a/spec/controllers/api/web/push_subscriptions_controller_spec.rb +++ b/spec/controllers/api/web/push_subscriptions_controller_spec.rb @@ -22,11 +22,16 @@ describe Api::Web::PushSubscriptionsController do let(:alerts_payload) do { data: { + policy: 'all', + alerts: { follow: true, + follow_request: false, favourite: false, reblog: true, mention: false, + poll: true, + status: false, } } } @@ -59,10 +64,11 @@ describe Api::Web::PushSubscriptionsController do push_subscription = Web::PushSubscription.find_by(endpoint: create_payload[:subscription][:endpoint]) - expect(push_subscription.data['alerts']['follow']).to eq(alerts_payload[:data][:alerts][:follow].to_s) - expect(push_subscription.data['alerts']['favourite']).to eq(alerts_payload[:data][:alerts][:favourite].to_s) - expect(push_subscription.data['alerts']['reblog']).to eq(alerts_payload[:data][:alerts][:reblog].to_s) - expect(push_subscription.data['alerts']['mention']).to eq(alerts_payload[:data][:alerts][:mention].to_s) + expect(push_subscription.data['policy']).to eq 'all' + + %w(follow follow_request favourite reblog mention poll status).each do |type| + expect(push_subscription.data['alerts'][type]).to eq(alerts_payload[:data][:alerts][type.to_sym].to_s) + end end end end @@ -81,10 +87,11 @@ describe Api::Web::PushSubscriptionsController do push_subscription = Web::PushSubscription.find_by(endpoint: create_payload[:subscription][:endpoint]) - expect(push_subscription.data['alerts']['follow']).to eq(alerts_payload[:data][:alerts][:follow].to_s) - expect(push_subscription.data['alerts']['favourite']).to eq(alerts_payload[:data][:alerts][:favourite].to_s) - expect(push_subscription.data['alerts']['reblog']).to eq(alerts_payload[:data][:alerts][:reblog].to_s) - expect(push_subscription.data['alerts']['mention']).to eq(alerts_payload[:data][:alerts][:mention].to_s) + expect(push_subscription.data['policy']).to eq 'all' + + %w(follow follow_request favourite reblog mention poll status).each do |type| + expect(push_subscription.data['alerts'][type]).to eq(alerts_payload[:data][:alerts][type.to_sym].to_s) + end end end end diff --git a/spec/models/web/push_subscription_spec.rb b/spec/models/web/push_subscription_spec.rb index c6665611c..b44904369 100644 --- a/spec/models/web/push_subscription_spec.rb +++ b/spec/models/web/push_subscription_spec.rb @@ -1,16 +1,94 @@ require 'rails_helper' RSpec.describe Web::PushSubscription, type: :model do - let(:alerts) { { mention: true, reblog: false, follow: true, follow_request: false, favourite: true } } - let(:push_subscription) { Web::PushSubscription.new(data: { alerts: alerts }) } + let(:account) { Fabricate(:account) } + + let(:policy) { 'all' } + + let(:data) do + { + policy: policy, + + alerts: { + mention: true, + reblog: false, + follow: true, + follow_request: false, + favourite: true, + }, + } + end + + subject { described_class.new(data: data) } describe '#pushable?' do - it 'obeys alert settings' do - expect(push_subscription.send(:pushable?, Notification.new(activity_type: 'Mention'))).to eq true - expect(push_subscription.send(:pushable?, Notification.new(activity_type: 'Status'))).to eq false - expect(push_subscription.send(:pushable?, Notification.new(activity_type: 'Follow'))).to eq true - expect(push_subscription.send(:pushable?, Notification.new(activity_type: 'FollowRequest'))).to eq false - expect(push_subscription.send(:pushable?, Notification.new(activity_type: 'Favourite'))).to eq true + let(:notification_type) { :mention } + let(:notification) { Fabricate(:notification, account: account, type: notification_type) } + + %i(mention reblog follow follow_request favourite).each do |type| + context "when notification is a #{type}" do + let(:notification_type) { type } + + it "returns boolean corresonding to alert setting" do + expect(subject.pushable?(notification)).to eq data[:alerts][type] + end + end + end + + context 'when policy is all' do + let(:policy) { 'all' } + + it 'returns true' do + expect(subject.pushable?(notification)).to eq true + end + end + + context 'when policy is none' do + let(:policy) { 'none' } + + it 'returns false' do + expect(subject.pushable?(notification)).to eq false + end + end + + context 'when policy is followed' do + let(:policy) { 'followed' } + + context 'and notification is from someone you follow' do + before do + account.follow!(notification.from_account) + end + + it 'returns true' do + expect(subject.pushable?(notification)).to eq true + end + end + + context 'and notification is not from someone you follow' do + it 'returns false' do + expect(subject.pushable?(notification)).to eq false + end + end + end + + context 'when policy is follower' do + let(:policy) { 'follower' } + + context 'and notification is from someone who follows you' do + before do + notification.from_account.follow!(account) + end + + it 'returns true' do + expect(subject.pushable?(notification)).to eq true + end + end + + context 'and notification is not from someone who follows you' do + it 'returns false' do + expect(subject.pushable?(notification)).to eq false + end + end end end end -- cgit From b3ceb3dcc4df62803aa967d7aecee686973a8996 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sat, 17 Apr 2021 03:14:25 +0200 Subject: Add canonical e-mail blocks for suspended accounts (#16049) Prevent new accounts from being created using the same underlying e-mail as a suspended account using extensions and period permutations. Stores e-mails as a SHA256 hash --- app/helpers/email_helper.rb | 18 +++++++++ app/models/account.rb | 14 +++++++ app/models/canonical_email_block.rb | 27 +++++++++++++ app/validators/blacklisted_email_validator.rb | 30 +++++++++----- ...20210416200740_create_canonical_email_blocks.rb | 10 +++++ db/schema.rb | 12 +++++- .../canonical_email_block_fabricator.rb | 4 ++ spec/models/canonical_email_block_spec.rb | 47 ++++++++++++++++++++++ .../validators/blacklisted_email_validator_spec.rb | 29 +++++++++---- 9 files changed, 171 insertions(+), 20 deletions(-) create mode 100644 app/helpers/email_helper.rb create mode 100644 app/models/canonical_email_block.rb create mode 100644 db/migrate/20210416200740_create_canonical_email_blocks.rb create mode 100644 spec/fabricators/canonical_email_block_fabricator.rb create mode 100644 spec/models/canonical_email_block_spec.rb (limited to 'app/models') diff --git a/app/helpers/email_helper.rb b/app/helpers/email_helper.rb new file mode 100644 index 000000000..360783c62 --- /dev/null +++ b/app/helpers/email_helper.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module EmailHelper + def self.included(base) + base.extend(self) + end + + def email_to_canonical_email(str) + username, domain = str.downcase.split('@', 2) + username, = username.gsub('.', '').split('+', 2) + + "#{username}@#{domain}" + end + + def email_to_canonical_email_hash(str) + Digest::SHA2.new(256).hexdigest(email_to_canonical_email(str)) + end +end diff --git a/app/models/account.rb b/app/models/account.rb index 80689d4aa..a573365de 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -235,6 +235,7 @@ class Account < ApplicationRecord transaction do create_deletion_request! update!(suspended_at: date, suspension_origin: origin) + create_canonical_email_block! end end @@ -242,6 +243,7 @@ class Account < ApplicationRecord transaction do deletion_request&.destroy! update!(suspended_at: nil, suspension_origin: nil) + destroy_canonical_email_block! end end @@ -569,4 +571,16 @@ class Account < ApplicationRecord def clean_feed_manager FeedManager.instance.clean_feeds!(:home, [id]) end + + def create_canonical_email_block! + return unless local? && user_email.present? + + CanonicalEmailBlock.create(reference_account: self, email: user_email) + end + + def destroy_canonical_email_block! + return unless local? + + CanonicalEmailBlock.where(reference_account: self).delete_all + end end diff --git a/app/models/canonical_email_block.rb b/app/models/canonical_email_block.rb new file mode 100644 index 000000000..a8546d65a --- /dev/null +++ b/app/models/canonical_email_block.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true +# == Schema Information +# +# Table name: canonical_email_blocks +# +# id :bigint(8) not null, primary key +# canonical_email_hash :string default(""), not null +# reference_account_id :bigint(8) not null +# created_at :datetime not null +# updated_at :datetime not null +# + +class CanonicalEmailBlock < ApplicationRecord + include EmailHelper + + belongs_to :reference_account, class_name: 'Account' + + validates :canonical_email_hash, presence: true + + def email=(email) + self.canonical_email_hash = email_to_canonical_email_hash(email) + end + + def self.block?(email) + where(canonical_email_hash: email_to_canonical_email_hash(email)).exists? + end +end diff --git a/app/validators/blacklisted_email_validator.rb b/app/validators/blacklisted_email_validator.rb index 1ca73fdcc..eb66ad93d 100644 --- a/app/validators/blacklisted_email_validator.rb +++ b/app/validators/blacklisted_email_validator.rb @@ -6,26 +6,25 @@ class BlacklistedEmailValidator < ActiveModel::Validator @email = user.email - user.errors.add(:email, :blocked) if blocked_email? + user.errors.add(:email, :blocked) if blocked_email_provider? + user.errors.add(:email, :taken) if blocked_canonical_email? end private - def blocked_email? - on_blacklist? || not_on_whitelist? + def blocked_email_provider? + disallowed_through_email_domain_block? || disallowed_through_configuration? || not_allowed_through_configuration? end - def on_blacklist? - return true if EmailDomainBlock.block?(@email) - return false if Rails.configuration.x.email_domains_blacklist.blank? - - domains = Rails.configuration.x.email_domains_blacklist.gsub('.', '\.') - regexp = Regexp.new("@(.+\\.)?(#{domains})", true) + def blocked_canonical_email? + CanonicalEmailBlock.block?(@email) + end - regexp.match?(@email) + def disallowed_through_email_domain_block? + EmailDomainBlock.block?(@email) end - def not_on_whitelist? + def not_allowed_through_configuration? return false if Rails.configuration.x.email_domains_whitelist.blank? domains = Rails.configuration.x.email_domains_whitelist.gsub('.', '\.') @@ -33,4 +32,13 @@ class BlacklistedEmailValidator < ActiveModel::Validator @email !~ regexp end + + def disallowed_through_configuration? + return false if Rails.configuration.x.email_domains_blacklist.blank? + + domains = Rails.configuration.x.email_domains_blacklist.gsub('.', '\.') + regexp = Regexp.new("@(.+\\.)?(#{domains})", true) + + regexp.match?(@email) + end end diff --git a/db/migrate/20210416200740_create_canonical_email_blocks.rb b/db/migrate/20210416200740_create_canonical_email_blocks.rb new file mode 100644 index 000000000..a1f1660bf --- /dev/null +++ b/db/migrate/20210416200740_create_canonical_email_blocks.rb @@ -0,0 +1,10 @@ +class CreateCanonicalEmailBlocks < ActiveRecord::Migration[6.1] + def change + create_table :canonical_email_blocks do |t| + t.string :canonical_email_hash, null: false, default: '', index: { unique: true } + t.belongs_to :reference_account, null: false, foreign_key: { on_cascade: :delete, to_table: 'accounts' } + + t.timestamps + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 599cd873b..dcbbf4aea 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: 2021_03_24_171613) do +ActiveRecord::Schema.define(version: 2021_04_16_200740) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -280,6 +280,15 @@ ActiveRecord::Schema.define(version: 2021_03_24_171613) do t.index ["status_id"], name: "index_bookmarks_on_status_id" end + create_table "canonical_email_blocks", force: :cascade do |t| + t.string "canonical_email_hash", default: "", null: false + t.bigint "reference_account_id", null: false + t.datetime "created_at", precision: 6, null: false + t.datetime "updated_at", precision: 6, null: false + t.index ["canonical_email_hash"], name: "index_canonical_email_blocks_on_canonical_email_hash", unique: true + t.index ["reference_account_id"], name: "index_canonical_email_blocks_on_reference_account_id" + end + create_table "conversation_mutes", force: :cascade do |t| t.bigint "conversation_id", null: false t.bigint "account_id", null: false @@ -991,6 +1000,7 @@ ActiveRecord::Schema.define(version: 2021_03_24_171613) do add_foreign_key "blocks", "accounts", name: "fk_4269e03e65", on_delete: :cascade add_foreign_key "bookmarks", "accounts", on_delete: :cascade add_foreign_key "bookmarks", "statuses", on_delete: :cascade + add_foreign_key "canonical_email_blocks", "accounts", column: "reference_account_id" add_foreign_key "conversation_mutes", "accounts", name: "fk_225b4212bb", on_delete: :cascade add_foreign_key "conversation_mutes", "conversations", on_delete: :cascade add_foreign_key "custom_filters", "accounts", on_delete: :cascade diff --git a/spec/fabricators/canonical_email_block_fabricator.rb b/spec/fabricators/canonical_email_block_fabricator.rb new file mode 100644 index 000000000..a0b6e0d22 --- /dev/null +++ b/spec/fabricators/canonical_email_block_fabricator.rb @@ -0,0 +1,4 @@ +Fabricator(:canonical_email_block) do + email "test@example.com" + reference_account { Fabricate(:account) } +end diff --git a/spec/models/canonical_email_block_spec.rb b/spec/models/canonical_email_block_spec.rb new file mode 100644 index 000000000..8e0050d65 --- /dev/null +++ b/spec/models/canonical_email_block_spec.rb @@ -0,0 +1,47 @@ +require 'rails_helper' + +RSpec.describe CanonicalEmailBlock, type: :model do + describe '#email=' do + let(:target_hash) { '973dfe463ec85785f5f95af5ba3906eedb2d931c24e69824a89ea65dba4e813b' } + + it 'sets canonical_email_hash' do + subject.email = 'test@example.com' + expect(subject.canonical_email_hash).to eq target_hash + end + + it 'sets the same hash even with dot permutations' do + subject.email = 't.e.s.t@example.com' + expect(subject.canonical_email_hash).to eq target_hash + end + + it 'sets the same hash even with extensions' do + subject.email = 'test+mastodon1@example.com' + expect(subject.canonical_email_hash).to eq target_hash + end + + it 'sets the same hash with different casing' do + subject.email = 'Test@EXAMPLE.com' + expect(subject.canonical_email_hash).to eq target_hash + end + end + + describe '.block?' do + let!(:canonical_email_block) { Fabricate(:canonical_email_block, email: 'foo@bar.com') } + + it 'returns true for the same email' do + expect(described_class.block?('foo@bar.com')).to be true + end + + it 'returns true for the same email with dots' do + expect(described_class.block?('f.oo@bar.com')).to be true + end + + it 'returns true for the same email with extensions' do + expect(described_class.block?('foo+spam@bar.com')).to be true + end + + it 'returns false for different email' do + expect(described_class.block?('hoge@bar.com')).to be false + end + end +end diff --git a/spec/validators/blacklisted_email_validator_spec.rb b/spec/validators/blacklisted_email_validator_spec.rb index 53b355a57..f7d5e01bc 100644 --- a/spec/validators/blacklisted_email_validator_spec.rb +++ b/spec/validators/blacklisted_email_validator_spec.rb @@ -9,23 +9,36 @@ RSpec.describe BlacklistedEmailValidator, type: :validator do before do allow(user).to receive(:valid_invitation?) { false } - allow_any_instance_of(described_class).to receive(:blocked_email?) { blocked_email } - described_class.new.validate(user) + allow_any_instance_of(described_class).to receive(:blocked_email_provider?) { blocked_email } end - context 'blocked_email?' do + subject { described_class.new.validate(user); errors } + + context 'when e-mail provider is blocked' do let(:blocked_email) { true } - it 'calls errors.add' do - expect(errors).to have_received(:add).with(:email, :blocked) + it 'adds error' do + expect(subject).to have_received(:add).with(:email, :blocked) end end - context '!blocked_email?' do + context 'when e-mail provider is not blocked' do let(:blocked_email) { false } - it 'not calls errors.add' do - expect(errors).not_to have_received(:add).with(:email, :blocked) + it 'does not add errors' do + expect(subject).not_to have_received(:add).with(:email, :blocked) + end + + context 'when canonical e-mail is blocked' do + let(:other_user) { Fabricate(:user, email: 'i.n.f.o@mail.com') } + + before do + other_user.account.suspend! + end + + it 'adds error' do + expect(subject).to have_received(:add).with(:email, :taken) + end end end end -- cgit