diff --git a/.reek b/.reek index d525639f3ea..0d9ef661906 100644 --- a/.reek +++ b/.reek @@ -95,22 +95,7 @@ UtilityFunction: exclude: - complete_idv_session DuplicateMethodCall: - exclude: - - complete_2fa_confirmation - - complete_idv_profile - - expect_current_address_in_profile - - stub_subject - - stub_idv_session - - saml_settings - - sign_up_and_2fa - - raw_xml_response - - rotate_attribute_encryption_key - - rotate_attribute_encryption_key_with_invalid_queue - - rotate_hmac_key - - sign_in_user - - stub_auth - - stub_sign_in - - stub_verify_steps_one_and_two + enabled: false FeatureEnvy: enabled: false NestedIterators: diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index ea4ea0fb654..30aeae40614 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -81,7 +81,11 @@ def decorated_user end def after_sign_in_path_for(user) - stored_location_for(user) || sp_session[:request_url] || profile_path + stored_location_for(user) || sp_session[:request_url] || signed_in_path + end + + def signed_in_path + user_fully_authenticated? ? profile_path : user_two_factor_authentication_path end def render_401 diff --git a/app/controllers/sign_up/completions_controller.rb b/app/controllers/sign_up/completions_controller.rb index de7b22af7e0..40da88f8992 100644 --- a/app/controllers/sign_up/completions_controller.rb +++ b/app/controllers/sign_up/completions_controller.rb @@ -18,7 +18,7 @@ def update Analytics::USER_REGISTRATION_AGENCY_HANDOFF_COMPLETE, service_provider_attributes ) - redirect_to after_sign_in_path_for(current_user) + redirect_to sp_session[:request_url] end private diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb index c462eaedf12..eb255c02d62 100644 --- a/app/controllers/users/sessions_controller.rb +++ b/app/controllers/users/sessions_controller.rb @@ -17,14 +17,11 @@ def new def create track_authentication_attempt(params[:user][:email]) - if current_user && user_locked_out?(current_user) - render 'two_factor_authentication/shared/max_login_attempts_reached' - sign_out - return - end + return process_locked_out_user if current_user && user_locked_out?(current_user) super cache_active_profile + store_sp_metadata_in_session unless request_id.empty? end def active @@ -53,6 +50,11 @@ def check_user_needs_redirect end end + def process_locked_out_user + render 'two_factor_authentication/shared/max_login_attempts_reached' + sign_out + end + def now @_now ||= Time.zone.now end @@ -91,13 +93,21 @@ def cache_active_profile end def user_signed_in_and_not_locked_out?(user) - return false unless current_user.present? - + return false unless current_user !user_locked_out?(user) end def user_locked_out?(user) UserDecorator.new(user).blocked_from_entering_2fa_code? end + + def store_sp_metadata_in_session + return if sp_session[:issuer] + StoreSpMetadataInSession.new(session: session, request_id: request_id).call + end + + def request_id + params[:user].fetch(:request_id, '') + end end end diff --git a/app/services/store_sp_metadata_in_session.rb b/app/services/store_sp_metadata_in_session.rb new file mode 100644 index 00000000000..449bd0f511a --- /dev/null +++ b/app/services/store_sp_metadata_in_session.rb @@ -0,0 +1,27 @@ +class StoreSpMetadataInSession + def initialize(session:, request_id:) + @session = session + @request_id = request_id + end + + def call + session[:sp] = { + issuer: sp_request.issuer, + loa3: loa3_requested?, + request_url: sp_request.url, + request_id: sp_request.uuid, + } + end + + private + + attr_reader :session, :request_id + + def sp_request + @sp_request ||= ServiceProviderRequest.find_by(uuid: request_id) + end + + def loa3_requested? + sp_request.loa == Saml::Idp::Constants::LOA3_AUTHN_CONTEXT_CLASSREF + end +end diff --git a/app/views/devise/sessions/new.html.slim b/app/views/devise/sessions/new.html.slim index f87599139b6..26dd46ca76c 100644 --- a/app/views/devise/sessions/new.html.slim +++ b/app/views/devise/sessions/new.html.slim @@ -7,6 +7,7 @@ h1.h3.my0 = decorated_session.new_session_heading html: { autocomplete: 'off', role: 'form' }) do |f| = f.input :email, required: true, input_html: { class: 'mb4' } = f.input :password, required: true + = f.input :request_id, as: :hidden, input_html: { value: params[:request_id] } = f.button :submit, t('links.next'), class: 'btn-wide' - link = link_to t('notices.sign_in_consent.link'), MarketingSite.privacy_url, target: '_blank' diff --git a/spec/controllers/sign_up/completions_controller_spec.rb b/spec/controllers/sign_up/completions_controller_spec.rb index b0cce1d8288..9df0fa02b4c 100644 --- a/spec/controllers/sign_up/completions_controller_spec.rb +++ b/spec/controllers/sign_up/completions_controller_spec.rb @@ -63,7 +63,10 @@ context 'LOA1' do it 'tracks analytics' do stub_sign_in - subject.session[:sp] = { loa3: false } + subject.session[:sp] = { + loa3: false, + request_url: 'http://example.com', + } patch :update @@ -78,7 +81,10 @@ it 'tracks analytics' do user = create(:user, profiles: [create(:profile, :verified, :active)]) stub_sign_in(user) - subject.session[:sp] = { loa3: true } + subject.session[:sp] = { + loa3: true, + request_url: 'http://example.com', + } patch :update diff --git a/spec/features/saml/loa1_sso_spec.rb b/spec/features/saml/loa1_sso_spec.rb index 3b2edfe82a2..efa19f3f133 100644 --- a/spec/features/saml/loa1_sso_spec.rb +++ b/spec/features/saml/loa1_sso_spec.rb @@ -74,6 +74,22 @@ expect(current_path).to eq sign_up_completed_path end + + it 'after session timeout, signing in takes user back to SP' do + allow(FeatureManagement).to receive(:prefill_otp_codes?).and_return(true) + + user = create(:user, :signed_up) + saml_authn_request = auth_request.create(saml_settings) + + visit saml_authn_request + sp_request_id = ServiceProviderRequest.last.uuid + page.set_rack_session(sp: {}) + visit new_user_session_url(request_id: sp_request_id) + fill_in_credentials_and_submit(user.email, user.password) + click_submit_default + + expect(current_url).to eq saml_authn_request + end end context 'fully signed up user is signed in with email and password only' do diff --git a/spec/support/controller_helper.rb b/spec/support/controller_helper.rb index 4b5559ece6e..e869b894db2 100644 --- a/spec/support/controller_helper.rb +++ b/spec/support/controller_helper.rb @@ -25,6 +25,7 @@ def sign_in_before_2fa(user = create(:user, :signed_up)) sign_in_as_user(user) controller.current_user.send_new_otp allow(controller).to receive(:user_fully_authenticated?).and_return(false) + allow(controller).to receive(:signed_in_path).and_return(profile_path) end def stub_sign_in(user = build(:user, password: VALID_PASSWORD)) @@ -42,6 +43,7 @@ def stub_sign_in_before_2fa(user = build(:user, password: VALID_PASSWORD)) allow(request.env['warden']).to receive(:session).and_return(user: {}) allow(controller).to receive(:current_user).and_return(user) allow(controller).to receive(:user_fully_authenticated?).and_return(false) + allow(controller).to receive(:signed_in_path).and_return(profile_path) end def stub_verify_steps_one_and_two(user)