diff --git a/app/controllers/api/internal/sessions_controller.rb b/app/controllers/api/internal/sessions_controller.rb index 8a1d81a1767..e376c6aa8c8 100644 --- a/app/controllers/api/internal/sessions_controller.rb +++ b/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 diff --git a/app/javascript/packages/request/index.spec.ts b/app/javascript/packages/request/index.spec.ts index c94404acea8..34b1455edad 100644 --- a/app/javascript/packages/request/index.spec.ts +++ b/app/javascript/packages/request/index.spec.ts @@ -1,7 +1,7 @@ import sinon from 'sinon'; import type { SinonStub } from 'sinon'; import { useSandbox } from '@18f/identity-test-helpers'; -import { request } from '.'; +import { request, ResponseError } from '.'; describe('request', () => { const sandbox = useSandbox(); @@ -241,13 +241,14 @@ describe('request', () => { }); it('throws an error', async () => { - await request('https://example.com', { read: false }) - .then(() => { - throw new Error('Unexpected promise resolution'); - }) - .catch((error) => { - expect(error).to.exist(); - }); + let didCatch = false; + await request('https://example.com').catch((error: ResponseError) => { + expect(error).to.exist(); + expect(error.status).to.equal(400); + didCatch = true; + }); + + expect(didCatch).to.be.true(); }); context('with read=false option', () => { diff --git a/app/javascript/packages/request/index.ts b/app/javascript/packages/request/index.ts index a5b88b97ebc..0a94fc7fc66 100644 --- a/app/javascript/packages/request/index.ts +++ b/app/javascript/packages/request/index.ts @@ -1,5 +1,9 @@ type CSRFGetter = () => string | undefined; +export class ResponseError extends Error { + status: number; +} + interface RequestOptions extends RequestInit { /** * Either boolean or unstringified POJO to send with the request as JSON. Defaults to true. @@ -102,7 +106,9 @@ export async function request(url: string, options: Partial = {} if (read) { if (!response.ok) { - throw new Error(); + const error = new ResponseError(); + error.status = response.status; + throw error; } return json ? response.json() : response.text(); diff --git a/app/javascript/packages/session/requests.spec.ts b/app/javascript/packages/session/requests.spec.ts index dec5362091d..169ca37ea6b 100644 --- a/app/javascript/packages/session/requests.spec.ts +++ b/app/javascript/packages/session/requests.spec.ts @@ -7,73 +7,152 @@ 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 }); + }); + }); + + context('server responds with 500', () => { + before(() => { + server = setupServer( + rest.get<{}, {}>(STATUS_API_ENDPOINT, (_req, res, ctx) => res(ctx.status(500))), + ); + server.listen(); + }); + + after(() => { + server.close(); + }); + + it('throws an error', async () => { + await expect(requestSessionStatus()).to.be.rejected(); }); }); }); describe('extendSession', () => { - const timeout = new Date(Date.now() + 1000).toISOString(); - let server: SetupServer; - before(() => { - server = setupServer( - rest.post<{}, {}, SessionStatusResponse>(KEEP_ALIVE_API_ENDPOINT, (_req, res, ctx) => - res(ctx.json({ live: true, timeout })), - ), - ); - server.listen(); + + context('session active', () => { + const timeout = new Date(Date.now() + 1000).toISOString(); + + before(() => { + server = setupServer( + rest.post<{}, {}, SessionLiveStatusResponse>(KEEP_ALIVE_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 extendSession(); + + expect(result).to.deep.equal({ isLive: true, timeout: new Date(timeout) }); + }); }); - after(() => { - server.close(); + context('server responds with 401', () => { + before(() => { + server = setupServer( + rest.post<{}, {}>(KEEP_ALIVE_API_ENDPOINT, (_req, res, ctx) => res(ctx.status(401))), + ); + server.listen(); + }); + + after(() => { + server.close(); + }); + + it('resolves to the status', async () => { + const result = await extendSession(); + + expect(result).to.deep.equal({ isLive: false }); + }); }); - it('resolves to the status', async () => { - const result = await extendSession(); + context('server responds with 500', () => { + before(() => { + server = setupServer( + rest.post<{}, {}>(KEEP_ALIVE_API_ENDPOINT, (_req, res, ctx) => res(ctx.status(500))), + ); + server.listen(); + }); + + after(() => { + server.close(); + }); - expect(result).to.deep.equal({ isLive: true, timeout }); + it('throws an error', async () => { + await expect(extendSession()).to.be.rejected(); + }); }); }); diff --git a/app/javascript/packages/session/requests.ts b/app/javascript/packages/session/requests.ts index afc9deb1089..8165ad24976 100644 --- a/app/javascript/packages/session/requests.ts +++ b/app/javascript/packages/session/requests.ts @@ -1,10 +1,10 @@ -import { request } from '@18f/identity-request'; +import { request, ResponseError } 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. @@ -12,25 +12,74 @@ export interface SessionStatusResponse { 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, - timeout, -}); +function mapSessionStatusResponse( + response: R, +): SessionLiveStatus; +function mapSessionStatusResponse( + response: R, +): SessionTimedOutStatus; +function mapSessionStatusResponse< + R extends SessionLiveStatusResponse | SessionTimedOutStatusResponse, +>({ live, timeout }: R): SessionLiveStatus | SessionTimedOutStatus { + return live ? { isLive: true, timeout: new Date(timeout) } : { isLive: false }; +} + +/** + * Handles a thrown error from a session endpoint, interpreting an unauthorized request (401) as + * effectively an inactive session. Any other error is re-thrown as being unexpected. + * + * @param error Error thrown from request. + */ +function handleUnauthorizedStatusResponse(error: ResponseError) { + if (error.status === 401) { + return { live: false, timeout: null }; + } + + throw error; +} /** * Request the current session status. Returns a promise resolving to the current session status. @@ -38,7 +87,9 @@ const mapSessionStatusResponse = ({ live, timeout }: SessionStatusResponse): Ses * @return A promise resolving to the current session status */ export const requestSessionStatus = (): Promise => - request(STATUS_API_ENDPOINT).then(mapSessionStatusResponse); + request(STATUS_API_ENDPOINT) + .catch(handleUnauthorizedStatusResponse) + .then(mapSessionStatusResponse); /** * Request that the current session be kept alive. Returns a promise resolving to the updated @@ -47,6 +98,6 @@ export const requestSessionStatus = (): Promise => * @return A promise resolving to the updated session status. */ export const extendSession = (): Promise => - request(KEEP_ALIVE_API_ENDPOINT, { method: 'POST' }).then( - mapSessionStatusResponse, - ); + request(KEEP_ALIVE_API_ENDPOINT, { method: 'POST' }) + .catch(handleUnauthorizedStatusResponse) + .then(mapSessionStatusResponse); diff --git a/app/javascript/packs/session-timeout-ping.ts b/app/javascript/packs/session-timeout-ping.ts index ae699b7a8b7..7765be26f82 100644 --- a/app/javascript/packs/session-timeout-ping.ts +++ b/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(); }); } diff --git a/package.json b/package.json index 3d1ccabcd78..3651b3978d6 100644 --- a/package.json +++ b/package.json @@ -56,6 +56,7 @@ "@types/intl-tel-input": "^17.0.6", "@types/mocha": "^10.0.0", "@types/newrelic": "^7.0.3", + "@types/node": "^20.2.5", "@types/react": "^17.0.39", "@types/react-dom": "^17.0.11", "@types/sinon": "^10.0.13", @@ -87,7 +88,7 @@ "stylelint": "^15.5.0", "svgo": "^2.8.0", "swr": "^2.0.0", - "typescript": "^4.8.4", + "typescript": "^5.0.4", "webpack-dev-server": "^4.11.1", "whatwg-fetch": "^3.4.0" }, diff --git a/spec/controllers/api/internal/sessions_controller_spec.rb b/spec/controllers/api/internal/sessions_controller_spec.rb index 32252b94c6b..0f2eaa581d8 100644 --- a/spec/controllers/api/internal/sessions_controller_spec.rb +++ b/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 diff --git a/spec/features/saml/ial1_sso_spec.rb b/spec/features/saml/ial1_sso_spec.rb index 92a689b415e..b5d4919b669 100644 --- a/spec/features/saml/ial1_sso_spec.rb +++ b/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['session_expires_at'] = Time.zone.now - 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 diff --git a/yarn.lock b/yarn.lock index d6f693931cc..7d9ba847366 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1483,6 +1483,11 @@ resolved "https://registry.yarnpkg.com/@types/node/-/node-18.0.6.tgz#0ba49ac517ad69abe7a1508bc9b3a5483df9d5d7" integrity sha512-/xUq6H2aQm261exT6iZTMifUySEt4GR5KX8eYyY+C4MSNPqSh9oNIP7tz2GLKTlFaiBbgZNxffoR3CVRG+cljw== +"@types/node@^20.2.5": + version "20.2.5" + resolved "https://registry.yarnpkg.com/@types/node/-/node-20.2.5.tgz#26d295f3570323b2837d322180dfbf1ba156fefb" + integrity sha512-JJulVEQXmiY9Px5axXHeYGLSjhkZEnD+MDPDGbCbIAbMslkKwmygtZFy1X6s/075Yo94sf8GuSlFfPzysQrWZQ== + "@types/normalize-package-data@^2.4.0": version "2.4.1" resolved "https://registry.yarnpkg.com/@types/normalize-package-data/-/normalize-package-data-2.4.1.tgz#d3357479a0fdfdd5907fe67e17e0a85c906e1301" @@ -6632,10 +6637,10 @@ typed-array-length@^1.0.4: for-each "^0.3.3" is-typed-array "^1.1.9" -typescript@^4.8.4: - version "4.8.4" - resolved "https://registry.yarnpkg.com/typescript/-/typescript-4.8.4.tgz#c464abca159669597be5f96b8943500b238e60e6" - integrity sha512-QCh+85mCy+h0IGff8r5XWzOVSbBO+KfeYrMQh7NJ58QujwcE22u+NUSmUxqF+un70P9GXKxa2HCNiTTMJknyjQ== +typescript@^5.0.4: + version "5.0.4" + resolved "https://registry.yarnpkg.com/typescript/-/typescript-5.0.4.tgz#b217fd20119bd61a94d4011274e0ab369058da3b" + integrity sha512-cW9T5W9xY37cc+jfEnaUvX91foxtHkza3Nw3wkoF4sSlKn0MONdkdEndig/qPBWXNkmplh3NzayQzCiHM4/hqw== unbox-primitive@^1.0.2: version "1.0.2"