diff --git a/.rubocop.yml b/.rubocop.yml index 9fb15de4d44..82e55a94c32 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -6,7 +6,6 @@ require: - rubocop-rails - rubocop-rspec - - rubocop-capybara - ./lib/linters/i18n_helper_html_linter.rb - ./lib/linters/analytics_event_name_linter.rb - ./lib/linters/localized_validation_message_linter.rb @@ -15,9 +14,12 @@ require: - ./lib/linters/redirect_back_linter.rb - ./lib/linters/url_options_linter.rb - ./lib/linters/errors_add_linter.rb + - ./lib/linters/capybara_current_url_expect_linter.rb + - ./lib/linters/capybara_current_path_equality_linter.rb plugins: - rubocop-performance + - rubocop-capybara AllCops: Exclude: @@ -46,12 +48,45 @@ Bundler/DuplicatedGem: Bundler/InsecureProtocolSource: Enabled: true +Capybara/AmbiguousClick: + Enabled: false + Capybara/CurrentPathExpectation: Enabled: true +Capybara/FindAllFirst: + Enabled: true + +Capybara/MatchStyle: + Enabled: true + +Capybara/RedundantWithinFind: + Enabled: true + +Capybara/RSpec/PredicateMatcher: + Enabled: true + +Capybara/SpecificActions: + Enabled: true + +Capybara/SpecificFinders: + Enabled: false + +Capybara/SpecificMatcher: + Enabled: false + +Capybara/VisibilityMatcher: + Enabled: false + Gemspec/DuplicatedAssignment: Enabled: true +IdentityIdp/CapybaraCurrentUrlExpectLinter: + Enabled: true + +IdentityIdp/CapybaraCurrentPathEqualityLinter: + Enabled: true + IdentityIdp/I18nHelperHtmlLinter: Enabled: true Include: diff --git a/Gemfile.lock b/Gemfile.lock index de081646de0..954aff1d128 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -633,8 +633,9 @@ GEM rubocop-ast (1.46.0) parser (>= 3.3.7.2) prism (~> 1.4) - rubocop-capybara (2.21.0) - rubocop (~> 1.41) + rubocop-capybara (2.22.1) + lint_roller (~> 1.1) + rubocop (~> 1.72, >= 1.72.1) rubocop-performance (1.25.0) lint_roller (~> 1.1) rubocop (>= 1.75.0, < 2.0) diff --git a/lib/linters/capybara_current_path_equality_linter.rb b/lib/linters/capybara_current_path_equality_linter.rb new file mode 100644 index 00000000000..7d02262bada --- /dev/null +++ b/lib/linters/capybara_current_path_equality_linter.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module IdentityIdp + # This lint is similar to + # https://docs.rubocop.org/rubocop-capybara/cops_capybara.html#capybaracurrentpathexpectation + # and our `IdentityIdp::CapybaraCurrentUrlExpectLinter` in that it is intended to help prevent + # race conditions. A comparison of the `current_path` or `current_url` relies on page + # navigation timing which can be asynchronous and lead to inconsistent results. + # + # These cases typically come up when trying to support multiple action paths in a feature + # test. We should prefer being explicit about what actions are expected rather than relying + # on changing a shared helper, even if it results in a more verbose test. + # Using method parameters to control the flow is another option. The critical part is the + # conditional should be stable regardless of the timing of browser activity. + # + # @example + # #bad + # return if page.current_path == idv_document_capture_path + # + class CapybaraCurrentPathEqualityLinter < RuboCop::Cop::Base + include RangeHelp + + MSG = 'Do not compare equality of `current_path` in Capybara feature specs - instead,' \ + ' use the `have_current_path` matcher on `page` or avoid it entirely' + + RESTRICT_ON_SEND = %i[== !=].freeze + + def_node_matcher :current_path_equality_lhs, <<~PATTERN + (send (send {(send nil? :page) nil?} :current_path) ${:== :!=} (...)) + PATTERN + + def_node_matcher :current_path_equality_rhs, <<~PATTERN + (send (...) ${:== :!=} (send {(send nil? :page) nil?} :current_path)) + PATTERN + + def on_send(node) + if current_path_equality_lhs(node) || current_path_equality_rhs(node) + add_offense(node) + end + end + end + end + end +end diff --git a/lib/linters/capybara_current_url_expect_linter.rb b/lib/linters/capybara_current_url_expect_linter.rb new file mode 100644 index 00000000000..5857ef72be3 --- /dev/null +++ b/lib/linters/capybara_current_url_expect_linter.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module IdentityIdp + # This lint is a very similar to + # https://docs.rubocop.org/rubocop-capybara/cops_capybara.html#capybaracurrentpathexpectation + # but for `current_url` instead of `current_path`. The reasoning is the same in that it + # ensures usage of Capybara's waiting functionality. This linter is not autocorrectable. + # + # @example + # #bad + # expect(current_url).to eq authentication_methods_setup_url + # expect(current_url).to eq 'http://localhost:3001/auth/result' + # + # #good + # expect(page).to have_current_path(authentication_methods_setup_path) + # expect(page).to have_current_path('http://localhost:3001/auth/result', url: true) + # + class CapybaraCurrentUrlExpectLinter < RuboCop::Cop::Base + include RangeHelp + + MSG = 'Do not set an RSpec expectation on `current_url` in ' \ + 'Capybara feature specs - instead, use the ' \ + '`have_current_path` matcher on `page`' + + RESTRICT_ON_SEND = %i[expect].freeze + + # @!method expectation_set_on_current_url(node) + def_node_matcher :expectation_set_on_current_url, <<~PATTERN + (send nil? :expect (send {(send nil? :page) nil?} :current_url)) + PATTERN + + # Supported matchers: eq(...) / match(/regexp/) / match('regexp') + # @!method as_is_matcher(node) + def_node_matcher :as_is_matcher, <<~PATTERN + (send + #expectation_set_on_current_url ${:to :to_not :not_to} + ${(send nil? :eq ...) (send nil? :match (...)) (send nil? :include ...) (send nil? :start_with ...)}) + PATTERN + + # @!method regexp_node_matcher(node) + def_node_matcher :regexp_node_matcher, <<~PATTERN + (send + #expectation_set_on_current_url ${:to :to_not :not_to} + $(send nil? :match ${str dstr xstr})) + PATTERN + + def on_send(node) + expectation_set_on_current_url(node) do + as_is_matcher(node.parent) do + add_offense(node.parent.loc.selector) + end + + regexp_node_matcher(node.parent) do + add_offense(node.parent.loc.selector) + end + end + end + end + end + end +end diff --git a/spec/features/account_creation/completions_cancel_spec.rb b/spec/features/account_creation/completions_cancel_spec.rb index 7bd3fbbd06e..2065a0b5c8d 100644 --- a/spec/features/account_creation/completions_cancel_spec.rb +++ b/spec/features/account_creation/completions_cancel_spec.rb @@ -27,6 +27,7 @@ url: true, ignore_query: true, ) - expect(current_url).to start_with('http://localhost:7654/auth/result?error=access_denied') + params = UriService.params(current_url) + expect(params['error']).to eq('access_denied') end end diff --git a/spec/features/idv/cancel_spec.rb b/spec/features/idv/cancel_spec.rb index b954e52ec49..66902e536d2 100644 --- a/spec/features/idv/cancel_spec.rb +++ b/spec/features/idv/cancel_spec.rb @@ -20,13 +20,13 @@ end it 'shows the user a cancellation message with the option to go back to the step' do + expect(page).to have_current_path(idv_agreement_path) expect(page).to have_content(t('doc_auth.headings.verify_identity')) - original_path = current_path click_link t('links.cancel') + expect(page).to have_current_path(idv_cancel_path(step: 'agreement')) expect(page).to have_content(t('idv.cancel.headings.prompt.standard')) - expect(page).to have_current_path(idv_cancel_path, ignore_query: true) expect(fake_analytics).to have_logged_event( 'IdV: cancellation visited', hash_including(step: 'agreement'), @@ -35,12 +35,13 @@ expect(page).to have_unique_form_landmark_labels expect(page).to have_button(t('idv.cancel.actions.start_over')) + expect(page).to have_no_button(t('idv.cancel.actions.exit', app_name: APP_NAME)) expect(page).to have_button(t('idv.cancel.actions.account_page')) expect(page).to have_button(t('idv.cancel.actions.keep_going')) click_on(t('idv.cancel.actions.keep_going')) + expect(page).to have_current_path(idv_agreement_path) expect(page).to have_content(t('doc_auth.headings.lets_go')) - expect(page).to have_current_path(original_path) expect(fake_analytics).to have_logged_event( 'IdV: cancellation go back', step: 'agreement', @@ -51,8 +52,8 @@ expect(page).to have_content(t('doc_auth.headings.verify_identity')) click_link t('links.cancel') + expect(page).to have_current_path(idv_cancel_path(step: 'agreement')) expect(page).to have_content(t('idv.cancel.headings.prompt.standard')) - expect(page).to have_current_path(idv_cancel_path, ignore_query: true) expect(fake_analytics).to have_logged_event( 'IdV: cancellation visited', hash_including(step: 'agreement'), @@ -66,8 +67,8 @@ click_on t('idv.cancel.actions.start_over') - expect(page).to have_content(t('doc_auth.instructions.getting_started')) expect(page).to have_current_path(idv_welcome_path) + expect(page).to have_content(t('doc_auth.instructions.getting_started')) expect(fake_analytics).to have_logged_event( 'IdV: start over', step: 'agreement', @@ -78,8 +79,8 @@ expect(page).to have_content(t('doc_auth.headings.verify_identity')) click_link t('links.cancel') + expect(page).to have_current_path(idv_cancel_path(step: 'agreement')) expect(page).to have_content(t('idv.cancel.headings.prompt.standard')) - expect(page).to have_current_path(idv_cancel_path, ignore_query: true) expect(fake_analytics).to have_logged_event( 'IdV: cancellation visited', hash_including(step: 'agreement'), @@ -118,6 +119,7 @@ expect(page).to have_content(t('doc_auth.info.ssn')) click_link t('links.cancel') + expect(page).to have_current_path(idv_cancel_path(step: 'ssn')) expect(page).to have_content(t('idv.cancel.headings.prompt.standard')) expect(fake_analytics).to have_logged_event( 'IdV: cancellation visited', @@ -174,8 +176,8 @@ click_link t('links.cancel') + expect(page).to have_current_path(idv_cancel_path(step: 'agreement')) expect(page).to have_content(t('idv.cancel.headings.prompt.standard')) - expect(page).to have_current_path(idv_cancel_path, ignore_query: true) expect(fake_analytics).to have_logged_event( 'IdV: cancellation visited', hash_including(step: 'agreement'), @@ -194,7 +196,10 @@ url: true, ignore_query: true, ) - expect(current_url).to start_with('http://localhost:7654/auth/result?error=access_denied') + + params = UriService.params(current_url) + expect(params['error']).to eq('access_denied') + expect(fake_analytics).to have_logged_event( 'IdV: cancellation confirmed', step: 'agreement', diff --git a/spec/features/idv/doc_auth/document_capture_spec.rb b/spec/features/idv/doc_auth/document_capture_spec.rb index d4e00216fb8..f009108fe2c 100644 --- a/spec/features/idv/doc_auth/document_capture_spec.rb +++ b/spec/features/idv/doc_auth/document_capture_spec.rb @@ -164,7 +164,7 @@ complete_doc_auth_steps_before_document_capture_step end - it 'user can go through verification uploading ID and selfie on seprerate pages' do + it 'user can go through verification uploading ID and selfie on separate pages' do expect(page).to have_current_path(idv_document_capture_url) expect(page).not_to have_content(t('doc_auth.tips.document_capture_selfie_text1')) attach_images @@ -553,22 +553,13 @@ fill_out_ssn_form_ok click_idv_continue complete_verify_step - # expect(page).to have_content(t('doc_auth.headings.document_capture_selfie')) expect(page).to have_current_path(idv_phone_url) end end end context 'when a selfie is required by the SP' do - context 'on mobile platform', allow_browser_log: true do - before do - # mock mobile device as cameraCapable, this allows us to process - allow_any_instance_of(ActionController::Parameters) - .to receive(:[]).and_wrap_original do |impl, param_name| - param_name.to_sym == :skip_hybrid_handoff ? '' : impl.call(param_name) - end - end - + context 'on mobile platform', driver: :headless_chrome_mobile, allow_browser_log: true do context 'with a passing selfie' do it 'proceeds to the next page with valid info, including a selfie image' do perform_in_browser(:mobile) do @@ -647,11 +638,6 @@ # when there are multiple doc auth errors on front and back it 'shows the correct error message for the given error' do perform_in_browser(:mobile) do - if page.current_path == idv_how_to_verify_path - click_button t('forms.buttons.continue_online') - else - click_continue - end use_id_image('ial2_test_credential_multiple_doc_auth_failures_both_sides.yml') click_continue click_button 'Take photo' diff --git a/spec/features/idv/doc_auth/redo_document_capture_spec.rb b/spec/features/idv/doc_auth/redo_document_capture_spec.rb index 296caf18b84..90e9c292b12 100644 --- a/spec/features/idv/doc_auth/redo_document_capture_spec.rb +++ b/spec/features/idv/doc_auth/redo_document_capture_spec.rb @@ -415,15 +415,7 @@ end context 'when a selfie is required by the SP' do - context 'on mobile platform', allow_browser_log: true do - before do - # mock mobile device as cameraCapable, this allows us to process - allow_any_instance_of(ActionController::Parameters) - .to receive(:[]).and_wrap_original do |impl, param_name| - param_name.to_sym == :skip_hybrid_handoff ? '' : impl.call(param_name) - end - end - + context 'on mobile platform', driver: :headless_chrome_mobile, allow_browser_log: true do context 'with a passing selfie' do it 'proceeds to the next page with valid info, including a selfie image' do perform_in_browser(:mobile) do @@ -463,9 +455,6 @@ # when there are multiple doc auth errors on front and back it 'shows the correct error message for the given error' do perform_in_browser(:mobile) do - if page.current_path == idv_how_to_verify_path - click_button t('forms.buttons.continue_online') - end use_id_image('ial2_test_credential_multiple_doc_auth_failures_both_sides.yml') click_continue click_button 'Take photo' diff --git a/spec/features/idv/doc_auth/verify_info_step_spec.rb b/spec/features/idv/doc_auth/verify_info_step_spec.rb index 17e0529eb57..d0427cc7226 100644 --- a/spec/features/idv/doc_auth/verify_info_step_spec.rb +++ b/spec/features/idv/doc_auth/verify_info_step_spec.rb @@ -21,307 +21,309 @@ } end - before do - allow_any_instance_of(ApplicationController).to receive(:analytics).and_return(fake_analytics) - allow_any_instance_of(ApplicationController).to receive(:attempts_api_tracker).and_return( - attempts_api_tracker, - ) - sign_in_and_2fa_user(user) - complete_doc_auth_steps_before_ssn_step - end - - context 'with good ssn' do + context 'no outage' do before do - fill_out_ssn_form_ok - click_idv_continue - end - - it 'allows the user to enter in a new address and displays updated info' do - click_link t('idv.buttons.change_address_label') - fill_in 'idv_form_zipcode', with: '12345' - fill_in 'idv_form_address2', with: 'Apt 3E' - - click_button t('forms.buttons.submit.update') - - expect(page).to have_current_path(idv_verify_info_path) - - expect(page).to have_content('12345') - expect(page).to have_content('Apt 3E') - - complete_verify_step - - expect(fake_analytics).to have_logged_event( - 'IdV: doc auth verify proofing results', - hash_including( - address_edited: true, - address_line2_present: true, - analytics_id: 'Doc Auth', - ), + allow_any_instance_of(ApplicationController).to receive(:analytics).and_return(fake_analytics) + allow_any_instance_of(ApplicationController).to receive(:attempts_api_tracker).and_return( + attempts_api_tracker, ) + sign_in_and_2fa_user(user) + complete_doc_auth_steps_before_ssn_step end - it 'allows the user to enter in a new ssn and displays updated info' do - click_link t('idv.buttons.change_ssn_label') - - expect(page).to have_current_path(idv_ssn_path) - expect(page).to_not have_content(t('doc_auth.headings.capture_complete')) - expect( - find_field(t('idv.form.ssn_label')).value, - ).to eq(DocAuthHelper::GOOD_SSN.gsub(/\D/, '')) - - fill_in t('idv.form.ssn_label'), with: '900456789' - click_button t('forms.buttons.submit.update') + context 'with good ssn' do + before do + fill_out_ssn_form_ok + click_idv_continue + end - expect(fake_analytics).to have_logged_event( - 'IdV: doc auth redo_ssn submitted', - ) + it 'allows the user to enter in a new address and displays updated info' do + click_link t('idv.buttons.change_address_label') + fill_in 'idv_form_zipcode', with: '12345' + fill_in 'idv_form_address2', with: 'Apt 3E' - expect(page).to have_current_path(idv_verify_info_path) + click_button t('forms.buttons.submit.update') - expect(page).to have_text('9**-**-***9') - check t('forms.ssn.show') - expect(page).to have_text('900-45-6789') - end + expect(page).to have_current_path(idv_verify_info_path) - it 'logs analytics event on submit' do - complete_verify_step + expect(page).to have_content('12345') + expect(page).to have_content('Apt 3E') - expect(fake_analytics).to have_logged_event( - 'IdV: doc auth verify proofing results', - hash_including(address_edited: false, address_line2_present: false), - ) - end - end + complete_verify_step - it 'does not proceed to the next page if resolution fails' do - fill_out_ssn_form_with_ssn_that_fails_resolution - click_idv_continue - complete_verify_step + expect(fake_analytics).to have_logged_event( + 'IdV: doc auth verify proofing results', + hash_including( + address_edited: true, + address_line2_present: true, + analytics_id: 'Doc Auth', + ), + ) + end - expect(page).to have_current_path(idv_session_errors_warning_path) - expect_step_indicator_current_step(t('step_indicator.flows.idv.verify_info')) - click_on t('idv.failure.button.warning') + it 'allows the user to enter in a new ssn and displays updated info' do + click_link t('idv.buttons.change_ssn_label') - expect(page).to have_current_path(idv_verify_info_path) - end + expect(page).to have_current_path(idv_ssn_path) + expect(page).to_not have_content(t('doc_auth.headings.capture_complete')) + expect( + find_field(t('idv.form.ssn_label')).value, + ).to eq(DocAuthHelper::GOOD_SSN.gsub(/\D/, '')) - it 'does not proceed to the next page if resolution raises an exception' do - fill_out_ssn_form_with_ssn_that_raises_exception + fill_in t('idv.form.ssn_label'), with: '900456789' + click_button t('forms.buttons.submit.update') - click_idv_continue - complete_verify_step + expect(fake_analytics).to have_logged_event( + 'IdV: doc auth redo_ssn submitted', + ) - expect(fake_analytics).to have_logged_event( - 'IdV: doc auth exception visited', - step_name: 'verify_info', - remaining_submit_attempts: 5, - ) - expect(page).to have_current_path(idv_session_errors_exception_path) + expect(page).to have_current_path(idv_verify_info_path) - click_on t('idv.failure.button.warning') + expect(page).to have_text('9**-**-***9') + check t('forms.ssn.show') + expect(page).to have_text('900-45-6789') + end - expect(page).to have_current_path(idv_verify_info_path) - end + it 'logs analytics event on submit' do + complete_verify_step - context 'resolution rate limiting' do - let(:max_resolution_attempts) { 3 } - before do - allow(IdentityConfig.store).to receive(:idv_max_attempts) - .and_return(max_resolution_attempts) + expect(fake_analytics).to have_logged_event( + 'IdV: doc auth verify proofing results', + hash_including(address_edited: false, address_line2_present: false), + ) + end + end + it 'does not proceed to the next page if resolution fails' do fill_out_ssn_form_with_ssn_that_fails_resolution click_idv_continue - end + complete_verify_step - # proof_ssn_max_attempts is 10, vs 5 for resolution, so it doesn't get triggered - it 'rate limits resolution and continues when it expires' do - expect(attempts_api_tracker).to receive(:idv_rate_limited).with( - limiter_type: :idv_resolution, - ).twice + expect(page).to have_current_path(idv_session_errors_warning_path) + expect_step_indicator_current_step(t('step_indicator.flows.idv.verify_info')) + click_on t('idv.failure.button.warning') - (max_resolution_attempts - 2).times do - complete_verify_step - expect(page).to have_current_path(idv_session_errors_warning_path) - click_try_again - end + expect(page).to have_current_path(idv_verify_info_path) + end - # Check that last attempt shows correct warning text - complete_verify_step - expect(page).to have_current_path(idv_session_errors_warning_path) - expect(page).to have_content( - strip_tags( - t('idv.failure.attempts_html.one'), - ), - ) - click_try_again + it 'does not proceed to the next page if resolution raises an exception' do + fill_out_ssn_form_with_ssn_that_raises_exception + click_idv_continue complete_verify_step - expect(page).to have_current_path(idv_session_errors_failure_path) - expect(page).not_to have_css('.step-indicator__step--current', text: text, wait: 5) + expect(fake_analytics).to have_logged_event( - 'Rate Limit Reached', - limiter_type: :idv_resolution, + 'IdV: doc auth exception visited', step_name: 'verify_info', + remaining_submit_attempts: 5, ) + expect(page).to have_current_path(idv_session_errors_exception_path) - visit idv_verify_info_url - expect(page).to have_current_path(idv_session_errors_failure_path) - - # Manual expiration is needed because Redis timestamp doesn't always match ruby timestamp - RateLimiter.new(user: user, rate_limit_type: :idv_resolution).reset! - travel_to(IdentityConfig.store.idv_attempt_window_in_hours.hours.from_now + 1) do - sign_in_and_2fa_user(user) - complete_doc_auth_steps_before_verify_step - complete_verify_step + click_on t('idv.failure.button.warning') - expect(page).to have_current_path(idv_phone_path) - expect(RateLimiter.new(user: user, rate_limit_type: :idv_resolution)).to_not be_limited - end + expect(page).to have_current_path(idv_verify_info_path) end - it 'allows user to cancel identify verification' do - click_link t('links.cancel') - expect(page).to have_current_path(idv_cancel_path(step: 'verify')) - end - end + context 'resolution rate limiting' do + let(:max_resolution_attempts) { 3 } + before do + allow(IdentityConfig.store).to receive(:idv_max_attempts) + .and_return(max_resolution_attempts) - context 'ssn rate limiting' do - # Simulates someone trying same SSN with second account - let(:max_resolution_attempts) { 4 } - let(:max_ssn_attempts) { 3 } + fill_out_ssn_form_with_ssn_that_fails_resolution + click_idv_continue + end - before do - allow(IdentityConfig.store).to receive(:idv_max_attempts) - .and_return(max_resolution_attempts) + # proof_ssn_max_attempts is 10, vs 5 for resolution, so it doesn't get triggered + it 'rate limits resolution and continues when it expires' do + expect(attempts_api_tracker).to receive(:idv_rate_limited).with( + limiter_type: :idv_resolution, + ).twice - allow(IdentityConfig.store).to receive(:proof_ssn_max_attempts) - .and_return(max_ssn_attempts) + (max_resolution_attempts - 2).times do + complete_verify_step + expect(page).to have_current_path(idv_session_errors_warning_path) + click_try_again + end - fill_out_ssn_form_with_ssn_that_fails_resolution - click_idv_continue - (max_ssn_attempts - 1).times do + # Check that last attempt shows correct warning text complete_verify_step expect(page).to have_current_path(idv_session_errors_warning_path) + expect(page).to have_content( + strip_tags( + t('idv.failure.attempts_html.one'), + ), + ) click_try_again - end - end - it 'rate limits ssn and continues when it expires' do - expect(attempts_api_tracker).to receive(:idv_rate_limited).with( - limiter_type: :proof_ssn, - ).twice + complete_verify_step + expect(page).to have_current_path(idv_session_errors_failure_path) + expect(page).not_to have_css('.step-indicator__step--current', text: text, wait: 5) + expect(fake_analytics).to have_logged_event( + 'Rate Limit Reached', + limiter_type: :idv_resolution, + step_name: 'verify_info', + ) - complete_verify_step - expect(page).to have_current_path(idv_session_errors_ssn_failure_path) - expect(fake_analytics).to have_logged_event( - 'Rate Limit Reached', - limiter_type: :proof_ssn, - step_name: 'verify_info', - ) + visit idv_verify_info_url + expect(page).to have_current_path(idv_session_errors_failure_path) - visit idv_verify_info_url - # second rate limit event - expect(page).to have_current_path(idv_session_errors_ssn_failure_path) + # Manual expiration is needed because Redis timestamp doesn't always match ruby timestamp + RateLimiter.new(user: user, rate_limit_type: :idv_resolution).reset! + travel_to(IdentityConfig.store.idv_attempt_window_in_hours.hours.from_now + 1) do + sign_in_and_2fa_user(user) + complete_doc_auth_steps_before_verify_step + complete_verify_step - # Manual expiration is needed because Redis timestamp doesn't always match ruby timestamp - RateLimiter.new(user: user, rate_limit_type: :idv_resolution).reset! - travel_to(IdentityConfig.store.idv_attempt_window_in_hours.hours.from_now + 1) do - sign_in_and_2fa_user(user) - complete_doc_auth_steps_before_verify_step - complete_verify_step + expect(page).to have_current_path(idv_phone_path) + expect(RateLimiter.new(user: user, rate_limit_type: :idv_resolution)).to_not be_limited + end + end - expect(page).to have_current_path(idv_phone_path) - expect(RateLimiter.new(user: user, rate_limit_type: :idv_resolution)).to_not be_limited + it 'allows user to cancel identify verification' do + click_link t('links.cancel') + expect(page).to have_current_path(idv_cancel_path(step: 'verify')) end end - it 'continues to next step if ssn successful on last attempt' do - click_link t('idv.buttons.change_ssn_label') + context 'ssn rate limiting' do + # Simulates someone trying same SSN with second account + let(:max_resolution_attempts) { 4 } + let(:max_ssn_attempts) { 3 } + + before do + allow(IdentityConfig.store).to receive(:idv_max_attempts) + .and_return(max_resolution_attempts) + + allow(IdentityConfig.store).to receive(:proof_ssn_max_attempts) + .and_return(max_ssn_attempts) + + fill_out_ssn_form_with_ssn_that_fails_resolution + click_idv_continue + (max_ssn_attempts - 1).times do + complete_verify_step + expect(page).to have_current_path(idv_session_errors_warning_path) + click_try_again + end + end + + it 'rate limits ssn and continues when it expires' do + expect(attempts_api_tracker).to receive(:idv_rate_limited).with( + limiter_type: :proof_ssn, + ).twice - expect(page).to have_current_path(idv_ssn_path) - expect(page).to_not have_content(t('doc_auth.headings.capture_complete')) - expect( - find_field(t('idv.form.ssn_label')).value, - ).not_to eq(DocAuthHelper::GOOD_SSN.gsub(/\D/, '')) + complete_verify_step + expect(page).to have_current_path(idv_session_errors_ssn_failure_path) + expect(fake_analytics).to have_logged_event( + 'Rate Limit Reached', + limiter_type: :proof_ssn, + step_name: 'verify_info', + ) - fill_in t('idv.form.ssn_label'), with: '900456789' - click_button t('forms.buttons.submit.update') - complete_verify_step + visit idv_verify_info_url + # second rate limit event + expect(page).to have_current_path(idv_session_errors_ssn_failure_path) - expect(page).to have_current_path(idv_phone_path) - expect(fake_analytics).not_to have_logged_event( - 'Rate Limit Reached', - limiter_type: :proof_ssn, - step_name: 'verify_info', - ) - end - end + # Manual expiration is needed because Redis timestamp doesn't always match ruby timestamp + RateLimiter.new(user: user, rate_limit_type: :idv_resolution).reset! + travel_to(IdentityConfig.store.idv_attempt_window_in_hours.hours.from_now + 1) do + sign_in_and_2fa_user(user) + complete_doc_auth_steps_before_verify_step + complete_verify_step - context 'AAMVA' do - let(:mock_state_id_jurisdiction) do - [Idp::Constants::MOCK_IDV_APPLICANT_STATE_ID_JURISDICTION] - end + expect(page).to have_current_path(idv_phone_path) + expect(RateLimiter.new(user: user, rate_limit_type: :idv_resolution)).to_not be_limited + end + end - context 'when the user lives in an AAMVA supported state' do - it 'performs a resolution and state ID check' do - allow(IdentityConfig.store).to receive(:aamva_supported_jurisdictions).and_return( - mock_state_id_jurisdiction, - ) - expect_any_instance_of(Proofing::Mock::IdMockClient).to receive(:proof).with( - hash_including( - **Idp::Constants::MOCK_IDV_APPLICANT, - ), - ).and_call_original + it 'continues to next step if ssn successful on last attempt' do + click_link t('idv.buttons.change_ssn_label') - complete_ssn_step + expect(page).to have_current_path(idv_ssn_path) + expect(page).to_not have_content(t('doc_auth.headings.capture_complete')) + expect( + find_field(t('idv.form.ssn_label')).value, + ).not_to eq(DocAuthHelper::GOOD_SSN.gsub(/\D/, '')) + + fill_in t('idv.form.ssn_label'), with: '900456789' + click_button t('forms.buttons.submit.update') complete_verify_step + + expect(page).to have_current_path(idv_phone_path) + expect(fake_analytics).not_to have_logged_event( + 'Rate Limit Reached', + limiter_type: :proof_ssn, + step_name: 'verify_info', + ) end end - context 'when the user does not live in an AAMVA supported state' do - it 'does not perform the state ID check' do - allow(IdentityConfig.store).to receive(:aamva_supported_jurisdictions).and_return( - IdentityConfig.store.aamva_supported_jurisdictions - + context 'AAMVA' do + let(:mock_state_id_jurisdiction) do + [Idp::Constants::MOCK_IDV_APPLICANT_STATE_ID_JURISDICTION] + end + + context 'when the user lives in an AAMVA supported state' do + it 'performs a resolution and state ID check' do + allow(IdentityConfig.store).to receive(:aamva_supported_jurisdictions).and_return( mock_state_id_jurisdiction, - ) - expect_any_instance_of(Proofing::Mock::IdMockClient).to_not receive(:proof) + ) + expect_any_instance_of(Proofing::Mock::IdMockClient).to receive(:proof).with( + hash_including( + **Idp::Constants::MOCK_IDV_APPLICANT, + ), + ).and_call_original + + complete_ssn_step + complete_verify_step + end + end - complete_ssn_step - complete_verify_step + context 'when the user does not live in an AAMVA supported state' do + it 'does not perform the state ID check' do + allow(IdentityConfig.store).to receive(:aamva_supported_jurisdictions).and_return( + IdentityConfig.store.aamva_supported_jurisdictions - + mock_state_id_jurisdiction, + ) + expect_any_instance_of(Proofing::Mock::IdMockClient).to_not receive(:proof) + + complete_ssn_step + complete_verify_step + end end end - end - context 'async missing' do - it 'allows resubmitting form' do - complete_ssn_step + context 'async missing' do + it 'allows resubmitting form' do + complete_ssn_step - allow(DocumentCaptureSession).to receive(:find_by) - .and_return(nil) + allow(DocumentCaptureSession).to receive(:find_by) + .and_return(nil) - complete_verify_step - expect(fake_analytics).to have_logged_event('IdV: proofing resolution result missing') - expect(page).to have_content(t('idv.failure.timeout')) - expect(page).to have_current_path(idv_verify_info_path) - allow(DocumentCaptureSession).to receive(:find_by).and_call_original - complete_verify_step - expect(page).to have_current_path(idv_phone_path) + complete_verify_step + expect(fake_analytics).to have_logged_event('IdV: proofing resolution result missing') + expect(page).to have_content(t('idv.failure.timeout')) + expect(page).to have_current_path(idv_verify_info_path) + allow(DocumentCaptureSession).to receive(:find_by).and_call_original + complete_verify_step + expect(page).to have_current_path(idv_phone_path) + end end - end - context 'async timed out' do - it 'allows resubmitting form' do - complete_ssn_step + context 'async timed out' do + it 'allows resubmitting form' do + complete_ssn_step - allow(DocumentCaptureSession).to receive(:find_by) - .and_return(nil) + allow(DocumentCaptureSession).to receive(:find_by) + .and_return(nil) - complete_verify_step - expect(page).to have_content(t('idv.failure.timeout')) - expect(page).to have_current_path(idv_verify_info_path) - allow(DocumentCaptureSession).to receive(:find_by).and_call_original - complete_verify_step - expect(page).to have_current_path(idv_phone_path) + complete_verify_step + expect(page).to have_content(t('idv.failure.timeout')) + expect(page).to have_current_path(idv_verify_info_path) + allow(DocumentCaptureSession).to receive(:find_by).and_call_original + complete_verify_step + expect(page).to have_current_path(idv_phone_path) + end end end @@ -330,7 +332,13 @@ allow_any_instance_of(OutageStatus).to receive(:any_phone_vendor_outage?).and_return(true) visit_idp_from_sp_with_ial2(:oidc) sign_in_and_2fa_user(user) - complete_doc_auth_steps_before_verify_step + complete_doc_auth_steps_before_welcome_step + click_idv_continue # Acknowledge mail-only alert + complete_welcome_step + complete_agreement_step + complete_hybrid_handoff_step + complete_document_capture_step + complete_ssn_step end it 'should be at the verify step page' do diff --git a/spec/features/idv/hybrid_mobile/entry_spec.rb b/spec/features/idv/hybrid_mobile/entry_spec.rb index 408c91908ca..79bd4992aed 100644 --- a/spec/features/idv/hybrid_mobile/entry_spec.rb +++ b/spec/features/idv/hybrid_mobile/entry_spec.rb @@ -32,12 +32,12 @@ Capybara.using_session('mobile') do visit link_to_visit # Should have redirected to the actual doc capture url - expect(current_url).to eql(idv_hybrid_mobile_document_capture_url) + expect(page).to have_current_path(idv_hybrid_mobile_document_capture_path) # Confirm that we end up on the LN / Mock page even if we try to # go to the Socure one. visit idv_hybrid_mobile_socure_document_capture_url - expect(page).to have_current_path(idv_hybrid_mobile_document_capture_url) + expect(page).to have_current_path(idv_hybrid_mobile_document_capture_path) end end @@ -54,12 +54,12 @@ Capybara.using_session('mobile') do visit link_to_visit # Should have redirected to the actual doc capture url - expect(current_url).to eql(idv_hybrid_mobile_socure_document_capture_url) + expect(page).to have_current_path(idv_hybrid_mobile_socure_document_capture_path) # Confirm that we end up on the LN / Mock page even if we try to # go to the Socure one. visit idv_hybrid_mobile_document_capture_url - expect(page).to have_current_path(idv_hybrid_mobile_socure_document_capture_url) + expect(page).to have_current_path(idv_hybrid_mobile_socure_document_capture_path) end end end @@ -79,7 +79,7 @@ Capybara.using_session('mobile') do visit link_to_visit # Should have redirected to the actual doc capture url - expect(current_url).to eql(idv_hybrid_mobile_document_capture_url) + expect(page).to have_current_path(idv_hybrid_mobile_document_capture_path) end end end @@ -99,7 +99,7 @@ Capybara.using_session('mobile') do visit link_to_visit - expect(current_url).to eql(root_url) + expect(page).to have_current_path(root_path) end end end diff --git a/spec/features/idv/in_person_spec.rb b/spec/features/idv/in_person_spec.rb index 2ff829c7046..e8719542272 100644 --- a/spec/features/idv/in_person_spec.rb +++ b/spec/features/idv/in_person_spec.rb @@ -232,13 +232,15 @@ end end - context 'the user fails remote docauth and starts IPP', allow_browser_log: true do + context 'the user fails remote doc auth and starts IPP', allow_browser_log: true do before do allow(IdentityConfig.store).to receive(:in_person_proofing_opt_in_enabled).and_return(true) visit_idp_from_sp_with_ial2(:oidc, **{ client_id: ipp_service_provider.issuer }) sign_in_via_branded_page(user) - complete_doc_auth_steps_before_document_capture_step + complete_welcome_step + complete_agreement_step + complete_hybrid_handoff_step # Fail docauth complete_document_capture_step_with_yml( diff --git a/spec/features/idv/outage_spec.rb b/spec/features/idv/outage_spec.rb index b6e71a21aff..f5107b38f10 100644 --- a/spec/features/idv/outage_spec.rb +++ b/spec/features/idv/outage_spec.rb @@ -152,7 +152,7 @@ def sign_in_with_idv_required(user:, sms_or_totp: :sms) click_on t('links.exit_login', app_name: APP_NAME) - expect(current_url).to eq 'https://example.com/' + expect(page).to have_current_path('https://example.com/', url: true) end it 'skips the hybrid handoff screen and proceeds to doc capture' do diff --git a/spec/features/idv/sp_follow_up_spec.rb b/spec/features/idv/sp_follow_up_spec.rb index 2821443da76..735a5d1f820 100644 --- a/spec/features/idv/sp_follow_up_spec.rb +++ b/spec/features/idv/sp_follow_up_spec.rb @@ -25,7 +25,7 @@ open_last_email click_email_link_matching(/return_to_sp\/account_verified_cta/) - expect(current_url).to eq(post_idv_follow_up_url) + expect(page).to have_current_path(post_idv_follow_up_url, url: true) end scenario 'receiving an email after passing fraud review' do @@ -43,7 +43,7 @@ open_last_email click_email_link_matching(/return_to_sp\/account_verified_cta/) - expect(current_url).to eq(post_idv_follow_up_url) + expect(page).to have_current_path(post_idv_follow_up_url, url: true) end context 'after entering a verify-by-mail code' do @@ -69,10 +69,10 @@ click_button t('idv.gpo.form.submit') acknowledge_and_confirm_personal_key - expect(page).to have_current_path(idv_sp_follow_up_path) + expect(page).to have_current_path(idv_sp_follow_up_url, url: true) click_on t('idv.by_mail.sp_follow_up.connect_account') - expect(current_url).to eq(post_idv_follow_up_url) + expect(page).to have_current_path(post_idv_follow_up_url, url: true) end scenario 'canceling on the CTA and visiting from the account page' do @@ -97,10 +97,10 @@ click_button t('idv.gpo.form.submit') acknowledge_and_confirm_personal_key - expect(page).to have_current_path(idv_sp_follow_up_path) + expect(page).to have_current_path(idv_sp_follow_up_url, url: true) click_on t('idv.by_mail.sp_follow_up.go_to_account') - expect(current_url).to eq(account_url) + expect(page).to have_current_path(account_path) expect(page).to have_content( t( @@ -115,7 +115,7 @@ ), ) - expect(current_url).to eq(post_idv_follow_up_url) + expect(page).to have_current_path(post_idv_follow_up_url, url: true) end end end diff --git a/spec/features/multiple_emails/sp_sign_in_spec.rb b/spec/features/multiple_emails/sp_sign_in_spec.rb index 54c2705713f..551a582a918 100644 --- a/spec/features/multiple_emails/sp_sign_in_spec.rb +++ b/spec/features/multiple_emails/sp_sign_in_spec.rb @@ -11,12 +11,12 @@ expect(emails.count).to eq(2) - emails.each do |email| + emails.each.with_index do |email, index| visit_idp_from_oidc_sp(scope: 'openid email') signin(email, user.password) fill_in_code_with_last_phone_otp click_submit_default - click_agree_and_continue if current_path == sign_up_completed_path + click_agree_and_continue if index == 0 expect(oidc_decoded_id_token[:email]).to eq(emails.first) expect(oidc_decoded_id_token[:all_emails]).to be_nil @@ -76,12 +76,12 @@ expect(emails.count).to eq(2) - emails.each do |email| + emails.each.with_index do |email, index| visit authn_request signin(email, user.password) fill_in_code_with_last_phone_otp click_submit_default_twice - click_agree_and_continue if current_path == sign_up_completed_path + click_agree_and_continue if index == 0 click_submit_default xmldoc = SamlResponseDoc.new('feature', 'response_assertion') diff --git a/spec/features/openid_connect/authorization_confirmation_spec.rb b/spec/features/openid_connect/authorization_confirmation_spec.rb index cf56f6db45d..77b740c7ded 100644 --- a/spec/features/openid_connect/authorization_confirmation_spec.rb +++ b/spec/features/openid_connect/authorization_confirmation_spec.rb @@ -37,7 +37,7 @@ def create_user_and_remember_device second_email = create(:email_address, user: user1) sign_in_user(user1, second_email.email) visit_idp_from_ial1_oidc_sp - expect(current_url).to match(user_authorization_confirmation_path) + expect(page).to have_current_path(user_authorization_confirmation_path) expect(page).to have_content shared_email continue_as(second_email.email) @@ -50,7 +50,7 @@ def create_user_and_remember_device second_email = create(:email_address, user: user1) sign_in_user(user1, second_email.email) visit_idp_from_ial1_oidc_sp - expect(current_url).to match(user_authorization_confirmation_path) + expect(page).to have_current_path(user_authorization_confirmation_path) expect(page).to have_content second_email.email continue_as(second_email.email) @@ -108,7 +108,7 @@ def create_user_and_remember_device it 'it allows the user to switch accounts prior to continuing to the SP' do sign_in_user(user1) visit_idp_from_ial1_oidc_sp - expect(current_url).to match(user_authorization_confirmation_path) + expect(page).to have_current_path(user_authorization_confirmation_path) continue_as(user2.email, user2.password) diff --git a/spec/features/openid_connect/openid_connect_spec.rb b/spec/features/openid_connect/openid_connect_spec.rb index 7f2c843fc61..a041aeaf9fd 100644 --- a/spec/features/openid_connect/openid_connect_spec.rb +++ b/spec/features/openid_connect/openid_connect_spec.rb @@ -529,9 +529,9 @@ state: state, ) - current_url_no_port = URI(current_url).tap { |uri| uri.port = nil }.to_s - expect(current_url_no_port).to include( - "http://www.example.com/openid_connect/logout?id_token_hint=#{id_token}", + expect(page).to have_current_path( + "http://www.example.com/openid_connect/logout?id_token_hint=#{id_token}&post_logout_redirect_uri=gov.gsa.openidconnect.test://result/signout&state=#{state}", + url: true, ) expect(page).to have_content(t('openid_connect.logout.errors.id_token_hint_present')) end @@ -933,12 +933,12 @@ it 'displays the branded page' do visit_idp_from_ial1_oidc_sp - expect(current_url).to eq(root_url) + expect(page).to have_current_path(root_path) expect_branded_experience visit_idp_from_ial1_oidc_sp - expect(current_url).to eq(root_url) + expect(page).to have_current_path(root_path) expect_branded_experience end end @@ -951,7 +951,7 @@ sign_in_live_with_2fa(user) sp = ServiceProvider.find_by(issuer: 'urn:gov:gsa:openidconnect:sp:server') - expect(current_url).to eq(sign_up_completed_url) + expect(page).to have_current_path(sign_up_completed_path) expect(page).to have_content( t( 'titles.sign_up.completion_first_sign_in', @@ -1100,7 +1100,7 @@ sp = ServiceProvider.find_by(issuer: 'urn:gov:gsa:openidconnect:sp:server') click_link t('links.cancel') - expect(current_url).to eq new_user_session_url(request_id: sp_request_id) + expect(page).to have_current_path(new_user_session_path(request_id: sp_request_id)) expect(page).to have_content t('links.back_to_sp', sp: sp.friendly_name) end end @@ -1118,7 +1118,7 @@ confirm_email_in_a_different_browser(email) click_agree_and_continue - expect(current_url).to eq new_user_session_url + expect(page).to have_current_path(new_user_session_path) expect(page) .to have_content t('instructions.go_back_to_mobile_app', friendly_name: 'Example iOS App') end diff --git a/spec/features/openid_connect/phishing_resistant_required_spec.rb b/spec/features/openid_connect/phishing_resistant_required_spec.rb index b3664efeccc..db29d217889 100644 --- a/spec/features/openid_connect/phishing_resistant_required_spec.rb +++ b/spec/features/openid_connect/phishing_resistant_required_spec.rb @@ -64,7 +64,7 @@ sign_in_before_2fa(user) visit_idp_from_ial1_oidc_sp_requesting_aal3(prompt: 'select_account') - expect(current_url).to eq(login_two_factor_piv_cac_url) + expect(page).to have_current_path(login_two_factor_piv_cac_path) click_on t('two_factor_authentication.login_options_link_text') expect(has_2fa_option?(:piv_cac)).to eq(true) expect(has_2fa_option?(:sms)).to eq(false) @@ -78,7 +78,7 @@ sign_in_before_2fa(user) visit_idp_from_ial1_oidc_sp_requesting_aal3(prompt: 'select_account') - expect(current_url).to eq(login_two_factor_webauthn_url) + expect(page).to have_current_path(login_two_factor_webauthn_path) click_on t('two_factor_authentication.login_options_link_text') expect(has_2fa_option?(:webauthn)).to eq(true) expect(has_2fa_option?(:sms)).to eq(false) @@ -92,7 +92,7 @@ sign_in_before_2fa(user) visit_idp_from_ial1_oidc_sp_requesting_aal3(prompt: 'select_account') - expect(current_url).to eq(login_two_factor_webauthn_url(platform: true)) + expect(page).to have_current_path(login_two_factor_webauthn_path(platform: true)) click_on t('two_factor_authentication.login_options_link_text') expect(has_2fa_option?(:webauthn_platform)).to eq(true) expect(has_2fa_option?(:sms)).to eq(false) @@ -103,7 +103,7 @@ sign_in_and_2fa_user(user_with_phishing_resistant_2fa) visit_idp_from_ial1_oidc_sp_requesting_aal3(prompt: 'select_account') - expect(current_url).to eq(login_two_factor_webauthn_url) + expect(page).to have_current_path(login_two_factor_webauthn_path) end context 'adding an ineligible method after authenticating with phishing-resistant' do @@ -142,7 +142,7 @@ sign_in_before_2fa(user) visit_idp_from_ial1_oidc_sp_requesting_phishing_resistant(prompt: 'select_account') - expect(current_url).to eq(login_two_factor_piv_cac_url) + expect(page).to have_current_path(login_two_factor_piv_cac_path) click_on t('two_factor_authentication.login_options_link_text') expect(has_2fa_option?(:piv_cac)).to eq(true) expect(has_2fa_option?(:sms)).to eq(false) @@ -156,7 +156,7 @@ sign_in_before_2fa(user) visit_idp_from_ial1_oidc_sp_requesting_phishing_resistant(prompt: 'select_account') - expect(current_url).to eq(login_two_factor_webauthn_url) + expect(page).to have_current_path(login_two_factor_webauthn_path) click_on t('two_factor_authentication.login_options_link_text') expect(has_2fa_option?(:webauthn)).to eq(true) expect(has_2fa_option?(:sms)).to eq(false) @@ -170,7 +170,7 @@ sign_in_before_2fa(user) visit_idp_from_ial1_oidc_sp_requesting_phishing_resistant(prompt: 'select_account') - expect(current_url).to eq(login_two_factor_webauthn_url(platform: true)) + expect(page).to have_current_path(login_two_factor_webauthn_path(platform: true)) click_on t('two_factor_authentication.login_options_link_text') expect(has_2fa_option?(:webauthn_platform)).to eq(true) expect(has_2fa_option?(:sms)).to eq(false) @@ -181,7 +181,7 @@ sign_in_and_2fa_user(user_with_phishing_resistant_2fa) visit_idp_from_ial1_oidc_sp_requesting_phishing_resistant(prompt: 'select_account') - expect(current_url).to eq(login_two_factor_webauthn_url) + expect(page).to have_current_path(login_two_factor_webauthn_path) end context 'adding an ineligible method after authenticating with phishing-resistant' do @@ -220,7 +220,7 @@ sign_in_before_2fa(user) visit_idp_from_ial1_oidc_sp_defaulting_to_aal3(prompt: 'select_account') - expect(current_url).to eq(login_two_factor_piv_cac_url) + expect(page).to have_current_path(login_two_factor_piv_cac_path) click_on t('two_factor_authentication.login_options_link_text') expect(has_2fa_option?(:piv_cac)).to eq(true) expect(has_2fa_option?(:sms)).to eq(false) @@ -234,7 +234,7 @@ sign_in_before_2fa(user) visit_idp_from_ial1_oidc_sp_defaulting_to_aal3(prompt: 'select_account') - expect(current_url).to eq(login_two_factor_webauthn_url) + expect(page).to have_current_path(login_two_factor_webauthn_path) click_on t('two_factor_authentication.login_options_link_text') expect(has_2fa_option?(:webauthn)).to eq(true) expect(has_2fa_option?(:sms)).to eq(false) @@ -248,7 +248,7 @@ sign_in_before_2fa(user) visit_idp_from_ial1_oidc_sp_defaulting_to_aal3(prompt: 'select_account') - expect(current_url).to eq(login_two_factor_webauthn_url(platform: true)) + expect(page).to have_current_path(login_two_factor_webauthn_path(platform: true)) click_on t('two_factor_authentication.login_options_link_text') expect(has_2fa_option?(:webauthn_platform)).to eq(true) expect(has_2fa_option?(:sms)).to eq(false) @@ -259,7 +259,7 @@ sign_in_and_2fa_user(user_with_phishing_resistant_2fa) visit_idp_from_ial1_oidc_sp_defaulting_to_aal3(prompt: 'select_account') - expect(current_url).to eq(login_two_factor_webauthn_url) + expect(page).to have_current_path(login_two_factor_webauthn_path) end context 'adding an ineligible method after authenticating with phishing-resistant' do diff --git a/spec/features/openid_connect/redirect_uri_validation_spec.rb b/spec/features/openid_connect/redirect_uri_validation_spec.rb index 693bc943522..e7279778fff 100644 --- a/spec/features/openid_connect/redirect_uri_validation_spec.rb +++ b/spec/features/openid_connect/redirect_uri_validation_spec.rb @@ -73,12 +73,12 @@ sp_redirect_uri = "http://localhost:7654/auth/result?error=access_denied&state=#{state}" click_on t('links.back_to_sp', sp: 'Test SP') - expect(current_url).to eq(sp_redirect_uri) + expect(page).to have_current_path(sp_redirect_uri, url: true) visit new_user_session_path(request_id: '123', redirect_uri: 'evil.com') click_on t('links.back_to_sp', sp: 'Test SP') - expect(current_url).to eq(sp_redirect_uri) + expect(page).to have_current_path(sp_redirect_uri, url: true) end end diff --git a/spec/features/remember_device/cookie_expiration_spec.rb b/spec/features/remember_device/cookie_expiration_spec.rb index 7591f1d9df9..7e977553301 100644 --- a/spec/features/remember_device/cookie_expiration_spec.rb +++ b/spec/features/remember_device/cookie_expiration_spec.rb @@ -10,7 +10,7 @@ expire_cookies sign_in_user(user) - expect(current_url).to match(%r{/account}) + expect(page).to have_current_path(account_path) end def sign_in_user_with_remember_device diff --git a/spec/features/remember_device/user_opted_preference_spec.rb b/spec/features/remember_device/user_opted_preference_spec.rb index c6cee94e1cc..27b2d381f3f 100644 --- a/spec/features/remember_device/user_opted_preference_spec.rb +++ b/spec/features/remember_device/user_opted_preference_spec.rb @@ -23,7 +23,7 @@ end it 'requires the user to 2fa again and has an unchecked remember device checkbox upon sign in' do - expect(current_url).to include('/login/two_factor/authenticator') + expect(page).to have_current_path login_two_factor_authenticator_path expect(page).to have_unchecked_field('remember_device') end end @@ -50,7 +50,7 @@ end it 'requires the user to 2fa again and has an unchecked remember device checkbox upon sign in' do - expect(current_url).to include('/login/two_factor/webauthn') + expect(page).to have_current_path login_two_factor_webauthn_path expect(page).to have_unchecked_field('remember_device') end @@ -78,7 +78,7 @@ end it 'requires the user to 2fa again and has an unchecked remember device checkbox upon sign in' do - expect(current_url).to include('login/two_factor/sms') + expect(page).to have_current_path login_two_factor_path(otp_delivery_preference: 'sms') expect(page).to have_unchecked_field('remember_device') end end @@ -98,7 +98,7 @@ end it 'requires the user to 2fa again and has an unchecked remember device checkbox upon sign in' do - expect(current_url).to include('/login/two_factor/authenticator') + expect(page).to have_current_path login_two_factor_authenticator_path expect(page).to have_unchecked_field('remember_device') end end @@ -126,7 +126,7 @@ end it 'requires the user to 2fa again and has an unchecked remember device checkbox upon sign in' do - expect(current_url).to include('login/two_factor/webauthn') + expect(page).to have_current_path login_two_factor_webauthn_path expect(page).to have_unchecked_field('remember_device') end end @@ -145,7 +145,7 @@ end it 'requires the user to 2fa again and has an unchecked remember device checkbox upon sign in' do - expect(current_url).to include('login/two_factor/sms') + expect(page).to have_current_path login_two_factor_path(otp_delivery_preference: 'sms') expect(page).to have_unchecked_field('remember_device') end end diff --git a/spec/features/saml/authorization_confirmation_spec.rb b/spec/features/saml/authorization_confirmation_spec.rb index 98e6eaa90e5..69b5f8885f6 100644 --- a/spec/features/saml/authorization_confirmation_spec.rb +++ b/spec/features/saml/authorization_confirmation_spec.rb @@ -40,11 +40,11 @@ def create_user_and_remember_device sign_in_user(user1, second_email.email) visit request_url - expect(current_url).to match(user_authorization_confirmation_path) + expect(page).to have_current_path(user_authorization_confirmation_path) expect(page).to have_content shared_email continue_as(shared_email) - expect(current_url).to eq(complete_saml_url) + expect(page).to have_current_path complete_saml_path end context 'with requested attributes contains only email' do @@ -92,14 +92,14 @@ def create_user_and_remember_device sign_in_user(user1) visit request_url - expect(current_url).to match(user_authorization_confirmation_path) + expect(page).to have_current_path(user_authorization_confirmation_path) continue_as(user2.email, user2.password) # Can't remember both users' devices? fill_in_code_with_last_phone_otp click_submit_default - expect(current_url).to eq(complete_saml_url) + expect(page).to have_current_path complete_saml_path end it 'does not render an error if a user goes back after opting to switch accounts' do @@ -163,7 +163,7 @@ def create_user_and_remember_device # second visit visit request_url - expect(current_url).to eq(request_url) + expect(page).to have_current_path(request_url, url: true) end it 'redirects to the account page with no sp in session' do @@ -193,7 +193,7 @@ def create_user_and_remember_device click_agree_and_continue - expect(current_url).to eq complete_saml_url + expect(page).to have_current_path complete_saml_path expect(page.get_rack_session.keys).to include('sp') end end diff --git a/spec/features/saml/ial1/account_creation_spec.rb b/spec/features/saml/ial1/account_creation_spec.rb index 34f0d952511..61a2d2c4b83 100644 --- a/spec/features/saml/ial1/account_creation_spec.rb +++ b/spec/features/saml/ial1/account_creation_spec.rb @@ -10,7 +10,7 @@ click_link t('links.create_account') click_link t('links.cancel') - expect(current_url).to eq new_user_session_url(request_id: sp_request_id) + expect(page).to have_current_path(new_user_session_path(request_id: sp_request_id)) end end @@ -22,13 +22,14 @@ click_confirmation_link_in_email('test@test.com') click_link t('links.cancel_account_creation') - expect(current_url).to eq sign_up_cancel_url + expect(page).to have_current_path(sign_up_cancel_path) expect do click_button t('forms.buttons.cancel') end.to change(User, :count).by(-1) - expect(current_url).to eq \ - new_user_session_url(request_id: ServiceProviderRequestProxy.last.uuid) + expect(page).to have_current_path( + new_user_session_path(request_id: ServiceProviderRequestProxy.last.uuid), + ) end it 'redirects to the password page after cancelling the cancellation' do @@ -39,12 +40,12 @@ previous_url = current_url click_link t('links.cancel_account_creation') - expect(current_url).to eq sign_up_cancel_url + expect(page).to have_current_path(sign_up_cancel_path) expect do click_link t('links.go_back') end.to change(User, :count).by(0) - expect(current_url).to eq previous_url + expect(page).to have_current_path(previous_url, url: true) end end end diff --git a/spec/features/saml/ial1_sso_spec.rb b/spec/features/saml/ial1_sso_spec.rb index 5fd8a166e48..df9e6bbd7ba 100644 --- a/spec/features/saml/ial1_sso_spec.rb +++ b/spec/features/saml/ial1_sso_spec.rb @@ -25,7 +25,7 @@ click_agree_and_continue - expect(current_url).to eq complete_saml_url + expect(page).to have_current_path complete_saml_path expect(page.get_rack_session.keys).to include('sp') end end @@ -40,7 +40,7 @@ click_submit_default_twice click_agree_and_continue - expect(current_url).to eq complete_saml_url + expect(page).to have_current_path complete_saml_path visit root_path expect(page).to have_current_path account_path @@ -54,7 +54,7 @@ visit saml_authn_request_url - expect(current_url).to match new_user_session_path + expect(page).to have_current_path new_user_session_path expect(page).to have_content(sp_content) expect(page).to_not have_css('.usa-accordion__heading') end @@ -84,7 +84,7 @@ session['warden.user.user.session']['last_request_at'] = 30.minutes.ago.to_i end - expect(page).to have_current_path(new_user_session_path(request_id: sp_request_id), wait: 5) + expect(page).to have_current_path(new_user_session_path(request_id: sp_request_id)) allow(IdentityConfig.store).to receive(:session_check_delay).and_call_original allow(IdentityConfig.store).to receive(:session_check_frequency).and_call_original @@ -93,7 +93,7 @@ click_submit_default # SAML does internal redirect using JavaScript prior to showing consent screen - expect(page).to have_current_path(sign_up_completed_path, wait: 5) + expect(page).to have_current_path(sign_up_completed_path) click_agree_and_continue expect(page).to have_current_path(test_saml_decode_assertion_path) @@ -113,7 +113,7 @@ it 'redirects user to verify attributes page' do sp = ServiceProvider.find_by(issuer: 'http://localhost:3000') - expect(current_url).to eq(sign_up_completed_url) + expect(page).to have_current_path(sign_up_completed_path) expect(page).to have_content( t( 'titles.sign_up.completion_first_sign_in', @@ -124,7 +124,7 @@ it 'returns to sp after clicking continue' do click_agree_and_continue - expect(current_url).to eq(complete_saml_url) + expect(page).to have_current_path complete_saml_path end it 'it confirms the user wants to continue to the SP after signing in again' do @@ -138,10 +138,10 @@ visit saml_authn_request - expect(current_url).to match(user_authorization_confirmation_path) + expect(page).to have_current_path(user_authorization_confirmation_path) continue_as(user.email) - expect(current_url).to eq(complete_saml_url) + expect(page).to have_current_path complete_saml_path end end @@ -174,7 +174,7 @@ find_link(t('i18n.locale.es'), visible: false).click end - expect(current_url).to eq root_url(locale: :es, trailing_slash: true) + expect(page).to have_current_path(root_path(locale: :es, trailing_slash: true)) expect_branded_experience end end @@ -184,12 +184,12 @@ request_url = saml_authn_request_url visit request_url - expect(current_url).to eq root_url + expect(page).to have_current_path(root_path) expect_branded_experience visit request_url - expect(current_url).to eq root_url + expect(page).to have_current_path(root_path) expect_branded_experience end end @@ -204,7 +204,7 @@ sp = ServiceProvider.find_by(issuer: 'http://localhost:3000') click_link t('links.cancel') - expect(current_url).to eq new_user_session_url(request_id: sp_request_id) + expect(page).to have_current_path(new_user_session_path(request_id: sp_request_id)) expect(page).to have_content t('links.back_to_sp', sp: sp.friendly_name) end end @@ -227,7 +227,7 @@ fill_in_code_with_last_phone_otp click_submit_default - expect(current_url).to match new_user_session_path + expect(page).to have_current_path complete_saml_path click_submit_default click_agree_and_continue click_submit_default @@ -258,7 +258,7 @@ fill_in_code_with_last_phone_otp click_submit_default - expect(current_url).to match new_user_session_path + expect(page).to have_current_path complete_saml_path(locale: 'es') click_submit_default click_agree_and_continue click_submit_default diff --git a/spec/features/saml/ial2_sso_spec.rb b/spec/features/saml/ial2_sso_spec.rb index f5573dbe36b..2aecc83f21a 100644 --- a/spec/features/saml/ial2_sso_spec.rb +++ b/spec/features/saml/ial2_sso_spec.rb @@ -96,7 +96,7 @@ def sign_out_user perform_id_verification_with_gpo_without_confirming_code(user) - expect(current_url).to eq expected_gpo_return_to_sp_url + expect(page).to have_current_path(expected_gpo_return_to_sp_url, url: true) visit account_path click_link(t('account.index.verification.reactivate_button')) diff --git a/spec/features/saml/phishing_resistant_required_spec.rb b/spec/features/saml/phishing_resistant_required_spec.rb index 772a8b4b824..18fbd4ef1d2 100644 --- a/spec/features/saml/phishing_resistant_required_spec.rb +++ b/spec/features/saml/phishing_resistant_required_spec.rb @@ -75,7 +75,7 @@ authn_context: Saml::Idp::Constants::AAL2_PHISHING_RESISTANT_AUTHN_CONTEXT_CLASSREF, }, ) - expect(current_url).to eq(login_two_factor_piv_cac_url) + expect(page).to have_current_path(login_two_factor_piv_cac_path) click_on t('two_factor_authentication.login_options_link_text') expect(has_2fa_option?(:piv_cac)).to eq(true) expect(has_2fa_option?(:sms)).to eq(false) @@ -94,7 +94,7 @@ authn_context: Saml::Idp::Constants::AAL2_PHISHING_RESISTANT_AUTHN_CONTEXT_CLASSREF, }, ) - expect(current_url).to eq(login_two_factor_webauthn_url) + expect(page).to have_current_path(login_two_factor_webauthn_path) click_on t('two_factor_authentication.login_options_link_text') expect(has_2fa_option?(:webauthn)).to eq(true) expect(has_2fa_option?(:sms)).to eq(false) @@ -113,7 +113,7 @@ authn_context: Saml::Idp::Constants::AAL2_PHISHING_RESISTANT_AUTHN_CONTEXT_CLASSREF, }, ) - expect(current_url).to eq(login_two_factor_webauthn_url(platform: true)) + expect(page).to have_current_path(login_two_factor_webauthn_path(platform: true)) click_on t('two_factor_authentication.login_options_link_text') expect(has_2fa_option?(:webauthn_platform)).to eq(true) expect(has_2fa_option?(:sms)).to eq(false) @@ -171,7 +171,7 @@ issuer: sp1_issuer, authn_context: Saml::Idp::Constants::AAL3_AUTHN_CONTEXT_CLASSREF }, ) - expect(current_url).to eq(login_two_factor_piv_cac_url) + expect(page).to have_current_path(login_two_factor_piv_cac_path) click_on t('two_factor_authentication.login_options_link_text') expect(has_2fa_option?(:piv_cac)).to eq(true) expect(has_2fa_option?(:sms)).to eq(false) @@ -189,7 +189,7 @@ issuer: sp1_issuer, authn_context: Saml::Idp::Constants::AAL3_AUTHN_CONTEXT_CLASSREF }, ) - expect(current_url).to eq(login_two_factor_webauthn_url) + expect(page).to have_current_path(login_two_factor_webauthn_path) click_on t('two_factor_authentication.login_options_link_text') expect(has_2fa_option?(:webauthn)).to eq(true) expect(has_2fa_option?(:sms)).to eq(false) @@ -207,7 +207,7 @@ issuer: sp1_issuer, authn_context: Saml::Idp::Constants::AAL3_AUTHN_CONTEXT_CLASSREF }, ) - expect(current_url).to eq(login_two_factor_webauthn_url(platform: true)) + expect(page).to have_current_path(login_two_factor_webauthn_path(platform: true)) click_on t('two_factor_authentication.login_options_link_text') expect(has_2fa_option?(:webauthn_platform)).to eq(true) expect(has_2fa_option?(:sms)).to eq(false) @@ -265,7 +265,7 @@ issuer: aal3_issuer, authn_context: nil }, ) - expect(current_url).to eq(login_two_factor_piv_cac_url) + expect(page).to have_current_path(login_two_factor_piv_cac_path) click_on t('two_factor_authentication.login_options_link_text') expect(has_2fa_option?(:piv_cac)).to eq(true) expect(has_2fa_option?(:sms)).to eq(false) @@ -283,7 +283,7 @@ issuer: aal3_issuer, authn_context: nil }, ) - expect(current_url).to eq(login_two_factor_webauthn_url) + expect(page).to have_current_path(login_two_factor_webauthn_path) click_on t('two_factor_authentication.login_options_link_text') expect(has_2fa_option?(:webauthn)).to eq(true) expect(has_2fa_option?(:sms)).to eq(false) @@ -301,7 +301,7 @@ issuer: aal3_issuer, authn_context: nil }, ) - expect(current_url).to eq(login_two_factor_webauthn_url(platform: true)) + expect(page).to have_current_path(login_two_factor_webauthn_path(platform: true)) click_on t('two_factor_authentication.login_options_link_text') expect(has_2fa_option?(:webauthn_platform)).to eq(true) expect(has_2fa_option?(:sms)).to eq(false) @@ -316,7 +316,7 @@ }, ) - expect(current_url).to eq(login_two_factor_webauthn_url) + expect(page).to have_current_path(login_two_factor_webauthn_path) end context 'adding an ineligible method after authenticating with phishing-resistant' do diff --git a/spec/features/saml/redirect_uri_validation_spec.rb b/spec/features/saml/redirect_uri_validation_spec.rb index a2e0cc06e80..321f2522f5d 100644 --- a/spec/features/saml/redirect_uri_validation_spec.rb +++ b/spec/features/saml/redirect_uri_validation_spec.rb @@ -23,7 +23,7 @@ click_agree_and_continue click_submit_default_twice - expect(current_url).to eq sp.acs_url + expect(page).to have_current_path(sp.acs_url, url: true) end end end diff --git a/spec/features/saml/saml_spec.rb b/spec/features/saml/saml_spec.rb index 049f6a59ad5..cb7fa4c8173 100644 --- a/spec/features/saml/saml_spec.rb +++ b/spec/features/saml/saml_spec.rb @@ -23,7 +23,7 @@ click_agree_and_continue click_submit_default_twice - expect(current_url).to eq sp.acs_url + expect(page).to have_current_path(sp.acs_url, url: true) end end @@ -159,8 +159,8 @@ end it 'redirects to /test/saml/decode_assertion after submitting the form' do - expect(page.current_url) - .to eq(saml_settings.assertion_consumer_service_url) + expect(page) + .to have_current_path(saml_settings.assertion_consumer_service_url, url: true) end it 'stores SP identifier in Identity model' do @@ -518,7 +518,7 @@ ) expect(fake_analytics.events['SAML Auth'].count).to eq 2 - expect(current_url).to eq sp.acs_url + expect(page).to have_current_path(sp.acs_url, url: true) end it 'logs one SAML Auth Requested event and two SAML Auth events for IAL2 request' do @@ -565,7 +565,7 @@ ) expect(fake_analytics.events['SAML Auth'].count).to eq 2 - expect(current_url).to eq sp.acs_url + expect(page).to have_current_path(sp.acs_url, url: true) end end @@ -595,7 +595,7 @@ ) expect(fake_analytics.events['SAML Auth'].count).to eq 2 - expect(current_url).to eq sp.acs_url + expect(page).to have_current_path(sp.acs_url, url: true) end end end diff --git a/spec/features/two_factor_authentication/sign_in_spec.rb b/spec/features/two_factor_authentication/sign_in_spec.rb index 1d4adda1a56..4b1dcc9dd25 100644 --- a/spec/features/two_factor_authentication/sign_in_spec.rb +++ b/spec/features/two_factor_authentication/sign_in_spec.rb @@ -529,17 +529,26 @@ def attempt_to_bypass_2fa sign_in_before_2fa(user) click_link t('links.help'), match: :first - expect(current_url).to eq MarketingSite.help_url + expect(page).to have_current_path( + MarketingSite.help_url, + url: true, + ) visit login_two_factor_path(otp_delivery_preference: 'sms') click_link t('links.contact'), match: :first - expect(current_url).to eq MarketingSite.contact_url + expect(page).to have_current_path( + MarketingSite.contact_url, + url: true, + ) visit login_two_factor_path(otp_delivery_preference: 'sms') click_link t('links.privacy_policy'), match: :first - expect(current_url).to eq MarketingSite.security_and_privacy_practices_url + expect(page).to have_current_path( + MarketingSite.security_and_privacy_practices_url, + url: true, + ) end end diff --git a/spec/features/users/sign_in_spec.rb b/spec/features/users/sign_in_spec.rb index 57dfb7d2e8a..80f7d55cffc 100644 --- a/spec/features/users/sign_in_spec.rb +++ b/spec/features/users/sign_in_spec.rb @@ -64,8 +64,8 @@ click_on t('account.login.piv_cac') fill_in_piv_cac_credentials_and_submit(user, user.piv_cac_configurations.first.x509_dn_uuid) - expect(current_url).to eq rules_of_use_url - accept_rules_of_use_and_continue_if_displayed + expect(page).to have_current_path rules_of_use_path + accept_rules_of_use_and_continue expect(oidc_redirect_url).to start_with service_provider.redirect_uris.first end @@ -93,13 +93,13 @@ click_on t('account.login.piv_cac') fill_in_piv_cac_credentials_and_submit(user, user.piv_cac_configurations.first.x509_dn_uuid) - expect(current_url).to eq capture_password_url + expect(page).to have_current_path(capture_password_path) fill_in 'Password', with: user.password click_submit_default - expect(current_url).to eq rules_of_use_url - accept_rules_of_use_and_continue_if_displayed + expect(page).to have_current_path rules_of_use_path + accept_rules_of_use_and_continue expect(oidc_redirect_url).to start_with service_provider.redirect_uris.first end @@ -538,7 +538,7 @@ fill_in_credentials_and_submit(user.email, user.password) - expect(current_url).to eq new_user_session_url(request_id: '123') + expect(page).to have_current_path(new_user_session_path(request_id: '123')) expect(page).to have_content t('errors.general') end end @@ -675,7 +675,6 @@ it 'signs out the user if they choose to cancel' do user = create(:user, :fully_registered) signin(user.email, user.password) - accept_rules_of_use_and_continue_if_displayed click_link t('two_factor_authentication.login_options_link_text') click_on t('links.cancel') @@ -754,7 +753,7 @@ click_on t('account.login.piv_cac') fill_in_piv_cac_credentials_and_submit(user, 'foo') - expect(current_url).to eq account_url + expect(page).to have_current_path(account_path) Capybara.reset_session! @@ -762,7 +761,7 @@ click_on t('account.login.piv_cac') fill_in_piv_cac_credentials_and_submit(user, 'bar') - expect(current_url).to eq account_url + expect(page).to have_current_path(account_path) end end @@ -846,7 +845,7 @@ click_agree_and_continue - expect(current_url).to eq complete_saml_url + expect(page).to have_current_path complete_saml_path end it 'returns ial2 info for a verified user' do @@ -875,7 +874,7 @@ click_agree_and_continue - expect(current_url).to eq complete_saml_url + expect(page).to have_current_path complete_saml_path end end diff --git a/spec/features/visitors/email_confirmation_spec.rb b/spec/features/visitors/email_confirmation_spec.rb index e38158bcc7c..05dea4ab10d 100644 --- a/spec/features/visitors/email_confirmation_spec.rb +++ b/spec/features/visitors/email_confirmation_spec.rb @@ -13,7 +13,7 @@ check t('sign_up.terms', app_name: APP_NAME) click_submit_default - expect(page).to have_current_path(sign_up_verify_email_url) + expect(page).to have_current_path(sign_up_verify_email_path) expect(page).not_to have_content(t('errors.registration.terms')) end @@ -47,7 +47,7 @@ click_button t('forms.buttons.continue') - expect(current_url).to eq authentication_methods_setup_url + expect(page).to have_current_path(authentication_methods_setup_path) expect(page).to_not have_content t('devise.confirmations.confirmed_but_must_set_password') end @@ -91,7 +91,7 @@ visit sign_up_create_email_confirmation_url(confirmation_token: @raw_confirmation_token) - expect(current_url).to eq account_url + expect(page).to have_current_path(account_path) end end @@ -102,9 +102,9 @@ visit sign_up_create_email_confirmation_url(confirmation_token: @raw_confirmation_token) + expect(page).to have_current_path(new_user_session_path) action = t('devise.confirmations.sign_in') expect(page).to have_content t('devise.confirmations.already_confirmed', action:) - expect(current_url).to eq new_user_session_url end end diff --git a/spec/features/visitors/set_password_spec.rb b/spec/features/visitors/set_password_spec.rb index 6c7edfe9d99..b4bbf7ef0c9 100644 --- a/spec/features/visitors/set_password_spec.rb +++ b/spec/features/visitors/set_password_spec.rb @@ -7,8 +7,8 @@ fill_in t('forms.password'), with: '' click_button t('forms.buttons.continue') + expect(page).to have_current_path sign_up_create_password_path expect(page).to have_content t('errors.messages.blank') - expect(current_url).to eq sign_up_create_password_url end context 'password field is blank when JS is on', js: true do @@ -78,8 +78,8 @@ click_button t('forms.buttons.continue') + expect(page).to have_current_path sign_up_create_password_path expect(page).to have_content('characters') - expect(current_url).to eq sign_up_create_password_url end scenario 'visitor gets password help message' do diff --git a/spec/features/webauthn/hidden_spec.rb b/spec/features/webauthn/hidden_spec.rb index e7c318df07a..90a905ab266 100644 --- a/spec/features/webauthn/hidden_spec.rb +++ b/spec/features/webauthn/hidden_spec.rb @@ -106,7 +106,7 @@ expect(webauthn_option_hidden?).to eq(false) choose t('two_factor_authentication.login_options.webauthn_platform') click_continue - expect(current_url).to eq(login_two_factor_webauthn_url(platform: true)) + expect(page).to have_current_path(login_two_factor_webauthn_path(platform: true)) end context 'if the webauthn credential is not their default mfa method when signing in' do @@ -126,7 +126,7 @@ click_on t('two_factor_authentication.login_options_link_text') choose t('two_factor_authentication.login_options.webauthn_platform') click_continue - expect(current_url).to eq(login_two_factor_webauthn_url(platform: true)) + expect(page).to have_current_path(login_two_factor_webauthn_path(platform: true)) end end end diff --git a/spec/features/webauthn/sign_in_spec.rb b/spec/features/webauthn/sign_in_spec.rb index 78d1e602bcb..99f6986212d 100644 --- a/spec/features/webauthn/sign_in_spec.rb +++ b/spec/features/webauthn/sign_in_spec.rb @@ -80,7 +80,7 @@ mock_webauthn_verification_challenge sign_in_user(user) - expect(current_url).to eq(login_two_factor_webauthn_url(platform: true)) + expect(page).to have_current_path(login_two_factor_webauthn_path(platform: true)) mock_cancelled_webauthn_authentication { click_webauthn_authenticate_button } expect(page).to have_content(t('two_factor_authentication.webauthn_platform_header_text')) @@ -104,7 +104,7 @@ expect(page).to have_current_path(login_two_factor_options_path) select_2fa_option('webauthn_platform', visible: :all) click_continue - expect(current_url).to eq(login_two_factor_webauthn_url(platform: true)) + expect(page).to have_current_path(login_two_factor_webauthn_path(platform: true)) end end end diff --git a/spec/lib/linters/capybara_current_path_equality_linter_spec.rb b/spec/lib/linters/capybara_current_path_equality_linter_spec.rb new file mode 100644 index 00000000000..331f619cc80 --- /dev/null +++ b/spec/lib/linters/capybara_current_path_equality_linter_spec.rb @@ -0,0 +1,66 @@ +require 'rubocop' +require 'rubocop/rspec/cop_helper' +require 'rubocop/rspec/expect_offense' + +require 'rails_helper' +require_relative '../../../lib/linters/capybara_current_path_equality_linter' + +RSpec.describe RuboCop::Cop::IdentityIdp::CapybaraCurrentPathEqualityLinter do + include CopHelper + include RuboCop::RSpec::ExpectOffense + + let(:config) { RuboCop::Config.new } + let(:cop) { RuboCop::Cop::IdentityIdp::CapybaraCurrentPathEqualityLinter.new(config) } + + it 'registers offense when doing equality check on method/variable' do + expect_offense(<<~RUBY) + current_path == a_path + ^^^^^^^^^^^^^^^^^^^^^^ IdentityIdp/CapybaraCurrentPathEqualityLinter: Do not compare equality of `current_path` in Capybara feature specs - instead, use the `have_current_path` matcher on `page` or avoid it entirely + RUBY + end + + it 'registers offense when doing inequality check on method/variable' do + expect_offense(<<~RUBY) + current_path != a_path + ^^^^^^^^^^^^^^^^^^^^^^ IdentityIdp/CapybaraCurrentPathEqualityLinter: Do not compare equality of `current_path` in Capybara feature specs - instead, use the `have_current_path` matcher on `page` or avoid it entirely + RUBY + end + + it 'registers offense when doing equality check on method/variable' do + expect_offense(<<~RUBY) + page.current_path == a_path + ^^^^^^^^^^^^^^^^^^^^^^^^^^^ IdentityIdp/CapybaraCurrentPathEqualityLinter: Do not compare equality of `current_path` in Capybara feature specs - instead, use the `have_current_path` matcher on `page` or avoid it entirely + RUBY + end + + it 'registers offense when doing equality check on method/variable with explicit page call' do + expect_offense(<<~RUBY) + page.current_path == a_path + ^^^^^^^^^^^^^^^^^^^^^^^^^^^ IdentityIdp/CapybaraCurrentPathEqualityLinter: Do not compare equality of `current_path` in Capybara feature specs - instead, use the `have_current_path` matcher on `page` or avoid it entirely + RUBY + end + + it 'registers offense when doing inequality check on method/variable with explicit page call' do + expect_offense(<<~RUBY) + page.current_path != a_path + ^^^^^^^^^^^^^^^^^^^^^^^^^^^ IdentityIdp/CapybaraCurrentPathEqualityLinter: Do not compare equality of `current_path` in Capybara feature specs - instead, use the `have_current_path` matcher on `page` or avoid it entirely + RUBY + end + + it 'registers offense when doing equality check on method/variable on left-hand side' do + expect_offense(<<~RUBY) + a_path == current_path + ^^^^^^^^^^^^^^^^^^^^^^ IdentityIdp/CapybaraCurrentPathEqualityLinter: Do not compare equality of `current_path` in Capybara feature specs - instead, use the `have_current_path` matcher on `page` or avoid it entirely + RUBY + end + + it 'does not register offense for unrelated comparisons' do + expect_no_offenses(<<~RUBY) + 1 == 1 + RUBY + + expect_no_offenses(<<~RUBY) + 1 != 1 + RUBY + end +end diff --git a/spec/lib/linters/capybara_current_url_expect_linter_spec.rb b/spec/lib/linters/capybara_current_url_expect_linter_spec.rb new file mode 100644 index 00000000000..41115daf261 --- /dev/null +++ b/spec/lib/linters/capybara_current_url_expect_linter_spec.rb @@ -0,0 +1,72 @@ +require 'rubocop' +require 'rubocop/rspec/cop_helper' +require 'rubocop/rspec/expect_offense' + +require 'rails_helper' +require_relative '../../../lib/linters/capybara_current_url_expect_linter' + +RSpec.describe RuboCop::Cop::IdentityIdp::CapybaraCurrentUrlExpectLinter do + include CopHelper + include RuboCop::RSpec::ExpectOffense + + let(:config) { RuboCop::Config.new } + let(:cop) { RuboCop::Cop::IdentityIdp::CapybaraCurrentUrlExpectLinter.new(config) } + + it 'registers offense when expecting current_url with eq' do + expect_offense(<<~RUBY) + expect(current_url).to eq root_url + ^^ IdentityIdp/CapybaraCurrentUrlExpectLinter: Do not set an RSpec expectation on `current_url` in Capybara feature specs - instead, use the `have_current_path` matcher on `page` + RUBY + end + + it 'registers an offense with negation' do + expect_offense(<<~RUBY) + expect(current_url).not_to eq root_url + ^^^^^^ IdentityIdp/CapybaraCurrentUrlExpectLinter: Do not set an RSpec expectation on `current_url` in Capybara feature specs - instead, use the `have_current_path` matcher on `page` + RUBY + end + + it 'registers offense when calling expecting current_url with include' do + expect_offense(<<~RUBY) + expect(current_url).to include('/') + ^^ IdentityIdp/CapybaraCurrentUrlExpectLinter: Do not set an RSpec expectation on `current_url` in Capybara feature specs - instead, use the `have_current_path` matcher on `page` + RUBY + end + + it 'registers offense when calling expecting current_url with start_with' do + expect_offense(<<~RUBY) + expect(current_url).to start_with('/') + ^^ IdentityIdp/CapybaraCurrentUrlExpectLinter: Do not set an RSpec expectation on `current_url` in Capybara feature specs - instead, use the `have_current_path` matcher on `page` + RUBY + end + + it 'registers offense when calling expecting current_url with match regular expression' do + expect_offense(<<~RUBY) + expect(current_url).to match /localhost/ + ^^ IdentityIdp/CapybaraCurrentUrlExpectLinter: Do not set an RSpec expectation on `current_url` in Capybara feature specs - instead, use the `have_current_path` matcher on `page` + RUBY + end + + it 'registers offense when calling expecting current_url with match string' do + expect_offense(<<~RUBY) + expect(current_url).to match 'localhost' + ^^ IdentityIdp/CapybaraCurrentUrlExpectLinter: Do not set an RSpec expectation on `current_url` in Capybara feature specs - instead, use the `have_current_path` matcher on `page` + RUBY + end + + it 'does not register offense for correct usage' do + expect_no_offenses(<<~RUBY) + expect(page).to have_current_path(root_path) + RUBY + + expect_no_offenses(<<~RUBY) + expect(page).to have_current_path('http://localhost:4001', url: true) + RUBY + end + + it 'does not register offense for unrelated expectations' do + expect_no_offenses(<<~RUBY) + expect(user.created_at).to eq 3 + RUBY + end +end diff --git a/spec/support/features/doc_auth_helper.rb b/spec/support/features/doc_auth_helper.rb index 45e9a37e7bb..d1a988aa707 100644 --- a/spec/support/features/doc_auth_helper.rb +++ b/spec/support/features/doc_auth_helper.rb @@ -43,8 +43,10 @@ def click_send_link end def complete_doc_auth_steps_before_welcome_step(expect_accessible: false) - visit idv_welcome_url unless current_path == idv_welcome_url - click_idv_continue if current_path == idv_mail_only_warning_path + # rubocop:disable IdentityIdp/CapybaraCurrentPathEqualityLinter + # This should be refactored at some point to not require the path conditional + visit idv_welcome_path unless current_path == idv_welcome_path + # rubocop:enable IdentityIdp/CapybaraCurrentPathEqualityLinter expect_page_to_have_no_accessibility_violations(page) if expect_accessible end @@ -60,11 +62,7 @@ def complete_doc_auth_steps_before_agreement_step(expect_accessible: false) end def complete_agreement_step - find( - 'label', - text: t('doc_auth.instructions.consent', app_name: APP_NAME), - wait: 5, - ).click + check t('doc_auth.instructions.consent', app_name: APP_NAME) click_on t('doc_auth.buttons.continue') end @@ -84,9 +82,9 @@ def complete_hybrid_handoff_step def complete_doc_auth_steps_before_document_capture_step(expect_accessible: false) complete_doc_auth_steps_before_hybrid_handoff_step(expect_accessible: expect_accessible) # JavaScript-enabled mobile devices will skip directly to document capture, so stop as complete. - return if page.current_path == idv_document_capture_path - if IdentityConfig.store.in_person_proofing_opt_in_enabled && - page.current_path == idv_how_to_verify_path + return if page.mode == :headless_chrome_mobile + + if IdentityConfig.store.in_person_proofing_opt_in_enabled click_on t('forms.buttons.continue_online') end complete_hybrid_handoff_step @@ -97,7 +95,7 @@ def complete_up_to_how_to_verify_step_for_opt_in_ipp(remote: true) complete_doc_auth_steps_before_welcome_step complete_welcome_step complete_agreement_step - return if page.current_path == idv_hybrid_handoff_path && remote + return if page.mode != :headless_chrome_mobile && remote if remote click_on t('forms.buttons.continue_online') else diff --git a/spec/support/features/doc_capture_helper.rb b/spec/support/features/doc_capture_helper.rb index 3b796c4f7ff..da8ea9fb357 100644 --- a/spec/support/features/doc_capture_helper.rb +++ b/spec/support/features/doc_capture_helper.rb @@ -32,16 +32,6 @@ def using_doc_capture_session(user = user_with_2fa) end end - def complete_doc_capture_steps_before_document_capture_step(user = user_with_2fa) - complete_doc_capture_steps_before_first_step(user) unless - current_path == idv_hybrid_mobile_document_capture_path - end - - def complete_doc_capture_steps_before_capture_complete_step(user = user_with_2fa) - complete_doc_capture_steps_before_document_capture_step(user) - attach_and_submit_images - end - def mock_doc_captured(user_id, response = DocAuth::Response.new(success: true)) user = User.find(user_id) user.document_capture_sessions.last.store_result_from_response(response) diff --git a/spec/support/features/session_helper.rb b/spec/support/features/session_helper.rb index 3ee33593170..4082561ed26 100644 --- a/spec/support/features/session_helper.rb +++ b/spec/support/features/session_helper.rb @@ -17,8 +17,6 @@ def sign_up_with(email) end def choose_another_security_option(option) - accept_rules_of_use_and_continue_if_displayed - click_link t('two_factor_authentication.login_options_link_text') expect(page).to have_current_path login_two_factor_options_path @@ -243,24 +241,23 @@ def click_send_one_time_code def sign_in_live_with_2fa(user = user_with_2fa) sign_in_user(user) + expect(page).to have_current_path(login_two_factor_path(otp_delivery_preference: 'sms')) uncheck(t('forms.messages.remember_device')) fill_in_code_with_last_phone_otp click_submit_default + expect(page).to_not have_current_path(login_two_factor_path(otp_delivery_preference: 'sms')) user end def fill_in_code_with_last_phone_otp - accept_rules_of_use_and_continue_if_displayed fill_in I18n.t('components.one_time_code_input.label'), with: last_phone_otp end def fill_in_code_with_last_totp(user) - accept_rules_of_use_and_continue_if_displayed fill_in 'code', with: last_totp(user) end - def accept_rules_of_use_and_continue_if_displayed - return unless current_path == rules_of_use_path + def accept_rules_of_use_and_continue check 'rules_of_use_form[terms_accepted]' click_button t('forms.buttons.continue') end @@ -325,43 +322,43 @@ def session_store def sign_up_user_from_sp_without_confirming_email(email) sp_request_id = ServiceProviderRequestProxy.last.uuid - expect(current_url).to eq new_user_session_url + expect(page).to have_current_path(new_user_session_path) expect_branded_experience click_sign_in_from_landing_page_then_click_create_account - expect(current_url).to eq sign_up_email_url + expect(page).to have_current_path sign_up_email_path expect_branded_experience visit_landing_page_and_click_create_account_with_request_id(sp_request_id) - expect(current_url).to eq sign_up_email_url + expect(page).to have_current_path sign_up_email_path expect_branded_experience submit_form_with_invalid_email - expect(current_url).to eq sign_up_email_url + expect(page).to have_current_path sign_up_email_path expect_branded_experience submit_form_with_valid_but_wrong_email - expect(current_url).to eq sign_up_verify_email_url + expect(page).to have_current_path sign_up_verify_email_path expect_branded_experience click_link_to_use_a_different_email - expect(current_url).to eq sign_up_email_url + expect(page).to have_current_path sign_up_email_path expect_branded_experience submit_form_with_valid_email(email) - expect(current_url).to eq sign_up_verify_email_url + expect(page).to have_current_path sign_up_verify_email_path expect(last_email.html_part.body.raw_source).to include "?_request_id=#{sp_request_id}" expect_branded_experience click_link_to_resend_the_email - expect(current_url).to eq sign_up_verify_email_url(resend: true) + expect(page).to have_current_path sign_up_verify_email_path(resend: true) expect_branded_experience attempt_to_confirm_email_with_invalid_token(sp_request_id) @@ -488,6 +485,13 @@ def confirm_email(email) def confirm_email_and_password(email) find_link(t('links.create_account')).click submit_form_with_valid_email(email) + + if I18n.locale != I18n.default_locale + expect(page).to have_current_path(sign_up_verify_email_path(locale: I18n.locale)) + else + expect(page).to have_current_path(sign_up_verify_email_path) + end + click_confirmation_link_in_email(email) submit_form_with_valid_password end diff --git a/spec/support/idv_examples/sp_handoff.rb b/spec/support/idv_examples/sp_handoff.rb index ee43a76e175..8a2c1f901a5 100644 --- a/spec/support/idv_examples/sp_handoff.rb +++ b/spec/support/idv_examples/sp_handoff.rb @@ -173,7 +173,7 @@ def expect_successful_saml_handoff if javascript_enabled? expect(page).to have_current_path test_saml_decode_assertion_path else - expect(current_url).to eq @saml_authn_request + expect(page).to have_current_path(@saml_authn_request, url: true) end expect(xmldoc.phone_number.children.children.to_s).to eq(Phonelib.parse(profile_phone).e164) end diff --git a/spec/support/idv_examples/sp_requested_attributes.rb b/spec/support/idv_examples/sp_requested_attributes.rb index 39541f07e72..b4ca0638725 100644 --- a/spec/support/idv_examples/sp_requested_attributes.rb +++ b/spec/support/idv_examples/sp_requested_attributes.rb @@ -64,12 +64,16 @@ click_submit_default if sp == :oidc - expect(current_url).to include('http://localhost:7654/auth/result') + expect(page).to have_current_path( + 'http://localhost:7654/auth/result', + url: true, + ignore_query: true, + ) elsif sp == :saml if javascript_enabled? expect(page).to have_current_path(test_saml_decode_assertion_path) else - expect(current_url).to include(api_saml_auth_url(path_year: PATH_YEAR)) + expect(page).to have_current_path(api_saml_auth_url(path_year: PATH_YEAR)) end end end diff --git a/spec/support/saml_auth_helper.rb b/spec/support/saml_auth_helper.rb index fcd0ab10632..60173c44685 100644 --- a/spec/support/saml_auth_helper.rb +++ b/spec/support/saml_auth_helper.rb @@ -231,7 +231,7 @@ def login_and_confirm_sp(user, protocol) fill_in_code_with_last_phone_otp protocol == :saml ? click_submit_default_twice : click_submit_default - expect(current_url).to match new_user_session_path + expect(page).to have_current_path(sign_up_completed_path) expect(page).to have_content(t('titles.sign_up.completion_first_sign_in', sp: 'Test SP')) click_agree_and_continue diff --git a/spec/support/shared_examples/account_creation.rb b/spec/support/shared_examples/account_creation.rb index 6ac469d7354..22c1cb8c824 100644 --- a/spec/support/shared_examples/account_creation.rb +++ b/spec/support/shared_examples/account_creation.rb @@ -5,8 +5,8 @@ register_user click_agree_and_continue - if :sp == :saml - expect(current_url).to eq UriService.add_params(@saml_authn_request, locale: :es) + if sp == :saml + expect(page).to have_current_path complete_saml_path(locale: :es) elsif sp == :oidc redirect_uri = URI(oidc_redirect_url) @@ -21,7 +21,7 @@ register_user_with_authenticator_app click_agree_and_continue - expect(current_url).to eq complete_saml_url if sp == :saml + expect(page).to have_current_path complete_saml_path if sp == :saml if sp == :oidc redirect_uri = URI(oidc_redirect_url) @@ -51,9 +51,11 @@ end if sp == :oidc - redirect_uri = URI(current_url) - - expect(redirect_uri.to_s).to start_with('http://localhost:7654/auth/result') + expect(page).to have_current_path( + 'http://localhost:7654/auth/result', + url: true, + ignore_query: true, + ) end end end @@ -64,7 +66,7 @@ register_user_with_piv_cac click_agree_and_continue - expect(current_url).to eq complete_saml_url if sp == :saml + expect(page).to have_current_path complete_saml_path if sp == :saml if sp == :oidc redirect_uri = URI(oidc_redirect_url) @@ -99,9 +101,11 @@ end if sp == :oidc - redirect_uri = URI(current_url) - - expect(redirect_uri.to_s).to start_with('http://localhost:7654/auth/result') + expect(page).to have_current_path( + 'http://localhost:7654/auth/result', + url: true, + ignore_query: true, + ) end end end @@ -122,7 +126,7 @@ continue_as(first_email) - expect(current_url).to eq complete_saml_url if sp == :saml + expect(page).to have_current_path complete_saml_path if sp == :saml if sp == :oidc redirect_uri = URI(oidc_redirect_url) @@ -143,7 +147,7 @@ continue_as(second_email) - expect(current_url).to eq complete_saml_url if sp == :saml + expect(page).to have_current_path complete_saml_path if sp == :saml if sp == :oidc redirect_uri = URI(oidc_redirect_url) diff --git a/spec/support/shared_examples/sign_in.rb b/spec/support/shared_examples/sign_in.rb index 9c0867a4f7f..2a15d57447b 100644 --- a/spec/support/shared_examples/sign_in.rb +++ b/spec/support/shared_examples/sign_in.rb @@ -10,12 +10,12 @@ fill_in_code_with_last_phone_otp sp == :saml ? click_submit_default_twice : click_submit_default - expect(current_url).to eq(sign_up_completed_url(locale: 'es')) + expect(page).to have_current_path(sign_up_completed_path(locale: 'es')) click_agree_and_continue if sp == :saml - expect(current_url).to eq UriService.add_params(complete_saml_url, locale: 'es') + expect(page).to have_current_path(complete_saml_url(locale: 'es')) elsif sp == :oidc redirect_uri = URI(oidc_redirect_url) @@ -47,9 +47,9 @@ fill_in_credentials_and_submit(user.email, user.password) fill_in_code_with_last_phone_otp click_submit_default - click_submit_default if current_path == complete_saml_path + click_submit_default if sp == :saml click_agree_and_continue - click_submit_default if current_path == complete_saml_path + click_submit_default if sp == :saml expect(analytics).to have_logged_event( 'SP redirect initiated', @@ -85,7 +85,7 @@ click_continue continue_as - expect(current_url).to eq complete_saml_url if sp == :saml + expect(page).to have_current_path complete_saml_path if sp == :saml if sp == :oidc redirect_uri = URI(oidc_redirect_url) @@ -146,7 +146,7 @@ click_submit_default click_agree_and_continue - expect(current_url).to eq complete_saml_url if sp == :saml + expect(page).to have_current_path complete_saml_path if sp == :saml if sp == :oidc redirect_uri = URI(oidc_redirect_url) @@ -167,7 +167,6 @@ fill_in_credentials_and_submit(user.email, new_password) fill_in_code_with_last_phone_otp click_submit_default - click_submit_default if current_path == complete_saml_path expect(page).to have_current_path reactivate_account_path @@ -181,7 +180,7 @@ click_agree_and_continue - expect(current_url).to eq complete_saml_url if sp == :saml + expect(page).to have_current_path complete_saml_path if sp == :saml if sp == :oidc redirect_uri = URI(oidc_redirect_url) @@ -403,7 +402,7 @@ def ial1_sign_in_with_personal_key_goes_to_sp(sp) click_submit_default click_agree_and_continue - expect(current_url).to eq complete_saml_url if sp == :saml + expect(page).to have_current_path complete_saml_path if sp == :saml return unless sp == :oidc @@ -436,7 +435,7 @@ def ial2_sign_in_with_piv_cac_goes_to_sp(sp) fill_in_piv_cac_credentials_and_submit(user) # capture password before redirecting to SP - expect(current_url).to eq capture_password_url + expect(page).to have_current_path(capture_password_path) fill_in_password_and_submit(user.password) # With JavaScript disabled, user needs to manually click button to proceed @@ -475,7 +474,7 @@ def no_authn_context_sign_in_with_piv_cac_goes_to_sp(sp) fill_in_piv_cac_credentials_and_submit(user) # capture password before redirecting to SP - expect(current_url).to eq capture_password_url + expect(page).to have_current_path(capture_password_path) fill_in_password_and_submit(user.password) # With JavaScript disabled, user needs to manually click button to proceed @@ -500,7 +499,7 @@ def ial2_sign_in_with_piv_cac_gets_sign_in_failure_error(sp) click_on t('account.login.piv_cac') fill_in_piv_cac_credentials_and_submit(user) - expect(current_url).to eq capture_password_url + expect(page).to have_current_path(capture_password_path) max_allowed_attempts = IdentityConfig.store.password_max_attempts (max_allowed_attempts - 1).times do