Improve Error Messaging for Self-Deletion Confirmation Flow (#12266) [deploy]

* Adds guard clauses and better error messages to UsersController #confirm_destroy

* Adds tests around guard clauses in UsersController #confirm_destroy

* Adds a custom error for invalid tokens in UsersController#confirm_destroy

* Refactor user_destroy_token.rb initialize method to be more succinct

* Removes unnecessary code from user_self_destroy_spec.rb and fixes failing InvalidToken spec

* Reworks code in #confirm_destroy to be consistent with other #destroy methods

* Adjust token mismatch test to use mismatched tokens

* Refactors #confirm_destroy and adds a before_action to Users::Controller
 - Reworks the logic within #confirm_destroy and removes excess code
 - Adds set_or_sign_in_user before_action for #confirm_destroy
 - Removes custom InvalidToken error message

* Cleans up spec and adds an additional test around logged out user edge case

* Fixes failing tests due to changes in #confirm_delete flow

* Adjusts test logic in user_self_destroy_spec.rb

* Refactors code by removing custom before_action and rewriting conditionals

app/controllers/users_controller.rb

@@ -1,8 +1,10 @@
 class UsersController < ApplicationController
   before_action :set_no_cache_header
   before_action :raise_suspended, only: %i[update]
-  before_action :set_user, only: %i[update confirm_destroy request_destroy full_delete remove_identity]
-  after_action :verify_authorized, except: %i[index signout_confirm add_org_admin remove_org_admin remove_from_org]
+  before_action :set_user, only: %i[update request_destroy full_delete remove_identity]
+  # rubocop:disable Layout/LineLength
+  after_action :verify_authorized, except: %i[index signout_confirm add_org_admin remove_org_admin remove_from_org confirm_destroy]
+  # rubocop:enable Layout/LineLength
   before_action :authenticate_user!, only: %i[onboarding_update onboarding_checkbox_update]
   before_action :set_suggested_users, only: %i[index]
   before_action :initialize_stripe, only: %i[edit]
@@ -92,10 +94,25 @@ class UsersController < ApplicationController
   end
 
   def confirm_destroy
-    destroy_token = Rails.cache.read("user-destroy-token-#{@user.id}")
-    raise ActionController::RoutingError, "Not Found" unless destroy_token.present? && destroy_token == params[:token]
+    @user = current_user
 
-    set_current_tab("account")
+    if @user
+      authorize @user
+    else
+      flash[:alert] = "You must be logged in to proceed with account deletion."
+      redirect_to sign_up_path and return
+    end
+
+    destroy_token = Rails.cache.read("user-destroy-token-#{@user.id}")
+
+    # rubocop:disable Layout/LineLength
+    if destroy_token.blank?
+      flash[:settings_notice] = "Your token has expired, please request a new one. Tokens only last for 12 hours after account deletion is initiated."
+      redirect_to user_settings_path("account")
+    else
+      raise ActionController::RoutingError, "Not Found" unless destroy_token == params[:token]
+    end
+    # rubocop:enable Layout/LineLength
   end
 
   def full_delete

spec/requests/user/user_destroy_spec.rb

@@ -133,10 +133,9 @@ RSpec.describe "UserDestroy", type: :request do
         sign_in user
       end
 
-      it "renders not_found if user doesn't have a destroy_token" do
-        expect do
-          get user_confirm_destroy_path(token: token)
-        end.to raise_error(ActionController::RoutingError)
+      it "displays a flash message if user doesn't have a destroy_token" do
+        get user_confirm_destroy_path(token: token)
+        expect(flash[:settings_notice]).to include("Your token has expired,")
       end
 
       it "renders not_found if destroy_token != token" do
@@ -154,10 +153,9 @@ RSpec.describe "UserDestroy", type: :request do
     end
 
     context "without a user" do
-      it "renders not_found" do
-        expect do
-          get user_confirm_destroy_path(token: token)
-        end.to raise_error(ActiveRecord::RecordNotFound)
+      it "displays a flash message" do
+        get user_confirm_destroy_path(token: token)
+        expect(flash[:alert]).to include("You must be logged in")
       end
     end
   end

spec/system/user/user_self_destroy_spec.rb

@@ -2,6 +2,7 @@ require "rails_helper"
 
 RSpec.describe "User destroys their profile", type: :system, js: true do
   let(:user) { create(:user) }
+  let(:token) { SecureRandom.hex(10) }
 
   before do
     sign_in user
@@ -14,8 +15,29 @@ RSpec.describe "User destroys their profile", type: :system, js: true do
     expect(Users::RequestDestroy).to have_received(:call).with(user)
   end
 
+  it "displays a detailed error message when the user is not logged in" do
+    sign_out user
+    visit "/users/confirm_destroy/#{token}"
+    expect(page).to have_text("You must be logged in to proceed with account deletion.")
+  end
+
+  it "displays a detailed error message when the user's token is invalid" do
+    visit "/users/confirm_destroy/#{token}"
+    # rubocop:disable Layout/LineLength
+    expect(page).to have_text("Your token has expired, please request a new one. Tokens only last for 12 hours after account deletion is initiated.")
+    # rubocop:enable Layout/LineLength
+  end
+
+  it "raises a 'Not Found' error if there is a token mismatch" do
+    visit "/settings/account"
+    click_button "Delete Account"
+    allow(Rails.cache).to receive(:read).and_return(token)
+    expect do
+      get user_confirm_destroy_path(token: SecureRandom.hex(10))
+    end.to raise_error(ActionController::RoutingError)
+  end
+
   it "destroys an account" do
-    token = SecureRandom.hex(10)
     allow(Rails.cache).to receive(:read).and_return(token)
     visit "/users/confirm_destroy/#{token}"
     fill_in "delete__account__username__field", with: user.username