diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 1833c0db8e4..7f80f621e36 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -542,10 +542,7 @@ def user_duplicate_profiles_detected? return false unless sp_eligible_for_one_account? profile = current_user&.active_profile return false unless profile - DuplicateProfileConfirmation.where( - profile_id: profile.id, - confirmed_all: nil, - ).present? + user_session[:duplicate_profile_ids].present? end def sp_eligible_for_one_account? diff --git a/app/controllers/duplicate_profiles_detected_controller.rb b/app/controllers/duplicate_profiles_detected_controller.rb index 775558a9b69..aaa9d5a19ac 100644 --- a/app/controllers/duplicate_profiles_detected_controller.rb +++ b/app/controllers/duplicate_profiles_detected_controller.rb @@ -5,19 +5,24 @@ class DuplicateProfilesDetectedController < ApplicationController before_action :redirect_unless_user_has_active_duplicate_profile_confirmation def show - @dupe_profiles_detected_presenter = DuplicateProfilesDetectedPresenter.new(user: current_user) + @dupe_profiles_detected_presenter = DuplicateProfilesDetectedPresenter.new( + user: current_user, user_session: user_session, + ) analytics.one_account_duplicate_profiles_detected end def do_not_recognize_profiles analytics.one_account_unknown_profile_detected - dupe_profile_confirmation.mark_some_profiles_not_recognized + + user_session.delete(:duplicate_profile_ids) + redirect_to after_sign_in_path_for(current_user) end def recognize_all_profiles analytics.one_account_recognize_all_profiles - dupe_profile_confirmation.mark_all_profiles_recognized + + user_session.delete(:duplicate_profile_ids) redirect_to after_sign_in_path_for(current_user) end @@ -25,17 +30,10 @@ def recognize_all_profiles def redirect_unless_user_has_active_duplicate_profile_confirmation if current_user&.active_profile.present? - if dupe_profile_confirmation && dupe_profile_confirmation&.confirmed_all.nil? + if user_session[:duplicate_profile_ids].present? return end end redirect_to root_url end - - def dupe_profile_confirmation - return unless current_user.active_profile - @dupe_profile_confirmation ||= DuplicateProfileConfirmation.find_by( - profile_id: current_user.active_profile.id, - ) - end end diff --git a/app/models/profile.rb b/app/models/profile.rb index 0bbe5519995..b6dc89b7d37 100644 --- a/app/models/profile.rb +++ b/app/models/profile.rb @@ -16,7 +16,6 @@ class Profile < ApplicationRecord # rubocop:enable Rails/InverseOf has_many :gpo_confirmation_codes, dependent: :destroy has_one :in_person_enrollment, dependent: :destroy - has_many :duplicate_profile_confirmations, dependent: :destroy validates :active, uniqueness: { scope: :user_id, if: :active? } diff --git a/app/presenters/duplicate_profiles_detected_presenter.rb b/app/presenters/duplicate_profiles_detected_presenter.rb index 698860e42c6..94032cc58e7 100644 --- a/app/presenters/duplicate_profiles_detected_presenter.rb +++ b/app/presenters/duplicate_profiles_detected_presenter.rb @@ -3,13 +3,11 @@ class DuplicateProfilesDetectedPresenter include ActionView::Helpers::TranslationHelper - attr_reader :user, :dupe_profile_confirmation + attr_reader :user, :user_session - def initialize(user:) + def initialize(user:, user_session:) @user = user - @dupe_profile_confirmation = DuplicateProfileConfirmation.where( - profile_id: user.active_profile.id, - ).last + @user_session = user_session end def heading @@ -21,7 +19,7 @@ def intro end def duplicate_profiles - profile_ids = dupe_profile_confirmation.duplicate_profile_ids + profile_ids = user_session[:duplicate_profile_ids] profiles = Profile.where(id: profile_ids) profiles.map do |profile| @@ -55,6 +53,6 @@ def dont_recognize_some_profiles private def multiple_dupe_profiles? - dupe_profile_confirmation.duplicate_profile_ids.count > 1 + user_session[:duplicate_profile_ids].count > 1 end end diff --git a/app/services/duplicate_profile_checker.rb b/app/services/duplicate_profile_checker.rb index 692d4fd9e2b..d8136d4ea22 100644 --- a/app/services/duplicate_profile_checker.rb +++ b/app/services/duplicate_profile_checker.rb @@ -11,29 +11,15 @@ def initialize(user:, user_session:, sp:) end def check_for_duplicate_profiles - return unless user_has_ial2_profile? + return unless user_has_ial2_profile? && user_sp_eligible_for_one_account? cacher = Pii::Cacher.new(user, user_session) pii = cacher.fetch(profile.id) duplicate_ssn_finder = Idv::DuplicateSsnFinder.new(user:, ssn: pii[:ssn]) associated_profiles = duplicate_ssn_finder.associated_facial_match_profiles_with_ssn if !duplicate_ssn_finder.ial2_profile_ssn_is_unique? - confirmation = DuplicateProfileConfirmation.find_by(profile_id: profile.id) - if confirmation - if !(confirmation.duplicate_profile_ids == associated_profiles.map(&:id)) - confirmation.update( - confirmed_at: Time.zone.now, - confirmed_all: false, - duplicate_profile_ids: associated_profiles.map(&:id), - ) - end - else - DuplicateProfileConfirmation.create( - profile_id: profile.id, - confirmed_at: Time.zone.now, - duplicate_profile_ids: associated_profiles.map(&:id), - ) - end + ids = associated_profiles.map(&:id) + user_session[:duplicate_profile_ids] = ids end end @@ -42,4 +28,8 @@ def check_for_duplicate_profiles def user_has_ial2_profile? user.identity_verified_with_facial_match? end + + def user_sp_eligible_for_one_account? + IdentityConfig.store.eligible_one_account_providers.include?(sp&.issuer) + end end diff --git a/app/services/idv/duplicate_ssn_finder.rb b/app/services/idv/duplicate_ssn_finder.rb index 4459bc4621b..324e69494f7 100644 --- a/app/services/idv/duplicate_ssn_finder.rb +++ b/app/services/idv/duplicate_ssn_finder.rb @@ -10,11 +10,15 @@ def initialize(user:, ssn:) end def ssn_is_unique? - Profile.where(ssn_signature: ssn_signatures).where.not(user_id: user.id).empty? + Profile.where(ssn_signature: ssn_signatures) + .where(initiating_service_provider_issuer: sp_eligible_for_one_account) + .where.not(user_id: user.id).empty? end def associated_facial_match_profiles_with_ssn - Profile.active.facial_match.where(ssn_signature: ssn_signatures).where.not(user_id: user.id) + Profile.active.facial_match.where(ssn_signature: ssn_signatures) + .where(initiating_service_provider_issuer: sp_eligible_for_one_account) + .where.not(user_id: user.id) end def ial2_profile_ssn_is_unique? @@ -43,5 +47,9 @@ def ssn_signatures end end end + + def sp_eligible_for_one_account + IdentityConfig.store.eligible_one_account_providers + end end end diff --git a/app/views/duplicate_profiles_detected/show.html.erb b/app/views/duplicate_profiles_detected/show.html.erb index 5f448952275..1d48a7b5697 100644 --- a/app/views/duplicate_profiles_detected/show.html.erb +++ b/app/views/duplicate_profiles_detected/show.html.erb @@ -21,11 +21,13 @@ timestamp_html: render(TimeComponent.new(time: dupe_profile[:created_at])), ) %>
+ <% if dupe_profile[:last_sign_in] %><%= t( 'duplicate_profiles_detected.last_sign_in_at_html', timestamp_html: render(TimeComponent.new(time: dupe_profile[:last_sign_in])), ) %>
+ <% end %> <% end %> diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 469926af6ef..a63f0de104f 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -666,21 +666,6 @@ def index expect(response.body).to eq('false') end end - - context 'when unconfirmed duplicate profile confirmations exist' do - before do - create( - :duplicate_profile_confirmation, - profile: active_profile, - confirmed_all: nil, - ) - end - - it 'returns true' do - get :index - expect(response.body).to eq('true') - end - end end end end diff --git a/spec/controllers/duplicate_profiles_detected_controller_spec.rb b/spec/controllers/duplicate_profiles_detected_controller_spec.rb index 14bb03ab7ee..84cdc2c26e8 100644 --- a/spec/controllers/duplicate_profiles_detected_controller_spec.rb +++ b/spec/controllers/duplicate_profiles_detected_controller_spec.rb @@ -7,6 +7,7 @@ before do stub_sign_in(user) stub_analytics + session[:duplicate_profile_ids] = profile2.id end describe '#show' do @@ -21,11 +22,7 @@ context 'when user has an active duplicate profile confirmation' do before do - DuplicateProfileConfirmation.create( - profile_id: user.active_profile.id, - confirmed_at: Time.zone.now, - duplicate_profile_ids: [profile2.id], - ) + allow(controller).to receive(:user_session).and_return(session) end it 'renders the show template' do @@ -34,7 +31,8 @@ end it 'initializes the DuplicateProfilesDetectedPresenter' do - expect(DuplicateProfilesDetectedPresenter).to receive(:new).with(user: user) + expect(DuplicateProfilesDetectedPresenter).to receive(:new) + .with(user: user, user_session: session) get :show end @@ -50,11 +48,7 @@ describe '#do_not_recognize_profiles' do before do - @dupe_profile_confirmation = DuplicateProfileConfirmation.create( - profile_id: user.active_profile.id, - confirmed_at: Time.zone.now, - duplicate_profile_ids: [profile2.id], - ) + allow(controller).to receive(:user_session).and_return(session) end it 'logs an event' do @@ -64,35 +58,17 @@ :one_account_unknown_profile_detected, ) end - - it 'marks some accounts as not recognized' do - post :do_not_recognize_profiles - @dupe_profile_confirmation.reload - expect(@dupe_profile_confirmation.confirmed_all).to eq(false) - end end describe '#recognize_all_profiles' do before do - @dupe_profile_confirmation = DuplicateProfileConfirmation.create( - profile_id: user.active_profile.id, - confirmed_at: Time.zone.now, - duplicate_profile_ids: [profile2.id], - ) + allow(controller).to receive(:user_session).and_return(session) end it 'logs an analytics event' do post :recognize_all_profiles expect(@analytics).to have_logged_event end - - it 'marks profile dupe confirmation as recognized' do - post :recognize_all_profiles - - @dupe_profile_confirmation.reload - - expect(@dupe_profile_confirmation.confirmed_all).to eq(true) - end end describe '#redirect_unless_user_has_active_duplicate_profile_confirmation' do diff --git a/spec/jobs/resolution_proofing_job_spec.rb b/spec/jobs/resolution_proofing_job_spec.rb index b0849788565..09ab960a4fd 100644 --- a/spec/jobs/resolution_proofing_job_spec.rb +++ b/spec/jobs/resolution_proofing_job_spec.rb @@ -62,7 +62,9 @@ context 'when the SSN is not unique' do before do - create(:profile, pii: Idp::Constants::MOCK_IDV_APPLICANT_WITH_SSN) + allow(IdentityConfig.store).to receive(:eligible_one_account_providers) + .and_return(['urn:gov:gsa:openidconnect:inactive:sp:test']) + create(:profile, :facial_match_proof, pii: Idp::Constants::MOCK_IDV_APPLICANT_WITH_SSN) end it 'sets ssn_is_unique: false on the result' do diff --git a/spec/presenters/duplicate_profiles_detected_presenter_spec.rb b/spec/presenters/duplicate_profiles_detected_presenter_spec.rb index 40c4321d4ed..53fcf55b380 100644 --- a/spec/presenters/duplicate_profiles_detected_presenter_spec.rb +++ b/spec/presenters/duplicate_profiles_detected_presenter_spec.rb @@ -2,28 +2,15 @@ RSpec.describe DuplicateProfilesDetectedPresenter do let(:user) { create(:user, :proofed_with_selfie) } - let(:presenter) { described_class.new(user: user) } + let(:user_session) { {} } + let(:presenter) { described_class.new(user: user, user_session: user_session) } let(:profile2) { create(:profile, :facial_match_proof) } - before do - DuplicateProfileConfirmation.create( - profile_id: user.active_profile.id, - confirmed_at: Time.zone.now, - duplicate_profile_ids: [profile2.id], - ) - end - describe '#duplicate_profiles' do context 'when multiple duplicate profiles were found for user' do let(:profile3) { create(:profile, :facial_match_proof) } - before do - confirmation = DuplicateProfileConfirmation.find_by( - profile_id: user.active_profile.id, - ) - confirmation.update!( - duplicate_profile_ids: [profile2.id, profile3.id], - ) + user_session[:duplicate_profile_ids] = [profile2.id, profile3.id] end it 'should return multiple elements' do @@ -32,6 +19,9 @@ end context 'when a single duplicate profiles were found for user' do + before do + user_session[:duplicate_profile_ids] = [profile2.id] + end it 'should return singular element' do expect(presenter.duplicate_profiles.count).to eq(1) end @@ -43,12 +33,7 @@ let(:profile3) { create(:profile, :facial_match_proof) } before do - confirmation = DuplicateProfileConfirmation.find_by( - profile_id: user.active_profile.id, - ) - confirmation.update!( - duplicate_profile_ids: [profile2.id, profile3.id], - ) + user_session[:duplicate_profile_ids] = [profile2.id, profile3.id] end it 'should return plural text' do @@ -58,6 +43,9 @@ end context 'when a single duplicate profiles were found for user' do + before do + user_session[:duplicate_profile_ids] = [profile2.id] + end it 'should return singular text' do expect(presenter.recognize_all_profiles) .to eq(I18n.t('duplicate_profiles_detected.yes_single')) @@ -70,12 +58,7 @@ let(:profile3) { create(:profile, :facial_match_proof) } before do - confirmation = DuplicateProfileConfirmation.find_by( - profile_id: user.active_profile.id, - ) - confirmation.update!( - duplicate_profile_ids: [profile2.id, profile3.id], - ) + user_session[:duplicate_profile_ids] = [profile2.id, profile3.id] end it 'should return multiple text' do @@ -85,6 +68,10 @@ end context 'when a single duplicate profiles were found for user' do + before do + user_session[:duplicate_profile_ids] = [profile2.id] + end + it 'should return singular text' do expect(presenter.dont_recognize_some_profiles) .to eq(I18n.t('duplicate_profiles_detected.no_recognize_single')) diff --git a/spec/services/duplicate_profile_checker_spec.rb b/spec/services/duplicate_profile_checker_spec.rb index 3f86d53963a..20d2af32e34 100644 --- a/spec/services/duplicate_profile_checker_spec.rb +++ b/spec/services/duplicate_profile_checker_spec.rb @@ -30,68 +30,6 @@ end context 'when user has active IAL2 profile' do - context 'when user has already been verified for duplicate profile' do - let(:user2) { create(:user, :fully_registered) } - let!(:profile2) do - profile2 = create( - :profile, :active, - :facial_match_proof, - user: user2, - initiating_service_provider_issuer: sp.issuer - ) - profile2.encrypt_pii(active_pii, user2.password) - profile2.save - profile2 - end - - before do - DuplicateProfileConfirmation.create( - profile_id: profile.id, - confirmed_at: Time.zone.now, - duplicate_profile_ids: [profile2.id], - ) - end - - it 'does not create a new duplicate profile confirmation' do - expect(DuplicateProfileConfirmation.where(profile_id: profile.id).size).to eq(1) - dupe_profile_checker = DuplicateProfileChecker.new( - user: user, user_session: session, - sp: sp - ) - dupe_profile_checker.check_for_duplicate_profiles - - expect(DuplicateProfileConfirmation.where(profile_id: profile.id).size).to eq(1) - end - - context 'when a new duplicate profile has been added since last login' do - let(:user3) { create(:user, :fully_registered) } - let!(:profile3) do - profile3 = create( - :profile, :active, - :facial_match_proof, - user: user3, - initiating_service_provider_issuer: sp.issuer - ) - profile3.encrypt_pii(active_pii, user3.password) - profile3.save - profile3 - end - - it 'should update duplicate confirmation to include all ids' do - confirmation = DuplicateProfileConfirmation.where(profile_id: profile.id).first - expect(confirmation.duplicate_profile_ids).to eq([profile2.id]) - - dupe_profile_checker = DuplicateProfileChecker.new( - user: user, user_session: session, - sp: sp - ) - dupe_profile_checker.check_for_duplicate_profiles - confirmation.reload - expect(confirmation.duplicate_profile_ids).to eq([profile2.id, profile3.id]) - end - end - end - context 'when user has not been checked for duplicate profile' do context 'when user does not have other accounts with matching profile' do let(:user2) { create(:user, :proofed_with_selfie) } @@ -103,7 +41,7 @@ ) dupe_profile_checker.check_for_duplicate_profiles - expect(DuplicateProfileConfirmation.where(profile_id: profile.id)).to be_empty + expect(session[:duplicate_profile_ids]).to be(nil) end end @@ -128,15 +66,16 @@ end it 'creates a new duplicate profile confirmation entry' do - expect(DuplicateProfileConfirmation.where(profile_id: profile.id).first).to eq(nil) + allow(IdentityConfig.store).to receive(:eligible_one_account_providers) + .and_return([sp.issuer]) + expect(session[:duplicate_profile_ids]).to be(nil) dupe_profile_checker = DuplicateProfileChecker.new( user: user, user_session: session, sp: sp ) dupe_profile_checker.check_for_duplicate_profiles - - expect(DuplicateProfileConfirmation.where(profile_id: profile.id).first).to be_present + expect(session[:duplicate_profile_ids]).to eq([user2.profiles.last.id]) end end end @@ -160,7 +99,7 @@ ) dupe_profile_checker.check_for_duplicate_profiles - expect(DuplicateProfileConfirmation.where(profile_id: user.active_profile.id)).to be_empty + expect(session[:duplicate_profile_ids]).to be(nil) end end @@ -174,7 +113,7 @@ ) dupe_profile_checker.check_for_duplicate_profiles - expect(DuplicateProfileConfirmation.where(profile_id: profile.id)).to be_empty + expect(session[:duplicate_profile_ids]).to be(nil) end end end diff --git a/spec/services/idv/duplicate_ssn_finder_spec.rb b/spec/services/idv/duplicate_ssn_finder_spec.rb index f7463a788a7..f6207e241b9 100644 --- a/spec/services/idv/duplicate_ssn_finder_spec.rb +++ b/spec/services/idv/duplicate_ssn_finder_spec.rb @@ -7,19 +7,24 @@ subject { described_class.new(ssn: ssn, user: user) } + before do + allow(IdentityConfig.store).to receive(:eligible_one_account_providers) + .and_return(['urn:gov:gsa:openidconnect:inactive:sp:test']) + end + context 'when the ssn is unique' do it { expect(subject.ssn_is_unique?).to eq(true) } end context 'when ssn is already taken by another profile' do it 'returns false' do - create(:profile, pii: { ssn: ssn }) + create(:profile, :facial_match_proof, pii: { ssn: ssn }) expect(subject.ssn_is_unique?).to eq false end it 'recognizes fingerprint regardless of HMAC key age' do - create(:profile, pii: { ssn: ssn }) + create(:profile, :facial_match_proof, pii: { ssn: ssn }) rotate_hmac_key expect(subject.ssn_is_unique?).to eq false @@ -27,21 +32,21 @@ it 'recognizes fingerprint without dashes' do ssn_without_dashes = '123456789' - create(:profile, pii: { ssn: ssn_without_dashes }) + create(:profile, :facial_match_proof, pii: { ssn: ssn_without_dashes }) expect(subject.ssn_is_unique?).to eq false end it 'recognizes fingerprint when SSN has only the first dash' do ssn_with_first_dash = '123-456789' - create(:profile, pii: { ssn: ssn_with_first_dash }) + create(:profile, :facial_match_proof, pii: { ssn: ssn_with_first_dash }) expect(subject.ssn_is_unique?).to eq false end it 'recognizes fingerprint when SSN has only the second dash' do ssn_with_second_dash = '12345-6789' - create(:profile, pii: { ssn: ssn_with_second_dash }) + create(:profile, :facial_match_proof, pii: { ssn: ssn_with_second_dash }) expect(subject.ssn_is_unique?).to eq false end @@ -49,13 +54,13 @@ context 'when ssn is already taken by same profile' do it 'is valid' do - create(:profile, pii: { ssn: ssn }, user: user) + create(:profile, :facial_match_proof, pii: { ssn: ssn }, user: user) expect(subject.ssn_is_unique?).to eq true end it 'recognizes fingerprint regardless of HMAC key age' do - create(:profile, pii: { ssn: ssn }, user: user) + create(:profile, :facial_match_proof, pii: { ssn: ssn }, user: user) rotate_hmac_key expect(subject.ssn_is_unique?).to eq true @@ -68,6 +73,12 @@ let(:user) { create(:user) } subject { described_class.new(ssn: ssn, user: user) } + + before do + allow(IdentityConfig.store).to receive(:eligible_one_account_providers) + .and_return(['urn:gov:gsa:openidconnect:inactive:sp:test']) + end + context 'when profile is IAL2' do context 'when ssn is taken by different profile by and is IAL2' do it 'returns list different profile' do @@ -101,6 +112,11 @@ let(:user) { create(:user) } subject { described_class.new(ssn: ssn, user: user) } + + before do + allow(IdentityConfig.store).to receive(:eligible_one_account_providers) + .and_return(['urn:gov:gsa:openidconnect:inactive:sp:test']) + end context 'when profile is IAL2' do context 'when ssn is taken by different profile by and is IAL2' do it 'returns false' do