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