From ce5a96b52e1d727eb8a48816a0ab7d1dc30fd50c Mon Sep 17 00:00:00 2001
From: Trevor Bosaw <bosawt@gmail.com>
Date: Mon, 2 Dec 2024 09:41:25 -0800
Subject: [PATCH] [VI-740] Skipping MHV Account creation call during auth if
 user is authenticating from a 'custom' call (#19651)

---
 app/controllers/v1/sessions_controller.rb     | 10 ++++-
 app/services/login/after_login_actions.rb     |  8 ++--
 .../login/after_login_actions_spec.rb         | 44 ++++++++++---------
 3 files changed, 36 insertions(+), 26 deletions(-)

diff --git a/app/controllers/v1/sessions_controller.rb b/app/controllers/v1/sessions_controller.rb
index 3e7b6b72764..9a84db5b630 100644
--- a/app/controllers/v1/sessions_controller.rb
+++ b/app/controllers/v1/sessions_controller.rb
@@ -408,11 +408,17 @@ def set_cookies
     end
 
     def after_login_actions
-      client_id = url_service.tracker.payload_attr(:application)
-      Login::AfterLoginActions.new(@current_user, client_id).perform
+      Login::AfterLoginActions.new(@current_user, skip_mhv_account_creation).perform
       log_persisted_session_and_warnings
     end
 
+    def skip_mhv_account_creation
+      skip_mhv_account_creation_client = url_service.tracker.payload_attr(:application) == SAML::User::MHV_ORIGINAL_CSID
+      skip_mhv_account_creation_type = url_service.tracker.payload_attr(:type) == 'custom'
+
+      skip_mhv_account_creation_client || skip_mhv_account_creation_type
+    end
+
     def log_persisted_session_and_warnings
       obscure_token = Session.obscure_token(@session_object.token)
       Rails.logger.info("Logged in user with id #{@session_object&.uuid}, token #{obscure_token}")
diff --git a/app/services/login/after_login_actions.rb b/app/services/login/after_login_actions.rb
index e97f4145c32..18464aa4d6d 100644
--- a/app/services/login/after_login_actions.rb
+++ b/app/services/login/after_login_actions.rb
@@ -6,11 +6,11 @@ module Login
   class AfterLoginActions
     include Accountable
 
-    attr_reader :current_user, :client_id
+    attr_reader :current_user, :skip_mhv_account_creation
 
-    def initialize(user, client_id)
+    def initialize(user, skip_mhv_account_creation)
       @current_user = user
-      @client_id = client_id
+      @skip_mhv_account_creation = skip_mhv_account_creation
     end
 
     def perform
@@ -32,7 +32,7 @@ def perform
     private
 
     def create_mhv_account
-      return if client_id.in?(SAML::URLService::SKIP_MHV_ACCOUNT_CREATION_CLIENTS)
+      return if skip_mhv_account_creation
 
       current_user.create_mhv_account_async
     end
diff --git a/spec/services/login/after_login_actions_spec.rb b/spec/services/login/after_login_actions_spec.rb
index 88547d7fb5e..c12262c9d94 100644
--- a/spec/services/login/after_login_actions_spec.rb
+++ b/spec/services/login/after_login_actions_spec.rb
@@ -4,7 +4,7 @@
 
 RSpec.describe Login::AfterLoginActions do
   describe '#perform' do
-    let(:client_id) { 'some-client-id' }
+    let(:skip_mhv_account_creation) { false }
 
     context 'creating credential email' do
       let(:user) { create(:user, email:, idme_uuid:) }
@@ -13,7 +13,7 @@
       let(:email) { 'some-email' }
 
       it 'creates a user credential email with expected attributes' do
-        expect { described_class.new(user, client_id).perform }.to change(UserCredentialEmail, :count)
+        expect { described_class.new(user, skip_mhv_account_creation).perform }.to change(UserCredentialEmail, :count)
         user_credential_email = user.user_verification.user_credential_email
         expect(user_credential_email.credential_email).to eq(email)
       end
@@ -30,7 +30,9 @@
       after { Timecop.return }
 
       it 'creates a user acceptable verified credential with expected attributes' do
-        expect { described_class.new(user, client_id).perform }.to change(UserAcceptableVerifiedCredential, :count)
+        expect do
+          described_class.new(user, skip_mhv_account_creation).perform
+        end.to change(UserAcceptableVerifiedCredential, :count)
         user_avc = UserAcceptableVerifiedCredential.find_by(user_account: user.user_account)
         expect(user_avc.idme_verified_credential_at).to eq(expected_avc_at)
       end
@@ -47,7 +49,7 @@
 
       it 'does not call TUD account checkout' do
         expect_any_instance_of(TestUserDashboard::UpdateUser).not_to receive(:call)
-        described_class.new(user, client_id).perform
+        described_class.new(user, skip_mhv_account_creation).perform
       end
     end
 
@@ -62,7 +64,7 @@
 
       it 'calls TUD account checkout' do
         expect_any_instance_of(TestUserDashboard::UpdateUser).to receive(:call)
-        described_class.new(user, client_id).perform
+        described_class.new(user, skip_mhv_account_creation).perform
       end
     end
 
@@ -75,17 +77,17 @@
 
       context 'with non-existent login stats record' do
         it 'creates an account_login_stats record' do
-          expect { described_class.new(user, client_id).perform }.to \
+          expect { described_class.new(user, skip_mhv_account_creation).perform }.to \
             change(AccountLoginStat, :count).by(1)
         end
 
         it 'updates the correct login stats column' do
-          described_class.new(user, client_id).perform
+          described_class.new(user, skip_mhv_account_creation).perform
           expect(AccountLoginStat.last.send("#{login_type_stat}_at")).not_to be_nil
         end
 
         it 'updates the current_verification column' do
-          described_class.new(user, client_id).perform
+          described_class.new(user, skip_mhv_account_creation).perform
           expect(AccountLoginStat.last.current_verification).to eq('loa1')
         end
 
@@ -93,7 +95,7 @@
           login_type = 'something_invalid'
           allow_any_instance_of(UserIdentity).to receive(:sign_in).and_return(service_name: login_type)
 
-          expect { described_class.new(user, client_id).perform }.not_to \
+          expect { described_class.new(user, skip_mhv_account_creation).perform }.not_to \
             change(AccountLoginStat, :count)
         end
       end
@@ -107,7 +109,7 @@
         end
 
         it 'does not create another record' do
-          expect { described_class.new(user, client_id).perform }.not_to \
+          expect { described_class.new(user, skip_mhv_account_creation).perform }.not_to \
             change(AccountLoginStat, :count)
         end
 
@@ -115,7 +117,7 @@
           stat = AccountLoginStat.last
 
           expect do
-            described_class.new(user, client_id).perform
+            described_class.new(user, skip_mhv_account_creation).perform
             stat.reload
           end.to change(stat, :myhealthevet_at)
         end
@@ -126,7 +128,7 @@
           stat = AccountLoginStat.last
 
           expect do
-            described_class.new(user, client_id).perform
+            described_class.new(user, skip_mhv_account_creation).perform
             stat.reload
           end.not_to change(stat, :myhealthevet_at)
 
@@ -136,7 +138,7 @@
         it 'triggers sentry error if update fails' do
           allow_any_instance_of(AccountLoginStat).to receive(:update!).and_raise('Failure!')
           expect_any_instance_of(described_class).to receive(:log_error)
-          described_class.new(user, client_id).perform
+          described_class.new(user, skip_mhv_account_creation).perform
         end
       end
 
@@ -145,7 +147,7 @@
 
         it 'triggers sentry error message' do
           expect_any_instance_of(described_class).to receive(:no_account_log_message)
-          expect { described_class.new(user, client_id).perform }.not_to \
+          expect { described_class.new(user, skip_mhv_account_creation).perform }.not_to \
             change(AccountLoginStat, :count)
         end
       end
@@ -168,7 +170,7 @@
       shared_examples 'identity-mpi id validation' do
         it 'logs a warning when Identity & MPI values conflict' do
           expect(Rails.logger).to receive(:warn).at_least(:once).with(expected_error_message, expected_error_data)
-          described_class.new(loa3_user, client_id).perform
+          described_class.new(loa3_user, skip_mhv_account_creation).perform
         end
       end
 
@@ -218,18 +220,20 @@
         allow(user).to receive(:create_mhv_account_async)
       end
 
-      context 'when the client_id is not in SKIP_MHV_ACCOUNT_CREATION_CLIENTS' do
+      context 'when skip_mhv_account_creation is set to false' do
+        let(:skip_mhv_account_creation) { false }
+
         it 'calls create_mhv_account_async' do
-          described_class.new(user, client_id).perform
+          described_class.new(user, skip_mhv_account_creation).perform
           expect(user).to have_received(:create_mhv_account_async)
         end
       end
 
-      context 'when the client_id is in SKIP_MHV_ACCOUNT_CREATION_CLIENTS' do
-        let(:client_id) { 'mhv' }
+      context 'when skip_mhv_account_creation is set to true' do
+        let(:skip_mhv_account_creation) { true }
 
         it 'does not call create_mhv_account_async' do
-          described_class.new(user, client_id).perform
+          described_class.new(user, skip_mhv_account_creation).perform
           expect(user).not_to have_received(:create_mhv_account_async)
         end
       end