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.

Sign up for GitHub

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

API Key id part of transport request body #63221

Merged
merged 5 commits into from
Oct 5, 2020
Merged

API Key id part of transport request body #63221

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5ea139c
Done
albertzaharovits Oct 4, 2020
323792f
Merge branch 'master' into create_api_key_id_part_of_request
albertzaharovits Oct 5, 2020
a4e50cd
Review
albertzaharovits Oct 5, 2020
8230565
Merge branch 'master' into create_api_key_id_part_of_request
elasticmachine Oct 5, 2020
e9760b1
Update x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/s…
albertzaharovits Oct 5, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 23 additions & 1 deletion .../core/src/main/java/org/elasticsearch/xpack/core/security/action/CreateApiKeyRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.elasticsearch.action.support.WriteRequest;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.UUIDs;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.unit.TimeValue;
Expand All @@ -31,12 +32,16 @@
public final class CreateApiKeyRequest extends ActionRequest {
public static final WriteRequest.RefreshPolicy DEFAULT_REFRESH_POLICY = WriteRequest.RefreshPolicy.WAIT_UNTIL;

private final String id;
private String name;
private TimeValue expiration;
private List<RoleDescriptor> roleDescriptors = Collections.emptyList();
private WriteRequest.RefreshPolicy refreshPolicy = DEFAULT_REFRESH_POLICY;

public CreateApiKeyRequest() {}
public CreateApiKeyRequest() {
this.id = UUIDs.base64UUID(); // because auditing can currently only catch requests but not responses,
// we generate the API key id soonest so it's part of the request body so it is audited
}

/**
* Create API Key request constructor
Expand All @@ -45,13 +50,19 @@ public CreateApiKeyRequest() {}
* @param expiration to specify expiration for the API key
*/
public CreateApiKeyRequest(String name, @Nullable List<RoleDescriptor> roleDescriptors, @Nullable TimeValue expiration) {
this();
this.name = name;
this.roleDescriptors = (roleDescriptors == null) ? List.of() : List.copyOf(roleDescriptors);
this.expiration = expiration;
}

public CreateApiKeyRequest(StreamInput in) throws IOException {
super(in);
if (in.getVersion().onOrAfter(Version.V_7_10_0)) {
this.id = in.readString();
} else {
this.id = UUIDs.base64UUID();
}
if (in.getVersion().onOrAfter(Version.V_7_5_0)) {
this.name = in.readOptionalString();
} else {
Expand All @@ -62,6 +73,14 @@ public CreateApiKeyRequest(StreamInput in) throws IOException {
this.refreshPolicy = WriteRequest.RefreshPolicy.readFrom(in);
}

public String getId() {
return id;
}

public void setId() {
throw new UnsupportedOperationException("The API Key Id cannot be set, it must be generated randomly");
}

public String getName() {
return name;
}
Expand Down Expand Up @@ -116,6 +135,9 @@ public ActionRequestValidationException validate() {
@Override
public void writeTo(StreamOutput out) throws IOException {
super.writeTo(out);
if (out.getVersion().onOrAfter(Version.V_7_10_0)) {
out.writeString(id);
}
if (out.getVersion().onOrAfter(Version.V_7_5_0)) {
out.writeOptionalString(name);
} else {
Expand Down
12 changes: 12 additions & 0 deletions .../src/test/java/org/elasticsearch/xpack/core/security/action/CreateApiKeyRequestTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

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

import org.elasticsearch.Version;
import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.action.support.WriteRequest;
import org.elasticsearch.common.io.stream.BytesStreamOutput;
Expand Down Expand Up @@ -96,11 +97,22 @@ public void testSerialization() throws IOException {
}
request.setRoleDescriptors(descriptorList);

boolean testV710Bwc = randomBoolean();

try (BytesStreamOutput out = new BytesStreamOutput()) {
if (testV710Bwc) {
out.setVersion(Version.V_7_9_0); // a version before 7.10
}
request.writeTo(out);
try (StreamInput in = out.bytes().streamInput()) {
if (testV710Bwc) {
in.setVersion(Version.V_7_9_0);
}
final CreateApiKeyRequest serialized = new CreateApiKeyRequest(in);
assertEquals(name, serialized.getName());
if (false == testV710Bwc) {
assertEquals(request.getId(), serialized.getId()); // API key id is only preserved after v 7.10
}
assertEquals(expiration, serialized.getExpiration());
assertEquals(refreshPolicy, serialized.getRefreshPolicy());
if (nullOrEmptyRoleDescriptors) {
Expand Down
8 changes: 6 additions & 2 deletions ...k/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,15 +232,19 @@ private void createApiKeyAndIndexIt(Authentication authentication, CreateApiKeyR
final IndexRequest indexRequest =
client.prepareIndex(SECURITY_MAIN_ALIAS)
.setSource(builder)
.setId(request.getId())
.setRefreshPolicy(request.getRefreshPolicy())
.request();
final BulkRequest bulkRequest = toSingleItemBulkRequest(indexRequest);

securityIndex.prepareIndexIfNeededThenExecute(listener::onFailure, () ->
executeAsyncWithOrigin(client, SECURITY_ORIGIN, BulkAction.INSTANCE, bulkRequest,
TransportSingleItemBulkWriteAction.<IndexResponse>wrapBulkResponse(ActionListener.wrap(
indexResponse -> listener.onResponse(
new CreateApiKeyResponse(request.getName(), indexResponse.getId(), apiKey, expiration)),
indexResponse -> {
assert indexResponse.getId() == request.getId();
listener.onResponse(
new CreateApiKeyResponse(request.getName(), request.getId(), apiKey, expiration));
},
listener::onFailure))));
} catch (IOException e) {
listener.onFailure(e);
Expand Down
14 changes: 11 additions & 3 deletions ...gin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.elasticsearch.action.bulk.BulkAction;
import org.elasticsearch.action.bulk.BulkRequest;
import org.elasticsearch.action.index.IndexAction;
import org.elasticsearch.action.index.IndexRequest;
import org.elasticsearch.action.index.IndexRequestBuilder;
import org.elasticsearch.action.support.PlainActionFuture;
import org.elasticsearch.client.Client;
Expand Down Expand Up @@ -104,7 +105,6 @@
import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

public class ApiKeyServiceTests extends ESTestCase {
Expand Down Expand Up @@ -138,7 +138,7 @@ public void setupMocks() {
this.securityIndex = SecurityMocks.mockSecurityIndexManager();
}

public void testCreateApiKeyWillUseBulkAction() {
public void testCreateApiKeyUsesBulkIndexAction() {
final Settings settings = Settings.builder().put(XPackSettings.API_KEY_SERVICE_ENABLED_SETTING.getKey(), true).build();
final ApiKeyService service = createApiKeyService(settings);
final Authentication authentication = new Authentication(
Expand All @@ -148,8 +148,16 @@ public void testCreateApiKeyWillUseBulkAction() {
final CreateApiKeyRequest createApiKeyRequest = new CreateApiKeyRequest("key-1", null, null);
when(client.prepareIndex(anyString())).thenReturn(new IndexRequestBuilder(client, IndexAction.INSTANCE));
when(client.threadPool()).thenReturn(threadPool);
doAnswer(inv -> {
final Object[] args = inv.getArguments();
BulkRequest bulkRequest = (BulkRequest) args[1];
assertThat(bulkRequest.numberOfActions(), is(1));
assertThat(bulkRequest.requests().get(0), instanceOf(IndexRequest.class));
IndexRequest indexRequest = (IndexRequest) bulkRequest.requests().get(0);
assertThat(indexRequest.id(), is(createApiKeyRequest.getId()));
return null;
}).when(client).execute(eq(BulkAction.INSTANCE), any(BulkRequest.class), any());
service.createApiKey(authentication, createApiKeyRequest, Set.of(), new PlainActionFuture<>());
verify(client).execute(eq(BulkAction.INSTANCE), any(BulkRequest.class), any());
}

public void testGetCredentialsFromThreadContext() {
Expand Down