-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Cases] Add RBAC to remaining Cases APIs #98762
Changes from all commits
39b3950
6fb28ce
2e0239c
5a63e2e
a2c7842
f8dd425
d670b1c
ba380e9
7734644
86180fa
eb26150
3d4726d
518db99
a32b1ba
28f8b62
7e2778c
221d17c
de78d74
d048e07
f7ae701
5e46358
6cfd24f
efc6e82
0d39f83
8543b5b
8841e65
ba7df51
fbf5784
9fae3d9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,7 @@ import * as rt from 'io-ts'; | |
import { UserRT } from '../user'; | ||
import { CaseConnectorRt, ConnectorMappingsRt, ESCaseConnector } from '../connectors'; | ||
import { OmitProp } from '../runtime_types'; | ||
import { OWNER_FIELD } from './constants'; | ||
|
||
// TODO: we will need to add this type rt.literal('close-by-third-party') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. doesn't need to be done this PR? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Naa, not for the rbac work. Looks like that TODO was added 7 months ago or so. I don't believe we support closing by third party right now anyway 🤷♂️ |
||
const ClosureTypeRT = rt.union([rt.literal('close-by-user'), rt.literal('close-by-pushing')]); | ||
|
@@ -20,7 +21,9 @@ const CasesConfigureBasicRt = rt.type({ | |
owner: rt.string, | ||
}); | ||
|
||
const CasesConfigureBasicWithoutOwnerRt = rt.type(OmitProp(CasesConfigureBasicRt.props, 'owner')); | ||
const CasesConfigureBasicWithoutOwnerRt = rt.type( | ||
OmitProp(CasesConfigureBasicRt.props, OWNER_FIELD) | ||
); | ||
|
||
export const CasesConfigureRequestRt = CasesConfigureBasicRt; | ||
export const CasesConfigurePatchRt = rt.intersection([ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0; you may not use this file except in compliance with the Elastic License | ||
* 2.0. | ||
*/ | ||
|
||
/** | ||
* The field used for authorization in various entities within cases. | ||
*/ | ||
export const OWNER_FIELD = 'owner'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using this so we don't have to use the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Out of curiosity, why this field. I feel like you could technically do this for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah true, I guess I was thinking this might be helpful for the places where we're constructing a filter string to avoid accidental spelling errors in the future 🤷♂️ it's probably not necessary. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,7 @@ import { | |
CASE_COMMENT_SAVED_OBJECT, | ||
CASE_CONFIGURE_SAVED_OBJECT, | ||
CASE_SAVED_OBJECT, | ||
CASE_USER_ACTION_SAVED_OBJECT, | ||
} from '../../common/constants'; | ||
import { Verbs, ReadOperations, WriteOperations, OperationDetails } from './types'; | ||
|
||
|
@@ -101,6 +102,14 @@ export const Operations: Record<ReadOperations | WriteOperations, OperationDetai | |
docType: 'case', | ||
savedObjectType: CASE_SAVED_OBJECT, | ||
}, | ||
[WriteOperations.PushCase]: { | ||
type: EVENT_TYPES.change, | ||
name: WriteOperations.PushCase, | ||
action: 'push-case', | ||
verbs: updateVerbs, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit?: Should the push have separate verbs defined? 'push, pushing, pushed'? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm 🤔 good point, right now the verbs are tied to the valid ECS event types that we use |
||
docType: 'case', | ||
savedObjectType: CASE_SAVED_OBJECT, | ||
}, | ||
[WriteOperations.CreateConfiguration]: { | ||
type: EVENT_TYPES.creation, | ||
name: WriteOperations.CreateConfiguration, | ||
|
@@ -214,4 +223,22 @@ export const Operations: Record<ReadOperations | WriteOperations, OperationDetai | |
docType: 'comments', | ||
savedObjectType: CASE_COMMENT_SAVED_OBJECT, | ||
}, | ||
// stats operations | ||
[ReadOperations.GetCaseStatuses]: { | ||
type: EVENT_TYPES.access, | ||
name: ACCESS_CASE_OPERATION, | ||
action: 'find-case-statuses', | ||
verbs: accessVerbs, | ||
docType: 'cases', | ||
savedObjectType: CASE_SAVED_OBJECT, | ||
}, | ||
// user actions operations | ||
[ReadOperations.GetUserActions]: { | ||
type: EVENT_TYPES.access, | ||
name: ReadOperations.GetUserActions, | ||
action: 'get-user-actions', | ||
verbs: accessVerbs, | ||
docType: 'user actions', | ||
savedObjectType: CASE_USER_ACTION_SAVED_OBJECT, | ||
}, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -105,6 +105,7 @@ async function getSubCase({ | |
subCaseId: newSubCase.id, | ||
fields: ['status', 'sub_case'], | ||
newValue: JSON.stringify({ status: newSubCase.attributes.status }), | ||
owner: newSubCase.attributes.owner, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Adding the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you do end up keeping the owner constant, it's probably worth updating these to use it as well There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah good point |
||
}), | ||
], | ||
}); | ||
|
@@ -222,6 +223,7 @@ const addGeneratedAlerts = async ( | |
commentId: newComment.id, | ||
fields: ['comment'], | ||
newValue: JSON.stringify(query), | ||
owner: newComment.attributes.owner, | ||
}), | ||
], | ||
}); | ||
|
@@ -396,6 +398,7 @@ export const addComment = async ( | |
commentId: newComment.id, | ||
fields: ['comment'], | ||
newValue: JSON.stringify(query), | ||
owner: newComment.attributes.owner, | ||
}), | ||
], | ||
}); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -95,6 +95,7 @@ export async function deleteAll( | |
subCaseId: subCaseID, | ||
commentId: comment.id, | ||
fields: ['comment'], | ||
owner: comment.attributes.owner, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same here: |
||
}) | ||
), | ||
}); | ||
|
@@ -167,6 +168,7 @@ export async function deleteComment( | |
subCaseId: subCaseID, | ||
commentId: attachmentID, | ||
fields: ['comment'], | ||
owner: myComment.attributes.owner, | ||
}), | ||
], | ||
}); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -80,14 +80,14 @@ export const find = async ( | |
|
||
ensureSavedObjectsAreAuthorized([...cases.casesMap.values()]); | ||
|
||
// TODO: Make sure we do not leak information when authorization is on | ||
const [openCases, inProgressCases, closedCases] = await Promise.all([ | ||
...caseStatuses.map((status) => { | ||
const statusQuery = constructQueryOptions({ ...queryArgs, status, authorizationFilter }); | ||
return caseService.findCaseStatusStats({ | ||
soClient: savedObjectsClient, | ||
caseOptions: statusQuery.case, | ||
subCaseOptions: statusQuery.subCase, | ||
ensureSavedObjectsAreAuthorized, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Passing this authorization function down into the function since the function only returns totals. |
||
}); | ||
}), | ||
]); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are not allowing the
owner
field to be updated.