From ee5d097e4c1539bc4108e675200bd68bb312fb3d Mon Sep 17 00:00:00 2001 From: Larry Gregory Date: Thu, 29 Apr 2021 15:09:43 -0400 Subject: [PATCH 1/5] Reduce security plugin page load bundle --- .../capture_url/capture_url_app.ts | 4 ++- .../public/session/session_timeout.tsx | 33 ++++++++++++------- 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/x-pack/plugins/security/public/authentication/capture_url/capture_url_app.ts b/x-pack/plugins/security/public/authentication/capture_url/capture_url_app.ts index af45314c5bacb..97bbd0848e9c4 100644 --- a/x-pack/plugins/security/public/authentication/capture_url/capture_url_app.ts +++ b/x-pack/plugins/security/public/authentication/capture_url/capture_url_app.ts @@ -8,7 +8,6 @@ import type { ApplicationSetup, FatalErrorsSetup, HttpSetup } from 'src/core/public'; import { AUTH_URL_HASH_QUERY_STRING_PARAMETER } from '../../../common/constants'; -import { parseNext } from '../../../common/parse_next'; interface CreateDeps { application: ApplicationSetup; @@ -46,6 +45,9 @@ export const captureURLApp = Object.freeze({ appRoute: '/internal/security/capture-url', async mount() { try { + // This is an async import because it requires `url`, which is a sizable dependency. + // Otherwise this becomes part of the "page load bundle". + const { parseNext } = await import('../../../common/parse_next'); const url = new URL( parseNext(window.location.href, http.basePath.serverBasePath), window.location.origin diff --git a/x-pack/plugins/security/public/session/session_timeout.tsx b/x-pack/plugins/security/public/session/session_timeout.tsx index cc7eaa551b1b3..2288fce8d30af 100644 --- a/x-pack/plugins/security/public/session/session_timeout.tsx +++ b/x-pack/plugins/security/public/session/session_timeout.tsx @@ -5,7 +5,7 @@ * 2.0. */ -import { BroadcastChannel } from 'broadcast-channel'; +import type { BroadcastChannel as BroadcastChannelType } from 'broadcast-channel'; import type { HttpSetup, NotificationsSetup, Toast, ToastInput } from 'src/core/public'; @@ -45,7 +45,7 @@ export interface ISessionTimeout { } export class SessionTimeout implements ISessionTimeout { - private channel?: BroadcastChannel; + private channel?: BroadcastChannelType; private sessionInfo?: SessionInfo; private fetchTimer?: number; private warningTimer?: number; @@ -64,15 +64,26 @@ export class SessionTimeout implements ISessionTimeout { return; } - // subscribe to a broadcast channel for session timeout messages - // this allows us to synchronize the UX across tabs and avoid repetitive API calls - const name = `${this.tenant}/session_timeout`; - this.channel = new BroadcastChannel(name, { webWorkerSupport: false }); - this.channel.onmessage = this.handleSessionInfoAndResetTimers; - - // Triggers an initial call to the endpoint to get session info; - // when that returns, it will set the timeout - return this.fetchSessionInfoAndResetTimers(); + import('broadcast-channel') + .then(({ BroadcastChannel }) => { + // subscribe to a broadcast channel for session timeout messages + // this allows us to synchronize the UX across tabs and avoid repetitive API calls + const name = `${this.tenant}/session_timeout`; + this.channel = new BroadcastChannel(name, { webWorkerSupport: false }); + this.channel.onmessage = this.handleSessionInfoAndResetTimers; + }) + .catch((e) => { + // eslint-disable-next-line no-console + console.warn( + `Failed to load broadcast channel. Session management will not be kept in sync when multiple tabs are loaded.`, + e + ); + }) + .finally(() => { + // Triggers an initial call to the endpoint to get session info; + // when that returns, it will set the timeout + return this.fetchSessionInfoAndResetTimers(); + }); } stop() { From 757f01564bb138a69cea5b4cc91d295b31d72908 Mon Sep 17 00:00:00 2001 From: Larry Gregory Date: Thu, 29 Apr 2021 15:19:37 -0400 Subject: [PATCH 2/5] update session timeout tests --- .../public/session/session_timeout.test.tsx | 68 ++++++++++++------- 1 file changed, 45 insertions(+), 23 deletions(-) diff --git a/x-pack/plugins/security/public/session/session_timeout.test.tsx b/x-pack/plugins/security/public/session/session_timeout.test.tsx index d224edb8cafd4..6f1cbff33fb4a 100644 --- a/x-pack/plugins/security/public/session/session_timeout.test.tsx +++ b/x-pack/plugins/security/public/session/session_timeout.test.tsx @@ -7,7 +7,7 @@ import BroadcastChannel from 'broadcast-channel'; -import { mountWithIntl } from '@kbn/test/jest'; +import { mountWithIntl, nextTick } from '@kbn/test/jest'; import { coreMock } from 'src/core/public/mocks'; import { createSessionExpiredMock } from './session_expired.mock'; @@ -122,7 +122,8 @@ describe('Session Timeout', () => { describe('Lifecycle', () => { test(`starts and initializes on a non-anonymous path`, async () => { - await sessionTimeout.start(); + sessionTimeout.start(); + await nextTick(); // eslint-disable-next-line dot-notation expect(sessionTimeout['channel']).not.toBeUndefined(); expect(http.fetch).toHaveBeenCalledTimes(1); @@ -130,14 +131,16 @@ describe('Session Timeout', () => { test(`starts and does not initialize on an anonymous path`, async () => { http.anonymousPaths.isAnonymous.mockReturnValue(true); - await sessionTimeout.start(); + sessionTimeout.start(); + await nextTick(); // eslint-disable-next-line dot-notation expect(sessionTimeout['channel']).toBeUndefined(); expect(http.fetch).not.toHaveBeenCalled(); }); test(`stops`, async () => { - await sessionTimeout.start(); + sessionTimeout.start(); + await nextTick(); // eslint-disable-next-line dot-notation const close = jest.fn(sessionTimeout['channel']!.close); // eslint-disable-next-line dot-notation @@ -157,7 +160,8 @@ describe('Session Timeout', () => { ...defaultSessionInfo, idleTimeoutExpiration: now + 5_000_000_000, }); - await sessionTimeout.start(); + sessionTimeout.start(); + await nextTick(); // Advance timers far enough to call intermediate `setTimeout` multiple times, but before any // of the timers is supposed to be triggered. @@ -184,7 +188,8 @@ describe('Session Timeout', () => { }); test(`handles success`, async () => { - await sessionTimeout.start(); + sessionTimeout.start(); + await nextTick(); expect(http.fetch).toHaveBeenCalledTimes(1); // eslint-disable-next-line dot-notation @@ -195,7 +200,8 @@ describe('Session Timeout', () => { test(`handles error`, async () => { const mockErrorResponse = new Error('some-error'); http.fetch.mockRejectedValue(mockErrorResponse); - await sessionTimeout.start(); + sessionTimeout.start(); + await nextTick(); expect(http.fetch).toHaveBeenCalledTimes(1); // eslint-disable-next-line dot-notation @@ -206,7 +212,8 @@ describe('Session Timeout', () => { describe('warning toast', () => { test(`shows idle timeout warning toast`, async () => { - await sessionTimeout.start(); + sessionTimeout.start(); + await nextTick(); // we display the warning a minute before we expire the the session, which is 5 seconds before it actually expires jest.advanceTimersByTime(55 * 1000); @@ -218,7 +225,8 @@ describe('Session Timeout', () => { ...defaultSessionInfo, idleTimeoutExpiration: now + 5_000_000_000, }); - await sessionTimeout.start(); + sessionTimeout.start(); + await nextTick(); // we display the warning a minute before we expire the the session, which is 5 seconds before it actually expires jest.advanceTimersByTime(5_000_000_000 - 66 * 1000); @@ -236,7 +244,8 @@ describe('Session Timeout', () => { provider: { type: 'basic', name: 'basic1' }, }; http.fetch.mockResolvedValue(sessionInfo); - await sessionTimeout.start(); + sessionTimeout.start(); + await nextTick(); // we display the warning a minute before we expire the the session, which is 5 seconds before it actually expires jest.advanceTimersByTime(55 * 1000); @@ -250,7 +259,8 @@ describe('Session Timeout', () => { lifespanExpiration: now + 5_000_000_000, }; http.fetch.mockResolvedValue(sessionInfo); - await sessionTimeout.start(); + sessionTimeout.start(); + await nextTick(); // we display the warning a minute before we expire the the session, which is 5 seconds before it actually expires jest.advanceTimersByTime(5_000_000_000 - 66 * 1000); @@ -261,7 +271,8 @@ describe('Session Timeout', () => { }); test(`extend only results in an HTTP call if a warning is shown`, async () => { - await sessionTimeout.start(); + sessionTimeout.start(); + await nextTick(); expect(http.fetch).toHaveBeenCalledTimes(1); await sessionTimeout.extend('/foo'); @@ -287,7 +298,8 @@ describe('Session Timeout', () => { provider: { type: 'basic', name: 'basic1' }, }; http.fetch.mockResolvedValue(sessionInfo); - await sessionTimeout.start(); + sessionTimeout.start(); + await nextTick(); // we display the warning a minute before we expire the the session, which is 5 seconds before it actually expires jest.advanceTimersByTime(55 * 1000); @@ -299,7 +311,8 @@ describe('Session Timeout', () => { }); test(`extend hides displayed warning toast`, async () => { - await sessionTimeout.start(); + sessionTimeout.start(); + await nextTick(); expect(http.fetch).toHaveBeenCalledTimes(1); // we display the warning a minute before we expire the the session, which is 5 seconds before it actually expires @@ -319,7 +332,8 @@ describe('Session Timeout', () => { }); test(`extend does nothing for session-related routes`, async () => { - await sessionTimeout.start(); + sessionTimeout.start(); + await nextTick(); expect(http.fetch).toHaveBeenCalledTimes(1); // we display the warning a minute before we expire the the session, which is 5 seconds before it actually expires @@ -333,7 +347,8 @@ describe('Session Timeout', () => { }); test(`checks for updated session info before the warning displays`, async () => { - await sessionTimeout.start(); + sessionTimeout.start(); + await nextTick(); expect(http.fetch).toHaveBeenCalledTimes(1); // we check for updated session info 1 second before the warning is shown @@ -343,7 +358,8 @@ describe('Session Timeout', () => { }); test('clicking "extend" causes a new HTTP request (which implicitly extends the session)', async () => { - await sessionTimeout.start(); + sessionTimeout.start(); + await nextTick(); expect(http.fetch).toHaveBeenCalledTimes(1); // we display the warning a minute before we expire the the session, which is 5 seconds before it actually expires @@ -366,7 +382,8 @@ describe('Session Timeout', () => { lifespanExpiration: null, provider: { type: 'basic', name: 'basic1' }, }); - await sessionTimeout.start(); + sessionTimeout.start(); + await nextTick(); expect(http.fetch).toHaveBeenCalled(); jest.advanceTimersByTime(0); @@ -376,7 +393,8 @@ describe('Session Timeout', () => { describe('session expiration', () => { test(`expires the session 5 seconds before it really expires`, async () => { - await sessionTimeout.start(); + sessionTimeout.start(); + await nextTick(); jest.advanceTimersByTime(114 * 1000); expect(sessionExpired.logout).not.toHaveBeenCalled(); @@ -391,7 +409,8 @@ describe('Session Timeout', () => { idleTimeoutExpiration: now + 5_000_000_000, }); - await sessionTimeout.start(); + sessionTimeout.start(); + await nextTick(); jest.advanceTimersByTime(5_000_000_000 - 6000); expect(sessionExpired.logout).not.toHaveBeenCalled(); @@ -401,7 +420,8 @@ describe('Session Timeout', () => { }); test(`extend delays the expiration`, async () => { - await sessionTimeout.start(); + sessionTimeout.start(); + await nextTick(); expect(http.fetch).toHaveBeenCalledTimes(1); const elapsed = 114 * 1000; @@ -438,7 +458,8 @@ describe('Session Timeout', () => { lifespanExpiration: null, provider: { type: 'basic', name: 'basic1' }, }); - await sessionTimeout.start(); + sessionTimeout.start(); + await nextTick(); jest.advanceTimersByTime(0); expect(sessionExpired.logout).toHaveBeenCalled(); @@ -446,7 +467,8 @@ describe('Session Timeout', () => { test(`'null' sessionTimeout never logs you out`, async () => { http.fetch.mockResolvedValue({ now, idleTimeoutExpiration: null, lifespanExpiration: null }); - await sessionTimeout.start(); + sessionTimeout.start(); + await nextTick(); jest.advanceTimersByTime(Number.MAX_VALUE); expect(sessionExpired.logout).not.toHaveBeenCalled(); From 8c0ca779841c57bb49156851df309a30be2efd33 Mon Sep 17 00:00:00 2001 From: Larry Gregory Date: Fri, 30 Apr 2021 08:32:16 -0400 Subject: [PATCH 3/5] Update limits.yml --- packages/kbn-optimizer/limits.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/kbn-optimizer/limits.yml b/packages/kbn-optimizer/limits.yml index 0bb4594244a75..2e14fb966a2e7 100644 --- a/packages/kbn-optimizer/limits.yml +++ b/packages/kbn-optimizer/limits.yml @@ -67,12 +67,12 @@ pageLoadAssetSize: savedObjectsTagging: 59482 savedObjectsTaggingOss: 20590 searchprofiler: 67080 - security: 189428 + security: 95864 securityOss: 30806 securitySolution: 187863 share: 99061 snapshotRestore: 79032 - spaces: 387915 + spaces: 57868 telemetry: 51957 telemetryManagementSection: 38586 tileMap: 65337 From 576195a9cdb349f36c32689a34e7e271c138c204 Mon Sep 17 00:00:00 2001 From: Larry Gregory Date: Fri, 30 Apr 2021 08:32:30 -0400 Subject: [PATCH 4/5] Additional testing --- .../public/session/session_timeout.test.tsx | 26 ++++++++++++++----- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/x-pack/plugins/security/public/session/session_timeout.test.tsx b/x-pack/plugins/security/public/session/session_timeout.test.tsx index 6f1cbff33fb4a..cc6645e722be4 100644 --- a/x-pack/plugins/security/public/session/session_timeout.test.tsx +++ b/x-pack/plugins/security/public/session/session_timeout.test.tsx @@ -36,9 +36,7 @@ const expectIdleTimeoutWarningToast = ( "iconType": "clock", "text": Any, "title": "Warning", - "toastLifeTimeMs": ${toastLifeTimeMs}, - } - ` + "toastLifeTimeMs": ` ); }; @@ -57,9 +55,7 @@ const expectLifespanWarningToast = ( "iconType": "alert", "text": Any, "title": "Warning", - "toastLifeTimeMs": ${toastLifeTimeMs}, - } - ` + "toastLifeTimeMs": ` ); }; @@ -112,6 +108,7 @@ describe('Session Timeout', () => { afterEach(async () => { jest.clearAllMocks(); + jest.unmock('broadcast-channel'); sessionTimeout.stop(); }); @@ -129,6 +126,23 @@ describe('Session Timeout', () => { expect(http.fetch).toHaveBeenCalledTimes(1); }); + test(`starts and initializes if the broadcast channel fails to load`, async () => { + jest.mock('broadcast-channel', () => { + throw new Error('Unable to load broadcast channel!'); + }); + const consoleSpy = jest.spyOn(console, 'warn'); + + sessionTimeout.start(); + await nextTick(); + // eslint-disable-next-line dot-notation + expect(sessionTimeout['channel']).toBeUndefined(); + expect(http.fetch).toHaveBeenCalledTimes(1); + expect(consoleSpy).toHaveBeenCalledTimes(1); + expect(consoleSpy.mock.calls[0][0]).toMatchInlineSnapshot( + `"Failed to load broadcast channel. Session management will not be kept in sync when multiple tabs are loaded."` + ); + }); + test(`starts and does not initialize on an anonymous path`, async () => { http.anonymousPaths.isAnonymous.mockReturnValue(true); sessionTimeout.start(); From 985e83ec5365ecc2896d4377cba3649fde261882 Mon Sep 17 00:00:00 2001 From: Larry Gregory Date: Fri, 30 Apr 2021 08:39:30 -0400 Subject: [PATCH 5/5] fix snapshots --- .../security/public/session/session_timeout.test.tsx | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/security/public/session/session_timeout.test.tsx b/x-pack/plugins/security/public/session/session_timeout.test.tsx index cc6645e722be4..1faa105691259 100644 --- a/x-pack/plugins/security/public/session/session_timeout.test.tsx +++ b/x-pack/plugins/security/public/session/session_timeout.test.tsx @@ -36,7 +36,9 @@ const expectIdleTimeoutWarningToast = ( "iconType": "clock", "text": Any, "title": "Warning", - "toastLifeTimeMs": ` + "toastLifeTimeMs": ${toastLifeTimeMs}, + } + ` ); }; @@ -55,7 +57,9 @@ const expectLifespanWarningToast = ( "iconType": "alert", "text": Any, "title": "Warning", - "toastLifeTimeMs": ` + "toastLifeTimeMs": ${toastLifeTimeMs}, + } + ` ); };