Skip to content

Commit

Permalink
Change session timeout values to use duration instead of number (#52520
Browse files Browse the repository at this point in the history
…) (#53030)
  • Loading branch information
jportner authored Dec 13, 2019
1 parent 236ca45 commit 2d2ad0e
Show file tree
Hide file tree
Showing 10 changed files with 74 additions and 94 deletions.
16 changes: 9 additions & 7 deletions docs/settings/security-settings.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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.
14 changes: 8 additions & 6 deletions docs/user/security/securing-kibana.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
--------------------------------------------------------------------------------
--

Expand All @@ -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"
--------------------------------------------------------------------------------
--

Expand Down
19 changes: 9 additions & 10 deletions x-pack/legacy/plugins/security/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,18 @@ 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'),
public: 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,
public: HANDLED_IN_NEW_PLATFORM,
loginAssistanceMessage: HANDLED_IN_NEW_PLATFORM,
authorization: Joi.object({
legacyFallback: Joi.object({
enabled: Joi.boolean().default(true) // deprecated
Expand All @@ -41,7 +42,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();
},

Expand Down Expand Up @@ -105,8 +106,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'),
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -440,7 +441,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: {} },
Expand Down Expand Up @@ -479,8 +480,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: {} },
});
Expand Down Expand Up @@ -521,7 +522,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
) {
Expand Down Expand Up @@ -565,7 +566,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);
Expand All @@ -574,7 +575,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);
});
});

Expand Down
21 changes: 13 additions & 8 deletions x-pack/plugins/security/server/authentication/authenticator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { Duration } from 'moment';
import {
SessionStorageFactory,
SessionStorage,
Expand Down Expand Up @@ -173,12 +174,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.
Expand Down Expand Up @@ -227,7 +228,6 @@ 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;
}
Expand Down Expand Up @@ -494,11 +494,16 @@ export class Authenticator {
private calculateExpiry(
existingSession: ProviderSession | null
): { idleTimeoutExpiration: number | null; lifespanExpiration: number | null } {
let lifespanExpiration = this.lifespan && Date.now() + this.lifespan;
if (existingSession && existingSession.lifespanExpiration && this.lifespan) {
lifespanExpiration = existingSession.lifespanExpiration;
}
const idleTimeoutExpiration = this.idleTimeout && Date.now() + this.idleTimeout;
const now = Date.now();
// 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 };
}
Expand Down
5 changes: 4 additions & 1 deletion x-pack/plugins/security/server/authentication/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'] },
public: {},
Expand Down Expand Up @@ -88,7 +92,6 @@ describe('setupAuthentication()', () => {
encryptionKey: 'ab'.repeat(16),
secureCookies: true,
cookieName: 'my-sid-cookie',
authc: { providers: ['basic'] },
};

await setupAuthentication(mockSetupAuthenticationParams);
Expand Down
48 changes: 14 additions & 34 deletions x-pack/plugins/security/server/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -481,18 +481,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({
authc: { providers: ['basic'] },
});
const config = await createConfig$(contextMock, true)
.pipe(first())
.toPromise();
const { contextMock, config } = await mockAndCreateConfig(true, {}, { dist: true });
expect(config.encryptionKey).toEqual('ab'.repeat(16));
expect(config.secureCookies).toEqual(true);

expect(loggingServiceMock.collect(contextMock.logger).warn).toMatchInlineSnapshot(`
Array [
Expand All @@ -504,15 +508,7 @@ describe('createConfig$()', () => {
});

it('should log a warning if SSL is not configured', async () => {
const contextMock = coreMock.createPluginInitializerContext({
encryptionKey: 'a'.repeat(32),
secureCookies: false,
authc: { providers: ['basic'] },
});

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(`
Expand All @@ -525,15 +521,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,
authc: { providers: ['basic'] },
});

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(`
Expand All @@ -546,15 +534,7 @@ describe('createConfig$()', () => {
});

it('should set xpack.security.secureCookies if SSL is configured', async () => {
const contextMock = coreMock.createPluginInitializerContext({
encryptionKey: 'a'.repeat(32),
secureCookies: false,
authc: { providers: ['basic'] },
});

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([]);
Expand Down
20 changes: 10 additions & 10 deletions x-pack/plugins/security/server/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -34,10 +35,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.number(), schema.literal(null)], { defaultValue: null }),
lifespan: schema.oneOf([schema.number(), schema.literal(null)], { defaultValue: null }),
idleTimeout: schema.nullable(schema.duration()),
lifespan: schema.nullable(schema.duration()),
}),
secureCookies: schema.boolean({ defaultValue: false }),
public: schema.object({
Expand Down Expand Up @@ -114,17 +115,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;
Expand Down
4 changes: 0 additions & 4 deletions x-pack/plugins/security/server/plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,6 @@ describe('Security Plugin', () => {
"cookieName": "sid",
"loginAssistanceMessage": undefined,
"secureCookies": true,
"session": Object {
"idleTimeout": 1500,
"lifespan": null,
},
},
"license": Object {
"getFeatures": [Function],
Expand Down
8 changes: 0 additions & 8 deletions x-pack/plugins/security/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,6 @@ export interface PluginSetupContract {
registerPrivilegesWithCluster: () => void;
license: SecurityLicense;
config: RecursiveReadonly<{
session: {
idleTimeout: number | null;
lifespan: number | null;
};
secureCookies: boolean;
cookieName: string;
loginAssistanceMessage: string;
Expand Down Expand Up @@ -210,10 +206,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: config.session.idleTimeout,
lifespan: config.session.lifespan,
},
secureCookies: config.secureCookies,
cookieName: config.cookieName,
},
Expand Down

0 comments on commit 2d2ad0e

Please sign in to comment.