Add back user checks

elastic · Feb 11, 2021 · 993216a · 993216a
1 parent 33d2784
commit 993216a
Show file tree

Hide file tree

Showing 2 changed files with 68 additions and 7 deletions.
diff --git a/x-pack/plugins/data_enhanced/server/search/session/session_service.test.ts b/x-pack/plugins/data_enhanced/server/search/session/session_service.test.ts
@@ -40,6 +40,13 @@ describe('SearchSessionService', () => {
       name: 'my_realm_name',
     },
   } as AuthenticatedUser;
+  const mockUser2 = {
+    username: 'bar',
+    authentication_realm: {
+      type: 'bar',
+      name: 'bar',
+    },
+  } as AuthenticatedUser;
   const mockSavedObject: SavedObject<any> = {
     id: 'd7170a35-7e2c-48d6-8dec-9a056721b489',
     type: SEARCH_SESSION_TYPE,
@@ -179,6 +186,14 @@ describe('SearchSessionService', () => {
       expect(callAttributes).toHaveProperty('username', mockUser1.username);
     });
 
+    it('throws error if user conflicts', () => {
+      savedObjectsClient.get.mockResolvedValue(mockSavedObject);
+
+      expect(
+        service.get({ savedObjectsClient }, mockUser2, sessionId)
+      ).rejects.toMatchInlineSnapshot(`[Error: Not Found]`);
+    });
+
     it('works without security', async () => {
       savedObjectsClient.update.mockRejectedValue(
         SavedObjectsErrorHelpers.createGenericNotFoundError(sessionId)
@@ -559,6 +574,20 @@ describe('SearchSessionService', () => {
       expect(callAttributes).toHaveProperty('touched');
     });
 
+    it('throws if user conflicts', () => {
+      const mockUpdateSavedObject = {
+        ...mockSavedObject,
+        attributes: {},
+      };
+      savedObjectsClient.get.mockResolvedValue(mockSavedObject);
+      savedObjectsClient.update.mockResolvedValue(mockUpdateSavedObject);
+
+      const attributes = { name: 'new_name' };
+      expect(
+        service.update({ savedObjectsClient }, mockUser2, sessionId, attributes)
+      ).rejects.toMatchInlineSnapshot(`[Error: Not Found]`);
+    });
+
     it('works without security', async () => {
       const mockUpdateSavedObject = {
         ...mockSavedObject,
@@ -592,6 +621,14 @@ describe('SearchSessionService', () => {
       expect(callAttributes).toHaveProperty('touched');
     });
 
+    it('throws if user conflicts', () => {
+      savedObjectsClient.get.mockResolvedValue(mockSavedObject);
+
+      expect(
+        service.cancel({ savedObjectsClient }, mockUser2, sessionId)
+      ).rejects.toMatchInlineSnapshot(`[Error: Not Found]`);
+    });
+
     it('works without security', async () => {
       savedObjectsClient.get.mockResolvedValue(mockSavedObject);
 

diff --git a/x-pack/plugins/data_enhanced/server/search/session/session_service.ts b/x-pack/plugins/data_enhanced/server/search/session/session_service.ts
@@ -5,6 +5,7 @@
  * 2.0.
  */
 
+import { notFound } from '@hapi/boom';
 import {
   CoreSetup,
   CoreStart,
@@ -196,16 +197,18 @@ export class SearchSessionService
     );
   };
 
-  public get = (
+  public get = async (
     { savedObjectsClient }: SearchSessionDependencies,
     user: AuthenticatedUser | null,
     sessionId: string
   ) => {
     this.logger.debug(`get | ${sessionId}`);
-    return savedObjectsClient.get<SearchSessionSavedObjectAttributes>(
+    const session = await savedObjectsClient.get<SearchSessionSavedObjectAttributes>(
       SEARCH_SESSION_TYPE,
       sessionId
     );
+    this.throwOnUserConflict(user, session);
+    return session;
   };
 
   public find = (
@@ -239,13 +242,14 @@ export class SearchSessionService
     });
   };
 
-  public update = (
+  public update = async (
     deps: SearchSessionDependencies,
     user: AuthenticatedUser | null,
     sessionId: string,
     attributes: Partial<SearchSessionSavedObjectAttributes>
   ) => {
     this.logger.debug(`update | ${sessionId}`);
+    await this.get(deps, user, sessionId); // Verify correct user
     return deps.savedObjectsClient.update<SearchSessionSavedObjectAttributes>(
       SEARCH_SESSION_TYPE,
       sessionId,
@@ -256,33 +260,36 @@ export class SearchSessionService
     );
   };
 
-  public extend(
+  public async extend(
     deps: SearchSessionDependencies,
     user: AuthenticatedUser | null,
     sessionId: string,
     expires: Date
   ) {
     this.logger.debug(`extend | ${sessionId}`);
-
+    await this.get(deps, user, sessionId); // Verify correct user
     return this.update(deps, user, sessionId, { expires: expires.toISOString() });
   }
 
-  public cancel = (
+  public cancel = async (
     deps: SearchSessionDependencies,
     user: AuthenticatedUser | null,
     sessionId: string
   ) => {
     this.logger.debug(`delete | ${sessionId}`);
+    await this.get(deps, user, sessionId); // Verify correct user
     return this.update(deps, user, sessionId, {
       status: SearchSessionStatus.CANCELLED,
     });
   };
 
-  public delete = (
+  public delete = async (
     deps: SearchSessionDependencies,
     user: AuthenticatedUser | null,
     sessionId: string
   ) => {
+    this.logger.debug(`delete | ${sessionId}`);
+    await this.get(deps, user, sessionId); // Verify correct user
     return deps.savedObjectsClient.delete(SEARCH_SESSION_TYPE, sessionId);
   };
 
@@ -379,4 +386,21 @@ export class SearchSessionService
       };
     };
   };
+
+  private throwOnUserConflict = (
+    user: AuthenticatedUser | null,
+    session?: SavedObject<SearchSessionSavedObjectAttributes>
+  ) => {
+    if (user === null || !session) return;
+    if (
+      user.authentication_realm.type !== session.attributes.realmType ||
+      user.authentication_realm.name !== session.attributes.realmName ||
+      user.username !== session.attributes.username
+    ) {
+      this.logger.debug(
+        `User ${user.username} has no access to search session ${session.attributes.sessionId}`
+      );
+      throw notFound();
+    }
+  };
 }