-
Notifications
You must be signed in to change notification settings - Fork 76
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
Implemented ListIOCs API. #1064
Implemented ListIOCs API. #1064
Conversation
Signed-off-by: AWSHurneyt <[email protected]>
Signed-off-by: AWSHurneyt <[email protected]>
Signed-off-by: AWSHurneyt <[email protected]>
Signed-off-by: AWSHurneyt <[email protected]>
Signed-off-by: AWSHurneyt <[email protected]>
Signed-off-by: AWSHurneyt <[email protected]>
Signed-off-by: AWSHurneyt <[email protected]>
Signed-off-by: AWSHurneyt <[email protected]>
Signed-off-by: AWSHurneyt <[email protected]>
src/main/java/org/opensearch/securityanalytics/action/ListIOCsActionResponse.java
Show resolved
Hide resolved
src/main/java/org/opensearch/securityanalytics/action/ListIOCsActionResponse.java
Outdated
Show resolved
Hide resolved
…red at the IOC level. Signed-off-by: AWSHurneyt <[email protected]>
Signed-off-by: AWSHurneyt <[email protected]>
… IOC level. Signed-off-by: AWSHurneyt <[email protected]>
…otFoundException when calling ListIOCs API. Signed-off-by: AWSHurneyt <[email protected]>
|
||
public class ListIOCsAction extends ActionType<ListIOCsActionResponse> { | ||
public static final ListIOCsAction INSTANCE = new ListIOCsAction(); | ||
public static final String NAME = "cluster:admin/opensearch/securityanalytics/iocs/list"; |
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.
Including #1077 (comment)
should we call this
ioc/_search
or justcluster:admin/opensearch/securityanalytics/iocs
is also good enough i guesslet's standardize all list apis...
@eirsep I believe we plan to have a separate SearchIOCs API which will support receiving a query in the request body (similar to the SearchMonitor API), so I was reserving the _search
route for that. iocs/list
follows the pattern used by the ListCorrelations API, but we can change this if needed.
https://github.com/opensearch-project/security-analytics/blob/feature/threat_intel/src/main/java/org/opensearch/securityanalytics/action/ListCorrelationsAction.java#L11
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 need the search Ioc’s api.. that’s more flexible and if we are starting out with only one of these it should be the search API
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.
Will implement a search IOC API separately.
private int size; | ||
private SortOrder sortOrder; | ||
private String sortString; |
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.
Including #1077 (comment)
we need feedId as a parameter
check with @amsiglan if UX would be better suited to work with a search request rather than params?
@eirsep Added feedId
to the request object. I believe @amsiglan mentioned that a list API would be more useful for the UI table, but I'll sync with him to confirm.
Signed-off-by: AWSHurneyt <[email protected]>
…red at the IOC level. Signed-off-by: AWSHurneyt <[email protected]>
Signed-off-by: AWSHurneyt <[email protected]>
Signed-off-by: AWSHurneyt <[email protected]>
…otFoundException when calling ListIOCs API. Signed-off-by: AWSHurneyt <[email protected]>
Signed-off-by: AWSHurneyt <[email protected]>
Signed-off-by: AWSHurneyt <[email protected]>
# Conflicts: # src/main/java/org/opensearch/securityanalytics/SecurityAnalyticsPlugin.java # src/main/java/org/opensearch/securityanalytics/model/STIX2IOC.java # src/main/java/org/opensearch/securityanalytics/model/STIX2IOCDto.java # src/main/java/org/opensearch/securityanalytics/services/STIX2IOCConsumer.java # src/main/java/org/opensearch/securityanalytics/services/STIX2IOCFeedStore.java # src/main/java/org/opensearch/securityanalytics/services/STIX2IOCFetchService.java # src/main/resources/mappings/stix2_ioc_mapping.json # src/test/java/org/opensearch/securityanalytics/resthandler/ListIOCsRestApiIT.java # src/test/java/org/opensearch/securityanalytics/util/STIX2IOCGenerator.java
Signed-off-by: AWSHurneyt <[email protected]>
Signed-off-by: AWSHurneyt <[email protected]>
Signed-off-by: AWSHurneyt <[email protected]>
src/main/java/org/opensearch/securityanalytics/SecurityAnalyticsPlugin.java
Show resolved
Hide resolved
src/main/java/org/opensearch/securityanalytics/transport/TransportListIOCsAction.java
Show resolved
Hide resolved
int size = request.paramAsInt(ListIOCsActionRequest.SIZE_FIELD, 10); | ||
String sortOrder = request.param(ListIOCsActionRequest.SORT_ORDER_FIELD, ListIOCsActionRequest.SortOrder.asc.toString()); | ||
String sortString = request.param(ListIOCsActionRequest.SORT_STRING_FIELD, STIX2.NAME_FIELD); | ||
String search = request.param(ListIOCsActionRequest.SEARCH_FIELD, ""); |
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.
what is search field?
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.
QueryBuilders.queryStringQuery(request.getSearch()) | ||
.defaultOperator(Operator.OR) | ||
// .field(STIX2_IOC_NESTED_PATH + STIX2IOC.ID_FIELD) // Currently not a column in UX table | ||
.field(STIX2_IOC_NESTED_PATH + STIX2IOC.NAME_FIELD) |
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.
can we remove nesting?
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 think we still need to include it.
Elsewhere in the code, it looks like we need to provide the nested path; even when using QueryBuilders.nestedQuery()
.
https://github.com/opensearch-project/security-analytics/blob/feature/threat_intel/src/main/java/org/opensearch/securityanalytics/transport/TransportIndexCustomLogTypeAction.java#L449
The GetAlerts API doesn't need to provide a nested path
https://github.com/opensearch-project/alerting/blob/main/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportGetAlertsAction.kt#L138
However, the alerts values aren't nested in an alert object in the mappings for that system index.
https://github.com/opensearch-project/alerting/blob/main/alerting/src/main/resources/org/opensearch/alerting/alerts/alert_mapping.json#L28
src/main/java/org/opensearch/securityanalytics/action/ListIOCsActionRequest.java
Show resolved
Hide resolved
private ListIOCsActionRequest request; | ||
private ActionListener<ListIOCsActionResponse> listener; | ||
|
||
private final AtomicReference<Object> response; |
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.
what is this for?
d71579c
into
opensearch-project:feature/threat_intel
src/main/java/org/opensearch/securityanalytics/transport/TransportListIOCsAction.java
Show resolved
Hide resolved
* Removed unused imports. Removed redundant helper function. Signed-off-by: AWSHurneyt <[email protected]> * Added note about system index refactoring. Signed-off-by: AWSHurneyt <[email protected]> * Implemented draft of IocService. Signed-off-by: AWSHurneyt <[email protected]> * Made changes based on PR feedback. Signed-off-by: AWSHurneyt <[email protected]> * Fixed test helper function. Signed-off-by: AWSHurneyt <[email protected]> * Removed unused imports. Signed-off-by: AWSHurneyt <[email protected]> * Adjusted mappings based on PR feedback. Signed-off-by: AWSHurneyt <[email protected]> * Continuation of fetch IOC service implementation. Signed-off-by: AWSHurneyt <[email protected]> * Implemented ListtIOCs API. Signed-off-by: AWSHurneyt <[email protected]> * Removed "enabled" field from ListIOCs API as that will not be configured at the IOC level. Signed-off-by: AWSHurneyt <[email protected]> * Renamed response keys. Signed-off-by: AWSHurneyt <[email protected]> * Removed "enabled" field mapping as that will not be configured at the IOC level. Signed-off-by: AWSHurneyt <[email protected]> * Added feedId as a filter for LiistIOCs API. Added handling for IndexNotFoundException when calling ListIOCs API. Signed-off-by: AWSHurneyt <[email protected]> * Implemented ListtIOCs API. Signed-off-by: AWSHurneyt <[email protected]> * Removed "enabled" field from ListIOCs API as that will not be configured at the IOC level. Signed-off-by: AWSHurneyt <[email protected]> * Renamed response keys. Signed-off-by: AWSHurneyt <[email protected]> * Removed unused test suite. Signed-off-by: AWSHurneyt <[email protected]> * Added feedId as a filter for LiistIOCs API. Added handling for IndexNotFoundException when calling ListIOCs API. Signed-off-by: AWSHurneyt <[email protected]> * Added feedId as a filter for ListIOCs API. Signed-off-by: AWSHurneyt <[email protected]> * Fixed merge conflict. Signed-off-by: AWSHurneyt <[email protected]> * Removed unused test suite. Signed-off-by: AWSHurneyt <[email protected]> * Fixed test case. Signed-off-by: AWSHurneyt <[email protected]> * Fixed test index mappings. Signed-off-by: AWSHurneyt <[email protected]> --------- Signed-off-by: AWSHurneyt <[email protected]>
Description
Implemented ListIOCs API.
This PR overlaps with PR #1060, so leaving it as a draft for now.
Issues Resolved
[List any issues this PR will resolve]
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.