-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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] Attachments RBAC #97756
[Cases] Attachments RBAC #97756
Changes from 16 commits
39b3950
6fb28ce
2e0239c
5a63e2e
a2c7842
f8dd425
d670b1c
ba380e9
7734644
86180fa
eb26150
3d4726d
518db99
a32b1ba
28f8b62
7e2778c
221d17c
d048e07
f7ae701
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 |
---|---|---|
|
@@ -14,6 +14,7 @@ import { | |
SUB_CASES_URL, | ||
CASE_PUSH_URL, | ||
SUB_CASE_USER_ACTIONS_URL, | ||
CASE_CONFIGURE_DETAILS_URL, | ||
} from '../constants'; | ||
|
||
export const getCaseDetailsUrl = (id: string): string => { | ||
|
@@ -47,3 +48,7 @@ export const getSubCaseUserActionUrl = (caseID: string, subCaseId: string): stri | |
export const getCasePushUrl = (caseId: string, connectorId: string): string => { | ||
return CASE_PUSH_URL.replace('{case_id}', caseId).replace('{connector_id}', connectorId); | ||
}; | ||
|
||
export const getCaseConfigurationDetailsUrl = (configureID: string): string => { | ||
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. This is so the UI can send the correct patch request with the |
||
return CASE_CONFIGURE_DETAILS_URL.replace('{configuration_id}', configureID); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,8 +5,8 @@ | |
* 2.0. | ||
*/ | ||
|
||
import { OperationDetails } from '.'; | ||
import { AuditLogger, EventCategory, EventOutcome } from '../../../security/server'; | ||
import { DATABASE_CATEGORY, ECS_OUTCOMES, OperationDetails } from '.'; | ||
import { AuditLogger } from '../../../security/server'; | ||
|
||
enum AuthorizationResult { | ||
Unauthorized = 'Unauthorized', | ||
|
@@ -51,9 +51,9 @@ export class AuthorizationAuditLogger { | |
message: `${username ?? 'unknown user'} ${message}`, | ||
event: { | ||
action: operation.action, | ||
category: EventCategory.DATABASE, | ||
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. The security plugin no longer defines these. They're using the ECS library directly now. So I defined some constants for us. 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. maybe something to come back to. I defined some ECS in the RAC work we did (no overlap). maybe we should use library? 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 that could be helpful. The struggle was that the ECS library defines types but not the values from what I could tell 🤷♂️ . I don't anticipate needing to add more than what was in this PR but if we do then yeah maybe a library would be a good option. |
||
type: operation.type, | ||
outcome: EventOutcome.SUCCESS, | ||
category: DATABASE_CATEGORY, | ||
type: [operation.type], | ||
outcome: ECS_OUTCOMES.success, | ||
}, | ||
...(username != null && { | ||
user: { | ||
|
@@ -81,9 +81,9 @@ export class AuthorizationAuditLogger { | |
message: `${username ?? 'unknown user'} ${message}`, | ||
event: { | ||
action: operation.action, | ||
category: EventCategory.DATABASE, | ||
type: operation.type, | ||
outcome: EventOutcome.FAILURE, | ||
category: DATABASE_CATEGORY, | ||
type: [operation.type], | ||
outcome: ECS_OUTCOMES.failure, | ||
}, | ||
// add the user information if we have it | ||
...(username != null && { | ||
|
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.
I'm adding
owner
here even though that means that patch requests must include theowner
. This is to keep it consistent with how patch was already working. All of the fields need to be included in the patch and we have checks to make sure that certain fields liketype
andowner
aren't different in the request than what is already defined in the object.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.
I'm wondering if we should make a type for owner like:
Not for this PR, but I think it would be good to have strict control over that type and not just accept any old string.