diff --git a/app/controllers/users/two_factor_authentication_controller.rb b/app/controllers/users/two_factor_authentication_controller.rb index e4f285aa902..92337b8001b 100644 --- a/app/controllers/users/two_factor_authentication_controller.rb +++ b/app/controllers/users/two_factor_authentication_controller.rb @@ -45,7 +45,6 @@ def phone_configuration end def validate_otp_delivery_preference_and_send_code - delivery_preference = phone_configuration.delivery_preference result = otp_delivery_selection_form.submit(otp_delivery_preference: delivery_preference) analytics.track_event(Analytics::OTP_DELIVERY_SELECTION, result.to_h) @@ -57,6 +56,10 @@ def validate_otp_delivery_preference_and_send_code end end + def delivery_preference + phone_configuration&.delivery_preference || current_user.otp_delivery_preference + end + def update_otp_delivery_preference_if_needed OtpDeliveryPreferenceUpdater.new( user: current_user, @@ -67,8 +70,7 @@ def update_otp_delivery_preference_if_needed def handle_invalid_otp_delivery_preference(result) flash[:error] = result.errors[:phone].first - preference = current_user.phone_configuration.delivery_preference - redirect_to login_two_factor_url(otp_delivery_preference: preference) + redirect_to login_two_factor_url(otp_delivery_preference: delivery_preference) end def invalid_phone_number(exception, action:) @@ -179,7 +181,7 @@ def delivery_params end def phone_to_deliver_to - return current_user.phone_configuration.phone if authentication_context? + return phone_configuration&.phone if authentication_context? user_session[:unconfirmed_phone] end diff --git a/app/models/concerns/user_encrypted_attribute_overrides.rb b/app/models/concerns/user_encrypted_attribute_overrides.rb index ae5670e1115..4abaa71b73c 100644 --- a/app/models/concerns/user_encrypted_attribute_overrides.rb +++ b/app/models/concerns/user_encrypted_attribute_overrides.rb @@ -15,7 +15,8 @@ def find_with_email(email) email = email.downcase.strip email_fingerprint = create_fingerprint(email) - find_by(email_fingerprint: email_fingerprint) + resource = find_by(email_fingerprint: email_fingerprint) + resource if resource&.email == email end def create_fingerprint(email) diff --git a/app/models/user.rb b/app/models/user.rb index f2cca7486c9..653fcf16aa5 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -3,6 +3,7 @@ class User < ApplicationRecord self.ignored_columns = %w[ encrypted_password password_salt password_cost encryption_key recovery_code recovery_cost recovery_salt + encrypted_phone phone_confirmed_at ] include NonNullUuid @@ -22,7 +23,6 @@ class User < ApplicationRecord include EncryptableAttribute - encrypted_attribute(name: :phone) encrypted_attribute(name: :otp_secret_key) encrypted_attribute_without_setter(name: :email) diff --git a/app/services/pii/cacher.rb b/app/services/pii/cacher.rb index a16c548ec61..629a74912c9 100644 --- a/app/services/pii/cacher.rb +++ b/app/services/pii/cacher.rb @@ -38,6 +38,9 @@ def rotate_fingerprints(profile) def rotate_encrypted_attributes KeyRotator::AttributeEncryption.new(user).rotate + phone_configuration = user.phone_configuration + return if phone_configuration.blank? + KeyRotator::AttributeEncryption.new(phone_configuration).rotate end def stale_fingerprints?(profile) @@ -49,7 +52,7 @@ def stale_email_fingerprint? end def stale_attributes? - user.stale_encrypted_phone? || user.stale_encrypted_email? || + user.phone_configuration&.stale_encrypted_phone? || user.stale_encrypted_email? || user.stale_encrypted_otp_secret_key? end diff --git a/app/services/populate_phone_configurations_table.rb b/app/services/populate_phone_configurations_table.rb deleted file mode 100644 index a1c123da5ec..00000000000 --- a/app/services/populate_phone_configurations_table.rb +++ /dev/null @@ -1,41 +0,0 @@ -class PopulatePhoneConfigurationsTable - def initialize - @count = 0 - @total = 0 - end - - # :reek:DuplicateMethodCall - def call - # we don't have a uniqueness constraint in the database to let us blindly insert - # everything in a single SQL statement. So we have to load by batches and copy - # over. Much slower, but doesn't duplicate information. - User.in_batches(of: 1000) do |relation| - sleep(1) - process_batch(relation) - Rails.logger.info "#{@count} / #{@total}" - end - Rails.logger.info "Processed #{@count} user phone configurations" - end - - private - - # :reek:FeatureEnvy - def process_batch(relation) - User.transaction do - relation.each do |user| - @total += 1 - next if user.phone_configuration.present? || user.encrypted_phone.blank? - user.create_phone_configuration(phone_info_for_user(user)) - @count += 1 - end - end - end - - def phone_info_for_user(user) - { - encrypted_phone: user.encrypted_phone, - confirmed_at: user.phone_confirmed_at, - delivery_preference: user.otp_delivery_preference, - } - end -end diff --git a/app/services/update_user.rb b/app/services/update_user.rb index 1cc07776765..950de065250 100644 --- a/app/services/update_user.rb +++ b/app/services/update_user.rb @@ -5,7 +5,7 @@ def initialize(user:, attributes:) end def call - result = user.update!(attributes) + result = user.update!(attributes.except(:phone, :phone_confirmed_at)) manage_phone_configuration result end @@ -23,13 +23,7 @@ def manage_phone_configuration end def update_phone_configuration - configuration = user.phone_configuration - if phone_attributes[:phone].present? - configuration.update!(phone_attributes) - else - configuration.destroy - user.reload - end + user.phone_configuration.update!(phone_attributes) end def create_phone_configuration @@ -39,10 +33,10 @@ def create_phone_configuration def phone_attributes @phone_attributes ||= { - phone: attribute(:phone), - confirmed_at: attribute(:phone_confirmed_at), + phone: attributes[:phone], + confirmed_at: attributes[:phone_confirmed_at], delivery_preference: attribute(:otp_delivery_preference), - } + }.delete_if { |_, value| value.nil? } end # This returns the named attribute if it's included in the changes, even if diff --git a/lib/tasks/create_test_accounts.rb b/lib/tasks/create_test_accounts.rb index 91ae6df50c6..3dd22c658e3 100644 --- a/lib/tasks/create_test_accounts.rb +++ b/lib/tasks/create_test_accounts.rb @@ -15,12 +15,10 @@ def create_account(email: 'joe.smith@email.com', password: 'salty pickles', mfa_ user = User.create!(email: email) user.skip_confirmation! user.reset_password(password, password) - user.phone = mfa_phone || phone - user.phone_confirmed_at = Time.zone.now user.save! user.create_phone_configuration( phone: mfa_phone || phone, - confirmed_at: user.phone_confirmed_at, + confirmed_at: Time.zone.now, delivery_preference: user.otp_delivery_preference ) Event.create(user_id: user.id, event_type: :account_created) diff --git a/lib/tasks/dev.rake b/lib/tasks/dev.rake index a1c212a71f2..bff3595d76c 100644 --- a/lib/tasks/dev.rake +++ b/lib/tasks/dev.rake @@ -88,9 +88,11 @@ namespace :dev do user.encrypted_email = args[:ee].encrypted user.skip_confirmation! user.reset_password(args[:pw], args[:pw]) - user.phone = format('+1 (415) 555-%04d', args[:num]) - user.phone_confirmed_at = Time.zone.now - create_phone_configuration_for(user) + user.create_phone_configuration( + delivery_preference: user.otp_delivery_preference, + phone: format('+1 (415) 555-%04d', args[:num]), + confirmed_at: Time.zone.now + ) Event.create(user_id: user.id, event_type: :account_created) end @@ -105,12 +107,4 @@ namespace :dev do def fingerprint(email) Pii::Fingerprinter.fingerprint(email) end - - def create_phone_configuration_for(user) - user.create_phone_configuration( - phone: user.phone, - confirmed_at: user.phone_confirmed_at, - delivery_preference: user.otp_delivery_preference - ) - end end diff --git a/lib/tasks/migrate_phone_configurations.rake b/lib/tasks/migrate_phone_configurations.rake deleted file mode 100644 index f857e0e96f2..00000000000 --- a/lib/tasks/migrate_phone_configurations.rake +++ /dev/null @@ -1,7 +0,0 @@ -namespace :adhoc do - desc 'Copy phone configurations to the new table' - task populate_phone_configurations: :environment do - Rails.logger = Logger.new(STDOUT) - PopulatePhoneConfigurationsTable.new.call - end -end diff --git a/spec/controllers/account_recovery_setup_controller_spec.rb b/spec/controllers/account_recovery_setup_controller_spec.rb index 4c77947abf2..2064124b04b 100644 --- a/spec/controllers/account_recovery_setup_controller_spec.rb +++ b/spec/controllers/account_recovery_setup_controller_spec.rb @@ -24,7 +24,7 @@ context 'user is piv_cac enabled but not phone enabled' do it 'redirects to account_url' do - user = build(:user, :signed_up, :with_piv_or_cac, phone: nil) + user = build(:user, :signed_up, :with_piv_or_cac, with: { mfa_enabled: false }) stub_sign_in(user) get :index diff --git a/spec/controllers/idv/phone_controller_spec.rb b/spec/controllers/idv/phone_controller_spec.rb index c7ded6be30b..674c40d0ba0 100644 --- a/spec/controllers/idv/phone_controller_spec.rb +++ b/spec/controllers/idv/phone_controller_spec.rb @@ -20,7 +20,10 @@ end describe '#new' do - let(:user) { build(:user, phone: good_phone, phone_confirmed_at: Time.zone.now) } + let(:user) do + build(:user, :with_phone, + with: { phone: good_phone, confirmed_at: Time.zone.now }) + end before do stub_verify_steps_one_and_two(user) @@ -64,7 +67,7 @@ describe '#create' do context 'when form is invalid' do before do - user = build(:user, phone: '+1 (415) 555-0130') + user = build(:user, :with_phone, with: { phone: '+1 (415) 555-0130' }) stub_verify_steps_one_and_two(user) stub_analytics allow(@analytics).to receive(:track_event) @@ -105,7 +108,7 @@ end it 'tracks event with valid phone' do - user = build(:user, phone: good_phone, phone_confirmed_at: Time.zone.now) + user = build(:user, :with_phone, with: { phone: good_phone, confirmed_at: Time.zone.now }) stub_verify_steps_one_and_two(user) put :create, params: { idv_phone_form: { phone: good_phone } } @@ -124,7 +127,9 @@ context 'when same as user phone' do it 'redirects to result page and sets phone_confirmed_at' do - user = build(:user, phone: good_phone, phone_confirmed_at: Time.zone.now) + user = build(:user, :with_phone, with: { + phone: good_phone, confirmed_at: Time.zone.now + }) stub_verify_steps_one_and_two(user) put :create, params: { idv_phone_form: { phone: good_phone } } @@ -141,7 +146,9 @@ context 'when different from user phone' do it 'redirects to otp page and does not set phone_confirmed_at' do - user = build(:user, phone: '+1 (415) 555-0130', phone_confirmed_at: Time.zone.now) + user = build(:user, :with_phone, with: { + phone: '+1 (415) 555-0130', confirmed_at: Time.zone.now + }) stub_verify_steps_one_and_two(user) put :create, params: { idv_phone_form: { phone: good_phone } } @@ -159,7 +166,9 @@ end describe '#show' do - let(:user) { build(:user, phone: good_phone, phone_confirmed_at: Time.zone.now) } + let(:user) do + build(:user, :with_phone, with: { phone: good_phone, confirmed_at: Time.zone.now }) + end let(:params) { { phone: good_phone } } before do @@ -232,7 +241,9 @@ end let(:params) { { phone: bad_phone } } - let(:user) { build(:user, phone: bad_phone, phone_confirmed_at: Time.zone.now) } + let(:user) do + build(:user, :with_phone, with: { phone: bad_phone, confirmed_at: Time.zone.now }) + end it 'tracks event with invalid phone' do stub_analytics @@ -257,7 +268,9 @@ context 'attempt window has expired, previous attempts == max-1' do let(:two_days_ago) { Time.zone.now - 2.days } - let(:user) { build(:user, phone: good_phone, phone_confirmed_at: Time.zone.now) } + let(:user) do + build(:user, :with_phone, with: { phone: good_phone, confirmed_at: Time.zone.now }) + end before do user.idv_attempts = max_attempts - 1 @@ -275,7 +288,8 @@ end it 'passes the normalized phone to the background job' do - user = build(:user, phone: good_phone, phone_confirmed_at: Time.zone.now) + user = build(:user, :with_phone, with: { phone: good_phone, confirmed_at: Time.zone.now }) + stub_verify_steps_one_and_two(user) subject.params = { phone: normalized_phone } diff --git a/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb b/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb index 6dfd9445a2c..d6f131a0f1d 100644 --- a/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb @@ -33,7 +33,7 @@ end it 'tracks the page visit and context' do - user = build_stubbed(:user, phone: '+1 (703) 555-0100') + user = build_stubbed(:user, :with_phone, with: { phone: '+1 (703) 555-0100' }) stub_sign_in_before_2fa(user) stub_analytics @@ -322,8 +322,6 @@ end it 'does not update user phone or phone_confirmed_at attributes' do - expect(subject.current_user.phone).to eq('+1 202-555-1212') - expect(subject.current_user.phone_confirmed_at).to eq(@previous_phone_confirmed_at) expect(subject.current_user.phone_configuration.phone).to eq('+1 202-555-1212') expect( subject.current_user.phone_configuration.confirmed_at @@ -355,8 +353,6 @@ context 'when user does not have an existing phone number' do before do - subject.current_user.phone = nil - subject.current_user.phone_confirmed_at = nil subject.current_user.phone_configuration.destroy subject.current_user.phone_configuration = nil subject.current_user.create_direct_otp diff --git a/spec/controllers/two_factor_authentication/personal_key_verification_controller_spec.rb b/spec/controllers/two_factor_authentication/personal_key_verification_controller_spec.rb index 9d1de9b92c0..84ee076916e 100644 --- a/spec/controllers/two_factor_authentication/personal_key_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/personal_key_verification_controller_spec.rb @@ -80,7 +80,7 @@ let(:payload) { { personal_key_form: personal_key } } before do - stub_sign_in_before_2fa(build(:user, phone: '+1 (703) 555-1212')) + stub_sign_in_before_2fa(build(:user, :with_phone, with: { phone: '+1 (703) 555-1212' })) form = instance_double(PersonalKeyForm) response = FormResponse.new( success: false, errors: {}, extra: { multi_factor_auth_method: 'personal key' } @@ -100,7 +100,7 @@ context 'when the user enters an invalid personal key' do before do - stub_sign_in_before_2fa(build(:user, phone: '+1 (703) 555-1212')) + stub_sign_in_before_2fa(build(:user, :with_phone, with: { phone: '+1 (703) 555-1212' })) form = instance_double(PersonalKeyForm) response = FormResponse.new( success: false, errors: {}, extra: { multi_factor_auth_method: 'personal key' } diff --git a/spec/controllers/two_factor_authentication/piv_cac_verification_controller_spec.rb b/spec/controllers/two_factor_authentication/piv_cac_verification_controller_spec.rb index bc0f98abbfc..3f7dbc95674 100644 --- a/spec/controllers/two_factor_authentication/piv_cac_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/piv_cac_verification_controller_spec.rb @@ -3,7 +3,7 @@ describe TwoFactorAuthentication::PivCacVerificationController do let(:user) do create(:user, :signed_up, :with_piv_or_cac, - phone: '+1 (703) 555-0000') + with: { phone: '+1 (703) 555-0000' }) end let(:nonce) { 'once' } diff --git a/spec/controllers/users/phones_controller_spec.rb b/spec/controllers/users/phones_controller_spec.rb index 16dc088fd7a..de01fb134b4 100644 --- a/spec/controllers/users/phones_controller_spec.rb +++ b/spec/controllers/users/phones_controller_spec.rb @@ -5,8 +5,8 @@ include Features::LocalizationHelper describe '#phone' do - let(:user) { create(:user, :signed_up, phone: '+1 (202) 555-1234') } - let(:second_user) { create(:user, :signed_up, phone: '+1 (202) 555-5678') } + let(:user) { create(:user, :signed_up, with: { phone: '+1 (202) 555-1234' }) } + let(:second_user) { create(:user, :signed_up, with: { phone: '+1 (202) 555-5678' }) } let(:new_phone) { '202-555-4321' } context 'user changes phone' do @@ -25,7 +25,6 @@ it 'lets user know they need to confirm their new phone' do expect(flash[:notice]).to eq t('devise.registrations.phone_update_needs_confirmation') - expect(user.reload.phone).to_not eq '+1 202-555-4321' expect(user.reload.phone_configuration.phone).to_not eq '+1 202-555-4321' expect(@analytics).to have_received(:track_event). with(Analytics::PHONE_CHANGE_REQUESTED) @@ -48,7 +47,6 @@ otp_delivery_preference: 'sms' }, } - expect(user.reload.phone).to be_present expect(user.reload.phone_configuration.phone).to be_present expect(response).to render_template(:edit) end @@ -62,7 +60,7 @@ allow(@analytics).to receive(:track_event) put :update, params: { - user_phone_form: { phone: second_user.phone, + user_phone_form: { phone: second_user.phone_configuration.phone, international_code: 'US', otp_delivery_preference: 'sms' }, } @@ -70,8 +68,9 @@ it 'processes successfully and informs user' do expect(flash[:notice]).to eq t('devise.registrations.phone_update_needs_confirmation') - expect(user.reload.phone).to_not eq second_user.phone - expect(user.reload.phone_configuration.phone).to_not eq second_user.phone + expect(user.phone_configuration.reload.phone).to_not eq( + second_user.phone_configuration.phone + ) expect(@analytics).to have_received(:track_event). with(Analytics::PHONE_CHANGE_REQUESTED) expect(response).to redirect_to( @@ -86,7 +85,7 @@ context 'user updates with invalid phone' do it 'does not change the user phone number' do invalid_phone = '123' - user = build(:user, phone: '123-123-1234') + user = build(:user, :with_phone, with: { phone: '123-123-1234' }) stub_sign_in(user) put :update, params: { @@ -95,7 +94,6 @@ otp_delivery_preference: 'sms' }, } - expect(user.phone).not_to eq invalid_phone expect(user.phone_configuration.phone).not_to eq invalid_phone expect(response).to render_template(:edit) end @@ -106,7 +104,7 @@ stub_sign_in(user) put :update, params: { - user_phone_form: { phone: user.phone, + user_phone_form: { phone: user.phone_configuration.phone, international_code: 'US', otp_delivery_preference: 'sms' }, } diff --git a/spec/controllers/users/piv_cac_authentication_setup_controller_spec.rb b/spec/controllers/users/piv_cac_authentication_setup_controller_spec.rb index bd282961ad3..ab7ef139bfe 100644 --- a/spec/controllers/users/piv_cac_authentication_setup_controller_spec.rb +++ b/spec/controllers/users/piv_cac_authentication_setup_controller_spec.rb @@ -32,7 +32,7 @@ describe 'when signing in' do before(:each) { stub_sign_in_before_2fa(user) } let(:user) do - create(:user, :signed_up, :with_piv_or_cac, phone: '+1 (703) 555-0000') + create(:user, :signed_up, :with_piv_or_cac, with: { phone: '+1 (703) 555-0000' }) end describe 'GET index' do @@ -55,7 +55,7 @@ context 'without associated piv/cac' do let(:user) do - create(:user, :signed_up, phone: '+1 (703) 555-0000') + create(:user, :signed_up, with: { phone: '+1 (703) 555-0000' }) end before(:each) do diff --git a/spec/controllers/users/totp_setup_controller_spec.rb b/spec/controllers/users/totp_setup_controller_spec.rb index d564f335671..3414c76b037 100644 --- a/spec/controllers/users/totp_setup_controller_spec.rb +++ b/spec/controllers/users/totp_setup_controller_spec.rb @@ -15,7 +15,7 @@ context 'user is setting up authenticator app after account creation' do before do stub_analytics - user = build(:user, phone: '703-555-1212') + user = build(:user, :with_phone, with: { phone: '703-555-1212' }) stub_sign_in(user) allow(@analytics).to receive(:track_event) get :new diff --git a/spec/controllers/users/two_factor_authentication_controller_spec.rb b/spec/controllers/users/two_factor_authentication_controller_spec.rb index 522182857bb..3fcb40b8a8b 100644 --- a/spec/controllers/users/two_factor_authentication_controller_spec.rb +++ b/spec/controllers/users/two_factor_authentication_controller_spec.rb @@ -101,7 +101,7 @@ def index context 'when the user has already set up 2FA' do it 'sends OTP via otp_delivery_preference and prompts for OTP' do - stub_sign_in_before_2fa(build(:user, phone: '+1 (703) 555-1212')) + stub_sign_in_before_2fa(build(:user, :with_phone, with: { phone: '+1 (703) 555-1212' })) get :show diff --git a/spec/factories/phone_configurations.rb b/spec/factories/phone_configurations.rb index 9c64b38e7f4..cca856f2000 100644 --- a/spec/factories/phone_configurations.rb +++ b/spec/factories/phone_configurations.rb @@ -2,8 +2,9 @@ Faker::Config.locale = :en factory :phone_configuration do - confirmed_at Time.zone.now - phone '+1 202-555-1212' - user + confirmed_at { Time.zone.now } + phone { '+1 202-555-1212' } + mfa_enabled { true } + association :user end end diff --git a/spec/factories/users.rb b/spec/factories/users.rb index 0bc4c575dbd..783fa67ed25 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -2,33 +2,46 @@ Faker::Config.locale = :en factory :user do + transient do + with { {} } + end + confirmed_at Time.zone.now email { Faker::Internet.safe_email } password '!1a Z@6s' * 16 # Maximum length password. - after :build do |user| - if user.phone - user.build_phone_configuration( - phone: user.phone, - confirmed_at: user.phone_confirmed_at, - delivery_preference: user.otp_delivery_preference - ) + trait :with_phone do + after(:build) do |user, evaluator| + if user.phone_configuration.nil? + user.phone_configuration = build( + :phone_configuration, + { user: user, delivery_preference: user.otp_delivery_preference }.merge( + evaluator.with.slice(:phone, :confirmed_at, :delivery_preference, :mfa_enabled) + ) + ) + end end - end - after :stub do |user| - if user.phone - user.phone_configuration = build_stubbed(:phone_configuration, - user: user, - phone: user.phone, - confirmed_at: user.phone_confirmed_at, - delivery_preference: user.otp_delivery_preference) + after(:create) do |user, evaluator| + if user.phone_configuration.nil? + create(:phone_configuration, + { user: user, delivery_preference: user.otp_delivery_preference }.merge( + evaluator.with.slice(:phone, :confirmed_at, :delivery_preference, :mfa_enabled) + )) + user.reload + end end - end - trait :with_phone do - phone '+1 202-555-1212' - phone_confirmed_at Time.zone.now + after(:stub) do |user, evaluator| + if user.phone_configuration.nil? + user.phone_configuration = build_stubbed( + :phone_configuration, + { user: user, delivery_preference: user.otp_delivery_preference }.merge( + evaluator.with.slice(:phone, :confirmed_at, :delivery_preference, :mfa_enabled) + ) + ) + end + end end trait :with_piv_or_cac do diff --git a/spec/features/accessibility/user_pages_spec.rb b/spec/features/accessibility/user_pages_spec.rb index 9db6fc614b6..fb2d88d423c 100644 --- a/spec/features/accessibility/user_pages_spec.rb +++ b/spec/features/accessibility/user_pages_spec.rb @@ -53,7 +53,7 @@ describe 'SMS' do scenario 'enter 2fa phone OTP code page' do - user = create(:user, phone: '+1 (202) 555-1212') + user = create(:user, :with_phone, with: { phone: '+1 (202) 555-1212' }) sign_in_before_2fa(user) visit login_two_factor_path(otp_delivery_preference: 'sms') @@ -64,7 +64,7 @@ describe 'Voice' do scenario 'enter 2fa phone OTP code page' do - user = create(:user, phone: '+1 (202) 555-1212') + user = create(:user, :with_phone, with: { phone: '+1 (202) 555-1212' }) sign_in_before_2fa(user) visit login_two_factor_path(otp_delivery_preference: 'voice') diff --git a/spec/features/sign_in/two_factor_options_spec.rb b/spec/features/sign_in/two_factor_options_spec.rb index c1b49981453..38828d07ca5 100644 --- a/spec/features/sign_in/two_factor_options_spec.rb +++ b/spec/features/sign_in/two_factor_options_spec.rb @@ -43,7 +43,8 @@ context 'when the user only has SMS configured with a number that we cannot call' do it 'only displays SMS and Personal key' do - user = create(:user, :signed_up, otp_delivery_preference: 'sms', phone: '+12423270143') + user = create(:user, :signed_up, + otp_delivery_preference: 'sms', with: { phone: '+12423270143' }) sign_in_user(user) click_link t('two_factor_authentication.login_options_link_text') @@ -63,7 +64,8 @@ context "the user's otp_delivery_preference is voice but number is unsupported" do it 'only displays SMS and Personal key' do - user = create(:user, :signed_up, otp_delivery_preference: 'voice', phone: '+12423270143') + user = create(:user, :signed_up, + otp_delivery_preference: 'voice', with: { phone: '+12423270143' }) sign_in_user(user) click_link t('two_factor_authentication.login_options_link_text') diff --git a/spec/features/two_factor_authentication/change_factor_spec.rb b/spec/features/two_factor_authentication/change_factor_spec.rb index 06a53905f73..1de6a318652 100644 --- a/spec/features/two_factor_authentication/change_factor_spec.rb +++ b/spec/features/two_factor_authentication/change_factor_spec.rb @@ -20,7 +20,7 @@ mailer = instance_double(ActionMailer::MessageDelivery, deliver_later: true) allow(UserMailer).to receive(:phone_changed).with(user).and_return(mailer) - @previous_phone_confirmed_at = user.reload.phone_confirmed_at + @previous_phone_confirmed_at = user.phone_configuration.reload.confirmed_at new_phone = '+1 703-555-0100' visit manage_phone_path @@ -39,8 +39,7 @@ enter_incorrect_otp_code expect(page).to have_content t('devise.two_factor_authentication.invalid_otp') - expect(user.reload.phone).to_not eq new_phone - expect(user.reload.phone_configuration.phone).to_not eq new_phone + expect(user.phone_configuration.reload.phone).to_not eq new_phone expect(page).to have_link t('forms.two_factor.try_again'), href: manage_phone_path submit_correct_otp @@ -49,9 +48,8 @@ expect(UserMailer).to have_received(:phone_changed).with(user) expect(mailer).to have_received(:deliver_later) expect(page).to have_content new_phone - expect(user.reload.phone_confirmed_at).to_not eq(@previous_phone_confirmed_at) expect( - user.reload.phone_configuration.confirmed_at + user.phone_configuration.reload.confirmed_at ).to_not eq(@previous_phone_confirmed_at) visit login_two_factor_path(otp_delivery_preference: 'sms') @@ -188,7 +186,7 @@ PhoneVerification.adapter = FakeAdapter allow(FakeAdapter).to receive(:post).and_return(FakeAdapter::ErrorResponse.new) - user = create(:user, :signed_up, phone: '+17035551212') + user = create(:user, :signed_up, with: { phone: '+17035551212' }) visit new_user_session_path sign_in_live_with_2fa(user) visit manage_phone_path diff --git a/spec/features/two_factor_authentication/sign_in_spec.rb b/spec/features/two_factor_authentication/sign_in_spec.rb index a6eaaa5b6a8..885a4ed263e 100644 --- a/spec/features/two_factor_authentication/sign_in_spec.rb +++ b/spec/features/two_factor_authentication/sign_in_spec.rb @@ -34,7 +34,6 @@ expect(page).to_not have_content invalid_phone_message expect(current_path).to eq login_two_factor_path(otp_delivery_preference: 'sms') - expect(user.reload.phone).to_not eq '+1 (703) 555-1212' expect(user.reload.phone_configuration).to be_nil expect(user.sms?).to eq true end @@ -242,7 +241,7 @@ def submit_prefilled_otp_code scenario 'the user cannot change delivery method if phone is unsupported' do unsupported_phone = '+1 (242) 327-0143' - user = create(:user, :signed_up, phone: unsupported_phone) + user = create(:user, :signed_up, with: { phone: unsupported_phone }) sign_in_before_2fa(user) expect(page).to_not have_link t('links.two_factor_authentication.voice') @@ -374,8 +373,8 @@ def submit_prefilled_otp_code context '2 users with same phone number request OTP too many times within findtime' do it 'locks both users out' do allow(Figaro.env).to receive(:otp_delivery_blocklist_maxretry).and_return('3') - first_user = create(:user, :signed_up, phone: '+1 703-555-1212') - second_user = create(:user, :signed_up, phone: '+1 703-555-1212') + first_user = create(:user, :signed_up, with: { phone: '+1 703-555-1212' }) + second_user = create(:user, :signed_up, with: { phone: '+1 703-555-1212' }) max_attempts = Figaro.env.otp_delivery_blocklist_maxretry.to_i sign_in_before_2fa(first_user) @@ -410,7 +409,7 @@ def submit_prefilled_otp_code context 'When setting up 2FA for the first time' do it 'enforces rate limiting only for current phone' do - second_user = create(:user, :signed_up, phone: '202-555-1212') + second_user = create(:user, :signed_up, with: { phone: '202-555-1212' }) sign_in_before_2fa max_attempts = Figaro.env.otp_delivery_blocklist_maxretry.to_i @@ -552,7 +551,7 @@ def submit_prefilled_otp_code PhoneVerification.adapter = FakeAdapter allow(SmsOtpSenderJob).to receive(:perform_later) - user = create(:user, :signed_up, phone: '+212 661-289324') + user = create(:user, :signed_up, with: { phone: '+212 661-289324' }) sign_in_user(user) expect(SmsOtpSenderJob).to have_received(:perform_later).with( @@ -574,7 +573,7 @@ def submit_prefilled_otp_code PhoneVerification.adapter = FakeAdapter allow(FakeAdapter).to receive(:post).and_return(FakeAdapter::ErrorResponse.new) - user = create(:user, :signed_up, phone: '+212 661-289324') + user = create(:user, :signed_up, with: { phone: '+212 661-289324' }) sign_in_user(user) expect(current_path).to eq login_two_factor_path(otp_delivery_preference: 'sms') @@ -593,7 +592,9 @@ def submit_prefilled_otp_code PhoneVerification.adapter = FakeAdapter allow(FakeAdapter).to receive(:post).and_return(FakeAdapter::ErrorResponse.new) - user = create(:user, :signed_up, phone: '+17035551212', otp_delivery_preference: 'voice') + user = create(:user, :signed_up, + otp_delivery_preference: 'voice', + with: { phone: '+17035551212', delivery_preference: 'voice' }) sign_in_user(user) expect(current_path).to eq login_two_factor_path(otp_delivery_preference: 'voice') diff --git a/spec/features/users/piv_cac_management_spec.rb b/spec/features/users/piv_cac_management_spec.rb index 1d39546ab11..941add76a96 100644 --- a/spec/features/users/piv_cac_management_spec.rb +++ b/spec/features/users/piv_cac_management_spec.rb @@ -13,7 +13,7 @@ def find_form(page, attributes) context 'with no piv/cac associated yet' do let(:uuid) { SecureRandom.uuid } - let(:user) { create(:user, :signed_up, phone: '+1 202-555-1212') } + let(:user) { create(:user, :signed_up, :with_phone, with: { phone: '+1 202-555-1212' }) } context 'with a service provider allowed to use piv/cac' do let(:identity_with_sp) do @@ -106,9 +106,10 @@ def find_form(page, attributes) allow(FeatureManagement).to receive(:prefill_otp_codes?).and_return(true) stub_piv_cac_service - user.update(phone: nil, otp_secret_key: 'secret') + user.update(otp_secret_key: 'secret') user.phone_configuration.destroy - user.phone_configuration = nil + user.reload + expect(user.phone_configuration).to be_nil sign_in_and_2fa_user(user) visit account_path click_link t('forms.buttons.enable'), href: setup_piv_cac_url @@ -156,7 +157,7 @@ def find_form(page, attributes) scenario "doesn't allow unassociation of a piv/cac" do stub_piv_cac_service - user = create(:user, :signed_up, phone: '+1 202-555-1212') + user = create(:user, :signed_up, :with_phone, with: { phone: '+1 202-555-1212' }) sign_in_and_2fa_user(user) visit account_path form = find_form(page, action: disable_piv_cac_url) @@ -167,7 +168,7 @@ def find_form(page, attributes) context 'with a piv/cac associated and no identities allowing piv/cac' do let(:user) do - create(:user, :signed_up, :with_piv_or_cac, phone: '+1 202-555-1212') + create(:user, :signed_up, :with_piv_or_cac, :with_phone, with: { phone: '+1 202-555-1212' }) end scenario "doesn't allow association of another piv/cac with the account" do diff --git a/spec/features/users/sign_in_spec.rb b/spec/features/users/sign_in_spec.rb index 04f72777622..3e27ae5d831 100644 --- a/spec/features/users/sign_in_spec.rb +++ b/spec/features/users/sign_in_spec.rb @@ -255,19 +255,20 @@ expect { signin(email, password) }. to raise_error Encryption::EncryptionError, 'unable to decrypt attribute with any key' - user = User.find_with_email(email) + user = user.reload expect(user.encrypted_email).to eq encrypted_email end end context 'KMS is on and user enters incorrect password' do it 'redirects to root_path with user-friendly error message, not a 500 error' do + user = create(:user) + email = user.email allow(FeatureManagement).to receive(:use_kms?).and_return(true) stub_aws_kms_client_invalid_ciphertext allow(SessionEncryptorErrorHandler).to receive(:call) - user = create(:user) - signin(user.email, 'invalid') + signin(email, 'invalid') link_url = new_user_password_url @@ -320,7 +321,8 @@ it 'falls back to SMS with an error message' do allow(SmsOtpSenderJob).to receive(:perform_later) allow(VoiceOtpSenderJob).to receive(:perform_later) - user = create(:user, :signed_up, phone: '+1 441-295-9644', otp_delivery_preference: 'voice') + user = create(:user, :signed_up, + otp_delivery_preference: 'voice', with: { phone: '+1 441-295-9644' }) signin(user.email, user.password) expect(VoiceOtpSenderJob).to_not have_received(:perform_later) @@ -339,7 +341,8 @@ it 'displays an error message but does not send an SMS' do allow(SmsOtpSenderJob).to receive(:perform_later) allow(VoiceOtpSenderJob).to receive(:perform_later) - user = create(:user, :signed_up, phone: '+91 1234567890', otp_delivery_preference: 'sms') + user = create(:user, :signed_up, + otp_delivery_preference: 'sms', with: { phone: '+91 1234567890' }) signin(user.email, user.password) visit login_two_factor_path(otp_delivery_preference: 'voice', reauthn: false) @@ -359,7 +362,8 @@ it 'displays an error message but does not send an SMS' do allow(SmsOtpSenderJob).to receive(:perform_later) allow(VoiceOtpSenderJob).to receive(:perform_later) - user = create(:user, :signed_up, phone: '+91 1234567890', otp_delivery_preference: 'sms') + user = create(:user, :signed_up, + otp_delivery_preference: 'sms', with: { phone: '+91 1234567890' }) signin(user.email, user.password) visit otp_send_path( otp_delivery_selection_form: { otp_delivery_preference: 'voice', resend: true } @@ -381,7 +385,8 @@ it 'displays an error message but does not send an SMS' do allow(SmsOtpSenderJob).to receive(:perform_later) allow(VoiceOtpSenderJob).to receive(:perform_later) - user = create(:user, :signed_up, phone: '+91 1234567890', otp_delivery_preference: 'voice') + user = create(:user, :signed_up, + otp_delivery_preference: 'voice', with: { phone: '+91 1234567890' }) signin(user.email, user.password) visit otp_send_path( otp_delivery_selection_form: { otp_delivery_preference: 'voice', resend: true } diff --git a/spec/features/visitors/phone_confirmation_spec.rb b/spec/features/visitors/phone_confirmation_spec.rb index de472a7d4c0..f47f577aa3c 100644 --- a/spec/features/visitors/phone_confirmation_spec.rb +++ b/spec/features/visitors/phone_confirmation_spec.rb @@ -22,7 +22,6 @@ it 'updates phone_confirmed_at and redirects to acknowledge personal key' do click_button t('forms.buttons.submit.default') - expect(@user.reload.phone_confirmed_at).to be_present expect(@user.reload.phone_configuration.confirmed_at).to be_present expect(current_path).to eq sign_up_personal_key_path @@ -76,7 +75,6 @@ fill_in 'code', with: 'foobar' click_submit_default - expect(@user.reload.phone_confirmed_at).to be_nil expect(@user.reload.phone_configuration).to be_nil expect(page).to have_content t('devise.two_factor_authentication.invalid_otp') expect(current_path).to eq login_two_factor_path(otp_delivery_preference: 'sms') diff --git a/spec/forms/idv/phone_form_spec.rb b/spec/forms/idv/phone_form_spec.rb index 00412d34256..27997f499f1 100644 --- a/spec/forms/idv/phone_form_spec.rb +++ b/spec/forms/idv/phone_form_spec.rb @@ -52,14 +52,14 @@ end it 'uses the user phone number as the initial phone value' do - user = build_stubbed(:user, :signed_up, phone: '7035551234') + user = build_stubbed(:user, :signed_up, with: { phone: '7035551234' }) subject = Idv::PhoneForm.new({}, user) expect(subject.phone).to eq('+1 703-555-1234') end it 'does not use an international number as the initial phone value' do - user = build_stubbed(:user, :signed_up, phone: '+81 54 354 3643') + user = build_stubbed(:user, :signed_up, with: { phone: '+81 54 354 3643' }) subject = Idv::PhoneForm.new({}, user) expect(subject.phone).to eq(nil) diff --git a/spec/forms/user_phone_form_spec.rb b/spec/forms/user_phone_form_spec.rb index 157ee5b7dc1..e3cbbebae82 100644 --- a/spec/forms/user_phone_form_spec.rb +++ b/spec/forms/user_phone_form_spec.rb @@ -17,8 +17,8 @@ it 'loads initial values from the user object' do user = build_stubbed( - :user, - phone: '+1 (703) 500-5000', + :user, :with_phone, + with: { phone: '+1 (703) 500-5000' }, otp_delivery_preference: 'voice' ) subject = UserPhoneForm.new(user) @@ -29,7 +29,7 @@ end it 'infers the international code from the user phone number' do - user = build_stubbed(:user, phone: '+81 744 21 1234') + user = build_stubbed(:user, :with_phone, with: { phone: '+81 744 21 1234' }) subject = UserPhoneForm.new(user) expect(subject.international_code).to eq('JP') @@ -78,7 +78,6 @@ subject.submit(params) user.reload - expect(user.phone).to_not eq('+1 504 444 1643') expect(user.phone_configuration).to be_nil end @@ -221,7 +220,6 @@ context 'when a user has no phone' do it 'returns true' do user.phone_configuration.destroy - user.update!(phone: nil) user.reload params[:phone] = '+1 504 444 1643' diff --git a/spec/lib/tasks/rotate_rake_spec.rb b/spec/lib/tasks/rotate_rake_spec.rb index 505df392fe6..99e74498dd1 100644 --- a/spec/lib/tasks/rotate_rake_spec.rb +++ b/spec/lib/tasks/rotate_rake_spec.rb @@ -2,7 +2,7 @@ require 'rake' describe 'rotate' do - let(:user) { create(:user, phone: '703-555-5555') } + let(:user) { create(:user, :with_phone, with: { phone: '703-555-5555' }) } before do Rake.application.rake_require('lib/tasks/rotate', [Rails.root.to_s]) Rake::Task.define_task(:environment) @@ -15,10 +15,9 @@ describe 'attribute_encryption_key' do it 'runs successfully' do old_email = user.email - old_phone = user.phone + old_phone = user.phone_configuration.phone old_encrypted_email = user.encrypted_email - old_encrypted_phone = user.encrypted_phone - old_encrypted_configuration_phone = user.phone_configuration.encrypted_phone + old_encrypted_phone = user.phone_configuration.encrypted_phone rotate_attribute_encryption_key @@ -26,15 +25,10 @@ user.reload user.phone_configuration.reload - expect(user.phone).to eq old_phone expect(user.phone_configuration.phone).to eq old_phone expect(user.email).to eq old_email expect(user.encrypted_email).to_not eq old_encrypted_email - expect(user.encrypted_phone).to_not eq old_encrypted_phone - expect(user.phone_configuration.encrypted_phone).to_not eq old_encrypted_configuration_phone - expect(user.phone_configuration.phone).to eq user.phone - # this double checks that we're not using the same IV for both - expect(user.phone_configuration.encrypted_phone).to_not eq user.encrypted_phone + expect(user.phone_configuration.encrypted_phone).to_not eq old_encrypted_phone end it 'does not raise an exception when encrypting/decrypting a user' do diff --git a/spec/models/phone_configuration_spec.rb b/spec/models/phone_configuration_spec.rb index 4bef0f7e17f..32b37b31acd 100644 --- a/spec/models/phone_configuration_spec.rb +++ b/spec/models/phone_configuration_spec.rb @@ -12,8 +12,23 @@ let(:phone_configuration) { create(:phone_configuration, phone: phone) } describe 'creation' do - it 'stores an encrypted form of the password' do + it 'stores an encrypted form of the phone number' do expect(phone_configuration.encrypted_phone).to_not be_blank end end + + describe 'encrypted attributes' do + it 'decrypts phone' do + expect(phone_configuration.phone).to eq phone + end + + context 'with unnormalized phone' do + let(:phone) { ' 555 555 5555 ' } + let(:normalized_phone) { '555 555 5555' } + + it 'normalizes phone' do + expect(phone_configuration.phone).to eq normalized_phone + end + end + end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 7adf3b2b53b..c767d7fea98 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -376,20 +376,11 @@ expect(user.email).to eq 'foo@example.org' end - - it 'normalizes phone' do - user = create(:user, phone: ' 555 555 5555 ') - - expect(user.phone).to eq '555 555 5555' - expect(user.phone_configuration.phone).to eq '555 555 5555' - end end - it 'decrypts phone and otp_secret_key' do - user = create(:user, phone: '+1 (202) 555-1212', otp_secret_key: 'abc123') + it 'decrypts otp_secret_key' do + user = create(:user, otp_secret_key: 'abc123') - expect(user.phone).to eq '+1 (202) 555-1212' - expect(user.phone_configuration.phone).to eq '+1 (202) 555-1212' expect(user.otp_secret_key).to eq 'abc123' end end diff --git a/spec/requests/edit_user_spec.rb b/spec/requests/edit_user_spec.rb index 4a015d25343..057c8ed7894 100644 --- a/spec/requests/edit_user_spec.rb +++ b/spec/requests/edit_user_spec.rb @@ -4,7 +4,7 @@ include Features::MailerHelper include Features::ActiveJobHelper - let(:user) { create(:user, :signed_up, phone: '+1 (202) 555-1213') } + let(:user) { create(:user, :signed_up, with: { phone: '+1 (202) 555-1213' }) } def user_session session['warden.user.user.session'] diff --git a/spec/requests/openid_connect_authorize_spec.rb b/spec/requests/openid_connect_authorize_spec.rb index 8b5dcb1e1a0..efd42de8c34 100644 --- a/spec/requests/openid_connect_authorize_spec.rb +++ b/spec/requests/openid_connect_authorize_spec.rb @@ -1,7 +1,7 @@ require 'rails_helper' describe 'user signs in partially and visits openid_connect/authorize' do - let(:user) { create(:user, :signed_up, phone: '+1 (202) 555-1213') } + let(:user) { create(:user, :signed_up, with: { phone: '+1 (202) 555-1213' }) } it 'prompts the user to 2FA' do openid_test('select_account') diff --git a/spec/services/account_reset/cancel_spec.rb b/spec/services/account_reset/cancel_spec.rb index 377acedfc91..5fe0e876281 100644 --- a/spec/services/account_reset/cancel_spec.rb +++ b/spec/services/account_reset/cancel_spec.rb @@ -21,6 +21,10 @@ context 'when the token is valid' do context 'when the user has a phone enabled for SMS' do + before(:each) do + user.phone_configuration.update!(delivery_preference: :sms) + end + it 'notifies the user via SMS of the account reset cancellation' do token = create_account_reset_request_for(user) allow(SmsAccountResetCancellationNotifierJob).to receive(:perform_now) @@ -36,7 +40,6 @@ it 'does not notify the user via SMS' do token = create_account_reset_request_for(user) allow(SmsAccountResetCancellationNotifierJob).to receive(:perform_now) - user.update!(phone: nil) user.phone_configuration.destroy! user.reload @@ -84,7 +87,7 @@ context 'when the user does not have a phone enabled for SMS' do it 'does not notify the user via SMS' do allow(SmsAccountResetCancellationNotifierJob).to receive(:perform_now) - user.update!(phone: nil) + user.phone_configuration.update!(mfa_enabled: false) AccountReset::Cancel.new('foo').call diff --git a/spec/services/key_rotator/attribute_encryption_spec.rb b/spec/services/key_rotator/attribute_encryption_spec.rb index 7e1e49c8a12..a863b46bf1e 100644 --- a/spec/services/key_rotator/attribute_encryption_spec.rb +++ b/spec/services/key_rotator/attribute_encryption_spec.rb @@ -2,25 +2,22 @@ describe KeyRotator::AttributeEncryption do describe '#rotate' do + let(:rotator) { described_class.new(user) } + let(:user) { create(:user) } + it 're-encrypts email and phone' do - user = create(:user, phone: '213-555-5555') - rotator = described_class.new(user) old_encrypted_email = user.encrypted_email - old_encrypted_phone = user.encrypted_phone rotate_attribute_encryption_key rotator.rotate expect(user.encrypted_email).to_not eq old_encrypted_email - expect(user.encrypted_phone).to_not eq old_encrypted_phone end it 'does not change the `updated_at` timestamp' do - user = create(:user) old_updated_timestamp = user.updated_at rotate_attribute_encryption_key - rotator = described_class.new(user) rotator.rotate expect(user.updated_at).to eq old_updated_timestamp diff --git a/spec/services/pii/cacher_spec.rb b/spec/services/pii/cacher_spec.rb index bf5c62c1507..22a64d1fa7f 100644 --- a/spec/services/pii/cacher_spec.rb +++ b/spec/services/pii/cacher_spec.rb @@ -38,7 +38,7 @@ old_ssn_signature = profile.ssn_signature old_email_fingerprint = user.email_fingerprint old_encrypted_email = user.encrypted_email - old_encrypted_phone = user.encrypted_phone + old_encrypted_phone = user.phone_configuration.encrypted_phone old_encrypted_otp_secret_key = user.encrypted_otp_secret_key rotate_all_keys @@ -55,7 +55,7 @@ expect(user.email_fingerprint).to_not eq old_email_fingerprint expect(user.encrypted_email).to_not eq old_encrypted_email expect(profile.ssn_signature).to_not eq old_ssn_signature - expect(user.encrypted_phone).to_not eq old_encrypted_phone + expect(user.phone_configuration.encrypted_phone).to_not eq old_encrypted_phone expect(user.encrypted_otp_secret_key).to_not eq old_encrypted_otp_secret_key end diff --git a/spec/services/populate_phone_configurations_table_spec.rb b/spec/services/populate_phone_configurations_table_spec.rb deleted file mode 100644 index 19bd60bbffd..00000000000 --- a/spec/services/populate_phone_configurations_table_spec.rb +++ /dev/null @@ -1,48 +0,0 @@ -require 'rails_helper' - -describe PopulatePhoneConfigurationsTable do - let(:subject) { described_class.new } - - describe '#call' do - context 'a user with no phone' do - let!(:user) { create(:user) } - - it 'migrates nothing' do - subject.call - expect(user.reload.phone_configuration).to be_nil - end - end - - context 'a user with a phone' do - let!(:user) { create(:user, :with_phone) } - - context 'and no phone_configuration entry' do - before(:each) do - user.phone_configuration.delete - user.reload - end - - it 'migrates without decrypting and re-encrypting' do - expect(EncryptedAttribute).to_not receive(:new) - subject.call - end - - it 'migrates the phone' do - subject.call - configuration = user.reload.phone_configuration - expect(configuration.phone).to eq user.phone - expect(configuration.confirmed_at).to eq user.phone_confirmed_at - expect(configuration.delivery_preference).to eq user.otp_delivery_preference - end - end - - context 'and an existing phone_configuration entry' do - it 'adds no new rows' do - expect(PhoneConfiguration.where(user_id: user.id).count).to eq 1 - subject.call - expect(PhoneConfiguration.where(user_id: user.id).count).to eq 1 - end - end - end - end -end diff --git a/spec/services/remember_device_cookie_spec.rb b/spec/services/remember_device_cookie_spec.rb index 98bc69a4c02..d076b462fd7 100644 --- a/spec/services/remember_device_cookie_spec.rb +++ b/spec/services/remember_device_cookie_spec.rb @@ -2,7 +2,7 @@ describe RememberDeviceCookie do let(:phone_confirmed_at) { 90.days.ago } - let(:user) { create(:user, :with_phone, phone_confirmed_at: phone_confirmed_at) } + let(:user) { create(:user, :with_phone, with: { confirmed_at: phone_confirmed_at }) } let(:created_at) { Time.zone.now } subject { described_class.new(user_id: user.id, created_at: created_at) } @@ -74,7 +74,7 @@ context 'when the token does not refer to the current user' do it 'returns false' do - other_user = create(:user, phone_confirmed_at: 90.days.ago) + other_user = create(:user, :with_phone, with: { confirmed_at: 90.days.ago }) expect(subject.valid_for_user?(other_user)).to eq(false) end diff --git a/spec/services/update_user_spec.rb b/spec/services/update_user_spec.rb index 80153d22192..9a480bfb358 100644 --- a/spec/services/update_user_spec.rb +++ b/spec/services/update_user_spec.rb @@ -31,11 +31,11 @@ expect(phone_configuration.phone).to eq '+1 222 333-4444' end - it 'deletes the phone configuration' do + it 'does not delete the phone configuration' do attributes = { phone: nil } updater = UpdateUser.new(user: user, attributes: attributes) updater.call - expect(user.reload.phone_configuration).to be_nil + expect(user.reload.phone_configuration).to_not be_nil end end diff --git a/spec/support/features/session_helper.rb b/spec/support/features/session_helper.rb index 6decb223a87..03074679eec 100644 --- a/spec/support/features/session_helper.rb +++ b/spec/support/features/session_helper.rb @@ -110,12 +110,12 @@ def sign_in_and_2fa_user(user = user_with_2fa) end def user_with_2fa - create(:user, :signed_up, phone: '+1 202-555-1212', password: VALID_PASSWORD) + create(:user, :signed_up, with: { phone: '+1 202-555-1212' }, password: VALID_PASSWORD) end def user_with_piv_cac create(:user, :signed_up, :with_piv_or_cac, - phone: '+1 (703) 555-0000', + with: { phone: '+1 (703) 555-0000' }, password: VALID_PASSWORD) end diff --git a/spec/support/shared_examples/sign_in.rb b/spec/support/shared_examples/sign_in.rb index 9c0353c20af..751a42b03bf 100644 --- a/spec/support/shared_examples/sign_in.rb +++ b/spec/support/shared_examples/sign_in.rb @@ -174,7 +174,9 @@ it 'does not allow bypassing setting up backup phone' do stub_piv_cac_service - user = create(:user, :signed_up, :with_piv_or_cac, phone: nil) + user = create(:user, :signed_up, :with_piv_or_cac) + user.phone_configuration.destroy + user.reload visit_idp_from_sp_with_loa1(sp) click_link t('links.sign_in') fill_in_credentials_and_submit(user.email, user.password) diff --git a/spec/support/shared_examples_for_phone_validation.rb b/spec/support/shared_examples_for_phone_validation.rb index cf2c732b093..c876e6d1c91 100644 --- a/spec/support/shared_examples_for_phone_validation.rb +++ b/spec/support/shared_examples_for_phone_validation.rb @@ -13,10 +13,10 @@ describe 'phone uniqueness' do context 'when phone is already taken' do it 'is valid' do - second_user = build_stubbed(:user, :signed_up, phone: '+1 (202) 555-1213') + second_user = build_stubbed(:user, :signed_up, with: { phone: '+1 (202) 555-1213' }) allow(User).to receive(:exists?).with(email: 'new@gmail.com').and_return(false) allow(User).to receive(:exists?).with( - phone: second_user.phone_configuration.phone + phone_configuration: { phone: second_user.phone_configuration.phone } ).and_return(true) params[:phone] = second_user.phone_configuration.phone @@ -37,7 +37,6 @@ context 'when phone is same as current user' do it 'is valid' do - user.phone = '+1 (703) 500-5000' user.phone_configuration.phone = '+1 (703) 500-5000' params[:phone] = user.phone_configuration.phone result = subject.submit(params)