From 3c13a51f3947cc85f913ee1f98cebbdc8392846d Mon Sep 17 00:00:00 2001 From: Mitchell Henke Date: Thu, 2 Jun 2022 13:22:45 -0500 Subject: [PATCH] Drop deprecated user columns (#6438) changelog: Internal, Database, Drop deprecated user columns --- app/controllers/users_controller.rb | 32 --------- .../concerns/deprecated_user_attributes.rb | 2 +- app/models/user.rb | 2 +- app/views/sign_up/cancellations/new.html.erb | 18 ++--- config/routes.rb | 3 - ...0602005747_drop_deprecated_user_columns.rb | 19 ++++++ db/schema.rb | 5 +- spec/controllers/users_controller_spec.rb | 67 ------------------- spec/support/features/session_helper.rb | 2 +- 9 files changed, 28 insertions(+), 122 deletions(-) delete mode 100644 app/controllers/users_controller.rb create mode 100644 db/primary_migrate/20220602005747_drop_deprecated_user_columns.rb delete mode 100644 spec/controllers/users_controller_spec.rb diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb deleted file mode 100644 index 10992662a80..00000000000 --- a/app/controllers/users_controller.rb +++ /dev/null @@ -1,32 +0,0 @@ -class UsersController < ApplicationController - before_action :ensure_in_setup - - def destroy - track_account_deletion_event - url_after_cancellation = decorated_session.cancel_link_url - destroy_user - flash[:success] = t('sign_up.cancel.success') - redirect_to url_after_cancellation - end - - private - - def track_account_deletion_event - properties = ParseControllerFromReferer.new(request.referer).call - analytics.account_deletion(**properties) - end - - def destroy_user - user = current_user || User.find_by(confirmation_token: session[:user_confirmation_token]) - user&.destroy! - sign_out if user - end - - def ensure_in_setup - redirect_to root_url if !session[:user_confirmation_token] && two_factor_enabled - end - - def two_factor_enabled - current_user && MfaPolicy.new(current_user).two_factor_enabled? - end -end diff --git a/app/models/concerns/deprecated_user_attributes.rb b/app/models/concerns/deprecated_user_attributes.rb index 147404d4f0d..3bb60134096 100644 --- a/app/models/concerns/deprecated_user_attributes.rb +++ b/app/models/concerns/deprecated_user_attributes.rb @@ -2,7 +2,7 @@ module DeprecatedUserAttributes extend ActiveSupport::Concern DEPRECATED_ATTRIBUTES = %i[ - email_fingerprint encrypted_email email confirmed_at confirmation_token confirmation_sent_at + email_fingerprint encrypted_email email confirmed_at ].freeze def []=(attribute, value) diff --git a/app/models/user.rb b/app/models/user.rb index 8eec4e70361..65dcc450a86 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,5 +1,5 @@ class User < ApplicationRecord - self.ignored_columns = %w[totp_timestamp confirmation_token confirmation_sent_at] + self.ignored_columns = %w[totp_timestamp] include NonNullUuid include ::NewRelic::Agent::MethodTracer diff --git a/app/views/sign_up/cancellations/new.html.erb b/app/views/sign_up/cancellations/new.html.erb index 386c62d24b5..0d8d6f75094 100644 --- a/app/views/sign_up/cancellations/new.html.erb +++ b/app/views/sign_up/cancellations/new.html.erb @@ -14,19 +14,11 @@
  • <%= t('users.delete.bullet_4', app_name: APP_NAME) %>
    • - <% if IdentityConfig.store.new_sign_up_cancellation_url_enabled %> - <% c.action_button( - action: ->(**tag_options, &block) do - button_to(sign_up_destroy_path, method: :delete, **tag_options, &block) - end, - ) { t('forms.buttons.cancel') } %> - <% else %> - <% c.action_button( - action: ->(**tag_options, &block) do - button_to(destroy_user_path, method: :delete, **tag_options, &block) - end, - ) { t('forms.buttons.cancel') } %> - <% end %> + <% c.action_button( + action: ->(**tag_options, &block) do + button_to(sign_up_destroy_path, method: :delete, **tag_options, &block) + end, + ) { t('forms.buttons.cancel') } %> <% c.action_button( action: ->(**tag_options, &block) do diff --git a/config/routes.rb b/config/routes.rb index b89f9f8cf8f..69a71bffffe 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -273,9 +273,6 @@ match '/sign_out' => 'sign_out#destroy', via: %i[get post delete] - # Deprecated - delete '/users' => 'users#destroy', as: :destroy_user - get '/restricted' => 'banned_user#show', as: :banned_user scope '/verify', as: 'idv' do diff --git a/db/primary_migrate/20220602005747_drop_deprecated_user_columns.rb b/db/primary_migrate/20220602005747_drop_deprecated_user_columns.rb new file mode 100644 index 00000000000..fdd37e7bb70 --- /dev/null +++ b/db/primary_migrate/20220602005747_drop_deprecated_user_columns.rb @@ -0,0 +1,19 @@ +class DropDeprecatedUserColumns < ActiveRecord::Migration[6.1] + disable_ddl_transaction! + + def up + remove_index :users, column: [:confirmation_token], name: 'index_users_on_confirmation_token', algorithm: :concurrently + + safety_assured do + remove_column :users, :confirmation_token + remove_column :users, :confirmation_sent_at + end + end + + def down + add_column :users, :confirmation_token, :text + add_column :users, :confirmation_sent_at, :datetime + + add_index :users, ['confirmation_token'], name: 'index_users_on_confirmation_token', unique: true + end +end diff --git a/db/schema.rb b/db/schema.rb index b163470de3c..2110bd7575e 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_05_17_103312) do +ActiveRecord::Schema.define(version: 2022_06_02_005747) do # These are extensions that must be enabled in order to support this database enable_extension "pg_stat_statements" @@ -581,9 +581,7 @@ t.datetime "remember_created_at" t.datetime "created_at" t.datetime "updated_at" - t.string "confirmation_token", limit: 255 t.datetime "confirmed_at" - t.datetime "confirmation_sent_at" t.integer "second_factor_attempts_count", default: 0 t.string "uuid", limit: 255, null: false t.datetime "second_factor_locked_at" @@ -602,7 +600,6 @@ t.string "email_language", limit: 10 t.datetime "accepted_terms_at" t.datetime "encrypted_recovery_code_digest_generated_at" - t.index ["confirmation_token"], name: "index_users_on_confirmation_token", unique: true t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true t.index ["uuid"], name: "index_users_on_uuid", unique: true end diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb deleted file mode 100644 index 9c8edf6cfa6..00000000000 --- a/spec/controllers/users_controller_spec.rb +++ /dev/null @@ -1,67 +0,0 @@ -require 'rails_helper' - -describe UsersController do - describe '#destroy' do - it 'redirects and displays the flash message if no user is present' do - delete :destroy - - expect(response).to redirect_to(root_url) - expect(flash.now[:success]).to eq t('sign_up.cancel.success') - end - - it 'destroys the current user and redirects to sign in page, with a helpful flash message' do - sign_in_as_user - subject.session[:user_confirmation_token] = '1' - - expect { delete :destroy }.to change(User, :count).by(-1) - expect(response).to redirect_to(root_url) - expect(flash.now[:success]).to eq t('sign_up.cancel.success') - end - - it 'does not destroy the user if the user is not in setup mode and is after 2fa' do - sign_in_as_user - - expect { delete :destroy }.to change(User, :count).by(0) - end - - it 'does not destroy the user if the user is not in setup mode and is before 2fa' do - sign_in_before_2fa - - expect { delete :destroy }.to change(User, :count).by(0) - end - - it 'redirects to the branded start page if the user came from an SP' do - session[:sp] = { issuer: 'http://localhost:3000', request_id: 'foo' } - - delete :destroy - - expect(response). - to redirect_to new_user_session_path(request_id: 'foo') - end - - it 'tracks the event in analytics when referer is nil' do - stub_analytics - properties = { request_came_from: 'no referer' } - - expect(@analytics).to receive(:track_event).with('Account Deletion Requested', properties) - - delete :destroy - end - - it 'tracks the event in analytics when referer is present' do - stub_analytics - request.env['HTTP_REFERER'] = 'http://example.com/' - properties = { request_came_from: 'users/sessions#new' } - - expect(@analytics).to receive(:track_event).with('Account Deletion Requested', properties) - - delete :destroy - end - - it 'calls ParseControllerFromReferer' do - expect_any_instance_of(ParseControllerFromReferer).to receive(:call).and_call_original - - delete :destroy - end - end -end diff --git a/spec/support/features/session_helper.rb b/spec/support/features/session_helper.rb index fd55a2e2cb1..34fc441520c 100644 --- a/spec/support/features/session_helper.rb +++ b/spec/support/features/session_helper.rb @@ -227,7 +227,7 @@ def user_with_piv_cac end def confirm_last_user - @raw_confirmation_token, = Devise.token_generator.generate(User, :confirmation_token) + @raw_confirmation_token, = Devise.token_generator.generate(EmailAddress, :confirmation_token) User.last.email_addresses.first.update( confirmation_token: @raw_confirmation_token, confirmation_sent_at: Time.zone.now,