get all findings as part of findings API enhancement by riysaxen-amzn · Pull Request #803 · opensearch-project/security-analytics

riysaxen-amzn · 2024-01-16T07:47:09Z

Description

As part of the commit ->

The query params detector_id and detectorType have become optional for the findings/_search API. As part of the enhancement, the API can now return all the findings for all the detectors.
Added new params severity and detectionType in findings API
Added corresponding integration tests for the above functionality

Corresponding Alerting and Common-Utils PRs

Issues Resolved

#795

Check List

New functionality includes testing.
- All tests pass
New functionality has been documented.
- New functionality has javadoc added
Commits are signed per the DCO using --signoff

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.

Signed-off-by: Riya Saxena <riysaxen@amazon.com>

src/main/java/org/opensearch/securityanalytics/action/GetFindingsRequest.java

codecov · 2024-01-16T19:55:41Z

Codecov Report

Attention: Patch coverage is 0% with 41 lines in your changes are missing coverage. Please review.

Project coverage is 24.76%. Comparing base (75c4429) to head (2ffe265).
Report is 1 commits behind head on main.

❗ Current head 2ffe265 differs from pull request most recent head e967376. Consider uploading reports for the commit e967376 to get more accurate results

Files	Patch %	Lines
...nalytics/transport/TransportGetFindingsAction.java	0.00%	40 Missing ⚠️
...h/securityanalytics/action/GetFindingsRequest.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main     #803      +/-   ##
============================================
- Coverage     24.97%   24.76%   -0.22%     
+ Complexity     1043     1027      -16     
============================================
  Files           277      277              
  Lines         12771    12722      -49     
  Branches       1391     1402      +11     
============================================
- Hits           3190     3150      -40     
  Misses         9307     9307              
+ Partials        274      265       -9

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

src/main/java/org/opensearch/securityanalytics/transport/TransportGetFindingsAction.java

src/test/java/org/opensearch/securityanalytics/findings/FindingIT.java

eirsep · 2024-01-18T21:00:52Z

i am not sure this design is correct or maybe i have grossly misunderstood the approach.

Can you add a detailed approach of how findings from detectors of all log types are being fetched?

eirsep

i am not sure this design is correct or maybe i have grossly misunderstood the approach.

Can you add a detailed approach of how findings from detectors of all log types are being fetched?

riysaxen-amzn · 2024-01-18T21:45:36Z

i am not sure this design is correct or maybe i have grossly misunderstood the approach.

Can you add a detailed approach of how findings from detectors of all log types are being fetched?

So, the idea is first to get all the detectors using matchAllQuery [TransportGetFindingsAction.java#L190]. Now to get the findings for all these detectors for all the logTypes, we can search query on index-pattern .opensearch-sap-*-findings, this will give us all the findings for all the LogTypes [TransportGetFindingsAction.java#L143]

src/main/java/org/opensearch/securityanalytics/transport/TransportGetFindingsAction.java

eirsep · 2024-01-18T22:57:18Z

src/main/java/org/opensearch/securityanalytics/transport/TransportGetFindingsAction.java

-                public void onFailure(Exception e) {
+                    findingsService.getFindings(
+                            detectors,
+                            request.getLogType() == null ? "*" : request.getLogType(),


how is findings service already handling "*"? I don't see a change in FIndingsService class in this PR

eirsep · 2024-01-18T23:28:37Z

i am not sure this design is correct or maybe i have grossly misunderstood the approach.
Can you add a detailed approach of how findings from detectors of all log types are being fetched?

So, the idea is first to get all the detectors using matchAllQuery [TransportGetFindingsAction.java#L190]. Now to get the findings for all these detectors for all the logTypes, we can search query on index-pattern .opensearch-sap-*-findings, this will give us all the findings for all the LogTypes [TransportGetFindingsAction.java#L143]

Gotcha! thank you. I wasn't aware that we already are searching on the "all finding indices pattern"

Can you create 2 detectors for different log types and test fetching findings for only one of the log types and verify that behaviour is correct?

riysaxen-amzn · 2024-01-18T23:34:12Z

i am not sure this design is correct or maybe i have grossly misunderstood the approach.
Can you add a detailed approach of how findings from detectors of all log types are being fetched?

So, the idea is first to get all the detectors using matchAllQuery [TransportGetFindingsAction.java#L190]. Now to get the findings for all these detectors for all the logTypes, we can search query on index-pattern .opensearch-sap-*-findings, this will give us all the findings for all the LogTypes [TransportGetFindingsAction.java#L143]

Gotcha! thank you. I wasn't aware that we already are searching on the "all finding indices pattern"

Can you create 2 detectors for different log types and test fetching findings for only one of the log types and verify that behaviour is correct?

this test is creating detectors of two different logTypes and verifying the behavior -> https://github.com/opensearch-project/security-analytics/pull/803/files#diff-d37f0c96e5ab58c9a80449652179f36f9696044aa275fc7c7f33ce2677fa6793

Signed-off-by: Riya Saxena <riysaxen@amazon.com>

engechas · 2024-03-06T17:44:07Z

src/main/java/org/opensearch/securityanalytics/resthandler/RestGetFindingsAction.java

+        if (startTimeParam != null && !startTimeParam.isEmpty()) {
+            try {
+                startTime = Instant.ofEpochMilli(Long.parseLong(startTimeParam));
+            } catch (NumberFormatException | NullPointerException | DateTimeException e) {
+                // Handle the parsing error
+                // For example, log the error or provide a default value
+                startTime = Instant.now(); // Default value or fallback
+            }
+        }


Minor - could create a private method for this to avoid the duplication with the endTime parsing

jowg-amazon

LGTM

jowg-amazon · 2024-03-09T00:17:10Z

Fix merge conflicts + make sure CI's pass

Signed-off-by: Riya <69919272+riysaxen-amzn@users.noreply.github.com>

approved

Signed-off-by: Subhobrata Dey <sbcd90@gmail.com>

fix integ tests for findings

opensearch-trigger-bot · 2024-03-11T05:27:19Z

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.x 2.x
# Navigate to the new working tree
cd .worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-803-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 0d1d599fea536ee2afe088f70ee1980a0ec572fe
# Push it to GitHub
git push --set-upstream origin backport/backport-803-to-2.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-803-to-2.x.

…ect#803) * get all findings as part of findings API enhancement Signed-off-by: Riya Saxena <riysaxen@amazon.com> * findingsAPI feature enhancements (address comments to prev PR) Signed-off-by: Riya Saxena <riysaxen@amazon.com> * findingsAPI feature enhancements (address comments to prev PR) Signed-off-by: Riya Saxena <riysaxen@amazon.com> * added support for param in Finding API Signed-off-by: Riya Saxena <riysaxen@amazon.com> * added detectionType as param for Findings API enhancements Signed-off-by: Riya Saxena <riysaxen@amazon.com> * added few tests to validate findings by params Signed-off-by: Riya Saxena <riysaxen@amazon.com> * added test for searchString param in FindingsAPI Signed-off-by: Riya Saxena <riysaxen@amazon.com> * adding addiional params findingIds, startTime and endTime as findings API enhancement Signed-off-by: Riya Saxena <riysaxen@amazon.com> * added params in getFindingsByDetectorId func * changed the startTime and endTime req input format * fix merge conflixt * fix integ test failures in findings API * fix integ tests * fix integ tests for findings Signed-off-by: Subhobrata Dey <sbcd90@gmail.com> --------- Signed-off-by: Riya Saxena <riysaxen@amazon.com> Signed-off-by: Riya <69919272+riysaxen-amzn@users.noreply.github.com> Signed-off-by: Subhobrata Dey <sbcd90@gmail.com> Co-authored-by: Subhobrata Dey <sbcd90@gmail.com>

* get all findings as part of findings API enhancement (#803) * get all findings as part of findings API enhancement Signed-off-by: Riya Saxena <riysaxen@amazon.com> * findingsAPI feature enhancements (address comments to prev PR) Signed-off-by: Riya Saxena <riysaxen@amazon.com> * findingsAPI feature enhancements (address comments to prev PR) Signed-off-by: Riya Saxena <riysaxen@amazon.com> * added support for param in Finding API Signed-off-by: Riya Saxena <riysaxen@amazon.com> * added detectionType as param for Findings API enhancements Signed-off-by: Riya Saxena <riysaxen@amazon.com> * added few tests to validate findings by params Signed-off-by: Riya Saxena <riysaxen@amazon.com> * added test for searchString param in FindingsAPI Signed-off-by: Riya Saxena <riysaxen@amazon.com> * adding addiional params findingIds, startTime and endTime as findings API enhancement Signed-off-by: Riya Saxena <riysaxen@amazon.com> * added params in getFindingsByDetectorId func * changed the startTime and endTime req input format * fix merge conflixt * fix integ test failures in findings API * fix integ tests * fix integ tests for findings Signed-off-by: Subhobrata Dey <sbcd90@gmail.com> --------- Signed-off-by: Riya Saxena <riysaxen@amazon.com> Signed-off-by: Riya <69919272+riysaxen-amzn@users.noreply.github.com> Signed-off-by: Subhobrata Dey <sbcd90@gmail.com> Co-authored-by: Subhobrata Dey <sbcd90@gmail.com> * fix integ test (#918) Signed-off-by: Joanne Wang <jowg@amazon.com> * Feature findings api enhancements (#914) * get all findings as part of findings API enhancement Signed-off-by: Riya Saxena <riysaxen@amazon.com> * findingsAPI feature enhancements (address comments to prev PR) Signed-off-by: Riya Saxena <riysaxen@amazon.com> * findingsAPI feature enhancements (address comments to prev PR) Signed-off-by: Riya Saxena <riysaxen@amazon.com> * added support for param in Finding API Signed-off-by: Riya Saxena <riysaxen@amazon.com> * added detectionType as param for Findings API enhancements Signed-off-by: Riya Saxena <riysaxen@amazon.com> * added few tests to validate findings by params Signed-off-by: Riya Saxena <riysaxen@amazon.com> * added test for searchString param in FindingsAPI Signed-off-by: Riya Saxena <riysaxen@amazon.com> * adding addiional params findingIds, startTime and endTime as findings API enhancement Signed-off-by: Riya Saxena <riysaxen@amazon.com> * added params in getFindingsByDetectorId func * changed the startTime and endTime req input format * fix merge conflixt * fix integ test failures in findings API * fix integ tests * refactored the logic Signed-off-by: Riya Saxena <riysaxen@amazon.com> * remove unused imports * address the pr comments Signed-off-by: Riya Saxena <riysaxen@amazon.com> * address pr comments Signed-off-by: Riya Saxena <riysaxen@amazon.com> * SA integ tests fix * SA integ tests fix * fix integ tests for findings Signed-off-by: Subhobrata Dey <sbcd90@gmail.com> * fix conflixt errors Signed-off-by: Riya Saxena <riysaxen@amazon.com> * fix conflixt errors Signed-off-by: Riya Saxena <riysaxen@amazon.com> * fix conflixt errors Signed-off-by: Riya Saxena <riysaxen@amazon.com> * fix conflixt errors Signed-off-by: Riya Saxena <riysaxen@amazon.com> * fix integ tests Signed-off-by: Riya Saxena <riysaxen@amazon.com> * fix integ tests Signed-off-by: Riya Saxena <riysaxen@amazon.com> * fix integ tests Signed-off-by: Riya Saxena <riysaxen@amazon.com> * fix flaky integ tests Signed-off-by: Riya Saxena <riysaxen@amazon.com> * address pr comments Signed-off-by: Riya Saxena <riysaxen@amazon.com> --------- Signed-off-by: Riya Saxena <riysaxen@amazon.com> Signed-off-by: Riya <69919272+riysaxen-amzn@users.noreply.github.com> Signed-off-by: Subhobrata Dey <sbcd90@gmail.com> Co-authored-by: Subhobrata Dey <sbcd90@gmail.com> --------- Signed-off-by: Riya Saxena <riysaxen@amazon.com> Signed-off-by: Riya <69919272+riysaxen-amzn@users.noreply.github.com> Signed-off-by: Subhobrata Dey <sbcd90@gmail.com> Signed-off-by: Joanne Wang <jowg@amazon.com> Co-authored-by: Riya <69919272+riysaxen-amzn@users.noreply.github.com> Co-authored-by: Subhobrata Dey <sbcd90@gmail.com>

…ect#803) * get all findings as part of findings API enhancement Signed-off-by: Riya Saxena <riysaxen@amazon.com> * findingsAPI feature enhancements (address comments to prev PR) Signed-off-by: Riya Saxena <riysaxen@amazon.com> * findingsAPI feature enhancements (address comments to prev PR) Signed-off-by: Riya Saxena <riysaxen@amazon.com> * added support for param in Finding API Signed-off-by: Riya Saxena <riysaxen@amazon.com> * added detectionType as param for Findings API enhancements Signed-off-by: Riya Saxena <riysaxen@amazon.com> * added few tests to validate findings by params Signed-off-by: Riya Saxena <riysaxen@amazon.com> * added test for searchString param in FindingsAPI Signed-off-by: Riya Saxena <riysaxen@amazon.com> * adding addiional params findingIds, startTime and endTime as findings API enhancement Signed-off-by: Riya Saxena <riysaxen@amazon.com> * added params in getFindingsByDetectorId func * changed the startTime and endTime req input format * fix merge conflixt * fix integ test failures in findings API * fix integ tests * fix integ tests for findings Signed-off-by: Subhobrata Dey <sbcd90@gmail.com> --------- Signed-off-by: Riya Saxena <riysaxen@amazon.com> Signed-off-by: Riya <69919272+riysaxen-amzn@users.noreply.github.com> Signed-off-by: Subhobrata Dey <sbcd90@gmail.com> Co-authored-by: Subhobrata Dey <sbcd90@gmail.com>

* support object fields in aggregation based sigma rules (#789) Signed-off-by: Subhobrata Dey <sbcd90@gmail.com> * Fix duplicate ecs mappings which returns incorrect log index field in mapping view API (#786) (#788) * field mapping changes Signed-off-by: Joanne Wang <jowg@amazon.com> * add integ test Signed-off-by: Joanne Wang <jowg@amazon.com> * turn unmappedfieldaliases as set and add integ test Signed-off-by: Joanne Wang <jowg@amazon.com> * add comments Signed-off-by: Joanne Wang <jowg@amazon.com> * fix integ tests Signed-off-by: Joanne Wang <jowg@amazon.com> * moved logic to method for better readability Signed-off-by: Joanne Wang <jowg@amazon.com> --------- Signed-off-by: Joanne Wang <jowg@amazon.com> * Fix get mappings view API incorrectly returning ecs path (#867) * add logic and integ tests to not add duplicate to log-types-config index Signed-off-by: Joanne Wang <jowg@amazon.com> * change naming for existingFieldMapping and change contains to equals Signed-off-by: Joanne Wang <jowg@amazon.com> --------- Signed-off-by: Joanne Wang <jowg@amazon.com> * fix integ test (#918) Signed-off-by: Joanne Wang <jowg@amazon.com> * get all findings as part of findings API enhancement (#803) * get all findings as part of findings API enhancement Signed-off-by: Riya Saxena <riysaxen@amazon.com> * findingsAPI feature enhancements (address comments to prev PR) Signed-off-by: Riya Saxena <riysaxen@amazon.com> * findingsAPI feature enhancements (address comments to prev PR) Signed-off-by: Riya Saxena <riysaxen@amazon.com> * added support for param in Finding API Signed-off-by: Riya Saxena <riysaxen@amazon.com> * added detectionType as param for Findings API enhancements Signed-off-by: Riya Saxena <riysaxen@amazon.com> * added few tests to validate findings by params Signed-off-by: Riya Saxena <riysaxen@amazon.com> * added test for searchString param in FindingsAPI Signed-off-by: Riya Saxena <riysaxen@amazon.com> * adding addiional params findingIds, startTime and endTime as findings API enhancement Signed-off-by: Riya Saxena <riysaxen@amazon.com> * added params in getFindingsByDetectorId func * changed the startTime and endTime req input format * fix merge conflixt * fix integ test failures in findings API * fix integ tests * fix integ tests for findings Signed-off-by: Subhobrata Dey <sbcd90@gmail.com> --------- Signed-off-by: Riya Saxena <riysaxen@amazon.com> Signed-off-by: Riya <69919272+riysaxen-amzn@users.noreply.github.com> Signed-off-by: Subhobrata Dey <sbcd90@gmail.com> Co-authored-by: Subhobrata Dey <sbcd90@gmail.com> * Feature findings api enhancements (#914) * get all findings as part of findings API enhancement Signed-off-by: Riya Saxena <riysaxen@amazon.com> * findingsAPI feature enhancements (address comments to prev PR) Signed-off-by: Riya Saxena <riysaxen@amazon.com> * findingsAPI feature enhancements (address comments to prev PR) Signed-off-by: Riya Saxena <riysaxen@amazon.com> * added support for param in Finding API Signed-off-by: Riya Saxena <riysaxen@amazon.com> * added detectionType as param for Findings API enhancements Signed-off-by: Riya Saxena <riysaxen@amazon.com> * added few tests to validate findings by params Signed-off-by: Riya Saxena <riysaxen@amazon.com> * added test for searchString param in FindingsAPI Signed-off-by: Riya Saxena <riysaxen@amazon.com> * adding addiional params findingIds, startTime and endTime as findings API enhancement Signed-off-by: Riya Saxena <riysaxen@amazon.com> * added params in getFindingsByDetectorId func * changed the startTime and endTime req input format * fix merge conflixt * fix integ test failures in findings API * fix integ tests * refactored the logic Signed-off-by: Riya Saxena <riysaxen@amazon.com> * remove unused imports * address the pr comments Signed-off-by: Riya Saxena <riysaxen@amazon.com> * address pr comments Signed-off-by: Riya Saxena <riysaxen@amazon.com> * SA integ tests fix * SA integ tests fix * fix integ tests for findings Signed-off-by: Subhobrata Dey <sbcd90@gmail.com> * fix conflixt errors Signed-off-by: Riya Saxena <riysaxen@amazon.com> * fix conflixt errors Signed-off-by: Riya Saxena <riysaxen@amazon.com> * fix conflixt errors Signed-off-by: Riya Saxena <riysaxen@amazon.com> * fix conflixt errors Signed-off-by: Riya Saxena <riysaxen@amazon.com> * fix integ tests Signed-off-by: Riya Saxena <riysaxen@amazon.com> * fix integ tests Signed-off-by: Riya Saxena <riysaxen@amazon.com> * fix integ tests Signed-off-by: Riya Saxena <riysaxen@amazon.com> * fix flaky integ tests Signed-off-by: Riya Saxena <riysaxen@amazon.com> * address pr comments Signed-off-by: Riya Saxena <riysaxen@amazon.com> --------- Signed-off-by: Riya Saxena <riysaxen@amazon.com> Signed-off-by: Riya <69919272+riysaxen-amzn@users.noreply.github.com> Signed-off-by: Subhobrata Dey <sbcd90@gmail.com> Co-authored-by: Subhobrata Dey <sbcd90@gmail.com> * fix findings api integ tests Signed-off-by: Joanne Wang <jowg@amazon.com> --------- Signed-off-by: Subhobrata Dey <sbcd90@gmail.com> Signed-off-by: Joanne Wang <jowg@amazon.com> Signed-off-by: Riya Saxena <riysaxen@amazon.com> Signed-off-by: Riya <69919272+riysaxen-amzn@users.noreply.github.com> Co-authored-by: Subhobrata Dey <sbcd90@gmail.com> Co-authored-by: Riya <69919272+riysaxen-amzn@users.noreply.github.com>

get all findings as part of findings API enhancement

2ffe265

Signed-off-by: Riya Saxena <riysaxen@amazon.com>

riysaxen-amzn marked this pull request as ready for review January 16, 2024 17:12

riysaxen-amzn requested review from AWSHurneyt, amsiglan, eirsep, getsaurabh02, jowg-amazon, lezzago, praveensameneni and sbcd90 as code owners January 16, 2024 17:12

eirsep reviewed Jan 16, 2024

View reviewed changes

src/main/java/org/opensearch/securityanalytics/action/GetFindingsRequest.java Outdated Show resolved Hide resolved

jowg-amazon reviewed Jan 17, 2024

View reviewed changes

src/main/java/org/opensearch/securityanalytics/transport/TransportGetFindingsAction.java Outdated Show resolved Hide resolved

src/test/java/org/opensearch/securityanalytics/findings/FindingIT.java Show resolved Hide resolved

eirsep previously requested changes Jan 18, 2024

View reviewed changes