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

Invalidating cross cluster API keys requires manage_security #107411

Merged

Conversation

n1v0lg
Copy link
Contributor

@n1v0lg n1v0lg commented Apr 12, 2024

This PR updates the privilege model to require manage_security cluster privilege to invalidate cross cluster API keys, to better match the access requirements of the creation and update APIs. Requests made with lower privileges will receive descriptive errors in the response payload indicating failure to invalidate, for each cross cluster API key. There are no changes to invalidating REST API keys, nor to the Query or Get APIs.

@n1v0lg n1v0lg added >non-issue :Security/Security Security issues without another label labels Apr 12, 2024
@n1v0lg n1v0lg self-assigned this Apr 12, 2024
@n1v0lg n1v0lg changed the title POC invalidate cross cluster keys Invalidating cross cluster API keys requires manage_security Apr 15, 2024
String apiKeyName = request.getName();
String username = request.getUserName();
String[] realms = Strings.hasText(request.getRealmName()) ? new String[] { request.getRealmName() } : null;

final Authentication authentication = securityContext.getAuthentication();
if (authentication == null) {
listener.onFailure(new IllegalStateException("authentication is required"));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was a bug -- we need to return, if we call onFailure.

@elasticsearchmachine
Copy link
Collaborator

Hi @n1v0lg, I've created a changelog YAML for you.

for (String apiKeyId : apiKeyIds) {
UpdateRequest request = client.prepareUpdate(SECURITY_MAIN_ALIAS, apiKeyId)
.setDoc(Map.of("api_key_invalidated", true, "invalidation_time", invalidationTime))
.request();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Drive-by clean up: skipping request() since the add method on the bulk request builder is deprecated for request instances, and we should pass a builder instead.

@n1v0lg n1v0lg marked this pull request as ready for review April 15, 2024 12:40
@n1v0lg n1v0lg requested a review from jakelandis April 15, 2024 12:41
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-security (Team:Security)

@elasticsearchmachine elasticsearchmachine added the Team:Security Meta label for security team label Apr 15, 2024
@n1v0lg n1v0lg added :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC and removed :Security/Security Security issues without another label labels Apr 15, 2024
Copy link
Contributor

@jakelandis jakelandis left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for handling this !

@n1v0lg
Copy link
Contributor Author

n1v0lg commented Apr 15, 2024

@elasticmachine update branch

@n1v0lg n1v0lg added the auto-merge Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Apr 15, 2024
@n1v0lg
Copy link
Contributor Author

n1v0lg commented Apr 15, 2024

@elasticmachine run elasticsearch-ci/part-1-fips

@n1v0lg
Copy link
Contributor Author

n1v0lg commented Apr 15, 2024

@elasticmachine update branch

@n1v0lg n1v0lg removed the auto-merge Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Apr 15, 2024
@n1v0lg
Copy link
Contributor Author

n1v0lg commented Apr 16, 2024

@elasticmachine update branch

@n1v0lg n1v0lg added the auto-merge Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Apr 16, 2024
@elasticsearchmachine elasticsearchmachine merged commit d851c93 into elastic:main Apr 16, 2024
17 of 18 checks passed
@n1v0lg n1v0lg deleted the poc-invalidate-cross-cluster-api-keys branch April 16, 2024 07:11
elasticsearchmachine pushed a commit that referenced this pull request May 6, 2024
This PR documents privilege requirements for cross-cluster API key
invalidation, which were updated in
#107411.
n1v0lg added a commit to n1v0lg/elasticsearch that referenced this pull request May 6, 2024
This PR documents privilege requirements for cross-cluster API key
invalidation, which were updated in
elastic#107411.
elasticsearchmachine pushed a commit that referenced this pull request May 6, 2024
This PR documents privilege requirements for cross-cluster API key
invalidation, which were updated in
#107411.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-merge Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) >enhancement :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC Team:Security Meta label for security team v8.14.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants