Merge pull request #1496 from 18F/ab-reactivate-session

Use AccountReactivationSession object

app/controllers/concerns/account_recovery_concern.rb

@@ -5,4 +5,11 @@ def confirm_password_reset_profile
     return if current_user.decorate.password_reset_profile
     redirect_to root_url
   end
+
+  def reactivate_account_session
+    @_reactivate_account_session ||= ReactivateAccountSession.new(
+      user: current_user,
+      user_session: user_session
+    )
+  end
 end

app/controllers/reactivate_account_controller.rb

@@ -4,12 +4,10 @@ class ReactivateAccountController < ApplicationController
   before_action :confirm_two_factor_authenticated
   before_action :confirm_password_reset_profile
 
-  def index
-    user_session[:acknowledge_personal_key] ||= true
-  end
+  def index; end
 
   def update
-    user_session.delete(:acknowledge_personal_key)
+    reactivate_account_session.suspend
     redirect_to verify_url
   end
 end

app/controllers/users/verify_password_controller.rb

@@ -18,9 +18,7 @@ def update
       result = verify_password_form.submit
 
       if result.success?
-        flash[:personal_key] = result.extra[:personal_key]
-        user_session.delete(:account_recovery)
-        redirect_to account_url
+        handle_success(result)
       else
         render :new
       end
@@ -29,12 +27,19 @@ def update
     private
 
     def confirm_personal_key
-      account_recovery = user_session[:account_recovery]
-      redirect_to root_url unless account_recovery[:personal_key]
+      return if reactivate_account_session.personal_key?
+      redirect_to root_url
     end
 
     def decrypted_pii
-      @_decrypted_pii ||= Pii::Attributes.new_from_json(user_session[:decrypted_pii])
+      pii = reactivate_account_session.decrypted_pii
+      @_decrypted_pii ||= Pii::Attributes.new_from_json(pii)
+    end
+
+    def handle_success(result)
+      flash[:personal_key] = result.extra[:personal_key]
+      reactivate_account_session.clear
+      redirect_to account_url
     end
 
     def verify_password_form

app/controllers/users/verify_personal_key_controller.rb

@@ -4,11 +4,9 @@ class VerifyPersonalKeyController < ApplicationController
 
     before_action :confirm_two_factor_authenticated
     before_action :confirm_password_reset_profile
-    before_action :init_account_recovery, only: [:create]
+    before_action :init_account_recovery, only: [:new]
 
     def new
-      flash.now[:notice] = t('notices.account_recovery') unless user_session[:account_recovery]
-
       @personal_key_form = VerifyPersonalKeyForm.new(
         user: current_user,
         personal_key: ''
@@ -28,20 +26,19 @@ def create
     private
 
     def init_account_recovery
-      user_session[:account_recovery] ||= {
-        personal_key: false,
-      }
+      return if reactivate_account_session.started?
+
+      flash.now[:notice] = t('notices.account_recovery')
+      reactivate_account_session.start
     end
 
     def handle_success(result)
-      user_session[:account_recovery][:personal_key] = true
-      user_session[:decrypted_pii] = result.extra[:decrypted_pii]
-
+      reactivate_account_session.store_decrypted_pii(result.extra[:decrypted_pii])
       redirect_to verify_password_url
     end
 
     def handle_failure(result)
-      flash[:error] = result.errors[:personal_key].last
+      flash.now[:error] = result.errors[:personal_key].last
       render :new
     end
 

app/controllers/verify_controller.rb

@@ -1,5 +1,6 @@
 class VerifyController < ApplicationController
   include IdvSession
+  include AccountRecoveryConcern
 
   before_action :confirm_two_factor_authenticated
   before_action :confirm_idv_needed, only: %i[cancel fail]
@@ -28,14 +29,11 @@ def fail
   private
 
   def profile_needs_reactivation?
-    return unless password_reset_profile && user_session[:acknowledge_personal_key] == true
+    return unless reactivate_account_session.started?
+    confirm_password_reset_profile
     redirect_to reactivate_account_url
   end
 
-  def password_reset_profile
-    current_user.decorate.password_reset_profile
-  end
-
   def active_profile?
     current_user.active_profile.present?
   end

app/services/reactivate_account_session.rb

@@ -0,0 +1,55 @@
+class ReactivateAccountSession
+  SESSION_KEY = :reactivate_account
+
+  def initialize(user:, user_session:)
+    @user = user
+    @session = user_session
+
+    session[SESSION_KEY] ||= generate_session
+  end
+
+  def clear
+    session.delete(SESSION_KEY)
+  end
+
+  def start
+    reactivate_account_session[:active] = true
+  end
+
+  def started?
+    reactivate_account_session[:active]
+  end
+
+  def suspend
+    session[SESSION_KEY] = generate_session
+  end
+
+  def store_decrypted_pii(pii)
+    reactivate_account_session[:personal_key] = true
+    reactivate_account_session[:pii] = pii
+  end
+
+  def personal_key?
+    reactivate_account_session[:personal_key]
+  end
+
+  def decrypted_pii
+    reactivate_account_session[:pii]
+  end
+
+  private
+
+  attr_reader :session
+
+  def generate_session
+    {
+      active: false,
+      personal_key: false,
+      pii: nil,
+    }
+  end
+
+  def reactivate_account_session
+    session[SESSION_KEY]
+  end
+end

spec/controllers/reactivate_account_controller_spec.rb

@@ -23,12 +23,6 @@
 
         expect(subject).to render_template(:index)
       end
-
-      it 'sets a key on the user session for future redirect guidance' do
-        get :index
-
-        expect(subject.user_session[:acknowledge_personal_key]).to eq true
-      end
     end
 
     context 'wthout a password reset profile' do

spec/controllers/users/verify_password_controller_spec.rb

@@ -8,7 +8,6 @@
 
   before do
     stub_sign_in(user)
-    subject.user_session[:account_recovery] = recovery_hash
   end
 
   context 'without password_reset_profile' do
@@ -21,12 +20,34 @@
     end
   end
 
+  context 'without personal key flag set' do
+    let(:profiles) { [create(:profile, deactivation_reason: :password_reset)] }
+
+    describe '#new' do
+      it 'redirects to the root url' do
+        get :new
+        expect(response).to redirect_to(root_url)
+      end
+    end
+
+    describe '#update' do
+      it 'redirects to the root url' do
+        get :new
+        expect(response).to redirect_to(root_url)
+      end
+    end
+  end
+
   context 'with password reset profile' do
     let(:profiles) { [create(:profile, deactivation_reason: :password_reset)] }
     let(:response_ok) { FormResponse.new(success: true, errors: {}, extra: { personal_key: key }) }
     let(:response_bad) { FormResponse.new(success: false, errors: {}) }
     let(:key) { 'key' }
 
+    before do
+      allow(subject.reactivate_account_session).to receive(:personal_key?).and_return(personal_key)
+    end
+
     describe '#new' do
       it 'renders the `new` template' do
         get :new

spec/controllers/users/verify_personal_key_controller_spec.rb

@@ -63,7 +63,7 @@
       it 'stores that the personal key was entered in the user session' do
         post :create, personal_key: personal_key
 
-        expect(subject.user_session[:account_recovery][:personal_key]).to eq(true)
+        expect(subject.reactivate_account_session.personal_key?).to eq(true)
       end
     end
 

spec/controllers/verify_controller_spec.rb

@@ -38,7 +38,7 @@
     it 'redirects to account recovery if user has a password reset profile' do
       profile = create(:profile, deactivation_reason: :password_reset)
       stub_sign_in(profile.user)
-      allow(subject).to receive(:user_session).and_return(acknowledge_personal_key: true)
+      allow(subject.reactivate_account_session).to receive(:started?).and_return(true)
 
       get :index
 

spec/services/reactivate_account_session_spec.rb

@@ -0,0 +1,94 @@
+require 'rails_helper'
+
+describe ReactivateAccountSession do
+  let(:user) { build(:user) }
+  let(:user_session) { {} }
+
+  before do
+    @reactivate_account_session = ReactivateAccountSession.new(
+      user: user,
+      user_session: user_session
+    )
+  end
+
+  describe '#clear' do
+    it 'deletes the reactivate account session object from user_session' do
+      expect(user_session).to have_key(:reactivate_account)
+
+      @reactivate_account_session.clear
+
+      expect(user_session).to be_empty
+    end
+  end
+
+  describe '#start' do
+    it 'sets the session object `active` flag to true' do
+      @reactivate_account_session.start
+      expect(user_session[:reactivate_account][:active]).to be(true)
+    end
+  end
+
+  describe '#started?' do
+    it 'initializes set to false' do
+      expect(@reactivate_account_session.started?).to be(false)
+    end
+
+    it 'returns a boolean if the account reactivate flow has started or not' do
+      @reactivate_account_session.start
+      expect(@reactivate_account_session.started?).to be(true)
+    end
+  end
+
+  describe '#suspend' do
+    it 'sets the reactivate account object back to its defaults' do
+      pii = {}
+
+      @reactivate_account_session.start
+      @reactivate_account_session.store_decrypted_pii(pii)
+
+      expect(@reactivate_account_session.started?).to be(true)
+      expect(@reactivate_account_session.personal_key?).to be(true)
+      expect(@reactivate_account_session.decrypted_pii).to be(pii)
+
+      @reactivate_account_session.suspend
+
+      expect(@reactivate_account_session.started?).to be(false)
+      expect(@reactivate_account_session.personal_key?).to be(false)
+      expect(@reactivate_account_session.decrypted_pii).to eq(nil)
+    end
+  end
+
+  describe '#store_decrypted_pii' do
+    it 'stores the supplied object in the session and toggles `personal_key` flag' do
+      pii = {}
+      @reactivate_account_session.store_decrypted_pii(pii)
+      account_reactivation_obj = user_session[:reactivate_account]
+      expect(account_reactivation_obj[:personal_key]).to be(true)
+      expect(account_reactivation_obj[:pii]).to eq(pii)
+    end
+  end
+
+  describe '#personal_key?' do
+    it 'defaults to false' do
+      expect(@reactivate_account_session.personal_key?).to be(false)
+    end
+
+    it 'returns a boolean indicating if the user hsa validated their personal key' do
+      @reactivate_account_session.store_decrypted_pii({})
+      expect(@reactivate_account_session.personal_key?).to be(true)
+    end
+  end
+
+  describe '#decrypted_pii' do
+    it 'returns nil as a default' do
+      expect(@reactivate_account_session.decrypted_pii).to eq(nil)
+    end
+
+    it 'returns the pii stored in the session' do
+      pii = {}
+      @reactivate_account_session.store_decrypted_pii(pii)
+
+      expect(@reactivate_account_session.decrypted_pii).to eq(pii)
+    end
+  end
+end