Skip to content

Commit

Permalink
[VI-740] Skipping MHV Account creation call during auth if user is au…
Browse files Browse the repository at this point in the history
…thenticating from a 'custom' call (#19651)
  • Loading branch information
bosawt authored Dec 2, 2024
1 parent 603349d commit ce5a96b
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 26 deletions.
10 changes: 8 additions & 2 deletions app/controllers/v1/sessions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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}")
Expand Down
8 changes: 4 additions & 4 deletions app/services/login/after_login_actions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
44 changes: 24 additions & 20 deletions spec/services/login/after_login_actions_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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:) }
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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

Expand All @@ -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

Expand All @@ -75,25 +77,25 @@

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

it 'does not create a record if login_type is not valid' do
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
Expand All @@ -107,15 +109,15 @@
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

it 'overwrites existing value if login type was seen previously' do
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
Expand All @@ -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)

Expand All @@ -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

Expand All @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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
Expand Down

0 comments on commit ce5a96b

Please sign in to comment.