From 6eea3f8f9c41114002782d7a44993004bdd64832 Mon Sep 17 00:00:00 2001 From: Claire Date: Thu, 20 Jan 2022 13:37:31 +0100 Subject: [PATCH 1/9] Add post edited notice in admin and public UIs (#17335) * Add edited toot flag on public pages * Add toot edit flag to admin pages --- app/views/admin/reports/_status.html.haml | 3 +++ app/views/statuses/_detailed_status.html.haml | 5 +++++ app/views/statuses/_simple_status.html.haml | 3 +++ config/locales/en.yml | 1 + 4 files changed, 12 insertions(+) diff --git a/app/views/admin/reports/_status.html.haml b/app/views/admin/reports/_status.html.haml index 924b0e9c255..4e06d4bbf0c 100644 --- a/app/views/admin/reports/_status.html.haml +++ b/app/views/admin/reports/_status.html.haml @@ -27,6 +27,9 @@ · = link_to ActivityPub::TagManager.instance.url_for(status), class: 'detailed-status__datetime', target: stream_link_target, rel: 'noopener noreferrer' do %time.formatted{ datetime: status.created_at.iso8601, title: l(status.created_at) }= l(status.created_at) + - if status.edited? + · + = t('statuses.edited_at', date: l(status.edited_at)) - if status.discarded? · %span.negative-hint= t('admin.statuses.deleted') diff --git a/app/views/statuses/_detailed_status.html.haml b/app/views/statuses/_detailed_status.html.haml index 6b3b8130672..cd5ed52af4a 100644 --- a/app/views/statuses/_detailed_status.html.haml +++ b/app/views/statuses/_detailed_status.html.haml @@ -37,10 +37,15 @@ .detailed-status__meta %data.dt-published{ value: status.created_at.to_time.iso8601 } + - if status.edited? + %data.dt-updated{ value: status.edited_at.to_time.iso8601 } = link_to ActivityPub::TagManager.instance.url_for(status), class: 'detailed-status__datetime u-url u-uid', target: stream_link_target, rel: 'noopener noreferrer' do %time.formatted{ datetime: status.created_at.iso8601, title: l(status.created_at) }= l(status.created_at) · + - if status.edited? + = t('statuses.edited_at', date: l(status.edited_at)) + · %span.detailed-status__visibility-icon = visibility_icon status · diff --git a/app/views/statuses/_simple_status.html.haml b/app/views/statuses/_simple_status.html.haml index 728e6b9b096..0139d2016c3 100644 --- a/app/views/statuses/_simple_status.html.haml +++ b/app/views/statuses/_simple_status.html.haml @@ -7,6 +7,9 @@ %span.status__visibility-icon>< = visibility_icon status %time.time-ago{ datetime: status.created_at.iso8601, title: l(status.created_at) }= l(status.created_at) + - if status.edited? + %abbr{ title: t('statuses.edited_at', date: l(status.edited_at.to_date)) } + * %data.dt-published{ value: status.created_at.to_time.iso8601 } .p-author.h-card diff --git a/config/locales/en.yml b/config/locales/en.yml index 36ac896643f..13a252a47b2 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1309,6 +1309,7 @@ en: disallowed_hashtags: one: 'contained a disallowed hashtag: %{tags}' other: 'contained the disallowed hashtags: %{tags}' + edited_at: Edited %{date} errors: in_reply_not_found: The post you are trying to reply to does not appear to exist. language_detection: Automatically detect language From 1e8c885e5a5a3456785578440168983ce21266ab Mon Sep 17 00:00:00 2001 From: Claire Date: Thu, 20 Jan 2022 14:51:23 +0100 Subject: [PATCH 2/9] Change mastodon:webpush:generate_vapid_key task to not require functional env (#17338) Fixes #17297 --- lib/tasks/mastodon.rake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/tasks/mastodon.rake b/lib/tasks/mastodon.rake index d905a07dad4..a89af677807 100644 --- a/lib/tasks/mastodon.rake +++ b/lib/tasks/mastodon.rake @@ -441,7 +441,7 @@ namespace :mastodon do namespace :webpush do desc 'Generate VAPID key' - task generate_vapid_key: :environment do + task :generate_vapid_key do vapid_key = Webpush.generate_key puts "VAPID_PRIVATE_KEY=#{vapid_key.private_key}" puts "VAPID_PUBLIC_KEY=#{vapid_key.public_key}" From 3a103cd317fd56aca27fca01e03647df44e3ffd2 Mon Sep 17 00:00:00 2001 From: Claire Date: Thu, 20 Jan 2022 20:56:21 +0100 Subject: [PATCH 3/9] Fix text being incorrectly pre-selected in composer textarea on /share (#17339) Fixes #17295 --- .../mastodon/features/compose/components/compose_form.js | 3 ++- .../features/compose/containers/compose_form_container.js | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/app/javascript/mastodon/features/compose/components/compose_form.js b/app/javascript/mastodon/features/compose/components/compose_form.js index ba2d20cc7e6..d224222ace5 100644 --- a/app/javascript/mastodon/features/compose/components/compose_form.js +++ b/app/javascript/mastodon/features/compose/components/compose_form.js @@ -60,6 +60,7 @@ class ComposeForm extends ImmutablePureComponent { onPickEmoji: PropTypes.func.isRequired, showSearch: PropTypes.bool, anyMedia: PropTypes.bool, + isInReply: PropTypes.bool, singleColumn: PropTypes.bool, }; @@ -149,7 +150,7 @@ class ComposeForm extends ImmutablePureComponent { if (this.props.focusDate !== prevProps.focusDate) { let selectionEnd, selectionStart; - if (this.props.preselectDate !== prevProps.preselectDate) { + if (this.props.preselectDate !== prevProps.preselectDate && this.props.isInReply) { selectionEnd = this.props.text.length; selectionStart = this.props.text.search(/\s/) + 1; } else if (typeof this.props.caretPosition === 'number') { diff --git a/app/javascript/mastodon/features/compose/containers/compose_form_container.js b/app/javascript/mastodon/features/compose/containers/compose_form_container.js index 37a0e8845b4..c44850294d0 100644 --- a/app/javascript/mastodon/features/compose/containers/compose_form_container.js +++ b/app/javascript/mastodon/features/compose/containers/compose_form_container.js @@ -25,6 +25,7 @@ const mapStateToProps = state => ({ isUploading: state.getIn(['compose', 'is_uploading']), showSearch: state.getIn(['search', 'submitted']) && !state.getIn(['search', 'hidden']), anyMedia: state.getIn(['compose', 'media_attachments']).size > 0, + isInReply: state.getIn(['compose', 'in_reply_to']) !== null, }); const mapDispatchToProps = (dispatch) => ({ From 96f0b33c8b994440199bccda14123d0569c6bcc5 Mon Sep 17 00:00:00 2001 From: Claire Date: Sun, 23 Jan 2022 13:53:58 +0100 Subject: [PATCH 4/9] Remove old duplicate index (#17245) Some Mastodon versions (v1.1 and v1.2) had a duplicate index in `db/schema.rb` without any migration script creating it. #2224 (included in v1.3) removed the duplicate index from the file but did not provide a migration script to remove it. This means that any instance that was installed from v1.1 or v1.2's source code has a duplicate index and a corresponding warning in PgHero. Instances set up using an earlier or later Mastodon version do not have this issue. This PR removes the duplicate index if it is present. --- .../20220105163928_remove_mentions_status_id_index.rb | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 db/migrate/20220105163928_remove_mentions_status_id_index.rb diff --git a/db/migrate/20220105163928_remove_mentions_status_id_index.rb b/db/migrate/20220105163928_remove_mentions_status_id_index.rb new file mode 100644 index 00000000000..56e90371928 --- /dev/null +++ b/db/migrate/20220105163928_remove_mentions_status_id_index.rb @@ -0,0 +1,9 @@ +class RemoveMentionsStatusIdIndex < ActiveRecord::Migration[6.1] + def up + remove_index :mentions, name: :mentions_status_id_index if index_exists?(:mentions, :status_id, name: :mentions_status_id_index) + end + + def down + # As this index should not exist and is a duplicate of another index, do not re-create it + end +end From 8a07ecd3773b1beae607bfe1edde62104654d64f Mon Sep 17 00:00:00 2001 From: Claire Date: Sun, 23 Jan 2022 15:46:30 +0100 Subject: [PATCH 5/9] Remove leftover database columns from Devise::Models::Rememberable (#17191) * Remove leftover database columns from Devise::Models::Rememberable * Update fix-duplication maintenance script * Improve errors/warnings in the fix-duplicates maintenance script --- app/models/user.rb | 12 ++++--- ...10_remove_index_users_on_remember_token.rb | 13 +++++++ ...18183123_remove_rememberable_from_users.rb | 8 +++++ db/schema.rb | 5 +-- lib/mastodon/maintenance_cli.rb | 34 ++++++++++++------- 5 files changed, 51 insertions(+), 21 deletions(-) create mode 100644 db/post_migrate/20220118183010_remove_index_users_on_remember_token.rb create mode 100644 db/post_migrate/20220118183123_remove_rememberable_from_users.rb diff --git a/app/models/user.rb b/app/models/user.rb index 49dcb81560c..c2bc5b5901f 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -10,7 +10,6 @@ # encrypted_password :string default(""), not null # reset_password_token :string # reset_password_sent_at :datetime -# remember_created_at :datetime # sign_in_count :integer default(0), not null # current_sign_in_at :datetime # last_sign_in_at :datetime @@ -32,7 +31,6 @@ # disabled :boolean default(FALSE), not null # moderator :boolean default(FALSE), not null # invite_id :bigint(8) -# remember_token :string # chosen_languages :string is an Array # created_by_application_id :bigint(8) # approved :boolean default(TRUE), not null @@ -44,6 +42,11 @@ # class User < ApplicationRecord + self.ignored_columns = %w( + remember_created_at + remember_token + ) + include Settings::Extend include UserRoles @@ -329,10 +332,9 @@ class User < ApplicationRecord end def reset_password! - # First, change password to something random, invalidate the remember-me token, - # and deactivate all sessions + # First, change password to something random and deactivate all sessions transaction do - update(remember_token: nil, remember_created_at: nil, password: SecureRandom.hex) + update(password: SecureRandom.hex) session_activations.destroy_all end diff --git a/db/post_migrate/20220118183010_remove_index_users_on_remember_token.rb b/db/post_migrate/20220118183010_remove_index_users_on_remember_token.rb new file mode 100644 index 00000000000..367d489de94 --- /dev/null +++ b/db/post_migrate/20220118183010_remove_index_users_on_remember_token.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class RemoveIndexUsersOnRememberToken < ActiveRecord::Migration[6.1] + disable_ddl_transaction! + + def up + remove_index :users, name: :index_users_on_remember_token + end + + def down + add_index :users, :remember_token, algorithm: :concurrently, unique: true, name: :index_users_on_remember_token + end +end diff --git a/db/post_migrate/20220118183123_remove_rememberable_from_users.rb b/db/post_migrate/20220118183123_remove_rememberable_from_users.rb new file mode 100644 index 00000000000..1e274c6e0bf --- /dev/null +++ b/db/post_migrate/20220118183123_remove_rememberable_from_users.rb @@ -0,0 +1,8 @@ +class RemoveRememberableFromUsers < ActiveRecord::Migration[6.1] + def change + safety_assured do + remove_column :users, :remember_token, :string, null: true, default: nil + remove_column :users, :remember_created_at, :datetime, null: true, default: nil + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 4e0f76dcdb4..fd4633d6952 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2022_01_16_202951) do +ActiveRecord::Schema.define(version: 2022_01_18_183123) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -937,7 +937,6 @@ ActiveRecord::Schema.define(version: 2022_01_16_202951) do t.string "encrypted_password", default: "", null: false t.string "reset_password_token" t.datetime "reset_password_sent_at" - t.datetime "remember_created_at" t.integer "sign_in_count", default: 0, null: false t.datetime "current_sign_in_at" t.datetime "last_sign_in_at" @@ -959,7 +958,6 @@ ActiveRecord::Schema.define(version: 2022_01_16_202951) do t.boolean "disabled", default: false, null: false t.boolean "moderator", default: false, null: false t.bigint "invite_id" - t.string "remember_token" t.string "chosen_languages", array: true t.bigint "created_by_application_id" t.boolean "approved", default: true, null: false @@ -972,7 +970,6 @@ ActiveRecord::Schema.define(version: 2022_01_16_202951) do t.index ["confirmation_token"], name: "index_users_on_confirmation_token", unique: true t.index ["created_by_application_id"], name: "index_users_on_created_by_application_id" t.index ["email"], name: "index_users_on_email", unique: true - t.index ["remember_token"], name: "index_users_on_remember_token", unique: true t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true end diff --git a/lib/mastodon/maintenance_cli.rb b/lib/mastodon/maintenance_cli.rb index 47e2d78bb1e..00861df7743 100644 --- a/lib/mastodon/maintenance_cli.rb +++ b/lib/mastodon/maintenance_cli.rb @@ -14,7 +14,7 @@ module Mastodon end MIN_SUPPORTED_VERSION = 2019_10_01_213028 - MAX_SUPPORTED_VERSION = 2021_05_26_193025 + MAX_SUPPORTED_VERSION = 2022_01_18_183123 # Stubs to enjoy ActiveRecord queries while not depending on a particular # version of the code/database @@ -84,13 +84,14 @@ module Mastodon owned_classes = [ Status, StatusPin, MediaAttachment, Poll, Report, Tombstone, Favourite, - Follow, FollowRequest, Block, Mute, AccountIdentityProof, + Follow, FollowRequest, Block, Mute, AccountModerationNote, AccountPin, AccountStat, ListAccount, PollVote, Mention ] owned_classes << AccountDeletionRequest if ActiveRecord::Base.connection.table_exists?(:account_deletion_requests) owned_classes << AccountNote if ActiveRecord::Base.connection.table_exists?(:account_notes) owned_classes << FollowRecommendationSuppression if ActiveRecord::Base.connection.table_exists?(:follow_recommendation_suppressions) + owned_classes << AccountIdentityProof if ActiveRecord::Base.connection.table_exists?(:account_identity_proofs) owned_classes.each do |klass| klass.where(account_id: other_account.id).find_each do |record| @@ -139,17 +140,22 @@ module Mastodon @prompt = TTY::Prompt.new if ActiveRecord::Migrator.current_version < MIN_SUPPORTED_VERSION - @prompt.warn 'Your version of the database schema is too old and is not supported by this script.' - @prompt.warn 'Please update to at least Mastodon 3.0.0 before running this script.' + @prompt.error 'Your version of the database schema is too old and is not supported by this script.' + @prompt.error 'Please update to at least Mastodon 3.0.0 before running this script.' exit(1) elsif ActiveRecord::Migrator.current_version > MAX_SUPPORTED_VERSION @prompt.warn 'Your version of the database schema is more recent than this script, this may cause unexpected errors.' - exit(1) unless @prompt.yes?('Continue anyway?') + exit(1) unless @prompt.yes?('Continue anyway? (Yes/No)') + end + + if Sidekiq::ProcessSet.new.any? + @prompt.error 'It seems Sidekiq is running. All Mastodon processes need to be stopped when using this script.' + exit(1) end @prompt.warn 'This task will take a long time to run and is potentially destructive.' @prompt.warn 'Please make sure to stop Mastodon and have a backup.' - exit(1) unless @prompt.yes?('Continue?') + exit(1) unless @prompt.yes?('Continue? (Yes/No)') deduplicate_users! deduplicate_account_domain_blocks! @@ -236,12 +242,14 @@ module Mastodon end end - ActiveRecord::Base.connection.select_all("SELECT string_agg(id::text, ',') AS ids FROM users WHERE remember_token IS NOT NULL GROUP BY remember_token HAVING count(*) > 1").each do |row| - users = User.where(id: row['ids'].split(',')).sort_by(&:updated_at).reverse.drop(1) - @prompt.warn "Unsetting remember token for those accounts: #{users.map(&:account).map(&:acct).join(', ')}" + if ActiveRecord::Migrator.current_version < 20220118183010 + ActiveRecord::Base.connection.select_all("SELECT string_agg(id::text, ',') AS ids FROM users WHERE remember_token IS NOT NULL GROUP BY remember_token HAVING count(*) > 1").each do |row| + users = User.where(id: row['ids'].split(',')).sort_by(&:updated_at).reverse.drop(1) + @prompt.warn "Unsetting remember token for those accounts: #{users.map(&:account).map(&:acct).join(', ')}" - users.each do |user| - user.update!(remember_token: nil) + users.each do |user| + user.update!(remember_token: nil) + end end end @@ -257,7 +265,7 @@ module Mastodon @prompt.say 'Restoring users indexes…' ActiveRecord::Base.connection.add_index :users, ['confirmation_token'], name: 'index_users_on_confirmation_token', unique: true ActiveRecord::Base.connection.add_index :users, ['email'], name: 'index_users_on_email', unique: true - ActiveRecord::Base.connection.add_index :users, ['remember_token'], name: 'index_users_on_remember_token', unique: true + ActiveRecord::Base.connection.add_index :users, ['remember_token'], name: 'index_users_on_remember_token', unique: true if ActiveRecord::Migrator.current_version < 20220118183010 ActiveRecord::Base.connection.add_index :users, ['reset_password_token'], name: 'index_users_on_reset_password_token', unique: true end @@ -274,6 +282,8 @@ module Mastodon end def deduplicate_account_identity_proofs! + return unless ActiveRecord::Base.connection.table_exists?(:account_identity_proofs) + remove_index_if_exists!(:account_identity_proofs, 'index_account_proofs_on_account_and_provider_and_username') @prompt.say 'Removing duplicate account identity proofs…' From cfa583fa7111cfc16b9ce548f9d9b58963f154bd Mon Sep 17 00:00:00 2001 From: Claire Date: Sun, 23 Jan 2022 15:50:41 +0100 Subject: [PATCH 6/9] Remove support for OAUTH_REDIRECT_AT_SIGN_IN (#17287) Fixes #15959 Introduced in #6540, OAUTH_REDIRECT_AT_SIGN_IN allowed skipping the log-in form to instead redirect to the external OmniAuth login provider. However, it did not prevent the log-in form on /about introduced by #10232 from appearing, and completely broke with the introduction of #15228. As I restoring that previous log-in flow without introducing a security vulnerability may require extensive care and knowledge of how OmniAuth works, this commit removes support for OAUTH_REDIRECT_AT_SIGN_IN instead for the time being. --- .env.nanobox | 4 ---- app/controllers/auth/sessions_controller.rb | 16 ---------------- config/initializers/omniauth.rb | 1 - 3 files changed, 21 deletions(-) diff --git a/.env.nanobox b/.env.nanobox index ad941c947c8..51dfdbd58fb 100644 --- a/.env.nanobox +++ b/.env.nanobox @@ -202,10 +202,6 @@ SMTP_FROM_ADDRESS=notifications@${APP_NAME}.nanoapp.io # Name of the pam service used for checking if an user can register (pam "account" section is evaluated) (nil (disabled) by default) # PAM_CONTROLLED_SERVICE=rpam -# Global OAuth settings (optional) : -# If you have only one strategy, you may want to enable this -# OAUTH_REDIRECT_AT_SIGN_IN=true - # Optional CAS authentication (cf. omniauth-cas) : # CAS_ENABLED=true # CAS_URL=https://sso.myserver.com/ diff --git a/app/controllers/auth/sessions_controller.rb b/app/controllers/auth/sessions_controller.rb index 3337a43c458..4d2695bf57a 100644 --- a/app/controllers/auth/sessions_controller.rb +++ b/app/controllers/auth/sessions_controller.rb @@ -13,14 +13,6 @@ class Auth::SessionsController < Devise::SessionsController before_action :set_instance_presenter, only: [:new] before_action :set_body_classes - def new - Devise.omniauth_configs.each do |provider, config| - return redirect_to(omniauth_authorize_path(resource_name, provider)) if config.strategy.redirect_at_sign_in - end - - super - end - def create super do |resource| # We only need to call this if this hasn't already been @@ -87,14 +79,6 @@ class Auth::SessionsController < Devise::SessionsController end end - def after_sign_out_path_for(_resource_or_scope) - Devise.omniauth_configs.each_value do |config| - return root_path if config.strategy.redirect_at_sign_in - end - - super - end - def require_no_authentication super diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb index 5039b4c1f03..19d59f1551c 100644 --- a/config/initializers/omniauth.rb +++ b/config/initializers/omniauth.rb @@ -5,7 +5,6 @@ end Devise.setup do |config| # Devise omniauth strategies options = {} - options[:redirect_at_sign_in] = ENV['OAUTH_REDIRECT_AT_SIGN_IN'] == 'true' # CAS strategy if ENV['CAS_ENABLED'] == 'true' From bddd9ba36d9f4e86e2a4bbea77f967c143afa2cc Mon Sep 17 00:00:00 2001 From: Claire Date: Sun, 23 Jan 2022 15:52:58 +0100 Subject: [PATCH 7/9] Add OMNIAUTH_ONLY environment variable to enforce externa log-in (#17288) * Remove support for OAUTH_REDIRECT_AT_SIGN_IN Fixes #15959 Introduced in #6540, OAUTH_REDIRECT_AT_SIGN_IN allowed skipping the log-in form to instead redirect to the external OmniAuth login provider. However, it did not prevent the log-in form on /about introduced by #10232 from appearing, and completely broke with the introduction of #15228. As I restoring that previous log-in flow without introducing a security vulnerability may require extensive care and knowledge of how OmniAuth works, this commit removes support for OAUTH_REDIRECT_AT_SIGN_IN instead for the time being. * Add OMNIAUTH_ONLY environment variable to enforce external log-in only * Disable user registration when OMNIAUTH_ONLY is set to true * Replace log-in links When OMNIAUTH_ONLY is set with exactly one OmniAuth provider --- app/controllers/api/v1/accounts_controller.rb | 6 +++- .../auth/registrations_controller.rb | 6 +++- app/helpers/application_helper.rb | 26 ++++++++++++++++- app/views/about/_login.html.haml | 29 ++++++++++++------- app/views/auth/sessions/new.html.haml | 25 ++++++++-------- app/views/auth/shared/_links.html.haml | 2 +- app/views/layouts/public.html.haml | 2 +- app/views/statuses/_status.html.haml | 2 +- config/locales/en.yml | 1 + 9 files changed, 71 insertions(+), 28 deletions(-) diff --git a/app/controllers/api/v1/accounts_controller.rb b/app/controllers/api/v1/accounts_controller.rb index cbccd64f375..5c47158e02c 100644 --- a/app/controllers/api/v1/accounts_controller.rb +++ b/app/controllers/api/v1/accounts_controller.rb @@ -83,10 +83,14 @@ class Api::V1::AccountsController < Api::BaseController end def check_enabled_registrations - forbidden if single_user_mode? || !allowed_registrations? + forbidden if single_user_mode? || omniauth_only? || !allowed_registrations? end def allowed_registrations? Setting.registrations_mode != 'none' end + + def omniauth_only? + ENV['OMNIAUTH_ONLY'] == 'true' + end end diff --git a/app/controllers/auth/registrations_controller.rb b/app/controllers/auth/registrations_controller.rb index 3c1730f25f4..f37e906fded 100644 --- a/app/controllers/auth/registrations_controller.rb +++ b/app/controllers/auth/registrations_controller.rb @@ -81,13 +81,17 @@ class Auth::RegistrationsController < Devise::RegistrationsController end def check_enabled_registrations - redirect_to root_path if single_user_mode? || !allowed_registrations? + redirect_to root_path if single_user_mode? || omniauth_only? || !allowed_registrations? end def allowed_registrations? Setting.registrations_mode != 'none' || @invite&.valid_for_use? end + def omniauth_only? + ENV['OMNIAUTH_ONLY'] == 'true' + end + def invite_code if params[:user] params[:user][:invite_code] diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 34fc46615f6..9e16de5b5bf 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -50,13 +50,37 @@ module ApplicationHelper end def available_sign_up_path - if closed_registrations? + if closed_registrations? || omniauth_only? 'https://joinmastodon.org/#getting-started' else new_user_registration_path end end + def omniauth_only? + ENV['OMNIAUTH_ONLY'] == 'true' + end + + def link_to_login(name = nil, html_options = nil, &block) + target = new_user_session_path + + if omniauth_only? && Devise.mappings[:user].omniauthable? && User.omniauth_providers.size == 1 + target = omniauth_authorize_path(:user, User.omniauth_providers[0]) + html_options ||= {} + html_options[:method] = :post + end + + if block_given? + link_to(target, html_options, &block) + else + link_to(name, target, html_options) + end + end + + def provider_sign_in_link(provider) + link_to I18n.t("auth.providers.#{provider}", default: provider.to_s.chomp('_oauth2').capitalize), omniauth_authorize_path(:user, provider), class: "button button-#{provider}", method: :post + end + def open_deletion? Setting.open_deletion end diff --git a/app/views/about/_login.html.haml b/app/views/about/_login.html.haml index fa58f04d736..0f19e816438 100644 --- a/app/views/about/_login.html.haml +++ b/app/views/about/_login.html.haml @@ -1,13 +1,22 @@ -= simple_form_for(new_user, url: user_session_path, namespace: 'login') do |f| - .fields-group - - if use_seamless_external_login? - = f.input :email, placeholder: t('simple_form.labels.defaults.username_or_email'), input_html: { 'aria-label' => t('simple_form.labels.defaults.username_or_email') }, hint: false - - else - = f.input :email, placeholder: t('simple_form.labels.defaults.email'), input_html: { 'aria-label' => t('simple_form.labels.defaults.email') }, hint: false +- unless omniauth_only? + = simple_form_for(new_user, url: user_session_path, namespace: 'login') do |f| + .fields-group + - if use_seamless_external_login? + = f.input :email, placeholder: t('simple_form.labels.defaults.username_or_email'), input_html: { 'aria-label' => t('simple_form.labels.defaults.username_or_email') }, hint: false + - else + = f.input :email, placeholder: t('simple_form.labels.defaults.email'), input_html: { 'aria-label' => t('simple_form.labels.defaults.email') }, hint: false - = f.input :password, placeholder: t('simple_form.labels.defaults.password'), input_html: { 'aria-label' => t('simple_form.labels.defaults.password') }, hint: false + = f.input :password, placeholder: t('simple_form.labels.defaults.password'), input_html: { 'aria-label' => t('simple_form.labels.defaults.password') }, hint: false - .actions - = f.button :button, t('auth.login'), type: :submit, class: 'button button-primary' + .actions + = f.button :button, t('auth.login'), type: :submit, class: 'button button-primary' - %p.hint.subtle-hint= link_to t('auth.trouble_logging_in'), new_user_password_path + %p.hint.subtle-hint= link_to t('auth.trouble_logging_in'), new_user_password_path + +- if Devise.mappings[:user].omniauthable? and User.omniauth_providers.any? + .simple_form.alternative-login + %h4= omniauth_only? ? t('auth.log_in_with') : t('auth.or_log_in_with') + + .actions + - User.omniauth_providers.each do |provider| + = provider_sign_in_link(provider) diff --git a/app/views/auth/sessions/new.html.haml b/app/views/auth/sessions/new.html.haml index 9713bdaebfd..a4323d1d9a4 100644 --- a/app/views/auth/sessions/new.html.haml +++ b/app/views/auth/sessions/new.html.haml @@ -4,24 +4,25 @@ - content_for :header_tags do = render partial: 'shared/og' -= simple_form_for(resource, as: resource_name, url: session_path(resource_name)) do |f| - .fields-group - - if use_seamless_external_login? - = f.input :email, autofocus: true, wrapper: :with_label, label: t('simple_form.labels.defaults.username_or_email'), input_html: { 'aria-label' => t('simple_form.labels.defaults.username_or_email') }, hint: false - - else - = f.input :email, autofocus: true, wrapper: :with_label, label: t('simple_form.labels.defaults.email'), input_html: { 'aria-label' => t('simple_form.labels.defaults.email') }, hint: false - .fields-group - = f.input :password, wrapper: :with_label, label: t('simple_form.labels.defaults.password'), input_html: { 'aria-label' => t('simple_form.labels.defaults.password'), :autocomplete => 'off' }, hint: false +- unless omniauth_only? + = simple_form_for(resource, as: resource_name, url: session_path(resource_name)) do |f| + .fields-group + - if use_seamless_external_login? + = f.input :email, autofocus: true, wrapper: :with_label, label: t('simple_form.labels.defaults.username_or_email'), input_html: { 'aria-label' => t('simple_form.labels.defaults.username_or_email') }, hint: false + - else + = f.input :email, autofocus: true, wrapper: :with_label, label: t('simple_form.labels.defaults.email'), input_html: { 'aria-label' => t('simple_form.labels.defaults.email') }, hint: false + .fields-group + = f.input :password, wrapper: :with_label, label: t('simple_form.labels.defaults.password'), input_html: { 'aria-label' => t('simple_form.labels.defaults.password'), :autocomplete => 'off' }, hint: false - .actions - = f.button :button, t('auth.login'), type: :submit + .actions + = f.button :button, t('auth.login'), type: :submit - if devise_mapping.omniauthable? and resource_class.omniauth_providers.any? .simple_form.alternative-login - %h4= t('auth.or_log_in_with') + %h4= omniauth_only? ? t('auth.log_in_with') : t('auth.or_log_in_with') .actions - resource_class.omniauth_providers.each do |provider| - = link_to t("auth.providers.#{provider}", default: provider.to_s.chomp("_oauth2").capitalize), omniauth_authorize_path(resource_name, provider), class: "button button-#{provider}", method: :post + = provider_sign_in_link(provider) .form-footer= render 'auth/shared/links' diff --git a/app/views/auth/shared/_links.html.haml b/app/views/auth/shared/_links.html.haml index 66ed5b93f38..f078e2f7ece 100644 --- a/app/views/auth/shared/_links.html.haml +++ b/app/views/auth/shared/_links.html.haml @@ -3,7 +3,7 @@ %li= link_to t('settings.account_settings'), edit_user_registration_path - else - if controller_name != 'sessions' - %li= link_to t('auth.login'), new_user_session_path + %li= link_to_login t('auth.login') - if controller_name != 'registrations' %li= link_to t('auth.register'), available_sign_up_path diff --git a/app/views/layouts/public.html.haml b/app/views/layouts/public.html.haml index bdb8a3a8ec7..069931cfd65 100644 --- a/app/views/layouts/public.html.haml +++ b/app/views/layouts/public.html.haml @@ -22,7 +22,7 @@ - if user_signed_in? = link_to t('settings.back'), root_url, class: 'nav-link nav-button webapp-btn' - else - = link_to t('auth.login'), new_user_session_path, class: 'webapp-btn nav-link nav-button' + = link_to_login t('auth.login'), class: 'webapp-btn nav-link nav-button' = link_to t('auth.register'), available_sign_up_path, class: 'webapp-btn nav-link nav-button' .container= yield diff --git a/app/views/statuses/_status.html.haml b/app/views/statuses/_status.html.haml index 9f3197d0dbd..3b7152753af 100644 --- a/app/views/statuses/_status.html.haml +++ b/app/views/statuses/_status.html.haml @@ -56,6 +56,6 @@ - if include_threads && !embedded_view? && !user_signed_in? .entry{ class: entry_classes } - = link_to new_user_session_path, class: 'load-more load-gap' do + = link_to_login class: 'load-more load-gap' do = fa_icon 'comments' = t('statuses.sign_in_to_participate') diff --git a/config/locales/en.yml b/config/locales/en.yml index 13a252a47b2..85aa87c7a64 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -844,6 +844,7 @@ en: invalid_reset_password_token: Password reset token is invalid or expired. Please request a new one. link_to_otp: Enter a two-factor code from your phone or a recovery code link_to_webauth: Use your security key device + log_in_with: Log in with login: Log in logout: Logout migrate_account: Move to a different account From a63495230a3a28e022504f36356cd75b17b635ba Mon Sep 17 00:00:00 2001 From: Claire Date: Sun, 23 Jan 2022 16:01:25 +0100 Subject: [PATCH 8/9] Change `percent` to `rate` in retention metrics API (#16910) --- app/javascript/mastodon/components/admin/Retention.js | 6 +++--- app/lib/admin/metrics/retention.rb | 4 ++-- app/serializers/rest/admin/cohort_serializer.rb | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/javascript/mastodon/components/admin/Retention.js b/app/javascript/mastodon/components/admin/Retention.js index 3a7aaed9d87..47c9e715136 100644 --- a/app/javascript/mastodon/components/admin/Retention.js +++ b/app/javascript/mastodon/components/admin/Retention.js @@ -88,7 +88,7 @@ export default class Retention extends React.PureComponent { {data[0].data.slice(1).map((retention, i) => { - const average = data.reduce((sum, cohort, k) => cohort.data[i + 1] ? sum + (cohort.data[i + 1].percent - sum)/(k + 1) : sum, 0); + const average = data.reduce((sum, cohort, k) => cohort.data[i + 1] ? sum + (cohort.data[i + 1].rate - sum)/(k + 1) : sum, 0); return ( @@ -118,8 +118,8 @@ export default class Retention extends React.PureComponent { {cohort.data.slice(1).map(retention => ( -
- +
+
))} diff --git a/app/lib/admin/metrics/retention.rb b/app/lib/admin/metrics/retention.rb index 6b9dfde499e..0179a6e282d 100644 --- a/app/lib/admin/metrics/retention.rb +++ b/app/lib/admin/metrics/retention.rb @@ -6,7 +6,7 @@ class Admin::Metrics::Retention end class CohortData < ActiveModelSerializers::Model - attributes :date, :percent, :value + attributes :date, :rate, :value end def initialize(start_at, end_at, frequency) @@ -59,7 +59,7 @@ class Admin::Metrics::Retention current_cohort.data << CohortData.new( date: row['retention_period'], - percent: rate.to_f, + rate: rate.to_f, value: value.to_s ) end diff --git a/app/serializers/rest/admin/cohort_serializer.rb b/app/serializers/rest/admin/cohort_serializer.rb index 56b35c6991b..f6817361655 100644 --- a/app/serializers/rest/admin/cohort_serializer.rb +++ b/app/serializers/rest/admin/cohort_serializer.rb @@ -4,7 +4,7 @@ class REST::Admin::CohortSerializer < ActiveModel::Serializer attributes :period, :frequency class CohortDataSerializer < ActiveModel::Serializer - attributes :date, :percent, :value + attributes :date, :rate, :value def date object.date.iso8601 From 0a120d86d28e3f2e20455f56c1656f5d5f2f4af6 Mon Sep 17 00:00:00 2001 From: Claire Date: Sun, 23 Jan 2022 18:10:10 +0100 Subject: [PATCH 9/9] Fix error-prone SQL queries (#15828) * Fix error-prone SQL queries in Account search While this code seems to not present an actual vulnerability, one could easily be introduced by mistake due to how the query is built. This PR parameterises the `to_tsquery` input to make the query more robust. * Harden code for Status#tagged_with_all and Status#tagged_with_none Those two scopes aren't used in a way that could be vulnerable to an SQL injection, but keeping them unchanged might be a hazard. * Remove unneeded spaces surrounding tsquery term * Please CodeClimate * Move advanced_search_for SQL template to its own function This avoids one level of indentation while making clearer that the SQL template isn't build from all the dynamic parameters of advanced_search_for. * Add tests covering tagged_with, tagged_with_all and tagged_with_none * Rewrite tagged_with_none to avoid multiple joins and make it more robust * Remove obsolete brakeman warnings * Revert "Remove unneeded spaces surrounding tsquery term" The two queries are not strictly equivalent. This reverts commit 86f16c537e06c6ba4a8b250f25dcce9f049023ff. --- app/models/account.rb | 113 +++++++++++++++++++------------------ app/models/status.rb | 7 +-- config/brakeman.ignore | 80 -------------------------- spec/models/status_spec.rb | 81 ++++++++++++++++++++++++++ 4 files changed, 142 insertions(+), 139 deletions(-) diff --git a/app/models/account.rb b/app/models/account.rb index c459125c7b6..771cc0b1ba8 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -427,6 +427,9 @@ class Account < ApplicationRecord end class << self + DISALLOWED_TSQUERY_CHARACTERS = /['?\\:‘’]/.freeze + TEXTSEARCH = "(setweight(to_tsvector('simple', accounts.display_name), 'A') || setweight(to_tsvector('simple', accounts.username), 'B') || setweight(to_tsvector('simple', coalesce(accounts.domain, '')), 'C'))" + def readonly_attributes super - %w(statuses_count following_count followers_count) end @@ -437,70 +440,29 @@ class Account < ApplicationRecord end def search_for(terms, limit = 10, offset = 0) - textsearch, query = generate_query_for_search(terms) + tsquery = generate_query_for_search(terms) sql = <<-SQL.squish SELECT accounts.*, - ts_rank_cd(#{textsearch}, #{query}, 32) AS rank + ts_rank_cd(#{TEXTSEARCH}, to_tsquery('simple', :tsquery), 32) AS rank FROM accounts - WHERE #{query} @@ #{textsearch} + WHERE to_tsquery('simple', :tsquery) @@ #{TEXTSEARCH} AND accounts.suspended_at IS NULL AND accounts.moved_to_account_id IS NULL ORDER BY rank DESC - LIMIT ? OFFSET ? + LIMIT :limit OFFSET :offset SQL - records = find_by_sql([sql, limit, offset]) + records = find_by_sql([sql, limit: limit, offset: offset, tsquery: tsquery]) ActiveRecord::Associations::Preloader.new.preload(records, :account_stat) records end def advanced_search_for(terms, account, limit = 10, following = false, offset = 0) - textsearch, query = generate_query_for_search(terms) - - if following - sql = <<-SQL.squish - WITH first_degree AS ( - SELECT target_account_id - FROM follows - WHERE account_id = ? - UNION ALL - SELECT ? - ) - SELECT - accounts.*, - (count(f.id) + 1) * ts_rank_cd(#{textsearch}, #{query}, 32) AS rank - FROM accounts - LEFT OUTER JOIN follows AS f ON (accounts.id = f.account_id AND f.target_account_id = ?) - WHERE accounts.id IN (SELECT * FROM first_degree) - AND #{query} @@ #{textsearch} - AND accounts.suspended_at IS NULL - AND accounts.moved_to_account_id IS NULL - GROUP BY accounts.id - ORDER BY rank DESC - LIMIT ? OFFSET ? - SQL - - records = find_by_sql([sql, account.id, account.id, account.id, limit, offset]) - else - sql = <<-SQL.squish - SELECT - accounts.*, - (count(f.id) + 1) * ts_rank_cd(#{textsearch}, #{query}, 32) AS rank - FROM accounts - LEFT OUTER JOIN follows AS f ON (accounts.id = f.account_id AND f.target_account_id = ?) OR (accounts.id = f.target_account_id AND f.account_id = ?) - WHERE #{query} @@ #{textsearch} - AND accounts.suspended_at IS NULL - AND accounts.moved_to_account_id IS NULL - GROUP BY accounts.id - ORDER BY rank DESC - LIMIT ? OFFSET ? - SQL - - records = find_by_sql([sql, account.id, account.id, limit, offset]) - end - + tsquery = generate_query_for_search(terms) + sql = advanced_search_for_sql_template(following) + records = find_by_sql([sql, id: account.id, limit: limit, offset: offset, tsquery: tsquery]) ActiveRecord::Associations::Preloader.new.preload(records, :account_stat) records end @@ -522,12 +484,55 @@ class Account < ApplicationRecord private - def generate_query_for_search(terms) - terms = Arel.sql(connection.quote(terms.gsub(/['?\\:]/, ' '))) - textsearch = "(setweight(to_tsvector('simple', accounts.display_name), 'A') || setweight(to_tsvector('simple', accounts.username), 'B') || setweight(to_tsvector('simple', coalesce(accounts.domain, '')), 'C'))" - query = "to_tsquery('simple', ''' ' || #{terms} || ' ''' || ':*')" + def generate_query_for_search(unsanitized_terms) + terms = unsanitized_terms.gsub(DISALLOWED_TSQUERY_CHARACTERS, ' ') - [textsearch, query] + # The final ":*" is for prefix search. + # The trailing space does not seem to fit any purpose, but `to_tsquery` + # behaves differently with and without a leading space if the terms start + # with `./`, `../`, or `.. `. I don't understand why, so, in doubt, keep + # the same query. + "' #{terms} ':*" + end + + def advanced_search_for_sql_template(following) + if following + <<-SQL.squish + WITH first_degree AS ( + SELECT target_account_id + FROM follows + WHERE account_id = :id + UNION ALL + SELECT :id + ) + SELECT + accounts.*, + (count(f.id) + 1) * ts_rank_cd(#{TEXTSEARCH}, to_tsquery('simple', :tsquery), 32) AS rank + FROM accounts + LEFT OUTER JOIN follows AS f ON (accounts.id = f.account_id AND f.target_account_id = :id) + WHERE accounts.id IN (SELECT * FROM first_degree) + AND to_tsquery('simple', :tsquery) @@ #{TEXTSEARCH} + AND accounts.suspended_at IS NULL + AND accounts.moved_to_account_id IS NULL + GROUP BY accounts.id + ORDER BY rank DESC + LIMIT :limit OFFSET :offset + SQL + else + <<-SQL.squish + SELECT + accounts.*, + (count(f.id) + 1) * ts_rank_cd(#{TEXTSEARCH}, to_tsquery('simple', :tsquery), 32) AS rank + FROM accounts + LEFT OUTER JOIN follows AS f ON (accounts.id = f.account_id AND f.target_account_id = :id) OR (accounts.id = f.target_account_id AND f.account_id = :id) + WHERE to_tsquery('simple', :tsquery) @@ #{TEXTSEARCH} + AND accounts.suspended_at IS NULL + AND accounts.moved_to_account_id IS NULL + GROUP BY accounts.id + ORDER BY rank DESC + LIMIT :limit OFFSET :offset + SQL + end end end diff --git a/app/models/status.rb b/app/models/status.rb index 3358d6891b8..47671c0f529 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -99,15 +99,12 @@ class Status < ApplicationRecord scope :not_excluded_by_account, ->(account) { where.not(account_id: account.excluded_from_timeline_account_ids) } scope :not_domain_blocked_by_account, ->(account) { account.excluded_from_timeline_domains.blank? ? left_outer_joins(:account) : left_outer_joins(:account).where('accounts.domain IS NULL OR accounts.domain NOT IN (?)', account.excluded_from_timeline_domains) } scope :tagged_with_all, ->(tag_ids) { - Array(tag_ids).reduce(self) do |result, id| + Array(tag_ids).map(&:to_i).reduce(self) do |result, id| result.joins("INNER JOIN statuses_tags t#{id} ON t#{id}.status_id = statuses.id AND t#{id}.tag_id = #{id}") end } scope :tagged_with_none, ->(tag_ids) { - Array(tag_ids).reduce(self) do |result, id| - result.joins("LEFT OUTER JOIN statuses_tags t#{id} ON t#{id}.status_id = statuses.id AND t#{id}.tag_id = #{id}") - .where("t#{id}.tag_id IS NULL") - end + where('NOT EXISTS (SELECT * FROM statuses_tags forbidden WHERE forbidden.status_id = statuses.id AND forbidden.tag_id IN (?))', tag_ids) } cache_associated :application, diff --git a/config/brakeman.ignore b/config/brakeman.ignore index c032e5412ac..4245b719248 100644 --- a/config/brakeman.ignore +++ b/config/brakeman.ignore @@ -60,46 +60,6 @@ "confidence": "High", "note": "" }, - { - "warning_type": "SQL Injection", - "warning_code": 0, - "fingerprint": "6e4051854bb62e2ddbc671f82d6c2328892e1134b8b28105ecba9b0122540714", - "check_name": "SQL", - "message": "Possible SQL injection", - "file": "app/models/account.rb", - "line": 484, - "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", - "code": "find_by_sql([\" WITH first_degree AS (\\n SELECT target_account_id\\n FROM follows\\n WHERE account_id = ?\\n UNION ALL\\n SELECT ?\\n )\\n SELECT\\n accounts.*,\\n (count(f.id) + 1) * ts_rank_cd(#{textsearch}, #{query}, 32) AS rank\\n FROM accounts\\n LEFT OUTER JOIN follows AS f ON (accounts.id = f.account_id AND f.target_account_id = ?)\\n WHERE accounts.id IN (SELECT * FROM first_degree)\\n AND #{query} @@ #{textsearch}\\n AND accounts.suspended_at IS NULL\\n AND accounts.moved_to_account_id IS NULL\\n GROUP BY accounts.id\\n ORDER BY rank DESC\\n LIMIT ? OFFSET ?\\n\".squish, account.id, account.id, account.id, limit, offset])", - "render_path": null, - "location": { - "type": "method", - "class": "Account", - "method": "advanced_search_for" - }, - "user_input": "textsearch", - "confidence": "Medium", - "note": "" - }, - { - "warning_type": "SQL Injection", - "warning_code": 0, - "fingerprint": "6f075c1484908e3ec9bed21ab7cf3c7866be8da3881485d1c82e13093aefcbd7", - "check_name": "SQL", - "message": "Possible SQL injection", - "file": "app/models/status.rb", - "line": 105, - "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", - "code": "result.joins(\"LEFT OUTER JOIN statuses_tags t#{id} ON t#{id}.status_id = statuses.id AND t#{id}.tag_id = #{id}\")", - "render_path": null, - "location": { - "type": "method", - "class": "Status", - "method": null - }, - "user_input": "id", - "confidence": "Weak", - "note": "" - }, { "warning_type": "SQL Injection", "warning_code": 0, @@ -180,26 +140,6 @@ "confidence": "Medium", "note": "" }, - { - "warning_type": "SQL Injection", - "warning_code": 0, - "fingerprint": "9251d682c4e2840e1b2fea91e7d758efe2097ecb7f6255c065e3750d25eb178c", - "check_name": "SQL", - "message": "Possible SQL injection", - "file": "app/models/account.rb", - "line": 453, - "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", - "code": "find_by_sql([\" SELECT\\n accounts.*,\\n ts_rank_cd(#{textsearch}, #{query}, 32) AS rank\\n FROM accounts\\n WHERE #{query} @@ #{textsearch}\\n AND accounts.suspended_at IS NULL\\n AND accounts.moved_to_account_id IS NULL\\n ORDER BY rank DESC\\n LIMIT ? OFFSET ?\\n\".squish, limit, offset])", - "render_path": null, - "location": { - "type": "method", - "class": "Account", - "method": "search_for" - }, - "user_input": "textsearch", - "confidence": "Medium", - "note": "" - }, { "warning_type": "Redirect", "warning_code": 18, @@ -270,26 +210,6 @@ "confidence": "Weak", "note": "" }, - { - "warning_type": "SQL Injection", - "warning_code": 0, - "fingerprint": "e21d8fee7a5805761679877ca35ed1029c64c45ef3b4012a30262623e1ba8bb9", - "check_name": "SQL", - "message": "Possible SQL injection", - "file": "app/models/account.rb", - "line": 500, - "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", - "code": "find_by_sql([\" SELECT\\n accounts.*,\\n (count(f.id) + 1) * ts_rank_cd(#{textsearch}, #{query}, 32) AS rank\\n FROM accounts\\n LEFT OUTER JOIN follows AS f ON (accounts.id = f.account_id AND f.target_account_id = ?) OR (accounts.id = f.target_account_id AND f.account_id = ?)\\n WHERE #{query} @@ #{textsearch}\\n AND accounts.suspended_at IS NULL\\n AND accounts.moved_to_account_id IS NULL\\n GROUP BY accounts.id\\n ORDER BY rank DESC\\n LIMIT ? OFFSET ?\\n\".squish, account.id, account.id, limit, offset])", - "render_path": null, - "location": { - "type": "method", - "class": "Account", - "method": "advanced_search_for" - }, - "user_input": "textsearch", - "confidence": "Medium", - "note": "" - }, { "warning_type": "Mass Assignment", "warning_code": 105, diff --git a/spec/models/status_spec.rb b/spec/models/status_spec.rb index 20fb894e77f..653575778b2 100644 --- a/spec/models/status_spec.rb +++ b/spec/models/status_spec.rb @@ -267,6 +267,87 @@ RSpec.describe Status, type: :model do end end + describe '.tagged_with' do + let(:tag1) { Fabricate(:tag) } + let(:tag2) { Fabricate(:tag) } + let(:tag3) { Fabricate(:tag) } + let!(:status1) { Fabricate(:status, tags: [tag1]) } + let!(:status2) { Fabricate(:status, tags: [tag2]) } + let!(:status3) { Fabricate(:status, tags: [tag3]) } + let!(:status4) { Fabricate(:status, tags: []) } + let!(:status5) { Fabricate(:status, tags: [tag1, tag2, tag3]) } + + context 'when given one tag' do + it 'returns the expected statuses' do + expect(Status.tagged_with([tag1.id]).reorder(:id).pluck(:id).uniq).to eq [status1.id, status5.id] + expect(Status.tagged_with([tag2.id]).reorder(:id).pluck(:id).uniq).to eq [status2.id, status5.id] + expect(Status.tagged_with([tag3.id]).reorder(:id).pluck(:id).uniq).to eq [status3.id, status5.id] + end + end + + context 'when given multiple tags' do + it 'returns the expected statuses' do + expect(Status.tagged_with([tag1.id, tag2.id]).reorder(:id).pluck(:id).uniq).to eq [status1.id, status2.id, status5.id] + expect(Status.tagged_with([tag1.id, tag3.id]).reorder(:id).pluck(:id).uniq).to eq [status1.id, status3.id, status5.id] + expect(Status.tagged_with([tag2.id, tag3.id]).reorder(:id).pluck(:id).uniq).to eq [status2.id, status3.id, status5.id] + end + end + end + + describe '.tagged_with_all' do + let(:tag1) { Fabricate(:tag) } + let(:tag2) { Fabricate(:tag) } + let(:tag3) { Fabricate(:tag) } + let!(:status1) { Fabricate(:status, tags: [tag1]) } + let!(:status2) { Fabricate(:status, tags: [tag2]) } + let!(:status3) { Fabricate(:status, tags: [tag3]) } + let!(:status4) { Fabricate(:status, tags: []) } + let!(:status5) { Fabricate(:status, tags: [tag1, tag2]) } + + context 'when given one tag' do + it 'returns the expected statuses' do + expect(Status.tagged_with_all([tag1.id]).reorder(:id).pluck(:id).uniq).to eq [status1.id, status5.id] + expect(Status.tagged_with_all([tag2.id]).reorder(:id).pluck(:id).uniq).to eq [status2.id, status5.id] + expect(Status.tagged_with_all([tag3.id]).reorder(:id).pluck(:id).uniq).to eq [status3.id] + end + end + + context 'when given multiple tags' do + it 'returns the expected statuses' do + expect(Status.tagged_with_all([tag1.id, tag2.id]).reorder(:id).pluck(:id).uniq).to eq [status5.id] + expect(Status.tagged_with_all([tag1.id, tag3.id]).reorder(:id).pluck(:id).uniq).to eq [] + expect(Status.tagged_with_all([tag2.id, tag3.id]).reorder(:id).pluck(:id).uniq).to eq [] + end + end + end + + describe '.tagged_with_none' do + let(:tag1) { Fabricate(:tag) } + let(:tag2) { Fabricate(:tag) } + let(:tag3) { Fabricate(:tag) } + let!(:status1) { Fabricate(:status, tags: [tag1]) } + let!(:status2) { Fabricate(:status, tags: [tag2]) } + let!(:status3) { Fabricate(:status, tags: [tag3]) } + let!(:status4) { Fabricate(:status, tags: []) } + let!(:status5) { Fabricate(:status, tags: [tag1, tag2, tag3]) } + + context 'when given one tag' do + it 'returns the expected statuses' do + expect(Status.tagged_with_none([tag1.id]).reorder(:id).pluck(:id).uniq).to eq [status2.id, status3.id, status4.id] + expect(Status.tagged_with_none([tag2.id]).reorder(:id).pluck(:id).uniq).to eq [status1.id, status3.id, status4.id] + expect(Status.tagged_with_none([tag3.id]).reorder(:id).pluck(:id).uniq).to eq [status1.id, status2.id, status4.id] + end + end + + context 'when given multiple tags' do + it 'returns the expected statuses' do + expect(Status.tagged_with_none([tag1.id, tag2.id]).reorder(:id).pluck(:id).uniq).to eq [status3.id, status4.id] + expect(Status.tagged_with_none([tag1.id, tag3.id]).reorder(:id).pluck(:id).uniq).to eq [status2.id, status4.id] + expect(Status.tagged_with_none([tag2.id, tag3.id]).reorder(:id).pluck(:id).uniq).to eq [status1.id, status4.id] + end + end + end + describe '.permitted_for' do subject { described_class.permitted_for(target_account, account).pluck(:visibility) }