Refresh session index after cleanup

elastic · Mar 4, 2022 · 40b7cbf · 40b7cbf
1 parent 2336649
commit 40b7cbf
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 1 deletion.
diff --git a/x-pack/plugins/security/server/session_management/session_index.test.ts b/x-pack/plugins/security/server/session_management/session_index.test.ts
@@ -215,6 +215,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 () => {
@@ -227,7 +228,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 () => {
@@ -388,6 +402,7 @@ describe('Session index', () => {
         }
       );
       expect(mockElasticsearchClient.closePointInTime).toHaveBeenCalledTimes(1);
+      expect(mockElasticsearchClient.indices.refresh).toHaveBeenCalledTimes(1);
     });
 
     it('when only `idleTimeout` is configured', async () => {
@@ -474,6 +489,7 @@ describe('Session index', () => {
         }
       );
       expect(mockElasticsearchClient.closePointInTime).toHaveBeenCalledTimes(1);
+      expect(mockElasticsearchClient.indices.refresh).toHaveBeenCalledTimes(1);
     });
 
     it('when both `lifespan` and `idleTimeout` are configured', async () => {
@@ -570,6 +586,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 () => {
@@ -714,6 +731,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 () => {
@@ -729,6 +747,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 () => {
@@ -742,6 +761,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 () => {

diff --git a/x-pack/plugins/security/server/session_management/session_index.ts b/x-pack/plugins/security/server/session_management/session_index.ts
@@ -448,6 +448,7 @@ export class SessionIndex {
     logger.debug(`Running cleanup routine.`);
 
     let error: Error | undefined;
+    let haveDeletedSessions = false;
     try {
       for await (const sessionValues of this.getSessionValuesInBatches()) {
         const operations: Array<Required<Pick<BulkOperationContainer, 'delete'>>> = [];
@@ -457,6 +458,7 @@ export class SessionIndex {
           operations.push({ delete: { _id } });
         });
         if (operations.length > 0) {
+          haveDeletedSessions = true;
           const bulkResponse = await elasticsearchClient.bulk(
             {
               index: this.indexName,
@@ -489,6 +491,15 @@ export class SessionIndex {
       error = err;
     }
 
+    if (haveDeletedSessions) {
+      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;