Skip to content

Enforce license for cross-cluster API key APIs #96307

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

Merged
ywangd merged 4 commits into from
May 25, 2023
Merged

Enforce license for cross-cluster API key APIs #96307

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
abf8d77
Enforce license for cross-cluster API key APIs
ywangd May 23, 2023
af9a003
revert accidental change
ywangd May 24, 2023
2c1c120
Merge remote-tracking branch 'origin/main' into rcs-speicali-api-key-…
ywangd May 24, 2023
c777310
fix test
ywangd May 24, 2023
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
4 changes: 1 addition & 3 deletions ...lasticsearch/xpack/remotecluster/RemoteClusterSecurityLicensingAndFeatureUsageRestIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public class RemoteClusterSecurityLicensingAndFeatureUsageRestIT extends Abstrac
.name("fulfilling-cluster")
.nodes(1)
.apply(commonClusterConfig)
.setting("xpack.license.self_generated.type", "basic")
.setting("xpack.license.self_generated.type", "trial")
Copy link
Copy Markdown
Member Author

@ywangd ywangd May 24, 2023

Choose a reason for hiding this comment

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

Need to change the initial license for FC to be trial. Otherwise the whole test suite fails because it cannot create the cross-cluster API key. Fortunately, this change does not really impact the essence of what we are trying to test here.

.setting("remote_cluster_server.enabled", "true")
.setting("remote_cluster.port", "0")
.setting("xpack.security.remote_cluster_server.ssl.enabled", "true")
Expand Down Expand Up @@ -113,7 +113,6 @@ protected void configureRemoteCluster(boolean isProxyMode) throws Exception {
}

public void testCrossClusterAccessFeatureTrackingAndLicensing() throws Exception {
assertBasicLicense(fulfillingClusterClient);
assertBasicLicense(client());

final boolean useProxyMode = randomBoolean();
Expand Down Expand Up @@ -167,7 +166,6 @@ public void testCrossClusterAccessFeatureTrackingAndLicensing() throws Exception
assertRequestFailsDueToUnsupportedLicense(() -> performRequestWithRemoteSearchUser(searchRequest));

// We start the trial license which supports all features.
startTrialLicense(fulfillingClusterClient);
startTrialLicense(client());

// Check that feature is not tracked before we send CCS request.
Expand Down
53 changes: 53 additions & 0 deletions ...ic/src/javaRestTest/java/org/elasticsearch/xpack/security/SecurityWithBasicLicenseIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ public void testWithBasicLicense() throws Exception {

assertUserProfileFeatures(false);
checkRemoteIndicesXPackUsage();
assertFailToCreateAndUpdateCrossClusterApiKeys();
}

public void testWithTrialLicense() throws Exception {
Expand Down Expand Up @@ -80,6 +81,7 @@ public void testWithTrialLicense() throws Exception {
assertReadWithApiKey(apiKeyCredentials2, "/index*/_search", true);
assertUserProfileFeatures(true);
checkRemoteIndicesXPackUsage();
assertSuccessToCreateAndUpdateCrossClusterApiKeys();
} finally {
revertTrial();
assertAuthenticateWithToken(accessToken, false);
Expand All @@ -95,6 +97,7 @@ public void testWithTrialLicense() throws Exception {
assertReadWithApiKey(apiKeyCredentials2, "/index1/_doc/1", false);
assertUserProfileFeatures(false);
checkRemoteIndicesXPackUsage();
assertFailToCreateAndUpdateCrossClusterApiKeys();
}
}

Expand Down Expand Up @@ -567,4 +570,54 @@ private void assertUserProfileFeatures(boolean clusterHasTrialLicense) throws IO
assertThat(e.getMessage(), containsString("current license is non-compliant for [user-profile-collaboration]"));
}
}

private void assertFailToCreateAndUpdateCrossClusterApiKeys() {
if (false == TcpTransport.isUntrustedRemoteClusterEnabled()) {
return;
}

final Request createRequest = new Request("POST", "/_security/cross_cluster/api_key");
createRequest.setJsonEntity("""
{
"name": "cc-key",
"access": {
"search": [ { "names": ["*"] } ]
}
}""");
final ResponseException e1 = expectThrows(ResponseException.class, () -> adminClient().performRequest(createRequest));
assertThat(e1.getResponse().getStatusLine().getStatusCode(), equalTo(403));
assertThat(e1.getMessage(), containsString("current license is non-compliant for [advanced-remote-cluster-security]"));

final Request updateRequest = new Request("PUT", "/_security/cross_cluster/api_key/" + randomAlphaOfLength(20));
updateRequest.setJsonEntity("""
{
"metadata": { }
}""");
final ResponseException e2 = expectThrows(ResponseException.class, () -> adminClient().performRequest(updateRequest));
assertThat(e2.getResponse().getStatusLine().getStatusCode(), equalTo(403));
assertThat(e2.getMessage(), containsString("current license is non-compliant for [advanced-remote-cluster-security]"));
}

private void assertSuccessToCreateAndUpdateCrossClusterApiKeys() throws IOException {
if (false == TcpTransport.isUntrustedRemoteClusterEnabled()) {
return;
}

final Request createRequest = new Request("POST", "/_security/cross_cluster/api_key");
createRequest.setJsonEntity("""
{
"name": "cc-key",
"access": {
"search": [ { "names": ["*"] } ]
}
}""");
final ObjectPath createResponse = assertOKAndCreateObjectPath(adminClient().performRequest(createRequest));

final Request updateRequest = new Request("PUT", "/_security/cross_cluster/api_key/" + createResponse.evaluate("id"));
updateRequest.setJsonEntity("""
{
"metadata": { }
}""");
assertOK(adminClient().performRequest(updateRequest));
}
}
14 changes: 14 additions & 0 deletions ...g/elasticsearch/xpack/security/rest/action/apikey/RestCreateCrossClusterApiKeyAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import org.elasticsearch.client.internal.node.NodeClient;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.core.TimeValue;
import org.elasticsearch.license.LicenseUtils;
import org.elasticsearch.license.XPackLicenseState;
import org.elasticsearch.rest.RestRequest;
import org.elasticsearch.rest.action.RestToXContentListener;
Expand All @@ -26,6 +27,7 @@
import static org.elasticsearch.rest.RestRequest.Method.POST;
import static org.elasticsearch.xcontent.ConstructingObjectParser.constructorArg;
import static org.elasticsearch.xcontent.ConstructingObjectParser.optionalConstructorArg;
import static org.elasticsearch.xpack.security.Security.ADVANCED_REMOTE_CLUSTER_SECURITY_FEATURE;

/**
* Rest action to create an API key specific to cross cluster access via the dedicate remote cluster server port
Expand Down Expand Up @@ -79,4 +81,16 @@ protected RestChannelConsumer innerPrepareRequest(final RestRequest request, fin
new RestToXContentListener<>(channel)
);
}

@Override
protected Exception checkFeatureAvailable(RestRequest request) {
final Exception failedFeature = super.checkFeatureAvailable(request);
if (failedFeature != null) {
return failedFeature;
} else if (ADVANCED_REMOTE_CLUSTER_SECURITY_FEATURE.checkWithoutTracking(licenseState)) {
Copy link
Copy Markdown
Member Author

@ywangd ywangd May 24, 2023

Choose a reason for hiding this comment

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

I went back and forth with whether this should check or checkWithoutTracking. At the end, I went with checkWithoutTracking because it is more consistent to track actual usage of cross-cluster access. Otherwise, the stats can be skewed by just playing with these APIs.

return null;
} else {
return LicenseUtils.newComplianceException(ADVANCED_REMOTE_CLUSTER_SECURITY_FEATURE.getName());
}
}
Comment on lines +85 to +95
Copy link
Copy Markdown
Member Author

@ywangd ywangd May 24, 2023

Choose a reason for hiding this comment

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

This method is duplicated in RestUpdateCrossClusterApiKeyAction. If we get to add more REST APIs for cross-cluster API keys, we can extract a superclass to have this method in one place. For now I am keeping it this way. Please let me know if you think otherwise.

}
14 changes: 14 additions & 0 deletions ...g/elasticsearch/xpack/security/rest/action/apikey/RestUpdateCrossClusterApiKeyAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

import org.elasticsearch.client.internal.node.NodeClient;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.license.LicenseUtils;
import org.elasticsearch.license.XPackLicenseState;
import org.elasticsearch.rest.RestRequest;
import org.elasticsearch.rest.action.RestToXContentListener;
Expand All @@ -24,6 +25,7 @@

import static org.elasticsearch.rest.RestRequest.Method.PUT;
import static org.elasticsearch.xcontent.ConstructingObjectParser.optionalConstructorArg;
import static org.elasticsearch.xpack.security.Security.ADVANCED_REMOTE_CLUSTER_SECURITY_FEATURE;

public final class RestUpdateCrossClusterApiKeyAction extends ApiKeyBaseRestHandler {

Expand Down Expand Up @@ -64,5 +66,17 @@ protected RestChannelConsumer innerPrepareRequest(final RestRequest request, fin
);
}

@Override
protected Exception checkFeatureAvailable(RestRequest request) {
final Exception failedFeature = super.checkFeatureAvailable(request);
if (failedFeature != null) {
return failedFeature;
} else if (ADVANCED_REMOTE_CLUSTER_SECURITY_FEATURE.checkWithoutTracking(licenseState)) {
return null;
} else {
return LicenseUtils.newComplianceException(ADVANCED_REMOTE_CLUSTER_SECURITY_FEATURE.getName());
}
}

record Payload(CrossClusterApiKeyRoleDescriptorBuilder builder, Map<String, Object> metadata) {}
}
55 changes: 53 additions & 2 deletions ...sticsearch/xpack/security/rest/action/apikey/RestCreateCrossClusterApiKeyActionTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,14 @@

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

import org.apache.lucene.util.SetOnce;
import org.elasticsearch.client.internal.node.NodeClient;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.license.XPackLicenseState;
import org.elasticsearch.license.MockLicenseState;
import org.elasticsearch.rest.AbstractRestChannel;
import org.elasticsearch.rest.RestChannel;
import org.elasticsearch.rest.RestResponse;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.rest.FakeRestRequest;
import org.elasticsearch.xcontent.NamedXContentRegistry;
Expand All @@ -20,20 +23,34 @@
import org.elasticsearch.xpack.core.security.action.apikey.CreateCrossClusterApiKeyAction;
import org.elasticsearch.xpack.core.security.action.apikey.CreateCrossClusterApiKeyRequest;
import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
import org.elasticsearch.xpack.security.Security;
import org.mockito.ArgumentCaptor;

import java.util.List;

import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.nullValue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

public class RestCreateCrossClusterApiKeyActionTests extends ESTestCase {

private MockLicenseState licenseState;
private RestCreateCrossClusterApiKeyAction action;

@Override
public void setUp() throws Exception {
super.setUp();
licenseState = MockLicenseState.createMock();
when(licenseState.isAllowed(Security.ADVANCED_REMOTE_CLUSTER_SECURITY_FEATURE)).thenReturn(true);
action = new RestCreateCrossClusterApiKeyAction(Settings.EMPTY, licenseState);
}

public void testCreateApiKeyRequestHasTypeOfCrossCluster() throws Exception {
final FakeRestRequest restRequest = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY).withContent(new BytesArray("""
{
Expand All @@ -49,7 +66,6 @@ public void testCreateApiKeyRequestHasTypeOfCrossCluster() throws Exception {
}
}"""), XContentType.JSON).build();

final var action = new RestCreateCrossClusterApiKeyAction(Settings.EMPTY, mock(XPackLicenseState.class));
final NodeClient client = mock(NodeClient.class);
action.handleRequest(restRequest, mock(RestChannel.class), client);

Expand Down Expand Up @@ -80,4 +96,39 @@ public void testCreateApiKeyRequestHasTypeOfCrossCluster() throws Exception {
);
assertThat(request.getMetadata(), nullValue());
}

public void testLicenseEnforcement() throws Exception {
// Disallow by license
when(licenseState.isAllowed(Security.ADVANCED_REMOTE_CLUSTER_SECURITY_FEATURE)).thenReturn(false);

final FakeRestRequest restRequest = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY).withContent(new BytesArray("""
{
"name": "my-key",
"access": {
"search": [
{
"names": [
"logs"
]
}
]
}
}"""), XContentType.JSON).build();
final SetOnce<RestResponse> responseSetOnce = new SetOnce<>();
final RestChannel restChannel = new AbstractRestChannel(restRequest, randomBoolean()) {
@Override
public void sendResponse(RestResponse restResponse) {
responseSetOnce.set(restResponse);
}
};

action.handleRequest(restRequest, restChannel, mock(NodeClient.class));

final RestResponse restResponse = responseSetOnce.get();
assertThat(restResponse.status().getStatus(), equalTo(403));
assertThat(
restResponse.content().utf8ToString(),
containsString("current license is non-compliant for [advanced-remote-cluster-security]")
);
}
}
49 changes: 47 additions & 2 deletions ...sticsearch/xpack/security/rest/action/apikey/RestUpdateCrossClusterApiKeyActionTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,15 @@

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

import org.apache.lucene.util.SetOnce;
import org.elasticsearch.client.internal.node.NodeClient;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.license.XPackLicenseState;
import org.elasticsearch.license.MockLicenseState;
import org.elasticsearch.rest.AbstractRestChannel;
import org.elasticsearch.rest.RestChannel;
import org.elasticsearch.rest.RestResponse;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.rest.FakeRestRequest;
import org.elasticsearch.xcontent.NamedXContentRegistry;
Expand All @@ -21,22 +24,36 @@
import org.elasticsearch.xpack.core.security.action.apikey.CrossClusterApiKeyRoleDescriptorBuilder;
import org.elasticsearch.xpack.core.security.action.apikey.UpdateCrossClusterApiKeyAction;
import org.elasticsearch.xpack.core.security.action.apikey.UpdateCrossClusterApiKeyRequest;
import org.elasticsearch.xpack.security.Security;
import org.mockito.ArgumentCaptor;

import java.util.List;
import java.util.Map;

import static org.elasticsearch.xpack.core.security.action.apikey.CreateCrossClusterApiKeyRequestTests.randomCrossClusterApiKeyAccessField;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.nullValue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

public class RestUpdateCrossClusterApiKeyActionTests extends ESTestCase {

private MockLicenseState licenseState;
private RestUpdateCrossClusterApiKeyAction action;

@Override
public void setUp() throws Exception {
super.setUp();
licenseState = MockLicenseState.createMock();
when(licenseState.isAllowed(Security.ADVANCED_REMOTE_CLUSTER_SECURITY_FEATURE)).thenReturn(true);
action = new RestUpdateCrossClusterApiKeyAction(Settings.EMPTY, licenseState);
}

public void testUpdateHasTypeOfCrossCluster() throws Exception {
final String id = randomAlphaOfLength(10);
final String access = randomCrossClusterApiKeyAccessField();
Expand All @@ -49,7 +66,6 @@ public void testUpdateHasTypeOfCrossCluster() throws Exception {
XContentType.JSON
).withParams(Map.of("id", id)).build();

final var action = new RestUpdateCrossClusterApiKeyAction(Settings.EMPTY, mock(XPackLicenseState.class));
final NodeClient client = mock(NodeClient.class);
action.handleRequest(restRequest, mock(RestChannel.class), client);

Expand All @@ -68,4 +84,33 @@ public void testUpdateHasTypeOfCrossCluster() throws Exception {
assertThat(request.getMetadata(), nullValue());
}
}

public void testLicenseEnforcement() throws Exception {
// Disallow by license
when(licenseState.isAllowed(Security.ADVANCED_REMOTE_CLUSTER_SECURITY_FEATURE)).thenReturn(false);

final FakeRestRequest restRequest = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY).withContent(
new BytesArray("""
{
"metadata": {}
}"""),
XContentType.JSON
).withParams(Map.of("id", randomAlphaOfLength(10))).build();
final SetOnce<RestResponse> responseSetOnce = new SetOnce<>();
final RestChannel restChannel = new AbstractRestChannel(restRequest, randomBoolean()) {
@Override
public void sendResponse(RestResponse restResponse) {
responseSetOnce.set(restResponse);
}
};

action.handleRequest(restRequest, restChannel, mock(NodeClient.class));

final RestResponse restResponse = responseSetOnce.get();
assertThat(restResponse.status().getStatus(), equalTo(403));
assertThat(
restResponse.content().utf8ToString(),
containsString("current license is non-compliant for [advanced-remote-cluster-security]")
);
}
}