Skip to content

Commit

Permalink
Support updates of API key attributes [REST and transport layer] (#88186
Browse files Browse the repository at this point in the history
)


REST and transport layer implementation to add support for updating
attributes of existing API keys. This allows end-users to modify
privileges and metadata associated with API keys dynamically, without
requiring rolling out new API keys every time there is a change.

The new route supports updates to one API key, given its ID:

PUT /_security/api_key/{id}

The request body consists of optional fields role_descriptors and
metadata. If a request field is absent, the existing value of the
field on the given API key is retained. If a request field is set to
{} it replaces the existing value with {}. Explicit null-values for
request fields are not allowed and will produce a 400.
limited_by_role_descriptors, creator, and version are
automatically updated on every call. Attributes a replaced, not merged.

Only the owner user of an API key can update it. API keys cannot update
themselves, nor can other users (even users with all or
manage_security cluster privileges).

Relates: #87870
  • Loading branch information
n1v0lg committed Jul 7, 2022
1 parent b1070d3 commit 12bf451
Show file tree
Hide file tree
Showing 17 changed files with 759 additions and 215 deletions.
5 changes: 5 additions & 0 deletions docs/changelog/88186.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 88186
summary: Support updates of API key attributes (single operation route)
area: Authentication
type: feature
issues: []
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

package org.elasticsearch.xpack.core.security.action.apikey;

import org.elasticsearch.action.ActionType;

public final class UpdateApiKeyAction extends ActionType<UpdateApiKeyResponse> {

public static final String NAME = "cluster:admin/xpack/security/api_key/update";
public static final UpdateApiKeyAction INSTANCE = new UpdateApiKeyAction();

private UpdateApiKeyAction() {
super(NAME, UpdateApiKeyResponse::new);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,11 @@ public final class UpdateApiKeyRequest extends ActionRequest {
@Nullable
private final List<RoleDescriptor> roleDescriptors;

public UpdateApiKeyRequest(String id, @Nullable List<RoleDescriptor> roleDescriptors, @Nullable Map<String, Object> metadata) {
public UpdateApiKeyRequest(
final String id,
@Nullable final List<RoleDescriptor> roleDescriptors,
@Nullable final Map<String, Object> metadata
) {
this.id = Objects.requireNonNull(id, "API key ID must not be null");
this.roleDescriptors = roleDescriptors;
this.metadata = metadata;
Expand All @@ -40,7 +44,7 @@ public UpdateApiKeyRequest(String id, @Nullable List<RoleDescriptor> roleDescrip
public UpdateApiKeyRequest(StreamInput in) throws IOException {
super(in);
this.id = in.readString();
this.roleDescriptors = readOptionalList(in);
this.roleDescriptors = in.readOptionalList(RoleDescriptor::new);
this.metadata = in.readMap();
}

Expand All @@ -65,21 +69,12 @@ public ActionRequestValidationException validate() {
public void writeTo(StreamOutput out) throws IOException {
super.writeTo(out);
out.writeString(id);
writeOptionalList(out);
out.writeOptionalCollection(roleDescriptors);
out.writeGenericMap(metadata);
}

private List<RoleDescriptor> readOptionalList(StreamInput in) throws IOException {
return in.readBoolean() ? in.readList(RoleDescriptor::new) : null;
}

private void writeOptionalList(StreamOutput out) throws IOException {
if (roleDescriptors == null) {
out.writeBoolean(false);
} else {
out.writeBoolean(true);
out.writeList(roleDescriptors);
}
public static UpdateApiKeyRequest usingApiKeyId(String id) {
return new UpdateApiKeyRequest(id, null, null);
}

public String getId() {
Expand All @@ -93,4 +88,17 @@ public Map<String, Object> getMetadata() {
public List<RoleDescriptor> getRoleDescriptors() {
return roleDescriptors;
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
UpdateApiKeyRequest that = (UpdateApiKeyRequest) o;
return id.equals(that.id) && Objects.equals(metadata, that.metadata) && Objects.equals(roleDescriptors, that.roleDescriptors);
}

@Override
public int hashCode() {
return Objects.hash(id, metadata, roleDescriptors);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import org.elasticsearch.xpack.core.security.action.apikey.GrantApiKeyRequest;
import org.elasticsearch.xpack.core.security.action.apikey.InvalidateApiKeyRequest;
import org.elasticsearch.xpack.core.security.action.apikey.QueryApiKeyRequest;
import org.elasticsearch.xpack.core.security.action.apikey.UpdateApiKeyRequest;
import org.elasticsearch.xpack.core.security.authc.Authentication;
import org.elasticsearch.xpack.core.security.authc.AuthenticationField;
import org.elasticsearch.xpack.core.security.authc.RealmDomain;
Expand Down Expand Up @@ -61,6 +62,12 @@ private ManageOwnClusterPermissionCheck() {
protected boolean extendedCheck(String action, TransportRequest request, Authentication authentication) {
if (request instanceof CreateApiKeyRequest) {
return true;
} else if (request instanceof UpdateApiKeyRequest) {
// Note: we return `true` here even if the authenticated entity is an API key. API keys *cannot* update themselves,
// however this is a business logic restriction, rather than one driven solely by privileges. We therefore enforce this
// limitation at the transport layer, in `TransportUpdateApiKeyAction`.
// Ownership of an API key, for regular users, is enforced at the service layer.
return true;
} else if (request instanceof final GetApiKeyRequest getApiKeyRequest) {
return checkIfUserIsOwnerOfApiKeys(
authentication,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

package org.elasticsearch.xpack.core.security.action.apikey;

import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.common.io.stream.BytesStreamOutput;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.test.ESTestCase;
Expand All @@ -15,6 +16,11 @@
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;

import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.containsStringIgnoringCase;
import static org.hamcrest.Matchers.equalTo;

public class UpdateApiKeyRequestTests extends ESTestCase {

Expand Down Expand Up @@ -50,4 +56,46 @@ public void testSerialization() throws IOException {
}
}
}

public void testMetadataKeyValidation() {
final var reservedKey = "_" + randomAlphaOfLengthBetween(0, 10);
final var metadataValue = randomAlphaOfLengthBetween(1, 10);
UpdateApiKeyRequest request = new UpdateApiKeyRequest(randomAlphaOfLength(10), null, Map.of(reservedKey, metadataValue));
final ActionRequestValidationException ve = request.validate();
assertNotNull(ve);
assertThat(ve.validationErrors().size(), equalTo(1));
assertThat(ve.validationErrors().get(0), containsString("API key metadata keys may not start with [_]"));
}

public void testRoleDescriptorValidation() {
final var request1 = new UpdateApiKeyRequest(
randomAlphaOfLength(10),
List.of(
new RoleDescriptor(
randomAlphaOfLength(5),
new String[] { "manage_index_template" },
new RoleDescriptor.IndicesPrivileges[] {
RoleDescriptor.IndicesPrivileges.builder().indices("*").privileges("rad").build() },
new RoleDescriptor.ApplicationResourcePrivileges[] {
RoleDescriptor.ApplicationResourcePrivileges.builder()
.application(randomFrom("app*tab", "app 1"))
.privileges(randomFrom(" ", "\n"))
.resources("resource")
.build() },
null,
null,
Map.of("_key", "value"),
null
)
),
null
);
final ActionRequestValidationException ve1 = request1.validate();
assertNotNull(ve1);
assertThat(ve1.validationErrors().get(0), containsString("unknown cluster privilege"));
assertThat(ve1.validationErrors().get(1), containsString("unknown index privilege"));
assertThat(ve1.validationErrors().get(2), containsStringIgnoringCase("application name"));
assertThat(ve1.validationErrors().get(3), containsStringIgnoringCase("Application privilege names"));
assertThat(ve1.validationErrors().get(4), containsStringIgnoringCase("role descriptor metadata keys may not start with "));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,10 @@ public static Authentication.RealmRef randomRealmRef(boolean underDomain, boolea
}
}

public static RealmConfig.RealmIdentifier randomRealmIdentifier(boolean includeInternal) {
return new RealmConfig.RealmIdentifier(randomRealmTypeSupplier(includeInternal).get(), ESTestCase.randomAlphaOfLengthBetween(3, 8));
}

private static Supplier<String> randomRealmTypeSupplier(boolean includeInternal) {
final Supplier<String> randomAllRealmTypeSupplier = () -> ESTestCase.randomFrom(
"reserved",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import org.elasticsearch.xpack.core.security.action.apikey.InvalidateApiKeyRequest;
import org.elasticsearch.xpack.core.security.action.apikey.QueryApiKeyAction;
import org.elasticsearch.xpack.core.security.action.apikey.QueryApiKeyRequest;
import org.elasticsearch.xpack.core.security.action.apikey.UpdateApiKeyRequest;
import org.elasticsearch.xpack.core.security.authc.Authentication;
import org.elasticsearch.xpack.core.security.authc.AuthenticationTestHelper;
import org.elasticsearch.xpack.core.security.authc.AuthenticationTests;
Expand All @@ -40,12 +41,21 @@ public void testAuthenticationWithApiKeyAllowsAccessToApiKeyActionsWhenItIsOwner
final Authentication authentication = AuthenticationTests.randomApiKeyAuthentication(userJoe, apiKeyId);
final TransportRequest getApiKeyRequest = GetApiKeyRequest.usingApiKeyId(apiKeyId, randomBoolean());
final TransportRequest invalidateApiKeyRequest = InvalidateApiKeyRequest.usingApiKeyId(apiKeyId, randomBoolean());

assertTrue(clusterPermission.check("cluster:admin/xpack/security/api_key/get", getApiKeyRequest, authentication));
assertTrue(clusterPermission.check("cluster:admin/xpack/security/api_key/invalidate", invalidateApiKeyRequest, authentication));
assertFalse(clusterPermission.check("cluster:admin/something", mock(TransportRequest.class), authentication));
}

public void testAuthenticationForUpdateApiKeyAllowsAll() {
final ClusterPermission clusterPermission = ManageOwnApiKeyClusterPrivilege.INSTANCE.buildPermission(ClusterPermission.builder())
.build();
final String apiKeyId = randomAlphaOfLengthBetween(4, 7);
final Authentication authentication = AuthenticationTestHelper.builder().build();
final TransportRequest updateApiKeyRequest = UpdateApiKeyRequest.usingApiKeyId(apiKeyId);

assertTrue(clusterPermission.check("cluster:admin/xpack/security/api_key/update", updateApiKeyRequest, authentication));
}

public void testAuthenticationWithApiKeyDeniesAccessToApiKeyActionsWhenItIsNotOwner() {
final ClusterPermission clusterPermission = ManageOwnApiKeyClusterPrivilege.INSTANCE.buildPermission(ClusterPermission.builder())
.build();
Expand All @@ -69,8 +79,10 @@ public void testAuthenticationWithUserAllowsAccessToApiKeyActionsWhenItIsOwner()

TransportRequest getApiKeyRequest = GetApiKeyRequest.usingRealmAndUserName(realmRef.getName(), "joe");
TransportRequest invalidateApiKeyRequest = InvalidateApiKeyRequest.usingRealmAndUserName(realmRef.getName(), "joe");
TransportRequest updateApiKeyRequest = UpdateApiKeyRequest.usingApiKeyId(randomAlphaOfLength(10));
assertTrue(clusterPermission.check("cluster:admin/xpack/security/api_key/get", getApiKeyRequest, authentication));
assertTrue(clusterPermission.check("cluster:admin/xpack/security/api_key/invalidate", invalidateApiKeyRequest, authentication));
assertTrue(clusterPermission.check("cluster:admin/xpack/security/api_key/update", updateApiKeyRequest, authentication));

assertFalse(clusterPermission.check("cluster:admin/something", mock(TransportRequest.class), authentication));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ public class Constants {
"cluster:admin/xpack/security/api_key/grant",
"cluster:admin/xpack/security/api_key/invalidate",
"cluster:admin/xpack/security/api_key/query",
"cluster:admin/xpack/security/api_key/update",
"cluster:admin/xpack/security/cache/clear",
"cluster:admin/xpack/security/delegate_pki",
"cluster:admin/xpack/security/enroll/node",
Expand Down

0 comments on commit 12bf451

Please sign in to comment.