Extend session index fields mapping with a session creation timestamp. (#145997)

This commit is contained in:
Aleh Zasypkin 2022-11-24 17:21:08 +01:00 committed by GitHub
parent 95c4d73a13
commit 8f790d7d6a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 273 additions and 27 deletions

View file

@ -29,6 +29,7 @@ export const sessionMock = {
provider: { type: 'basic', name: 'basic1' },
idleTimeoutExpiration: null,
lifespanExpiration: null,
createdAt: 1234567890,
state: undefined,
metadata: { index: sessionIndexMock.createValue(sessionValue.metadata?.index) },
...sessionValue,

View file

@ -229,6 +229,7 @@ describe('Session', () => {
value: {
idleTimeoutExpiration: now + 1,
lifespanExpiration: now + 1,
createdAt: 1234567890,
metadata: { index: mockSessionIndexValue },
provider: { name: 'basic1', type: 'basic' },
sid: 'some-long-sid',
@ -262,6 +263,7 @@ describe('Session', () => {
value: {
idleTimeoutExpiration: now + 1,
lifespanExpiration: now + 1,
createdAt: 1234567890,
metadata: { index: mockSessionIndexValue },
provider: { name: 'basic1', type: 'basic' },
sid: 'some-long-sid',
@ -271,6 +273,44 @@ describe('Session', () => {
expect(mockSessionCookie.clear).not.toHaveBeenCalled();
expect(mockSessionIndex.invalidate).not.toHaveBeenCalled();
});
it('returns session value with 0 as `createdAt` if it is not set', async () => {
mockSessionCookie.get.mockResolvedValue(
sessionCookieMock.createValue({
aad: mockAAD,
idleTimeoutExpiration: now + 1,
lifespanExpiration: now + 1,
})
);
const mockSessionIndexValue = sessionIndexMock.createValue({
idleTimeoutExpiration: now - 1,
lifespanExpiration: now + 1,
createdAt: undefined,
content: await encryptContent(
{ username: 'some-user', state: 'some-state', userProfileId: 'uid' },
mockAAD
),
});
mockSessionIndex.get.mockResolvedValue(mockSessionIndexValue);
await expect(session.get(httpServerMock.createKibanaRequest())).resolves.toEqual({
error: null,
value: {
idleTimeoutExpiration: now + 1,
lifespanExpiration: now + 1,
createdAt: 0,
metadata: { index: mockSessionIndexValue },
provider: { name: 'basic1', type: 'basic' },
sid: 'some-long-sid',
state: 'some-state',
username: 'some-user',
userProfileId: 'uid',
},
});
expect(mockSessionCookie.clear).not.toHaveBeenCalled();
expect(mockSessionIndex.invalidate).not.toHaveBeenCalled();
});
});
describe('#create', () => {
@ -282,6 +322,7 @@ describe('Session', () => {
sid: mockSID,
idleTimeoutExpiration: now + 123,
lifespanExpiration: now + 456,
createdAt: 123456,
});
mockSessionIndex.create.mockResolvedValue(mockSessionIndexValue);
@ -301,6 +342,7 @@ describe('Session', () => {
provider: { name: 'basic1', type: 'basic' },
idleTimeoutExpiration: now + 123,
lifespanExpiration: now + 456,
createdAt: 123456,
metadata: { index: mockSessionIndexValue },
});
@ -314,6 +356,7 @@ describe('Session', () => {
usernameHash: '8ac76453d769d4fd14b3f41ad4933f9bd64321972cd002de9b847e117435b08b',
idleTimeoutExpiration: now + 123,
lifespanExpiration: now + 456,
createdAt: 123456,
});
// Properly creates session cookie value.
@ -334,6 +377,7 @@ describe('Session', () => {
sid: mockSID,
idleTimeoutExpiration: now + 123,
lifespanExpiration: now + 456,
createdAt: 123456,
});
mockSessionIndex.create.mockResolvedValue(mockSessionIndexValue);
@ -349,6 +393,7 @@ describe('Session', () => {
provider: { name: 'basic1', type: 'basic' },
idleTimeoutExpiration: now + 123,
lifespanExpiration: now + 456,
createdAt: 123456,
metadata: { index: mockSessionIndexValue },
});
@ -361,6 +406,7 @@ describe('Session', () => {
provider: { name: 'basic1', type: 'basic' },
idleTimeoutExpiration: now + 123,
lifespanExpiration: now + 456,
createdAt: 123456,
});
// Properly creates session cookie value.
@ -442,6 +488,7 @@ describe('Session', () => {
provider: { name: 'basic1', type: 'basic' },
idleTimeoutExpiration: now + 123,
lifespanExpiration: now + 1,
createdAt: 1234567890,
metadata: { index: mockSessionIndexValue },
});
@ -455,6 +502,7 @@ describe('Session', () => {
usernameHash: '35133597af273830c3f139c72501e676338f28a39dca8ff62d5c2b8bfba75f69',
idleTimeoutExpiration: now + 123,
lifespanExpiration: now + 1,
createdAt: 1234567890,
metadata: { primaryTerm: 1, sequenceNumber: 1 },
});
@ -501,6 +549,7 @@ describe('Session', () => {
provider: { name: 'basic1', type: 'basic' },
idleTimeoutExpiration: now + 123,
lifespanExpiration: now + 1,
createdAt: 1234567890,
metadata: { index: mockSessionIndexValue },
});
@ -513,6 +562,7 @@ describe('Session', () => {
provider: { name: 'basic1', type: 'basic' },
idleTimeoutExpiration: now + 123,
lifespanExpiration: now + 1,
createdAt: 1234567890,
metadata: { primaryTerm: 1, sequenceNumber: 1 },
});

View file

@ -51,6 +51,12 @@ export interface SessionValue {
*/
lifespanExpiration: number | null;
/**
* The Unix time in ms which is the time when the session was initially created. The value can also be 0 indicating
* the migrated session that was created before `createdAt` field was introduced.
*/
createdAt: number;
/**
* Session value that is fed to the authentication provider. The shape is unknown upfront and
* entirely determined by the authentication provider that owns the current session.
@ -206,7 +212,10 @@ export class Session {
async create(
request: KibanaRequest,
sessionValue: Readonly<
Omit<SessionValue, 'sid' | 'idleTimeoutExpiration' | 'lifespanExpiration' | 'metadata'>
Omit<
SessionValue,
'sid' | 'idleTimeoutExpiration' | 'lifespanExpiration' | 'createdAt' | 'metadata'
>
>
) {
const [sid, aad] = await Promise.all([
@ -226,6 +235,7 @@ export class Session {
...publicSessionValue,
...sessionExpirationInfo,
sid,
createdAt: Date.now(),
usernameHash: username && Session.getUsernameHash(username),
content: await this.crypto.encrypt(JSON.stringify({ username, userProfileId, state }), aad),
});
@ -242,7 +252,7 @@ export class Session {
}
/**
* Creates or updates session value for the specified request.
* Updates session value for the specified request.
* @param request Request instance to set session value for.
* @param sessionValue Session value parameters.
*/
@ -258,7 +268,10 @@ export class Session {
sessionValue.provider,
sessionCookieValue.lifespanExpiration
);
const { username, userProfileId, state, metadata, ...publicSessionInfo } = sessionValue;
// We filter out the `createdAt` field and rely on the one stored in `metadata.index` since it isn't
// supposed to be updated after it was initially set during creation.
const { username, userProfileId, state, metadata, createdAt, ...publicSessionInfo } =
sessionValue;
// First try to store session in the index and only then in the cookie to make sure cookie is
// only updated if server side session is created successfully.
@ -483,6 +496,8 @@ export class Session {
username,
userProfileId,
state,
// If the session was created before `createdAt` field was introduced, we set it to 0.
createdAt: publicSessionValue.createdAt ?? 0,
metadata: { index: sessionIndexValue },
};
}

View file

@ -122,7 +122,7 @@ export class SessionCookie {
}
/**
* Creates or updates session value for the specified request.
* Sets session value for the specified request.
* @param request Request instance to set session value for.
* @param sessionValue Session value parameters.
*/

View file

@ -25,6 +25,7 @@ export const sessionIndexMock = {
provider: { type: 'basic', name: 'basic1' },
idleTimeoutExpiration: null,
lifespanExpiration: null,
createdAt: 1234567890,
content: 'some-encrypted-content',
metadata: { primaryTerm: 1, sequenceNumber: 1 },
...sessionValue,

View file

@ -20,7 +20,11 @@ import type { AuditLogger } from '../audit';
import { auditLoggerMock } from '../audit/mocks';
import { ConfigSchema, createConfig } from '../config';
import { securityMock } from '../mocks';
import { getSessionIndexSettings, SessionIndex } from './session_index';
import {
getSessionIndexSettings,
SESSION_INDEX_MAPPINGS_VERSION_META_FIELD_NAME,
SessionIndex,
} from './session_index';
import { sessionIndexMock } from './session_index.mock';
describe('Session index', () => {
@ -166,6 +170,98 @@ describe('Session index', () => {
});
});
it('updates mappings for existing index without version in the meta', async () => {
mockElasticsearchClient.indices.existsTemplate.mockResponse(false);
mockElasticsearchClient.indices.existsIndexTemplate.mockResponse(false);
mockElasticsearchClient.indices.exists.mockResponse(true);
mockElasticsearchClient.indices.getMapping.mockResolvedValue({
[indexName]: {
mappings: { _meta: {} },
},
});
await sessionIndex.initialize();
assertExistenceChecksPerformed();
expect(mockElasticsearchClient.indices.deleteTemplate).not.toHaveBeenCalled();
expect(mockElasticsearchClient.indices.deleteIndexTemplate).not.toHaveBeenCalled();
expect(mockElasticsearchClient.indices.create).not.toHaveBeenCalled();
expect(mockElasticsearchClient.indices.putAlias).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.indices.putAlias).toHaveBeenCalledWith({
index: indexName,
name: aliasName,
});
expect(mockElasticsearchClient.indices.getMapping).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.indices.getMapping).toHaveBeenCalledWith({ index: indexName });
expect(mockElasticsearchClient.indices.putMapping).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.indices.putMapping).toHaveBeenCalledWith({
index: indexName,
...getSessionIndexSettings({ indexName, aliasName }).mappings,
});
});
it('updates mappings for existing index if version in meta is lower than the current version', async () => {
mockElasticsearchClient.indices.existsTemplate.mockResponse(false);
mockElasticsearchClient.indices.existsIndexTemplate.mockResponse(false);
mockElasticsearchClient.indices.exists.mockResponse(true);
mockElasticsearchClient.indices.getMapping.mockResolvedValue({
[indexName]: {
mappings: { _meta: { [SESSION_INDEX_MAPPINGS_VERSION_META_FIELD_NAME]: '8.6.9' } },
},
});
await sessionIndex.initialize();
assertExistenceChecksPerformed();
expect(mockElasticsearchClient.indices.deleteTemplate).not.toHaveBeenCalled();
expect(mockElasticsearchClient.indices.deleteIndexTemplate).not.toHaveBeenCalled();
expect(mockElasticsearchClient.indices.create).not.toHaveBeenCalled();
expect(mockElasticsearchClient.indices.putAlias).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.indices.putAlias).toHaveBeenCalledWith({
index: indexName,
name: aliasName,
});
expect(mockElasticsearchClient.indices.getMapping).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.indices.getMapping).toHaveBeenCalledWith({ index: indexName });
expect(mockElasticsearchClient.indices.putMapping).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.indices.putMapping).toHaveBeenCalledWith({
index: indexName,
...getSessionIndexSettings({ indexName, aliasName }).mappings,
});
});
it('does not update mappings for existing index if version in meta is greater or equal to the current version', async () => {
mockElasticsearchClient.indices.existsTemplate.mockResponse(false);
mockElasticsearchClient.indices.existsIndexTemplate.mockResponse(false);
mockElasticsearchClient.indices.exists.mockResponse(true);
mockElasticsearchClient.indices.getMapping.mockResolvedValue({
[indexName]: {
mappings: { _meta: { [SESSION_INDEX_MAPPINGS_VERSION_META_FIELD_NAME]: '8.7.0' } },
},
});
await sessionIndex.initialize();
assertExistenceChecksPerformed();
expect(mockElasticsearchClient.indices.deleteTemplate).not.toHaveBeenCalled();
expect(mockElasticsearchClient.indices.deleteIndexTemplate).not.toHaveBeenCalled();
expect(mockElasticsearchClient.indices.create).not.toHaveBeenCalled();
expect(mockElasticsearchClient.indices.putAlias).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.indices.putAlias).toHaveBeenCalledWith({
index: indexName,
name: aliasName,
});
expect(mockElasticsearchClient.indices.getMapping).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.indices.getMapping).toHaveBeenCalledWith({ index: indexName });
expect(mockElasticsearchClient.indices.putMapping).not.toHaveBeenCalled();
});
it('creates index if it does not exist', async () => {
mockElasticsearchClient.indices.existsTemplate.mockResponse(false);
mockElasticsearchClient.indices.existsIndexTemplate.mockResponse(false);
@ -177,6 +273,8 @@ describe('Session index', () => {
expect(mockElasticsearchClient.indices.deleteTemplate).not.toHaveBeenCalled();
expect(mockElasticsearchClient.indices.deleteIndexTemplate).not.toHaveBeenCalled();
expect(mockElasticsearchClient.indices.putAlias).not.toHaveBeenCalled();
expect(mockElasticsearchClient.indices.getMapping).not.toHaveBeenCalled();
expect(mockElasticsearchClient.indices.putMapping).not.toHaveBeenCalled();
expect(mockElasticsearchClient.indices.create).toHaveBeenCalledWith(
getSessionIndexSettings({ indexName, aliasName })
);

View file

@ -10,6 +10,7 @@ import type {
BulkOperationContainer,
SortResults,
} from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
import semver from 'semver';
import type { ElasticsearchClient, Logger } from '@kbn/core/server';
@ -43,6 +44,17 @@ export type InvalidateSessionsFilter =
*/
const SESSION_INDEX_TEMPLATE_VERSION = 1;
/**
* The current version of the session index mappings.
*/
const SESSION_INDEX_MAPPINGS_VERSION = '8.7.0';
/**
* Name of the session index mappings _meta field storing session index version. Named after a field used for the
* Elasticsearch security-specific indices with the same purpose.
*/
export const SESSION_INDEX_MAPPINGS_VERSION_META_FIELD_NAME = 'security-version';
/**
* Number of sessions to remove per batch during cleanup.
*/
@ -86,10 +98,12 @@ export function getSessionIndexSettings({
},
mappings: {
dynamic: 'strict',
_meta: { [SESSION_INDEX_MAPPINGS_VERSION_META_FIELD_NAME]: SESSION_INDEX_MAPPINGS_VERSION },
properties: {
usernameHash: { type: 'keyword' },
provider: { properties: { name: { type: 'keyword' }, type: { type: 'keyword' } } },
idleTimeoutExpiration: { type: 'date' },
createdAt: { type: 'date' },
lifespanExpiration: { type: 'date' },
accessAgreementAcknowledged: { type: 'boolean' },
content: { type: 'binary' },
@ -130,6 +144,12 @@ export interface SessionIndexValue {
*/
lifespanExpiration: number | null;
/**
* The Unix time in ms which is the time when the session was initially created. The missing value indicates that the
* session was created before `createdAt` was introduced.
*/
createdAt?: number;
/**
* Indicates whether user acknowledged access agreement or not.
*/
@ -515,30 +535,17 @@ export class SessionIndex {
throw err;
}
// Initialize session index:
// Ensure the alias is attached to the already existing index,
// or create a new index if it doesn't exist.
if (indexExists) {
this.options.logger.debug('Session index already exists. Attaching alias to index...');
const sessionIndexSettings = getSessionIndexSettings({
indexName: this.indexName,
aliasName: this.aliasName,
});
// Prior to https://github.com/elastic/kibana/pull/134900, sessions would be written directly against the session index.
// Now, we write sessions against a new session index alias. This call ensures that the alias exists, and is attached to the index.
// This operation is safe to repeat, even if the alias already exists. This seems safer than retrieving the index details, and inspecting
// it to see if the alias already exists.
// Initialize session index:
// Ensure the alias is attached to the already existing index and field mappings are up-to-date,
// or create a new index if it doesn't exist.
if (!indexExists) {
try {
await this.options.elasticsearchClient.indices.putAlias({
index: this.indexName,
name: this.aliasName,
});
} catch (err) {
this.options.logger.error(`Failed to attach alias to session index: ${err.message}`);
throw err;
}
} else {
try {
await this.options.elasticsearchClient.indices.create(
getSessionIndexSettings({ indexName: this.indexName, aliasName: this.aliasName })
);
await this.options.elasticsearchClient.indices.create(sessionIndexSettings);
this.options.logger.debug('Successfully created session index.');
} catch (err) {
// There can be a race condition if index is created by another Kibana instance.
@ -549,6 +556,59 @@ export class SessionIndex {
throw err;
}
}
return;
}
this.options.logger.debug(
'Session index already exists. Attaching alias to the index and ensuring up-to-date mappings...'
);
// Prior to https://github.com/elastic/kibana/pull/134900, sessions would be written directly against the session index.
// Now, we write sessions against a new session index alias. This call ensures that the alias exists, and is attached to the index.
// This operation is safe to repeat, even if the alias already exists. This seems safer than retrieving the index details, and inspecting
// it to see if the alias already exists.
try {
await this.options.elasticsearchClient.indices.putAlias({
index: this.indexName,
name: this.aliasName,
});
} catch (err) {
this.options.logger.error(`Failed to attach alias to session index: ${err.message}`);
throw err;
}
let indexMappingsVersion: string | undefined;
try {
const indexMappings = await this.options.elasticsearchClient.indices.getMapping({
index: this.indexName,
});
indexMappingsVersion =
indexMappings[this.indexName]?.mappings?._meta?.[
SESSION_INDEX_MAPPINGS_VERSION_META_FIELD_NAME
];
} catch (err) {
this.options.logger.error(`Failed to retrieve session index metadata: ${err.message}`);
throw err;
}
if (!indexMappingsVersion || semver.lt(indexMappingsVersion, SESSION_INDEX_MAPPINGS_VERSION)) {
this.options.logger.debug(
`Updating session index mappings from ${
indexMappingsVersion ?? 'unknown'
} to ${SESSION_INDEX_MAPPINGS_VERSION} version.`
);
try {
await this.options.elasticsearchClient.indices.putMapping({
index: this.indexName,
...sessionIndexSettings.mappings,
});
this.options.logger.debug('Successfully updated session index mappings.');
} catch (err) {
this.options.logger.error(`Failed to update session index mappings: ${err.message}`);
throw err;
}
}
}

View file

@ -12,11 +12,24 @@ import { FtrProviderContext } from '../../ftr_provider_context';
export default function ({ getService }: FtrProviderContext) {
const supertestWithoutAuth = getService('supertestWithoutAuth');
const config = getService('config');
const es = getService('es');
const kibanaServerConfig = config.get('servers.kibana');
const validUsername = kibanaServerConfig.username;
const validPassword = kibanaServerConfig.password;
// Get `createdAt` values from all existing sessions. It's the easiest solution
// to check that the values weren't modified since we cannot get the specific
// session id used during a test run, and we might have lots of unrelated
// sessions in the index created by the tests that didn't clean up the index.
async function getSessionsCreatedAt() {
const searchResponse = await es.search<{ createdAt: number }>({
index: '.kibana_security_session*',
});
return searchResponse.hits.hits.map((hit) => hit._source!.createdAt).sort();
}
describe('Session', () => {
let sessionCookie: Cookie;
@ -81,9 +94,17 @@ export default function ({ getService }: FtrProviderContext) {
// browsers will follow the redirect and return the new session info, but this testing framework does not
// we simulate that behavior in this test by sending another GET request
const { body } = await getSessionInfo();
// Make sure that all sessions have populated `createdAt` field.
const sessionsCreatedAtBeforeExtension = await getSessionsCreatedAt();
expect(sessionsCreatedAtBeforeExtension.every((createdAt) => createdAt > 0)).to.be(true);
await extendSession();
const { body: body2 } = await getSessionInfo();
expect(body2.expiresInMs).not.to.be.lessThan(body.expiresInMs);
// Check that session extension didn't alter `createdAt`.
expect(await getSessionsCreatedAt()).to.eql(sessionsCreatedAtBeforeExtension);
});
});
});