From dec1fb71f4958771e8bb80d5415956fd3428e429 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 15 Jul 2025 02:27:36 -0400 Subject: [PATCH 01/14] Add coverage for `FollowRecommendationMute` model (#35376) --- app/models/follow_recommendation_mute.rb | 2 +- .../models/follow_recommendation_mute_spec.rb | 30 +++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) create mode 100644 spec/models/follow_recommendation_mute_spec.rb diff --git a/app/models/follow_recommendation_mute.rb b/app/models/follow_recommendation_mute.rb index d166d0a620..ef931988e7 100644 --- a/app/models/follow_recommendation_mute.rb +++ b/app/models/follow_recommendation_mute.rb @@ -14,7 +14,7 @@ class FollowRecommendationMute < ApplicationRecord belongs_to :account belongs_to :target_account, class_name: 'Account' - validates :target_account, uniqueness: { scope: :account_id } + validates :target_account_id, uniqueness: { scope: :account_id } after_commit :invalidate_follow_recommendations_cache diff --git a/spec/models/follow_recommendation_mute_spec.rb b/spec/models/follow_recommendation_mute_spec.rb new file mode 100644 index 0000000000..0141d9042a --- /dev/null +++ b/spec/models/follow_recommendation_mute_spec.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe FollowRecommendationMute do + describe 'Associations' do + it { is_expected.to belong_to(:account) } + it { is_expected.to belong_to(:target_account).class_name('Account') } + end + + describe 'Validations' do + subject { Fabricate.build :follow_recommendation_mute } + + it { is_expected.to validate_uniqueness_of(:target_account_id).scoped_to(:account_id) } + end + + describe 'Callbacks' do + describe 'Maintaining the recommendation cache' do + let(:account) { Fabricate :account } + let(:cache_key) { "follow_recommendations/#{account.id}" } + + before { Rails.cache.write(cache_key, 123) } + + it 'purges on save' do + expect { Fabricate :follow_recommendation_mute, account: account } + .to(change { Rails.cache.exist?(cache_key) }.from(true).to(false)) + end + end + end +end From 16372970859ba2edc2655aa2b222cf1d19ff9e47 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 15 Jul 2025 02:28:40 -0400 Subject: [PATCH 02/14] Add coverage for `CustomFilterStatus` model (#35374) --- app/models/custom_filter_status.rb | 8 +++--- spec/models/custom_filter_status_spec.rb | 33 ++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) create mode 100644 spec/models/custom_filter_status_spec.rb diff --git a/app/models/custom_filter_status.rb b/app/models/custom_filter_status.rb index 58b61cd79d..c85b811280 100644 --- a/app/models/custom_filter_status.rb +++ b/app/models/custom_filter_status.rb @@ -17,12 +17,14 @@ class CustomFilterStatus < ApplicationRecord belongs_to :custom_filter belongs_to :status - validates :status, uniqueness: { scope: :custom_filter } - validate :validate_status_access + validates :status_id, uniqueness: { scope: :custom_filter_id } + validate :validate_status_access, if: [:custom_filter_account, :status] + + delegate :account, to: :custom_filter, prefix: true, allow_nil: true private def validate_status_access - errors.add(:status_id, :invalid) unless StatusPolicy.new(custom_filter.account, status).show? + errors.add(:status_id, :invalid) unless StatusPolicy.new(custom_filter_account, status).show? end end diff --git a/spec/models/custom_filter_status_spec.rb b/spec/models/custom_filter_status_spec.rb new file mode 100644 index 0000000000..3f161c00bd --- /dev/null +++ b/spec/models/custom_filter_status_spec.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe CustomFilterStatus do + describe 'Associations' do + it { is_expected.to belong_to(:custom_filter) } + it { is_expected.to belong_to(:status) } + end + + describe 'Validations' do + subject { Fabricate.build :custom_filter_status } + + it { is_expected.to validate_uniqueness_of(:status_id).scoped_to(:custom_filter_id) } + + describe 'Status access' do + subject { Fabricate.build :custom_filter_status, custom_filter:, status: } + + let(:custom_filter) { Fabricate :custom_filter } + let(:status) { Fabricate :status } + + context 'when policy allows filter account to access status' do + it { is_expected.to allow_value(status.id).for(:status_id) } + end + + context 'when policy does not allow filter account to access status' do + before { status.account.touch(:suspended_at) } + + it { is_expected.to_not allow_value(status.id).for(:status_id) } + end + end + end +end From 30344d6abf4e81d191e563358ec19b7a9edeb4b4 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 15 Jul 2025 02:31:00 -0400 Subject: [PATCH 03/14] Confirm `User#login_activities` in auth/sessions spec (#35372) --- .../auth/omniauth_callbacks_controller.rb | 3 +- app/controllers/auth/sessions_controller.rb | 31 +++++++++++-------- .../settings/login_activities_controller.rb | 2 +- app/models/user.rb | 1 + .../auth/sessions_controller_spec.rb | 19 +++++++++--- spec/models/user_spec.rb | 1 + 6 files changed, 36 insertions(+), 21 deletions(-) diff --git a/app/controllers/auth/omniauth_callbacks_controller.rb b/app/controllers/auth/omniauth_callbacks_controller.rb index 9d496220a3..16de03fd72 100644 --- a/app/controllers/auth/omniauth_callbacks_controller.rb +++ b/app/controllers/auth/omniauth_callbacks_controller.rb @@ -38,8 +38,7 @@ class Auth::OmniauthCallbacksController < Devise::OmniauthCallbacksController private def record_login_activity - LoginActivity.create( - user: @user, + @user.login_activities.create( success: true, authentication_method: :omniauth, provider: @provider, diff --git a/app/controllers/auth/sessions_controller.rb b/app/controllers/auth/sessions_controller.rb index 2808066aaf..c52bda67b0 100644 --- a/app/controllers/auth/sessions_controller.rb +++ b/app/controllers/auth/sessions_controller.rb @@ -151,12 +151,11 @@ class Auth::SessionsController < Devise::SessionsController sign_in(user) flash.delete(:notice) - LoginActivity.create( - user: user, - success: true, - authentication_method: security_measure, - ip: request.remote_ip, - user_agent: request.user_agent + user.login_activities.create( + request_details.merge( + authentication_method: security_measure, + success: true + ) ) UserMailer.suspicious_sign_in(user, request.remote_ip, request.user_agent, Time.now.utc).deliver_later! if @login_is_suspicious @@ -167,13 +166,12 @@ class Auth::SessionsController < Devise::SessionsController end def on_authentication_failure(user, security_measure, failure_reason) - LoginActivity.create( - user: user, - success: false, - authentication_method: security_measure, - failure_reason: failure_reason, - ip: request.remote_ip, - user_agent: request.user_agent + user.login_activities.create( + request_details.merge( + authentication_method: security_measure, + failure_reason: failure_reason, + success: false + ) ) # Only send a notification email every hour at most @@ -182,6 +180,13 @@ class Auth::SessionsController < Devise::SessionsController UserMailer.failed_2fa(user, request.remote_ip, request.user_agent, Time.now.utc).deliver_later! end + def request_details + { + ip: request.remote_ip, + user_agent: request.user_agent, + } + end + def second_factor_attempts_key(user) "2fa_auth_attempts:#{user.id}:#{Time.now.utc.hour}" end diff --git a/app/controllers/settings/login_activities_controller.rb b/app/controllers/settings/login_activities_controller.rb index 50e2d70cb9..ae32dbf557 100644 --- a/app/controllers/settings/login_activities_controller.rb +++ b/app/controllers/settings/login_activities_controller.rb @@ -5,6 +5,6 @@ class Settings::LoginActivitiesController < Settings::BaseController skip_before_action :require_functional! def index - @login_activities = LoginActivity.where(user: current_user).order(id: :desc).page(params[:page]) + @login_activities = current_user.login_activities.order(id: :desc).page(params[:page]) end end diff --git a/app/models/user.rb b/app/models/user.rb index 966ffbe9c3..762522f282 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -90,6 +90,7 @@ class User < ApplicationRecord has_many :applications, class_name: 'Doorkeeper::Application', as: :owner, dependent: nil has_many :backups, inverse_of: :user, dependent: nil has_many :invites, inverse_of: :user, dependent: nil + has_many :login_activities, inverse_of: :user, dependent: :destroy has_many :markers, inverse_of: :user, dependent: :destroy has_many :webauthn_credentials, dependent: :destroy has_many :ips, class_name: 'UserIp', inverse_of: :user, dependent: nil diff --git a/spec/controllers/auth/sessions_controller_spec.rb b/spec/controllers/auth/sessions_controller_spec.rb index c852b16a69..949af2a425 100644 --- a/spec/controllers/auth/sessions_controller_spec.rb +++ b/spec/controllers/auth/sessions_controller_spec.rb @@ -100,11 +100,14 @@ RSpec.describe Auth::SessionsController do let(:user) { Fabricate(:user, email: 'foo@bar.com', password: 'abcdefgh') } context 'when using a valid password' do - before do + subject do post :create, params: { user: { email: user.email, password: user.password } } end it 'redirects to home and logs the user in' do + expect { subject } + .to change(user.login_activities.where(success: true), :count).by(1) + expect(response).to redirect_to(root_path) expect(controller.current_user).to eq user @@ -265,10 +268,9 @@ RSpec.describe Auth::SessionsController do it 'does not log the user in, sets a flash message, and sends a suspicious sign in email', :inline_jobs do emails = capture_emails do - Auth::SessionsController::MAX_2FA_ATTEMPTS_PER_HOUR.times do - post :create, params: { user: { otp_attempt: '1234' } }, session: { attempt_user_id: user.id, attempt_user_updated_at: user.updated_at.to_s } - expect(controller.current_user).to be_nil - end + expect { process_maximum_two_factor_attempts } + .to change(user.login_activities.where(success: false), :count).by(1) + post :create, params: { user: { otp_attempt: user.current_otp } }, session: { attempt_user_id: user.id, attempt_user_updated_at: user.updated_at.to_s } end @@ -286,6 +288,13 @@ RSpec.describe Auth::SessionsController do subject: eq(I18n.t('user_mailer.failed_2fa.subject')) ) end + + def process_maximum_two_factor_attempts + Auth::SessionsController::MAX_2FA_ATTEMPTS_PER_HOUR.times do + post :create, params: { user: { otp_attempt: '1234' } }, session: { attempt_user_id: user.id, attempt_user_updated_at: user.updated_at.to_s } + expect(controller.current_user).to be_nil + end + end end context 'when using a valid OTP' do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index b732b2d84d..c71b7a600b 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -24,6 +24,7 @@ RSpec.describe User do describe 'Associations' do it { is_expected.to belong_to(:account).required } + it { is_expected.to have_many(:login_activities) } end describe 'Validations' do From 6c2db9b1cf36d146556ade8cec7f6cba93ee6774 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Tue, 15 Jul 2025 06:32:00 +0000 Subject: [PATCH 04/14] fix(deps): update dependency vite-plugin-static-copy to v3.1.1 (#35367) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- yarn.lock | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/yarn.lock b/yarn.lock index ea96821fbe..f06168c04a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5764,7 +5764,7 @@ __metadata: languageName: node linkType: hard -"chokidar@npm:^3.5.3": +"chokidar@npm:^3.6.0": version: 3.6.0 resolution: "chokidar@npm:3.6.0" dependencies: @@ -13904,17 +13904,17 @@ __metadata: linkType: hard "vite-plugin-static-copy@npm:^3.1.0": - version: 3.1.0 - resolution: "vite-plugin-static-copy@npm:3.1.0" + version: 3.1.1 + resolution: "vite-plugin-static-copy@npm:3.1.1" dependencies: - chokidar: "npm:^3.5.3" + chokidar: "npm:^3.6.0" fs-extra: "npm:^11.3.0" p-map: "npm:^7.0.3" picocolors: "npm:^1.1.1" tinyglobby: "npm:^0.2.14" peerDependencies: vite: ^5.0.0 || ^6.0.0 || ^7.0.0 - checksum: 10c0/dce43f12ecc71417f1afd530d15b316774fe0441c2502e48e2bfafcd07fd4ae90a5782621f932d8d12a8c8213bed6746e80d5452e2fb216ece2bcf7e80309f82 + checksum: 10c0/a4dd5d31212b037d4902d55c2ee83866e496857bf948f258599dc24ec61b4628cf0dd23e4a7d7dc189d33ad1489427e976fa95e4db61b394d0be4f63077ef92c languageName: node linkType: hard From 4b8e60682de0bf009069c074072e2ec145deee0d Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Tue, 15 Jul 2025 06:32:38 +0000 Subject: [PATCH 05/14] fix(deps): update dependency react-select to v5.10.2 (#35352) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- yarn.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/yarn.lock b/yarn.lock index f06168c04a..04b93f4ade 100644 --- a/yarn.lock +++ b/yarn.lock @@ -11298,8 +11298,8 @@ __metadata: linkType: hard "react-select@npm:^5.7.3": - version: 5.10.1 - resolution: "react-select@npm:5.10.1" + version: 5.10.2 + resolution: "react-select@npm:5.10.2" dependencies: "@babel/runtime": "npm:^7.12.0" "@emotion/cache": "npm:^11.4.0" @@ -11313,7 +11313,7 @@ __metadata: peerDependencies: react: ^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0 react-dom: ^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0 - checksum: 10c0/0d10a249b96150bd648f2575d59c848b8fac7f4d368a97ae84e4aaba5bbc1035deba4cdc82e49a43904b79ec50494505809618b0e98022b2d51e7629551912ed + checksum: 10c0/2027ef57b0a375d1accdad60550329dc0911b31d429ec6eb9ae391ae9baf9f26991b0ff8615eb99de319a55ce6e9382107783e615cb2116522ed0275b214b7f7 languageName: node linkType: hard From 82a6ff091f79c4933a10dc901db1411625629013 Mon Sep 17 00:00:00 2001 From: diondiondion Date: Tue, 15 Jul 2025 09:52:34 +0200 Subject: [PATCH 06/14] fix: Improve `Dropdown` component accessibility (#35373) --- .../mastodon/components/dropdown_menu.tsx | 80 ++++++------------- .../mastodon/components/icon_button.tsx | 13 --- .../navigation_panel/components/more_link.tsx | 26 +++--- .../styles/mastodon/components.scss | 8 +- 4 files changed, 47 insertions(+), 80 deletions(-) diff --git a/app/javascript/mastodon/components/dropdown_menu.tsx b/app/javascript/mastodon/components/dropdown_menu.tsx index 23d77f0dda..d9c87e93a7 100644 --- a/app/javascript/mastodon/components/dropdown_menu.tsx +++ b/app/javascript/mastodon/components/dropdown_menu.tsx @@ -5,6 +5,7 @@ import { useCallback, cloneElement, Children, + useId, } from 'react'; import classNames from 'classnames'; @@ -16,6 +17,7 @@ import Overlay from 'react-overlays/Overlay'; import type { OffsetValue, UsePopperOptions, + Placement, } from 'react-overlays/esm/usePopper'; import { fetchRelationships } from 'mastodon/actions/accounts'; @@ -295,6 +297,11 @@ interface DropdownProps { title?: string; disabled?: boolean; scrollable?: boolean; + placement?: Placement; + /** + * Prevent the `ScrollableList` with this scrollKey + * from being scrolled while the dropdown is open + */ scrollKey?: string; status?: ImmutableMap; forceDropdown?: boolean; @@ -316,6 +323,7 @@ export const Dropdown = ({ title = 'Menu', disabled, scrollable, + placement = 'bottom', status, forceDropdown = false, renderItem, @@ -331,16 +339,15 @@ export const Dropdown = ({ ); const [currentId] = useState(id++); const open = currentId === openDropdownId; - const activeElement = useRef(null); - const targetRef = useRef(null); + const buttonRef = useRef(null); + const menuId = useId(); const prefetchAccountId = status ? status.getIn(['account', 'id']) : undefined; const handleClose = useCallback(() => { - if (activeElement.current) { - activeElement.current.focus({ preventScroll: true }); - activeElement.current = null; + if (buttonRef.current) { + buttonRef.current.focus({ preventScroll: true }); } dispatch( @@ -375,7 +382,7 @@ export const Dropdown = ({ [handleClose, onItemClick, items], ); - const handleClick = useCallback( + const toggleDropdown = useCallback( (e: React.MouseEvent | React.KeyboardEvent) => { const { type } = e; @@ -423,38 +430,6 @@ export const Dropdown = ({ ], ); - const handleMouseDown = useCallback(() => { - if (!open && document.activeElement instanceof HTMLElement) { - activeElement.current = document.activeElement; - } - }, [open]); - - const handleButtonKeyDown = useCallback( - (e: React.KeyboardEvent) => { - switch (e.key) { - case ' ': - case 'Enter': - handleMouseDown(); - break; - } - }, - [handleMouseDown], - ); - - const handleKeyPress = useCallback( - (e: React.KeyboardEvent) => { - switch (e.key) { - case ' ': - case 'Enter': - handleClick(e); - e.stopPropagation(); - e.preventDefault(); - break; - } - }, - [handleClick], - ); - useEffect(() => { return () => { if (currentId === openDropdownId) { @@ -465,14 +440,16 @@ export const Dropdown = ({ let button: React.ReactElement; + const buttonProps = { + disabled, + onClick: toggleDropdown, + 'aria-expanded': open, + 'aria-controls': menuId, + ref: buttonRef, + }; + if (children) { - button = cloneElement(Children.only(children), { - onClick: handleClick, - onMouseDown: handleMouseDown, - onKeyDown: handleButtonKeyDown, - onKeyPress: handleKeyPress, - ref: targetRef, - }); + button = cloneElement(Children.only(children), buttonProps); } else if (icon && iconComponent) { button = ( ({ iconComponent={iconComponent} title={title} active={open} - disabled={disabled} - onClick={handleClick} - onMouseDown={handleMouseDown} - onKeyDown={handleButtonKeyDown} - onKeyPress={handleKeyPress} - ref={targetRef} + {...buttonProps} /> ); } else { @@ -499,13 +471,13 @@ export const Dropdown = ({ {({ props, arrowProps, placement }) => ( -
+
; onMouseDown?: React.MouseEventHandler; onKeyDown?: React.KeyboardEventHandler; - onKeyPress?: React.KeyboardEventHandler; active?: boolean; expanded?: boolean; style?: React.CSSProperties; @@ -45,7 +44,6 @@ export const IconButton = forwardRef( activeStyle, onClick, onKeyDown, - onKeyPress, onMouseDown, active = false, disabled = false, @@ -85,16 +83,6 @@ export const IconButton = forwardRef( [disabled, onClick], ); - const handleKeyPress: React.KeyboardEventHandler = - useCallback( - (e) => { - if (!disabled) { - onKeyPress?.(e); - } - }, - [disabled, onKeyPress], - ); - const handleMouseDown: React.MouseEventHandler = useCallback( (e) => { @@ -161,7 +149,6 @@ export const IconButton = forwardRef( onClick={handleClick} onMouseDown={handleMouseDown} onKeyDown={handleKeyDown} - onKeyPress={handleKeyPress} // eslint-disable-line @typescript-eslint/no-deprecated style={buttonStyle} tabIndex={tabIndex} disabled={disabled} diff --git a/app/javascript/mastodon/features/navigation_panel/components/more_link.tsx b/app/javascript/mastodon/features/navigation_panel/components/more_link.tsx index a26935eacf..a3477ec4e5 100644 --- a/app/javascript/mastodon/features/navigation_panel/components/more_link.tsx +++ b/app/javascript/mastodon/features/navigation_panel/components/more_link.tsx @@ -50,16 +50,22 @@ export const MoreLink: React.FC = () => { const menu = useMemo(() => { const arr: MenuItem[] = [ - { text: intl.formatMessage(messages.filters), href: '/filters' }, - { text: intl.formatMessage(messages.mutes), to: '/mutes' }, - { text: intl.formatMessage(messages.blocks), to: '/blocks' }, { - text: intl.formatMessage(messages.domainBlocks), - to: '/domain_blocks', + href: '/filters', + text: intl.formatMessage(messages.filters), + }, + { + to: '/mutes', + text: intl.formatMessage(messages.mutes), + }, + { + to: '/blocks', + text: intl.formatMessage(messages.blocks), + }, + { + to: '/domain_blocks', + text: intl.formatMessage(messages.domainBlocks), }, - ]; - - arr.push( null, { href: '/settings/privacy', @@ -77,7 +83,7 @@ export const MoreLink: React.FC = () => { href: '/settings/export', text: intl.formatMessage(messages.importExport), }, - ); + ]; if (canManageReports(permissions)) { arr.push(null, { @@ -106,7 +112,7 @@ export const MoreLink: React.FC = () => { }, [intl, dispatch, permissions]); return ( - +