Allow only action.type to be changed with read permission

elastic · Feb 13, 2025 · a40f68f · a40f68f
1 parent 9c69036
commit a40f68f
Show file tree

Hide file tree

Showing 2 changed files with 78 additions and 44 deletions.
diff --git a/...plugins/security_solution/server/endpoint/routes/workflow_insights/update_insight.test.ts b/...plugins/security_solution/server/endpoint/routes/workflow_insights/update_insight.test.ts
@@ -9,6 +9,7 @@ import { httpServerMock, httpServiceMock } from '@kbn/core/server/mocks';
 import { registerUpdateInsightsRoute } from './update_insight';
 import { createMockEndpointAppContext, getRegisteredVersionedRouteMock } from '../../mocks';
 import { WORKFLOW_INSIGHTS_UPDATE_ROUTE } from '../../../../common/endpoint/constants';
+import type { EndpointAppContext } from '../../types';
 
 jest.mock('../../services', () => ({
   securityWorkflowInsightsService: {
@@ -21,56 +22,55 @@ const updateMock = jest.requireMock('../../services').securityWorkflowInsightsSe
 
 describe('Update Insights Route Handler', () => {
   let mockResponse: ReturnType<typeof httpServerMock.createResponseFactory>;
-  let callRoute: (
-    params: Record<string, string>,
-    body: Record<string, string>,
-    authz?: Record<string, boolean>
-  ) => Promise<void>;
 
-  beforeEach(() => {
+  const callRoute = async (
+    params: Record<string, string>,
+    body: Record<string, unknown>,
+    authz: Record<string, boolean> = {
+      canWriteWorkflowInsights: true,
+      canReadWorkflowInsights: true,
+    }
+  ) => {
     mockResponse = httpServerMock.createResponseFactory();
 
     const mockEndpointContext = createMockEndpointAppContext();
     const router = httpServiceMock.createRouter();
 
-    registerUpdateInsightsRoute(router, mockEndpointContext);
-
-    callRoute = async (
-      params,
-      body,
-      authz = { canWriteWorkflowInsights: true, canReadWorkflowInsights: true }
-    ) => {
-      const mockContext = {
-        core: {
-          security: {
-            authc: {
-              getCurrentUser: jest
-                .fn()
-                .mockReturnValue({ username: 'test-user', roles: ['admin'] }),
-            },
+    const mockContext = {
+      ...mockEndpointContext,
+      service: {
+        ...mockEndpointContext.service,
+        getEndpointAuthz: jest.fn().mockResolvedValue(authz),
+      },
+      securitySolution: {
+        getEndpointAuthz: jest.fn().mockResolvedValue(authz),
+      },
+      core: {
+        security: {
+          authc: {
+            getCurrentUser: jest.fn().mockReturnValue({ username: 'test-user', roles: ['admin'] }),
           },
         },
-        securitySolution: {
-          getEndpointAuthz: jest.fn().mockResolvedValue(authz),
-        },
-      };
+      },
+    };
 
-      const request = httpServerMock.createKibanaRequest({
-        method: 'put',
-        path: WORKFLOW_INSIGHTS_UPDATE_ROUTE,
-        params,
-        body,
-      });
+    registerUpdateInsightsRoute(router, mockContext as unknown as EndpointAppContext);
 
-      const { routeHandler } = getRegisteredVersionedRouteMock(
-        router,
-        'put',
-        WORKFLOW_INSIGHTS_UPDATE_ROUTE,
-        '1'
-      )!;
-      await routeHandler(mockContext, request, mockResponse);
-    };
-  });
+    const request = httpServerMock.createKibanaRequest({
+      method: 'put',
+      path: WORKFLOW_INSIGHTS_UPDATE_ROUTE,
+      params,
+      body,
+    });
+
+    const { routeHandler } = getRegisteredVersionedRouteMock(
+      router,
+      'put',
+      WORKFLOW_INSIGHTS_UPDATE_ROUTE,
+      '1'
+    )!;
+    await routeHandler(mockContext, request, mockResponse);
+  };
 
   describe('with valid privileges', () => {
     it('should update insight and return the updated data', async () => {
@@ -102,14 +102,38 @@ describe('Update Insights Route Handler', () => {
   });
 
   describe('with invalid privileges', () => {
-    it('should return forbidden if user lacks read privileges', async () => {
+    it('should return forbidden if user lacks write and read privileges', async () => {
       await callRoute(
         { insightId: '1' },
-        { name: 'Updated Insight' },
-        { canWriteWorkflowInsights: false }
+        { action: { type: 'remediated' } },
+        { canWriteWorkflowInsights: false, canReadWorkflowInsights: false }
       );
 
       expect(mockResponse.forbidden).toHaveBeenCalled();
     });
+
+    it('should allow update if user has no write privileges but read and the body only contains action.type', async () => {
+      const updateBody = { action: { type: 'remediated' } }; // Only action.type in the body
+      updateMock.mockResolvedValue({ id: 1, ...updateBody });
+      await callRoute({ insightId: '1' }, updateBody, {
+        canWriteWorkflowInsights: false,
+        canReadWorkflowInsights: true,
+      });
+
+      expect(updateMock).toHaveBeenCalledWith('1', updateBody);
+      expect(mockResponse.ok).toHaveBeenCalledWith({
+        body: { id: 1, action: { type: 'remediated' } },
+      });
+    });
+
+    it('should return forbidden if user has no write privileges but read and the body contains more than action.type', async () => {
+      const updateBody = { action: { type: 'remediated' }, value: 'changeme' };
+      await callRoute({ insightId: '1' }, updateBody, {
+        canWriteWorkflowInsights: false,
+        canReadWorkflowInsights: true,
+      });
+
+      expect(mockResponse.forbidden).toHaveBeenCalled();
+    });
   });
 });
diff --git a/...rity/plugins/security_solution/server/endpoint/routes/workflow_insights/update_insight.ts b/...rity/plugins/security_solution/server/endpoint/routes/workflow_insights/update_insight.ts
@@ -63,9 +63,19 @@ const updateInsightsRouteHandler = (
 > => {
   const logger = endpointContext.logFactory.get('workflowInsights');
 
+  const isOnlyActionTypeUpdate = (body: Partial<UpdateWorkflowInsightsRequestBody>): boolean => {
+    // Type guard is done by schema validation
+    if (!body?.action?.type) return false;
+    // Make sure the body only contains the action.type field
+    return Object.keys(body).length === 1 && Object.keys(body.action).length === 1;
+  };
+
   return async (_, request, response) => {
     const { insightId } = request.params;
-
+    const { canWriteWorkflowInsights } = await endpointContext.service.getEndpointAuthz(request);
+    if (!canWriteWorkflowInsights && !isOnlyActionTypeUpdate(request.body)) {
+      return response.forbidden({ body: 'Unauthorized to update workflow insights' });
+    }
     logger.debug(`Updating insight ${insightId}`);
     try {
       const body = await securityWorkflowInsightsService.update(