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

Add support for the type parameter to the Query API Key API #103695

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
23c2053
Runtime field support
albertzaharovits Dec 22, 2023
1db1806
Merge branch 'main' into query-api-key-type-runtime-field
albertzaharovits Dec 27, 2023
c84b841
WIP meh
albertzaharovits Dec 27, 2023
ba575e1
WIP
albertzaharovits Dec 27, 2023
6b3d7b6
Painless
albertzaharovits Dec 27, 2023
c465cde
Okish test
albertzaharovits Dec 27, 2023
aace531
An IT
albertzaharovits Dec 27, 2023
a1c7d61
Merge branch 'main' into query-api-key-type-runtime-field
albertzaharovits Dec 28, 2023
e4d64b1
Nits
albertzaharovits Dec 28, 2023
d13e199
WTF
albertzaharovits Dec 28, 2023
e8dba56
IT is polished now
albertzaharovits Dec 28, 2023
8ae29f2
Remove IT nit fixture
albertzaharovits Dec 28, 2023
7d666ad
Don't try to deserialize non-rest keys
albertzaharovits Dec 28, 2023
19a1e72
ApiKeyBackwardsCompatibilityIT
albertzaharovits Dec 28, 2023
2673e61
ApiKeyBackwardsCompatibilityIT
albertzaharovits Dec 28, 2023
6dc9189
Merge branch 'main' into query-api-key-type-runtime-field
albertzaharovits Dec 28, 2023
05653c5
only API keys created pre-8.9 are relevant for the rest-type query bw…
albertzaharovits Dec 29, 2023
859fa4b
testQueryCrossClusterApiKeysByType
albertzaharovits Dec 29, 2023
38458f1
Merge branch 'main' into query-api-key-type-runtime-field
albertzaharovits Dec 29, 2023
5ed7840
ApiKeyBoolQueryBuilderTests
albertzaharovits Dec 29, 2023
3dfe94a
javadoc and docs
albertzaharovits Dec 29, 2023
20727d1
Merge branch 'main' into query-api-key-type-runtime-field
albertzaharovits Jan 10, 2024
c64b574
Update x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/s…
albertzaharovits Jan 10, 2024
fb69b9e
Test nit
albertzaharovits Jan 10, 2024
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
5 changes: 5 additions & 0 deletions docs/reference/rest-api/security/query-api-key.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,11 @@ You can query the following public values associated with an API key.
`id`::
ID of the API key. Note `id` must be queried with the <<query-dsl-ids-query,`ids`>> query.

`type`::
API keys can be of type `rest`, if created via the <<security-api-create-api-key, Create API key>> or
the <<security-api-grant-api-key, Grant API key>> APIs, or of type `cross_cluster` if created via
the <<security-api-create-cross-cluster-api-key, Create Cross-Cluster API key>> API.

`name`::
Name of the API key.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@

import org.apache.http.HttpHeaders;
import org.elasticsearch.client.Request;
import org.elasticsearch.client.RequestOptions;
import org.elasticsearch.client.Response;
import org.elasticsearch.client.ResponseException;
import org.elasticsearch.common.settings.SecureString;
import org.elasticsearch.core.Strings;
import org.elasticsearch.core.Tuple;
import org.elasticsearch.test.XContentTestUtils;
Expand All @@ -21,6 +23,7 @@
import java.time.Instant;
import java.util.ArrayList;
import java.util.Base64;
import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.Set;
Expand All @@ -43,6 +46,8 @@ public class QueryApiKeyIT extends SecurityInBasicRestTestCase {
private static final String API_KEY_ADMIN_AUTH_HEADER = "Basic YXBpX2tleV9hZG1pbjpzZWN1cml0eS10ZXN0LXBhc3N3b3Jk";
private static final String API_KEY_USER_AUTH_HEADER = "Basic YXBpX2tleV91c2VyOnNlY3VyaXR5LXRlc3QtcGFzc3dvcmQ=";
private static final String TEST_USER_AUTH_HEADER = "Basic c2VjdXJpdHlfdGVzdF91c2VyOnNlY3VyaXR5LXRlc3QtcGFzc3dvcmQ=";
private static final String SYSTEM_WRITE_ROLE_NAME = "system_write";
private static final String SUPERUSER_WITH_SYSTEM_WRITE = "superuser_with_system_write";

public void testQuery() throws IOException {
createApiKeys();
Expand Down Expand Up @@ -297,6 +302,71 @@ public void testPagination() throws IOException, InterruptedException {
assertThat(responseMap2.get("count"), equalTo(0));
}

public void testTypeField() throws Exception {
final List<String> allApiKeyIds = new ArrayList<>(7);
for (int i = 0; i < 7; i++) {
allApiKeyIds.add(
createApiKey("typed_key_" + i, Map.of(), randomFrom(API_KEY_ADMIN_AUTH_HEADER, API_KEY_USER_AUTH_HEADER)).v1()
);
}
List<String> apiKeyIdsSubset = randomSubsetOf(allApiKeyIds);
List<String> apiKeyIdsSubsetDifference = new ArrayList<>(allApiKeyIds);
apiKeyIdsSubsetDifference.removeAll(apiKeyIdsSubset);

List<String> apiKeyRestTypeQueries = List.of("""
{"query": {"term": {"type": "rest" }}}""", """
Copy link
Contributor

Choose a reason for hiding this comment

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

Optional, but could also add a NOT cross_cluster query here since that's probably a query end-users might try.

{"query": {"bool": {"must_not": [{"term": {"type": "cross_cluster"}}, {"term": {"type": "other"}}]}}}""", """
{"query": {"prefix": {"type": "re" }}}""", """
{"query": {"wildcard": {"type": "r*t" }}}""", """
{"query": {"range": {"type": {"gte": "raaa", "lte": "rzzz"}}}}""");

for (String query : apiKeyRestTypeQueries) {
assertQuery(API_KEY_ADMIN_AUTH_HEADER, query, apiKeys -> {
assertThat(
apiKeys.stream().map(k -> (String) k.get("id")).toList(),
containsInAnyOrder(allApiKeyIds.toArray(new String[0]))
);
});
}

createSystemWriteRole(SYSTEM_WRITE_ROLE_NAME);
String systemWriteCreds = createUser(SUPERUSER_WITH_SYSTEM_WRITE, new String[] { "superuser", SYSTEM_WRITE_ROLE_NAME });

// test keys with no "type" field are still considered of type "rest"
// this is so in order to accommodate pre-8.9 API keys which where all of type "rest" implicitly
updateApiKeys(systemWriteCreds, "ctx._source.remove('type');", apiKeyIdsSubset);
for (String query : apiKeyRestTypeQueries) {
assertQuery(API_KEY_ADMIN_AUTH_HEADER, query, apiKeys -> {
assertThat(
apiKeys.stream().map(k -> (String) k.get("id")).toList(),
containsInAnyOrder(allApiKeyIds.toArray(new String[0]))
);
});
}

// but the same keys with type "other" are NOT of type "rest"
updateApiKeys(systemWriteCreds, "ctx._source['type']='other';", apiKeyIdsSubset);
for (String query : apiKeyRestTypeQueries) {
assertQuery(API_KEY_ADMIN_AUTH_HEADER, query, apiKeys -> {
assertThat(
apiKeys.stream().map(k -> (String) k.get("id")).toList(),
containsInAnyOrder(apiKeyIdsSubsetDifference.toArray(new String[0]))
);
});
}
// the complement set is not of type "rest" if it is "cross_cluster"
updateApiKeys(systemWriteCreds, "ctx._source['type']='rest';", apiKeyIdsSubset);
updateApiKeys(systemWriteCreds, "ctx._source['type']='cross_cluster';", apiKeyIdsSubsetDifference);
for (String query : apiKeyRestTypeQueries) {
assertQuery(API_KEY_ADMIN_AUTH_HEADER, query, apiKeys -> {
assertThat(
apiKeys.stream().map(k -> (String) k.get("id")).toList(),
containsInAnyOrder(apiKeyIdsSubset.toArray(new String[0]))
);
});
}
}

@SuppressWarnings("unchecked")
public void testSort() throws IOException {
final String authHeader = randomFrom(API_KEY_ADMIN_AUTH_HEADER, API_KEY_USER_AUTH_HEADER);
Expand Down Expand Up @@ -598,10 +668,73 @@ private String createAndInvalidateApiKey(String name, String authHeader) throws
return tuple.v1();
}

private void createUser(String name) throws IOException {
final Request request = new Request("POST", "/_security/user/" + name);
request.setJsonEntity("""
{"password":"super-strong-password","roles":[]}""");
assertOK(adminClient().performRequest(request));
private String createUser(String username) throws IOException {
return createUser(username, new String[0]);
}

private String createUser(String username, String[] roles) throws IOException {
final Request request = new Request("POST", "/_security/user/" + username);
Map<String, Object> body = Map.ofEntries(Map.entry("roles", roles), Map.entry("password", "super-strong-password".toString()));
request.setJsonEntity(XContentTestUtils.convertToXContent(body, XContentType.JSON).utf8ToString());
Response response = adminClient().performRequest(request);
assertOK(response);
return basicAuthHeaderValue(username, new SecureString("super-strong-password".toCharArray()));
}

private void createSystemWriteRole(String roleName) throws IOException {
final Request addRole = new Request("POST", "/_security/role/" + roleName);
addRole.setJsonEntity("""
{
"indices": [
{
"names": [ "*" ],
"privileges": ["all"],
"allow_restricted_indices" : true
}
]
}""");
Response response = adminClient().performRequest(addRole);
assertOK(response);
}

private void expectWarnings(Request request, String... expectedWarnings) {
final Set<String> expected = Set.of(expectedWarnings);
RequestOptions options = request.getOptions().toBuilder().setWarningsHandler(warnings -> {
final Set<String> actual = Set.copyOf(warnings);
// Return true if the warnings aren't what we expected; the client will treat them as a fatal error.
return actual.equals(expected) == false;
}).build();
request.setOptions(options);
}

private void updateApiKeys(String creds, String script, Collection<String> ids) throws IOException {
if (ids.isEmpty()) {
return;
}
final Request request = new Request("POST", "/.security/_update_by_query?refresh=true&wait_for_completion=true");
request.setJsonEntity(Strings.format("""
{
"script": {
"source": "%s",
"lang": "painless"
},
"query": {
"bool": {
"must": [
{"term": {"doc_type": "api_key"}},
{"ids": {"values": %s}}
]
}
}
}
""", script, ids.stream().map(id -> "\"" + id + "\"").collect(Collectors.toList())));
request.setOptions(request.getOptions().toBuilder().addHeader(HttpHeaders.AUTHORIZATION, creds));
expectWarnings(
request,
"this request accesses system indices: [.security-7],"
+ " but in a future major version, direct access to system indices will be prevented by default"
);
Response response = client().performRequest(request);
assertOK(response);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,10 @@
import java.io.IOException;
import java.time.Instant;
import java.time.temporal.ChronoUnit;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Set;
Expand All @@ -54,9 +56,11 @@
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.greaterThanOrEqualTo;
import static org.hamcrest.Matchers.hasEntry;
import static org.hamcrest.Matchers.hasItem;
import static org.hamcrest.Matchers.hasKey;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.iterableWithSize;
import static org.hamcrest.Matchers.lessThanOrEqualTo;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.notNullValue;
Expand Down Expand Up @@ -703,6 +707,73 @@ public void testRemoteIndicesSupportForApiKeys() throws IOException {

}

@SuppressWarnings("unchecked")
public void testQueryCrossClusterApiKeysByType() throws IOException {
final List<String> apiKeyIds = new ArrayList<>(3);
for (int i = 0; i < randomIntBetween(3, 5); i++) {
Request createRequest = new Request("POST", "/_security/cross_cluster/api_key");
createRequest.setJsonEntity(Strings.format("""
{
"name": "test-cross-key-query-%d",
"access": {
"search": [
{
"names": [ "whatever" ]
}
]
},
"metadata": { "tag": %d, "label": "rest" }
}""", i, i));
setUserForRequest(createRequest, MANAGE_SECURITY_USER, END_USER_PASSWORD);
ObjectPath createResponse = assertOKAndCreateObjectPath(client().performRequest(createRequest));
apiKeyIds.add(createResponse.evaluate("id"));
}
// the "cross_cluster" keys are not "rest" type
for (String restTypeQuery : List.of("""
{"query": {"term": {"type": "rest" }}}""", """
{"query": {"bool": {"must_not": {"term": {"type": "cross_cluster"}}}}}""", """
{"query": {"prefix": {"type": "re" }}}""", """
{"query": {"wildcard": {"type": "r*t" }}}""", """
{"query": {"range": {"type": {"gte": "raaa", "lte": "rzzz"}}}}""")) {
Request queryRequest = new Request("GET", "/_security/_query/api_key");
queryRequest.addParameter("with_limited_by", String.valueOf(randomBoolean()));
queryRequest.setJsonEntity(restTypeQuery);
setUserForRequest(queryRequest, MANAGE_API_KEY_USER, END_USER_PASSWORD);
ObjectPath queryResponse = assertOKAndCreateObjectPath(client().performRequest(queryRequest));
assertThat(queryResponse.evaluate("total"), is(0));
assertThat(queryResponse.evaluate("count"), is(0));
assertThat(queryResponse.evaluate("api_keys"), iterableWithSize(0));
}
for (String crossClusterTypeQuery : List.of("""
{"query": {"term": {"type": "cross_cluster" }}}""", """
{"query": {"bool": {"must_not": {"term": {"type": "rest"}}}}}""", """
{"query": {"prefix": {"type": "cro" }}}""", """
{"query": {"wildcard": {"type": "*oss_*er" }}}""", """
{"query": {"range": {"type": {"gte": "cross", "lte": "zzzz"}}}}""")) {
Request queryRequest = new Request("GET", "/_security/_query/api_key");
queryRequest.addParameter("with_limited_by", String.valueOf(randomBoolean()));
queryRequest.setJsonEntity(crossClusterTypeQuery);
setUserForRequest(queryRequest, MANAGE_API_KEY_USER, END_USER_PASSWORD);
ObjectPath queryResponse = assertOKAndCreateObjectPath(client().performRequest(queryRequest));
assertThat(queryResponse.evaluate("total"), is(apiKeyIds.size()));
assertThat(queryResponse.evaluate("count"), is(apiKeyIds.size()));
assertThat(queryResponse.evaluate("api_keys"), iterableWithSize(apiKeyIds.size()));
Iterator<?> apiKeys = ((List<?>) queryResponse.evaluate("api_keys")).iterator();
while (apiKeys.hasNext()) {
assertThat(apiKeyIds, hasItem((String) ((Map<String, Object>) apiKeys.next()).get("id")));
}
}
final Request queryRequest = new Request("GET", "/_security/_query/api_key");
queryRequest.addParameter("with_limited_by", String.valueOf(randomBoolean()));
queryRequest.setJsonEntity("""
{"query": {"bool": {"must": [{"term": {"type": "cross_cluster" }}, {"term": {"metadata.tag": 2}}]}}}""");
setUserForRequest(queryRequest, MANAGE_API_KEY_USER, END_USER_PASSWORD);
final ObjectPath queryResponse = assertOKAndCreateObjectPath(client().performRequest(queryRequest));
assertThat(queryResponse.evaluate("total"), is(1));
assertThat(queryResponse.evaluate("count"), is(1));
assertThat(queryResponse.evaluate("api_keys.0.name"), is("test-cross-key-query-2"));
}

public void testCreateCrossClusterApiKey() throws IOException {
final Request createRequest = new Request("POST", "/_security/cross_cluster/api_key");
createRequest.setJsonEntity("""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,26 @@
import org.elasticsearch.xpack.security.support.ApiKeyFieldNameTranslators;

import java.util.List;
import java.util.Map;
import java.util.concurrent.atomic.AtomicBoolean;

import static org.elasticsearch.xpack.security.support.SecuritySystemIndices.SECURITY_MAIN_ALIAS;

public final class TransportQueryApiKeyAction extends HandledTransportAction<QueryApiKeyRequest, QueryApiKeyResponse> {

// API keys with no "type" field are implicitly of type "rest" (this is the case for all API Keys created before v8.9).
// The below runtime field ensures that the "type" field can be used by the {@link RestQueryApiKeyAction},
// while making the implicit "rest" type feature transparent to the caller (hence all keys are either "rest"
// or "cross_cluster", and the "type" is always set).
// This can be improved, to get rid of the runtime performance impact of the runtime field, by reindexing
// the api key docs and setting the "type" to "rest" if empty. But the infrastructure to run such a maintenance
// task on a system index (once the cluster version permits) is not currently available.
public static final String API_KEY_TYPE_RUNTIME_MAPPING_FIELD = "runtime_key_type";
private static final Map<String, Object> API_KEY_TYPE_RUNTIME_MAPPING = Map.of(
API_KEY_TYPE_RUNTIME_MAPPING_FIELD,
Map.of("type", "keyword", "script", Map.of("source", "emit(field('type').get(\"rest\"));"))
);

private final ApiKeyService apiKeyService;
private final SecurityContext securityContext;

Expand Down Expand Up @@ -66,12 +81,19 @@ protected void doExecute(Task task, QueryApiKeyRequest request, ActionListener<Q
searchSourceBuilder.size(request.getSize());
}

final ApiKeyBoolQueryBuilder apiKeyBoolQueryBuilder = ApiKeyBoolQueryBuilder.build(
request.getQueryBuilder(),
request.isFilterForCurrentUser() ? authentication : null
);
final AtomicBoolean accessesApiKeyTypeField = new AtomicBoolean(false);
final ApiKeyBoolQueryBuilder apiKeyBoolQueryBuilder = ApiKeyBoolQueryBuilder.build(request.getQueryBuilder(), fieldName -> {
if (API_KEY_TYPE_RUNTIME_MAPPING_FIELD.equals(fieldName)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Wondering if we can skip the boolean ref here and do this directly, in the lambda:

// only add the query-level runtime field to the search request if it's actually referring the "type" field
if (API_KEY_TYPE_RUNTIME_MAPPING_FIELD.equals(fieldName)) {
    searchSourceBuilder.runtimeMappings(API_KEY_TYPE_RUNTIME_MAPPING);
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah no that wouldn't work because we need this to happen only once

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well... I don't think it needs to happen once, searchSourceBuilder.runtimeMappings(API_KEY_TYPE_RUNTIME_MAPPING) can be invoked multiple times, but I would still prefer the current implementation.

accessesApiKeyTypeField.set(true);
}
}, request.isFilterForCurrentUser() ? authentication : null);
searchSourceBuilder.query(apiKeyBoolQueryBuilder);

// only add the query-level runtime field to the search request if it's actually referring the "type" field
if (accessesApiKeyTypeField.get()) {
searchSourceBuilder.runtimeMappings(API_KEY_TYPE_RUNTIME_MAPPING);
}

if (request.getFieldSortBuilders() != null) {
translateFieldSortBuilders(request.getFieldSortBuilders(), searchSourceBuilder);
}
Expand Down
Loading