Soft-delete users (#4376)

* Soft-delete users

So we can retain a record of actions taken by users even if they are deleted, for security auditing purposes

* Use a default scope for not_deleted users

* Remove class references from migrations

* Refer to avo polymorphic types by class name

* Avoid loading Rubygem when loading Version

* Fix seed url

* Rewrite deleting again

* Remove dead code

* Use discard for soft-deleting users

* Flip callback ordering

Gemfile

@@ -54,6 +54,7 @@ gem "bcrypt", "~> 3.1"
 gem "maintenance_tasks", "~> 2.4"
 gem "strong_migrations", "~> 1.7"
 gem "phlex-rails", "~> 1.1"
+gem "discard", "~> 1.3"
 
 # Admin dashboard
 gem "avo", "~> 2.47"

Gemfile.lock

@@ -209,6 +209,8 @@ GEM
       rake (> 10, < 14)
       ruby-statistics (>= 2.1)
       thor (>= 0.19, < 2)
+    discard (1.3.0)
+      activerecord (>= 4.2, < 8)
     docile (1.4.0)
     dogstatsd-ruby (5.6.1)
     domain_name (0.6.20231109)
@@ -749,6 +751,7 @@ DEPENDENCIES
   dartsass-sprockets (~> 3.1)
   ddtrace (~> 1.19)
   derailed_benchmarks (~> 2.1)
+  discard (~> 1.3)
   dogstatsd-ruby (~> 5.5)
   dotenv-rails (~> 2.8)
   factory_bot_rails (~> 6.4)
@@ -898,6 +901,7 @@ CHECKSUMS
   dead_end (4.0.0) sha256=695c8438993bb4c5415b1618a1b6e0afcae849ef2812fb8cb3846723904307eb
   debase-ruby_core_source (3.3.1) sha256=ed904cae290edf0cf274ad707f8981bf1cefad8081e78d4bb71be2a483bc2c08
   derailed_benchmarks (2.1.2) sha256=eaadc6206ceeb5538ff8f5e04a0023d54ebdd95d04f33e8960fb95a5f189a14f
+  discard (1.3.0) sha256=55218997ca4dc11f0594f1bb2d1196c4a959ceb562f1ab6490130233598dda67
   docile (1.4.0) sha256=5f1734bde23721245c20c3d723e76c104208e1aa01277a69901ce770f0ebb8d3
   dogstatsd-ruby (5.6.1) sha256=615dd1328c57ab66fb48cbf83b38ce2060d8353707be0bc3deb0ae960a659982
   domain_name (0.6.20231109) sha256=5127a1521ecca79d54accefc393f6d19db8600c6224416004414f7eaa28aecbe

app/avo/resources/api_key_resource.rb

@@ -12,7 +12,7 @@ class ExpiredFilter < ScopeBooleanFilter; end
   field :user, as: :belongs_to, visible: ->(_) { false }
   field :owner, as: :belongs_to,
     polymorphic_as: :owner,
-    types: [::User, ::OIDC::TrustedPublisher::GitHubAction]
+    types: ["User", "OIDC::TrustedPublisher::GitHubAction"]
   field :last_accessed_at, as: :date_time
   field :soft_deleted_at, as: :date_time
   field :soft_deleted_rubygem_name, as: :text

app/avo/resources/audit_resource.rb

@@ -14,7 +14,7 @@ class AuditResource < Avo::BaseResource
 
     field :auditable, as: :belongs_to,
       polymorphic_as: :auditable,
-      types: [::User, ::WebHook],
+      types: %w[User WebHook],
       name: "Edited Record"
 
     heading "Action Details"

app/avo/resources/link_verification_resource.rb

@@ -9,7 +9,7 @@ class LinkVerificationResource < Avo::BaseResource
   # Fields generated from the model
   field :linkable, as: :belongs_to,
     polymorphic_as: :linkable,
-    types: [::Rubygem]
+    types: %w[Rubygem]
   field :uri, as: :text
   field :verified?, as: :boolean
   field :last_verified_at, as: :date_time

app/avo/resources/user_resource.rb

@@ -4,6 +4,10 @@ class UserResource < Avo::BaseResource
   self.search_query = lambda {
     scope.where("email LIKE ? OR handle LIKE ?", "%#{params[:q]}%", "%#{params[:q]}%")
   }
+  self.unscoped_queries_on_index = true
+
+  class DeletedFilter < ScopeBooleanFilter; end
+  filter DeletedFilter, arguments: { default: { not_deleted: true, deleted: false } }
 
   action BlockUser
   action CreateUser
@@ -34,6 +38,8 @@ class UserResource < Avo::BaseResource
   field :mail_fails, as: :number
   field :blocked_email, as: :text
 
+  field :deleted_at, as: :date_time
+
   tabs style: :pills do
     tab "Auth" do
       field :encrypted_password, as: :password, visible: ->(_) { false }

app/controllers/concerns/avo_auditable.rb

@@ -3,6 +3,8 @@ module AvoAuditable
 
   prepended do
     include Auditable
+
+    prepend_around_action :unscope_users
   end
 
   def perform_action_and_record_errors(&blk)
@@ -38,4 +40,8 @@ def after_create_path
 
     super
   end
+
+  def unscope_users(&)
+    User.unscoped(&)
+  end
 end

app/jobs/delete_user_job.rb

@@ -4,13 +4,12 @@ class DeleteUserJob < ApplicationJob
 
   def perform(user:)
     email = user.email
-    user.destroy!
-  rescue ActiveRecord::RecordNotDestroyed, ActiveRecord::NotNullViolation, ActiveRecord::DeleteRestrictionError => e
+    return if user.discarded?
+    user.discard!
+  rescue ActiveRecord::ActiveRecordError, Discard::RecordNotDiscarded => e
     # Catch the exception so we can log it, otherwise using `destroy` would give
     # us no hint as to why the deletion failed.
-    Rails.error.report(e, context: { user:, email: }, handled: true)
+    Rails.error.report(e, context: { user: user.as_json, email: }, handled: true)
     Mailer.deletion_failed(email).deliver_later
-  else
-    Mailer.deletion_complete(email).deliver_later
   end
 end

app/models/user.rb

@@ -1,13 +1,28 @@
 class User < ApplicationRecord
   include UserMultifactorMethods
   include Clearance::User
+
   include Gravtastic
   is_gravtastic default: "retro"
 
+  include Discard::Model
+  self.discard_column = :deleted_at
+
+  default_scope { not_deleted }
+
   before_save :_generate_confirmation_token_no_reset_unconfirmed_email, if: :will_save_change_to_unconfirmed_email?
   before_create :_generate_confirmation_token_no_reset_unconfirmed_email
+  before_discard :yank_gems
+  before_discard :expire_all_api_keys
+  before_discard :destroy_associations_for_discard
+  before_discard :clear_personal_attributes
+  after_discard :send_deletion_complete_email
   before_destroy :yank_gems
 
+  scope :not_deleted, -> { kept }
+  scope :deleted, -> { with_discarded.discarded }
+  scope :with_deleted, -> { with_discarded }
+
   has_many :ownerships, -> { confirmed }, dependent: :destroy, inverse_of: :user
 
   has_many :rubygems, through: :ownerships, source: :rubygem
@@ -25,7 +40,10 @@ class User < ApplicationRecord
   has_many :api_keys, dependent: :destroy, inverse_of: :owner, as: :owner
 
   has_many :ownership_calls, -> { opened }, dependent: :destroy, inverse_of: :user
+  has_many :closed_ownership_calls, -> { closed }, dependent: :destroy, inverse_of: :user, class_name: "OwnershipCall"
   has_many :ownership_requests, -> { opened }, dependent: :destroy, inverse_of: :user
+  has_many :closed_ownership_requests, -> { closed }, dependent: :destroy, inverse_of: :user, class_name: "OwnershipRequest"
+  has_many :approved_ownership_requests, -> { approved }, dependent: :destroy, inverse_of: :user, class_name: "OwnershipRequest"
 
   has_many :audits, as: :auditable, dependent: :nullify
 
@@ -280,4 +298,37 @@ def toxic_email_domain
 
     errors.add(:email, I18n.t("activerecord.errors.messages.blocked", domain: domain)) if toxic
   end
+
+  def expire_all_api_keys
+    api_keys.unexpired.expire_all!
+  end
+
+  def destroy_associations_for_discard
+    ownerships.unscope(where: :confirmed_at).destroy_all
+    ownership_requests.update_all(status: :closed)
+    ownership_calls.unscope(where: :status).destroy_all
+    oidc_pending_trusted_publishers.destroy_all
+    subscriptions.destroy_all
+    web_hooks.destroy_all
+    webauthn_credentials.destroy_all
+    webauthn_verification&.destroy!
+  end
+
+  def clear_personal_attributes
+    @email_before_discard = email
+    update!(
+      email: "deleted+#{id}@rubygems.org",
+      handle: nil, email_confirmed: false,
+      unconfirmed_email: nil, blocked_email: nil,
+      api_key: nil, confirmation_token: nil, remember_token: nil,
+      twitter_username: nil, webauthn_id: nil, full_name: nil,
+      totp_seed: nil, mfa_hashed_recovery_codes: nil,
+      mfa_level: :disabled,
+      password: SecureRandom.hex(20).encode("UTF-8")
+    )
+  end
+
+  def send_deletion_complete_email
+    Mailer.deletion_complete(@email_before_discard).deliver_later
+  end
 end

app/models/version.rb

@@ -7,7 +7,7 @@ class Version < ApplicationRecord # rubocop:disable Metrics/ClassLength
   has_many :dependencies, -> { order("rubygems.name ASC").includes(:rubygem) }, dependent: :destroy, inverse_of: "version"
   has_many :audits, as: :auditable, inverse_of: :auditable, dependent: :nullify
   has_one :gem_download, inverse_of: :version, dependent: :destroy
-  belongs_to :pusher, class_name: "User", inverse_of: false, optional: true
+  belongs_to :pusher, class_name: "User", inverse_of: :pushed_versions, optional: true
   belongs_to :pusher_api_key, class_name: "ApiKey", inverse_of: :pushed_versions, optional: true
   has_one :deletion, dependent: :delete, inverse_of: :version, required: false
   has_one :yanker, through: :deletion, source: :user, inverse_of: :yanked_versions, required: false
@@ -27,8 +27,8 @@ class Version < ApplicationRecord # rubocop:disable Metrics/ClassLength
   serialize :cert_chain, coder: CertificateChainSerializer
 
   validates :number, length: { maximum: Gemcutter::MAX_FIELD_LENGTH }, format: { with: Patterns::VERSION_PATTERN }
-  validates :platform, length: { maximum: Gemcutter::MAX_FIELD_LENGTH }, format: { with: Rubygem::NAME_PATTERN }
-  validates :gem_platform, length: { maximum: Gemcutter::MAX_FIELD_LENGTH }, format: { with: Rubygem::NAME_PATTERN },
+  validates :platform, length: { maximum: Gemcutter::MAX_FIELD_LENGTH }, format: { with: Patterns::NAME_PATTERN }
+  validates :gem_platform, length: { maximum: Gemcutter::MAX_FIELD_LENGTH }, format: { with: Patterns::NAME_PATTERN },
             if: -> { validation_context == :create || gem_platform_changed? }
   validates :full_name, presence: true, uniqueness: { case_sensitive: false },
             if: -> { validation_context == :create || full_name_changed? }

db/migrate/20090821044418_add_scope_to_dependencies.rb

@@ -1,8 +1,8 @@
 class AddScopeToDependencies < ActiveRecord::Migration[4.2]
   def self.up
     add_column :dependencies, :scope, :string
-    Dependency.update_all(scope: "runtime")
-    announce "Please reprocess all gems after this migration"
+    # Dependency.update_all(scope: "runtime")
+    # announce "Please reprocess all gems after this migration"
   end
 
   def self.down

db/migrate/20090826035230_fix_version_dates.rb

@@ -2,7 +2,7 @@ class FixVersionDates < ActiveRecord::Migration[4.2]
   def self.up
     rename_column :versions, :created_at, :built_at
     add_column :versions, :created_at, :datetime
-    Version.update_all("created_at = updated_at")
+    # Version.update_all("created_at = updated_at")
   end
 
   def self.down

db/migrate/20090917035818_add_prerelease_to_versions.rb

@@ -1,7 +1,7 @@
 class AddPrereleaseToVersions < ActiveRecord::Migration[4.2]
   def self.up
     add_column :versions, :prerelease, :boolean
-    Version.update_all(prerelease: false)
+    # Version.update_all(prerelease: false)
   end
 
   def self.down

db/migrate/20090930181320_add_position_to_versions.rb

@@ -2,7 +2,7 @@ class AddPositionToVersions < ActiveRecord::Migration[4.2]
   def self.up
     add_column :versions, :position, :integer
 
-    Rubygem.find_each(&:reorder_versions)
+    # Rubygem.find_each(&:reorder_versions)
   end
 
   def self.down

db/migrate/20091009213456_reorder_versions_again.rb

@@ -1,6 +1,6 @@
 class ReorderVersionsAgain < ActiveRecord::Migration[4.2]
   def self.up
-    Rubygem.find_each(&:reorder_versions)
+    # Rubygem.find_each(&:reorder_versions)
   end
 
   def self.down

db/migrate/20091021203534_add_latest_to_version.rb

@@ -2,7 +2,7 @@ class AddLatestToVersion < ActiveRecord::Migration[4.2]
   def self.up
     add_column :versions, :latest, :boolean
 
-    Rubygem.find_each(&:reorder_versions)
+    # Rubygem.find_each(&:reorder_versions)
   end
 
   def self.down

db/migrate/20091026234707_fix_dependencies.rb

@@ -1,20 +1,20 @@
 class FixDependencies < ActiveRecord::Migration[4.2]
   def self.up
-    # fix bad version reqs
-    Dependency.find_each do |dep|
-      reqs = dep.requirements
-      begin
-        Gem::Requirement.new(reqs)
-      rescue ArgumentError
-        list = reqs.split(/(>=)|(<=)|(~>)|(>)|(<)|(=)/).reject(&:empty?)
-        fixed = "#{list[0]}#{list[1]}, #{list[2]}#{list[3]}"
+    # # fix bad version reqs
+    # Dependency.find_each do |dep|
+    #   reqs = dep.requirements
+    #   begin
+    #     Gem::Requirement.new(reqs)
+    #   rescue ArgumentError
+    #     list = reqs.split(/(>=)|(<=)|(~>)|(>)|(<)|(=)/).reject(&:empty?)
+    #     fixed = "#{list[0]}#{list[1]}, #{list[2]}#{list[3]}"
 
-        dep.update_attribute(:requirements, fixed)
-      end
-    end
+    #     dep.update_attribute(:requirements, fixed)
+    #   end
+    # end
 
-    # kill bad deps too
-    Dependency.includes(:rubygem).where(rubygems: { id: nil }).destroy_all
+    # # kill bad deps too
+    # Dependency.includes(:rubygem).where(rubygems: { id: nil }).destroy_all
   end
 
   def self.down

db/migrate/20110805014415_remove_improperly_embedded_yaml_data.rb

@@ -1,9 +1,9 @@
 class RemoveImproperlyEmbeddedYamlData < ActiveRecord::Migration[4.2]
   def self.up
-    Dependency.where("requirements like '%YAML::Syck::DefaultKey%'").find_each do |d|
-      d.requirements = d.clean_requirements
-      d.save(validate: false)
-    end
+    # Dependency.where("requirements like '%YAML::Syck::DefaultKey%'").find_each do |d|
+    #   d.requirements = d.clean_requirements
+    #   d.save(validate: false)
+    # end
   end
 
   def self.down

db/migrate/20200508234114_add_indexed_to_rubygems.rb

@@ -3,8 +3,8 @@ def change
     add_column :rubygems, :indexed, :boolean, null: false, default: false
     add_index :rubygems, :indexed
 
-    say_with_time "populating indexed rubygems table column" do
-      Rubygem.joins(:versions).where(versions: { indexed: true }).update_all(indexed: true)
-    end
+    # say_with_time "populating indexed rubygems table column" do
+    #   Rubygem.joins(:versions).where(versions: { indexed: true }).update_all(indexed: true)
+    # end
   end
 end

db/migrate/20220609221942_remove_orphaned_owners.rb

@@ -1,6 +1,6 @@
 class RemoveOrphanedOwners < ActiveRecord::Migration[7.0]
   def up
-    Ownership.where.missing(:user).destroy_all
+    # Ownership.where.missing(:user).destroy_all
   end
 
   def down

db/migrate/20230702010946_create_gem_name_reservations.rb

@@ -65,7 +65,7 @@ def change
 
     reversible do |change|
       change.up do
-        GemNameReservation.insert_all(ORIGINAL_GEM_NAME_RESERVED_LIST.map { |name| { name: name } })
+        # GemNameReservation.insert_all(ORIGINAL_GEM_NAME_RESERVED_LIST.map { |name| { name: name } })
       end
     end
   end

db/migrate/20240120010424_add_deleted_at_to_user.rb

@@ -0,0 +1,5 @@
+class AddDeletedAtToUser < ActiveRecord::Migration[7.1]
+  def change
+    add_column :users, :deleted_at, :datetime
+  end
+end

db/schema.rb

@@ -428,6 +428,7 @@
     t.string "totp_seed"
     t.string "mfa_hashed_recovery_codes", default: [], array: true
     t.boolean "public_email", default: false, null: false
+    t.datetime "deleted_at"
     t.index ["email"], name: "index_users_on_email"
     t.index ["handle"], name: "index_users_on_handle"
     t.index ["id", "confirmation_token"], name: "index_users_on_id_and_confirmation_token"