Update session timeout poll response handling to omit timeout on inactive session

app/controllers/api/internal/sessions_controller.rb

@@ -11,13 +11,13 @@ class SessionsController < ApplicationController
       respond_to :json
 
       def show
-        render json: { live: live?, timeout: timeout }
+        render json: status_response
       end
 
       def update
         analytics.session_kept_alive if live?
         update_last_request_at
-        render json: { live: live?, timeout: timeout }
+        render json: status_response
       end
 
       def destroy
@@ -29,21 +29,21 @@ def destroy
 
       private
 
+      def status_response
+        { live: live?, timeout: live?.presence && timeout }
+      end
+
       def skip_devise_hooks
         request.env['devise.skip_timeout'] = true
         request.env['devise.skip_trackable'] = true
       end
 
       def live?
-        timeout.future?
+        timeout.present? && timeout.future?
       end
 
       def timeout
-        if last_request_at.present?
-          Time.zone.at(last_request_at + User.timeout_in)
-        else
-          Time.current
-        end
+        Time.zone.at(last_request_at + User.timeout_in) if last_request_at.present?
       end
 
       def last_request_at

app/javascript/packages/session/requests.spec.ts

@@ -7,49 +7,72 @@ import {
   requestSessionStatus,
   extendSession,
 } from './requests';
-import type { SessionStatusResponse } from './requests';
+import type { SessionLiveStatusResponse, SessionTimedOutStatusResponse } from './requests';
 
 describe('requestSessionStatus', () => {
-  let isLive: boolean;
-  let timeout: string;
-
   let server: SetupServer;
-  before(() => {
-    server = setupServer(
-      rest.get<{}, {}, SessionStatusResponse>(STATUS_API_ENDPOINT, (_req, res, ctx) =>
-        res(ctx.json({ live: isLive, timeout })),
-      ),
-    );
-    server.listen();
-  });
-
-  after(() => {
-    server.close();
-  });
 
   context('session inactive', () => {
-    beforeEach(() => {
-      isLive = false;
-      timeout = new Date().toISOString();
+    before(() => {
+      server = setupServer(
+        rest.get<{}, {}, SessionTimedOutStatusResponse>(STATUS_API_ENDPOINT, (_req, res, ctx) =>
+          res(ctx.json({ live: false, timeout: null })),
+        ),
+      );
+      server.listen();
+    });
+
+    after(() => {
+      server.close();
     });
 
     it('resolves to the status', async () => {
       const result = await requestSessionStatus();
 
-      expect(result).to.deep.equal({ isLive: false, timeout });
+      expect(result).to.deep.equal({ isLive: false });
     });
   });
 
   context('session active', () => {
-    beforeEach(() => {
-      isLive = true;
+    let timeout: string;
+
+    before(() => {
       timeout = new Date(Date.now() + 1000).toISOString();
+      server = setupServer(
+        rest.get<{}, {}, SessionLiveStatusResponse>(STATUS_API_ENDPOINT, (_req, res, ctx) =>
+          res(ctx.json({ live: true, timeout })),
+        ),
+      );
+      server.listen();
+    });
+
+    after(() => {
+      server.close();
+    });
+
+    it('resolves to the status', async () => {
+      const result = await requestSessionStatus();
+
+      expect(result).to.deep.equal({ isLive: true, timeout: new Date(timeout) });
+    });
+  });
+
+  context('server responds with 401', () => {
+    before(() => {
+      server = setupServer(
+        rest.get<{}, {}>(STATUS_API_ENDPOINT, (_req, res, ctx) => res(ctx.status(401))),
+      );
+      server.listen();
+    });
+
+    after(() => {
+      server.close();
     });
 
     it('resolves to the status', async () => {
       const result = await requestSessionStatus();
 
-      expect(result).to.deep.equal({ isLive: true, timeout });
+      expect(result).to.deep.equal({ isLive: false });
     });
   });
 });
@@ -60,7 +83,7 @@ describe('extendSession', () => {
   let server: SetupServer;
   before(() => {
     server = setupServer(
-      rest.post<{}, {}, SessionStatusResponse>(KEEP_ALIVE_API_ENDPOINT, (_req, res, ctx) =>
+      rest.post<{}, {}, SessionLiveStatusResponse>(KEEP_ALIVE_API_ENDPOINT, (_req, res, ctx) =>
         res(ctx.json({ live: true, timeout })),
       ),
     );
@@ -74,6 +97,6 @@ describe('extendSession', () => {
   it('resolves to the status', async () => {
     const result = await extendSession();
 
-    expect(result).to.deep.equal({ isLive: true, timeout });
+    expect(result).to.deep.equal({ isLive: true, timeout: new Date(timeout) });
   });
 });

app/javascript/packages/session/requests.ts

@@ -1,44 +1,82 @@
 import { request } from '@18f/identity-request';
 
-export interface SessionStatusResponse {
+export interface SessionLiveStatusResponse {
   /**
    * Whether the session is still active.
    */
-  live: boolean;
+  live: true;
 
   /**
    * ISO8601-formatted date string for session timeout.
    */
   timeout: string;
 }
 
-export interface SessionStatus {
+export interface SessionTimedOutStatusResponse {
   /**
    * Whether the session is still active.
    */
-  isLive: boolean;
+  live: false;
 
   /**
    * ISO8601-formatted date string for session timeout.
    */
-  timeout: string;
+  timeout: null;
+}
+
+type SessionStatusResponse = SessionLiveStatusResponse | SessionTimedOutStatusResponse;
+
+interface SessionLiveStatus {
+  /**
+   * Whether the session is still active.
+   */
+  isLive: true;
+
+  /**
+   * ISO8601-formatted date string for session timeout.
+   */
+  timeout: Date;
 }
 
+interface SessionTimedOutStatus {
+  /**
+   * Whether the session is still active.
+   */
+  isLive: false;
+
+  /**
+   * ISO8601-formatted date string for session timeout.
+   */
+  timeout?: undefined;
+}
+
+export type SessionStatus = SessionLiveStatus | SessionTimedOutStatus;
+
 export const STATUS_API_ENDPOINT = '/active';
 export const KEEP_ALIVE_API_ENDPOINT = '/sessions/keepalive';
 
-const mapSessionStatusResponse = ({ live, timeout }: SessionStatusResponse): SessionStatus => ({
-  isLive: live,
+function mapSessionStatusResponse<R extends SessionLiveStatusResponse>(
+  response: R,
+): SessionLiveStatus;
+function mapSessionStatusResponse<R extends SessionTimedOutStatusResponse>(
+  response: R,
+): SessionTimedOutStatus;
+function mapSessionStatusResponse<R extends SessionStatusResponse>({
+  live,
   timeout,
-});
+}: R): SessionLiveStatus | SessionTimedOutStatus {
+  return live ? { isLive: true, timeout: new Date(timeout) } : { isLive: false };
+}
 
 /**
  * Request the current session status. Returns a promise resolving to the current session status.
  *
  * @return A promise resolving to the current session status
  */
 export const requestSessionStatus = (): Promise<SessionStatus> =>
-  request<SessionStatusResponse>(STATUS_API_ENDPOINT).then(mapSessionStatusResponse);
+  request<SessionStatusResponse>(STATUS_API_ENDPOINT)
+    .catch(() => ({ live: false, timeout: null }))
+    .then(mapSessionStatusResponse);
 
 /**
  * Request that the current session be kept alive. Returns a promise resolving to the updated

app/javascript/packs/session-timeout-ping.ts

@@ -47,20 +47,20 @@ function handleTimeout(redirectURL: string) {
   forceRedirect(redirectURL);
 }
 
-function success(data: SessionStatus) {
-  if (!data.isLive) {
+function success({ isLive, timeout }: SessionStatus) {
+  if (!isLive) {
     if (timeoutUrl) {
       handleTimeout(timeoutUrl);
     }
     return;
   }
 
-  const timeRemaining = new Date(data.timeout).valueOf() - Date.now();
+  const timeRemaining = timeout.valueOf() - Date.now();
   const showWarning = timeRemaining < warning;
   if (showWarning) {
     modal.show();
     countdownEls.forEach((countdownEl) => {
-      countdownEl.expiration = new Date(data.timeout);
+      countdownEl.expiration = timeout;
       countdownEl.start();
     });
   }

spec/controllers/api/internal/sessions_controller_spec.rb

@@ -15,7 +15,7 @@
     subject(:response) { JSON.parse(get(:show).body, symbolize_names: true) }
 
     it 'responds with live and timeout properties' do
-      expect(response).to eq(live: false, timeout: Time.zone.now.as_json)
+      expect(response).to eq(live: false, timeout: nil)
     end
 
     context 'signed in' do
@@ -45,10 +45,7 @@
           let(:delay) { User.timeout_in + 1.second }
 
           it 'responds with live and timeout properties' do
-            expect(response).to eq(
-              live: false,
-              timeout: (User.timeout_in - delay).from_now.as_json,
-            )
+            expect(response).to eq(live: false, timeout: nil)
           end
         end
       end

spec/features/saml/ial1_sso_spec.rb

@@ -70,22 +70,34 @@
       )
     end
 
-    it 'after session timeout, signing in takes user back to SP' do
+    it 'after session timeout, signing in takes user back to SP', :js, allow_browser_log: true do
+      allow(IdentityConfig.store).to receive(:session_check_delay).and_return(0)
+
       user = create(:user, :fully_registered)
       request_url = saml_authn_request_url
 
       visit request_url
       sp_request_id = ServiceProviderRequestProxy.last.uuid
+      fill_in_credentials_and_submit(user.email, user.password)
+
+      Warden.on_next_request do |proxy|
+        proxy.env['devise.skip_trackable'] = true
+        session = proxy.env['rack.session']
+        session['warden.user.user.session']['last_request_at'] = User.timeout_in.ago.to_i - 1
+      end
 
-      visit timeout_path
-      expect(current_url).to eq root_url(request_id: sp_request_id)
+      expect(page).to have_current_path(new_user_session_path(request_id: sp_request_id), wait: 5)
+      allow(IdentityConfig.store).to receive(:session_check_delay).and_call_original
 
       fill_in_credentials_and_submit(user.email, user.password)
       fill_in_code_with_last_phone_otp
-      click_submit_default_twice
+      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)
       click_agree_and_continue
 
-      expect(current_url).to eq complete_saml_url
+      expect(page).to have_current_path(test_saml_decode_assertion_path)
     end
   end