diff options
| -rw-r--r-- | app/controllers/api/v1/accounts_controller.rb | 10 | ||||
| -rw-r--r-- | app/models/account.rb | 9 | ||||
| -rw-r--r-- | app/services/process_mentions_service.rb | 3 | ||||
| -rw-r--r-- | spec/models/account_spec.rb | 178 |
4 files changed, 194 insertions, 6 deletions
diff --git a/app/controllers/api/v1/accounts_controller.rb b/app/controllers/api/v1/accounts_controller.rb index 5134bfb94..5537cc9b0 100644 --- a/app/controllers/api/v1/accounts_controller.rb +++ b/app/controllers/api/v1/accounts_controller.rb @@ -9,6 +9,8 @@ class Api::V1::AccountsController < Api::BaseController before_action :require_user!, except: [:show, :create] before_action :set_account, except: [:create] + before_action :check_account_approval, except: [:create] + before_action :check_account_confirmation, except: [:create] before_action :check_enabled_registrations, only: [:create] skip_before_action :require_authenticated_user!, only: :create @@ -74,6 +76,14 @@ class Api::V1::AccountsController < Api::BaseController @account = Account.find(params[:id]) end + def check_account_approval + raise(ActiveRecord::RecordNotFound) if @account.local? && @account.user_pending? + end + + def check_account_confirmation + raise(ActiveRecord::RecordNotFound) if @account.local? && !@account.user_confirmed? + end + def relationships(**options) AccountRelationshipsPresenter.new([@account.id], current_user.account_id, **options) end diff --git a/app/models/account.rb b/app/models/account.rb index 7b460b054..bd94142c4 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -109,7 +109,8 @@ class Account < ApplicationRecord scope :matches_username, ->(value) { where(arel_table[:username].matches("#{value}%")) } scope :matches_display_name, ->(value) { where(arel_table[:display_name].matches("#{value}%")) } scope :matches_domain, ->(value) { where(arel_table[:domain].matches("%#{value}%")) } - scope :searchable, -> { without_suspended.where(moved_to_account_id: nil) } + scope :without_unapproved, -> { left_outer_joins(:user).remote.or(left_outer_joins(:user).merge(User.approved.confirmed)) } + scope :searchable, -> { without_unapproved.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 :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')) } @@ -193,7 +194,7 @@ class Account < ApplicationRecord end def searchable? - !(suspended? || moved?) + !(suspended? || moved?) && (!local? || (approved? && confirmed?)) end def possibly_stale? @@ -461,9 +462,11 @@ class Account < ApplicationRecord accounts.*, ts_rank_cd(#{TEXTSEARCH}, to_tsquery('simple', :tsquery), 32) AS rank FROM accounts + LEFT JOIN users ON accounts.id = users.account_id WHERE to_tsquery('simple', :tsquery) @@ #{TEXTSEARCH} AND accounts.suspended_at IS NULL AND accounts.moved_to_account_id IS NULL + AND (accounts.domain IS NOT NULL OR (users.approved = TRUE AND users.confirmed_at IS NOT NULL)) ORDER BY rank DESC LIMIT :limit OFFSET :offset SQL @@ -539,9 +542,11 @@ class Account < ApplicationRecord (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) + LEFT JOIN users ON accounts.id = users.account_id WHERE to_tsquery('simple', :tsquery) @@ #{TEXTSEARCH} AND accounts.suspended_at IS NULL AND accounts.moved_to_account_id IS NULL + AND (accounts.domain IS NOT NULL OR (users.approved = TRUE AND users.confirmed_at IS NOT NULL)) GROUP BY accounts.id ORDER BY rank DESC LIMIT :limit OFFSET :offset diff --git a/app/services/process_mentions_service.rb b/app/services/process_mentions_service.rb index 9d239fc65..8c63b611d 100644 --- a/app/services/process_mentions_service.rb +++ b/app/services/process_mentions_service.rb @@ -37,6 +37,9 @@ class ProcessMentionsService < BaseService mentioned_account = Account.find_remote(username, domain) + # Unapproved and unconfirmed accounts should not be mentionable + next if mentioned_account&.local? && !(mentioned_account.user_confirmed? && mentioned_account.user_approved?) + # If the account cannot be found or isn't the right protocol, # first try to resolve it if mention_undeliverable?(mentioned_account) diff --git a/spec/models/account_spec.rb b/spec/models/account_spec.rb index 681134d49..dc0ca3da3 100644 --- a/spec/models/account_spec.rb +++ b/spec/models/account_spec.rb @@ -350,6 +350,45 @@ RSpec.describe Account, type: :model do ) end + it 'does not return suspended users' do + match = Fabricate( + :account, + display_name: 'Display Name', + username: 'username', + domain: 'example.com', + suspended: true + ) + + results = Account.search_for('username') + expect(results).to eq [] + end + + it 'does not return unapproved users' do + match = Fabricate( + :account, + display_name: 'Display Name', + username: 'username' + ) + + match.user.update(approved: false) + + results = Account.search_for('username') + expect(results).to eq [] + end + + it 'does not return unconfirmed users' do + match = Fabricate( + :account, + display_name: 'Display Name', + username: 'username' + ) + + match.user.update(confirmed_at: nil) + + results = Account.search_for('username') + expect(results).to eq [] + end + it 'accepts ?, \, : and space as delimiter' do match = Fabricate( :account, @@ -422,8 +461,114 @@ RSpec.describe Account, type: :model do end describe '.advanced_search_for' do + let(:account) { Fabricate(:account) } + + context 'when limiting search to followed accounts' do + it 'accepts ?, \, : and space as delimiter' do + match = Fabricate( + :account, + display_name: 'A & l & i & c & e', + username: 'username', + domain: 'example.com' + ) + account.follow!(match) + + results = Account.advanced_search_for('A?l\i:c e', account, 10, true) + expect(results).to eq [match] + end + + it 'does not return non-followed accounts' do + match = Fabricate( + :account, + display_name: 'A & l & i & c & e', + username: 'username', + domain: 'example.com' + ) + + results = Account.advanced_search_for('A?l\i:c e', account, 10, true) + expect(results).to eq [] + end + + it 'does not return suspended users' do + match = Fabricate( + :account, + display_name: 'Display Name', + username: 'username', + domain: 'example.com', + suspended: true + ) + + results = Account.advanced_search_for('username', account, 10, true) + expect(results).to eq [] + end + + it 'does not return unapproved users' do + match = Fabricate( + :account, + display_name: 'Display Name', + username: 'username' + ) + + match.user.update(approved: false) + + results = Account.advanced_search_for('username', account, 10, true) + expect(results).to eq [] + end + + it 'does not return unconfirmed users' do + match = Fabricate( + :account, + display_name: 'Display Name', + username: 'username' + ) + + match.user.update(confirmed_at: nil) + + results = Account.advanced_search_for('username', account, 10, true) + expect(results).to eq [] + end + end + + it 'does not return suspended users' do + match = Fabricate( + :account, + display_name: 'Display Name', + username: 'username', + domain: 'example.com', + suspended: true + ) + + results = Account.advanced_search_for('username', account) + expect(results).to eq [] + end + + it 'does not return unapproved users' do + match = Fabricate( + :account, + display_name: 'Display Name', + username: 'username' + ) + + match.user.update(approved: false) + + results = Account.advanced_search_for('username', account) + expect(results).to eq [] + end + + it 'does not return unconfirmed users' do + match = Fabricate( + :account, + display_name: 'Display Name', + username: 'username' + ) + + match.user.update(confirmed_at: nil) + + results = Account.advanced_search_for('username', account) + expect(results).to eq [] + end + it 'accepts ?, \, : and space as delimiter' do - account = Fabricate(:account) match = Fabricate( :account, display_name: 'A & l & i & c & e', @@ -437,18 +582,17 @@ RSpec.describe Account, type: :model do it 'limits by 10 by default' do 11.times { Fabricate(:account, display_name: "Display Name") } - results = Account.search_for("display") + results = Account.advanced_search_for("display", account) expect(results.size).to eq 10 end it 'accepts arbitrary limits' do 2.times { Fabricate(:account, display_name: "Display Name") } - results = Account.search_for("display", 1) + results = Account.advanced_search_for("display", account, 1) expect(results.size).to eq 1 end it 'ranks followed accounts higher' do - account = Fabricate(:account) match = Fabricate(:account, username: "Matching") followed_match = Fabricate(:account, username: "Matcher") Fabricate(:follow, account: account, target_account: followed_match) @@ -775,6 +919,32 @@ RSpec.describe Account, type: :model do expect(Account.suspended).to match_array([account_1]) end end + + describe 'searchable' do + let!(:suspended_local) { Fabricate(:account, suspended: true, username: 'suspended_local') } + let!(:suspended_remote) { Fabricate(:account, suspended: true, domain: 'example.org', username: 'suspended_remote') } + let!(:silenced_local) { Fabricate(:account, silenced: true, username: 'silenced_local') } + let!(:silenced_remote) { Fabricate(:account, silenced: true, domain: 'example.org', username: 'silenced_remote') } + let!(:unconfirmed) { Fabricate(:user, confirmed_at: nil).account } + let!(:unapproved) { Fabricate(:user, approved: false).account } + let!(:unconfirmed_unapproved) { Fabricate(:user, confirmed_at: nil, approved: false).account } + let!(:local_account) { Fabricate(:account, username: 'local_account') } + let!(:remote_account) { Fabricate(:account, domain: 'example.org', username: 'remote_account') } + + before do + # Accounts get automatically-approved depending on settings, so ensure they aren't approved + unapproved.user.update(approved: false) + unconfirmed_unapproved.user.update(approved: false) + end + + it 'returns every usable non-suspended account' do + expect(Account.searchable).to match_array([silenced_local, silenced_remote, local_account, remote_account]) + end + + it 'does not mess with previously-applied scopes' do + expect(Account.where.not(id: remote_account.id).searchable).to match_array([silenced_local, silenced_remote, local_account]) + end + end end context 'when is local' do |