From 909ead274611fed8fe819e640a4e98facefdd155 Mon Sep 17 00:00:00 2001
From: Joe Portner <5295965+jportner@users.noreply.github.com>
Date: Mon, 9 Dec 2019 13:38:21 -0500
Subject: [PATCH 1/8] Change session timeout values to use duration instead of
 number

Now these can be formatted in the config file as human-readable
strings.
---
 docs/user/security/securing-kibana.asciidoc   | 14 +++++----
 .../authentication/authenticator.test.ts      | 11 +++----
 .../server/authentication/authenticator.ts    |  6 ++--
 x-pack/plugins/security/server/config.ts      |  4 +--
 x-pack/plugins/security/server/plugin.ts      |  5 ++--
 x-pack/plugins/security/server/utils.test.ts  | 30 +++++++++++++++++++
 x-pack/plugins/security/server/utils.ts       | 15 ++++++++++
 7 files changed, 67 insertions(+), 18 deletions(-)
 create mode 100644 x-pack/plugins/security/server/utils.test.ts
 create mode 100644 x-pack/plugins/security/server/utils.ts

diff --git a/docs/user/security/securing-kibana.asciidoc b/docs/user/security/securing-kibana.asciidoc
index 60f5473f43b9d..a68a2ee285ee3 100644
--- a/docs/user/security/securing-kibana.asciidoc
+++ b/docs/user/security/securing-kibana.asciidoc
@@ -59,13 +59,14 @@ For more information, see <<security-settings-kb,Security Settings in {kib}>>.
 . Optional: Set a timeout to expire idle sessions. By default, a session stays
 active until the browser is closed. To define a sliding session expiration, set
 the `xpack.security.session.idleTimeout` property in the `kibana.yml`
-configuration file. The idle timeout is specified in milliseconds. For example,
-set the idle timeout to 600000 to expire idle sessions after 10 minutes:
+configuration file. The idle timeout is formatted as a duration of
+`<count>[ms|s|m|h|d|w|M|Y]` (e.g. '70ms', '5s', '3d', '1Y'). For example, set
+the idle timeout to expire idle sessions after 10 minutes:
 +
 --
 [source,yaml]
 --------------------------------------------------------------------------------
-xpack.security.session.idleTimeout: 600000
+xpack.security.session.idleTimeout: "10m"
 --------------------------------------------------------------------------------
 --
 
@@ -74,13 +75,14 @@ the "absolute timeout". By default, a session stays active until the browser is
 closed. If an idle timeout is defined, a session can still be extended
 indefinitely. To define a maximum session lifespan, set the
 `xpack.security.session.lifespan` property in the `kibana.yml` configuration
-file. The lifespan is specified in milliseconds. For example, set the lifespan
-to 28800000 to expire sessions after 8 hours:
+file. The lifespan is formatted as a duration of `<count>[ms|s|m|h|d|w|M|Y]`
+(e.g. '70ms', '5s', '3d', '1Y'). For example, set the lifespan to expire
+sessions after 8 hours:
 +
 --
 [source,yaml]
 --------------------------------------------------------------------------------
-xpack.security.session.lifespan: 28800000
+xpack.security.session.lifespan: "8h"
 --------------------------------------------------------------------------------
 --
 
diff --git a/x-pack/plugins/security/server/authentication/authenticator.test.ts b/x-pack/plugins/security/server/authentication/authenticator.test.ts
index a81246c8f78b0..55602d3e08d87 100644
--- a/x-pack/plugins/security/server/authentication/authenticator.test.ts
+++ b/x-pack/plugins/security/server/authentication/authenticator.test.ts
@@ -7,6 +7,7 @@
 jest.mock('./providers/basic', () => ({ BasicAuthenticationProvider: jest.fn() }));
 
 import Boom from 'boom';
+import { duration, Duration } from 'moment';
 import { SessionStorage } from '../../../../../src/core/server';
 
 import {
@@ -478,8 +479,8 @@ describe('Authenticator', () => {
       // Create new authenticator with non-null session `idleTimeout` and `lifespan`.
       mockOptions = getMockOptions({
         session: {
-          idleTimeout: hr * 2,
-          lifespan: hr * 8,
+          idleTimeout: duration(hr * 2),
+          lifespan: duration(hr * 8),
         },
         authc: { providers: ['basic'], oidc: {}, saml: {} },
       });
@@ -520,7 +521,7 @@ describe('Authenticator', () => {
       const currentDate = new Date(Date.UTC(2019, 10, 10)).valueOf();
 
       async function createAndUpdateSession(
-        lifespan: number | null,
+        lifespan: Duration | null,
         oldExpiration: number | null,
         newExpiration: number | null
       ) {
@@ -564,7 +565,7 @@ describe('Authenticator', () => {
       }
 
       it('does not change a non-null lifespan expiration when configured to non-null value.', async () => {
-        await createAndUpdateSession(hr * 8, 1234, 1234);
+        await createAndUpdateSession(duration(hr * 8), 1234, 1234);
       });
       it('does not change a null lifespan expiration when configured to null value.', async () => {
         await createAndUpdateSession(null, null, null);
@@ -573,7 +574,7 @@ describe('Authenticator', () => {
         await createAndUpdateSession(null, 1234, null);
       });
       it('does change a null lifespan expiration when configured to non-null value', async () => {
-        await createAndUpdateSession(hr * 8, null, currentDate + hr * 8);
+        await createAndUpdateSession(duration(hr * 8), null, currentDate + hr * 8);
       });
     });
 
diff --git a/x-pack/plugins/security/server/authentication/authenticator.ts b/x-pack/plugins/security/server/authentication/authenticator.ts
index 8f947349cb2e8..58095b6c21584 100644
--- a/x-pack/plugins/security/server/authentication/authenticator.ts
+++ b/x-pack/plugins/security/server/authentication/authenticator.ts
@@ -31,6 +31,7 @@ import {
 import { AuthenticationResult } from './authentication_result';
 import { DeauthenticationResult } from './deauthentication_result';
 import { Tokens } from './tokens';
+import { durationToMs } from '../utils';
 import { SessionInfo } from '../../public/types';
 
 /**
@@ -225,9 +226,8 @@ export class Authenticator {
     );
     this.serverBasePath = this.options.basePath.serverBasePath || '/';
 
-    // only set these vars if they are defined in options (otherwise coalesce to existing/default)
-    this.idleTimeout = this.options.config.session.idleTimeout;
-    this.lifespan = this.options.config.session.lifespan;
+    this.idleTimeout = durationToMs(this.options.config.session.idleTimeout);
+    this.lifespan = durationToMs(this.options.config.session.lifespan);
   }
 
   /**
diff --git a/x-pack/plugins/security/server/config.ts b/x-pack/plugins/security/server/config.ts
index c7d990f81369e..faeaac4f59a2b 100644
--- a/x-pack/plugins/security/server/config.ts
+++ b/x-pack/plugins/security/server/config.ts
@@ -36,8 +36,8 @@ export const ConfigSchema = schema.object(
     ),
     sessionTimeout: schema.maybe(schema.oneOf([schema.number(), schema.literal(null)])), // DEPRECATED
     session: schema.object({
-      idleTimeout: schema.oneOf([schema.number(), schema.literal(null)], { defaultValue: null }),
-      lifespan: schema.oneOf([schema.number(), schema.literal(null)], { defaultValue: null }),
+      idleTimeout: schema.oneOf([schema.duration(), schema.literal(null)], { defaultValue: null }),
+      lifespan: schema.oneOf([schema.duration(), schema.literal(null)], { defaultValue: null }),
     }),
     secureCookies: schema.boolean({ defaultValue: false }),
     authc: schema.object({
diff --git a/x-pack/plugins/security/server/plugin.ts b/x-pack/plugins/security/server/plugin.ts
index cb197ecaf7e10..121390dd3b233 100644
--- a/x-pack/plugins/security/server/plugin.ts
+++ b/x-pack/plugins/security/server/plugin.ts
@@ -25,6 +25,7 @@ import { Authentication, setupAuthentication } from './authentication';
 import { Authorization, setupAuthorization } from './authorization';
 import { createConfig$ } from './config';
 import { defineRoutes } from './routes';
+import { durationToMs } from './utils';
 import { SecurityLicenseService, SecurityLicense } from './licensing';
 import { setupSavedObjects } from './saved_objects';
 import { SecurityAuditLogger } from './audit';
@@ -206,8 +207,8 @@ export class Plugin {
         config: {
           loginAssistanceMessage: config.loginAssistanceMessage,
           session: {
-            idleTimeout: config.session.idleTimeout,
-            lifespan: config.session.lifespan,
+            idleTimeout: durationToMs(config.session.idleTimeout),
+            lifespan: durationToMs(config.session.lifespan),
           },
           secureCookies: config.secureCookies,
           cookieName: config.cookieName,
diff --git a/x-pack/plugins/security/server/utils.test.ts b/x-pack/plugins/security/server/utils.test.ts
new file mode 100644
index 0000000000000..cc3a6b2360eb9
--- /dev/null
+++ b/x-pack/plugins/security/server/utils.test.ts
@@ -0,0 +1,30 @@
+/*
+ * 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 { duration } from 'moment';
+import { durationToMs } from './utils';
+
+describe('security utils', () => {
+  describe('#durationToMs', () => {
+    const sixtySeconds = 60000;
+
+    it('converts a duration to a number', () => {
+      const _duration = duration(sixtySeconds);
+      const result = durationToMs(_duration);
+      expect(result).toEqual(sixtySeconds);
+    });
+
+    it('returns a number', () => {
+      const result = durationToMs(sixtySeconds);
+      expect(result).toEqual(sixtySeconds);
+    });
+
+    it('returns null', () => {
+      const result = durationToMs(null);
+      expect(result).toEqual(null);
+    });
+  });
+});
diff --git a/x-pack/plugins/security/server/utils.ts b/x-pack/plugins/security/server/utils.ts
new file mode 100644
index 0000000000000..57b09e05038a2
--- /dev/null
+++ b/x-pack/plugins/security/server/utils.ts
@@ -0,0 +1,15 @@
+/*
+ * 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 { Duration } from 'moment';
+
+export function durationToMs(duration: number | Duration | null) {
+  if (duration === null || typeof duration === 'number') {
+    return duration;
+  }
+
+  return duration.asMilliseconds();
+}

From fa225f36056dbacf407f59ca920ede4e354639f1 Mon Sep 17 00:00:00 2001
From: Joe Portner <5295965+jportner@users.noreply.github.com>
Date: Tue, 10 Dec 2019 11:55:35 -0500
Subject: [PATCH 2/8] Use schema.nullable where applicable

---
 x-pack/plugins/security/server/config.ts | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/x-pack/plugins/security/server/config.ts b/x-pack/plugins/security/server/config.ts
index faeaac4f59a2b..2e67b3eb9b49d 100644
--- a/x-pack/plugins/security/server/config.ts
+++ b/x-pack/plugins/security/server/config.ts
@@ -34,10 +34,10 @@ export const ConfigSchema = schema.object(
       schema.maybe(schema.string({ minLength: 32 })),
       schema.string({ minLength: 32, defaultValue: 'a'.repeat(32) })
     ),
-    sessionTimeout: schema.maybe(schema.oneOf([schema.number(), schema.literal(null)])), // DEPRECATED
+    sessionTimeout: schema.maybe(schema.nullable(schema.number())), // DEPRECATED
     session: schema.object({
-      idleTimeout: schema.oneOf([schema.duration(), schema.literal(null)], { defaultValue: null }),
-      lifespan: schema.oneOf([schema.duration(), schema.literal(null)], { defaultValue: null }),
+      idleTimeout: schema.nullable(schema.duration()),
+      lifespan: schema.nullable(schema.duration()),
     }),
     secureCookies: schema.boolean({ defaultValue: false }),
     authc: schema.object({

From 5371f5477370420e069ba62a80a6beb0abf3d1cf Mon Sep 17 00:00:00 2001
From: Joe Portner <5295965+jportner@users.noreply.github.com>
Date: Thu, 12 Dec 2019 11:19:17 -0500
Subject: [PATCH 3/8] Use Duration for `session.idleTimeout` and
 `session.lifespan`

The plugin now uses Duration natively instead of converting these
values to numbers.
Also removed unused vars from legacyCompat.
---
 x-pack/legacy/plugins/security/index.js       | 17 +++++------
 .../authentication/authenticator.test.ts      |  2 +-
 .../server/authentication/authenticator.ts    | 15 +++++-----
 x-pack/plugins/security/server/config.ts      | 14 ++++-----
 x-pack/plugins/security/server/plugin.ts      |  9 ------
 x-pack/plugins/security/server/utils.test.ts  | 30 -------------------
 x-pack/plugins/security/server/utils.ts       | 15 ----------
 7 files changed, 24 insertions(+), 78 deletions(-)
 delete mode 100644 x-pack/plugins/security/server/utils.test.ts
 delete mode 100644 x-pack/plugins/security/server/utils.ts

diff --git a/x-pack/legacy/plugins/security/index.js b/x-pack/legacy/plugins/security/index.js
index 3a6f3692bc0b6..55963ae4b5c3d 100644
--- a/x-pack/legacy/plugins/security/index.js
+++ b/x-pack/legacy/plugins/security/index.js
@@ -21,16 +21,17 @@ export const security = (kibana) => new kibana.Plugin({
   require: ['kibana', 'elasticsearch', 'xpack_main'],
 
   config(Joi) {
+    const HANDLED_IN_NEW_PLATFORM = Joi.any().description('This key is handled in the new platform security plugin ONLY');
     return Joi.object({
       enabled: Joi.boolean().default(true),
-      cookieName: Joi.any().description('This key is handled in the new platform security plugin ONLY'),
-      encryptionKey: Joi.any().description('This key is handled in the new platform security plugin ONLY'),
+      cookieName: HANDLED_IN_NEW_PLATFORM,
+      encryptionKey: HANDLED_IN_NEW_PLATFORM,
       session: Joi.object({
-        idleTimeout: Joi.any().description('This key is handled in the new platform security plugin ONLY'),
-        lifespan: Joi.any().description('This key is handled in the new platform security plugin ONLY'),
+        idleTimeout: HANDLED_IN_NEW_PLATFORM,
+        lifespan: HANDLED_IN_NEW_PLATFORM,
       }).default(),
-      secureCookies: Joi.any().description('This key is handled in the new platform security plugin ONLY'),
-      loginAssistanceMessage: Joi.any().description('This key is handled in the new platform security plugin ONLY'),
+      secureCookies: HANDLED_IN_NEW_PLATFORM,
+      loginAssistanceMessage: HANDLED_IN_NEW_PLATFORM,
       authorization: Joi.object({
         legacyFallback: Joi.object({
           enabled: Joi.boolean().default(true) // deprecated
@@ -39,7 +40,7 @@ export const security = (kibana) => new kibana.Plugin({
       audit: Joi.object({
         enabled: Joi.boolean().default(false)
       }).default(),
-      authc: Joi.any().description('This key is handled in the new platform security plugin ONLY')
+      authc: HANDLED_IN_NEW_PLATFORM
     }).default();
   },
 
@@ -91,8 +92,6 @@ export const security = (kibana) => new kibana.Plugin({
         secureCookies: securityPlugin.__legacyCompat.config.secureCookies,
         session: {
           tenant: server.newPlatform.setup.core.http.basePath.serverBasePath,
-          idleTimeout: securityPlugin.__legacyCompat.config.session.idleTimeout,
-          lifespan: securityPlugin.__legacyCompat.config.session.lifespan,
         },
         enableSpaceAwarePrivileges: server.config().get('xpack.spaces.enabled'),
       };
diff --git a/x-pack/plugins/security/server/authentication/authenticator.test.ts b/x-pack/plugins/security/server/authentication/authenticator.test.ts
index 55602d3e08d87..dd580c890bf94 100644
--- a/x-pack/plugins/security/server/authentication/authenticator.test.ts
+++ b/x-pack/plugins/security/server/authentication/authenticator.test.ts
@@ -440,7 +440,7 @@ describe('Authenticator', () => {
       // Create new authenticator with non-null session `idleTimeout`.
       mockOptions = getMockOptions({
         session: {
-          idleTimeout: 3600 * 24,
+          idleTimeout: duration(3600 * 24),
           lifespan: null,
         },
         authc: { providers: ['basic'], oidc: {}, saml: {} },
diff --git a/x-pack/plugins/security/server/authentication/authenticator.ts b/x-pack/plugins/security/server/authentication/authenticator.ts
index 58095b6c21584..8aff6ca1a1b23 100644
--- a/x-pack/plugins/security/server/authentication/authenticator.ts
+++ b/x-pack/plugins/security/server/authentication/authenticator.ts
@@ -4,6 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
+import { Duration } from 'moment';
 import {
   SessionStorageFactory,
   SessionStorage,
@@ -31,7 +32,6 @@ import {
 import { AuthenticationResult } from './authentication_result';
 import { DeauthenticationResult } from './deauthentication_result';
 import { Tokens } from './tokens';
-import { durationToMs } from '../utils';
 import { SessionInfo } from '../../public/types';
 
 /**
@@ -173,12 +173,12 @@ export class Authenticator {
   /**
    * Session timeout in ms. If `null` session will stay active until the browser is closed.
    */
-  private readonly idleTimeout: number | null = null;
+  private readonly idleTimeout: Duration | null = null;
 
   /**
    * Session max lifespan in ms. If `null` session may live indefinitely.
    */
-  private readonly lifespan: number | null = null;
+  private readonly lifespan: Duration | null = null;
 
   /**
    * Internal authenticator logger.
@@ -226,8 +226,8 @@ export class Authenticator {
     );
     this.serverBasePath = this.options.basePath.serverBasePath || '/';
 
-    this.idleTimeout = durationToMs(this.options.config.session.idleTimeout);
-    this.lifespan = durationToMs(this.options.config.session.lifespan);
+    this.idleTimeout = this.options.config.session.idleTimeout;
+    this.lifespan = this.options.config.session.lifespan;
   }
 
   /**
@@ -492,11 +492,12 @@ export class Authenticator {
   private calculateExpiry(
     existingSession: ProviderSession | null
   ): { idleTimeoutExpiration: number | null; lifespanExpiration: number | null } {
-    let lifespanExpiration = this.lifespan && Date.now() + this.lifespan;
+    const now = Date.now();
+    let lifespanExpiration = this.lifespan && now + this.lifespan.asMilliseconds();
     if (existingSession && existingSession.lifespanExpiration && this.lifespan) {
       lifespanExpiration = existingSession.lifespanExpiration;
     }
-    const idleTimeoutExpiration = this.idleTimeout && Date.now() + this.idleTimeout;
+    const idleTimeoutExpiration = this.idleTimeout && now + this.idleTimeout.asMilliseconds();
 
     return { idleTimeoutExpiration, lifespanExpiration };
   }
diff --git a/x-pack/plugins/security/server/config.ts b/x-pack/plugins/security/server/config.ts
index 2e67b3eb9b49d..b3f96497b0538 100644
--- a/x-pack/plugins/security/server/config.ts
+++ b/x-pack/plugins/security/server/config.ts
@@ -8,6 +8,7 @@ import crypto from 'crypto';
 import { Observable } from 'rxjs';
 import { map } from 'rxjs/operators';
 import { schema, Type, TypeOf } from '@kbn/config-schema';
+import { duration } from 'moment';
 import { PluginInitializerContext } from '../../../../src/core/server';
 
 export type ConfigType = ReturnType<typeof createConfig$> extends Observable<infer P>
@@ -90,17 +91,16 @@ export function createConfig$(context: PluginInitializerContext, isTLSEnabled: b
       // "sessionTimeout" is deprecated and replaced with "session.idleTimeout"
       // however, NP does not yet have a mechanism to automatically rename deprecated keys
       // for the time being, we'll do it manually:
-      const sess = config.session;
-      const session = {
-        idleTimeout: (sess && sess.idleTimeout) || config.sessionTimeout || null,
-        lifespan: (sess && sess.lifespan) || null,
-      };
-
+      const deprecatedSessionTimeout =
+        typeof config.sessionTimeout === 'number' ? duration(config.sessionTimeout) : null;
       const val = {
         ...config,
         encryptionKey,
         secureCookies,
-        session,
+        session: {
+          ...config.session,
+          idleTimeout: config.session.idleTimeout || deprecatedSessionTimeout,
+        },
       };
       delete val.sessionTimeout; // DEPRECATED
       return val;
diff --git a/x-pack/plugins/security/server/plugin.ts b/x-pack/plugins/security/server/plugin.ts
index 2b7618e66de7c..a395278a5143e 100644
--- a/x-pack/plugins/security/server/plugin.ts
+++ b/x-pack/plugins/security/server/plugin.ts
@@ -25,7 +25,6 @@ import { Authentication, setupAuthentication } from './authentication';
 import { Authorization, setupAuthorization } from './authorization';
 import { createConfig$ } from './config';
 import { defineRoutes } from './routes';
-import { durationToMs } from './utils';
 import { SecurityLicenseService, SecurityLicense } from './licensing';
 import { setupSavedObjects } from './saved_objects';
 import { SecurityAuditLogger } from './audit';
@@ -73,10 +72,6 @@ export interface PluginSetupContract {
     registerPrivilegesWithCluster: () => void;
     license: SecurityLicense;
     config: RecursiveReadonly<{
-      session: {
-        idleTimeout: number | null;
-        lifespan: number | null;
-      };
       secureCookies: boolean;
       cookieName: string;
       loginAssistanceMessage: string;
@@ -210,10 +205,6 @@ export class Plugin {
         // exception may be `sessionTimeout` as other parts of the app may want to know it.
         config: {
           loginAssistanceMessage: config.loginAssistanceMessage,
-          session: {
-            idleTimeout: durationToMs(config.session.idleTimeout),
-            lifespan: durationToMs(config.session.lifespan),
-          },
           secureCookies: config.secureCookies,
           cookieName: config.cookieName,
         },
diff --git a/x-pack/plugins/security/server/utils.test.ts b/x-pack/plugins/security/server/utils.test.ts
deleted file mode 100644
index cc3a6b2360eb9..0000000000000
--- a/x-pack/plugins/security/server/utils.test.ts
+++ /dev/null
@@ -1,30 +0,0 @@
-/*
- * 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 { duration } from 'moment';
-import { durationToMs } from './utils';
-
-describe('security utils', () => {
-  describe('#durationToMs', () => {
-    const sixtySeconds = 60000;
-
-    it('converts a duration to a number', () => {
-      const _duration = duration(sixtySeconds);
-      const result = durationToMs(_duration);
-      expect(result).toEqual(sixtySeconds);
-    });
-
-    it('returns a number', () => {
-      const result = durationToMs(sixtySeconds);
-      expect(result).toEqual(sixtySeconds);
-    });
-
-    it('returns null', () => {
-      const result = durationToMs(null);
-      expect(result).toEqual(null);
-    });
-  });
-});
diff --git a/x-pack/plugins/security/server/utils.ts b/x-pack/plugins/security/server/utils.ts
deleted file mode 100644
index 57b09e05038a2..0000000000000
--- a/x-pack/plugins/security/server/utils.ts
+++ /dev/null
@@ -1,15 +0,0 @@
-/*
- * 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 { Duration } from 'moment';
-
-export function durationToMs(duration: number | Duration | null) {
-  if (duration === null || typeof duration === 'number') {
-    return duration;
-  }
-
-  return duration.asMilliseconds();
-}

From 0175b5fffbf329031f076cecad0849d6f0c07f0e Mon Sep 17 00:00:00 2001
From: Joe Portner <5295965+jportner@users.noreply.github.com>
Date: Thu, 12 Dec 2019 13:02:27 -0500
Subject: [PATCH 4/8] Fix unit test

---
 x-pack/plugins/security/server/plugin.test.ts | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/x-pack/plugins/security/server/plugin.test.ts b/x-pack/plugins/security/server/plugin.test.ts
index 0569f5f4de3a6..cce928976accc 100644
--- a/x-pack/plugins/security/server/plugin.test.ts
+++ b/x-pack/plugins/security/server/plugin.test.ts
@@ -52,10 +52,6 @@ describe('Security Plugin', () => {
                     "cookieName": "sid",
                     "loginAssistanceMessage": undefined,
                     "secureCookies": true,
-                    "session": Object {
-                      "idleTimeout": 1500,
-                      "lifespan": null,
-                    },
                   },
                   "license": Object {
                     "getFeatures": [Function],

From 3c0c065103fd5bf83b2b57542ddca8713dc68899 Mon Sep 17 00:00:00 2001
From: Joe Portner <5295965+jportner@users.noreply.github.com>
Date: Thu, 12 Dec 2019 15:24:11 -0500
Subject: [PATCH 5/8] Fix another unit test

Even though `session.lifespan` should be `null` by default, the
`session` object is undefined by default. When the config is parsed
we want to make sure that both `session.idleTimeout` and
`session.lifespan` are null by default.
---
 x-pack/plugins/security/server/config.ts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/x-pack/plugins/security/server/config.ts b/x-pack/plugins/security/server/config.ts
index b3f96497b0538..c344fbd6f7db9 100644
--- a/x-pack/plugins/security/server/config.ts
+++ b/x-pack/plugins/security/server/config.ts
@@ -98,8 +98,8 @@ export function createConfig$(context: PluginInitializerContext, isTLSEnabled: b
         encryptionKey,
         secureCookies,
         session: {
-          ...config.session,
-          idleTimeout: config.session.idleTimeout || deprecatedSessionTimeout,
+          idleTimeout: config.session?.idleTimeout || deprecatedSessionTimeout,
+          lifespan: config.session?.lifespan || null,
         },
       };
       delete val.sessionTimeout; // DEPRECATED

From fc9a19b40740ac4e078b71b96023fcda5327bca4 Mon Sep 17 00:00:00 2001
From: Joe Portner <5295965+jportner@users.noreply.github.com>
Date: Fri, 13 Dec 2019 10:53:49 -0500
Subject: [PATCH 6/8] Update docs again

---
 docs/settings/security-settings.asciidoc | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/docs/settings/security-settings.asciidoc b/docs/settings/security-settings.asciidoc
index a2c05e4d87325..d6dd4378da1b7 100644
--- a/docs/settings/security-settings.asciidoc
+++ b/docs/settings/security-settings.asciidoc
@@ -50,15 +50,17 @@ this to `true` if SSL is configured outside of {kib} (for example, you are
 routing requests through a load balancer or proxy).
 
 `xpack.security.session.idleTimeout`::
-Sets the session duration (in milliseconds). By default, sessions stay active
-until the browser is closed. When this is set to an explicit idle timeout, closing
-the browser still requires the user to log back in to {kib}.
+Sets the session duration. The format is a string of `<count>[ms|s|m|h|d|w|M|Y]`
+(e.g. '70ms', '5s', '3d', '1Y'). By default, sessions stay active until the
+browser is closed. When this is set to an explicit idle timeout, closing the
+browser still requires the user to log back in to {kib}.
 
 `xpack.security.session.lifespan`::
-Sets the maximum duration (in milliseconds), also known as "absolute timeout". By
-default, a session can be renewed indefinitely. When this value is set, a session
-will end once its lifespan is exceeded, even if the user is not idle. NOTE: if
-`idleTimeout` is not set, this setting will still cause sessions to expire.
+Sets the maximum duration, also known as "absolute timeout". The format is a
+string of `<count>[ms|s|m|h|d|w|M|Y]` (e.g. '70ms', '5s', '3d', '1Y'). By default,
+a session can be renewed indefinitely. When this value is set, a session will end
+once its lifespan is exceeded, even if the user is not idle. NOTE: if `idleTimeout`
+is not set, this setting will still cause sessions to expire.
 
 `xpack.security.loginAssistanceMessage`::
 Adds a message to the login screen. Useful for displaying information about maintenance windows, links to corporate sign up pages etc.

From f3b7df6a74272dd7af4ff9539accacb3a80d9e72 Mon Sep 17 00:00:00 2001
From: Joe Portner <5295965+jportner@users.noreply.github.com>
Date: Fri, 13 Dec 2019 10:55:16 -0500
Subject: [PATCH 7/8] Clarify expiry calculation

---
 .../security/server/authentication/authenticator.ts  | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/x-pack/plugins/security/server/authentication/authenticator.ts b/x-pack/plugins/security/server/authentication/authenticator.ts
index 8aff6ca1a1b23..e95c5de6d961f 100644
--- a/x-pack/plugins/security/server/authentication/authenticator.ts
+++ b/x-pack/plugins/security/server/authentication/authenticator.ts
@@ -493,10 +493,14 @@ export class Authenticator {
     existingSession: ProviderSession | null
   ): { idleTimeoutExpiration: number | null; lifespanExpiration: number | null } {
     const now = Date.now();
-    let lifespanExpiration = this.lifespan && now + this.lifespan.asMilliseconds();
-    if (existingSession && existingSession.lifespanExpiration && this.lifespan) {
-      lifespanExpiration = existingSession.lifespanExpiration;
-    }
+    // if we are renewing an existing session, use its `lifespanExpiration` -- otherwise, set this value
+    // based on the configured server `lifespan`.
+    // note, if the server had a `lifespan` set and then removes it, remove `lifespanExpiration` on renewed sessions
+    // also, if the server did not have a `lifespan` set and then adds it, add `lifespanExpiration` on renewed sessions
+    const lifespanExpiration =
+      existingSession?.lifespanExpiration && this.lifespan
+        ? existingSession.lifespanExpiration
+        : this.lifespan && now + this.lifespan.asMilliseconds();
     const idleTimeoutExpiration = this.idleTimeout && now + this.idleTimeout.asMilliseconds();
 
     return { idleTimeoutExpiration, lifespanExpiration };

From 595f8af8f8c8c4b5b298c8880133dd5e470b8a17 Mon Sep 17 00:00:00 2001
From: Joe Portner <5295965+jportner@users.noreply.github.com>
Date: Fri, 13 Dec 2019 12:14:00 -0500
Subject: [PATCH 8/8] Simplify config and fix tests

We had some unnecessary complexity in the config to satisfy test
mocks as they were written. Chagned the tests and simplified said
config.
---
 .../server/authentication/index.test.ts       |  5 +-
 x-pack/plugins/security/server/config.test.ts | 48 ++++++-------------
 x-pack/plugins/security/server/config.ts      |  4 +-
 3 files changed, 21 insertions(+), 36 deletions(-)

diff --git a/x-pack/plugins/security/server/authentication/index.test.ts b/x-pack/plugins/security/server/authentication/index.test.ts
index ff7cf876adbef..6a0057e97dcf0 100644
--- a/x-pack/plugins/security/server/authentication/index.test.ts
+++ b/x-pack/plugins/security/server/authentication/index.test.ts
@@ -60,6 +60,10 @@ describe('setupAuthentication()', () => {
       coreMock.createPluginInitializerContext({
         encryptionKey: 'ab'.repeat(16),
         secureCookies: true,
+        session: {
+          idleTimeout: null,
+          lifespan: null,
+        },
         cookieName: 'my-sid-cookie',
         authc: { providers: ['basic'] },
       }),
@@ -87,7 +91,6 @@ describe('setupAuthentication()', () => {
       encryptionKey: 'ab'.repeat(16),
       secureCookies: true,
       cookieName: 'my-sid-cookie',
-      authc: { providers: ['basic'] },
     };
 
     await setupAuthentication(mockSetupAuthenticationParams);
diff --git a/x-pack/plugins/security/server/config.test.ts b/x-pack/plugins/security/server/config.test.ts
index 9ddb3e6e96b90..f7374eedb5520 100644
--- a/x-pack/plugins/security/server/config.test.ts
+++ b/x-pack/plugins/security/server/config.test.ts
@@ -254,19 +254,22 @@ describe('config schema', () => {
 });
 
 describe('createConfig$()', () => {
+  const mockAndCreateConfig = async (isTLSEnabled: boolean, value = {}, context?: any) => {
+    const contextMock = coreMock.createPluginInitializerContext(
+      // we must use validate to avoid errors in `createConfig$`
+      ConfigSchema.validate(value, context)
+    );
+    return await createConfig$(contextMock, isTLSEnabled)
+      .pipe(first())
+      .toPromise()
+      .then(config => ({ contextMock, config }));
+  };
   it('should log a warning and set xpack.security.encryptionKey if not set', async () => {
     const mockRandomBytes = jest.requireMock('crypto').randomBytes;
     mockRandomBytes.mockReturnValue('ab'.repeat(16));
 
-    const contextMock = coreMock.createPluginInitializerContext({});
-    const config = await createConfig$(contextMock, true)
-      .pipe(first())
-      .toPromise();
-    expect(config).toEqual({
-      encryptionKey: 'ab'.repeat(16),
-      secureCookies: true,
-      session: { idleTimeout: null, lifespan: null },
-    });
+    const { contextMock, config } = await mockAndCreateConfig(true, {}, { dist: true });
+    expect(config.encryptionKey).toEqual('ab'.repeat(16));
 
     expect(loggingServiceMock.collect(contextMock.logger).warn).toMatchInlineSnapshot(`
                         Array [
@@ -278,14 +281,7 @@ describe('createConfig$()', () => {
   });
 
   it('should log a warning if SSL is not configured', async () => {
-    const contextMock = coreMock.createPluginInitializerContext({
-      encryptionKey: 'a'.repeat(32),
-      secureCookies: false,
-    });
-
-    const config = await createConfig$(contextMock, false)
-      .pipe(first())
-      .toPromise();
+    const { contextMock, config } = await mockAndCreateConfig(false, {});
     expect(config.secureCookies).toEqual(false);
 
     expect(loggingServiceMock.collect(contextMock.logger).warn).toMatchInlineSnapshot(`
@@ -298,14 +294,7 @@ describe('createConfig$()', () => {
   });
 
   it('should log a warning if SSL is not configured yet secure cookies are being used', async () => {
-    const contextMock = coreMock.createPluginInitializerContext({
-      encryptionKey: 'a'.repeat(32),
-      secureCookies: true,
-    });
-
-    const config = await createConfig$(contextMock, false)
-      .pipe(first())
-      .toPromise();
+    const { contextMock, config } = await mockAndCreateConfig(false, { secureCookies: true });
     expect(config.secureCookies).toEqual(true);
 
     expect(loggingServiceMock.collect(contextMock.logger).warn).toMatchInlineSnapshot(`
@@ -318,14 +307,7 @@ describe('createConfig$()', () => {
   });
 
   it('should set xpack.security.secureCookies if SSL is configured', async () => {
-    const contextMock = coreMock.createPluginInitializerContext({
-      encryptionKey: 'a'.repeat(32),
-      secureCookies: false,
-    });
-
-    const config = await createConfig$(contextMock, true)
-      .pipe(first())
-      .toPromise();
+    const { contextMock, config } = await mockAndCreateConfig(true, {});
     expect(config.secureCookies).toEqual(true);
 
     expect(loggingServiceMock.collect(contextMock.logger).warn).toEqual([]);
diff --git a/x-pack/plugins/security/server/config.ts b/x-pack/plugins/security/server/config.ts
index c344fbd6f7db9..b3f96497b0538 100644
--- a/x-pack/plugins/security/server/config.ts
+++ b/x-pack/plugins/security/server/config.ts
@@ -98,8 +98,8 @@ export function createConfig$(context: PluginInitializerContext, isTLSEnabled: b
         encryptionKey,
         secureCookies,
         session: {
-          idleTimeout: config.session?.idleTimeout || deprecatedSessionTimeout,
-          lifespan: config.session?.lifespan || null,
+          ...config.session,
+          idleTimeout: config.session.idleTimeout || deprecatedSessionTimeout,
         },
       };
       delete val.sessionTimeout; // DEPRECATED