Make all providers to preserve original URL when session expires.

x-pack/plugins/security/common/constants.ts

@@ -19,3 +19,6 @@ export const APPLICATION_PREFIX = 'kibana-';
 export const RESERVED_PRIVILEGES_APPLICATION_WILDCARD = 'kibana-*';
 
 export const AUTH_PROVIDER_HINT_QUERY_STRING_PARAMETER = 'auth_provider_hint';
+export const LOGOUT_PROVIDER_QUERY_STRING_PARAMETER = 'provider';
+export const LOGOUT_REASON_QUERY_STRING_PARAMETER = 'msg';
+export const NEXT_URL_QUERY_STRING_PARAMETER = 'next';

x-pack/plugins/security/common/model/authenticated_user.ts

@@ -4,8 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import type { AuthenticationProvider } from '../types';
-import { User } from './user';
+import type { AuthenticationProvider, User } from '.';
 
 const REALMS_ELIGIBLE_FOR_PASSWORD_CHANGE = ['reserved', 'native'];
 

x-pack/plugins/security/common/model/authentication_provider.test.ts

@@ -0,0 +1,21 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+import { shouldProviderUseLoginForm } from './authentication_provider';
+
+describe('#shouldProviderUseLoginForm', () => {
+  ['basic', 'token'].forEach((providerType) => {
+    it(`returns "true" for "${providerType}" provider`, () => {
+      expect(shouldProviderUseLoginForm(providerType)).toEqual(true);
+    });
+  });
+
+  ['anonymous', 'http', 'kerberos', 'oidc', 'pki', 'saml'].forEach((providerType) => {
+    it(`returns "false" for "${providerType}" provider`, () => {
+      expect(shouldProviderUseLoginForm(providerType)).toEqual(false);
+    });
+  });
+});

x-pack/plugins/security/common/model/authentication_provider.ts

@@ -0,0 +1,21 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+/**
+ * Type and name tuple to identify provider used to authenticate user.
+ */
+export interface AuthenticationProvider {
+  type: string;
+  name: string;
+}
+
+/**
+ * Checks whether authentication provider with the specified type uses Kibana's native login form.
+ * @param providerType Type of the authentication provider.
+ */
+export function shouldProviderUseLoginForm(providerType: string) {
+  return providerType === 'basic' || providerType === 'token';
+}

x-pack/plugins/security/common/model/index.ts

@@ -7,6 +7,7 @@
 export { ApiKey, ApiKeyToInvalidate } from './api_key';
 export { User, EditUser, getUserDisplayName } from './user';
 export { AuthenticatedUser, canUserChangePassword } from './authenticated_user';
+export { AuthenticationProvider, shouldProviderUseLoginForm } from './authentication_provider';
 export { BuiltinESPrivileges } from './builtin_es_privileges';
 export { RawKibanaPrivileges, RawKibanaFeaturePrivileges } from './raw_kibana_privileges';
 export { FeaturesPrivileges } from './features_privileges';

x-pack/plugins/security/common/parse_next.ts

@@ -5,19 +5,21 @@
  */
 
 import { parse } from 'url';
+import { NEXT_URL_QUERY_STRING_PARAMETER } from './constants';
 import { isInternalURL } from './is_internal_url';
 
 export function parseNext(href: string, basePath = '') {
   const { query, hash } = parse(href, true);
-  if (!query.next) {
+
+  let next = query[NEXT_URL_QUERY_STRING_PARAMETER];
+  if (!next) {
     return `${basePath}/`;
   }
 
-  let next: string;
-  if (Array.isArray(query.next) && query.next.length > 0) {
-    next = query.next[0];
+  if (Array.isArray(next) && next.length > 0) {
+    next = next[0];
   } else {
-    next = query.next as string;
+    next = next as string;
   }
 
   // validate that `next` is not attempting a redirect to somewhere

x-pack/plugins/security/common/types.ts

@@ -4,13 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-/**
- * Type and name tuple to identify provider used to authenticate user.
- */
-export interface AuthenticationProvider {
-  type: string;
-  name: string;
-}
+import type { AuthenticationProvider } from './model';
 
 export interface SessionInfo {
   now: number;

x-pack/plugins/security/public/authentication/logged_out/logged_out_page.test.tsx

@@ -0,0 +1,39 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+import React from 'react';
+import { EuiButton } from '@elastic/eui';
+import { mountWithIntl } from '@kbn/test/jest';
+import { LoggedOutPage } from './logged_out_page';
+
+import { coreMock } from '../../../../../../src/core/public/mocks';
+
+describe('LoggedOutPage', () => {
+  beforeAll(() => {
+    Object.defineProperty(window, 'location', {
+      value: { href: 'https://some-host' },
+      writable: true,
+    });
+  });
+
+  it('points to a base path if `next` parameter is not provided', async () => {
+    const basePathMock = coreMock.createStart({ basePath: '/mock-base-path' }).http.basePath;
+    const wrapper = mountWithIntl(<LoggedOutPage basePath={basePathMock} />);
+
+    expect(wrapper.find(EuiButton).prop('href')).toBe('/mock-base-path/');
+  });
+
+  it('properly parses `next` parameter', async () => {
+    window.location.href = `https://host.com/mock-base-path/security/logged_out?next=${encodeURIComponent(
+      '/mock-base-path/app/home#/?_g=()'
+    )}`;
+
+    const basePathMock = coreMock.createStart({ basePath: '/mock-base-path' }).http.basePath;
+    const wrapper = mountWithIntl(<LoggedOutPage basePath={basePathMock} />);
+
+    expect(wrapper.find(EuiButton).prop('href')).toBe('/mock-base-path/app/home#/?_g=()');
+  });
+});

x-pack/plugins/security/public/authentication/logged_out/logged_out_page.tsx

@@ -9,6 +9,7 @@ import ReactDOM from 'react-dom';
 import { EuiButton } from '@elastic/eui';
 import { FormattedMessage } from '@kbn/i18n/react';
 import { CoreStart, IBasePath } from 'src/core/public';
+import { parseNext } from '../../../common/parse_next';
 import { AuthenticationStatePage } from '../components';
 
 interface Props {
@@ -25,7 +26,7 @@ export function LoggedOutPage({ basePath }: Props) {
         />
       }
     >
-      <EuiButton href={basePath.prepend('/')}>
+      <EuiButton href={parseNext(window.location.href, basePath.serverBasePath)}>
         <FormattedMessage id="xpack.security.loggedOut.login" defaultMessage="Log in" />
       </EuiButton>
     </AuthenticationStatePage>

x-pack/plugins/security/public/authentication/login/login_page.tsx

@@ -15,7 +15,10 @@ import { EuiFlexGroup, EuiFlexItem, EuiIcon, EuiSpacer, EuiTitle } from '@elasti
 import { i18n } from '@kbn/i18n';
 import { FormattedMessage } from '@kbn/i18n/react';
 import { CoreStart, FatalErrorsStart, HttpStart, NotificationsStart } from 'src/core/public';
-import { AUTH_PROVIDER_HINT_QUERY_STRING_PARAMETER } from '../../../common/constants';
+import {
+  AUTH_PROVIDER_HINT_QUERY_STRING_PARAMETER,
+  LOGOUT_REASON_QUERY_STRING_PARAMETER,
+} from '../../../common/constants';
 import { LoginState } from '../../../common/login_state';
 import { LoginForm, DisabledLoginForm } from './components';
 
@@ -219,7 +222,7 @@ export class LoginPage extends Component<Props, State> {
         http={this.props.http}
         notifications={this.props.notifications}
         selector={selector}
-        infoMessage={infoMessageMap.get(query.msg?.toString())}
+        infoMessage={infoMessageMap.get(query[LOGOUT_REASON_QUERY_STRING_PARAMETER]?.toString())}
         loginAssistanceMessage={this.props.loginAssistanceMessage}
         loginHelp={loginHelp}
         authProviderHint={query[AUTH_PROVIDER_HINT_QUERY_STRING_PARAMETER]?.toString()}

x-pack/plugins/security/public/session/session_expired.ts

@@ -4,20 +4,28 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
+import {
+  LOGOUT_PROVIDER_QUERY_STRING_PARAMETER,
+  LOGOUT_REASON_QUERY_STRING_PARAMETER,
+  NEXT_URL_QUERY_STRING_PARAMETER,
+} from '../../common/constants';
+
 export interface ISessionExpired {
   logout(): void;
 }
 
 const getNextParameter = () => {
   const { location } = window;
   const next = encodeURIComponent(`${location.pathname}${location.search}${location.hash}`);
-  return `&next=${next}`;
+  return `&${NEXT_URL_QUERY_STRING_PARAMETER}=${next}`;
 };
 
 const getProviderParameter = (tenant: string) => {
   const key = `${tenant}/session_provider`;
   const providerName = sessionStorage.getItem(key);
-  return providerName ? `&provider=${encodeURIComponent(providerName)}` : '';
+  return providerName
+    ? `&${LOGOUT_PROVIDER_QUERY_STRING_PARAMETER}=${encodeURIComponent(providerName)}`
+    : '';
 };
 
 export class SessionExpired {
@@ -26,6 +34,8 @@ export class SessionExpired {
   logout() {
     const next = getNextParameter();
     const provider = getProviderParameter(this.tenant);
-    window.location.assign(`${this.logoutUrl}?msg=SESSION_EXPIRED${next}${provider}`);
+    window.location.assign(
+      `${this.logoutUrl}?${LOGOUT_REASON_QUERY_STRING_PARAMETER}=SESSION_EXPIRED${next}${provider}`
+    );
   }
 }

x-pack/plugins/security/server/audit/security_audit_logger.ts

@@ -4,7 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import { AuthenticationProvider } from '../../common/types';
+import type { AuthenticationProvider } from '../../common/model';
 import { LegacyAuditLogger } from './audit_service';
 
 /**

x-pack/plugins/security/server/authentication/authenticator.test.ts

@@ -111,31 +111,78 @@ describe('Authenticator', () => {
       ).toThrowError('Provider name "__http__" is reserved.');
     });
 
-    it('properly sets `loggedOut` URL.', () => {
-      const basicAuthenticationProviderMock = jest.requireMock('./providers/basic')
-        .BasicAuthenticationProvider;
+    describe('#options.urls.loggedOut', () => {
+      it('points to /login if provider requires login form', () => {
+        const authenticationProviderMock = jest.requireMock(`./providers/basic`)
+          .BasicAuthenticationProvider;
+        authenticationProviderMock.mockClear();
+        new Authenticator(getMockOptions());
+        const getLoggedOutURL = authenticationProviderMock.mock.calls[0][0].urls.loggedOut;
 
-      basicAuthenticationProviderMock.mockClear();
-      new Authenticator(getMockOptions());
-      expect(basicAuthenticationProviderMock).toHaveBeenCalledWith(
-        expect.objectContaining({
-          urls: {
-            loggedOut: '/mock-server-basepath/security/logged_out',
-          },
-        }),
-        expect.anything()
-      );
+        expect(getLoggedOutURL(httpServerMock.createKibanaRequest())).toBe(
+          '/mock-server-basepath/login?msg=LOGGED_OUT'
+        );
 
-      basicAuthenticationProviderMock.mockClear();
-      new Authenticator(getMockOptions({ selector: { enabled: true } }));
-      expect(basicAuthenticationProviderMock).toHaveBeenCalledWith(
-        expect.objectContaining({
-          urls: {
-            loggedOut: `/mock-server-basepath/login?msg=LOGGED_OUT`,
-          },
-        }),
-        expect.anything()
-      );
+        expect(
+          getLoggedOutURL(
+            httpServerMock.createKibanaRequest({
+              query: { next: '/app/ml/encode me', msg: 'SESSION_EXPIRED' },
+            })
+          )
+        ).toBe('/mock-server-basepath/login?next=%2Fapp%2Fml%2Fencode+me&msg=SESSION_EXPIRED');
+      });
+
+      it('points to /login if login selector is enabled', () => {
+        const authenticationProviderMock = jest.requireMock(`./providers/saml`)
+          .SAMLAuthenticationProvider;
+        authenticationProviderMock.mockClear();
+        new Authenticator(
+          getMockOptions({
+            selector: { enabled: true },
+            providers: { saml: { saml1: { order: 0, realm: 'realm' } } },
+          })
+        );
+        const getLoggedOutURL = authenticationProviderMock.mock.calls[0][0].urls.loggedOut;
+
+        expect(getLoggedOutURL(httpServerMock.createKibanaRequest())).toBe(
+          '/mock-server-basepath/login?msg=LOGGED_OUT'
+        );
+
+        expect(
+          getLoggedOutURL(
+            httpServerMock.createKibanaRequest({
+              query: { next: '/app/ml/encode me', msg: 'SESSION_EXPIRED' },
+            })
+          )
+        ).toBe('/mock-server-basepath/login?next=%2Fapp%2Fml%2Fencode+me&msg=SESSION_EXPIRED');
+      });
+
+      it('points to /security/logged_out if login selector is NOT enabled', () => {
+        const authenticationProviderMock = jest.requireMock(`./providers/saml`)
+          .SAMLAuthenticationProvider;
+        authenticationProviderMock.mockClear();
+        new Authenticator(
+          getMockOptions({
+            selector: { enabled: false },
+            providers: { saml: { saml1: { order: 0, realm: 'realm' } } },
+          })
+        );
+        const getLoggedOutURL = authenticationProviderMock.mock.calls[0][0].urls.loggedOut;
+
+        expect(getLoggedOutURL(httpServerMock.createKibanaRequest())).toBe(
+          '/mock-server-basepath/security/logged_out?msg=LOGGED_OUT'
+        );
+
+        expect(
+          getLoggedOutURL(
+            httpServerMock.createKibanaRequest({
+              query: { next: '/app/ml/encode me', msg: 'SESSION_EXPIRED' },
+            })
+          )
+        ).toBe(
+          '/mock-server-basepath/security/logged_out?next=%2Fapp%2Fml%2Fencode+me&msg=SESSION_EXPIRED'
+        );
+      });
     });
 
     describe('HTTP authentication provider', () => {
@@ -1769,7 +1816,9 @@ describe('Authenticator', () => {
     });
 
     it('if session does not exist but provider name is valid, returns whatever authentication provider returns.', async () => {
-      const request = httpServerMock.createKibanaRequest({ query: { provider: 'basic1' } });
+      const request = httpServerMock.createKibanaRequest({
+        query: { provider: 'basic1' },
+      });
       mockOptions.session.get.mockResolvedValue(null);
 
       mockBasicAuthenticationProvider.logout.mockResolvedValue(
@@ -1782,7 +1831,7 @@ describe('Authenticator', () => {
 
       expect(mockBasicAuthenticationProvider.logout).toHaveBeenCalledTimes(1);
       expect(mockBasicAuthenticationProvider.logout).toHaveBeenCalledWith(request, null);
-      expect(mockOptions.session.clear).not.toHaveBeenCalled();
+      expect(mockOptions.session.clear).toHaveBeenCalled();
     });
 
     it('if session does not exist and provider name is not available, returns whatever authentication provider returns.', async () => {
@@ -1811,7 +1860,7 @@ describe('Authenticator', () => {
       );
 
       expect(mockBasicAuthenticationProvider.logout).not.toHaveBeenCalled();
-      expect(mockOptions.session.clear).not.toHaveBeenCalled();
+      expect(mockOptions.session.clear).toHaveBeenCalled();
     });
   });