Skip to content

Commit

Permalink
Removed unecessary response interceptor and improved copy
Browse files Browse the repository at this point in the history
  • Loading branch information
thomheymann committed May 4, 2021
1 parent 54de738 commit 45e56d8
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 97 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,7 @@ import { of } from 'rxjs';

import { I18nProvider } from '@kbn/i18n/react';

import {
createSessionExpirationToast,
SessionExpirationBody,
SessionExpirationTitle,
} from './session_expiration_toast';
import { createSessionExpirationToast, SessionExpirationToast } from './session_expiration_toast';
import type { SessionState } from './session_timeout';

describe('createSessionExpirationToast', () => {
Expand All @@ -34,14 +30,14 @@ describe('createSessionExpirationToast', () => {
iconType: 'clock',
onClose: expect.any(Function),
text: expect.any(Function),
title: expect.any(Function),
title: expect.any(String),
toastLifeTimeMs: 2147483647,
})
);
});
});

describe('SessionExpirationTitle', () => {
describe('SessionExpirationToast', () => {
it('renders session expiration time', () => {
const sessionState$ = of<SessionState>({
lastExtensionTime: Date.now(),
Expand All @@ -51,14 +47,12 @@ describe('SessionExpirationTitle', () => {

const { getByText } = render(
<I18nProvider>
<SessionExpirationTitle sessionState$={sessionState$} />
<SessionExpirationToast sessionState$={sessionState$} onExtend={jest.fn()} />
</I18nProvider>
);
getByText(/Session expires in [0-9]+ seconds/);
getByText(/You will be logged out in [0-9]+ seconds/);
});
});

describe('SessionExpirationBody', () => {
it('renders extend button if session can be extended', () => {
const sessionState$ = of<SessionState>({
lastExtensionTime: Date.now(),
Expand All @@ -69,10 +63,10 @@ describe('SessionExpirationBody', () => {

const { getByRole } = render(
<I18nProvider>
<SessionExpirationBody sessionState$={sessionState$} onExtend={onExtend} />
<SessionExpirationToast sessionState$={sessionState$} onExtend={onExtend} />
</I18nProvider>
);
fireEvent.click(getByRole('button', { name: 'Keep me logged in' }));
fireEvent.click(getByRole('button', { name: 'Stay logged in' }));

expect(onExtend).toHaveBeenCalled();
});
Expand All @@ -87,9 +81,9 @@ describe('SessionExpirationBody', () => {

const { queryByRole } = render(
<I18nProvider>
<SessionExpirationBody sessionState$={sessionState$} onExtend={onExtend} />
<SessionExpirationToast sessionState$={sessionState$} onExtend={onExtend} />
</I18nProvider>
);
expect(queryByRole('button', { name: 'Keep me logged in' })).toBeNull();
expect(queryByRole('button', { name: 'Stay logged in' })).toBeNull();
});
});
72 changes: 31 additions & 41 deletions x-pack/plugins/security/public/session/session_expiration_toast.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,37 +5,41 @@
* 2.0.
*/

import { EuiButton, EuiFlexGroup, EuiFlexItem } from '@elastic/eui';
import { EuiButton, EuiFlexGroup, EuiFlexItem, EuiSpacer } from '@elastic/eui';
import type { FunctionComponent } from 'react';
import React from 'react';
import useAsyncFn from 'react-use/lib/useAsyncFn';
import useObservable from 'react-use/lib/useObservable';
import type { Observable } from 'rxjs';

import { i18n } from '@kbn/i18n';
import { FormattedMessage, FormattedRelative } from '@kbn/i18n/react';
import type { ToastInput } from 'src/core/public';

import { toMountPoint } from '../../../../../src/plugins/kibana_react/public';
import { SESSION_GRACE_PERIOD_MS } from '../../common/constants';
import type { SessionState } from './session_timeout';

export interface SessionExpirationTitleProps {
export interface SessionExpirationToastProps {
sessionState$: Observable<SessionState>;
onExtend: () => Promise<any>;
}

export const SessionExpirationTitle: FunctionComponent<SessionExpirationTitleProps> = ({
export const SessionExpirationToast: FunctionComponent<SessionExpirationToastProps> = ({
sessionState$,
onExtend,
}) => {
const state = useObservable(sessionState$);
const [{ loading }, extend] = useAsyncFn(onExtend);

if (!state || !state.expiresInMs) {
return null;
}

return (
const expirationWarning = (
<FormattedMessage
id="xpack.security.sessionExpirationToast.title"
defaultMessage="Session expires {timeout}"
id="xpack.security.sessionExpirationToast.body"
defaultMessage="You will be logged out {timeout}."
values={{
timeout: (
<FormattedRelative
Expand All @@ -46,45 +50,27 @@ export const SessionExpirationTitle: FunctionComponent<SessionExpirationTitlePro
}}
/>
);
};

export interface SessionExpirationBodyProps {
sessionState$: Observable<SessionState>;
onExtend: () => Promise<any>;
}

export const SessionExpirationBody: FunctionComponent<SessionExpirationBodyProps> = ({
sessionState$,
onExtend,
}) => {
const state = useObservable(sessionState$);
const [{ loading }, extend] = useAsyncFn(onExtend);

if (!state) {
return null;
}

if (state.canBeExtended) {
return (
<EuiFlexGroup justifyContent="flexEnd" gutterSize="s">
<EuiFlexItem grow={false}>
<EuiButton size="s" color="warning" isLoading={loading} onClick={extend}>
<FormattedMessage
id="xpack.security.sessionExpirationToast.extendButton"
defaultMessage="Keep me logged in"
/>
</EuiButton>
</EuiFlexItem>
</EuiFlexGroup>
<>
{expirationWarning}
<EuiSpacer size="m" />
<EuiFlexGroup justifyContent="flexEnd" gutterSize="s">
<EuiFlexItem grow={false}>
<EuiButton size="s" color="warning" isLoading={loading} onClick={extend}>
<FormattedMessage
id="xpack.security.sessionExpirationToast.extendButton"
defaultMessage="Stay logged in"
/>
</EuiButton>
</EuiFlexItem>
</EuiFlexGroup>
</>
);
}

return (
<FormattedMessage
id="xpack.security.components.sessionExpirationToast.endOfLifeWarning"
defaultMessage="Any unsaved changes will be lost."
/>
);
return expirationWarning;
};

export const createSessionExpirationToast = (
Expand All @@ -95,8 +81,12 @@ export const createSessionExpirationToast = (
return {
color: 'warning',
iconType: 'clock',
title: toMountPoint(<SessionExpirationTitle sessionState$={sessionState$} />),
text: toMountPoint(<SessionExpirationBody sessionState$={sessionState$} onExtend={onExtend} />),
title: i18n.translate('xpack.security.sessionExpirationToast.title', {
defaultMessage: 'Session timeout',
}),
text: toMountPoint(
<SessionExpirationToast sessionState$={sessionState$} onExtend={onExtend} />
),
onClose,
toastLifeTimeMs: 0x7fffffff, // Toast is hidden based on observable so using maximum possible timeout
};
Expand Down
18 changes: 0 additions & 18 deletions x-pack/plugins/security/public/session/session_timeout.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,24 +127,6 @@ describe('SessionTimeout', () => {
nowMock.mockClear();
});

it('extends session when receiving HTTP response', async () => {
const { sessionTimeout, http } = createSessionTimeout();
await sessionTimeout.start();
expect(http.fetch).toHaveBeenCalledTimes(1);

// Increment system time enough so that session extension gets triggered
nowMock.mockReturnValue(Date.now() + SESSION_EXTENSION_THROTTLE_MS + 10);
const [{ response: handleHttpResponse }] = http.intercept.mock.calls[0];
handleHttpResponse!({ fetchOptions: { asSystemRequest: false } } as any, {} as any);

expect(http.fetch).toHaveBeenCalledTimes(2);
expect(http.fetch).toHaveBeenLastCalledWith(
SESSION_ROUTE,
expect.objectContaining({ asSystemRequest: false })
);
nowMock.mockClear();
});

it('refetches session info before warning is displayed', async () => {
const { sessionTimeout, http } = createSessionTimeout(60 * 60 * 1000);
await sessionTimeout.start();
Expand Down
24 changes: 1 addition & 23 deletions x-pack/plugins/security/public/session/session_timeout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import { skip, tap, throttleTime } from 'rxjs/operators';

import type {
HttpFetchOptionsWithPath,
HttpResponse,
HttpSetup,
NotificationsSetup,
Toast,
Expand Down Expand Up @@ -125,24 +124,6 @@ export class SessionTimeout {
}
};

/**
* HTTP response interceptor which allows us to prevent extending the session manually if it has
* already been extended as part of an API call.
*/
private handleHttpResponse = (httpResponse: HttpResponse) => {
// Ignore session endpoint which is already handled by fetch callback
if (httpResponse.fetchOptions.path === SESSION_ROUTE) {
return;
}

// Extend session unless we're dealing with a system request
if (httpResponse.fetchOptions.asSystemRequest === false) {
if (this.shouldExtend()) {
this.fetchSessionInfo(true);
}
}
};

/**
* Event handler that tracks user activity and extends the session if needed.
*/
Expand Down Expand Up @@ -202,10 +183,7 @@ export class SessionTimeout {

// Intercept HTTP requests if session can expire
if (!this.removeHttpInterceptor) {
this.removeHttpInterceptor = this.http.intercept({
request: this.handleHttpRequest,
response: this.handleHttpResponse,
});
this.removeHttpInterceptor = this.http.intercept({ request: this.handleHttpRequest });
}

if (!this.stopVisibilityMonitor) {
Expand Down

0 comments on commit 45e56d8

Please sign in to comment.