Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[SECURITY_SOLUTION][ENDPOINT] Trusted Apps - fix error for duplicate fields to correctly mention the field at fault #79853

Merged
merged 2 commits into from
Oct 7, 2020
Merged

[SECURITY_SOLUTION][ENDPOINT] Trusted Apps - fix error for duplicate fields to correctly mention the field at fault #79853

merged 2 commits into from
Oct 7, 2020

Conversation

paul-tavares
Copy link
Contributor

Summary

Fixes Trusted Apps API validation for duplicate fields always reporting that Hash was at fault even when Path might have been the value entered more than once in the UI

Duplicate Path's:

image

Duplicate Hashes:

image

@paul-tavares paul-tavares added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:Endpoint Management Feature:Endpoint Elastic Endpoint feature v7.10.0 v7.11.0 labels Oct 7, 2020
@paul-tavares paul-tavares requested review from a team as code owners October 7, 2020 14:52
@paul-tavares paul-tavares self-assigned this Oct 7, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/endpoint-management (Team:Endpoint Management)

@elasticmachine
Copy link
Contributor

Pinging @elastic/endpoint-app-team (Feature:Endpoint)

kevinlog
kevinlog reviewed Oct 7, 2020
View reviewed changes
x-pack/plugins/security_solution/common/endpoint/schema/trusted_apps.ts
const entryFieldLabels: { [k in TrustedApp['entries'][0]['field']]: string } = {
'process.hash.*': 'Hash',
'process.executable.caseless': 'Path',
'process.code_signature': 'Signer',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Signer?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, its already part of the Type (TrustedApp), thus its coming up as required in this object (typescript error). I did not want to exclude it out of the type here in order to keep the type simpler.

@nnamdifrankie
Copy link
Contributor

No tests or test update?

@paul-tavares
Copy link
Contributor Author

@nnamdifrankie The test currently for the Schema do not validate the actual error message text - only that the schema throw' s indicating an error was hit. it does that so that if we ever switch schema providers (ex. move to io-ts), then the tests should still work.

That being said, this particular validation is custom - meaning that its not coming out of the Schema library, but rather our of our own custom validator - so I guess that pattern detailed above can be broken. I will add one (or two) now.

Thanks

@nnamdifrankie
Copy link
Contributor

our own custom validator - so I guess that pattern detailed above can be broken. I will add one (or two) now.

Thanks.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@paul-tavares paul-tavares merged commit 4da338a into elastic:master Oct 7, 2020
@paul-tavares paul-tavares deleted the fix/olm-405-server-validation-error-msg branch October 7, 2020 17:35
@paul-tavares paul-tavares mentioned this pull request Oct 7, 2020
Merged
paul-tavares added a commit to paul-tavares/kibana that referenced this pull request Oct 7, 2020
@paul-tavares
[SECURITY_SOLUTION][ENDPOINT] Trusted Apps - fix error for duplicate …
392a1c2 
…fields to correctly mention the field at fault (elastic#79853)

* Fix error for duplicate fields to correctly mention the field at fault
* Add new tests to duplicate field validation
paul-tavares added a commit to paul-tavares/kibana that referenced this pull request Oct 7, 2020
@paul-tavares
[SECURITY_SOLUTION][ENDPOINT] Trusted Apps - fix error for duplicate …
7bc369c 
…fields to correctly mention the field at fault (elastic#79853)

* Fix error for duplicate fields to correctly mention the field at fault
* Add new tests to duplicate field validation
@paul-tavares paul-tavares mentioned this pull request Oct 7, 2020
Merged
paul-tavares added a commit that referenced this pull request Oct 7, 2020
@paul-tavares
[SECURITY_SOLUTION][ENDPOINT] Trusted Apps - fix error for duplicate …
3d6b3e8 
…fields to correctly mention the field at fault (#79853) (#79885)

* Fix error for duplicate fields to correctly mention the field at fault
* Add new tests to duplicate field validation
paul-tavares added a commit that referenced this pull request Oct 7, 2020
@paul-tavares
[SECURITY_SOLUTION][ENDPOINT] Trusted Apps - fix error for duplicate …
0ac81d3 
…fields to correctly mention the field at fault (#79853) (#79888)

* Fix error for duplicate fields to correctly mention the field at fault
* Add new tests to duplicate field validation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nnamdifrankie nnamdifrankie nnamdifrankie approved these changes

@kevinlog kevinlog kevinlog approved these changes

Assignees

@paul-tavares paul-tavares

Labels
Feature:Endpoint Elastic Endpoint feature release_note:skip Skip the PR/issue when compiling release notes v7.10.0 v7.11.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@paul-tavares @elasticmachine @nnamdifrankie @kibanamachine @kevinlog