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.

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

[DOCS] Update API key API #88499

Merged
merged 58 commits into from
Jul 21, 2022
Merged

[DOCS] Update API key API #88499

merged 58 commits into from
Jul 21, 2022

Conversation

n1v0lg
Copy link
Contributor

@n1v0lg n1v0lg commented Jul 13, 2022

API docs for the new PUT /_security/api_key/<id> API.

Link to doc preview.

@n1v0lg n1v0lg added >docs General docs changes :Security/Security Security issues without another label labels Jul 13, 2022
@n1v0lg n1v0lg self-assigned this Jul 13, 2022
====
A call to this API might change the API key's access scope, even if you don't specify new
<<security-api-update-api-key-api-key-role-descriptors,`role_descriptors`>> in the request.
This behavior can occur if the owner user's permissions have changed since the creation or last update to the API key.
Copy link
Contributor Author

@n1v0lg n1v0lg Jul 19, 2022

Choose a reason for hiding this comment

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

The preceding paragraph now explains that the user permissions snapshot is auto-updated so I've removed the now redundant explanation from the callout.

Copy link
Member

@ywangd ywangd left a comment

Choose a reason for hiding this comment

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

LGTM

The content looks correct to me. I'll defer to @lockewritesdocs for polishing the writing. Thanks!

It's not possible to update expired API keys, or API keys that have been invalidated by <<security-api-invalidate-api-key,invalidate API Key>>.

This API supports updates to an API key's access scope, i.e., its permissions, and its metadata.
The access scope of an API key is derived from the <<security-api-update-api-key-api-key-role-descriptors,`role_descriptors`>> you specify in the request, and a snapshot of the owner user's permissions at the time of the request.
Copy link
Member

Choose a reason for hiding this comment

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

We can be more explicit here. Instead of saying "is derived", we can just say "is an intersection of ...".

Copy link
Contributor Author

@n1v0lg n1v0lg Jul 20, 2022

Choose a reason for hiding this comment

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

I don't think "intersection of" is technically correct here without the disclaimer that when role_descriptors is empty (i.e., not assigned) it's not the intersection. I went with "derived" because it side-steps that problem, and references the role desc section where we accurately describe how the permissions are derived.

x-pack/docs/en/rest-api/security/update-api-key.asciidoc Outdated Show resolved Hide resolved
x-pack/docs/en/rest-api/security/update-api-key.asciidoc Outdated Show resolved Hide resolved
x-pack/docs/en/rest-api/security/update-api-key.asciidoc Outdated Show resolved Hide resolved
x-pack/docs/en/rest-api/security/update-api-key.asciidoc Outdated Show resolved Hide resolved
x-pack/docs/en/rest-api/security/update-api-key.asciidoc Outdated Show resolved Hide resolved
@n1v0lg
Copy link
Contributor Author

n1v0lg commented Jul 21, 2022

@elasticmachine update branch plz

Copy link
Contributor

@lockewritesdocs lockewritesdocs left a comment

Choose a reason for hiding this comment

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

Left a few suggested non-blocking changes. Thanks for iterating on this one @n1v0lg!

Comment on lines 45 to 46
The request body is optional.
Following parameters can be specified in it:
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think that we need the additional (Optional) tag. What if we changed this to read:

You can specify the following parameters in the request body, which is optional.

Each of the parameters have an (Optional) tag already, so I think that this statement highlights that the entire request body is optional without explicitly including an additional tag.

}
----

The API key's effective permissions after the update will be the intersection of the supplied role descriptors and the <<security-api-update-api-key-examples-user-permissions, authenticated user's permissions>>:
Copy link
Contributor

Choose a reason for hiding this comment

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

+1 for including this information for now so that we have it somewhere. We can move it in a subsequent PR for creating an overview of API keys. Thanks @n1v0lg!

x-pack/docs/en/rest-api/security/update-api-key.asciidoc Outdated Show resolved Hide resolved
x-pack/docs/en/rest-api/security/update-api-key.asciidoc Outdated Show resolved Hide resolved
@n1v0lg
Copy link
Contributor Author

n1v0lg commented Jul 21, 2022

@elasticmachine run elasticsearch-ci/part-2

@n1v0lg n1v0lg merged commit 5b85f2e into elastic:master Jul 21, 2022
@n1v0lg n1v0lg deleted the update-api-keys-docs branch July 21, 2022 16:54
weizijun added a commit to weizijun/elasticsearch that referenced this pull request Jul 22, 2022
* upstream/master: (40 commits)
  Fix CI job naming
  [ML] disallow autoscaling downscaling in two trained model assignment scenarios (elastic#88623)
  Add "Vector Search" area to changelog schema
  [DOCS] Update API key API (elastic#88499)
  Enable the pipeline on the feature branch (elastic#88672)
  Adding the ability to register a PeerFinderListener to Coordinator (elastic#88626)
  [DOCS] Fix transform painless example syntax (elastic#88364)
  [ML] Muting InternalCategorizationAggregationTests testReduceRandom (elastic#88685)
  Fix double rounding errors for disk usage (elastic#88683)
  Replace health request with a state observer. (elastic#88641)
  [ML] Fail model deployment if all allocations cannot be provided (elastic#88656)
  Upgrade to OpenJDK 18.0.2+9 (elastic#88675)
  [ML] make bucket_correlation aggregation generally available (elastic#88655)
  Adding cardinality support for random_sampler agg (elastic#86838)
  Use custom task instead of generic AckedClusterStateUpdateTask (elastic#88643)
  Reinstate test cluster throttling behavior (elastic#88664)
  Mute testReadBlobWithPrematureConnectionClose
  Simplify plugin descriptor tests (elastic#88659)
  Add CI job for testing more job parallelism
  [ML] make deployment infer requests fully cancellable (elastic#88649)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>docs General docs changes :Security/Security Security issues without another label Team:Docs Meta label for docs team Team:Security Meta label for security team v8.4.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants