From 33f06a4ae72176623c57ad3b203aac999ebb2b93 Mon Sep 17 00:00:00 2001 From: Jed Fox Date: Tue, 6 Dec 2022 17:54:02 -0500 Subject: Fix the top border of verified account fields (#22006) --- app/javascript/styles/mastodon/components.scss | 2 ++ 1 file changed, 2 insertions(+) (limited to 'app') diff --git a/app/javascript/styles/mastodon/components.scss b/app/javascript/styles/mastodon/components.scss index 45b1ac501..79a9a4bd7 100644 --- a/app/javascript/styles/mastodon/components.scss +++ b/app/javascript/styles/mastodon/components.scss @@ -7138,10 +7138,12 @@ noscript { .verified { border: 1px solid rgba($valid-value-color, 0.5); + margin-top: -1px; &:first-child { border-top-left-radius: 4px; border-top-right-radius: 4px; + margin-top: 0; } &:last-child { -- cgit From 69137f4a90874442cc5fefdf86dad7c4a4884bdc Mon Sep 17 00:00:00 2001 From: Claire Date: Wed, 7 Dec 2022 00:10:53 +0100 Subject: Fix irreversible and whole_word parameters handling in /api/v1/filters (#21988) Fixes #21965 --- app/controllers/api/v1/filters_controller.rb | 6 ++--- app/models/custom_filter.rb | 2 +- spec/controllers/api/v1/filters_controller_spec.rb | 26 +++++++++++++++++++--- 3 files changed, 27 insertions(+), 7 deletions(-) (limited to 'app') diff --git a/app/controllers/api/v1/filters_controller.rb b/app/controllers/api/v1/filters_controller.rb index 149139b40..772791b25 100644 --- a/app/controllers/api/v1/filters_controller.rb +++ b/app/controllers/api/v1/filters_controller.rb @@ -13,7 +13,7 @@ class Api::V1::FiltersController < Api::BaseController def create ApplicationRecord.transaction do - filter_category = current_account.custom_filters.create!(resource_params) + filter_category = current_account.custom_filters.create!(filter_params) @filter = filter_category.keywords.create!(keyword_params) end @@ -52,11 +52,11 @@ class Api::V1::FiltersController < Api::BaseController end def resource_params - params.permit(:phrase, :expires_in, :irreversible, context: []) + params.permit(:phrase, :expires_in, :irreversible, :whole_word, context: []) end def filter_params - resource_params.slice(:expires_in, :irreversible, :context) + resource_params.slice(:phrase, :expires_in, :irreversible, :context) end def keyword_params diff --git a/app/models/custom_filter.rb b/app/models/custom_filter.rb index da2a91493..5a4a974be 100644 --- a/app/models/custom_filter.rb +++ b/app/models/custom_filter.rb @@ -54,7 +54,7 @@ class CustomFilter < ApplicationRecord end def irreversible=(value) - self.action = value ? :hide : :warn + self.action = ActiveModel::Type::Boolean.new.cast(value) ? :hide : :warn end def irreversible? diff --git a/spec/controllers/api/v1/filters_controller_spec.rb b/spec/controllers/api/v1/filters_controller_spec.rb index af1951f0b..8acb46a00 100644 --- a/spec/controllers/api/v1/filters_controller_spec.rb +++ b/spec/controllers/api/v1/filters_controller_spec.rb @@ -22,9 +22,11 @@ RSpec.describe Api::V1::FiltersController, type: :controller do describe 'POST #create' do let(:scopes) { 'write:filters' } + let(:irreversible) { true } + let(:whole_word) { false } before do - post :create, params: { phrase: 'magic', context: %w(home), irreversible: true } + post :create, params: { phrase: 'magic', context: %w(home), irreversible: irreversible, whole_word: whole_word } end it 'returns http success' do @@ -34,11 +36,29 @@ RSpec.describe Api::V1::FiltersController, type: :controller do it 'creates a filter' do filter = user.account.custom_filters.first expect(filter).to_not be_nil - expect(filter.keywords.pluck(:keyword)).to eq ['magic'] + expect(filter.keywords.pluck(:keyword, :whole_word)).to eq [['magic', whole_word]] expect(filter.context).to eq %w(home) - expect(filter.irreversible?).to be true + expect(filter.irreversible?).to be irreversible expect(filter.expires_at).to be_nil end + + context 'with different parameters' do + let(:irreversible) { false } + let(:whole_word) { true } + + it 'returns http success' do + expect(response).to have_http_status(200) + end + + it 'creates a filter' do + filter = user.account.custom_filters.first + expect(filter).to_not be_nil + expect(filter.keywords.pluck(:keyword, :whole_word)).to eq [['magic', whole_word]] + expect(filter.context).to eq %w(home) + expect(filter.irreversible?).to be irreversible + expect(filter.expires_at).to be_nil + end + end end describe 'GET #show' do -- cgit From c8849d6ceecfdb9c18284fcc57a7e29019b4cd05 Mon Sep 17 00:00:00 2001 From: Claire Date: Wed, 7 Dec 2022 00:15:24 +0100 Subject: Fix unbounded recursion in account discovery (#22025) * Fix trying to fetch posts from other users when fetching featured posts * Rate-limit discovery of new subdomains * Put a limit on recursively discovering new accounts --- Gemfile | 1 + Gemfile.lock | 1 + app/lib/activitypub/activity/create.rb | 2 +- app/lib/activitypub/activity/update.rb | 4 +- app/models/concerns/domain_materializable.rb | 15 +++- .../fetch_featured_collection_service.rb | 4 +- .../activitypub/fetch_remote_account_service.rb | 2 +- .../activitypub/fetch_remote_actor_service.rb | 4 +- .../activitypub/fetch_remote_status_service.rb | 8 +- .../activitypub/process_account_service.rb | 25 +++++- .../activitypub/process_status_update_service.rb | 5 +- .../activitypub/process_account_service_spec.rb | 94 ++++++++++++++++++++++ 12 files changed, 147 insertions(+), 18 deletions(-) (limited to 'app') diff --git a/Gemfile b/Gemfile index acb1ac53c..a399f5102 100644 --- a/Gemfile +++ b/Gemfile @@ -66,6 +66,7 @@ gem 'oj', '~> 3.13' gem 'ox', '~> 2.14' gem 'parslet' gem 'posix-spawn' +gem 'public_suffix', '~> 5.0' gem 'pundit', '~> 2.2' gem 'premailer-rails' gem 'rack-attack', '~> 6.6' diff --git a/Gemfile.lock b/Gemfile.lock index 390f8d9f0..f7d87d2a6 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -819,6 +819,7 @@ DEPENDENCIES private_address_check (~> 0.5) pry-byebug (~> 3.10) pry-rails (~> 0.3) + public_suffix (~> 5.0) puma (~> 5.6) pundit (~> 2.2) rack (~> 2.2.4) diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb index 73882e134..b15e66ca2 100644 --- a/app/lib/activitypub/activity/create.rb +++ b/app/lib/activitypub/activity/create.rb @@ -222,7 +222,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity return if tag['href'].blank? account = account_from_uri(tag['href']) - account = ActivityPub::FetchRemoteAccountService.new.call(tag['href']) if account.nil? + account = ActivityPub::FetchRemoteAccountService.new.call(tag['href'], request_id: @options[:request_id]) if account.nil? return if account.nil? diff --git a/app/lib/activitypub/activity/update.rb b/app/lib/activitypub/activity/update.rb index 5b3238ece..e7c3bc9bf 100644 --- a/app/lib/activitypub/activity/update.rb +++ b/app/lib/activitypub/activity/update.rb @@ -18,7 +18,7 @@ class ActivityPub::Activity::Update < ActivityPub::Activity def update_account return reject_payload! if @account.uri != object_uri - ActivityPub::ProcessAccountService.new.call(@account.username, @account.domain, @object, signed_with_known_key: true) + ActivityPub::ProcessAccountService.new.call(@account.username, @account.domain, @object, signed_with_known_key: true, request_id: @options[:request_id]) end def update_status @@ -28,6 +28,6 @@ class ActivityPub::Activity::Update < ActivityPub::Activity return if @status.nil? - ActivityPub::ProcessStatusUpdateService.new.call(@status, @object) + ActivityPub::ProcessStatusUpdateService.new.call(@status, @object, request_id: @options[:request_id]) end end diff --git a/app/models/concerns/domain_materializable.rb b/app/models/concerns/domain_materializable.rb index 88337f8c0..0eac6878e 100644 --- a/app/models/concerns/domain_materializable.rb +++ b/app/models/concerns/domain_materializable.rb @@ -3,11 +3,24 @@ module DomainMaterializable extend ActiveSupport::Concern + include Redisable + included do after_create_commit :refresh_instances_view end def refresh_instances_view - Instance.refresh unless domain.nil? || Instance.where(domain: domain).exists? + return if domain.nil? || Instance.exists?(domain: domain) + + Instance.refresh + count_unique_subdomains! + end + + def count_unique_subdomains! + second_and_top_level_domain = PublicSuffix.domain(domain, ignore_private: true) + with_redis do |redis| + redis.pfadd("unique_subdomains_for:#{second_and_top_level_domain}", domain) + redis.expire("unique_subdomains_for:#{second_and_top_level_domain}", 1.minute.seconds) + end end end diff --git a/app/services/activitypub/fetch_featured_collection_service.rb b/app/services/activitypub/fetch_featured_collection_service.rb index 50a187ad9..a746ef4d6 100644 --- a/app/services/activitypub/fetch_featured_collection_service.rb +++ b/app/services/activitypub/fetch_featured_collection_service.rb @@ -46,9 +46,9 @@ class ActivityPub::FetchFeaturedCollectionService < BaseService next unless item.is_a?(String) || item['type'] == 'Note' uri = value_or_id(item) - next if ActivityPub::TagManager.instance.local_uri?(uri) + next if ActivityPub::TagManager.instance.local_uri?(uri) || invalid_origin?(uri) - status = ActivityPub::FetchRemoteStatusService.new.call(uri, on_behalf_of: local_follower) + status = ActivityPub::FetchRemoteStatusService.new.call(uri, on_behalf_of: local_follower, expected_actor_uri: @account.uri, request_id: @options[:request_id]) next unless status&.account_id == @account.id status.id diff --git a/app/services/activitypub/fetch_remote_account_service.rb b/app/services/activitypub/fetch_remote_account_service.rb index ca7a8c6ca..7aba8269e 100644 --- a/app/services/activitypub/fetch_remote_account_service.rb +++ b/app/services/activitypub/fetch_remote_account_service.rb @@ -2,7 +2,7 @@ class ActivityPub::FetchRemoteAccountService < ActivityPub::FetchRemoteActorService # Does a WebFinger roundtrip on each call, unless `only_key` is true - def call(uri, id: true, prefetched_body: nil, break_on_redirect: false, only_key: false, suppress_errors: true) + def call(uri, id: true, prefetched_body: nil, break_on_redirect: false, only_key: false, suppress_errors: true, request_id: nil) actor = super return actor if actor.nil? || actor.is_a?(Account) diff --git a/app/services/activitypub/fetch_remote_actor_service.rb b/app/services/activitypub/fetch_remote_actor_service.rb index db09c38d8..a25fa54c4 100644 --- a/app/services/activitypub/fetch_remote_actor_service.rb +++ b/app/services/activitypub/fetch_remote_actor_service.rb @@ -10,7 +10,7 @@ class ActivityPub::FetchRemoteActorService < BaseService SUPPORTED_TYPES = %w(Application Group Organization Person Service).freeze # Does a WebFinger roundtrip on each call, unless `only_key` is true - def call(uri, id: true, prefetched_body: nil, break_on_redirect: false, only_key: false, suppress_errors: true) + def call(uri, id: true, prefetched_body: nil, break_on_redirect: false, only_key: false, suppress_errors: true, request_id: nil) return if domain_not_allowed?(uri) return ActivityPub::TagManager.instance.uri_to_actor(uri) if ActivityPub::TagManager.instance.local_uri?(uri) @@ -35,7 +35,7 @@ class ActivityPub::FetchRemoteActorService < BaseService check_webfinger! unless only_key - ActivityPub::ProcessAccountService.new.call(@username, @domain, @json, only_key: only_key, verified_webfinger: !only_key) + ActivityPub::ProcessAccountService.new.call(@username, @domain, @json, only_key: only_key, verified_webfinger: !only_key, request_id: request_id) rescue Error => e Rails.logger.debug "Fetching actor #{uri} failed: #{e.message}" raise unless suppress_errors diff --git a/app/services/activitypub/fetch_remote_status_service.rb b/app/services/activitypub/fetch_remote_status_service.rb index 803098245..21b9242f8 100644 --- a/app/services/activitypub/fetch_remote_status_service.rb +++ b/app/services/activitypub/fetch_remote_status_service.rb @@ -4,7 +4,8 @@ class ActivityPub::FetchRemoteStatusService < BaseService include JsonLdHelper # Should be called when uri has already been checked for locality - def call(uri, id: true, prefetched_body: nil, on_behalf_of: nil) + def call(uri, id: true, prefetched_body: nil, on_behalf_of: nil, expected_actor_uri: nil, request_id: nil) + @request_id = request_id @json = begin if prefetched_body.nil? fetch_resource(uri, id, on_behalf_of) @@ -30,6 +31,7 @@ class ActivityPub::FetchRemoteStatusService < BaseService end return if activity_json.nil? || object_uri.nil? || !trustworthy_attribution?(@json['id'], actor_uri) + return if expected_actor_uri.present? && actor_uri != expected_actor_uri return ActivityPub::TagManager.instance.uri_to_resource(object_uri, Status) if ActivityPub::TagManager.instance.local_uri?(object_uri) actor = account_from_uri(actor_uri) @@ -40,7 +42,7 @@ class ActivityPub::FetchRemoteStatusService < BaseService # activity as an update rather than create activity_json['type'] = 'Update' if equals_or_includes_any?(activity_json['type'], %w(Create)) && Status.where(uri: object_uri, account_id: actor.id).exists? - ActivityPub::Activity.factory(activity_json, actor).perform + ActivityPub::Activity.factory(activity_json, actor, request_id: request_id).perform end private @@ -52,7 +54,7 @@ class ActivityPub::FetchRemoteStatusService < BaseService def account_from_uri(uri) actor = ActivityPub::TagManager.instance.uri_to_resource(uri, Account) - actor = ActivityPub::FetchRemoteAccountService.new.call(uri, id: true) if actor.nil? || actor.possibly_stale? + actor = ActivityPub::FetchRemoteAccountService.new.call(uri, id: true, request_id: @request_id) if actor.nil? || actor.possibly_stale? actor end diff --git a/app/services/activitypub/process_account_service.rb b/app/services/activitypub/process_account_service.rb index 99bcb3835..2da9096c7 100644 --- a/app/services/activitypub/process_account_service.rb +++ b/app/services/activitypub/process_account_service.rb @@ -6,6 +6,9 @@ class ActivityPub::ProcessAccountService < BaseService include Redisable include Lockable + SUBDOMAINS_RATELIMIT = 10 + DISCOVERIES_PER_REQUEST = 400 + # Should be called with confirmed valid JSON # and WebFinger-resolved username and domain def call(username, domain, json, options = {}) @@ -15,9 +18,12 @@ class ActivityPub::ProcessAccountService < BaseService @json = json @uri = @json['id'] @username = username - @domain = domain + @domain = TagManager.instance.normalize_domain(domain) @collections = {} + # The key does not need to be unguessable, it just needs to be somewhat unique + @options[:request_id] ||= "#{Time.now.utc.to_i}-#{username}@#{domain}" + with_lock("process_account:#{@uri}") do @account = Account.remote.find_by(uri: @uri) if @options[:only_key] @account ||= Account.find_remote(@username, @domain) @@ -25,7 +31,18 @@ class ActivityPub::ProcessAccountService < BaseService @old_protocol = @account&.protocol @suspension_changed = false - create_account if @account.nil? + if @account.nil? + with_redis do |redis| + return nil if redis.pfcount("unique_subdomains_for:#{PublicSuffix.domain(@domain, ignore_private: true)}") >= SUBDOMAINS_RATELIMIT + + discoveries = redis.incr("discovery_per_request:#{@options[:request_id]}") + redis.expire("discovery_per_request:#{@options[:request_id]}", 5.minutes.seconds) + return nil if discoveries > DISCOVERIES_PER_REQUEST + end + + create_account + end + update_account process_tags @@ -150,7 +167,7 @@ class ActivityPub::ProcessAccountService < BaseService end def check_featured_collection! - ActivityPub::SynchronizeFeaturedCollectionWorker.perform_async(@account.id, { 'hashtag' => @json['featuredTags'].blank? }) + ActivityPub::SynchronizeFeaturedCollectionWorker.perform_async(@account.id, { 'hashtag' => @json['featuredTags'].blank?, 'request_id' => @options[:request_id] }) end def check_featured_tags_collection! @@ -254,7 +271,7 @@ class ActivityPub::ProcessAccountService < BaseService def moved_account account = ActivityPub::TagManager.instance.uri_to_resource(@json['movedTo'], Account) - account ||= ActivityPub::FetchRemoteAccountService.new.call(@json['movedTo'], id: true, break_on_redirect: true) + account ||= ActivityPub::FetchRemoteAccountService.new.call(@json['movedTo'], id: true, break_on_redirect: true, request_id: @options[:request_id]) account end diff --git a/app/services/activitypub/process_status_update_service.rb b/app/services/activitypub/process_status_update_service.rb index a0605b1a3..fad19f87f 100644 --- a/app/services/activitypub/process_status_update_service.rb +++ b/app/services/activitypub/process_status_update_service.rb @@ -5,7 +5,7 @@ class ActivityPub::ProcessStatusUpdateService < BaseService include Redisable include Lockable - def call(status, json) + def call(status, json, request_id: nil) raise ArgumentError, 'Status has unsaved changes' if status.changed? @json = json @@ -15,6 +15,7 @@ class ActivityPub::ProcessStatusUpdateService < BaseService @account = status.account @media_attachments_changed = false @poll_changed = false + @request_id = request_id # Only native types can be updated at the moment return @status if !expected_type? || already_updated_more_recently? @@ -191,7 +192,7 @@ class ActivityPub::ProcessStatusUpdateService < BaseService next if href.blank? account = ActivityPub::TagManager.instance.uri_to_resource(href, Account) - account ||= ActivityPub::FetchRemoteAccountService.new.call(href) + account ||= ActivityPub::FetchRemoteAccountService.new.call(href, request_id: @request_id) next if account.nil? diff --git a/spec/services/activitypub/process_account_service_spec.rb b/spec/services/activitypub/process_account_service_spec.rb index 7728b9ba8..2b20d17b1 100644 --- a/spec/services/activitypub/process_account_service_spec.rb +++ b/spec/services/activitypub/process_account_service_spec.rb @@ -109,4 +109,98 @@ RSpec.describe ActivityPub::ProcessAccountService, type: :service do end end end + + context 'discovering many subdomains in a short timeframe' do + before do + stub_const 'ActivityPub::ProcessAccountService::SUBDOMAINS_RATELIMIT', 5 + end + + let(:subject) do + 8.times do |i| + domain = "test#{i}.testdomain.com" + json = { + id: "https://#{domain}/users/1", + type: 'Actor', + inbox: "https://#{domain}/inbox", + }.with_indifferent_access + described_class.new.call('alice', domain, json) + end + end + + it 'creates at least some accounts' do + expect { subject }.to change { Account.remote.count }.by_at_least(2) + end + + it 'creates no more account than the limit allows' do + expect { subject }.to change { Account.remote.count }.by_at_most(5) + end + end + + context 'accounts referencing other accounts' do + before do + stub_const 'ActivityPub::ProcessAccountService::DISCOVERIES_PER_REQUEST', 5 + end + + let(:payload) do + { + '@context': ['https://www.w3.org/ns/activitystreams'], + id: 'https://foo.test/users/1', + type: 'Person', + inbox: 'https://foo.test/inbox', + featured: 'https://foo.test/users/1/featured', + preferredUsername: 'user1', + }.with_indifferent_access + end + + before do + 8.times do |i| + actor_json = { + '@context': ['https://www.w3.org/ns/activitystreams'], + id: "https://foo.test/users/#{i}", + type: 'Person', + inbox: 'https://foo.test/inbox', + featured: "https://foo.test/users/#{i}/featured", + preferredUsername: "user#{i}", + }.with_indifferent_access + status_json = { + '@context': ['https://www.w3.org/ns/activitystreams'], + id: "https://foo.test/users/#{i}/status", + attributedTo: "https://foo.test/users/#{i}", + type: 'Note', + content: "@user#{i + 1} test", + tag: [ + { + type: 'Mention', + href: "https://foo.test/users/#{i + 1}", + name: "@user#{i + 1 }", + } + ], + to: [ 'as:Public', "https://foo.test/users/#{i + 1}" ] + }.with_indifferent_access + featured_json = { + '@context': ['https://www.w3.org/ns/activitystreams'], + id: "https://foo.test/users/#{i}/featured", + type: 'OrderedCollection', + totelItems: 1, + orderedItems: [status_json], + }.with_indifferent_access + webfinger = { + subject: "acct:user#{i}@foo.test", + links: [{ rel: 'self', href: "https://foo.test/users/#{i}" }], + }.with_indifferent_access + stub_request(:get, "https://foo.test/users/#{i}").to_return(status: 200, body: actor_json.to_json, headers: { 'Content-Type': 'application/activity+json' }) + stub_request(:get, "https://foo.test/users/#{i}/featured").to_return(status: 200, body: featured_json.to_json, headers: { 'Content-Type': 'application/activity+json' }) + stub_request(:get, "https://foo.test/users/#{i}/status").to_return(status: 200, body: status_json.to_json, headers: { 'Content-Type': 'application/activity+json' }) + stub_request(:get, "https://foo.test/.well-known/webfinger?resource=acct:user#{i}@foo.test").to_return(body: webfinger.to_json, headers: { 'Content-Type': 'application/jrd+json' }) + end + end + + it 'creates at least some accounts' do + expect { subject.call('user1', 'foo.test', payload) }.to change { Account.remote.count }.by_at_least(2) + end + + it 'creates no more account than the limit allows' do + expect { subject.call('user1', 'foo.test', payload) }.to change { Account.remote.count }.by_at_most(5) + end + end end -- cgit From f6492a7c4d7cd08364ba507911f6b3c3df1c7e70 Mon Sep 17 00:00:00 2001 From: Francis Murillo Date: Tue, 6 Dec 2022 23:25:18 +0000 Subject: Log admin approve and reject account (#22088) * Log admin approve and reject account * Add unit tests for approve and reject logging --- app/controllers/admin/accounts_controller.rb | 2 + .../api/v1/admin/accounts_controller.rb | 2 + spec/controllers/admin/accounts_controller_spec.rb | 81 ++++++++++++++++++++++ .../api/v1/admin/accounts_controller_spec.rb | 18 +++++ 4 files changed, 103 insertions(+) (limited to 'app') diff --git a/app/controllers/admin/accounts_controller.rb b/app/controllers/admin/accounts_controller.rb index 40bf685c5..9beb8fde6 100644 --- a/app/controllers/admin/accounts_controller.rb +++ b/app/controllers/admin/accounts_controller.rb @@ -55,12 +55,14 @@ module Admin def approve authorize @account.user, :approve? @account.user.approve! + log_action :approve, @account.user redirect_to admin_accounts_path(status: 'pending'), notice: I18n.t('admin.accounts.approved_msg', username: @account.acct) end def reject authorize @account.user, :reject? DeleteAccountService.new.call(@account, reserve_email: false, reserve_username: false) + log_action :reject, @account.user redirect_to admin_accounts_path(status: 'pending'), notice: I18n.t('admin.accounts.rejected_msg', username: @account.acct) end diff --git a/app/controllers/api/v1/admin/accounts_controller.rb b/app/controllers/api/v1/admin/accounts_controller.rb index ae7f7d076..f48300072 100644 --- a/app/controllers/api/v1/admin/accounts_controller.rb +++ b/app/controllers/api/v1/admin/accounts_controller.rb @@ -54,12 +54,14 @@ class Api::V1::Admin::AccountsController < Api::BaseController def approve authorize @account.user, :approve? @account.user.approve! + log_action :approve, @account.user render json: @account, serializer: REST::Admin::AccountSerializer end def reject authorize @account.user, :reject? DeleteAccountService.new.call(@account, reserve_email: false, reserve_username: false) + log_action :reject, @account.user render_empty end diff --git a/spec/controllers/admin/accounts_controller_spec.rb b/spec/controllers/admin/accounts_controller_spec.rb index 1bd51a0c8..81d592ddd 100644 --- a/spec/controllers/admin/accounts_controller_spec.rb +++ b/spec/controllers/admin/accounts_controller_spec.rb @@ -147,6 +147,87 @@ RSpec.describe Admin::AccountsController, type: :controller do end end + describe 'POST #approve' do + subject { post :approve, params: { id: account.id } } + + let(:current_user) { Fabricate(:user, role: role) } + let(:account) { user.account } + let(:user) { Fabricate(:user) } + + before do + account.user.update(approved: false) + end + + context 'when user is admin' do + let(:role) { UserRole.find_by(name: 'Admin') } + + it 'succeeds in approving account' do + is_expected.to redirect_to admin_accounts_path(status: 'pending') + expect(user.reload).to be_approved + end + + it 'logs action' do + is_expected.to have_http_status :found + + log_item = Admin::ActionLog.last + + expect(log_item).to_not be_nil + expect(log_item.action).to eq :approve + expect(log_item.account_id).to eq current_user.account_id + expect(log_item.target_id).to eq account.user.id + end + end + + context 'when user is not admin' do + let(:role) { UserRole.everyone } + + it 'fails to approve account' do + is_expected.to have_http_status :forbidden + expect(user.reload).not_to be_approved + end + end + end + + describe 'POST #reject' do + subject { post :reject, params: { id: account.id } } + + let(:current_user) { Fabricate(:user, role: role) } + let(:account) { user.account } + let(:user) { Fabricate(:user) } + + before do + account.user.update(approved: false) + end + + context 'when user is admin' do + let(:role) { UserRole.find_by(name: 'Admin') } + + it 'succeeds in rejecting account' do + is_expected.to redirect_to admin_accounts_path(status: 'pending') + end + + it 'logs action' do + is_expected.to have_http_status :found + + log_item = Admin::ActionLog.last + + expect(log_item).to_not be_nil + expect(log_item.action).to eq :reject + expect(log_item.account_id).to eq current_user.account_id + expect(log_item.target_id).to eq account.user.id + end + end + + context 'when user is not admin' do + let(:role) { UserRole.everyone } + + it 'fails to reject account' do + is_expected.to have_http_status :forbidden + expect(user.reload).not_to be_approved + end + end + end + describe 'POST #redownload' do subject { post :redownload, params: { id: account.id } } diff --git a/spec/controllers/api/v1/admin/accounts_controller_spec.rb b/spec/controllers/api/v1/admin/accounts_controller_spec.rb index cd38030e0..8d35b86cb 100644 --- a/spec/controllers/api/v1/admin/accounts_controller_spec.rb +++ b/spec/controllers/api/v1/admin/accounts_controller_spec.rb @@ -100,6 +100,15 @@ RSpec.describe Api::V1::Admin::AccountsController, type: :controller do it 'approves user' do expect(account.reload.user_approved?).to be true end + + it 'logs action' do + log_item = Admin::ActionLog.last + + expect(log_item).to_not be_nil + expect(log_item.action).to eq :approve + expect(log_item.account_id).to eq user.account_id + expect(log_item.target_id).to eq account.user.id + end end describe 'POST #reject' do @@ -118,6 +127,15 @@ RSpec.describe Api::V1::Admin::AccountsController, type: :controller do it 'removes user' do expect(User.where(id: account.user.id).count).to eq 0 end + + it 'logs action' do + log_item = Admin::ActionLog.last + + expect(log_item).to_not be_nil + expect(log_item.action).to eq :reject + expect(log_item.account_id).to eq user.account_id + expect(log_item.target_id).to eq account.user.id + end end describe 'POST #enable' do -- cgit From 51098035ed2f42d949d6ec3f8ecdd5e68e73874d Mon Sep 17 00:00:00 2001 From: Claire Date: Wed, 7 Dec 2022 00:25:40 +0100 Subject: Fix “Sign up” button with closed registrations not opening modal on mobile (#22060) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #20840 --- .../mastodon/features/ui/components/header.js | 31 +++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) (limited to 'app') diff --git a/app/javascript/mastodon/features/ui/components/header.js b/app/javascript/mastodon/features/ui/components/header.js index bbb0ca1c6..1384bebda 100644 --- a/app/javascript/mastodon/features/ui/components/header.js +++ b/app/javascript/mastodon/features/ui/components/header.js @@ -6,6 +6,7 @@ import { registrationsOpen, me } from 'mastodon/initial_state'; import Avatar from 'mastodon/components/avatar'; import PropTypes from 'prop-types'; import { connect } from 'react-redux'; +import { openModal } from 'mastodon/actions/modal'; const Account = connect(state => ({ account: state.getIn(['accounts', me]), @@ -15,7 +16,14 @@ const Account = connect(state => ({ )); -export default @withRouter +const mapDispatchToProps = (dispatch) => ({ + openClosedRegistrationsModal() { + dispatch(openModal('CLOSED_REGISTRATIONS')); + }, +}); + +export default @connect(null, mapDispatchToProps) +@withRouter class Header extends React.PureComponent { static contextTypes = { @@ -23,12 +31,13 @@ class Header extends React.PureComponent { }; static propTypes = { + openClosedRegistrationsModal: PropTypes.func, location: PropTypes.object, }; render () { const { signedIn } = this.context.identity; - const { location } = this.props; + const { location, openClosedRegistrationsModal } = this.props; let content; @@ -40,10 +49,26 @@ class Header extends React.PureComponent { ); } else { + let signupButton; + + if (registrationsOpen) { + signupButton = ( + + + + ); + } else { + signupButton = ( + + ); + } + content = ( <> - + {signupButton} ); } -- cgit From 76454cc63803f3619c6c2352abac9646004e32c4 Mon Sep 17 00:00:00 2001 From: Claire Date: Wed, 7 Dec 2022 00:40:18 +0100 Subject: Fix UI header overflow on mobile (#21783) --- app/javascript/styles/mastodon/components.scss | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'app') diff --git a/app/javascript/styles/mastodon/components.scss b/app/javascript/styles/mastodon/components.scss index 79a9a4bd7..318e4b3de 100644 --- a/app/javascript/styles/mastodon/components.scss +++ b/app/javascript/styles/mastodon/components.scss @@ -2217,6 +2217,7 @@ $ui-header-height: 55px; z-index: 2; justify-content: space-between; align-items: center; + overflow: hidden; &__logo { display: inline-flex; @@ -2233,10 +2234,15 @@ $ui-header-height: 55px; align-items: center; gap: 10px; padding: 0 10px; + overflow: hidden; .button { flex: 0 0 auto; } + + .button-tertiary { + flex-shrink: 1; + } } } -- cgit From 21b208afcbb5b1a6f2f49eb6db478fc9a99ad272 Mon Sep 17 00:00:00 2001 From: Juan Xavier Gomez Date: Tue, 6 Dec 2022 18:40:37 -0500 Subject: Fix hidden overflow on interaction modal (#21763) * Fix hidden overflow on interaction modal * only update overflow y --- app/javascript/styles/mastodon/components.scss | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'app') diff --git a/app/javascript/styles/mastodon/components.scss b/app/javascript/styles/mastodon/components.scss index 318e4b3de..b56d43000 100644 --- a/app/javascript/styles/mastodon/components.scss +++ b/app/javascript/styles/mastodon/components.scss @@ -7981,7 +7981,8 @@ noscript { width: 600px; background: $ui-base-color; border-radius: 8px; - overflow: hidden; + overflow-x: hidden; + overflow-y: auto; position: relative; display: block; padding: 20px; -- cgit From b59fb28e90bc21d6fd1a6bafd13cfbd81ab5be54 Mon Sep 17 00:00:00 2001 From: Claire Date: Wed, 7 Dec 2022 02:35:39 +0100 Subject: Fix 500 error when trying to migrate to an invalid address (#21462) * Fix 500 error when trying to migrate to an invalid address * Add tests --- app/models/account_migration.rb | 2 +- app/models/form/redirect.rb | 2 +- spec/models/account_migration_spec.rb | 43 +++++++++++++++++++++++++++++++++++ 3 files changed, 45 insertions(+), 2 deletions(-) (limited to 'app') diff --git a/app/models/account_migration.rb b/app/models/account_migration.rb index 16276158d..fa8cb6013 100644 --- a/app/models/account_migration.rb +++ b/app/models/account_migration.rb @@ -59,7 +59,7 @@ class AccountMigration < ApplicationRecord def set_target_account self.target_account = ResolveAccountService.new.call(acct, skip_cache: true) - rescue Webfinger::Error, HTTP::Error, OpenSSL::SSL::SSLError, Mastodon::Error + rescue Webfinger::Error, HTTP::Error, OpenSSL::SSL::SSLError, Mastodon::Error, Addressable::URI::InvalidURIError # Validation will take care of it end diff --git a/app/models/form/redirect.rb b/app/models/form/redirect.rb index 795fdd057..3cd5731e6 100644 --- a/app/models/form/redirect.rb +++ b/app/models/form/redirect.rb @@ -32,7 +32,7 @@ class Form::Redirect def set_target_account @target_account = ResolveAccountService.new.call(acct, skip_cache: true) - rescue Webfinger::Error, HTTP::Error, OpenSSL::SSL::SSLError, Mastodon::Error + rescue Webfinger::Error, HTTP::Error, OpenSSL::SSL::SSLError, Mastodon::Error, Addressable::URI::InvalidURIError # Validation will take care of it end diff --git a/spec/models/account_migration_spec.rb b/spec/models/account_migration_spec.rb index 8461b4b28..5f66fe8da 100644 --- a/spec/models/account_migration_spec.rb +++ b/spec/models/account_migration_spec.rb @@ -1,5 +1,48 @@ require 'rails_helper' RSpec.describe AccountMigration, type: :model do + describe 'validations' do + let(:source_account) { Fabricate(:account) } + let(:target_acct) { target_account.acct } + let(:subject) { AccountMigration.new(account: source_account, acct: target_acct) } + + context 'with valid properties' do + let(:target_account) { Fabricate(:account, username: 'target', domain: 'remote.org') } + + before do + target_account.aliases.create!(acct: source_account.acct) + + service_double = double + allow(ResolveAccountService).to receive(:new).and_return(service_double) + allow(service_double).to receive(:call).with(target_acct, anything).and_return(target_account) + end + + it 'passes validations' do + expect(subject).to be_valid + end + end + + context 'with unresolveable account' do + let(:target_acct) { 'target@remote' } + + before do + service_double = double + allow(ResolveAccountService).to receive(:new).and_return(service_double) + allow(service_double).to receive(:call).with(target_acct, anything).and_return(nil) + end + + it 'has errors on acct field' do + expect(subject).to model_have_error_on_field(:acct) + end + end + + context 'with a space in the domain part' do + let(:target_acct) { 'target@remote. org' } + + it 'has errors on acct field' do + expect(subject).to model_have_error_on_field(:acct) + end + end + end end -- cgit