From 8a2d764d342d2aa47378b1fb0380629e763d64b5 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 28 Jan 2025 03:08:16 -0500 Subject: [PATCH 1/6] Reduce factory creation across `controllers/admin` specs (#33752) --- .../admin/domain_blocks_controller_spec.rb | 36 ++++--------------- .../email_domain_blocks_controller_spec.rb | 4 +-- .../admin/roles_controller_spec.rb | 32 +++++------------ .../admin/users/roles_controller_spec.rb | 12 ++----- 4 files changed, 18 insertions(+), 66 deletions(-) diff --git a/spec/controllers/admin/domain_blocks_controller_spec.rb b/spec/controllers/admin/domain_blocks_controller_spec.rb index 4d48a0d9d3..f3f08c7940 100644 --- a/spec/controllers/admin/domain_blocks_controller_spec.rb +++ b/spec/controllers/admin/domain_blocks_controller_spec.rb @@ -48,15 +48,11 @@ RSpec.describe Admin::DomainBlocksController do post :create, params: { domain_block: { domain: 'example.com', severity: 'silence' } } end - it 'records a block' do + it 'records a block, calls a worker, redirects' do expect(DomainBlock.exists?(domain: 'example.com', severity: 'silence')).to be true - end - it 'calls DomainBlockWorker' do expect(DomainBlockWorker).to have_received(:perform_async) - end - it 'redirects with a success message' do expect(flash[:notice]).to eq I18n.t('admin.domain_blocks.created_msg') expect(response).to redirect_to(admin_instances_path(limited: '1')) end @@ -68,15 +64,11 @@ RSpec.describe Admin::DomainBlocksController do post :create, params: { domain_block: { domain: 'example.com', severity: 'silence' } } end - it 'does not record a block' do + it 'does not record a block or call worker, renders new' do expect(DomainBlock.exists?(domain: 'example.com', severity: 'silence')).to be false - end - it 'does not call DomainBlockWorker' do expect(DomainBlockWorker).to_not have_received(:perform_async) - end - it 'renders new' do expect(response).to render_template :new end end @@ -87,15 +79,11 @@ RSpec.describe Admin::DomainBlocksController do post :create, params: { domain_block: { domain: 'example.com', severity: 'suspend', reject_media: true, reject_reports: true } } end - it 'does not record a block' do + it 'does not record a block or call worker, renders confirm suspension' do expect(DomainBlock.exists?(domain: 'example.com', severity: 'suspend')).to be false - end - it 'does not call DomainBlockWorker' do expect(DomainBlockWorker).to_not have_received(:perform_async) - end - it 'renders confirm_suspension' do expect(response).to render_template :confirm_suspension end end @@ -105,15 +93,11 @@ RSpec.describe Admin::DomainBlocksController do post :create, params: { :domain_block => { domain: 'example.com', severity: 'suspend', reject_media: true, reject_reports: true }, 'confirm' => '' } end - it 'records a block' do + it 'records a block and calls worker and redirects' do expect(DomainBlock.exists?(domain: 'example.com', severity: 'suspend')).to be true - end - it 'calls DomainBlockWorker' do expect(DomainBlockWorker).to have_received(:perform_async) - end - it 'redirects with a success message' do expect(flash[:notice]).to eq I18n.t('admin.domain_blocks.created_msg') expect(response).to redirect_to(admin_instances_path(limited: '1')) end @@ -130,15 +114,11 @@ RSpec.describe Admin::DomainBlocksController do post :create, params: { domain_block: { domain: 'example.com', severity: 'suspend', reject_media: true, reject_reports: true } } end - it 'does not record a block' do + it 'does not record a block or call worker, renders confirm suspension' do expect(DomainBlock.exists?(domain: 'example.com', severity: 'suspend')).to be false - end - it 'does not call DomainBlockWorker' do expect(DomainBlockWorker).to_not have_received(:perform_async) - end - it 'renders confirm_suspension' do expect(response).to render_template :confirm_suspension end end @@ -148,15 +128,11 @@ RSpec.describe Admin::DomainBlocksController do post :create, params: { :domain_block => { domain: 'example.com', severity: 'suspend', reject_media: true, reject_reports: true }, 'confirm' => '' } end - it 'updates the record' do + it 'updates the record and calls worker, redirects' do expect(DomainBlock.exists?(domain: 'example.com', severity: 'suspend')).to be true - end - it 'calls DomainBlockWorker' do expect(DomainBlockWorker).to have_received(:perform_async) - end - it 'redirects with a success message' do expect(flash[:notice]).to eq I18n.t('admin.domain_blocks.created_msg') expect(response).to redirect_to(admin_instances_path(limited: '1')) end diff --git a/spec/controllers/admin/email_domain_blocks_controller_spec.rb b/spec/controllers/admin/email_domain_blocks_controller_spec.rb index c7460c2110..f274c01281 100644 --- a/spec/controllers/admin/email_domain_blocks_controller_spec.rb +++ b/spec/controllers/admin/email_domain_blocks_controller_spec.rb @@ -58,11 +58,9 @@ RSpec.describe Admin::EmailDomainBlocksController do post :create, params: { email_domain_block: { domain: 'example.com' }, save: '' } end - it 'blocks the domain' do + it 'blocks the domain and redirects to email domain blocks' do expect(EmailDomainBlock.find_by(domain: 'example.com')).to_not be_nil - end - it 'redirects to e-mail domain blocks' do expect(response).to redirect_to(admin_email_domain_blocks_path) end end diff --git a/spec/controllers/admin/roles_controller_spec.rb b/spec/controllers/admin/roles_controller_spec.rb index 2c43a0ca87..173a89e5d5 100644 --- a/spec/controllers/admin/roles_controller_spec.rb +++ b/spec/controllers/admin/roles_controller_spec.rb @@ -68,11 +68,9 @@ RSpec.describe Admin::RolesController do let(:selected_position) { 1 } let(:selected_permissions_as_keys) { %w(manage_roles) } - it 'redirects to roles page' do + it 'redirects to roles page and creates role' do expect(response).to redirect_to(admin_roles_path) - end - it 'creates new role' do expect(UserRole.find_by(name: 'Bar')).to_not be_nil end end @@ -81,11 +79,9 @@ RSpec.describe Admin::RolesController do let(:selected_position) { 100 } let(:selected_permissions_as_keys) { %w(manage_roles) } - it 'renders new template' do + it 'renders new template and does not create role' do expect(response).to render_template(:new) - end - it 'does not create new role' do expect(UserRole.find_by(name: 'Bar')).to be_nil end end @@ -94,11 +90,9 @@ RSpec.describe Admin::RolesController do let(:selected_position) { 1 } let(:selected_permissions_as_keys) { %w(manage_roles manage_users manage_reports) } - it 'renders new template' do + it 'renders new template and does not create role' do expect(response).to render_template(:new) - end - it 'does not create new role' do expect(UserRole.find_by(name: 'Bar')).to be_nil end end @@ -109,11 +103,9 @@ RSpec.describe Admin::RolesController do let(:selected_position) { 1 } let(:selected_permissions_as_keys) { %w(manage_roles manage_users manage_reports) } - it 'redirects to roles page' do + it 'redirects to roles page and creates new role' do expect(response).to redirect_to(admin_roles_path) - end - it 'creates new role' do expect(UserRole.find_by(name: 'Bar')).to_not be_nil end end @@ -166,11 +158,9 @@ RSpec.describe Admin::RolesController do end context 'when user does not have permission to manage roles' do - it 'returns http forbidden' do + it 'returns http forbidden and does not update role' do expect(response).to have_http_status(403) - end - it 'does not update the role' do expect(role.reload.name).to eq 'Bar' end end @@ -179,11 +169,9 @@ RSpec.describe Admin::RolesController do let(:permissions) { UserRole::FLAGS[:manage_roles] } context 'when role has permissions the user doesn\'t' do - it 'renders edit template' do + it 'renders edit template and does not update role' do expect(response).to render_template(:edit) - end - it 'does not update the role' do expect(role.reload.name).to eq 'Bar' end end @@ -192,11 +180,9 @@ RSpec.describe Admin::RolesController do let(:permissions) { UserRole::FLAGS[:manage_roles] | UserRole::FLAGS[:manage_users] } context 'when user outranks the role' do - it 'redirects to roles page' do + it 'redirects to roles page and updates role' do expect(response).to redirect_to(admin_roles_path) - end - it 'updates the role' do expect(role.reload.name).to eq 'Baz' end end @@ -204,11 +190,9 @@ RSpec.describe Admin::RolesController do context 'when role outranks user' do let(:role_position) { current_role.position + 1 } - it 'returns http forbidden' do + it 'returns http forbidden and does not update role' do expect(response).to have_http_status(403) - end - it 'does not update the role' do expect(role.reload.name).to eq 'Bar' end end diff --git a/spec/controllers/admin/users/roles_controller_spec.rb b/spec/controllers/admin/users/roles_controller_spec.rb index bfc2bb151f..a7d59181d6 100644 --- a/spec/controllers/admin/users/roles_controller_spec.rb +++ b/spec/controllers/admin/users/roles_controller_spec.rb @@ -44,11 +44,9 @@ RSpec.describe Admin::Users::RolesController do let(:permissions) { UserRole::FLAGS[:manage_roles] } let(:position) { 1 } - it 'updates user role' do + it 'updates user role and redirects' do expect(user.reload.role_id).to eq selected_role&.id - end - it 'redirects back to account page' do expect(response).to redirect_to(admin_account_path(user.account_id)) end end @@ -57,11 +55,9 @@ RSpec.describe Admin::Users::RolesController do let(:permissions) { UserRole::FLAGS[:administrator] } let(:position) { 100 } - it 'does not update user role' do + it 'does not update user role and renders edit' do expect(user.reload.role_id).to eq previous_role&.id - end - it 'renders edit form' do expect(response).to render_template(:show) end end @@ -71,11 +67,9 @@ RSpec.describe Admin::Users::RolesController do let(:permissions) { UserRole::FLAGS[:manage_roles] } let(:position) { 1 } - it 'does not update user role' do + it 'does not update user role and returns http forbidden' do expect(user.reload.role_id).to eq previous_role&.id - end - it 'returns http forbidden' do expect(response).to have_http_status(403) end end From 6646a0a9fafd891d36fa3b0fafb9675ec26aa039 Mon Sep 17 00:00:00 2001 From: Claire Date: Tue, 28 Jan 2025 09:21:59 +0100 Subject: [PATCH 2/6] Fix parameter validation in our custom devise strategies (#33754) --- lib/devise/strategies/two_factor_ldap_authenticatable.rb | 2 +- lib/devise/strategies/two_factor_pam_authenticatable.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/devise/strategies/two_factor_ldap_authenticatable.rb b/lib/devise/strategies/two_factor_ldap_authenticatable.rb index c8258deb16..2efd1323a8 100644 --- a/lib/devise/strategies/two_factor_ldap_authenticatable.rb +++ b/lib/devise/strategies/two_factor_ldap_authenticatable.rb @@ -23,7 +23,7 @@ module Devise protected def valid_params? - params[scope] && params[scope][:password].present? + params[scope].is_a?(Hash) && params[scope][:password].present? end end end diff --git a/lib/devise/strategies/two_factor_pam_authenticatable.rb b/lib/devise/strategies/two_factor_pam_authenticatable.rb index 7263ba354a..2164b03234 100644 --- a/lib/devise/strategies/two_factor_pam_authenticatable.rb +++ b/lib/devise/strategies/two_factor_pam_authenticatable.rb @@ -22,7 +22,7 @@ module Devise protected def valid_params? - params[scope].respond_to?(:[]) && params[scope][:password].present? + params[scope].is_a?(Hash) && params[scope][:password].present? end end end From 0091459369f0b570e1b7d076b252a4ff741e6633 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Tue, 28 Jan 2025 09:39:50 +0100 Subject: [PATCH 3/6] Update RuboCop (non-major) to v1.71.0 (#33644) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- Gemfile.lock | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index a90aef56a1..fcbe74b9c0 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -370,7 +370,7 @@ GEM marcel (~> 1.0.1) mime-types terrapin (>= 0.6.0, < 2.0) - language_server-protocol (3.17.0.3) + language_server-protocol (3.17.0.4) launchy (3.0.1) addressable (~> 2.8) childprocess (~> 5.0) @@ -716,7 +716,7 @@ GEM rspec-mocks (~> 3.0) sidekiq (>= 5, < 8) rspec-support (3.13.2) - rubocop (1.70.0) + rubocop (1.71.0) json (~> 2.3) language_server-protocol (>= 3.17.0) parallel (~> 1.10) @@ -726,14 +726,14 @@ GEM rubocop-ast (>= 1.36.2, < 2.0) ruby-progressbar (~> 1.7) unicode-display_width (>= 2.4.0, < 4.0) - rubocop-ast (1.37.0) + rubocop-ast (1.38.0) parser (>= 3.3.1.0) rubocop-capybara (2.21.0) rubocop (~> 1.41) rubocop-performance (1.23.1) rubocop (>= 1.48.1, < 2.0) rubocop-ast (>= 1.31.1, < 2.0) - rubocop-rails (2.28.0) + rubocop-rails (2.29.1) activesupport (>= 4.2.0) rack (>= 1.1) rubocop (>= 1.52.0, < 2.0) From 80f72ee501980f092468f337a20efb5950e8beb0 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 28 Jan 2025 10:40:50 +0100 Subject: [PATCH 4/6] New Crowdin Translations (automated) (#33753) Co-authored-by: GitHub Actions --- app/javascript/mastodon/locales/cs.json | 9 +++++++++ app/javascript/mastodon/locales/es-MX.json | 4 ++-- app/javascript/mastodon/locales/ko.json | 1 + config/locales/cs.yml | 1 + config/locales/ko.yml | 3 ++- config/locales/simple_form.cs.yml | 2 ++ 6 files changed, 17 insertions(+), 3 deletions(-) diff --git a/app/javascript/mastodon/locales/cs.json b/app/javascript/mastodon/locales/cs.json index 05fb7200b5..66cc0727b6 100644 --- a/app/javascript/mastodon/locales/cs.json +++ b/app/javascript/mastodon/locales/cs.json @@ -86,6 +86,13 @@ "alert.unexpected.message": "Objevila se neočekávaná chyba.", "alert.unexpected.title": "Jejda!", "alt_text_badge.title": "Popisek", + "alt_text_modal.add_alt_text": "Přidat alt text", + "alt_text_modal.add_text_from_image": "Přidat text z obrázku", + "alt_text_modal.cancel": "Zrušit", + "alt_text_modal.change_thumbnail": "Změnit miniaturu", + "alt_text_modal.describe_for_people_with_hearing_impairments": "Popište to pro osoby se sluchovým postižením…", + "alt_text_modal.describe_for_people_with_visual_impairments": "Popište to pro osoby se zrakovým postižením…", + "alt_text_modal.done": "Hotovo", "announcement.announcement": "Oznámení", "annual_report.summary.archetype.booster": "Lovec obsahu", "annual_report.summary.archetype.lurker": "Špión", @@ -407,6 +414,8 @@ "ignore_notifications_modal.not_followers_title": "Ignorovat oznámení od lidí, kteří vás nesledují?", "ignore_notifications_modal.not_following_title": "Ignorovat oznámení od lidí, které nesledujete?", "ignore_notifications_modal.private_mentions_title": "Ignorovat oznámení z nevyžádaných soukromých zmínek?", + "info_button.label": "Nápověda", + "info_button.what_is_alt_text": "

Co je to alt text?

Alt text poskytuje popis obrázků pro lidi se zrakovými postižením, špatným připojením něbo těm, kteří potřebují více kontextu.

Můžete zlepšit přístupnost a porozumění napsáním jasného, stručného a objektivního alt textu.

  • Zachyťte důležité prvky
  • Shrňte text v obrázku
  • Použijte pravidelnou větnou skladbu
  • Vyhněte se nadbytečným informacím
  • U komplexních vizualizací (diagramy, mapy...) se zaměřte na trendy a klíčová zjištění
", "interaction_modal.action.favourite": "Chcete-li pokračovat, musíte oblíbit z vašeho účtu.", "interaction_modal.action.follow": "Chcete-li pokračovat, musíte sledovat z vašeho účtu.", "interaction_modal.action.reblog": "Chcete-li pokračovat, musíte dát boost z vašeho účtu.", diff --git a/app/javascript/mastodon/locales/es-MX.json b/app/javascript/mastodon/locales/es-MX.json index 26f070139f..473b489a0b 100644 --- a/app/javascript/mastodon/locales/es-MX.json +++ b/app/javascript/mastodon/locales/es-MX.json @@ -28,7 +28,7 @@ "account.enable_notifications": "Notificarme cuando @{name} publique algo", "account.endorse": "Destacar en mi perfil", "account.featured_tags.last_status_at": "Última publicación el {date}", - "account.featured_tags.last_status_never": "No hay publicaciones", + "account.featured_tags.last_status_never": "Sin publicaciones", "account.featured_tags.title": "Etiquetas destacadas de {name}", "account.follow": "Seguir", "account.follow_back": "Seguir también", @@ -146,7 +146,7 @@ "column.about": "Acerca de", "column.blocks": "Usuarios bloqueados", "column.bookmarks": "Marcadores", - "column.community": "Línea de tiempo local", + "column.community": "Cronología local", "column.create_list": "Crear lista", "column.direct": "Menciones privadas", "column.directory": "Buscar perfiles", diff --git a/app/javascript/mastodon/locales/ko.json b/app/javascript/mastodon/locales/ko.json index d1ee08dcad..88f35cfc30 100644 --- a/app/javascript/mastodon/locales/ko.json +++ b/app/javascript/mastodon/locales/ko.json @@ -414,6 +414,7 @@ "ignore_notifications_modal.not_followers_title": "나를 팔로우하지 않는 사람들의 알림을 무시할까요?", "ignore_notifications_modal.not_following_title": "내가 팔로우하지 않는 사람들의 알림을 무시할까요?", "ignore_notifications_modal.private_mentions_title": "요청하지 않은 개인 멘션 알림을 무시할까요?", + "info_button.label": "도움말", "interaction_modal.action.favourite": "계속하려면 내 계정으로 즐겨찾기해야 합니다.", "interaction_modal.action.follow": "계속하려면 내 계정으로 팔로우해야 합니다.", "interaction_modal.action.reblog": "계속하려면 내 계정으로 리블로그해야 합니다.", diff --git a/config/locales/cs.yml b/config/locales/cs.yml index 135975c3c8..3ca73a3a14 100644 --- a/config/locales/cs.yml +++ b/config/locales/cs.yml @@ -1247,6 +1247,7 @@ cs: too_fast: Formulář byl odeslán příliš rychle, zkuste to znovu. use_security_key: Použít bezpečnostní klíč user_agreement_html: Přečetl jsem si a souhlasím s podmínkami služby a ochranou osobních údajů + user_privacy_agreement_html: Četl jsem a souhlasím se zásadami ochrany osobních údajů author_attribution: example_title: Ukázkový text hint_html: Píšete novinové články nebo blog mimo Mastodon? Kontrolujte, jak Vám bude připisováno autorství, když jsou sdíleny na Mastodonu. diff --git a/config/locales/ko.yml b/config/locales/ko.yml index 1575e839e1..5405cabd9b 100644 --- a/config/locales/ko.yml +++ b/config/locales/ko.yml @@ -302,7 +302,7 @@ ko: deleted_account: 계정을 삭제했습니다 empty: 로그를 찾을 수 없습니다 filter_by_action: 동작 별 필터 - filter_by_user: 사용자로 거르기 + filter_by_user: 사용자 기준으로 필터 title: 감사 로그 unavailable_instance: "(도메인네임 사용불가)" announcements: @@ -1192,6 +1192,7 @@ ko: too_fast: 너무 빠르게 양식이 제출되었습니다, 다시 시도하세요. use_security_key: 보안 키 사용 user_agreement_html: 이용 약관과 개인정보처리방침을 읽었고 동의합니다 + user_privacy_agreement_html: 개인정보처리방침을 읽었고 동의합니다 author_attribution: example_title: 예시 텍스트 hint_html: 마스토돈 밖에서 뉴스나 블로그 글을 쓰시나요? 마스토돈에 공유되었을 때 어떻게 표시될지를 제어하세요. diff --git a/config/locales/simple_form.cs.yml b/config/locales/simple_form.cs.yml index 515c7fb9e8..37ad72a7e4 100644 --- a/config/locales/simple_form.cs.yml +++ b/config/locales/simple_form.cs.yml @@ -3,6 +3,7 @@ cs: simple_form: hints: account: + attribution_domains: Jeden na řádek. Chrání před falešným připisování autorství. discoverable: Vaše veřejné příspěvky a profil mohou být zobrazeny nebo doporučeny v různých oblastech Mastodonu a váš profil může být navrhován ostatním uživatelům. display_name: Vaše celé jméno nebo přezdívka. fields: Vaše domovská stránka, zájmena, věk, cokoliv chcete. @@ -155,6 +156,7 @@ cs: url: Kam budou události odesílány labels: account: + attribution_domains: Webové stránky s povolením Vám připsat autorství discoverable: Zobrazovat profil a příspěvky ve vyhledávacích algoritmech fields: name: Označení From 32aa83e9d79c5a11ea39c90ba3d6f67f091e6499 Mon Sep 17 00:00:00 2001 From: Claire Date: Tue, 28 Jan 2025 15:38:18 +0100 Subject: [PATCH 5/6] Fix polls not being validated on edition (#33755) --- app/models/poll.rb | 3 +- app/serializers/rest/instance_serializer.rb | 8 ++-- .../rest/v1/instance_serializer.rb | 8 ++-- app/validators/poll_expiration_validator.rb | 13 ++++++ ...validator.rb => poll_options_validator.rb} | 8 +--- spec/requests/api/v2/instance_spec.rb | 2 +- ...c.rb => poll_expiration_validator_spec.rb} | 16 +++++-- .../validators/poll_options_validator_spec.rb | 45 +++++++++++++++++++ 8 files changed, 82 insertions(+), 21 deletions(-) create mode 100644 app/validators/poll_expiration_validator.rb rename app/validators/{poll_validator.rb => poll_options_validator.rb} (57%) rename spec/validators/{poll_validator_spec.rb => poll_expiration_validator_spec.rb} (64%) create mode 100644 spec/validators/poll_options_validator_spec.rb diff --git a/app/models/poll.rb b/app/models/poll.rb index 93ef0cc589..7c59339b7e 100644 --- a/app/models/poll.rb +++ b/app/models/poll.rb @@ -37,7 +37,8 @@ class Poll < ApplicationRecord validates :options, presence: true validates :expires_at, presence: true, if: :local? - validates_with PollValidator, on: :create, if: :local? + validates_with PollOptionsValidator, if: :local? + validates_with PollExpirationValidator, if: -> { local? && expires_at_changed? } before_validation :prepare_options, if: :local? before_validation :prepare_votes_count diff --git a/app/serializers/rest/instance_serializer.rb b/app/serializers/rest/instance_serializer.rb index 936748707f..e15cba7cc8 100644 --- a/app/serializers/rest/instance_serializer.rb +++ b/app/serializers/rest/instance_serializer.rb @@ -87,10 +87,10 @@ class REST::InstanceSerializer < ActiveModel::Serializer }, polls: { - max_options: PollValidator::MAX_OPTIONS, - max_characters_per_option: PollValidator::MAX_OPTION_CHARS, - min_expiration: PollValidator::MIN_EXPIRATION, - max_expiration: PollValidator::MAX_EXPIRATION, + max_options: PollOptionsValidator::MAX_OPTIONS, + max_characters_per_option: PollOptionsValidator::MAX_OPTION_CHARS, + min_expiration: PollExpirationValidator::MIN_EXPIRATION, + max_expiration: PollExpirationValidator::MAX_EXPIRATION, }, translation: { diff --git a/app/serializers/rest/v1/instance_serializer.rb b/app/serializers/rest/v1/instance_serializer.rb index db83af4907..a1d51f50e4 100644 --- a/app/serializers/rest/v1/instance_serializer.rb +++ b/app/serializers/rest/v1/instance_serializer.rb @@ -70,10 +70,10 @@ class REST::V1::InstanceSerializer < ActiveModel::Serializer }, polls: { - max_options: PollValidator::MAX_OPTIONS, - max_characters_per_option: PollValidator::MAX_OPTION_CHARS, - min_expiration: PollValidator::MIN_EXPIRATION, - max_expiration: PollValidator::MAX_EXPIRATION, + max_options: PollOptionsValidator::MAX_OPTIONS, + max_characters_per_option: PollOptionsValidator::MAX_OPTION_CHARS, + min_expiration: PollExpirationValidator::MIN_EXPIRATION, + max_expiration: PollExpirationValidator::MAX_EXPIRATION, }, } end diff --git a/app/validators/poll_expiration_validator.rb b/app/validators/poll_expiration_validator.rb new file mode 100644 index 0000000000..ea8b08e186 --- /dev/null +++ b/app/validators/poll_expiration_validator.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class PollExpirationValidator < ActiveModel::Validator + MAX_EXPIRATION = 1.month.freeze + MIN_EXPIRATION = 5.minutes.freeze + + def validate(poll) + current_time = Time.now.utc + + poll.errors.add(:expires_at, I18n.t('polls.errors.duration_too_long')) if poll.expires_at.nil? || poll.expires_at - current_time > MAX_EXPIRATION + poll.errors.add(:expires_at, I18n.t('polls.errors.duration_too_short')) if poll.expires_at.present? && (poll.expires_at - current_time).ceil < MIN_EXPIRATION + end +end diff --git a/app/validators/poll_validator.rb b/app/validators/poll_options_validator.rb similarity index 57% rename from app/validators/poll_validator.rb rename to app/validators/poll_options_validator.rb index a327277963..0ac84f93f4 100644 --- a/app/validators/poll_validator.rb +++ b/app/validators/poll_options_validator.rb @@ -1,19 +1,13 @@ # frozen_string_literal: true -class PollValidator < ActiveModel::Validator +class PollOptionsValidator < ActiveModel::Validator MAX_OPTIONS = 4 MAX_OPTION_CHARS = 50 - MAX_EXPIRATION = 1.month.freeze - MIN_EXPIRATION = 5.minutes.freeze def validate(poll) - current_time = Time.now.utc - poll.errors.add(:options, I18n.t('polls.errors.too_few_options')) unless poll.options.size > 1 poll.errors.add(:options, I18n.t('polls.errors.too_many_options', max: MAX_OPTIONS)) if poll.options.size > MAX_OPTIONS poll.errors.add(:options, I18n.t('polls.errors.over_character_limit', max: MAX_OPTION_CHARS)) if poll.options.any? { |option| option.mb_chars.grapheme_length > MAX_OPTION_CHARS } poll.errors.add(:options, I18n.t('polls.errors.duplicate_options')) unless poll.options.uniq.size == poll.options.size - poll.errors.add(:expires_at, I18n.t('polls.errors.duration_too_long')) if poll.expires_at.nil? || poll.expires_at - current_time > MAX_EXPIRATION - poll.errors.add(:expires_at, I18n.t('polls.errors.duration_too_short')) if poll.expires_at.present? && (poll.expires_at - current_time).ceil < MIN_EXPIRATION end end diff --git a/spec/requests/api/v2/instance_spec.rb b/spec/requests/api/v2/instance_spec.rb index bdccfdb626..788d30fa69 100644 --- a/spec/requests/api/v2/instance_spec.rb +++ b/spec/requests/api/v2/instance_spec.rb @@ -59,7 +59,7 @@ RSpec.describe 'Instances' do description_limit: MediaAttachment::MAX_DESCRIPTION_LENGTH ), polls: include( - max_options: PollValidator::MAX_OPTIONS + max_options: PollOptionsValidator::MAX_OPTIONS ) ) ) diff --git a/spec/validators/poll_validator_spec.rb b/spec/validators/poll_expiration_validator_spec.rb similarity index 64% rename from spec/validators/poll_validator_spec.rb rename to spec/validators/poll_expiration_validator_spec.rb index f2a2534898..41b8c96211 100644 --- a/spec/validators/poll_validator_spec.rb +++ b/spec/validators/poll_expiration_validator_spec.rb @@ -2,7 +2,7 @@ require 'rails_helper' -RSpec.describe PollValidator do +RSpec.describe PollExpirationValidator do describe '#validate' do before do validator.validate(poll) @@ -14,16 +14,24 @@ RSpec.describe PollValidator do let(:options) { %w(foo bar) } let(:expires_at) { 1.day.from_now } - it 'have no errors' do + it 'has no errors' do expect(errors).to_not have_received(:add) end - context 'when expires is just 5 min ago' do + context 'when the poll expires in 5 min from now' do let(:expires_at) { 5.minutes.from_now } - it 'not calls errors add' do + it 'has no errors' do expect(errors).to_not have_received(:add) end end + + context 'when the poll expires in the past' do + let(:expires_at) { 5.minutes.ago } + + it 'has errors' do + expect(errors).to have_received(:add) + end + end end end diff --git a/spec/validators/poll_options_validator_spec.rb b/spec/validators/poll_options_validator_spec.rb new file mode 100644 index 0000000000..9e4ec744db --- /dev/null +++ b/spec/validators/poll_options_validator_spec.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe PollOptionsValidator do + describe '#validate' do + before do + validator.validate(poll) + end + + let(:validator) { described_class.new } + let(:poll) { instance_double(Poll, options: options, expires_at: expires_at, errors: errors) } + let(:errors) { instance_double(ActiveModel::Errors, add: nil) } + let(:options) { %w(foo bar) } + let(:expires_at) { 1.day.from_now } + + it 'has no errors' do + expect(errors).to_not have_received(:add) + end + + context 'when the poll has duplicate options' do + let(:options) { %w(foo foo) } + + it 'adds errors' do + expect(errors).to have_received(:add) + end + end + + context 'when the poll has no options' do + let(:options) { [] } + + it 'adds errors' do + expect(errors).to have_received(:add) + end + end + + context 'when the poll has too many options' do + let(:options) { Array.new(described_class::MAX_OPTIONS + 1) { |i| "option #{i}" } } + + it 'adds errors' do + expect(errors).to have_received(:add) + end + end + end +end From 5b291fcbe41564264954618fb1f4086a3be1c600 Mon Sep 17 00:00:00 2001 From: Claire Date: Tue, 28 Jan 2025 15:44:27 +0100 Subject: [PATCH 6/6] Fix incorrect signature after HTTP redirect (#33757) --- .../concerns/signature_verification.rb | 8 +-- app/lib/http_signature_draft.rb | 31 ++++++++++ app/lib/request.rb | 60 ++++++++++--------- spec/lib/request_spec.rb | 31 ++++++++-- 4 files changed, 91 insertions(+), 39 deletions(-) create mode 100644 app/lib/http_signature_draft.rb diff --git a/app/controllers/concerns/signature_verification.rb b/app/controllers/concerns/signature_verification.rb index 4ae63632c0..5f7ef8dd63 100644 --- a/app/controllers/concerns/signature_verification.rb +++ b/app/controllers/concerns/signature_verification.rb @@ -117,7 +117,7 @@ module SignatureVerification def verify_signature_strength! raise SignatureVerificationError, 'Mastodon requires the Date header or (created) pseudo-header to be signed' unless signed_headers.include?('date') || signed_headers.include?('(created)') - raise SignatureVerificationError, 'Mastodon requires the Digest header or (request-target) pseudo-header to be signed' unless signed_headers.include?(Request::REQUEST_TARGET) || signed_headers.include?('digest') + raise SignatureVerificationError, 'Mastodon requires the Digest header or (request-target) pseudo-header to be signed' unless signed_headers.include?(HttpSignatureDraft::REQUEST_TARGET) || signed_headers.include?('digest') raise SignatureVerificationError, 'Mastodon requires the Host header to be signed when doing a GET request' if request.get? && !signed_headers.include?('host') raise SignatureVerificationError, 'Mastodon requires the Digest header to be signed when doing a POST request' if request.post? && !signed_headers.include?('digest') end @@ -155,14 +155,14 @@ module SignatureVerification def build_signed_string(include_query_string: true) signed_headers.map do |signed_header| case signed_header - when Request::REQUEST_TARGET + when HttpSignatureDraft::REQUEST_TARGET if include_query_string - "#{Request::REQUEST_TARGET}: #{request.method.downcase} #{request.original_fullpath}" + "#{HttpSignatureDraft::REQUEST_TARGET}: #{request.method.downcase} #{request.original_fullpath}" else # Current versions of Mastodon incorrectly omit the query string from the (request-target) pseudo-header. # Therefore, temporarily support such incorrect signatures for compatibility. # TODO: remove eventually some time after release of the fixed version - "#{Request::REQUEST_TARGET}: #{request.method.downcase} #{request.path}" + "#{HttpSignatureDraft::REQUEST_TARGET}: #{request.method.downcase} #{request.path}" end when '(created)' raise SignatureVerificationError, 'Invalid pseudo-header (created) for rsa-sha256' unless signature_algorithm == 'hs2019' diff --git a/app/lib/http_signature_draft.rb b/app/lib/http_signature_draft.rb new file mode 100644 index 0000000000..fc0d498b29 --- /dev/null +++ b/app/lib/http_signature_draft.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +# This implements an older draft of HTTP Signatures: +# https://datatracker.ietf.org/doc/html/draft-cavage-http-signatures + +class HttpSignatureDraft + REQUEST_TARGET = '(request-target)' + + def initialize(keypair, key_id, full_path: true) + @keypair = keypair + @key_id = key_id + @full_path = full_path + end + + def request_target(verb, url) + if url.query.nil? || !@full_path + "#{verb} #{url.path}" + else + "#{verb} #{url.path}?#{url.query}" + end + end + + def sign(signed_headers, verb, url) + signed_headers = signed_headers.merge(REQUEST_TARGET => request_target(verb, url)) + signed_string = signed_headers.map { |key, value| "#{key.downcase}: #{value}" }.join("\n") + algorithm = 'rsa-sha256' + signature = Base64.strict_encode64(@keypair.sign(OpenSSL::Digest.new('SHA256'), signed_string)) + + "keyId=\"#{@key_id}\",algorithm=\"#{algorithm}\",headers=\"#{signed_headers.keys.join(' ').downcase}\",signature=\"#{signature}\"" + end +end diff --git a/app/lib/request.rb b/app/lib/request.rb index f984f0e63e..4e86cc2fdf 100644 --- a/app/lib/request.rb +++ b/app/lib/request.rb @@ -61,8 +61,6 @@ class PerOperationWithDeadline < HTTP::Timeout::PerOperation end class Request - REQUEST_TARGET = '(request-target)' - # We enforce a 5s timeout on DNS resolving, 5s timeout on socket opening # and 5s timeout on the TLS handshake, meaning the worst case should take # about 15s in total @@ -78,11 +76,18 @@ class Request @http_client = options.delete(:http_client) @allow_local = options.delete(:allow_local) @full_path = !options.delete(:omit_query_string) - @options = options.merge(socket_class: use_proxy? || @allow_local ? ProxySocket : Socket) - @options = @options.merge(timeout_class: PerOperationWithDeadline, timeout_options: TIMEOUT) + @options = { + follow: { + max_hops: 3, + on_redirect: ->(response, request) { re_sign_on_redirect(response, request) }, + }, + socket_class: use_proxy? || @allow_local ? ProxySocket : Socket, + }.merge(options) @options = @options.merge(proxy_url) if use_proxy? @headers = {} + @signing = nil + raise Mastodon::HostValidationError, 'Instance does not support hidden service connections' if block_hidden_service? set_common_headers! @@ -92,8 +97,9 @@ class Request def on_behalf_of(actor, sign_with: nil) raise ArgumentError, 'actor must not be nil' if actor.nil? - @actor = actor - @keypair = sign_with.present? ? OpenSSL::PKey::RSA.new(sign_with) : @actor.keypair + key_id = ActivityPub::TagManager.instance.key_uri_for(actor) + keypair = sign_with.present? ? OpenSSL::PKey::RSA.new(sign_with) : actor.keypair + @signing = HttpSignatureDraft.new(keypair, key_id, full_path: @full_path) self end @@ -119,7 +125,7 @@ class Request end def headers - (@actor ? @headers.merge('Signature' => signature) : @headers).without(REQUEST_TARGET) + (@signing ? @headers.merge('Signature' => signature) : @headers) end class << self @@ -134,14 +140,13 @@ class Request end def http_client - HTTP.use(:auto_inflate).follow(max_hops: 3) + HTTP.use(:auto_inflate) end end private def set_common_headers! - @headers[REQUEST_TARGET] = request_target @headers['User-Agent'] = Mastodon::Version.user_agent @headers['Host'] = @url.host @headers['Date'] = Time.now.utc.httpdate @@ -152,31 +157,28 @@ class Request @headers['Digest'] = "SHA-256=#{Digest::SHA256.base64digest(@options[:body])}" end - def request_target - if @url.query.nil? || !@full_path - "#{@verb} #{@url.path}" - else - "#{@verb} #{@url.path}?#{@url.query}" - end - end - def signature - algorithm = 'rsa-sha256' - signature = Base64.strict_encode64(@keypair.sign(OpenSSL::Digest.new('SHA256'), signed_string)) - - "keyId=\"#{key_id}\",algorithm=\"#{algorithm}\",headers=\"#{signed_headers.keys.join(' ').downcase}\",signature=\"#{signature}\"" + @signing.sign(@headers.without('User-Agent', 'Accept-Encoding'), @verb, @url) end - def signed_string - signed_headers.map { |key, value| "#{key.downcase}: #{value}" }.join("\n") - end + def re_sign_on_redirect(_response, request) + # Delete existing signature if there is one, since it will be invalid + request.headers.delete('Signature') - def signed_headers - @headers.without('User-Agent', 'Accept-Encoding') - end + return unless @signing.present? && @verb == :get - def key_id - ActivityPub::TagManager.instance.key_uri_for(@actor) + signed_headers = request.headers.to_h.slice(*@headers.keys) + unless @headers.keys.all? { |key| signed_headers.key?(key) } + # We have lost some headers in the process, so don't sign the new + # request, in order to avoid issuing a valid signature with fewer + # conditions than expected. + + Rails.logger.warn { "Some headers (#{@headers.keys - signed_headers.keys}) have been lost on redirect from {@uri} to #{request.uri}, this should not happen. Skipping signatures" } + return + end + + signature_value = @signing.sign(signed_headers.without('User-Agent', 'Accept-Encoding'), @verb, Addressable::URI.parse(request.uri)) + request.headers['Signature'] = signature_value end def http_client diff --git a/spec/lib/request_spec.rb b/spec/lib/request_spec.rb index f17cf637b9..79c400e098 100644 --- a/spec/lib/request_spec.rb +++ b/spec/lib/request_spec.rb @@ -60,16 +60,12 @@ RSpec.describe Request do expect(a_request(:get, 'http://example.com')).to have_been_made.once end - it 'sets headers' do - expect { |block| subject.perform(&block) }.to yield_control - expect(a_request(:get, 'http://example.com').with(headers: subject.headers)).to have_been_made - end - - it 'closes underlying connection' do + it 'makes a request with expected headers, yields, and closes the underlying connection' do allow(subject.send(:http_client)).to receive(:close) expect { |block| subject.perform(&block) }.to yield_control + expect(a_request(:get, 'http://example.com').with(headers: subject.headers)).to have_been_made expect(subject.send(:http_client)).to have_received(:close) end @@ -80,6 +76,29 @@ RSpec.describe Request do end end + context 'with a redirect and HTTP signatures' do + let(:account) { Fabricate(:account) } + + before do + stub_request(:get, 'http://example.com').to_return(status: 301, headers: { Location: 'http://redirected.example.com/foo' }) + stub_request(:get, 'http://redirected.example.com/foo').to_return(body: 'lorem ipsum') + end + + it 'makes a request with expected headers and follows redirects' do + expect { |block| subject.on_behalf_of(account).perform(&block) }.to yield_control + + # request.headers includes the `Signature` sent for the first request + expect(a_request(:get, 'http://example.com').with(headers: subject.headers)).to have_been_made.once + + # request.headers includes the `Signature`, but it has changed + expect(a_request(:get, 'http://redirected.example.com/foo').with(headers: subject.headers.merge({ 'Host' => 'redirected.example.com' }))).to_not have_been_made + + # `with(headers: )` matching tests for inclusion, so strip `Signature` + # This doesn't actually test that there is a signature, but it tests that the original signature is not passed + expect(a_request(:get, 'http://redirected.example.com/foo').with(headers: subject.headers.without('Signature').merge({ 'Host' => 'redirected.example.com' }))).to have_been_made.once + end + end + context 'with private host' do around do |example| WebMock.disable!