-
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
Add validation for threat intel source config #1393
Add validation for threat intel source config #1393
Conversation
Signed-off-by: Joanne Wang <[email protected]>
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 add test to verify #1366 is solved
Signed-off-by: Joanne Wang <[email protected]>
@@ -24,7 +24,19 @@ public class SourceConfigDtoValidator { | |||
public List<String> validateSourceConfigDto(SATIFSourceConfigDto sourceConfigDto) { |
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.
As a follow-up item, let's add validation for the refresh frequency.
@@ -24,7 +24,19 @@ public class SourceConfigDtoValidator { | |||
public List<String> validateSourceConfigDto(SATIFSourceConfigDto sourceConfigDto) { | |||
List<String> errorMsgs = new ArrayList<>(); | |||
|
|||
if (sourceConfigDto.getIocTypes().isEmpty()) { | |||
if (sourceConfigDto.getName() == null || sourceConfigDto.getName().isEmpty()) { |
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.
@jowg-amazon could we add length restrictions? The frontend uses this validation for names.
https://github.com/opensearch-project/security-analytics-dashboards-plugin/blob/main/public/utils/validation.ts#L7C14-L7C33
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.
added validation for names based on how frontend currently validates the name
errorMsgs.add("Name must not be empty"); | ||
} | ||
|
||
if (sourceConfigDto.getFormat() == null || sourceConfigDto.getFormat().isEmpty()) { |
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 may need to add length validation for this as well. Unless this gets validated somewhere downstream, the config could be created via backend API with something like 300k characters; which could disrupt functionality.
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.
Added length validation
Signed-off-by: Joanne Wang <[email protected]>
Added parsing tests to make sure null fields can be parsed |
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.
Changes LGTM. Approving assuming other's comments are addressed.
* add validation for source config and allow null to be read in parser Signed-off-by: Joanne Wang <[email protected]> * add parsing tests Signed-off-by: Joanne Wang <[email protected]> * add additional validation Signed-off-by: Joanne Wang <[email protected]> --------- Signed-off-by: Joanne Wang <[email protected]> (cherry picked from commit 364f42d) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* add validation for source config and allow null to be read in parser Signed-off-by: Joanne Wang <[email protected]> * add parsing tests Signed-off-by: Joanne Wang <[email protected]> * add additional validation Signed-off-by: Joanne Wang <[email protected]> --------- Signed-off-by: Joanne Wang <[email protected]> (cherry picked from commit 364f42d) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* add validation for source config and allow null to be read in parser Signed-off-by: Joanne Wang <[email protected]> * add parsing tests Signed-off-by: Joanne Wang <[email protected]> * add additional validation Signed-off-by: Joanne Wang <[email protected]> --------- Signed-off-by: Joanne Wang <[email protected]> (cherry picked from commit 364f42d) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* add validation for source config and allow null to be read in parser Signed-off-by: Joanne Wang <[email protected]> * add parsing tests Signed-off-by: Joanne Wang <[email protected]> * add additional validation Signed-off-by: Joanne Wang <[email protected]> --------- Signed-off-by: Joanne Wang <[email protected]> (cherry picked from commit 364f42d) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* add validation for source config and allow null to be read in parser Signed-off-by: Joanne Wang <[email protected]> * add parsing tests Signed-off-by: Joanne Wang <[email protected]> * add additional validation Signed-off-by: Joanne Wang <[email protected]> --------- Signed-off-by: Joanne Wang <[email protected]> (cherry picked from commit 364f42d) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* add validation for source config and allow null to be read in parser * add parsing tests * add additional validation --------- (cherry picked from commit 364f42d) Signed-off-by: Joanne Wang <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* add validation for source config and allow null to be read in parser * add parsing tests * add additional validation --------- (cherry picked from commit 364f42d) Signed-off-by: Joanne Wang <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* add validation for source config and allow null to be read in parser * add parsing tests * add additional validation --------- (cherry picked from commit 364f42d) Signed-off-by: Joanne Wang <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* add validation for source config and allow null to be read in parser * add parsing tests * add additional validation --------- (cherry picked from commit 364f42d) Signed-off-by: Joanne Wang <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* add validation for source config and allow null to be read in parser * add parsing tests * add additional validation --------- (cherry picked from commit 364f42d) Signed-off-by: Joanne Wang <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Description
Add source config validation so source config cannot be created without valid
name
,type
,format
,source
, andioc type
. Additionally allows source configs with null values to still be read and deleted.Related Issues
Resolves #1366
Check List
--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.