Skip to content
6 changes: 3 additions & 3 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ def cache_issuer_in_cookie
else
{
value: current_sp.issuer,
expires: IdentityConfig.store.session_timeout_in_minutes.minutes,
expires: IdentityConfig.store.session_timeout_in_seconds.seconds,
}
end
end
Expand All @@ -162,12 +162,12 @@ def redirect_with_flash_if_timeout
flash[:info] = t(
'notices.session_timedout',
app_name: APP_NAME,
minutes: IdentityConfig.store.session_timeout_in_minutes,
minutes: IdentityConfig.store.session_timeout_in_seconds.seconds.in_minutes.to_i,
)
elsif current_user.blank?
flash[:info] = t(
'notices.session_cleared',
minutes: IdentityConfig.store.session_timeout_in_minutes,
minutes: IdentityConfig.store.session_timeout_in_seconds.seconds.in_minutes.to_i,
)
end

Expand Down
2 changes: 1 addition & 1 deletion app/forms/openid_connect_token_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ def initialize(params)
ATTRS.each do |key|
instance_variable_set(:"@#{key}", params[key])
end
@session_expiration = IdentityConfig.store.session_timeout_in_minutes.minutes.ago
@session_expiration = IdentityConfig.store.session_timeout_in_seconds.seconds.ago
@identity = find_identity_with_code
end

Expand Down
24 changes: 12 additions & 12 deletions app/javascript/packs/session-expire-session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,8 @@ const warning = Number(warningEl.dataset.warning!) * 1000;
const sessionsURL = warningEl.dataset.sessionsUrl!;
const sessionTimeout = Number(warningEl.dataset.sessionTimeoutIn!) * 1000;
const modal = document.querySelector<ModalElement>('lg-modal.session-timeout-modal')!;
const keepaliveEl = document.getElementById('session-keepalive-btn');
const keepaliveButton = document.getElementById('session-keepalive-btn')!;
const countdownEls: NodeListOf<CountdownElement> = modal.querySelectorAll('lg-countdown');
const timeoutRefreshPath = warningEl.dataset.timeoutRefreshPath || '';

let sessionExpiration = new Date(Date.now() + sessionTimeout);

Expand All @@ -22,19 +21,20 @@ function showModal() {
});
}

function keepalive() {
function keepalive(event: MouseEvent) {
const isExpired = new Date() > sessionExpiration;
if (isExpired) {
document.location.href = timeoutRefreshPath;
} else {
modal.hide();
sessionExpiration = new Date(Date.now() + sessionTimeout);

setTimeout(showModal, sessionTimeout - warning);
countdownEls.forEach((countdownEl) => countdownEl.stop());
extendSession(sessionsURL);
return;
}

event.preventDefault();
modal.hide();
sessionExpiration = new Date(Date.now() + sessionTimeout);

setTimeout(showModal, sessionTimeout - warning);
countdownEls.forEach((countdownEl) => countdownEl.stop());
extendSession(sessionsURL);
}

keepaliveEl?.addEventListener('click', keepalive, false);
keepaliveButton.addEventListener('click', keepalive);
setTimeout(showModal, sessionTimeout - warning);
7 changes: 4 additions & 3 deletions app/javascript/packs/session-timeout-ping.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const timeoutURL = warningEl?.dataset.timeoutUrl!;
const sessionsURL = warningEl?.dataset.sessionsUrl!;

const modal = document.querySelector<ModalElement>('lg-modal.session-timeout-modal')!;
const keepaliveEl = document.getElementById('session-keepalive-btn');
const keepaliveButton = document.getElementById('session-keepalive-btn')!;
const countdownEls: NodeListOf<CountdownElement> = modal.querySelectorAll('lg-countdown');

function success({ isLive, timeout }: SessionStatus) {
Expand Down Expand Up @@ -46,11 +46,12 @@ function success({ isLive, timeout }: SessionStatus) {

const ping = () => requestSessionStatus(sessionsURL).then(success);

function keepalive() {
function keepalive(event: MouseEvent) {
event.preventDefault();
modal.hide();
countdownEls.forEach((countdownEl) => countdownEl.stop());
extendSession(sessionsURL);
}

keepaliveEl?.addEventListener('click', keepalive, false);
keepaliveButton.addEventListener('click', keepalive);
setTimeout(ping, start);
2 changes: 1 addition & 1 deletion app/views/session_timeout/_warning.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
</p>
</div>
<%= render ButtonComponent.new(
type: :button,
url: new_user_session_url(timeout: :session, request_id: sp_session[:request_id]),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is intentionally identical to the URL used for the fully-authenticated session expiration redirect URL:

timeout_url: new_user_session_url(timeout: :session, request_id: sp_session[:request_id]),

The thought being that we could get rid of session-timeout-ping.ts in favor of session-expire-session.ts, and it would "just work" the same way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only gotcha here is that including ?timeout=session means that a user will see the session timeout alert if they click "Continue sign in" after their session is fully expired. I think it's a reasonable and arguably-expected notice to be shown.

image

id: 'session-keepalive-btn',
big: true,
full_width: true,
Expand Down
2 changes: 1 addition & 1 deletion config/application.yml.default
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,7 @@ session_check_delay: 30
session_check_frequency: 30
session_encryption_key:
session_encryptor_alert_enabled: false
session_timeout_in_minutes: 15
session_timeout_in_seconds: 900
session_timeout_warning_seconds: 150
session_total_duration_timeout_in_minutes: 720
short_term_phone_otp_max_attempt_window_in_seconds: 10
Expand Down
2 changes: 1 addition & 1 deletion config/initializers/ahoy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

Ahoy.api = false
# Period of inactivity before a new visit is created
Ahoy.visit_duration = IdentityConfig.store.session_timeout_in_minutes.minutes
Ahoy.visit_duration = IdentityConfig.store.session_timeout_in_seconds.seconds
Ahoy.server_side_visits = false
Ahoy.geocode = false
Ahoy.user_agent_parser = :browser
Expand Down
2 changes: 1 addition & 1 deletion config/initializers/devise.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
config.skip_session_storage = [:http_auth]
config.strip_whitespace_keys = []
config.stretches = Rails.env.test? ? 1 : 12
config.timeout_in = IdentityConfig.store.session_timeout_in_minutes.minutes
config.timeout_in = IdentityConfig.store.session_timeout_in_seconds.seconds

config.warden do |manager|
manager.failure_app = CustomDeviseFailureApp
Expand Down
2 changes: 1 addition & 1 deletion config/initializers/session_store.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
write_private_id: true,

# Redis expires session after N minutes
ttl: IdentityConfig.store.session_timeout_in_minutes.minutes,
ttl: IdentityConfig.store.session_timeout_in_seconds.seconds,

key_prefix: "#{IdentityConfig.store.domain_name}:session:",
client_pool: REDIS_POOL,
Expand Down
2 changes: 1 addition & 1 deletion lib/identity_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,7 @@ def self.store
config.add(:session_check_frequency, type: :integer)
config.add(:session_encryption_key, type: :string)
config.add(:session_encryptor_alert_enabled, type: :boolean)
config.add(:session_timeout_in_minutes, type: :integer)
config.add(:session_timeout_in_seconds, type: :integer)
config.add(:session_timeout_warning_seconds, type: :integer)
config.add(:session_total_duration_timeout_in_minutes, type: :integer)
config.add(:show_unsupported_passkey_platform_authentication_setup, type: :boolean)
Expand Down
6 changes: 3 additions & 3 deletions spec/controllers/application_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def index

expect(cookies[:sp_issuer]).to eq(sp.issuer)
expect(cookie_expiration).to be_within(3.seconds).of(
IdentityConfig.store.session_timeout_in_minutes.minutes.from_now,
IdentityConfig.store.session_timeout_in_seconds.seconds.from_now,
)
end
end
Expand Down Expand Up @@ -414,7 +414,7 @@ def index
expect(flash[:info]).to eq t(
'notices.session_timedout',
app_name: APP_NAME,
minutes: IdentityConfig.store.session_timeout_in_minutes,
minutes: IdentityConfig.store.session_timeout_in_seconds.seconds.in_minutes.to_i,
)
end
end
Expand Down Expand Up @@ -447,7 +447,7 @@ def index

expect(flash[:info]).to eq t(
'notices.session_cleared',
minutes: IdentityConfig.store.session_timeout_in_minutes,
minutes: IdentityConfig.store.session_timeout_in_seconds.seconds.in_minutes.to_i,
)
end
end
Expand Down
30 changes: 26 additions & 4 deletions spec/features/users/sign_in_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,10 @@

scenario 'user sees warning before session times out' do
minutes_and = [
t('datetime.dotiw.minutes', count: IdentityConfig.store.session_timeout_in_minutes - 1),
t(
'datetime.dotiw.minutes',
count: IdentityConfig.store.session_timeout_in_seconds.seconds.in_minutes.to_i - 1,
),
t('datetime.dotiw.two_words_connector'),
].join('')

Expand All @@ -218,7 +221,7 @@

scenario 'user can continue browsing with refreshed CSRF token' do
token = first('[name=authenticity_token]', visible: false).value
click_button t('notices.timeout_warning.signed_in.continue')
click_on t('notices.timeout_warning.signed_in.continue')
expect(page).not_to have_css('.usa-js-modal--active')
expect(page).to have_css(
"[name=authenticity_token]:not([value='#{token}'])",
Expand Down Expand Up @@ -292,7 +295,7 @@

expect(page).to have_css('.usa-js-modal--active', wait: 10)

click_button t('notices.timeout_warning.partially_signed_in.continue')
click_on t('notices.timeout_warning.partially_signed_in.continue')

expect(page).not_to have_css('.usa-js-modal--active')
expect(find_field(t('forms.registration.labels.email')).value).not_to be_blank
Expand All @@ -309,10 +312,29 @@

expect(page).to have_css('.usa-js-modal--active', wait: 10)

click_button t('notices.timeout_warning.partially_signed_in.continue')
click_on t('notices.timeout_warning.partially_signed_in.continue')
expect(find_field(t('account.index.email')).value).not_to be_blank
end

it 'maintains partner request if the user continues after the session expires', js: true do
allow(IdentityConfig.store).to receive(:session_timeout_in_seconds).and_return(1)
allow(IdentityConfig.store).to receive(:session_check_delay).and_return(0)
allow(Devise).to receive(:timeout_in).and_return(1)

visit_idp_from_sp_with_ial1(:oidc)

expect(page).to have_content(
t(
'notices.timeout_warning.partially_signed_in.message_html',
time_left_in_session_html: t('datetime.dotiw.seconds', count: 0),
),
wait: 10,
)

click_on t('notices.timeout_warning.partially_signed_in.continue')
expect_branded_experience
end

it 'reloads the sign in page when cancel is clicked', js: true do
allow(Devise).to receive(:timeout_in)
.and_return(IdentityConfig.store.session_timeout_warning_seconds + 1)
Expand Down
1 change: 1 addition & 0 deletions spec/views/layouts/application.html.erb_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
)
allow(view.request).to receive(:original_fullpath).and_return('/foobar')
allow(view).to receive(:user_fully_authenticated?).and_return(false)
allow(view).to receive(:url_for).and_return('/')
view.title = title_content if title_content
end

Expand Down