Fix session cleanup test (#126966) (#127087)

(cherry picked from commit 2f83c6516d)

# Conflicts:
#	x-pack/plugins/security/server/session_management/session_index.ts
This commit is contained in:
Joe Portner 2022-03-07 20:04:38 -05:00 committed by GitHub
parent 2e217725ac
commit 066d6a2d46
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 50 additions and 13 deletions

View file

@ -270,6 +270,7 @@ describe('Session index', () => {
expect(mockElasticsearchClient.search).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.bulk).not.toHaveBeenCalled();
expect(mockElasticsearchClient.closePointInTime).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.indices.refresh).not.toHaveBeenCalled(); // since the search failed, we don't refresh the index
});
it('throws if bulk delete call to Elasticsearch fails', async () => {
@ -282,7 +283,20 @@ describe('Session index', () => {
expect(mockElasticsearchClient.openPointInTime).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.search).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.bulk).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.closePointInTime).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.closePointInTime).toHaveBeenCalledTimes(1); // since we attempted to delete sessions, we still refresh the index
});
it('does not throw if index refresh call to Elasticsearch fails', async () => {
const failureReason = new errors.ResponseError(
securityMock.createApiResponse(securityMock.createApiResponse({ body: { type: 'Uh oh.' } }))
);
mockElasticsearchClient.indices.refresh.mockRejectedValue(failureReason);
await sessionIndex.cleanUp();
expect(mockElasticsearchClient.openPointInTime).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.search).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.bulk).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.closePointInTime).toHaveBeenCalledTimes(1); // since we attempted to delete sessions, we still refresh the index
});
it('when neither `lifespan` nor `idleTimeout` is configured', async () => {
@ -443,6 +457,7 @@ describe('Session index', () => {
}
);
expect(mockElasticsearchClient.closePointInTime).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.indices.refresh).toHaveBeenCalledTimes(1);
});
it('when only `idleTimeout` is configured', async () => {
@ -529,6 +544,7 @@ describe('Session index', () => {
}
);
expect(mockElasticsearchClient.closePointInTime).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.indices.refresh).toHaveBeenCalledTimes(1);
});
it('when both `lifespan` and `idleTimeout` are configured', async () => {
@ -625,6 +641,7 @@ describe('Session index', () => {
}
);
expect(mockElasticsearchClient.closePointInTime).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.indices.refresh).toHaveBeenCalledTimes(1);
});
it('when both `lifespan` and `idleTimeout` are configured and multiple providers are enabled', async () => {
@ -769,6 +786,7 @@ describe('Session index', () => {
}
);
expect(mockElasticsearchClient.closePointInTime).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.indices.refresh).toHaveBeenCalledTimes(1);
});
it('should clean up sessions in batches of 10,000', async () => {
@ -788,6 +806,7 @@ describe('Session index', () => {
expect(mockElasticsearchClient.search).toHaveBeenCalledTimes(2);
expect(mockElasticsearchClient.bulk).toHaveBeenCalledTimes(2);
expect(mockElasticsearchClient.closePointInTime).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.indices.refresh).toHaveBeenCalledTimes(1);
});
it('should limit number of batches to 10', async () => {
@ -805,6 +824,7 @@ describe('Session index', () => {
expect(mockElasticsearchClient.search).toHaveBeenCalledTimes(10);
expect(mockElasticsearchClient.bulk).toHaveBeenCalledTimes(10);
expect(mockElasticsearchClient.closePointInTime).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.indices.refresh).toHaveBeenCalledTimes(1);
});
it('should log audit event', async () => {

View file

@ -447,20 +447,21 @@ export class SessionIndex {
* Trigger a removal of any outdated session values.
*/
async cleanUp() {
this.options.logger.debug(`Running cleanup routine.`);
const { auditLogger, elasticsearchClient, logger } = this.options;
logger.debug(`Running cleanup routine.`);
let error: Error | undefined;
let indexNeedsRefresh = false;
try {
for await (const sessionValues of this.getSessionValuesInBatches()) {
const operations: Array<Required<Pick<BulkOperationContainer, 'delete'>>> = [];
sessionValues.forEach(({ _id, _source }) => {
const { usernameHash, provider } = _source!;
this.options.auditLogger.log(
sessionCleanupEvent({ sessionId: _id, usernameHash, provider })
);
auditLogger.log(sessionCleanupEvent({ sessionId: _id, usernameHash, provider }));
operations.push({ delete: { _id } });
});
if (operations.length > 0) {
const { body: bulkResponse } = await this.options.elasticsearchClient.bulk(
const { body: bulkResponse } = await elasticsearchClient.bulk(
{
index: this.indexName,
operations,
@ -474,24 +475,40 @@ export class SessionIndex {
0
);
if (errorCount < bulkResponse.items.length) {
this.options.logger.warn(
logger.warn(
`Failed to clean up ${errorCount} of ${bulkResponse.items.length} invalid or expired sessions. The remaining sessions were cleaned up successfully.`
);
indexNeedsRefresh = true;
} else {
this.options.logger.error(
logger.error(
`Failed to clean up ${bulkResponse.items.length} invalid or expired sessions.`
);
}
} else {
this.options.logger.debug(
`Cleaned up ${bulkResponse.items.length} invalid or expired sessions.`
);
logger.debug(`Cleaned up ${bulkResponse.items.length} invalid or expired sessions.`);
indexNeedsRefresh = true;
}
}
}
} catch (err) {
this.options.logger.error(`Failed to clean up sessions: ${err.message}`);
throw err;
logger.error(`Failed to clean up sessions: ${err.message}`);
error = err;
}
if (indexNeedsRefresh) {
// Only refresh the index if we have actually deleted one or more sessions. The index will auto-refresh eventually anyway, this just
// ensures that searches after the cleanup process are accurate, and this only impacts integration tests.
try {
await elasticsearchClient.indices.refresh({ index: this.indexName });
logger.debug(`Refreshed session index.`);
} catch (err) {
logger.error(`Failed to refresh session index: ${err.message}`);
}
}
if (error) {
// If we couldn't fetch or delete sessions, throw an error so the task will be retried.
throw error;
}
}