Skip to content

Commit

Permalink
[EDR Workflows] Workflow Insights - Change update api RBAC (#210637)
Browse files Browse the repository at this point in the history
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
(cherry picked from commit f4365f6)
  • Loading branch information
szwarckonrad committed Feb 13, 2025
1 parent 4040d54 commit 7056ca2
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 41 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand All @@ -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,
});
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 () => {
Expand Down Expand Up @@ -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();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export const registerUpdateInsightsRoute = (
},
},
withEndpointAuthz(
{ all: ['canWriteWorkflowInsights'] },
{ all: ['canReadWorkflowInsights'] },
endpointContext.logFactory.get('workflowInsights'),
updateInsightsRouteHandler(endpointContext)
)
Expand All @@ -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(
Expand Down

0 comments on commit 7056ca2

Please sign in to comment.