From b71904816a30d29ef62bb35582fc47162a50d539 Mon Sep 17 00:00:00 2001 From: Claire Date: Thu, 22 Feb 2024 14:28:19 +0100 Subject: [PATCH 1/4] Change registrations to be disabled by default for new servers (#29280) --- app/javascript/packs/admin.jsx | 4 ++++ app/views/admin/settings/registrations/show.html.haml | 4 +++- config/locales/en.yml | 2 ++ config/settings.yml | 2 +- spec/spec_helper.rb | 6 ++++++ spec/support/streaming_server_manager.rb | 7 +++++++ 6 files changed, 23 insertions(+), 2 deletions(-) diff --git a/app/javascript/packs/admin.jsx b/app/javascript/packs/admin.jsx index d3bfc4ea90..817a5d46bc 100644 --- a/app/javascript/packs/admin.jsx +++ b/app/javascript/packs/admin.jsx @@ -145,6 +145,10 @@ Rails.delegate(document, '#form_admin_settings_enable_bootstrap_timeline_account const onChangeRegistrationMode = (target) => { const enabled = target.value === 'approved'; + [].forEach.call(document.querySelectorAll('.form_admin_settings_registrations_mode .warning-hint'), (warning_hint) => { + warning_hint.style.display = target.value === 'open' ? 'inline' : 'none'; + }); + [].forEach.call(document.querySelectorAll('#form_admin_settings_require_invite_text'), (input) => { input.disabled = !enabled; if (enabled) { diff --git a/app/views/admin/settings/registrations/show.html.haml b/app/views/admin/settings/registrations/show.html.haml index 168f109757..4ece27bf4e 100644 --- a/app/views/admin/settings/registrations/show.html.haml +++ b/app/views/admin/settings/registrations/show.html.haml @@ -10,9 +10,11 @@ %p.lead= t('admin.settings.registrations.preamble') + .flash-message= t('admin.settings.registrations.moderation_recommandation') + .fields-row .fields-row__column.fields-row__column-6.fields-group - = f.input :registrations_mode, collection: %w(open approved none), wrapper: :with_label, include_blank: false, label_method: ->(mode) { I18n.t("admin.settings.registrations_mode.modes.#{mode}") } + = f.input :registrations_mode, collection: %w(open approved none), wrapper: :with_label, include_blank: false, label_method: ->(mode) { I18n.t("admin.settings.registrations_mode.modes.#{mode}") }, warning_hint: I18n.t('admin.settings.registrations_mode.warning_hint') .fields-row__column.fields-row__column-6.fields-group = f.input :require_invite_text, as: :boolean, wrapper: :with_label, disabled: !approved_registrations? diff --git a/config/locales/en.yml b/config/locales/en.yml index a7a83391d4..efd603740a 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -767,6 +767,7 @@ en: disabled: To no one users: To logged-in local users registrations: + moderation_recommandation: Please make sure you have an adequate and reactive moderation team before you open registrations to everyone! preamble: Control who can create an account on your server. title: Registrations registrations_mode: @@ -774,6 +775,7 @@ en: approved: Approval required for sign up none: Nobody can sign up open: Anyone can sign up + warning_hint: We recommend using “Approval required for sign up” unless you are confident your moderation team can handle spam and malicious registrations in a timely fashion. security: authorized_fetch: Require authentication from federated servers authorized_fetch_hint: Requiring authentication from federated servers enables stricter enforcement of both user-level and server-level blocks. However, this comes at the cost of a performance penalty, reduces the reach of your replies, and may introduce compatibility issues with some federated services. In addition, this will not prevent dedicated actors from fetching your public posts and accounts. diff --git a/config/settings.yml b/config/settings.yml index 67297c26ce..208c8e3760 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -9,7 +9,7 @@ defaults: &defaults site_terms: '' site_contact_username: '' site_contact_email: '' - registrations_mode: 'open' + registrations_mode: 'none' profile_directory: true closed_registrations_message: '' timeline_preview: true diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index dc60976d05..8a01792a19 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -23,6 +23,12 @@ RSpec.configure do |config| config.before :suite do Rails.application.load_seed Chewy.strategy(:bypass) + + # NOTE: we switched registrations mode to closed by default, but the specs + # very heavily rely on having it enabled by default, as it relies on users + # being approved by default except in select cases where explicitly testing + # other registration modes + Setting.registrations_mode = 'open' end config.after :suite do diff --git a/spec/support/streaming_server_manager.rb b/spec/support/streaming_server_manager.rb index 39657586f2..3381918299 100644 --- a/spec/support/streaming_server_manager.rb +++ b/spec/support/streaming_server_manager.rb @@ -102,6 +102,13 @@ RSpec.configure do |config| self.use_transactional_tests = false DatabaseCleaner.cleaning do + # NOTE: we switched registrations mode to closed by default, but the specs + # very heavily rely on having it enabled by default, as it relies on users + # being approved by default except in select cases where explicitly testing + # other registration modes + # Also needs to be set per-example here because of the database cleaner. + Setting.registrations_mode = 'open' + example.run end From a9496882fc1257c3169fc2fdcb8afd1549044da7 Mon Sep 17 00:00:00 2001 From: Claire Date: Thu, 22 Feb 2024 15:52:14 +0100 Subject: [PATCH 2/4] Fix auto-close email being sent to users with devops permissions instead of settings permissions (#29355) --- app/workers/scheduler/auto_close_registrations_scheduler.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/workers/scheduler/auto_close_registrations_scheduler.rb b/app/workers/scheduler/auto_close_registrations_scheduler.rb index 17516dd23f..6874502915 100644 --- a/app/workers/scheduler/auto_close_registrations_scheduler.rb +++ b/app/workers/scheduler/auto_close_registrations_scheduler.rb @@ -26,7 +26,7 @@ class Scheduler::AutoCloseRegistrationsScheduler def switch_to_approval_mode! Setting.registrations_mode = 'approved' - User.those_who_can(:view_devops).includes(:account).find_each do |user| + User.those_who_can(:manage_settings).includes(:account).find_each do |user| AdminMailer.with(recipient: user.account).auto_close_registrations.deliver_later end end From 5152dd869e4884117732f7ff3c41c4fbc109b1ee Mon Sep 17 00:00:00 2001 From: Claire Date: Thu, 22 Feb 2024 18:31:50 +0100 Subject: [PATCH 3/4] Fix link verifications when page size exceeds 1MB (#29358) --- app/services/verify_link_service.rb | 2 +- spec/services/verify_link_service_spec.rb | 27 ++++++++++++----------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/app/services/verify_link_service.rb b/app/services/verify_link_service.rb index 707aeb4e08..b317fc31a8 100644 --- a/app/services/verify_link_service.rb +++ b/app/services/verify_link_service.rb @@ -19,7 +19,7 @@ class VerifyLinkService < BaseService def perform_request! @body = Request.new(:get, @url).add_headers('Accept' => 'text/html').perform do |res| - res.code == 200 ? res.body_with_limit : nil + res.code == 200 ? res.truncated_body : nil end end diff --git a/spec/services/verify_link_service_spec.rb b/spec/services/verify_link_service_spec.rb index 415788cb58..d06344f9cc 100644 --- a/spec/services/verify_link_service_spec.rb +++ b/spec/services/verify_link_service_spec.rb @@ -76,6 +76,20 @@ RSpec.describe VerifyLinkService, type: :service do end context 'when a document is truncated but the link back is valid' do + let(:html) do + " + + + + " + end + + it 'marks the field as verified' do + expect(field.verified?).to be true + end + end + + context 'when a link tag might be truncated' do let(:html) do " @@ -89,19 +103,6 @@ RSpec.describe VerifyLinkService, type: :service do end end - context 'when a link back might be truncated' do - let(:html) do - " - - - Date: Thu, 22 Feb 2024 22:27:24 +0100 Subject: [PATCH 4/4] Fix processing of `Link` objects in `Image` objects (#29335) --- .../activitypub/process_account_service.rb | 11 +++++--- .../process_account_service_spec.rb | 27 +++++++++++++++++-- 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/app/services/activitypub/process_account_service.rb b/app/services/activitypub/process_account_service.rb index 9e787ace50..b667e97f4d 100644 --- a/app/services/activitypub/process_account_service.rb +++ b/app/services/activitypub/process_account_service.rb @@ -201,10 +201,15 @@ class ActivityPub::ProcessAccountService < BaseService value = first_of_value(@json[key]) return if value.nil? - return value['url'] if value.is_a?(Hash) - image = fetch_resource_without_id_validation(value) - image['url'] if image + if value.is_a?(String) + value = fetch_resource_without_id_validation(value) + return if value.nil? + end + + value = first_of_value(value['url']) if value.is_a?(Hash) && value['type'] == 'Image' + value = value['href'] if value.is_a?(Hash) + value if value.is_a?(String) end def public_key diff --git a/spec/services/activitypub/process_account_service_spec.rb b/spec/services/activitypub/process_account_service_spec.rb index 824577d1b0..b13869f357 100644 --- a/spec/services/activitypub/process_account_service_spec.rb +++ b/spec/services/activitypub/process_account_service_spec.rb @@ -5,7 +5,7 @@ require 'rails_helper' RSpec.describe ActivityPub::ProcessAccountService, type: :service do subject { described_class.new } - context 'with property values' do + context 'with property values, an avatar, and a profile header' do let(:payload) do { id: 'https://foo.test', @@ -16,10 +16,29 @@ RSpec.describe ActivityPub::ProcessAccountService, type: :service do { type: 'PropertyValue', name: 'Occupation', value: 'Unit test' }, { type: 'PropertyValue', name: 'non-string', value: %w(foo bar) }, ], + image: { + type: 'Image', + mediaType: 'image/png', + url: 'https://foo.test/image.png', + }, + icon: { + type: 'Image', + url: [ + { + mediaType: 'image/png', + href: 'https://foo.test/icon.png', + }, + ], + }, }.with_indifferent_access end - it 'parses out of attachment' do + before do + stub_request(:get, 'https://foo.test/image.png').to_return(request_fixture('avatar.txt')) + stub_request(:get, 'https://foo.test/icon.png').to_return(request_fixture('avatar.txt')) + end + + it 'parses property values, avatar and profile header as expected' do account = subject.call('alice', 'example.com', payload) expect(account.fields) @@ -37,6 +56,10 @@ RSpec.describe ActivityPub::ProcessAccountService, type: :service do name: eq('Occupation'), value: eq('Unit test') ) + expect(account).to have_attributes( + avatar_remote_url: 'https://foo.test/icon.png', + header_remote_url: 'https://foo.test/image.png' + ) end end