honeymoose/OpenSearch
1
0
Fork 0
mirror of https://github.com/honeymoose/OpenSearch.git synced 2025-02-17 02:14:54 +00:00
Code Issues Packages Projects Releases Wiki Activity

Remove uniqueness constraint for API key name and make it optional (#47549) (#47959)

Browse Source 
Since we cannot guarantee the uniqueness of the API key `name` this commit removes the constraint and makes this field optional.

Closes #46646
This commit is contained in:
Yogesh Gaikwad 2019-10-12 22:22:16 +11:00 committed by GitHub
parent dc8080e88a
commit ac209c142c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 60 additions and 113 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
client/rest-high-level/src/main/java/org/elasticsearch/client/security/CreateApiKeyRequest.java
View File

@ -22,7 +22,6 @@ package org.elasticsearch.client.security;
import org.elasticsearch.client.Validatable;
import org.elasticsearch.client.security.user.privileges.Role;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.xcontent.ToXContentObject;
import org.elasticsearch.common.xcontent.XContentBuilder;
@ -47,12 +46,9 @@ public final class CreateApiKeyRequest implements Validatable, ToXContentObject
* @param roles list of {@link Role}s
* @param expiration to specify expiration for the API key
*/
public CreateApiKeyRequest(String name, List<Role> roles, @Nullable TimeValue expiration, @Nullable final RefreshPolicy refreshPolicy) {
if (Strings.hasText(name)) {
this.name = name;
} else {
throw new IllegalArgumentException("name must not be null or empty");
}
public CreateApiKeyRequest(@Nullable String name, List<Role> roles, @Nullable TimeValue expiration,
@Nullable final RefreshPolicy refreshPolicy) {
this.name = name;
this.roles = Objects.requireNonNull(roles, "roles may not be null");
this.expiration = expiration;
this.refreshPolicy = (refreshPolicy == null) ? RefreshPolicy.getDefault() : refreshPolicy;

4
client/rest-high-level/src/main/java/org/elasticsearch/client/security/support/ApiKey.java
View File

@ -21,6 +21,7 @@ package org.elasticsearch.client.security.support;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.xcontent.ConstructingObjectParser;
import org.elasticsearch.common.xcontent.ObjectParser;
import org.elasticsearch.common.xcontent.XContentParser;
import java.io.IOException;
@ -131,7 +132,8 @@ public final class ApiKey {
(args[3] == null) ? null : Instant.ofEpochMilli((Long) args[3]), (Boolean) args[4], (String) args[5], (String) args[6]);
});
static {
PARSER.declareString(constructorArg(), new ParseField("name"));
PARSER.declareField(optionalConstructorArg(), (p, c) -> p.textOrNull(), new ParseField("name"),
ObjectParser.ValueType.STRING_OR_NULL);
PARSER.declareString(constructorArg(), new ParseField("id"));
PARSER.declareLong(constructorArg(), new ParseField("creation"));
PARSER.declareLong(optionalConstructorArg(), new ParseField("expiration"));

4
client/rest-high-level/src/test/java/org/elasticsearch/client/security/GetApiKeyResponseTests.java
View File

@ -40,7 +40,9 @@ public class GetApiKeyResponseTests extends ESTestCase {
"user-a", "realm-x");
ApiKey apiKeyInfo2 = createApiKeyInfo("name2", "id-2", Instant.ofEpochMilli(100000L), Instant.ofEpochMilli(10000000L), true,
"user-b", "realm-y");
GetApiKeyResponse response = new GetApiKeyResponse(Arrays.asList(apiKeyInfo1, apiKeyInfo2));
ApiKey apiKeyInfo3 = createApiKeyInfo(null, "id-3", Instant.ofEpochMilli(100000L), null, true,
"user-c", "realm-z");
GetApiKeyResponse response = new GetApiKeyResponse(Arrays.asList(apiKeyInfo1, apiKeyInfo2, apiKeyInfo3));
final XContentType xContentType = randomFrom(XContentType.values());
final XContentBuilder builder = XContentFactory.contentBuilder(xContentType);
toXContent(response, builder);

6
docs/java-rest/high-level/security/create-api-key.asciidoc
View File

@ -12,8 +12,8 @@ API Key can be created using this API.
[id="{upid}-{api}-request"]
==== Create API Key Request
A +{request}+ contains name for the API key,
list of role descriptors to define permissions and
A +{request}+ contains an optional name for the API key,
an optional list of role descriptors to define permissions and
optional expiration for the generated API key.
If expiration is not provided then by default the API
keys do not expire.
@ -37,4 +37,4 @@ expiration.
include-tagged::{doc-tests-file}[{api}-response]
--------------------------------------------------
<1> the API key that can be used to authenticate to Elasticsearch.
<2> expiration if the API keys expire
<2> expiration if the API keys expire

2
x-pack/docs/en/rest-api/security/create-api-keys.asciidoc
View File

@ -44,7 +44,7 @@ service.
The following parameters can be specified in the body of a POST or PUT request:
`name`::
(string) Specifies the name for this API key.
(Optional, string) Specifies the name for this API key.
`role_descriptors`::
(Optional, array-of-role-descriptor) An array of role descriptors for this API

13
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/ApiKey.java
View File

@ -6,6 +6,7 @@
package org.elasticsearch.xpack.core.security.action;
import org.elasticsearch.Version;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
@ -49,7 +50,11 @@ public final class ApiKey implements ToXContentObject, Writeable {
}
public ApiKey(StreamInput in) throws IOException {
this.name = in.readString();
if (in.getVersion().onOrAfter(Version.V_7_5_0)) {
this.name = in.readOptionalString();
} else {
this.name = in.readString();
}
this.id = in.readString();
this.creation = in.readInstant();
this.expiration = in.readOptionalInstant();
@ -103,7 +108,11 @@ public final class ApiKey implements ToXContentObject, Writeable {
@Override
public void writeTo(StreamOutput out) throws IOException {
out.writeString(name);
if (out.getVersion().onOrAfter(Version.V_7_5_0)) {
out.writeOptionalString(name);
} else {
out.writeString(name);
}
out.writeString(id);
out.writeInstant(creation);
out.writeOptionalInstant(expiration);

34
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/CreateApiKeyRequest.java
View File

@ -6,11 +6,11 @@
package org.elasticsearch.xpack.core.security.action;
import org.elasticsearch.Version;
import org.elasticsearch.action.ActionRequest;
import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.action.support.WriteRequest;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.unit.TimeValue;
@ -43,19 +43,19 @@ public final class CreateApiKeyRequest extends ActionRequest {
* @param roleDescriptors list of {@link RoleDescriptor}s
* @param expiration to specify expiration for the API key
*/
public CreateApiKeyRequest(String name, @Nullable List<RoleDescriptor> roleDescriptors, @Nullable TimeValue expiration) {
if (Strings.hasText(name)) {
this.name = name;
} else {
throw new IllegalArgumentException("name must not be null or empty");
}
public CreateApiKeyRequest(@Nullable String name, @Nullable List<RoleDescriptor> roleDescriptors, @Nullable TimeValue expiration) {
this.name = name;
this.roleDescriptors = (roleDescriptors == null) ? Collections.emptyList() : Collections.unmodifiableList(roleDescriptors);
this.expiration = expiration;
}
public CreateApiKeyRequest(StreamInput in) throws IOException {
super(in);
this.name = in.readString();
if (in.getVersion().onOrAfter(Version.V_7_5_0)) {
this.name = in.readOptionalString();
} else {
this.name = in.readString();
}
this.expiration = in.readOptionalTimeValue();
this.roleDescriptors = Collections.unmodifiableList(in.readList(RoleDescriptor::new));
this.refreshPolicy = WriteRequest.RefreshPolicy.readFrom(in);
@ -65,12 +65,8 @@ public final class CreateApiKeyRequest extends ActionRequest {
return name;
}
public void setName(String name) {
if (Strings.hasText(name)) {
this.name = name;
} else {
throw new IllegalArgumentException("name must not be null or empty");
}
public void setName(@Nullable String name) {
this.name = name;
}
public TimeValue getExpiration() {
@ -100,9 +96,7 @@ public final class CreateApiKeyRequest extends ActionRequest {
@Override
public ActionRequestValidationException validate() {
ActionRequestValidationException validationException = null;
if (Strings.isNullOrEmpty(name)) {
validationException = addValidationError("name is required", validationException);
} else {
if (name != null) {
if (name.length() > 256) {
validationException = addValidationError("name may not be more than 256 characters long", validationException);
}
@ -119,7 +113,11 @@ public final class CreateApiKeyRequest extends ActionRequest {
@Override
public void writeTo(StreamOutput out) throws IOException {
super.writeTo(out);
out.writeString(name);
if (out.getVersion().onOrAfter(Version.V_7_5_0)) {
out.writeOptionalString(name);
} else {
out.writeString(name);
}
out.writeOptionalTimeValue(expiration);
out.writeList(roleDescriptors);
refreshPolicy.writeTo(out);

10
x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/CreateApiKeyRequestTests.java
View File

@ -29,20 +29,12 @@ public class CreateApiKeyRequestTests extends ESTestCase {
CreateApiKeyRequest request = new CreateApiKeyRequest();
ActionRequestValidationException ve = request.validate();
assertNotNull(ve);
assertThat(ve.validationErrors().size(), is(1));
assertThat(ve.validationErrors().get(0), containsString("name is required"));
assertNull(ve);
request.setName(name);
ve = request.validate();
assertNull(ve);
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> request.setName(""));
assertThat(e.getMessage(), containsString("name must not be null or empty"));
e = expectThrows(IllegalArgumentException.class, () -> request.setName(null));
assertThat(e.getMessage(), containsString("name must not be null or empty"));
request.setName(randomAlphaOfLength(257));
ve = request.validate();
assertNotNull(ve);

11
x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyResponseTests.java
View File

@ -24,8 +24,9 @@ import static org.hamcrest.Matchers.equalTo;
public class GetApiKeyResponseTests extends ESTestCase {
public void testSerialization() throws IOException {
boolean withApiKeyName = randomBoolean();
boolean withExpiration = randomBoolean();
ApiKey apiKeyInfo = createApiKeyInfo(randomAlphaOfLength(4), randomAlphaOfLength(5), Instant.now(),
ApiKey apiKeyInfo = createApiKeyInfo((withApiKeyName) ? randomAlphaOfLength(4) : null, randomAlphaOfLength(5), Instant.now(),
(withExpiration) ? Instant.now() : null, false, randomAlphaOfLength(4), randomAlphaOfLength(5));
GetApiKeyResponse response = new GetApiKeyResponse(Collections.singletonList(apiKeyInfo));
try (BytesStreamOutput output = new BytesStreamOutput()) {
@ -42,7 +43,9 @@ public class GetApiKeyResponseTests extends ESTestCase {
"user-a", "realm-x");
ApiKey apiKeyInfo2 = createApiKeyInfo("name2", "id-2", Instant.ofEpochMilli(100000L), Instant.ofEpochMilli(10000000L), true,
"user-b", "realm-y");
GetApiKeyResponse response = new GetApiKeyResponse(Arrays.asList(apiKeyInfo1, apiKeyInfo2));
ApiKey apiKeyInfo3 = createApiKeyInfo(null, "id-3", Instant.ofEpochMilli(100000L), null, true,
"user-c", "realm-z");
GetApiKeyResponse response = new GetApiKeyResponse(Arrays.asList(apiKeyInfo1, apiKeyInfo2, apiKeyInfo3));
XContentBuilder builder = XContentFactory.jsonBuilder();
response.toXContent(builder, ToXContent.EMPTY_PARAMS);
assertThat(Strings.toString(builder), equalTo(
@ -51,7 +54,9 @@ public class GetApiKeyResponseTests extends ESTestCase {
+ "{\"id\":\"id-1\",\"name\":\"name1\",\"creation\":100000,\"expiration\":10000000,\"invalidated\":false,"
+ "\"username\":\"user-a\",\"realm\":\"realm-x\"},"
+ "{\"id\":\"id-2\",\"name\":\"name2\",\"creation\":100000,\"expiration\":10000000,\"invalidated\":true,"
+ "\"username\":\"user-b\",\"realm\":\"realm-y\"}"
+ "\"username\":\"user-b\",\"realm\":\"realm-y\"},"
+ "{\"id\":\"id-3\",\"name\":null,\"creation\":100000,\"invalidated\":true,"
+ "\"username\":\"user-c\",\"realm\":\"realm-z\"}"
+ "]"
+ "}"));
}

39
x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java
View File

@ -22,7 +22,6 @@ import org.elasticsearch.action.get.GetRequest;
import org.elasticsearch.action.get.GetResponse;
import org.elasticsearch.action.index.IndexAction;
import org.elasticsearch.action.index.IndexRequest;
import org.elasticsearch.action.search.SearchAction;
import org.elasticsearch.action.search.SearchRequest;
import org.elasticsearch.action.support.WriteRequest.RefreshPolicy;
import org.elasticsearch.action.update.UpdateRequest;
@ -194,46 +193,10 @@ public class ApiKeyService {
if (authentication == null) {
listener.onFailure(new IllegalArgumentException("authentication must be provided"));
} else {
/*
* Check if requested API key name already exists to avoid duplicate key names,
* this check is best effort as there could be two nodes executing search and
* then index concurrently allowing a duplicate name.
*/
checkDuplicateApiKeyNameAndCreateApiKey(authentication, request, userRoles, listener);
createApiKeyAndIndexIt(authentication, request, userRoles, listener);
}
}
private void checkDuplicateApiKeyNameAndCreateApiKey(Authentication authentication, CreateApiKeyRequest request,
Set<RoleDescriptor> userRoles,
ActionListener<CreateApiKeyResponse> listener) {
final BoolQueryBuilder boolQuery = QueryBuilders.boolQuery()
.filter(QueryBuilders.termQuery("doc_type", "api_key"))
.filter(QueryBuilders.termQuery("name", request.getName()))
.filter(QueryBuilders.termQuery("api_key_invalidated", false));
final BoolQueryBuilder expiredQuery = QueryBuilders.boolQuery()
.should(QueryBuilders.rangeQuery("expiration_time").lte(Instant.now().toEpochMilli()))
.should(QueryBuilders.boolQuery().mustNot(QueryBuilders.existsQuery("expiration_time")));
boolQuery.filter(expiredQuery);
final SearchRequest searchRequest = client.prepareSearch(SECURITY_MAIN_ALIAS)
.setQuery(boolQuery)
.setVersion(false)
.setSize(1)
.request();
securityIndex.prepareIndexIfNeededThenExecute(listener::onFailure, () ->
executeAsyncWithOrigin(client, SECURITY_ORIGIN, SearchAction.INSTANCE, searchRequest,
ActionListener.wrap(
indexResponse -> {
if (indexResponse.getHits().getTotalHits().value > 0) {
listener.onFailure(traceLog("create api key", new ElasticsearchSecurityException(
"Error creating api key as api key with name [{}] already exists", request.getName())));
} else {
createApiKeyAndIndexIt(authentication, request, userRoles, listener);
}
},
listener::onFailure)));
}
private void createApiKeyAndIndexIt(Authentication authentication, CreateApiKeyRequest request, Set<RoleDescriptor> roleDescriptorSet,
ActionListener<CreateApiKeyResponse> listener) {
final Instant created = clock.instant();

40
x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java
View File

@ -172,45 +172,25 @@ public class ApiKeyIntegTests extends SecurityIntegTestCase {
assertThat(e.status(), is(RestStatus.FORBIDDEN));
}
public void testCreateApiKeyFailsWhenApiKeyWithSameNameAlreadyExists() throws InterruptedException, ExecutionException {
public void testMultipleApiKeysCanHaveSameName() {
String keyName = randomAlphaOfLength(5);
int noOfApiKeys = randomIntBetween(2, 5);
List<CreateApiKeyResponse> responses = new ArrayList<>();
{
final RoleDescriptor descriptor = new RoleDescriptor("role", new String[] { "monitor" }, null, null);
for (int i = 0; i < noOfApiKeys; i++) {
final RoleDescriptor descriptor = new RoleDescriptor("role", new String[]{"monitor"}, null, null);
Client client = client().filterWithHeader(Collections.singletonMap("Authorization", UsernamePasswordToken
.basicAuthHeaderValue(SecuritySettingsSource.TEST_SUPERUSER, SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING)));
.basicAuthHeaderValue(SecuritySettingsSource.TEST_SUPERUSER, SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING)));
SecurityClient securityClient = new SecurityClient(client);
final CreateApiKeyResponse response = securityClient.prepareCreateApiKey().setName(keyName).setExpiration(null)
.setRoleDescriptors(Collections.singletonList(descriptor)).get();
.setRoleDescriptors(Collections.singletonList(descriptor)).get();
assertNotNull(response.getId());
assertNotNull(response.getKey());
responses.add(response);
}
final RoleDescriptor descriptor = new RoleDescriptor("role", new String[] { "monitor" }, null, null);
Client client = client().filterWithHeader(Collections.singletonMap("Authorization",
UsernamePasswordToken.basicAuthHeaderValue(SecuritySettingsSource.TEST_SUPERUSER,
SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING)));
SecurityClient securityClient = new SecurityClient(client);
ElasticsearchSecurityException e = expectThrows(ElasticsearchSecurityException.class, () -> securityClient.prepareCreateApiKey()
.setName(keyName)
.setExpiration(TimeValue.timeValueHours(TimeUnit.DAYS.toHours(7L)))
.setRoleDescriptors(Collections.singletonList(descriptor))
.get());
assertThat(e.getMessage(), equalTo("Error creating api key as api key with name ["+keyName+"] already exists"));
// Now invalidate the API key
PlainActionFuture<InvalidateApiKeyResponse> listener = new PlainActionFuture<>();
securityClient.invalidateApiKey(InvalidateApiKeyRequest.usingApiKeyName(keyName, false), listener);
InvalidateApiKeyResponse invalidateResponse = listener.get();
verifyInvalidateResponse(1, responses, invalidateResponse);
// try to create API key with same name, should succeed now
CreateApiKeyResponse createResponse = securityClient.prepareCreateApiKey().setName(keyName)
.setExpiration(TimeValue.timeValueHours(TimeUnit.DAYS.toHours(7L)))
.setRoleDescriptors(Collections.singletonList(descriptor)).get();
assertNotNull(createResponse.getId());
assertNotNull(createResponse.getKey());
assertThat(responses.size(), is(noOfApiKeys));
for (int i = 0; i < noOfApiKeys; i++) {
assertThat(responses.get(i).getName(), is(keyName));
}
}
public void testInvalidateApiKeysForRealm() throws InterruptedException, ExecutionException {