Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce security plugin page load bundle #98819

Merged
merged 5 commits into from
Apr 30, 2021
Merged

Reduce security plugin page load bundle #98819

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ee5d097
Reduce security plugin page load bundle
legrego Apr 29, 2021
757f015
update session timeout tests
legrego Apr 29, 2021
8c0ca77
Update limits.yml
legrego Apr 30, 2021
576195a
Additional testing
legrego Apr 30, 2021
985e83e
fix snapshots
legrego Apr 30, 2021
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
4 changes: 2 additions & 2 deletions packages/kbn-optimizer/limits.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member Author

Choose a reason for hiding this comment

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

Not strictly required for this PR, but we did the work to drop the spaces bundle already, so we should make sure it stays that way

telemetry: 51957
telemetryManagementSection: 38586
tileMap: 65337
Expand Down
4 changes: 3 additions & 1 deletion x-pack/plugins/security/public/authentication/capture_url/capture_url_app.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
86 changes: 63 additions & 23 deletions x-pack/plugins/security/public/session/session_timeout.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -112,6 +112,7 @@ describe('Session Timeout', () => {

afterEach(async () => {
jest.clearAllMocks();
jest.unmock('broadcast-channel');
sessionTimeout.stop();
});

Expand All @@ -122,22 +123,42 @@ 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);
});

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);
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
Expand All @@ -157,7 +178,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.
Expand All @@ -184,7 +206,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
Expand All @@ -195,7 +218,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
Expand All @@ -206,7 +230,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);
Expand All @@ -218,7 +243,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);
Expand All @@ -236,7 +262,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);
Expand All @@ -250,7 +277,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);
Expand All @@ -261,7 +289,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');
Expand All @@ -287,7 +316,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);
Expand All @@ -299,7 +329,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
Expand All @@ -319,7 +350,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
Expand All @@ -333,7 +365,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
Expand All @@ -343,7 +376,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
Expand All @@ -366,7 +400,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);
Expand All @@ -376,7 +411,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();
Expand All @@ -391,7 +427,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();
Expand All @@ -401,7 +438,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;
Expand Down Expand Up @@ -438,15 +476,17 @@ describe('Session Timeout', () => {
lifespanExpiration: null,
provider: { type: 'basic', name: 'basic1' },
});
await sessionTimeout.start();
sessionTimeout.start();
await nextTick();

jest.advanceTimersByTime(0);
expect(sessionExpired.logout).toHaveBeenCalled();
});

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();
Expand Down
33 changes: 22 additions & 11 deletions x-pack/plugins/security/public/session/session_timeout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -45,7 +45,7 @@ export interface ISessionTimeout {
}

export class SessionTimeout implements ISessionTimeout {
private channel?: BroadcastChannel<SessionInfo>;
private channel?: BroadcastChannelType<SessionInfo>;
private sessionInfo?: SessionInfo;
private fetchTimer?: number;
private warningTimer?: number;
Expand All @@ -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() {
Expand Down