Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions app/controllers/users/sessions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ class SessionsController < Devise::SessionsController

skip_before_action :session_expires_at, only: [:active]
skip_before_action :require_no_authentication, only: [:new]
before_action :store_sp_metadata_in_session, only: [:new]
before_action :check_user_needs_redirect, only: [:new]
before_action :apply_secure_headers_override, only: [:new]
before_action :configure_permitted_parameters, only: [:new]
Expand Down Expand Up @@ -88,7 +89,6 @@ def process_locked_out_user
def handle_valid_authentication
sign_in(resource_name, resource)
cache_active_profile
store_sp_metadata_in_session unless request_id.empty?
redirect_to user_two_factor_authentication_url
end

Expand Down Expand Up @@ -118,6 +118,7 @@ def track_authentication_attempt(email)
user_id: user.uuid,
user_locked_out: user_locked_out?(user),
stored_location: session['user_return_to'],
sp_request_url_present: sp_session[:request_url].present?,
}

analytics.track_event(Analytics::EMAIL_AND_PASSWORD_AUTH, properties)
Expand All @@ -144,12 +145,12 @@ def user_locked_out?(user)
end

def store_sp_metadata_in_session
return if sp_session[:issuer]
return if sp_session[:issuer] || request_id.empty?
StoreSpMetadataInSession.new(session: session, request_id: request_id).call
end

def request_id
params[:user].fetch(:request_id, '')
params.fetch(:request_id, '')
end
end
end
22 changes: 22 additions & 0 deletions spec/controllers/users/sessions_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@
user_id: user.uuid,
user_locked_out: false,
stored_location: 'http://example.com',
sp_request_url_present: false,
}

expect(@analytics).to receive(:track_event).
Expand All @@ -178,6 +179,7 @@
user_id: user.uuid,
user_locked_out: false,
stored_location: nil,
sp_request_url_present: false,
}

expect(@analytics).to receive(:track_event).
Expand All @@ -193,6 +195,7 @@
user_id: 'anonymous-uuid',
user_locked_out: false,
stored_location: nil,
sp_request_url_present: false,
}

expect(@analytics).to receive(:track_event).
Expand All @@ -214,6 +217,7 @@
user_id: user.uuid,
user_locked_out: true,
stored_location: nil,
sp_request_url_present: false,
}

expect(@analytics).to receive(:track_event).
Expand All @@ -222,6 +226,23 @@
post :create, params: { user: { email: user.email.upcase, password: user.password } }
end

it 'tracks the presence of SP request_url in session' do
subject.session[:sp] = { request_url: 'http://example.com' }
stub_analytics
analytics_hash = {
success: false,
user_id: 'anonymous-uuid',
user_locked_out: false,
stored_location: nil,
sp_request_url_present: true,
}

expect(@analytics).to receive(:track_event).
with(Analytics::EMAIL_AND_PASSWORD_AUTH, analytics_hash)

post :create, params: { user: { email: 'foo@example.com', password: 'password' } }
end

context 'LOA1 user' do
it 'computes one SCrypt hash for the user password' do
user = create(:user, :signed_up)
Expand Down Expand Up @@ -281,6 +302,7 @@
user_id: user.uuid,
user_locked_out: false,
stored_location: nil,
sp_request_url_present: false,
}

expect(@analytics).to receive(:track_event).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

feature 'Remembering a 2FA device' do
include IdvHelper
include SamlAuthHelper

before do
allow(FeatureManagement).to receive(:prefill_otp_codes?).and_return(true)
Expand All @@ -22,6 +23,7 @@ def remember_device_and_sign_out_user
end

it_behaves_like 'remember device'
it_behaves_like 'remember device after being idle on sign in page'
end

context 'sign up' do
Expand Down
33 changes: 33 additions & 0 deletions spec/support/shared_examples/remember_device.rb
Original file line number Diff line number Diff line change
Expand Up @@ -83,3 +83,36 @@
expect(current_url).to start_with('http://localhost:7654/auth/result')
end
end

shared_examples 'remember device after being idle on sign in page' do
it 'redirects to the OIDC SP even though session is deleted' do
# We want to simulate a user that has already visited an OIDC SP and that
# has checked "remember me for 30 days", such that the next URL the app will
# redirect to after signing in with email and password is the SP redirect
# URI.
user = remember_device_and_sign_out_user
IdentityLinker.new(
user, 'urn:gov:gsa:openidconnect:sp:server'
).link_identity(verified_attributes: %w[email])

visit_idp_from_sp_with_loa1(:oidc)
request_id = ServiceProviderRequest.last.uuid
click_link t('links.sign_in')

Timecop.travel(Devise.timeout_in + 1.minute) do
# Simulate being idle on the sign in page long enough for the session to
# be deleted from Redis, but since Redis doesn't respect Timecop, we need
# to expire the session manually.
session_store.send(:destroy_session_from_sid, session_cookie.value)
# Simulate refreshing the page with JS to avoid a CSRF error
visit new_user_session_url(request_id: request_id)

expect(page.response_headers['Content-Security-Policy']).
to(include('form-action \'self\' http://localhost:7654/auth/result'))

fill_in_credentials_and_submit(user.email, user.password)

expect(current_url).to start_with('http://localhost:7654/auth/result')
end
end
end