YAML tests and docs for viewing API key role descriptors #89186
Conversation
This PR expands existing YAML tests and docs for the new role_descriptors field returned in both Get and Query API key calls. Relates: elastic#89166, elastic#89058
ywangd
added
>docs
elasticsearchmachine
added
Team:Docs
|
Pinging @elastic/es-docs (Team:Docs) |
|
Pinging @elastic/es-security (Team:Security) |
| --- | ||
| setup: | ||
| - skip: | ||
| features: headers |
There was a problem hiding this comment.
Adding a new YAML file instead of adding matches for role_descriptors to existing ones because:
- The matches are quite verbose and they clutter the existing tests, while keeping them separately is easier to have more targeted setup.
- It can be even more verbose once we add support for limited_role_descriptors
- Matching multiple API keys in response of QueryApiKey requires sorting and it is easier to setup in a new test (than retrofitting into existing ones)
There was a problem hiding this comment.
Could go with one of 40_{view or show or read}_role_descriptors.yml to make it more explicit that it's about Get and Query. Tots optional though.
There was a problem hiding this comment.
Changed to 40_view_role_descriptors.yml.
| <10> The role descriptors that are assigned to this API key when it was <<api-key-role-descriptors,created>> | ||
| or last <<security-api-update-api-key-api-key-role-descriptors,updated>>. Note the API key's | ||
| effective permissions are an intersection of its assigned privileges and the point-in-time snapshot of | ||
| the owner user's permissions. |
There was a problem hiding this comment.
Nothing to address now, but once we have the API key overview page we can link to a section on API key permissions there, for more detail.
| --- | ||
| setup: | ||
| - skip: | ||
| features: headers |
There was a problem hiding this comment.
Could go with one of 40_{view or show or read}_role_descriptors.yml to make it more explicit that it's about Get and Query. Tots optional though.
| @@ -15,7 +15,11 @@ Retrieves information for one or more API keys. | |||
| [[security-api-get-api-key-prereqs]] | |||
| ==== {api-prereq-title} | |||
|
|
|||
| * To use this API, you must have at least the `manage_api_key` cluster privilege. | |||
| * To use this API, you must have at least the `manage_own_api_key` cluster privilege. | |||
| * If you have only the `manage_own_api_key` privilege, this API returns only | |||
| "applications": [ ], | ||
| "run_as": [ ], | ||
| "metadata": { }, | ||
| "transient_metadata": { |
There was a problem hiding this comment.
transient_metadata feels more like an internal value as opposed to something that would be part of the public API, but we already expose it via the roles API as well so it makes sense to stay consistent.
There was a problem hiding this comment.
No it is intentional for transient_metadata to be in the response, at least for regular roles. When a role has DLS/FLS but the cluster license does not allow it, the transient_metadata will have this information to let user know that the role is disabled and for what reason, e.g.:
"transient_metadata": {
"unlicensed_features": ["dls", "fls"],
"enabled": false
}
That said, the license enforcement for API key's permission works differently and does not rely on the transient metadata. But for future proof and consistent, I'd say it's better to keep it in the response.
There was a problem hiding this comment.
Got it. I didn't realize this was meant for end-users. Makes sense to keep it in the response 👍
Co-authored-by: Nikolaj Volgushev <[email protected]>
|
@elasticmachine update branch |
|
@elasticmachine run elasticsearch-ci/docs |
Co-authored-by: Nikolaj Volgushev <[email protected]>
ywangd
added
the
auto-merge-without-approval
|
@elasticmachine run elasticsearch-ci/part-1 |
This PR expands existing YAML tests and docs for the new
role_descriptors field returned in both Get and Query API key calls.
Relates: #89166, #89058