Fix ACL documentation discrepancies from source code audit#305
Open
sscarduzio wants to merge 2 commits intodevelopfrom
Open
Fix ACL documentation discrepancies from source code audit#305sscarduzio wants to merge 2 commits intodevelopfrom
sscarduzio wants to merge 2 commits intodevelopfrom
Conversation
- Fix `maxBodyLength` → `max_body_length` (correct snake_case key name) - Add missing `response_headers` rule documentation - Add group rule aliases table (groups, roles, any_of, all_of, etc.) - Add `external_authorization` alias note for `groups_provider_authorization` - Add "Incompatible rule combinations" section (kibana_access conflicts) - Document auth-only rule requirement (must pair with authentication rule) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This comment was marked as resolved.
This comment was marked as resolved.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Add sentence clarifying that groups/roles aliases apply only to ACL rule keys inside access blocks, not to the users section groups field. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
sscarduzio
commented
Mar 18, 2026
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
coutoPL
requested changes
Mar 19, 2026
| - then we check whether the authorized user groups are permitted in context of the rule | ||
|
|
||
| {% hint style="info" %} | ||
| **Groups rule aliases**: For backwards compatibility, the following YAML key aliases are recognized and treated identically to their canonical names: |
Collaborator
There was a problem hiding this comment.
We intentionally didn't mention it. We do care about backward compatibility, but we want our users to migrate to the new format. And most importantly, we don't want our new users (or existing users who are starting to use the feature) to not use the older format.
IMO, we should remove this section.
|
|
||
| #### `groups_provider_authorization` | ||
|
|
||
| Also available as the alias `external_authorization`. |
|
|
||
| A list of api keys expected in the header `X-Api-Key` | ||
|
|
||
| #### `response_headers` |
Collaborator
There was a problem hiding this comment.
no such rule in the ROR ES code 🙃
|
|
||
| | Rule A | Rule B | Reason | | ||
| |--------|--------|--------| | ||
| | `kibana_access` | `actions` | Kibana access internally manages action filtering | |
Collaborator
There was a problem hiding this comment.
The kibana_access rule is deprecated. We should not focus so much on it. The access level is now part of the kibana rule
| | `kibana_access` | `response_fields` | Kibana access conflicts with response field filtering | | ||
|
|
||
| {% hint style="info" %} | ||
| The composite `kibana` rule does **not** have these conflicts — it replaces the legacy separate rules (`kibana_access`, `kibana_index`, `kibana_hide_apps`, `kibana_template_index`) and handles these interactions internally. |
Collaborator
There was a problem hiding this comment.
This is not true.
The table should look like this:
| Rule A | Rule B | Additional condition | Reason |
|---|---|---|---|
kibana |
actions |
when kibana.access != unrestricted |
Kibana access internally manages action filtering |
kibana |
filter |
Kibana access conflicts with Document Level Security | |
kibana |
fields |
Kibana access conflicts with Field Level Security | |
kibana |
response_fields |
Kibana access conflicts with response field filtering |
Collaborator
|
@sscarduzio kindly reminder about this one |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Cross-referenced the public documentation against the plugin source code and found several discrepancies. This PR fixes the verified issues:
maxBodyLength→max_body_length: The YAML key was incorrectly documented in camelCase. The actual plugin key is snake_case.response_headersrule: This rule exists in the plugin but had zero documentation. Added description and usage example.groups_any_of,groups_all_of, etc.) have widely-used aliases (groups,roles,any_of, etc.) that were not mentioned. Added an alias reference table.external_authorizationalias missing: This alias forgroups_provider_authorizationwas not mentioned.kibana_access+actions/filter/fields/response_fields), plus the auth-only rule pairing requirement.Test plan
max_body_lengthis the correct key name in the plugin sourceresponse_headersrule description matches plugin behaviorRule.Name(...)definitions in sourceIncompatibleRuleschecks in source🤖 Generated with Claude Code
Summary by CodeRabbit