Backport fixes to 3.2 (#15360)

* Fix 2FA/sign-in token sessions being valid after password change (#14802)

If someone tries logging in to an account and is prompted for a 2FA
code or sign-in token, even if the account's password or e-mail is
updated in the meantime, the session will show the prompt and allow
the login process to complete with a valid 2FA code or sign-in token

* Fix Move handler not being triggered when failing to fetch target (#15107)

When failing to fetch the target account, the ProcessingWorker fails
as expected, but since it hasn't cleared the `move_in_progress` flag,
the next attempt at processing skips the `Move` activity altogether.

This commit changes it to clear the flag when encountering any
unexpected error on fetching the target account. This is likely to
occur because, of, e.g., a timeout, when many instances query the
same actor at the same time.

* Fix slow distinct queries where grouped queries are faster (#15287)

About 2x speed-up on inboxes query

* Fix possible inconsistencies in tag search (#14906)

Do not downcase the queried tag before passing it to postgres when searching:
- tags are not downcased on creation
- `arel_table[:name].lower.matches(pattern)` generates an ILIKE anyway
- if Postgres and Rails happen to use different case-folding rules,
  downcasing before query but not before insertion may mean that some
  tags with some casings are not searchable

* Fix updating account counters when account_stat is not yet created (#15108)

* Fix account processing failing because of large collections (#15027)

Fixes #15025

* Fix downloading remote media files when server returns empty filename (#14867)

Fixes #14817

* Fix webfinger redirect handling in ResolveAccountService (#15187)

* Fix webfinger redirect handling in ResolveAccountService

ResolveAccountService#process_webfinger! handled a one-step webfinger
redirection, but only accepting the result if it matched the exact URI passed
as input, defeating the point of a redirection check.

Instead, use the same logic as in `ActivityPub::FetchRemoteAccountService`,
updating the resulting `acct:` URI with the result of the first webfinger
query.

* Add tests

* Remove dependency on unused and unmaintained http_parser.rb gem (#14574)

It seems that years ago, the “http” gem dependend on the “http_parser.rb” gem
(it now depends on the “http-parser” gem), and, still years ago, we pulled
it from git in order to benefit from a bugfix that wasn't released yet (#7467).

* Add tootctl maintenance fix-duplicates (#14860, #15201, #15264, #15349, #15359)

* Fix old migration script not being able to run if it fails midway (#15361)

* Fix old migration script not being able to run if it fails midway

Improve the robustness of a migration script likely to fail because of database
corruption so it can run again once database corruptions are fixed.

* Display a specific error message in case of index corruption

Co-authored-by: Eugen Rochko <eugen@zeonfederated.com>
Co-authored-by: Claire <claire.github-309c@sitedethib.com>

Co-authored-by: Eugen Rochko <eugen@zeonfederated.com>
Co-authored-by: Claire <claire.github-309c@sitedethib.com>
This commit is contained in:
ThibG 2020-12-18 23:31:14 +01:00 committed by GitHub
parent a583e54023
commit 406adfca27
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
24 changed files with 821 additions and 77 deletions

View file

@ -82,7 +82,7 @@ class AccountsController < ApplicationController
end
def account_media_status_ids
@account.media_attachments.attached.reorder(nil).select(:status_id).distinct
@account.media_attachments.attached.reorder(nil).select(:status_id).group(:status_id)
end
def no_replies_scope

View file

@ -14,7 +14,7 @@ module Admin
@statuses = @account.statuses.where(visibility: [:public, :unlisted])
if params[:media]
account_media_status_ids = @account.media_attachments.attached.reorder(nil).select(:status_id).distinct
account_media_status_ids = @account.media_attachments.attached.reorder(nil).select(:status_id).group(:status_id)
@statuses.merge!(Status.where(id: account_media_status_ids))
end

View file

@ -102,7 +102,7 @@ class Api::BaseController < ApplicationController
elsif !current_user.approved?
render json: { error: 'Your login is currently pending approval' }, status: 403
else
set_user_activity
update_user_sign_in
end
end

View file

@ -7,6 +7,7 @@ class Auth::SessionsController < Devise::SessionsController
skip_before_action :require_no_authentication, only: [:create]
skip_before_action :require_functional!
skip_before_action :update_user_sign_in
include TwoFactorAuthenticationConcern
include SignInTokenAuthenticationConcern
@ -24,6 +25,7 @@ class Auth::SessionsController < Devise::SessionsController
def create
super do |resource|
resource.update_sign_in!(request, new_sign_in: true)
remember_me(resource)
flash.delete(:notice)
end
@ -41,7 +43,7 @@ class Auth::SessionsController < Devise::SessionsController
def find_user
if session[:attempt_user_id]
User.find(session[:attempt_user_id])
User.find_by(id: session[:attempt_user_id])
else
user = User.authenticate_with_ldap(user_params) if Devise.ldap_authentication
user ||= User.authenticate_with_pam(user_params) if Devise.pam_authentication
@ -74,6 +76,7 @@ class Auth::SessionsController < Devise::SessionsController
def require_no_authentication
super
# Delete flash message that isn't entirely useful and may be confusing in
# most cases because /web doesn't display/clear flash messages.
flash.delete(:alert) if flash[:alert] == I18n.t('devise.failure.already_authenticated')
@ -91,13 +94,30 @@ class Auth::SessionsController < Devise::SessionsController
def home_paths(resource)
paths = [about_path]
if single_user_mode? && resource.is_a?(User)
paths << short_account_path(username: resource.account)
end
paths
end
def continue_after?
truthy_param?(:continue)
end
def restart_session
clear_attempt_from_session
redirect_to new_user_session_path, alert: I18n.t('devise.failure.timeout')
end
def set_attempt_session(user)
session[:attempt_user_id] = user.id
session[:attempt_user_updated_at] = user.updated_at.to_s
end
def clear_attempt_from_session
session.delete(:attempt_user_id)
session.delete(:attempt_user_updated_at)
end
end

View file

@ -18,7 +18,9 @@ module SignInTokenAuthenticationConcern
def authenticate_with_sign_in_token
user = self.resource = find_user
if user_params[:sign_in_token_attempt].present? && session[:attempt_user_id]
if user.present? && session[:attempt_user_id].present? && session[:attempt_user_updated_at] != user.updated_at.to_s
restart_session
elsif user_params.key?(:sign_in_token_attempt) && session[:attempt_user_id]
authenticate_with_sign_in_token_attempt(user)
elsif user.present? && user.external_or_valid_password?(user_params[:password])
prompt_for_sign_in_token(user)
@ -27,7 +29,7 @@ module SignInTokenAuthenticationConcern
def authenticate_with_sign_in_token_attempt(user)
if valid_sign_in_token_attempt?(user)
session.delete(:attempt_user_id)
clear_attempt_from_session
remember_me(user)
sign_in(user)
else
@ -42,10 +44,10 @@ module SignInTokenAuthenticationConcern
UserMailer.sign_in_token(user, request.remote_ip, request.user_agent, Time.now.utc.to_s).deliver_later!
end
set_locale do
session[:attempt_user_id] = user.id
@body_classes = 'lighter'
render :sign_in_token
end
set_attempt_session(user)
@body_classes = 'lighter'
set_locale { render :sign_in_token }
end
end

View file

@ -21,7 +21,9 @@ module TwoFactorAuthenticationConcern
def authenticate_with_two_factor
user = self.resource = find_user
if user_params[:otp_attempt].present? && session[:attempt_user_id]
if user.present? && session[:attempt_user_id].present? && session[:attempt_user_updated_at] != user.updated_at.to_s
restart_session
elsif user_params[:otp_attempt].present? && session[:attempt_user_id]
authenticate_with_two_factor_attempt(user)
elsif user.present? && user.external_or_valid_password?(user_params[:password])
prompt_for_two_factor(user)
@ -30,7 +32,7 @@ module TwoFactorAuthenticationConcern
def authenticate_with_two_factor_attempt(user)
if valid_otp_attempt?(user)
session.delete(:attempt_user_id)
clear_attempt_from_session
remember_me(user)
sign_in(user)
else
@ -40,10 +42,10 @@ module TwoFactorAuthenticationConcern
end
def prompt_for_two_factor(user)
set_locale do
session[:attempt_user_id] = user.id
@body_classes = 'lighter'
render :two_factor
end
set_attempt_session(user)
@body_classes = 'lighter'
set_locale { render :two_factor }
end
end

View file

@ -6,14 +6,13 @@ module UserTrackingConcern
UPDATE_SIGN_IN_HOURS = 24
included do
before_action :set_user_activity
before_action :update_user_sign_in
end
private
def set_user_activity
return unless user_needs_sign_in_update?
current_user.update_tracked_fields!(request)
def update_user_sign_in
current_user.update_sign_in!(request) if user_needs_sign_in_update?
end
def user_needs_sign_in_update?

View file

@ -20,6 +20,9 @@ class ActivityPub::Activity::Move < ActivityPub::Activity
# Initiate a re-follow for each follower
MoveWorker.perform_async(origin_account.id, target_account.id)
rescue
unmark_as_processing!
raise
end
private

View file

@ -415,7 +415,7 @@ class Account < ApplicationRecord
end
def inboxes
urls = reorder(nil).where(protocol: :activitypub).pluck(Arel.sql("distinct coalesce(nullif(accounts.shared_inbox_url, ''), accounts.inbox_url)"))
urls = reorder(nil).where(protocol: :activitypub).group(:preferred_inbox_url).pluck(Arel.sql("coalesce(nullif(accounts.shared_inbox_url, ''), accounts.inbox_url) AS preferred_inbox_url"))
DeliveryFailureTracker.without_unavailable(urls)
end

View file

@ -21,26 +21,26 @@ class AccountStat < ApplicationRecord
def increment_count!(key)
update(attributes_for_increment(key))
rescue ActiveRecord::StaleObjectError
rescue ActiveRecord::StaleObjectError, ActiveRecord::RecordNotUnique
begin
reload_with_id
rescue ActiveRecord::RecordNotFound
# Nothing to do
else
retry
return
end
retry
end
def decrement_count!(key)
update(key => [public_send(key) - 1, 0].max)
rescue ActiveRecord::StaleObjectError
update(attributes_for_decrement(key))
rescue ActiveRecord::StaleObjectError, ActiveRecord::RecordNotUnique
begin
reload_with_id
rescue ActiveRecord::RecordNotFound
# Nothing to do
else
retry
return
end
retry
end
private
@ -51,8 +51,13 @@ class AccountStat < ApplicationRecord
attrs
end
def attributes_for_decrement(key)
attrs = { key => [public_send(key) - 1, 0].max }
attrs
end
def reload_with_id
self.id = find_by!(account: account).id if new_record?
self.id = self.class.find_by!(account: account).id if new_record?
reload
end
end

View file

@ -43,7 +43,7 @@ class Form::AccountBatch
end
def account_domains
accounts.pluck(Arel.sql('distinct domain')).compact
accounts.group(:domain).pluck(:domain).compact
end
def accounts

View file

@ -126,7 +126,7 @@ class Tag < ApplicationRecord
end
def search_for(term, limit = 5, offset = 0, options = {})
normalized_term = normalize(term.strip).mb_chars.downcase.to_s
normalized_term = normalize(term.strip)
pattern = sanitize_sql_like(normalized_term) + '%'
query = Tag.listable.where(arel_table[:name].lower.matches(pattern))
query = query.where(arel_table[:name].lower.eq(normalized_term).or(arel_table[:reviewed_at].not_eq(nil))) if options[:exclude_unreviewed]

View file

@ -61,7 +61,7 @@ class User < ApplicationRecord
devise :two_factor_backupable,
otp_number_of_backup_codes: 10
devise :registerable, :recoverable, :rememberable, :trackable, :validatable,
devise :registerable, :recoverable, :rememberable, :validatable,
:confirmable
include Omniauthable
@ -161,6 +161,24 @@ class User < ApplicationRecord
prepare_new_user! if new_user && approved?
end
def update_sign_in!(request, new_sign_in: false)
old_current, new_current = current_sign_in_at, Time.now.utc
self.last_sign_in_at = old_current || new_current
self.current_sign_in_at = new_current
old_current, new_current = current_sign_in_ip, request.remote_ip
self.last_sign_in_ip = old_current || new_current
self.current_sign_in_ip = new_current
if new_sign_in
self.sign_in_count ||= 0
self.sign_in_count += 1
end
save(validate: false) unless new_record?
prepare_returning_user!
end
def pending?
!approved?
end
@ -192,11 +210,6 @@ class User < ApplicationRecord
prepare_new_user!
end
def update_tracked_fields!(request)
super
prepare_returning_user!
end
def disable_two_factor!
self.otp_required_for_login = false
otp_backup_codes&.clear

View file

@ -196,7 +196,7 @@ class ActivityPub::ProcessAccountService < BaseService
total_items = collection.is_a?(Hash) && collection['totalItems'].present? && collection['totalItems'].is_a?(Numeric) ? collection['totalItems'] : nil
has_first_page = collection.is_a?(Hash) && collection['first'].present?
@collections[type] = [total_items, has_first_page]
rescue HTTP::Error, OpenSSL::SSL::SSLError
rescue HTTP::Error, OpenSSL::SSL::SSLError, Mastodon::LengthValidationError
@collections[type] = [nil, nil]
end

View file

@ -31,6 +31,7 @@ class ResolveAccountService < BaseService
# At this point we are in need of a Webfinger query, which may
# yield us a different username/domain through a redirect
process_webfinger!(@uri)
@domain = nil if TagManager.instance.local_domain?(@domain)
# Because the username/domain pair may be different than what
# we already checked, we need to check if we've already got
@ -75,21 +76,27 @@ class ResolveAccountService < BaseService
@uri = [@username, @domain].compact.join('@')
end
def process_webfinger!(uri, redirected = false)
def process_webfinger!(uri)
@webfinger = webfinger!("acct:#{uri}")
confirmed_username, confirmed_domain = @webfinger.subject.gsub(/\Aacct:/, '').split('@')
confirmed_username, confirmed_domain = split_acct(@webfinger.subject)
if confirmed_username.casecmp(@username).zero? && confirmed_domain.casecmp(@domain).zero?
@username = confirmed_username
@domain = confirmed_domain
@uri = uri
elsif !redirected
return process_webfinger!("#{confirmed_username}@#{confirmed_domain}", true)
else
raise WebfingerRedirectError, "The URI #{uri} tries to hijack #{@username}@#{@domain}"
return
end
@domain = nil if TagManager.instance.local_domain?(@domain)
# Account doesn't match, so it may have been redirected
@webfinger = webfinger!("acct:#{confirmed_username}@#{confirmed_domain}")
@username, @domain = split_acct(@webfinger.subject)
unless confirmed_username.casecmp(@username).zero? && confirmed_domain.casecmp(@domain).zero?
raise WebfingerRedirectError, "The URI #{uri} tries to hijack #{@username}@#{@domain}"
end
end
def split_acct(acct)
acct.gsub(/\Aacct:/, '').split('@')
end
def process_account!