LG-454 Refactor AccountReset::CancelController (#2385)

* LG-454 Refactor AccountReset::CancelController

**Why**: It was not adhering to our controller design convention, and
was breaking the analytics format contract.

**How**:
- Create a `AccountReset::Cancel` class that validates the token, and if 
successful, notifies the user via email and SMS if the user has a phone, 
then updates the user's account reset request to reflect that it was
cancelled.

- Rename `cancel` to `create` in the controller to adhere to our
CRUD-only guideline.

Benefits: 
- The controller is a lot leaner and cleaner. 
- It keeps all the actions in one place, as opposed to having some 
actions in the controller (the notifications) and some in 
`AccountResetService` (updating the request in the DB).
- It respects the Analytics API, and logs all error scenarios, including
missing tokens.

app/controllers/account_reset/cancel_controller.rb

@@ -1,30 +1,20 @@
 module AccountReset
   class CancelController < ApplicationController
-    def cancel
-      account_reset = AccountResetService.cancel_request(params[:token])
-      if account_reset
-        handle_success(account_reset.user)
-      else
-        handle_failure
-      end
+    def create
+      result = AccountReset::Cancel.new(params[:token]).call
+
+      analytics.track_event(Analytics::ACCOUNT_RESET, result.to_h)
+
+      handle_success if result.success?
+
       redirect_to root_url
     end
 
     private
 
-    def handle_success(user)
-      analytics.track_event(Analytics::ACCOUNT_RESET,
-                            event: :cancel, token_valid: true, user_id: user.uuid)
+    def handle_success
       sign_out if current_user
-      UserMailer.account_reset_cancel(user.email).deliver_later
-      phone = user.phone
-      SmsAccountResetCancellationNotifierJob.perform_now(phone: phone) if phone.present?
       flash[:success] = t('devise.two_factor_authentication.account_reset.successful_cancel')
     end
-
-    def handle_failure
-      return if params[:token].blank?
-      analytics.track_event(Analytics::ACCOUNT_RESET, event: :cancel, token_valid: false)
-    end
   end
 end

app/services/account_reset/cancel.rb

@@ -0,0 +1,67 @@
+module AccountReset
+  class Cancel
+    include ActiveModel::Model
+
+    validates :token, presence: { message: I18n.t('errors.account_reset.cancel_token_missing') }
+    validate :valid_token
+
+    def initialize(token)
+      @token = token
+    end
+
+    def call
+      @success = valid?
+
+      if success
+        notify_user_via_email_of_account_reset_cancellation
+        notify_user_via_phone_of_account_reset_cancellation if phone.present?
+        update_account_reset_request
+      end
+
+      FormResponse.new(success: success, errors: errors.messages, extra: extra_analytics_attributes)
+    end
+
+    private
+
+    attr_reader :success, :token
+
+    def valid_token
+      return if account_reset_request
+
+      errors.add(:token, I18n.t('errors.account_reset.cancel_token_invalid')) if token
+    end
+
+    def notify_user_via_email_of_account_reset_cancellation
+      UserMailer.account_reset_cancel(user.email).deliver_later
+    end
+
+    def notify_user_via_phone_of_account_reset_cancellation
+      SmsAccountResetCancellationNotifierJob.perform_now(phone: phone)
+    end
+
+    def update_account_reset_request
+      account_reset_request.update!(cancelled_at: Time.zone.now,
+                                    request_token: nil,
+                                    granted_token: nil)
+    end
+
+    def account_reset_request
+      @account_reset_request ||= AccountResetRequest.find_by(request_token: token)
+    end
+
+    def user
+      account_reset_request&.user || AnonymousUser.new
+    end
+
+    def phone
+      user.phone
+    end
+
+    def extra_analytics_attributes
+      {
+        event: 'cancel',
+        user_id: user.uuid,
+      }
+    end
+  end
+end

app/services/account_reset_service.rb

@@ -12,15 +12,6 @@ def create_request
                          granted_token: nil)
   end
 
-  def self.cancel_request(token)
-    account_reset = token.blank? ? nil : AccountResetRequest.find_by(request_token: token)
-    return false unless account_reset
-    account_reset.update(cancelled_at: Time.zone.now,
-                         request_token: nil,
-                         granted_token: nil)
-    account_reset
-  end
-
   def self.report_fraud(token)
     account_reset = token.blank? ? nil : AccountResetRequest.find_by(request_token: token)
     return false unless account_reset

config/locales/errors/en.yml

@@ -1,6 +1,9 @@
 ---
 en:
   errors:
+    account_reset:
+      cancel_token_invalid: cancel token is invalid
+      cancel_token_missing: cancel token is missing
     confirm_password_incorrect: Incorrect password.
     invalid_authenticity_token: Oops, something went wrong. Please try again.
     invalid_totp: Invalid code. Please try again.

config/locales/errors/es.yml

@@ -1,6 +1,9 @@
 ---
 es:
   errors:
+    account_reset:
+      cancel_token_invalid: NOT TRANSLATED YET
+      cancel_token_missing: NOT TRANSLATED YET
     confirm_password_incorrect: La contraseña es incorrecta.
     invalid_authenticity_token: "¡Oops! Algo salió mal. Inténtelo de nuevo."
     invalid_totp: El código es inválido. Vuelva a intentarlo.

config/locales/errors/fr.yml

@@ -1,6 +1,9 @@
 ---
 fr:
   errors:
+    account_reset:
+      cancel_token_invalid: NOT TRANSLATED YET
+      cancel_token_missing: NOT TRANSLATED YET
     confirm_password_incorrect: Mot de passe incorrect.
     invalid_authenticity_token: Oups, une erreur s'est produite. Veuillez essayer
       de nouveau.

config/routes.rb

@@ -58,7 +58,7 @@
 
       get '/account_reset/request' => 'account_reset/request#show'
       post '/account_reset/request' => 'account_reset/request#create'
-      get '/account_reset/cancel' => 'account_reset/cancel#cancel'
+      get '/account_reset/cancel' => 'account_reset/cancel#create'
       get '/account_reset/report_fraud' => 'account_reset/report_fraud#update'
       get '/account_reset/confirm_request' => 'account_reset/confirm_request#show'
       get '/account_reset/delete_account' => 'account_reset/delete_account#show'

spec/controllers/account_reset/cancel_controller_spec.rb

@@ -1,67 +1,84 @@
 require 'rails_helper'
 
 describe AccountReset::CancelController do
-  let(:user) { create(:user, :signed_up, phone: '+1 (703) 555-0000') }
+  include AccountResetHelper
+
+  let(:user) { create(:user, :signed_up) }
   before do
     TwilioService::Utils.telephony_service = FakeSms
   end
 
-  describe '#cancel' do
+  describe '#create' do
     it 'logs a good token to the analytics' do
-      AccountResetService.new(user).create_request
+      token = create_account_reset_request_for(user)
 
       stub_analytics
+      analytics_hash = {
+        success: true,
+        errors: {},
+        event: 'cancel',
+        user_id: user.uuid,
+      }
+
       expect(@analytics).to receive(:track_event).
-        with(Analytics::ACCOUNT_RESET,
-             event: :cancel, token_valid: true, user_id: user.uuid)
+        with(Analytics::ACCOUNT_RESET, analytics_hash)
 
-      post :cancel, params: { token: AccountResetRequest.all[0].request_token }
+      post :create, params: { token: token }
     end
 
     it 'logs a bad token to the analytics' do
       stub_analytics
+      analytics_hash = {
+        success: false,
+        errors: { token: [t('errors.account_reset.cancel_token_invalid')] },
+        event: 'cancel',
+        user_id: 'anonymous-uuid',
+      }
+
       expect(@analytics).to receive(:track_event).
-        with(Analytics::ACCOUNT_RESET, event: :cancel, token_valid: false)
+        with(Analytics::ACCOUNT_RESET, analytics_hash)
 
-      post :cancel, params: { token: 'FOO' }
+      post :create, params: { token: 'FOO' }
     end
 
-    it 'redirects to the root' do
-      post :cancel
-      expect(response).to redirect_to root_url
-    end
+    it 'logs a missing token to the analytics' do
+      stub_analytics
+      analytics_hash = {
+        success: false,
+        errors: { token: [t('errors.account_reset.cancel_token_missing')] },
+        event: 'cancel',
+        user_id: 'anonymous-uuid',
+      }
 
-    it 'sends an SMS if there is a phone' do
-      AccountResetService.new(user).create_request
-      allow(SmsAccountResetCancellationNotifierJob).to receive(:perform_now)
+      expect(@analytics).to receive(:track_event).
+        with(Analytics::ACCOUNT_RESET, analytics_hash)
 
-      post :cancel, params: { token: AccountResetRequest.all[0].request_token }
+      post :create
+    end
 
-      expect(SmsAccountResetCancellationNotifierJob).to have_received(:perform_now).with(
-        phone: user.phone
-      )
+    it 'redirects to the root without a flash message when the token is missing or invalid' do
+      post :create
+      expect(response).to redirect_to root_url
     end
 
-    it 'does not send an SMS if there is no phone' do
-      AccountResetService.new(user).create_request
-      allow(SmsAccountResetCancellationNotifierJob).to receive(:perform_now)
-      user.phone = nil
-      user.save!
-      user.phone_configuration.destroy
+    it 'redirects to the root with a flash message when the token is valid' do
+      token = create_account_reset_request_for(user)
 
-      post :cancel, params: { token: AccountResetRequest.all[0].request_token }
+      post :create, params: { token: token }
 
-      expect(SmsAccountResetCancellationNotifierJob).to_not have_received(:perform_now)
+      expect(flash[:success]).
+        to eq t('devise.two_factor_authentication.account_reset.successful_cancel')
+      expect(response).to redirect_to root_url
     end
 
-    it 'sends an email' do
-      AccountResetService.new(user).create_request
+    it 'signs the user out if signed in and if the token is valid' do
+      stub_sign_in(user)
+
+      token = create_account_reset_request_for(user)
 
-      @mailer = instance_double(ActionMailer::MessageDelivery, deliver_later: true)
-      allow(UserMailer).to receive(:account_reset_cancel).with(user.email).
-        and_return(@mailer)
+      expect(controller).to receive(:sign_out)
 
-      post :cancel, params: { token: AccountResetRequest.all[0].request_token }
+      post :create, params: { token: token }
     end
   end
 end

spec/features/account_reset/cancel_request_spec.rb

@@ -0,0 +1,54 @@
+require 'rails_helper'
+
+describe 'Account Reset Request: Cancellation' do
+  context 'user cancels right away from the first email' do
+    it 'cancels the request and does not delete the user', email: true do
+      TwilioService::Utils.telephony_service = FakeSms
+      user = create(:user, :signed_up)
+      signin(user.email, user.password)
+      click_link t('two_factor_authentication.login_options_link_text')
+      click_link t('devise.two_factor_authentication.account_reset.link')
+      click_button t('account_reset.request.yes_continue')
+      open_last_email
+      click_email_link_matching(/cancel\?token/)
+
+      expect(page).to have_current_path new_user_session_path
+      expect(page).
+        to have_content t('devise.two_factor_authentication.account_reset.successful_cancel')
+
+      signin(user.email, user.password)
+
+      expect(page).
+        to have_current_path login_two_factor_path(otp_delivery_preference: 'sms', reauthn: 'false')
+    end
+  end
+
+  context 'user cancels from the second email after the request has been granted' do
+    it 'cancels the request and does not delete the user', email: true do
+      TwilioService::Utils.telephony_service = FakeSms
+      user = create(:user, :signed_up)
+      signin(user.email, user.password)
+      click_link t('two_factor_authentication.login_options_link_text')
+      click_link t('devise.two_factor_authentication.account_reset.link')
+      click_button t('account_reset.request.yes_continue')
+      reset_email
+
+      Timecop.travel(Time.zone.now + 2.days) do
+        AccountResetService.grant_tokens_and_send_notifications
+        open_last_email
+        click_email_link_matching(/cancel\?token/)
+
+        expect(page).to have_current_path new_user_session_path
+        expect(page).
+          to have_content t('devise.two_factor_authentication.account_reset.successful_cancel')
+
+        signin(user.email, user.password)
+
+        expect(page).
+          to have_current_path(
+            login_two_factor_path(otp_delivery_preference: 'sms', reauthn: 'false')
+          )
+      end
+    end
+  end
+end