Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[VI-740] Skipping MHV Account creation call during auth if user is authenticating from a 'custom' call #19651

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading