Skip to content

Commit

Permalink
updates
Browse files Browse the repository at this point in the history
  • Loading branch information
bramleyjl committed Dec 10, 2024
1 parent 4459125 commit 0552683
Show file tree
Hide file tree
Showing 7 changed files with 195 additions and 25 deletions.
4 changes: 3 additions & 1 deletion app/controllers/concerns/sign_in/authentication.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def authenticate
@current_user = load_user_object
validate_request_ip
@current_user.present?
rescue Errors::AccessTokenExpiredError => e
rescue Errors::AccessTokenExpiredError, SignIn::Errors::MPILockedAccountError => e
render json: { errors: e }, status: :forbidden
rescue Errors::StandardError => e
handle_authenticate_error(e)
Expand All @@ -28,6 +28,8 @@ def load_user(skip_expiration_check: false)
@current_user = load_user_object
validate_request_ip
@current_user.present?
rescue SignIn::Errors::MPILockedAccountError => e
render json: { errors: e }, status: :forbidden
rescue Errors::AccessTokenExpiredError => e
render json: { errors: e }, status: :forbidden unless skip_expiration_check
rescue Errors::StandardError
Expand Down
15 changes: 8 additions & 7 deletions app/models/mpi_data.rb
Original file line number Diff line number Diff line change
Expand Up @@ -233,14 +233,15 @@ def find_profile
end

def query_mpi_profile(user_key:)
do_cached_with(key: user_key, force: true) do
queried_profile = find_profile
@mvi_response = queried_profile
queried_profile
rescue ArgumentError, MPI::Errors::ArgumentError => e
log_message_to_sentry("[MPI Data] Request error: #{e.message}", :warn)
return nil
mpi_response = find_profile
if mpi_response.cache?
cache(user_key, mpi_response)
@mvi_response = mpi_response
mpi_response.profile
end
rescue ArgumentError, MPI::Errors::ArgumentError => e
log_message_to_sentry("[MPI Data] Request error: #{e.message}", :warn)
nil
end

def add_ids(response)
Expand Down
14 changes: 10 additions & 4 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -208,10 +208,6 @@ def ssn_normalized
delegate :status, to: :mpi, prefix: true
delegate :vet360_id, to: :mpi

def query_mpi_profile
mpi_profile(break_cache: true)
end

def active_mhv_ids
mpi_profile&.active_mhv_ids
end
Expand Down Expand Up @@ -322,6 +318,16 @@ def set_mhv_ids(mhv_id)

# Other MPI

def validate_mpi_profile
errors = []
updated_profile = mpi_profile(break_cache: true)
return unless updated_profile

errors.push(:mpi_death_flag) unless updated_profile.deceased_date.nil?
errors.push(:mpi_theft_flag) unless updated_profile.id_theft_flag == false
errors
end

def invalidate_mpi_cache
return unless loa3? && mpi.mpi_response_is_cached? && mpi.mvi_response

Expand Down
11 changes: 7 additions & 4 deletions app/services/sign_in/user_loader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,14 @@ def validate_account_and_session
end

def validate_mpi_profile
mpi_profile = current_user.query_mpi_profile
return unless mpi_profile
profile_errors = current_user.validate_mpi_profile
return if profile_errors.empty?

raise Errors::MPILockedAccountError.new message: 'Death Flag Detected' unless mpi_profile.deceased_date.nil?
raise Errors::MPILockedAccountError.new message: 'Theft Flag Detected' unless mpi_profile.id_theft_flag == false
if profile_errors.include?(:mpi_death_flag)
raise Errors::MPILockedAccountError.new message: 'Death Flag Detected'
elsif profile_errors.include?(:mpi_theft_flag)
raise Errors::MPILockedAccountError.new message: 'Theft Flag Detected'
end
end

def user_attributes
Expand Down
127 changes: 120 additions & 7 deletions spec/controllers/sign_in/application_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,30 @@ def service_account_auth
end
end

shared_context 'mpi profile validation' do
context 'and the MPI profile has a deceased date' do
let(:deceased_date) { '20020202' }
let(:expected_errors) { 'Death Flag Detected' }

it 'raises an MPI locked account error' do
response = subject
expect(response).to have_http_status(:forbidden)
expect(JSON.parse(response.body)['errors']).to eq(expected_errors)
end
end

context 'and the MPI profile has an id theft flag' do
let(:id_theft_flag) { true }
let(:expected_errors) { 'Theft Flag Detected' }

it 'raises an MPI locked account error' do
response = subject
expect(response).to have_http_status(:forbidden)
expect(JSON.parse(response.body)['errors']).to eq(expected_errors)
end
end
end

context 'when authorization header does not exist' do
let(:access_token) { nil }

Expand Down Expand Up @@ -210,17 +234,33 @@ def service_account_auth
end

context 'and access_token is an active JWT' do
let(:access_token_object) { create(:access_token) }
let(:session) { create(:oauth_session, user_account:) }
let(:user_account) { create(:user_account_with_verification) }
let(:icn) { user_account.icn }
let(:access_token_object) { create(:access_token, user_uuid: user_account.id, session_handle: session.handle) }
let(:access_token) { SignIn::AccessTokenJwtEncoder.new(access_token: access_token_object).perform }
let(:expected_error) { SignIn::Errors::AccessTokenMalformedJWTError.to_s }
let!(:user) do
create(:user, :loa3, uuid: access_token_object.user_uuid, fingerprint: request.remote_ip)
create(:user, :loa3, icn:, uuid: user_account.id, fingerprint: request.remote_ip)
end
let(:user_serializer) { SignIn::IntrospectSerializer.new(user) }
let(:expected_introspect_response) { JSON.parse(user_serializer.to_json) }
let(:deceased_date) { nil }
let(:id_theft_flag) { false }
let(:mpi_profile) { build(:mpi_profile, icn:, deceased_date:, id_theft_flag:) }
let(:find_profile_response) { create(:find_profile_response, profile: mpi_profile) }

before do
allow_any_instance_of(MPI::Service)
.to receive(:find_profile_by_identifier)
.with(identifier: icn, identifier_type: MPI::Constants::ICN)
.and_return(find_profile_response)
end

it_behaves_like 'user fingerprint validation'

it_behaves_like 'mpi profile validation'

it 'returns ok status' do
expect(subject).to have_http_status(:ok)
end
Expand Down Expand Up @@ -263,6 +303,30 @@ def service_account_auth
end
end

shared_context 'mpi profile validation' do
context 'and the MPI profile has a deceased date' do
let(:deceased_date) { '20020202' }
let(:expected_errors) { 'Death Flag Detected' }

it 'raises an MPI locked account error' do
response = subject
expect(response).to have_http_status(:forbidden)
expect(JSON.parse(response.body)['errors']).to eq(expected_errors)
end
end

context 'and the MPI profile has an id theft flag' do
let(:id_theft_flag) { true }
let(:expected_errors) { 'Theft Flag Detected' }

it 'raises an MPI locked account error' do
response = subject
expect(response).to have_http_status(:forbidden)
expect(JSON.parse(response.body)['errors']).to eq(expected_errors)
end
end
end

context 'when authorization header does not exist' do
let(:expected_error) { 'Access token JWT is malformed' }
let(:access_token) { nil }
Expand Down Expand Up @@ -303,17 +367,36 @@ def service_account_auth
end

context 'and access_token is an active JWT' do
let(:access_token_object) { create(:access_token) }
let(:session) { create(:oauth_session, user_account:) }
let(:user_account) { create(:user_account_with_verification) }
let(:icn) { user_account.icn }
let(:access_token_object) do
create(:access_token, user_uuid: user_account.id, session_handle: session.handle)
end
let(:access_token_cookie) { SignIn::AccessTokenJwtEncoder.new(access_token: access_token_object).perform }
let(:access_token) { SignIn::AccessTokenJwtEncoder.new(access_token: access_token_object).perform }
let(:expected_error) { SignIn::Errors::AccessTokenMalformedJWTError.to_s }
let!(:user) do
create(:user, :loa3, uuid: access_token_object.user_uuid, fingerprint: request.remote_ip)
create(:user, :loa3, icn:, uuid: user_account.id, fingerprint: request.remote_ip)
end
let(:user_serializer) { SignIn::IntrospectSerializer.new(user) }
let(:expected_introspect_response) { JSON.parse(user_serializer.to_json) }
let(:deceased_date) { nil }
let(:id_theft_flag) { false }
let(:mpi_profile) { build(:mpi_profile, icn:, deceased_date:, id_theft_flag:) }
let(:find_profile_response) { create(:find_profile_response, profile: mpi_profile) }

before do
allow_any_instance_of(MPI::Service)
.to receive(:find_profile_by_identifier)
.with(identifier: icn, identifier_type: MPI::Constants::ICN)
.and_return(find_profile_response)
end

it_behaves_like 'user fingerprint validation'

it_behaves_like 'mpi profile validation'

it 'returns ok status' do
expect(subject).to have_http_status(:ok)
end
Expand Down Expand Up @@ -359,17 +442,33 @@ def service_account_auth
end

context 'and access_token is an active JWT' do
let(:access_token_object) { create(:access_token) }
let(:session) { create(:oauth_session, user_account:) }
let(:user_account) { create(:user_account_with_verification) }
let(:icn) { user_account.icn }
let(:access_token_object) { create(:access_token, user_uuid: user_account.id, session_handle: session.handle) }
let(:access_token) { SignIn::AccessTokenJwtEncoder.new(access_token: access_token_object).perform }
let(:expected_error) { SignIn::Errors::AccessTokenMalformedJWTError.to_s }
let!(:user) do
create(:user, :loa3, uuid: access_token_object.user_uuid, fingerprint: request.remote_ip)
create(:user, :loa3, icn:, uuid: user_account.id, fingerprint: request.remote_ip)
end
let(:user_serializer) { SignIn::IntrospectSerializer.new(user) }
let(:expected_introspect_response) { JSON.parse(user_serializer.to_json) }
let(:deceased_date) { nil }
let(:id_theft_flag) { false }
let(:mpi_profile) { build(:mpi_profile, icn:, deceased_date:, id_theft_flag:) }
let(:find_profile_response) { create(:find_profile_response, profile: mpi_profile) }

before do
allow_any_instance_of(MPI::Service)
.to receive(:find_profile_by_identifier)
.with(identifier: icn, identifier_type: MPI::Constants::ICN)
.and_return(find_profile_response)
end

it_behaves_like 'user fingerprint validation'

it_behaves_like 'mpi profile validation'

it 'returns ok status' do
expect(subject).to have_http_status(:ok)
end
Expand Down Expand Up @@ -540,12 +639,21 @@ def service_account_auth
subject { get :index }

context 'with a valid authenticated request' do
let(:session) { create(:oauth_session, user_account:) }
let(:user_account) { create(:user_account) }
let(:authorization) { "Bearer #{access_token}" }
let(:access_token_object) { create(:access_token) }
let(:access_token_object) { create(:access_token, user_uuid: user_account.id, session_handle: session.handle) }
let(:access_token) { SignIn::AccessTokenJwtEncoder.new(access_token: access_token_object).perform }
let(:find_profile_response) do
create(:find_profile_response, profile: build(:mpi_profile, icn: user_account.icn))
end

before do
request.headers['Authorization'] = authorization
allow_any_instance_of(MPI::Service)
.to receive(:find_profile_by_identifier)
.with(identifier: user_account.icn, identifier_type: MPI::Constants::ICN)
.and_return(find_profile_response)
end

it 'appends user uuid to payload' do
Expand Down Expand Up @@ -592,11 +700,16 @@ def service_account_auth
fingerprint: request.remote_ip)
end
let(:expected_error) { 'Service unavailable' }
let(:find_profile_response) { create(:find_profile_response, profile: build(:mpi_profile, icn: user_account.icn)) }

before do
request.headers['Authorization'] = authorization
allow_any_instance_of(Rx::Client).to receive(:connection).and_raise(Faraday::ConnectionFailed, 'some message')
allow(Settings.sentry).to receive(:dsn).and_return('T')
allow_any_instance_of(MPI::Service)
.to receive(:find_profile_by_identifier)
.with(identifier: user_account.icn, identifier_type: MPI::Constants::ICN)
.and_return(find_profile_response)
end

it 'makes a call to sentry with request uuid and service unavailable error' do
Expand Down
16 changes: 14 additions & 2 deletions spec/controllers/sign_in/audience_validator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,26 @@ def index
let(:mobile_client_id) { 'mobile123' }
let(:invalid_client_id) { 'invalid' }

let(:valid_access_token) { create(:access_token, audience: [web_client_id, mobile_client_id]) }
let(:invalid_access_token) { create(:access_token, audience: [invalid_client_id]) }
let(:session) { create(:oauth_session, user_account:) }
let(:user_account) { create(:user_account) }
let(:valid_access_token) do
create(:access_token, user_uuid: user_account.id, session_handle: session.handle,
audience: [web_client_id, mobile_client_id])
end
let(:invalid_access_token) do
create(:access_token, user_uuid: user_account.id, session_handle: session.handle, audience: [invalid_client_id])
end
let!(:user) { create(:user, :loa3, uuid: valid_access_token.user_uuid) }
let(:access_token_cookie) { SignIn::AccessTokenJwtEncoder.new(access_token: valid_access_token).perform }
let(:find_profile_response) { create(:find_profile_response, profile: build(:mpi_profile, icn: user_account.icn)) }

describe '#authenticate' do
before do
request.cookies[SignIn::Constants::Auth::ACCESS_TOKEN_COOKIE_NAME] = access_token_cookie
allow_any_instance_of(MPI::Service)
.to receive(:find_profile_by_identifier)
.with(identifier: user_account.icn, identifier_type: MPI::Constants::ICN)
.and_return(find_profile_response)
end

context 'with a valid audience' do
Expand Down
33 changes: 33 additions & 0 deletions spec/services/sign_in/user_loader_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,20 @@
auth_broker:,
client_id: }
end
let(:identifier) { user_icn }
let(:deceased_date) { nil }
let(:id_theft_flag) { false }
let(:mpi_profile) do
build(:mpi_profile, icn: user_icn, edipi:, vha_facility_ids:, deceased_date:, id_theft_flag:)
end
let(:find_profile_response) { create(:find_profile_response, profile: mpi_profile) }

before do
stub_mpi(build(:mpi_profile, edipi:, icn: user_icn, vha_facility_ids:))
allow_any_instance_of(MPI::Service)
.to receive(:find_profile_by_identifier)
.with(identifier:, identifier_type: MPI::Constants::ICN)
.and_return(find_profile_response)
end

context 'and user is authenticated with dslogon' do
Expand All @@ -78,6 +89,28 @@
end
end

context 'when validating the user\'s MPI profile' do
context 'and the MPI profile has a deceased date' do
let(:deceased_date) { '20020202' }
let(:expected_error) { SignIn::Errors::MPILockedAccountError }
let(:expected_error_message) { 'Death Flag Detected' }

it 'raises an MPI locked account error' do
expect { subject }.to raise_error(expected_error, expected_error_message)
end
end

context 'and the MPI profile has an id theft flag' do
let(:id_theft_flag) { true }
let(:expected_error) { SignIn::Errors::MPILockedAccountError }
let(:expected_error_message) { 'Theft Flag Detected' }

it 'raises an MPI locked account error' do
expect { subject }.to raise_error(expected_error, expected_error_message)
end
end
end

it 'reloads user object with expected attributes' do
reloaded_user = subject

Expand Down

0 comments on commit 0552683

Please sign in to comment.