From d7ff0f9e368298c834335d9b57b0572591eb7e80 Mon Sep 17 00:00:00 2001 From: Douglas Price Date: Tue, 18 Oct 2022 16:34:20 -0400 Subject: [PATCH 1/2] LG-7703: Force a user to re-verify their identity for IRS. If a user proofed their identity with a non-IRS SP, then visits an IRS SP, they will be forced to reproof. changelog: Improvements, Identity Verification, Require users to re-proof on first IRS visit. --- app/controllers/concerns/idv_session.rb | 1 + app/controllers/idv_controller.rb | 3 +- .../authorization_controller.rb | 1 + app/decorators/user_decorator.rb | 9 ++- spec/factories/service_providers.rb | 8 +++ spec/features/users/sign_in_irs_spec.rb | 67 +++++++++++++++++++ 6 files changed, 86 insertions(+), 3 deletions(-) create mode 100644 spec/features/users/sign_in_irs_spec.rb diff --git a/app/controllers/concerns/idv_session.rb b/app/controllers/concerns/idv_session.rb index 054d8ea5332..e4197f3c5e4 100644 --- a/app/controllers/concerns/idv_session.rb +++ b/app/controllers/concerns/idv_session.rb @@ -14,6 +14,7 @@ def confirm_idv_session_started def confirm_idv_needed return if effective_user.active_profile.blank? || decorated_session.requested_more_recent_verification? || + effective_user.decorate.reproof_for_irs?(service_provider: current_sp) || strict_ial2_upgrade_required? redirect_to idv_activated_url diff --git a/app/controllers/idv_controller.rb b/app/controllers/idv_controller.rb index e31ef3450b2..a8b1cc27259 100644 --- a/app/controllers/idv_controller.rb +++ b/app/controllers/idv_controller.rb @@ -7,7 +7,8 @@ class IdvController < ApplicationController before_action :profile_needs_reactivation?, only: [:index] def index - if decorated_session.requested_more_recent_verification? + if decorated_session.requested_more_recent_verification? || + current_user.decorate.reproof_for_irs?(service_provider: current_sp) verify_identity elsif active_profile? && !strict_ial2_upgrade_required? redirect_to idv_activated_url diff --git a/app/controllers/openid_connect/authorization_controller.rb b/app/controllers/openid_connect/authorization_controller.rb index 7cc78654249..8a203fe3d70 100644 --- a/app/controllers/openid_connect/authorization_controller.rb +++ b/app/controllers/openid_connect/authorization_controller.rb @@ -100,6 +100,7 @@ def identity_needs_verification? ((@authorize_form.ial2_requested? || @authorize_form.ial2_strict_requested?) && (current_user.decorate.identity_not_verified? || decorated_session.requested_more_recent_verification?)) || + current_user.decorate.reproof_for_irs?(service_provider: current_sp) || identity_needs_strict_ial2_verification? end diff --git a/app/decorators/user_decorator.rb b/app/decorators/user_decorator.rb index d24713f5904..d477dfa105f 100644 --- a/app/decorators/user_decorator.rb +++ b/app/decorators/user_decorator.rb @@ -57,8 +57,13 @@ def identity_not_verified? !identity_verified? end - def identity_verified? - user.active_profile.present? + def identity_verified?(service_provider: nil) + user.active_profile.present? && !reproof_for_irs?(service_provider: service_provider) + end + + def reproof_for_irs?(service_provider:) + service_provider&.irs_attempts_api_enabled && + !user.active_profile&.initiating_service_provider&.irs_attempts_api_enabled end def active_profile_newer_than_pending_profile? diff --git a/spec/factories/service_providers.rb b/spec/factories/service_providers.rb index 2d5371f3073..bd27d47121e 100644 --- a/spec/factories/service_providers.rb +++ b/spec/factories/service_providers.rb @@ -31,6 +31,14 @@ end end + trait :irs do + friendly_name { 'An IRS Service Provider' } + ial { 2 } + active { true } + irs_attempts_api_enabled { true } + redirect_uris { ['http://localhost:7654/auth/result'] } + end + factory :service_provider_without_help_text, traits: [:without_help_text] end end diff --git a/spec/features/users/sign_in_irs_spec.rb b/spec/features/users/sign_in_irs_spec.rb new file mode 100644 index 00000000000..483a63a6c61 --- /dev/null +++ b/spec/features/users/sign_in_irs_spec.rb @@ -0,0 +1,67 @@ +require 'rails_helper' + +feature 'Sign in to the IRS' do + before(:all) do + @original_capyabara_wait = Capybara.default_max_wait_time + Capybara.default_max_wait_time = 5 + end + + after(:all) do + Capybara.default_max_wait_time = @original_capyabara_wait + end + + include IdvHelper + + let(:irs) { create(:service_provider, :irs) } + let(:other_irs) { create(:service_provider, :irs) } + let(:not_irs) { create(:service_provider, active: true, ial: 2) } + + let(:initiating_service_provider_issuer) { irs.issuer } + + let(:user) do + create( + :profile, :active, :verified, + pii: { first_name: 'John', ssn: '111223333' }, + initiating_service_provider_issuer: initiating_service_provider_issuer + ).user + end + + context 'user verified with IRS returns to IRS' do + context 'user visits the same IRS SP they verified with' do + it "accepts the user's identity as verified" do + visit_idp_from_oidc_sp_with_ial2(client_id: irs.issuer) + fill_in_credentials_and_submit(user.email, user.password) + fill_in_code_with_last_phone_otp + click_submit_default + click_agree_and_continue + + expect(current_url).to start_with('http://localhost:7654/auth/result') + end + end + + context 'user visits different IRS SP than the one they verified with' do + it "accepts the user's identity as verified" do + visit_idp_from_oidc_sp_with_ial2(client_id: other_irs.issuer) + fill_in_credentials_and_submit(user.email, user.password) + fill_in_code_with_last_phone_otp + click_submit_default + click_agree_and_continue + + expect(current_url).to start_with('http://localhost:7654/auth/result') + end + end + end + + context 'user verified with other agency signs in to IRS' do + let(:initiating_service_provider_issuer) { not_irs.issuer } + + it 'forces the user to re-verify their identity' do + visit_idp_from_oidc_sp_with_ial2(client_id: irs.issuer) + fill_in_credentials_and_submit(user.email, user.password) + fill_in_code_with_last_phone_otp + click_submit_default + + expect(current_path).to eq(idv_doc_auth_step_path(step: :welcome)) + end + end +end From b877d1f8d82ca154de2214151556e2629eeee238 Mon Sep 17 00:00:00 2001 From: Douglas Price Date: Wed, 19 Oct 2022 16:18:02 -0400 Subject: [PATCH 2/2] working on the SAML side --- .../concerns/saml_idp_auth_concern.rb | 4 +- spec/features/users/sign_in_irs_spec.rb | 84 ++++++++++++++----- spec/support/features/idv_helper.rb | 42 +++++----- 3 files changed, 88 insertions(+), 42 deletions(-) diff --git a/app/controllers/concerns/saml_idp_auth_concern.rb b/app/controllers/concerns/saml_idp_auth_concern.rb index 3d655883ef0..5acf7eb9b33 100644 --- a/app/controllers/concerns/saml_idp_auth_concern.rb +++ b/app/controllers/concerns/saml_idp_auth_concern.rb @@ -115,7 +115,9 @@ def link_identity_from_session_data end def identity_needs_verification? - ial2_requested? && current_user.decorate.identity_not_verified? + ial2_requested? && + (current_user.decorate.identity_not_verified? || + current_user.decorate.reproof_for_irs?(service_provider: current_sp)) end def_delegators :ial_context, :ial2_requested? diff --git a/spec/features/users/sign_in_irs_spec.rb b/spec/features/users/sign_in_irs_spec.rb index 483a63a6c61..3af9cbdedea 100644 --- a/spec/features/users/sign_in_irs_spec.rb +++ b/spec/features/users/sign_in_irs_spec.rb @@ -11,6 +11,7 @@ end include IdvHelper + include SamlAuthHelper let(:irs) { create(:service_provider, :irs) } let(:other_irs) { create(:service_provider, :irs) } @@ -26,42 +27,81 @@ ).user end - context 'user verified with IRS returns to IRS' do - context 'user visits the same IRS SP they verified with' do - it "accepts the user's identity as verified" do - visit_idp_from_oidc_sp_with_ial2(client_id: irs.issuer) - fill_in_credentials_and_submit(user.email, user.password) - fill_in_code_with_last_phone_otp - click_submit_default - click_agree_and_continue + context 'OIDC' do + context 'user verified with IRS returns to IRS' do + context 'user visits the same IRS SP they verified with' do + it "accepts the user's identity as verified" do + visit_idp_from_oidc_sp_with_ial2(client_id: irs.issuer) + fill_in_credentials_and_submit(user.email, user.password) + fill_in_code_with_last_phone_otp + click_submit_default + + expect(current_path).to eq(sign_up_completed_path) + end + end + + context 'user visits different IRS SP than the one they verified with' do + it "accepts the user's identity as verified" do + visit_idp_from_oidc_sp_with_ial2(client_id: other_irs.issuer) + fill_in_credentials_and_submit(user.email, user.password) + fill_in_code_with_last_phone_otp + click_submit_default - expect(current_url).to start_with('http://localhost:7654/auth/result') + expect(current_path).to eq(sign_up_completed_path) + end end end - context 'user visits different IRS SP than the one they verified with' do - it "accepts the user's identity as verified" do - visit_idp_from_oidc_sp_with_ial2(client_id: other_irs.issuer) + context 'user verified with other agency signs in to IRS' do + let(:initiating_service_provider_issuer) { not_irs.issuer } + + it 'forces the user to re-verify their identity' do + visit_idp_from_oidc_sp_with_ial2(client_id: irs.issuer) fill_in_credentials_and_submit(user.email, user.password) fill_in_code_with_last_phone_otp click_submit_default - click_agree_and_continue - expect(current_url).to start_with('http://localhost:7654/auth/result') + expect(current_path).to eq(idv_doc_auth_step_path(step: :welcome)) end end end - context 'user verified with other agency signs in to IRS' do - let(:initiating_service_provider_issuer) { not_irs.issuer } + context 'SAML', js: true do + context 'user verified with IRS returns to IRS' do + context 'user visits the same IRS SP they verified with' do + it "accepts the user's identity as verified" do + visit_idp_from_saml_sp_with_ial2(issuer: irs.issuer) + fill_in_credentials_and_submit(user.email, user.password) + fill_in_code_with_last_phone_otp + click_submit_default + + expect(current_path).to eq(sign_up_completed_path) + end + end + + context 'user visits different IRS SP than the one they verified with' do + it "accepts the user's identity as verified" do + visit_idp_from_saml_sp_with_ial2(issuer: other_irs.issuer) + fill_in_credentials_and_submit(user.email, user.password) + fill_in_code_with_last_phone_otp + click_submit_default + + expect(current_path).to eq(sign_up_completed_path) + end + end + end - it 'forces the user to re-verify their identity' do - visit_idp_from_oidc_sp_with_ial2(client_id: irs.issuer) - fill_in_credentials_and_submit(user.email, user.password) - fill_in_code_with_last_phone_otp - click_submit_default + context 'user verified with other agency signs in to IRS' do + let(:initiating_service_provider_issuer) { not_irs.issuer } - expect(current_path).to eq(idv_doc_auth_step_path(step: :welcome)) + it 'forces the user to re-verify their identity' do + visit_idp_from_saml_sp_with_ial2(issuer: irs.issuer) + fill_in_credentials_and_submit(user.email, user.password) + fill_in_code_with_last_phone_otp + click_submit_default + + expect(current_path).to eq(idv_doc_auth_step_path(step: :welcome)) + end end end end diff --git a/spec/support/features/idv_helper.rb b/spec/support/features/idv_helper.rb index 238e1890048..672e7add34c 100644 --- a/spec/support/features/idv_helper.rb +++ b/spec/support/features/idv_helper.rb @@ -54,25 +54,7 @@ def choose_idv_otp_delivery_method_voice def visit_idp_from_sp_with_ial2(sp, **extra) if sp == :saml - saml_overrides = { - issuer: sp1_issuer, - authn_context: [ - Saml::Idp::Constants::IAL2_AUTHN_CONTEXT_CLASSREF, - "#{Saml::Idp::Constants::REQUESTED_ATTRIBUTES_CLASSREF}first_name:last_name email, ssn", - "#{Saml::Idp::Constants::REQUESTED_ATTRIBUTES_CLASSREF}phone", - ], - security: { - embed_sign: false, - }, - } - if javascript_enabled? - service_provider = ServiceProvider.find_by(issuer: sp1_issuer) - acs_url = URI.parse(service_provider.acs_url) - acs_url.host = page.server.host - acs_url.port = page.server.port - service_provider.update(acs_url: acs_url.to_s) - end - visit_saml_authn_request_url(overrides: saml_overrides) + visit_idp_from_saml_sp_with_ial2 elsif sp == :oidc @state = SecureRandom.hex @client_id = sp_oidc_issuer @@ -97,6 +79,28 @@ def service_provider_issuer(sp) end end + def visit_idp_from_saml_sp_with_ial2(issuer: sp1_issuer) + saml_overrides = { + issuer: issuer, + authn_context: [ + Saml::Idp::Constants::IAL2_AUTHN_CONTEXT_CLASSREF, + "#{Saml::Idp::Constants::REQUESTED_ATTRIBUTES_CLASSREF}first_name:last_name email, ssn", + "#{Saml::Idp::Constants::REQUESTED_ATTRIBUTES_CLASSREF}phone", + ], + security: { + embed_sign: false, + }, + } + if javascript_enabled? + service_provider = ServiceProvider.find_by(issuer: sp1_issuer) + acs_url = URI.parse(service_provider.acs_url) + acs_url.host = page.server.host + acs_url.port = page.server.port + service_provider.update(acs_url: acs_url.to_s) + end + visit_saml_authn_request_url(overrides: saml_overrides) + end + def visit_idp_from_oidc_sp_with_ial2( client_id: sp_oidc_issuer, state: SecureRandom.hex,