-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Omit runtime fields from FLS suggestions #78330
Omit runtime fields from FLS suggestions #78330
Conversation
@@ -8,6 +8,20 @@ import { schema } from '@kbn/config-schema'; | |||
import { RouteDefinitionParams } from '../index'; | |||
import { wrapIntoCustomErrorResponse } from '../../errors'; | |||
|
|||
interface FieldMappingResponse { |
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.
Improved type safety, at least until we can migrate away from the legacy ES client.
Pinging @elastic/kibana-security (Team:Security) |
@elasticmachine merge upstream |
ACK: will review today |
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.
LGTM, thanks! Tested locally and works as expected, just one nit and a question.
// 4. Use `Set` to get only unique field names. | ||
const fields = Array.from( |
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.
question: how do you feel about adding a simple jest test to test this logic?
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.
Good idea, will do!
|
||
const isRuntimeField = hasMapping && mappingValues[0]?.type === 'runtime'; | ||
|
||
// fields without mappings are internal fields such as `_routing` and `_index`, |
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.
👍
Co-authored-by: Aleh Zasypkin <[email protected]>
…omit-runtime-fields
💚 Build SucceededMetrics [docs]
History
To update your PR or re-run it, just comment with: |
Co-authored-by: Aleh Zasypkin <[email protected]> Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: Aleh Zasypkin <[email protected]> Co-authored-by: Elastic Machine <[email protected]>
This reverts commit 4525f0c.
elastic#79106)" This reverts commit f0be469.
…) (#79939) Co-authored-by: spalger <[email protected]>
Summary
Runtime fields are not securable via FLS - it's technically possible to assign them as a granted/denied field, but this is effectively a no-op. In order to avoid confusion, we are updating the list of available options to omit runtime fields.
Resolves #78329