Soft-delete users

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

app/avo/cards/users_metric.rb

@@ -4,6 +4,6 @@ class UsersMetric < Avo::Dashboards::MetricCard
   self.cols = 2
 
   def query
-    result User.count
+    result User.active.count
   end
 end

app/avo/resources/user_resource.rb

@@ -34,6 +34,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/api/v1/owners_controller.rb

@@ -17,7 +17,7 @@ def show
   def create
     return render_api_key_forbidden unless @api_key.can_add_owner?
 
-    owner = User.find_by_name(email_param)
+    owner = User.active.find_by_name(email_param)
     if owner
       ownership = @rubygem.ownerships.new(user: owner, authorizer: @api_key.user)
       if ownership.save
@@ -51,7 +51,7 @@ def destroy
   end
 
   def gems
-    user = User.find_by_slug(params[:handle])
+    user = User.active.find_by_slug(params[:handle])
     if user
       rubygems = user.rubygems.with_versions
       respond_to do |format|

app/controllers/api/v1/profiles_controller.rb

@@ -1,6 +1,6 @@
 class Api::V1::ProfilesController < Api::BaseController
   def show
-    @user = User.find_by_slug!(params[:id])
+    @user = User.active.find_by_slug!(params[:id])
     respond_to do |format|
       format.json { render json: @user }
       format.yaml { render yaml: @user }

app/controllers/concerns/webauthn_verifiable.rb

@@ -49,7 +49,7 @@ def webauthn_credential_scope
     if @user.present?
       @user.webauthn_credentials
     else
-      User.find_by(webauthn_id: @credential.user_handle)&.webauthn_credentials || WebauthnCredential.none
+      User.active.find_by(webauthn_id: @credential.user_handle)&.webauthn_credentials || WebauthnCredential.none
     end
   end
 

app/controllers/email_confirmations_controller.rb

@@ -75,7 +75,7 @@ def find_user_for_create
   end
 
   def validate_confirmation_token
-    @user = User.find_by(confirmation_token: token_params)
+    @user = User.active.find_by(confirmation_token: token_params)
     redirect_to root_path, alert: t("failure_when_forbidden") unless @user&.valid_confirmation_token?
   end
 

app/controllers/owners_controller.rb

@@ -33,7 +33,7 @@ def index
   end
 
   def create
-    owner = User.find_by_name(handle_params)
+    owner = User.active.find_by_name(handle_params)
     ownership = @rubygem.ownerships.new(user: owner, authorizer: current_user)
     if ownership.save
       OwnersMailer.ownership_confirmation(ownership).deliver_later

app/controllers/profiles_controller.rb

@@ -8,7 +8,7 @@ class ProfilesController < ApplicationController
   before_action :set_cache_headers, only: :edit
 
   def show
-    @user           = User.find_by_slug!(params[:id])
+    @user           = User.active.find_by_slug!(params[:id])
     rubygems        = @user.rubygems_downloaded
     @rubygems       = rubygems.slice!(0, 10)
     @extra_rubygems = rubygems

app/controllers/sessions_controller.rb

@@ -29,7 +29,7 @@ def create
   end
 
   def webauthn_create
-    @user = User.find(session[:mfa_user])
+    @user = User.active.find(session[:mfa_user])
 
     unless session_active?
       login_failure(t("multifactor_auths.session_expired"))
@@ -51,7 +51,7 @@ def webauthn_full_create
   end
 
   def otp_create
-    @user = User.find(session[:mfa_user])
+    @user = User.active.find(session[:mfa_user])
 
     if login_conditions_met?
       record_mfa_login_duration(mfa_type: "otp")

app/controllers/stats_controller.rb

@@ -3,7 +3,7 @@ class StatsController < ApplicationController
 
   def index
     @number_of_gems        = Rubygem.total_count
-    @number_of_users       = User.count
+    @number_of_users       = User.active.count
     @number_of_downloads   = GemDownload.total_count
     @most_downloaded       = Rubygem.by_downloads.includes(:gem_download).page(@page).per(Gemcutter::STATS_PER_PAGE)
     @most_downloaded_count = GemDownload.most_downloaded_gem_count

app/jobs/delete_user_job.rb

@@ -4,13 +4,36 @@ class DeleteUserJob < ApplicationJob
 
   def perform(user:)
     email = user.email
-    user.destroy!
-  rescue ActiveRecord::RecordNotDestroyed, ActiveRecord::NotNullViolation, ActiveRecord::DeleteRestrictionError => 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)
-    Mailer.deletion_failed(email).deliver_later
-  else
-    Mailer.deletion_complete(email).deliver_later
+    return if user.reload.deleted_at?
+
+    begin
+      user.transaction do
+        user.yank_gems
+        user.api_keys.expire_all!
+        user.class.reflect_on_all_associations.each do |association|
+          next if association.name == :api_keys
+          case association.options[:dependent]
+          when :destroy, :destroy_async
+            user.association(association.name).handle_dependency
+          end
+        end
+        user.update!(
+          deleted_at: Time.current, email: "deleted+#{user.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")
+        )
+        Mailer.deletion_complete(email).deliver_later
+      end
+    rescue ActiveRecord::ActiveRecordError => 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)
+      Mailer.deletion_failed(email).deliver_later
+    end
   end
 end

app/jobs/mfa_usage_stats_job.rb

@@ -2,10 +2,11 @@ class MfaUsageStatsJob < ApplicationJob
   queue_as "stats"
 
   def perform
-    non_mfa_users = User.where(totp_seed: nil).where.not(id: WebauthnCredential.select(:user_id)).count
-    totp_only_users = User.where.not(totp_seed: nil).where.not(id: WebauthnCredential.select(:user_id)).count
-    webauthn_only_users = User.where(totp_seed: nil).where(id: WebauthnCredential.select(:user_id)).count
-    webauthn_and_totp_users = User.where.not(totp_seed: nil).where(id: WebauthnCredential.select(:user_id)).count
+    users = User.active
+    non_mfa_users = users.where(totp_seed: nil).where.not(id: WebauthnCredential.select(:user_id)).count
+    totp_only_users = users.where.not(totp_seed: nil).where.not(id: WebauthnCredential.select(:user_id)).count
+    webauthn_only_users = users.where(totp_seed: nil).where(id: WebauthnCredential.select(:user_id)).count
+    webauthn_and_totp_users = users.where.not(totp_seed: nil).where(id: WebauthnCredential.select(:user_id)).count
 
     StatsD.gauge("mfa_usage_stats.non_mfa_users", non_mfa_users)
     StatsD.gauge("mfa_usage_stats.totp_only_users", totp_only_users)

app/models/user.rb

@@ -8,6 +8,9 @@ class User < ApplicationRecord
   before_create :_generate_confirmation_token_no_reset_unconfirmed_email
   before_destroy :yank_gems
 
+  scope :active, -> { where(deleted_at: nil) }
+  scope :deleted, -> { where.not(deleted_at: nil) }
+
   has_many :ownerships, -> { confirmed }, dependent: :destroy, inverse_of: :user
 
   has_many :rubygems, through: :ownerships, source: :rubygem
@@ -67,7 +70,8 @@ def self.authenticate(who, password)
     # to UTF-8.
     who = who.encode(Encoding::UTF_8)
 
-    user = find_by(email: who.downcase) || find_by(handle: who)
+    scope = active
+    user = scope.find_by(email: who.downcase) || scope.find_by(handle: who)
     user if user&.authenticated?(password)
   rescue BCrypt::Errors::InvalidHash, Encoding::UndefinedConversionError
     nil
@@ -257,6 +261,13 @@ def ld_context
     )
   end
 
+  def yank_gems
+    versions_to_yank = only_owner_gems.map(&:versions).flatten
+    versions_to_yank.each do |v|
+      deletions.create(version: v)
+    end
+  end
+
   private
 
   def update_email
@@ -272,13 +283,6 @@ def unconfirmed_email_exists?
     User.exists?(email: unconfirmed_email)
   end
 
-  def yank_gems
-    versions_to_yank = only_owner_gems.map(&:versions).flatten
-    versions_to_yank.each do |v|
-      deletions.create(version: v)
-    end
-  end
-
   def toxic_email_domain
     return unless (domain = email.split("@").last)
     toxic_domains_path = Pathname.new(Gemcutter::Application.config.toxic_domains_filepath)

app/models/version.rb

@@ -7,11 +7,14 @@ 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
 
+  belongs_to :active_pusher, -> { active }, class_name: "User", inverse_of: :pushed_versions, primary_key: :pusher_id, optional: true
+  has_one :active_yanker, -> { active }, through: :deletion, source: :user, inverse_of: :yanked_versions, required: false
+
   before_validation :set_canonical_number, if: :number_changed?
   before_validation :full_nameify!
   before_validation :gem_full_nameify!

app/views/rubygems/_gem_members.html.erb

@@ -27,10 +27,10 @@
     <% end %>
   <% end %>
 
-  <% if latest_version.pusher.present? %>
+  <% if latest_version.active_pusher.present? %>
     <h3 class="t-list__heading"><%= t '.pushed_by' %>:</h3>
     <div class="gem__users">
-      <%= link_to_user(latest_version.pusher) %>
+      <%= link_to_user(latest_version.active_pusher) %>
     </div>
   <% elsif latest_version.pusher_api_key&.owner.present? %>
     <h3 class="t-list__heading"><%= t '.pushed_by' %>:</h3>
@@ -39,10 +39,10 @@
     </div>
   <% end %>
 
-  <% if latest_version.yanker.present? %>
+  <% if latest_version.active_yanker.present? %>
     <h3 class="t-list__heading"><%= t '.yanked_by' %>:</h3>
     <div class="gem__users">
-      <%= link_to_user(latest_version.yanker) %>
+      <%= link_to_user(latest_version.active_yanker) %>
     </div>
   <% 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

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.1].define(version: 2024_01_10_052615) do
+ActiveRecord::Schema[7.1].define(version: 2024_01_20_010424) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "hstore"
   enable_extension "pgcrypto"
@@ -99,6 +99,19 @@
     t.index ["version_id"], name: "index_dependencies_on_version_id"
   end
 
+  create_table "events_user_events", force: :cascade do |t|
+    t.string "tag", null: false
+    t.string "trace_id"
+    t.bigint "user_id", null: false
+    t.bigint "ip_address_id"
+    t.jsonb "additional"
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
+    t.index ["ip_address_id"], name: "index_events_user_events_on_ip_address_id"
+    t.index ["tag"], name: "index_events_user_events_on_tag"
+    t.index ["user_id"], name: "index_events_user_events_on_user_id"
+  end
+
   create_table "gem_downloads", id: :serial, force: :cascade do |t|
     t.integer "rubygem_id", null: false
     t.integer "version_id", null: false
@@ -200,6 +213,16 @@
     t.index ["scheduled_at"], name: "index_good_jobs_on_scheduled_at", where: "(finished_at IS NULL)"
   end
 
+  create_table "ip_addresses", force: :cascade do |t|
+    t.inet "ip_address", null: false
+    t.text "hashed_ip_address", null: false
+    t.jsonb "geoip_info"
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
+    t.index ["hashed_ip_address"], name: "index_ip_addresses_on_hashed_ip_address", unique: true
+    t.index ["ip_address"], name: "index_ip_addresses_on_ip_address", unique: true
+  end
+
   create_table "link_verifications", force: :cascade do |t|
     t.string "linkable_type", null: false
     t.bigint "linkable_id", null: false
@@ -427,6 +450,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"
@@ -524,6 +548,8 @@
     t.index ["user_id"], name: "index_webauthn_verifications_on_user_id", unique: true
   end
 
+  add_foreign_key "events_user_events", "ip_addresses"
+  add_foreign_key "events_user_events", "users"
   add_foreign_key "oidc_api_key_roles", "oidc_providers"
   add_foreign_key "oidc_api_key_roles", "users"
   add_foreign_key "oidc_id_tokens", "api_keys"