Change session expiration to override on app leave behavior (elastic#…

…129384) * Change session expiration to override on app leave behavior * fix types
markov00 · Apr 5, 2022 · 735b4c9 · 735b4c9
1 parent 1ee5666
commit 735b4c9
Show file tree

Hide file tree

Showing 4 changed files with 59 additions and 38 deletions.
diff --git a/x-pack/plugins/security/public/plugin.tsx b/x-pack/plugins/security/public/plugin.tsx
@@ -6,7 +6,13 @@
  */
 
 import { i18n } from '@kbn/i18n';
-import type { CoreSetup, CoreStart, Plugin, PluginInitializerContext } from 'src/core/public';
+import type {
+  CoreSetup,
+  CoreStart,
+  HttpSetup,
+  Plugin,
+  PluginInitializerContext,
+} from 'src/core/public';
 import type { DataViewsPublicPluginStart } from 'src/plugins/data_views/public';
 import type { HomePublicPluginSetup } from 'src/plugins/home/public';
 import type { ManagementSetup, ManagementStart } from 'src/plugins/management/public';
@@ -56,7 +62,7 @@ export class SecurityPlugin
     >
 {
   private readonly config: ConfigType;
-  private sessionTimeout!: SessionTimeout;
+  private sessionTimeout?: SessionTimeout;
   private readonly authenticationService = new AuthenticationService();
   private readonly navControlService = new SecurityNavControlService();
   private readonly securityLicenseService = new SecurityLicenseService();
@@ -74,16 +80,6 @@ export class SecurityPlugin
     core: CoreSetup<PluginStartDependencies>,
     { home, licensing, management, share }: PluginSetupDependencies
   ): SecurityPluginSetup {
-    const { http, notifications } = core;
-    const { anonymousPaths } = http;
-
-    const logoutUrl = `${core.http.basePath.serverBasePath}/logout`;
-    const tenant = core.http.basePath.serverBasePath;
-
-    const sessionExpired = new SessionExpired(logoutUrl, tenant);
-    http.intercept(new UnauthorizedResponseHttpInterceptor(sessionExpired, anonymousPaths));
-    this.sessionTimeout = new SessionTimeout(notifications, sessionExpired, http, tenant);
-
     const { license } = this.securityLicenseService.setup({ license$: licensing.license$ });
 
     this.securityCheckupService.setup({ http: core.http });
@@ -99,7 +95,7 @@ export class SecurityPlugin
     this.navControlService.setup({
       securityLicense: license,
       authc: this.authc,
-      logoutUrl,
+      logoutUrl: getLogoutUrl(core.http),
     });
 
     accountManagementApp.create({
@@ -149,19 +145,25 @@ export class SecurityPlugin
     core: CoreStart,
     { management, share }: PluginStartDependencies
   ): SecurityPluginStart {
+    const { application, http, notifications, docLinks } = core;
+    const { anonymousPaths } = http;
+
+    const logoutUrl = getLogoutUrl(http);
+    const tenant = http.basePath.serverBasePath;
+
+    const sessionExpired = new SessionExpired(application, logoutUrl, tenant);
+    http.intercept(new UnauthorizedResponseHttpInterceptor(sessionExpired, anonymousPaths));
+    this.sessionTimeout = new SessionTimeout(notifications, sessionExpired, http, tenant);
+
     this.sessionTimeout.start();
-    this.securityCheckupService.start({
-      http: core.http,
-      notifications: core.notifications,
-      docLinks: core.docLinks,
-    });
+    this.securityCheckupService.start({ http, notifications, docLinks });
 
     if (management) {
-      this.managementService.start({ capabilities: core.application.capabilities });
+      this.managementService.start({ capabilities: application.capabilities });
     }
 
     if (share) {
-      this.anonymousAccessService.start({ http: core.http });
+      this.anonymousAccessService.start({ http });
     }
 
     return {
@@ -172,13 +174,17 @@ export class SecurityPlugin
   }
 
   public stop() {
-    this.sessionTimeout.stop();
+    this.sessionTimeout?.stop();
     this.navControlService.stop();
     this.securityLicenseService.stop();
     this.managementService.stop();
   }
 }
 
+function getLogoutUrl(http: HttpSetup) {
+  return `${http.basePath.serverBasePath}/logout`;
+}
+
 export interface SecurityPluginSetup {
   /**
    * Exposes authentication information about the currently logged in user.

diff --git a/x-pack/plugins/security/public/session/session_expired.test.ts b/x-pack/plugins/security/public/session/session_expired.test.ts
@@ -5,10 +5,13 @@
  * 2.0.
  */
 
+import { applicationServiceMock } from 'src/core/public/mocks';
+
 import { LogoutReason } from '../../common/types';
 import { SessionExpired } from './session_expired';
 
 describe('#logout', () => {
+  const application = applicationServiceMock.createStartContract();
   const mockGetItem = jest.fn().mockReturnValue(null);
   const CURRENT_URL = '/foo/bar?baz=quz#quuz';
   const LOGOUT_URL = '/logout';
@@ -26,13 +29,13 @@ describe('#logout', () => {
   beforeEach(() => {
     Object.defineProperty(window, 'location', {
       value: {
-        assign: jest.fn(),
         pathname: CURRENT_URL,
         search: '',
         hash: '',
       },
       configurable: true,
     });
+    application.navigateToUrl.mockClear();
     mockGetItem.mockReset();
   });
 
@@ -41,39 +44,42 @@ describe('#logout', () => {
   });
 
   it(`redirects user to the logout URL with 'msg' and 'next' parameters`, async () => {
-    const sessionExpired = new SessionExpired(LOGOUT_URL, TENANT);
+    const sessionExpired = new SessionExpired(application, LOGOUT_URL, TENANT);
     sessionExpired.logout(LogoutReason.SESSION_EXPIRED);
 
     const next = `&next=${encodeURIComponent(CURRENT_URL)}`;
-    await expect(window.location.assign).toHaveBeenCalledWith(
-      `${LOGOUT_URL}?msg=SESSION_EXPIRED${next}`
+    await expect(application.navigateToUrl).toHaveBeenCalledWith(
+      `${LOGOUT_URL}?msg=SESSION_EXPIRED${next}`,
+      { forceRedirect: true, skipAppLeave: true }
     );
   });
 
   it(`redirects user to the logout URL with custom reason 'msg'`, async () => {
-    const sessionExpired = new SessionExpired(LOGOUT_URL, TENANT);
+    const sessionExpired = new SessionExpired(application, LOGOUT_URL, TENANT);
     sessionExpired.logout(LogoutReason.AUTHENTICATION_ERROR);
 
     const next = `&next=${encodeURIComponent(CURRENT_URL)}`;
-    await expect(window.location.assign).toHaveBeenCalledWith(
-      `${LOGOUT_URL}?msg=AUTHENTICATION_ERROR${next}`
+    await expect(application.navigateToUrl).toHaveBeenCalledWith(
+      `${LOGOUT_URL}?msg=AUTHENTICATION_ERROR${next}`,
+      { forceRedirect: true, skipAppLeave: true }
     );
   });
 
   it(`adds 'provider' parameter when sessionStorage contains the provider name for this tenant`, async () => {
     const providerName = 'basic';
     mockGetItem.mockReturnValueOnce(providerName);
 
-    const sessionExpired = new SessionExpired(LOGOUT_URL, TENANT);
+    const sessionExpired = new SessionExpired(application, LOGOUT_URL, TENANT);
     sessionExpired.logout(LogoutReason.SESSION_EXPIRED);
 
     expect(mockGetItem).toHaveBeenCalledTimes(1);
     expect(mockGetItem).toHaveBeenCalledWith(`${TENANT}/session_provider`);
 
     const next = `&next=${encodeURIComponent(CURRENT_URL)}`;
     const provider = `&provider=${providerName}`;
-    await expect(window.location.assign).toBeCalledWith(
-      `${LOGOUT_URL}?msg=SESSION_EXPIRED${next}${provider}`
+    await expect(application.navigateToUrl).toBeCalledWith(
+      `${LOGOUT_URL}?msg=SESSION_EXPIRED${next}${provider}`,
+      { forceRedirect: true, skipAppLeave: true }
     );
   });
 });
diff --git a/x-pack/plugins/security/public/session/session_expired.ts b/x-pack/plugins/security/public/session/session_expired.ts
@@ -5,6 +5,8 @@
  * 2.0.
  */
 
+import type { ApplicationStart } from 'src/core/public';
+
 import {
   LOGOUT_PROVIDER_QUERY_STRING_PARAMETER,
   LOGOUT_REASON_QUERY_STRING_PARAMETER,
@@ -27,13 +29,18 @@ const getProviderParameter = (tenant: string) => {
 };
 
 export class SessionExpired {
-  constructor(private logoutUrl: string, private tenant: string) {}
+  constructor(
+    private application: ApplicationStart,
+    private logoutUrl: string,
+    private tenant: string
+  ) {}
 
   logout(reason: LogoutReason) {
     const next = getNextParameter();
     const provider = getProviderParameter(this.tenant);
-    window.location.assign(
-      `${this.logoutUrl}?${LOGOUT_REASON_QUERY_STRING_PARAMETER}=${reason}${next}${provider}`
+    this.application.navigateToUrl(
+      `${this.logoutUrl}?${LOGOUT_REASON_QUERY_STRING_PARAMETER}=${reason}${next}${provider}`,
+      { forceRedirect: true, skipAppLeave: true }
     );
   }
 }
diff --git a/x-pack/plugins/security/public/session/unauthorized_response_http_interceptor.test.ts b/x-pack/plugins/security/public/session/unauthorized_response_http_interceptor.test.ts
@@ -8,6 +8,7 @@
 // @ts-ignore
 import fetchMock from 'fetch-mock/es5/client';
 
+import { applicationServiceMock } from 'src/core/public/mocks';
 import { setup } from 'src/core/test_helpers/http_test_setup';
 
 import { SessionExpired } from './session_expired';
@@ -30,14 +31,15 @@ const setupHttp = (basePath: string) => {
   return http;
 };
 const tenant = '';
+const application = applicationServiceMock.createStartContract();
 
 afterEach(() => {
   fetchMock.restore();
 });
 
 it(`logs out 401 responses`, async () => {
   const http = setupHttp('/foo');
-  const sessionExpired = new SessionExpired(`${http.basePath}/logout`, tenant);
+  const sessionExpired = new SessionExpired(application, `${http.basePath}/logout`, tenant);
   const logoutPromise = new Promise<void>((resolve) => {
     jest.spyOn(sessionExpired, 'logout').mockImplementation(() => resolve());
   });
@@ -64,7 +66,7 @@ it(`ignores anonymous paths`, async () => {
   const http = setupHttp('/foo');
   const { anonymousPaths } = http;
   anonymousPaths.register('/bar');
-  const sessionExpired = new SessionExpired(`${http.basePath}/logout`, tenant);
+  const sessionExpired = new SessionExpired(application, `${http.basePath}/logout`, tenant);
   const interceptor = new UnauthorizedResponseHttpInterceptor(sessionExpired, anonymousPaths);
   http.intercept(interceptor);
   fetchMock.mock('*', 401);
@@ -75,7 +77,7 @@ it(`ignores anonymous paths`, async () => {
 
 it(`ignores errors which don't have a response, for example network connectivity issues`, async () => {
   const http = setupHttp('/foo');
-  const sessionExpired = new SessionExpired(`${http.basePath}/logout`, tenant);
+  const sessionExpired = new SessionExpired(application, `${http.basePath}/logout`, tenant);
   const interceptor = new UnauthorizedResponseHttpInterceptor(sessionExpired, http.anonymousPaths);
   http.intercept(interceptor);
   fetchMock.mock('*', new Promise((resolve, reject) => reject(new Error('Network is down'))));
@@ -86,7 +88,7 @@ it(`ignores errors which don't have a response, for example network connectivity
 
 it(`ignores requests which omit credentials`, async () => {
   const http = setupHttp('/foo');
-  const sessionExpired = new SessionExpired(`${http.basePath}/logout`, tenant);
+  const sessionExpired = new SessionExpired(application, `${http.basePath}/logout`, tenant);
   const interceptor = new UnauthorizedResponseHttpInterceptor(sessionExpired, http.anonymousPaths);
   http.intercept(interceptor);
   fetchMock.mock('*', 401);