[EDR Workflows] Workflow Insights - Change update api RBAC (#210637)

This PR addresses an issue where users with read privilege to insights
were unable to successfully complete a remediation path due to the
inability to mark an insight as remediated at the final step.

With this change, we adjust the required permissions for interacting
with the update API from writeWorkflowInsights to readWorkflowInsights.
The rationale behind this is that writeWorkflowInsights should signify
the ability to trigger new scans for insights, while
readWorkflowInsights should be sufficient for addressing found issues
without the option to generate new ones.


https://github.com/user-attachments/assets/8c1af654-d9e4-40d7-8718-1388677e8d46
This commit is contained in:
Konrad Szwarc 2025-02-13 17:14:51 +01:00 committed by GitHub
parent 4b4c5ce491
commit f4365f6e89
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 80 additions and 42 deletions

View file

@ -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,52 +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 }) => {
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,
});
const { routeHandler } = getRegisteredVersionedRouteMock(
router,
'put',
WORKFLOW_INSIGHTS_UPDATE_ROUTE,
'1'
)!;
await routeHandler(mockContext, request, mockResponse);
},
};
});
registerUpdateInsightsRoute(router, mockContext as unknown as EndpointAppContext);
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 () => {
@ -98,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();
});
});
});

View file

@ -46,7 +46,7 @@ export const registerUpdateInsightsRoute = (
},
},
withEndpointAuthz(
{ all: ['canWriteWorkflowInsights'] },
{ all: ['canReadWorkflowInsights'] },
endpointContext.logFactory.get('workflowInsights'),
updateInsightsRouteHandler(endpointContext)
)
@ -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(