User Profile - Add hint support to SuggestProfile API (#85890)

This PR adds both uids and labels hints to SuggestProfile API. It also
adds support for the data parameter in request body (in addition to URL
query parameter) and fuzzy search for name.
This commit is contained in:
Yang Wang 2022-04-20 15:08:27 +10:00 committed by GitHub
parent b2c9028384
commit 8d21fe9d9f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 723 additions and 49 deletions

View File

@ -0,0 +1,5 @@
pr: 85890
summary: User Profile - Add hint support to SuggestProfiles API
area: Security
type: enhancement
issues: []

View File

@ -7,12 +7,21 @@
package org.elasticsearch.xpack.core.security.action.profile;
import org.elasticsearch.ElasticsearchParseException;
import org.elasticsearch.action.ActionRequest;
import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.io.stream.Writeable;
import org.elasticsearch.core.Nullable;
import org.elasticsearch.core.Tuple;
import java.io.IOException;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
@ -26,11 +35,14 @@ public class SuggestProfilesRequest extends ActionRequest {
*/
private final String name;
private final int size;
@Nullable
private final Hint hint;
public SuggestProfilesRequest(Set<String> dataKeys, String name, int size) {
public SuggestProfilesRequest(Set<String> dataKeys, String name, int size, Hint hint) {
this.dataKeys = Objects.requireNonNull(dataKeys, "data parameter must not be null");
this.name = Objects.requireNonNull(name, "name must not be null");
this.size = size;
this.hint = hint;
}
public SuggestProfilesRequest(StreamInput in) throws IOException {
@ -38,6 +50,7 @@ public class SuggestProfilesRequest extends ActionRequest {
this.dataKeys = in.readSet(StreamInput::readString);
this.name = in.readOptionalString();
this.size = in.readVInt();
this.hint = in.readOptionalWriteable(Hint::new);
}
public Set<String> getDataKeys() {
@ -52,12 +65,17 @@ public class SuggestProfilesRequest extends ActionRequest {
return size;
}
public Hint getHint() {
return hint;
}
@Override
public void writeTo(StreamOutput out) throws IOException {
super.writeTo(out);
out.writeStringCollection(dataKeys);
out.writeOptionalString(name);
out.writeVInt(size);
out.writeOptionalWriteable(hint);
}
@Override
@ -66,6 +84,101 @@ public class SuggestProfilesRequest extends ActionRequest {
if (size < 0) {
validationException = addValidationError("[size] parameter cannot be negative but was [" + size + "]", validationException);
}
if (hint != null) {
validationException = hint.validate(validationException);
}
return validationException;
}
public static class Hint implements Writeable {
@Nullable
private final List<String> uids;
@Nullable
private final Map<String, List<String>> labels;
public Hint(List<String> uids, Map<String, Object> labelsInput) {
this.uids = uids == null ? null : List.copyOf(uids);
// Convert the data type. Business logic, e.g. single key, is enforced in request validation
if (labelsInput != null) {
final HashMap<String, List<String>> labels = new HashMap<>();
for (Map.Entry<String, Object> entry : labelsInput.entrySet()) {
final Object value = entry.getValue();
if (value instanceof String) {
labels.put(entry.getKey(), List.of((String) value));
} else if (value instanceof final List<?> listValue) {
final ArrayList<String> values = new ArrayList<>();
for (Object v : listValue) {
if (v instanceof final String stringValue) {
values.add(stringValue);
} else {
throw new ElasticsearchParseException("[labels] hint supports either string value or list of strings");
}
}
labels.put(entry.getKey(), List.copyOf(values));
} else {
throw new ElasticsearchParseException("[labels] hint supports either string or list of strings as its value");
}
}
this.labels = Map.copyOf(labels);
} else {
this.labels = null;
}
}
public Hint(StreamInput in) throws IOException {
this.uids = in.readStringList();
this.labels = in.readMapOfLists(StreamInput::readString, StreamInput::readString);
}
public List<String> getUids() {
return uids;
}
public Tuple<String, List<String>> getSingleLabel() {
if (labels == null) {
return null;
}
assert labels.size() == 1 : "labels hint support exactly one key";
final String labelKey = labels.keySet().iterator().next();
final List<String> labelValues = labels.get(labelKey);
assert false == labelValues.isEmpty() : "label values cannot be empty";
return new Tuple<>(labelKey, labelValues);
}
@Override
public void writeTo(StreamOutput out) throws IOException {
out.writeStringCollection(uids);
out.writeMapOfLists(labels, StreamOutput::writeString, StreamOutput::writeString);
}
private ActionRequestValidationException validate(ActionRequestValidationException validationException) {
if (uids == null && labels == null) {
return addValidationError("[hint] parameter cannot be empty", validationException);
}
if (uids != null && uids.isEmpty()) {
validationException = addValidationError("[uids] hint cannot be empty", validationException);
}
if (labels != null) {
if (labels.size() != 1) {
return addValidationError(
"[labels] hint supports a single key, got [" + Strings.collectionToCommaDelimitedString(labels.keySet()) + "]",
validationException
);
}
final String key = labels.keySet().iterator().next();
if (key.contains("*")) {
validationException = addValidationError("[labels] hint key cannot contain wildcard", validationException);
}
final List<String> values = labels.get(key);
if (values.isEmpty()) {
validationException = addValidationError("[labels] hint value cannot be empty", validationException);
}
}
return validationException;
}
}
}

View File

@ -0,0 +1,146 @@
/*
* 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.profile;
import org.elasticsearch.ElasticsearchParseException;
import org.elasticsearch.core.Tuple;
import org.elasticsearch.test.ESTestCase;
import java.util.List;
import java.util.Map;
import java.util.Set;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.nullValue;
public class SuggestProfilesRequestTests extends ESTestCase {
public void testValidation() {
final SuggestProfilesRequest request1 = new SuggestProfilesRequest(randomDataKeys(), randomName(), randomSize(), randomHint());
assertThat(request1.validate(), nullValue());
}
public void testValidationWillNotAllowNegativeSize() {
final SuggestProfilesRequest request1 = new SuggestProfilesRequest(
randomDataKeys(),
randomName(),
randomIntBetween(Integer.MIN_VALUE, -1),
randomHint()
);
assertThat(request1.validate().getMessage(), containsString("[size] parameter cannot be negative"));
}
public void testValidationWillNotAllowEmptyHints() {
final SuggestProfilesRequest request1 = new SuggestProfilesRequest(
randomDataKeys(),
randomName(),
randomSize(),
new SuggestProfilesRequest.Hint(null, null)
);
assertThat(request1.validate().getMessage(), containsString("[hint] parameter cannot be empty"));
final SuggestProfilesRequest request2 = new SuggestProfilesRequest(
randomDataKeys(),
randomName(),
randomSize(),
new SuggestProfilesRequest.Hint(List.of(), null)
);
assertThat(request2.validate().getMessage(), containsString("[uids] hint cannot be empty"));
}
public void testValidationLabels() {
final SuggestProfilesRequest request1 = new SuggestProfilesRequest(
randomDataKeys(),
randomName(),
randomSize(),
new SuggestProfilesRequest.Hint(
null,
randomFrom(Map.of(), randomMap(2, 5, () -> new Tuple<>(randomAlphaOfLength(20), randomAlphaOfLengthBetween(3, 8))))
)
);
assertThat(request1.validate().getMessage(), containsString("[labels] hint supports a single key"));
final SuggestProfilesRequest request2 = new SuggestProfilesRequest(
randomDataKeys(),
randomName(),
randomSize(),
new SuggestProfilesRequest.Hint(
null,
Map.of(randomFrom("*", "a*", "*b", "a*b"), randomList(1, 5, () -> randomAlphaOfLengthBetween(3, 8)))
)
);
assertThat(request2.validate().getMessage(), containsString("[labels] hint key cannot contain wildcard"));
final SuggestProfilesRequest request3 = new SuggestProfilesRequest(
randomDataKeys(),
randomName(),
randomSize(),
new SuggestProfilesRequest.Hint(null, Map.of(randomAlphaOfLength(5), List.of()))
);
assertThat(request3.validate().getMessage(), containsString("[labels] hint value cannot be empty"));
}
public void testErrorOnHintInstantiation() {
final ElasticsearchParseException e1 = expectThrows(
ElasticsearchParseException.class,
() -> new SuggestProfilesRequest.Hint(
null,
Map.of(randomAlphaOfLength(5), randomFrom(0, 42.0, randomBoolean(), Map.of(randomAlphaOfLength(5), randomAlphaOfLength(5))))
)
);
assertThat(e1.getMessage(), containsString("[labels] hint supports either string or list of strings as its value"));
final ElasticsearchParseException e2 = expectThrows(
ElasticsearchParseException.class,
() -> new SuggestProfilesRequest.Hint(null, Map.of(randomAlphaOfLength(5), List.of(0, randomAlphaOfLength(8))))
);
assertThat(e2.getMessage(), containsString("[labels] hint supports either string value or list of strings"));
}
private int randomSize() {
return randomIntBetween(0, Integer.MAX_VALUE);
}
private Set<String> randomDataKeys() {
return Set.copyOf(randomList(0, 5, () -> randomAlphaOfLengthBetween(3, 8)));
}
private String randomName() {
return randomAlphaOfLengthBetween(0, 8);
}
public static SuggestProfilesRequest.Hint randomHint() {
switch (randomIntBetween(0, 3)) {
case 0 -> {
return new SuggestProfilesRequest.Hint(randomList(1, 5, () -> randomAlphaOfLength(20)), null);
}
case 1 -> {
return new SuggestProfilesRequest.Hint(
null,
Map.of(
randomAlphaOfLengthBetween(3, 8),
randomFrom(randomAlphaOfLengthBetween(3, 8), randomList(1, 5, () -> randomAlphaOfLengthBetween(3, 8)))
)
);
}
case 2 -> {
return new SuggestProfilesRequest.Hint(
randomList(1, 5, () -> randomAlphaOfLength(20)),
Map.of(
randomAlphaOfLengthBetween(3, 8),
randomFrom(randomAlphaOfLengthBetween(3, 8), randomList(1, 5, () -> randomAlphaOfLengthBetween(3, 8)))
)
);
}
default -> {
return null;
}
}
}
}

View File

@ -140,22 +140,76 @@ public class ProfileIT extends ESRestTestCase {
assertThat(castToMap(profileMap1.get("data")), equalTo(Map.of("app1", Map.of("theme", "default"))));
}
public void testSearchProfile() throws IOException {
public void testSuggestProfile() throws IOException {
final Map<String, Object> activateProfileMap = doActivateProfile();
final String uid = (String) activateProfileMap.get("uid");
final Request searchProfilesRequest1 = new Request(randomFrom("GET", "POST"), "_security/profile/_suggest");
searchProfilesRequest1.setJsonEntity("""
final Request suggestProfilesRequest1 = new Request(randomFrom("GET", "POST"), "_security/profile/_suggest");
suggestProfilesRequest1.setJsonEntity("""
{
"name": "rac",
"size": 10
}""");
final Response searchProfilesResponse1 = adminClient().performRequest(searchProfilesRequest1);
assertOK(searchProfilesResponse1);
final Map<String, Object> searchProfileResponseMap1 = responseAsMap(searchProfilesResponse1);
assertThat(searchProfileResponseMap1, hasKey("took"));
assertThat(searchProfileResponseMap1.get("total"), equalTo(Map.of("value", 1, "relation", "eq")));
final Response suggestProfilesResponse1 = adminClient().performRequest(suggestProfilesRequest1);
assertOK(suggestProfilesResponse1);
final Map<String, Object> suggestProfileResponseMap1 = responseAsMap(suggestProfilesResponse1);
assertThat(suggestProfileResponseMap1, hasKey("took"));
assertThat(suggestProfileResponseMap1.get("total"), equalTo(Map.of("value", 1, "relation", "eq")));
@SuppressWarnings("unchecked")
final List<Map<String, Object>> users = (List<Map<String, Object>>) searchProfileResponseMap1.get("profiles");
final List<Map<String, Object>> users = (List<Map<String, Object>>) suggestProfileResponseMap1.get("profiles");
assertThat(users, hasSize(1));
assertThat(users.get(0).get("uid"), equalTo(uid));
}
// Purpose of this test is to ensure the hint field works in the REST layer, e.g. parsing correctly etc.
// It does not attempt to test whether the query works correctly once it reaches the transport and service layer
// (other than the behaviour that hint does not decide whether a record should be returned).
// More comprehensive tests for hint behaviours are performed in internal cluster test.
public void testSuggestProfileWithHint() throws IOException {
final Map<String, Object> activateProfileMap = doActivateProfile();
final String uid = (String) activateProfileMap.get("uid");
final Request suggestProfilesRequest1 = new Request(randomFrom("GET", "POST"), "_security/profile/_suggest");
final String payload;
switch (randomIntBetween(0, 2)) {
case 0 -> {
payload = """
{
"name": "rac",
"hint": {
"uids": ["%s"]
}
}
""".formatted("not-" + uid);
}
case 1 -> {
payload = """
{
"name": "rac",
"hint": {
"labels": {
"kibana.spaces": %s
}
}
}
""".formatted(randomBoolean() ? "\"demo\"" : "[\"demo\"]");
}
default -> {
payload = """
{
"name": "rac",
"hint": {
"uids": ["%s"],
"labels": {
"kibana.spaces": %s
}
}
}""".formatted("not-" + uid, randomBoolean() ? "\"demo\"" : "[\"demo\"]");
}
}
suggestProfilesRequest1.setJsonEntity(payload);
final Response suggestProfilesResponse1 = adminClient().performRequest(suggestProfilesRequest1);
final Map<String, Object> suggestProfileResponseMap1 = responseAsMap(suggestProfilesResponse1);
@SuppressWarnings("unchecked")
final List<Map<String, Object>> users = (List<Map<String, Object>>) suggestProfileResponseMap1.get("profiles");
assertThat(users, hasSize(1));
assertThat(users.get(0).get("uid"), equalTo(uid));
}

View File

@ -11,9 +11,12 @@ import org.apache.lucene.search.TotalHits;
import org.elasticsearch.action.admin.indices.get.GetIndexAction;
import org.elasticsearch.action.admin.indices.get.GetIndexRequest;
import org.elasticsearch.action.admin.indices.get.GetIndexResponse;
import org.elasticsearch.action.support.PlainActionFuture;
import org.elasticsearch.action.support.WriteRequest;
import org.elasticsearch.action.support.master.AcknowledgedResponse;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.index.engine.DocumentMissingException;
import org.elasticsearch.test.rest.ObjectPath;
import org.elasticsearch.xcontent.XContentType;
import org.elasticsearch.xpack.core.security.action.profile.GetProfileAction;
import org.elasticsearch.xpack.core.security.action.profile.GetProfileRequest;
@ -28,8 +31,13 @@ import org.elasticsearch.xpack.core.security.action.profile.UpdateProfileDataAct
import org.elasticsearch.xpack.core.security.action.profile.UpdateProfileDataRequest;
import org.elasticsearch.xpack.core.security.action.user.PutUserAction;
import org.elasticsearch.xpack.core.security.action.user.PutUserRequest;
import org.elasticsearch.xpack.core.security.authc.Authentication;
import org.elasticsearch.xpack.core.security.authc.AuthenticationTestHelper;
import org.elasticsearch.xpack.core.security.user.User;
import java.io.IOException;
import java.util.Arrays;
import java.util.Comparator;
import java.util.List;
import java.util.Map;
import java.util.Set;
@ -235,8 +243,9 @@ public class ProfileIntegTests extends AbstractProfileIntegTestCase {
);
}
public void testSuggestProfiles() {
final String nativeRacUserPasswordHash = new String(getFastStoredHashAlgoForTests().hash(NATIVE_RAC_USER_PASSWORD));
public void testSuggestProfilesWithName() {
final ProfileService profileService = getInstanceFromRandomNode(ProfileService.class);
final Map<String, String> users = Map.of(
"user_foo",
"Very Curious User Foo",
@ -248,14 +257,13 @@ public class ProfileIntegTests extends AbstractProfileIntegTestCase {
"Super Anxious Admin Qux"
);
users.forEach((key, value) -> {
final PutUserRequest putUserRequest1 = new PutUserRequest();
putUserRequest1.username(key);
putUserRequest1.fullName(value);
putUserRequest1.email(key.substring(5) + "email@example.org");
putUserRequest1.roles("rac_role");
putUserRequest1.passwordHash(nativeRacUserPasswordHash.toCharArray());
assertThat(client().execute(PutUserAction.INSTANCE, putUserRequest1).actionGet().created(), is(true));
doActivateProfile(key, NATIVE_RAC_USER_PASSWORD);
final Authentication authentication = AuthenticationTestHelper.builder()
.realm()
.user(new User(key, new String[] { "rac_role" }, value, key.substring(5) + "email@example.org", Map.of(), true))
.build();
final PlainActionFuture<Profile> future = new PlainActionFuture<>();
profileService.activateProfile(authentication, future);
future.actionGet();
});
final SuggestProfilesResponse.ProfileHit[] profiles1 = doSuggest("");
@ -284,6 +292,176 @@ public class ProfileIntegTests extends AbstractProfileIntegTestCase {
final SuggestProfilesResponse.ProfileHit[] profiles7 = doSuggest("example.org");
assertThat(extractUsernames(profiles7), equalTo(users.keySet()));
// Fuzzy match
final SuggestProfilesResponse.ProfileHit[] profiles8 = doSuggest("Kurious One");
assertThat(extractUsernames(profiles8), equalTo(Set.of("user_foo", "user_bar")));
}
public void testSuggestProfileWithData() {
final ProfileService profileService = getInstanceFromRandomNode(ProfileService.class);
final PlainActionFuture<Profile> future1 = new PlainActionFuture<>();
profileService.activateProfile(AuthenticationTestHelper.builder().realm().build(), future1);
final Profile profile = future1.actionGet();
final PlainActionFuture<AcknowledgedResponse> future2 = new PlainActionFuture<>();
profileService.updateProfileData(
new UpdateProfileDataRequest(
profile.uid(),
Map.of("spaces", "demo"),
Map.of("app1", Map.of("name", "app1", "status", "prod"), "app2", Map.of("name", "app2", "status", "dev")),
-1,
-1,
WriteRequest.RefreshPolicy.WAIT_UNTIL
),
future2
);
assertThat(future2.actionGet().isAcknowledged(), is(true));
// No application data is requested
final SuggestProfilesResponse.ProfileHit[] profileHits1 = doSuggest("");
assertThat(profileHits1.length, equalTo(1));
final Profile profileResult1 = profileHits1[0].profile();
assertThat(profileResult1.uid(), equalTo(profile.uid()));
assertThat(profileResult1.labels(), equalTo(Map.of("spaces", "demo")));
assertThat(profileResult1.applicationData(), anEmptyMap());
// Request a single key of application data
final SuggestProfilesResponse.ProfileHit[] profileHits2 = doSuggest("", Set.of("app1"));
assertThat(profileHits2.length, equalTo(1));
final Profile profileResult2 = profileHits2[0].profile();
assertThat(profileResult2.uid(), equalTo(profile.uid()));
assertThat(profileResult2.labels(), equalTo(Map.of("spaces", "demo")));
assertThat(profileResult2.applicationData(), equalTo(Map.of("app1", Map.of("name", "app1", "status", "prod"))));
// Request multiple keys
final SuggestProfilesResponse.ProfileHit[] profileHits3 = doSuggest(
"",
randomFrom(Set.of("*"), Set.of("app*"), Set.of("app1", "app2"))
);
assertThat(profileHits3.length, equalTo(1));
final Profile profileResult3 = profileHits3[0].profile();
assertThat(profileResult3.uid(), equalTo(profile.uid()));
assertThat(profileResult3.labels(), equalTo(Map.of("spaces", "demo")));
assertThat(
profileResult3.applicationData(),
equalTo(Map.of("app1", Map.of("name", "app1", "status", "prod"), "app2", Map.of("name", "app2", "status", "dev")))
);
}
public void testSuggestProfilesWithHint() throws IOException {
final ProfileService profileService = getInstanceFromRandomNode(ProfileService.class);
final List<String> spaces = List.of("space1", "space2", "space3", "space4", "*");
final List<Profile> profiles = spaces.stream().map(space -> {
final PlainActionFuture<Profile> future1 = new PlainActionFuture<>();
final String lastName = randomAlphaOfLengthBetween(3, 8);
final Authentication authentication = AuthenticationTestHelper.builder()
.realm()
.user(
new User(
"user_" + lastName,
new String[] { "rac_role" },
"User " + lastName,
"user_" + lastName + "@example.org",
Map.of(),
true
)
)
.build();
profileService.activateProfile(authentication, future1);
final Profile profile = future1.actionGet();
final PlainActionFuture<AcknowledgedResponse> future2 = new PlainActionFuture<>();
profileService.updateProfileData(
new UpdateProfileDataRequest(
profile.uid(),
Map.of("kibana", Map.of("space", space)),
Map.of(),
-1,
-1,
WriteRequest.RefreshPolicy.WAIT_UNTIL
),
future2
);
assertThat(future2.actionGet().isAcknowledged(), is(true));
final PlainActionFuture<Profile> future3 = new PlainActionFuture<>();
profileService.getProfile(profile.uid(), Set.of(), future3);
return future3.actionGet();
}).toList();
// Default order of last synchronized timestamp
final List<Profile> profileHits1 = Arrays.stream(doSuggest("")).map(SuggestProfilesResponse.ProfileHit::profile).toList();
assertThat(
profileHits1.stream().sorted(Comparator.comparingLong(Profile::lastSynchronized).reversed()).toList(),
equalTo(profileHits1)
);
// uid hint is a should clause which does not exclude records but ranks matching ones higher
final Profile hintedProfile2 = randomFrom(profiles);
final List<Profile> profileHits2 = Arrays.stream(
doSuggest(Set.of(), "user", new SuggestProfilesRequest.Hint(List.of(hintedProfile2.uid()), null))
).map(SuggestProfilesResponse.ProfileHit::profile).toList();
assertThat(profileHits2.size(), equalTo(5));
// First record has the matching uid
assertThat(profileHits2.get(0).uid(), equalTo(hintedProfile2.uid()));
// Rest follows order of last synced
assertThat(
profileHits2.stream().skip(1).sorted(Comparator.comparingLong(Profile::lastSynchronized).reversed()).toList(),
equalTo(profileHits2.subList(1, profileHits2.size()))
);
// labels hint is also a should clause which does not exclude records but ranks matching ones higher
final Profile hintedProfile3 = randomFrom(profiles);
final String hintedSpace3 = ObjectPath.evaluate(hintedProfile3.labels(), "kibana.space");
final List<Profile> profileHits3 = Arrays.stream(
doSuggest(Set.of(), "user", new SuggestProfilesRequest.Hint(null, Map.of("kibana.space", hintedSpace3)))
).map(SuggestProfilesResponse.ProfileHit::profile).toList();
assertThat(profileHits3.size(), equalTo(5));
// First record has the matching labels
assertThat(profileHits3.get(0).labels(), equalTo(Map.of("kibana", Map.of("space", hintedSpace3))));
assertThat(profileHits3.get(0).uid(), equalTo(hintedProfile3.uid()));
// Rest follows order of last synced
assertThat(
profileHits3.stream().skip(1).sorted(Comparator.comparingLong(Profile::lastSynchronized).reversed()).toList(),
equalTo(profileHits3.subList(1, profileHits3.size()))
);
// Both uid and labels hints
final List<Profile> hintedProfiles = randomSubsetOf(2, profiles);
final Profile hintedProfile4 = randomFrom(hintedProfiles);
final Object hintedSpace4 = ObjectPath.evaluate(hintedProfile4.labels(), "kibana.space");
final List<Profile> profileHits4 = Arrays.stream(
doSuggest(
Set.of(),
"user",
new SuggestProfilesRequest.Hint(hintedProfiles.stream().map(Profile::uid).toList(), Map.of("kibana.space", hintedSpace4))
)
).map(SuggestProfilesResponse.ProfileHit::profile).toList();
assertThat(profileHits4.size(), equalTo(5));
// First record has both matching uid and labels
assertThat(profileHits4.get(0).labels(), equalTo(Map.of("kibana", Map.of("space", hintedSpace4))));
assertThat(profileHits4.get(0).uid(), equalTo(hintedProfile4.uid()));
// Second record has only matching uid
assertThat(
profileHits4.get(1).uid(),
equalTo(hintedProfiles.stream().filter(p -> false == p.equals(hintedProfile4)).findFirst().orElseThrow().uid())
);
// Rest follows order of last synced
assertThat(
profileHits4.stream().skip(2).sorted(Comparator.comparingLong(Profile::lastSynchronized).reversed()).toList(),
equalTo(profileHits4.subList(2, profileHits4.size()))
);
// A record will not be included if name does not match even when it has matching hint
final Profile hintedProfile5 = randomFrom(profiles);
final List<Profile> profileHits5 = Arrays.stream(
doSuggest(
Set.of(),
hintedProfile5.user().fullName().substring(5),
new SuggestProfilesRequest.Hint(profiles.stream().map(Profile::uid).toList(), Map.of("kibana.space", spaces))
)
).map(SuggestProfilesResponse.ProfileHit::profile).toList();
assertThat(profileHits5.size(), equalTo(1));
assertThat(profileHits5.get(0).uid(), equalTo(hintedProfile5.uid()));
}
public void testProfileAPIsWhenIndexNotCreated() {
@ -375,8 +553,16 @@ public class ProfileIntegTests extends AbstractProfileIntegTestCase {
);
}
private SuggestProfilesResponse.ProfileHit[] doSuggest(String query) {
final SuggestProfilesRequest suggestProfilesRequest = new SuggestProfilesRequest(Set.of(), query, 10);
private SuggestProfilesResponse.ProfileHit[] doSuggest(String name) {
return doSuggest(name, Set.of());
}
private SuggestProfilesResponse.ProfileHit[] doSuggest(String name, Set<String> dataKeys) {
return doSuggest(dataKeys, name, null);
}
private SuggestProfilesResponse.ProfileHit[] doSuggest(Set<String> dataKeys, String name, SuggestProfilesRequest.Hint hint) {
final SuggestProfilesRequest suggestProfilesRequest = new SuggestProfilesRequest(dataKeys, name, 10, hint);
final SuggestProfilesResponse suggestProfilesResponse = client().execute(SuggestProfilesAction.INSTANCE, suggestProfilesRequest)
.actionGet();
assertThat(suggestProfilesResponse.getTotalHits().relation, is(TotalHits.Relation.EQUAL_TO));

View File

@ -34,9 +34,11 @@ import org.elasticsearch.client.internal.Client;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.Fuzziness;
import org.elasticsearch.common.xcontent.XContentHelper;
import org.elasticsearch.core.Nullable;
import org.elasticsearch.core.TimeValue;
import org.elasticsearch.core.Tuple;
import org.elasticsearch.index.engine.VersionConflictEngineException;
import org.elasticsearch.index.query.BoolQueryBuilder;
import org.elasticsearch.index.query.MultiMatchQueryBuilder;
@ -66,6 +68,7 @@ import java.time.Clock;
import java.time.Instant;
import java.util.Arrays;
import java.util.Iterator;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Optional;
@ -183,27 +186,7 @@ public class ProfileService {
new TotalHits(0, TotalHits.Relation.EQUAL_TO)
);
})).ifPresent(frozenProfileIndex -> {
final BoolQueryBuilder query = QueryBuilders.boolQuery().filter(QueryBuilders.termQuery("user_profile.enabled", true));
if (Strings.hasText(request.getName())) {
query.must(
QueryBuilders.multiMatchQuery(
request.getName(),
"user_profile.user.username",
"user_profile.user.username._2gram",
"user_profile.user.username._3gram",
"user_profile.user.full_name",
"user_profile.user.full_name._2gram",
"user_profile.user.full_name._3gram",
"user_profile.user.email"
).type(MultiMatchQueryBuilder.Type.BOOL_PREFIX)
);
}
final SearchRequest searchRequest = client.prepareSearch(SECURITY_PROFILE_ALIAS)
.setQuery(query)
.setSize(request.getSize())
.addSort("_score", SortOrder.DESC)
.addSort("user_profile.last_synchronized", SortOrder.DESC)
.request();
final SearchRequest searchRequest = buildSearchRequest(request);
frozenProfileIndex.checkIndexVersionThenExecute(
listener::onFailure,
@ -254,6 +237,46 @@ public class ProfileService {
doUpdate(buildUpdateRequest(uid, builder, refreshPolicy, -1, -1), listener.map(updateResponse -> AcknowledgedResponse.TRUE));
}
// package private for testing
SearchRequest buildSearchRequest(SuggestProfilesRequest request) {
final BoolQueryBuilder query = QueryBuilders.boolQuery().filter(QueryBuilders.termQuery("user_profile.enabled", true));
if (Strings.hasText(request.getName())) {
query.must(
QueryBuilders.multiMatchQuery(
request.getName(),
"user_profile.user.username",
"user_profile.user.username._2gram",
"user_profile.user.username._3gram",
"user_profile.user.full_name",
"user_profile.user.full_name._2gram",
"user_profile.user.full_name._3gram",
"user_profile.user.email"
).type(MultiMatchQueryBuilder.Type.BOOL_PREFIX).fuzziness(Fuzziness.AUTO)
);
}
final SuggestProfilesRequest.Hint hint = request.getHint();
if (hint != null) {
final List<String> hintedUids = hint.getUids();
if (hintedUids != null) {
assert false == hintedUids.isEmpty() : "uids hint cannot be empty";
query.should(QueryBuilders.termsQuery("user_profile.uid", hintedUids));
}
final Tuple<String, List<String>> label = hint.getSingleLabel();
if (label != null) {
final List<String> labelValues = label.v2();
query.should(QueryBuilders.termsQuery("user_profile.labels." + label.v1(), labelValues));
}
query.minimumShouldMatch(0);
}
return client.prepareSearch(SECURITY_PROFILE_ALIAS)
.setQuery(query)
.setSize(request.getSize())
.addSort("_score", SortOrder.DESC)
.addSort("user_profile.last_synchronized", SortOrder.DESC)
.request();
}
private void getVersionedDocument(String uid, ActionListener<VersionedDocument> listener) {
tryFreezeAndCheckIndex(listener).ifPresent(frozenProfileIndex -> {
final GetRequest getRequest = new GetRequest(SECURITY_PROFILE_ALIAS, uidToDocId(uid));

View File

@ -21,6 +21,7 @@ import org.elasticsearch.xpack.security.rest.action.SecurityBaseRestHandler;
import java.io.IOException;
import java.util.List;
import java.util.Map;
import java.util.Set;
import static org.elasticsearch.rest.RestRequest.Method.GET;
@ -31,12 +32,22 @@ public class RestSuggestProfilesAction extends SecurityBaseRestHandler {
static final ConstructingObjectParser<Payload, Void> PARSER = new ConstructingObjectParser<>(
"suggest_profile_request_payload",
a -> new Payload((String) a[0], (Integer) a[1])
a -> new Payload((String) a[0], (Integer) a[1], (PayloadHint) a[2], (String) a[3])
);
@SuppressWarnings("unchecked")
static final ConstructingObjectParser<PayloadHint, Void> HINT_PARSER = new ConstructingObjectParser<>(
"suggest_profile_request_payload_hint",
a -> new PayloadHint((List<String>) a[0], (Map<String, Object>) a[1])
);
static {
HINT_PARSER.declareStringArray(optionalConstructorArg(), new ParseField("uids"));
HINT_PARSER.declareObject(optionalConstructorArg(), (p, c) -> p.map(), new ParseField("labels"));
PARSER.declareString(optionalConstructorArg(), new ParseField("name"));
PARSER.declareInt(optionalConstructorArg(), new ParseField("size"));
PARSER.declareObject(optionalConstructorArg(), HINT_PARSER, new ParseField("hint"));
PARSER.declareString(optionalConstructorArg(), new ParseField("data"));
}
public RestSuggestProfilesAction(Settings settings, XPackLicenseState licenseState) {
@ -55,16 +66,28 @@ public class RestSuggestProfilesAction extends SecurityBaseRestHandler {
@Override
protected RestChannelConsumer innerPrepareRequest(RestRequest request, NodeClient client) throws IOException {
final Set<String> dataKeys = Strings.tokenizeByCommaToSet(request.param("data", null));
final Payload payload = request.hasContentOrSourceParam()
? PARSER.parse(request.contentOrSourceParamParser(), null)
: new Payload(null, null);
: new Payload(null, null, null, null);
final SuggestProfilesRequest suggestProfilesRequest = new SuggestProfilesRequest(dataKeys, payload.name(), payload.size());
final String data = request.param("data", null);
if (data != null && payload.data() != null) {
throw new IllegalArgumentException(
"The [data] parameter must be specified in either request body or as query parameter, but not both"
);
}
final Set<String> dataKeys = Strings.tokenizeByCommaToSet(data != null ? data : payload.data());
final SuggestProfilesRequest suggestProfilesRequest = new SuggestProfilesRequest(
dataKeys,
payload.name(),
payload.size(),
payload.hint() == null ? null : new SuggestProfilesRequest.Hint(payload.hint().uids(), payload.hint().labels())
);
return channel -> client.execute(SuggestProfilesAction.INSTANCE, suggestProfilesRequest, new RestToXContentListener<>(channel));
}
record Payload(String name, Integer size) {
record Payload(String name, Integer size, PayloadHint hint, String data) {
public String name() {
return name != null ? name : "";
@ -74,4 +97,7 @@ public class RestSuggestProfilesAction extends SecurityBaseRestHandler {
return size != null ? size : 10;
}
}
record PayloadHint(List<String> uids, Map<String, Object> labels) {}
}

View File

@ -12,15 +12,31 @@ import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.get.GetAction;
import org.elasticsearch.action.get.GetRequest;
import org.elasticsearch.action.get.GetResponse;
import org.elasticsearch.action.search.SearchAction;
import org.elasticsearch.action.search.SearchRequest;
import org.elasticsearch.action.search.SearchRequestBuilder;
import org.elasticsearch.action.support.PlainActionFuture;
import org.elasticsearch.client.internal.Client;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.Fuzziness;
import org.elasticsearch.core.Tuple;
import org.elasticsearch.index.query.BoolQueryBuilder;
import org.elasticsearch.index.query.MultiMatchQueryBuilder;
import org.elasticsearch.index.query.QueryBuilder;
import org.elasticsearch.index.query.QueryBuilders;
import org.elasticsearch.search.builder.SearchSourceBuilder;
import org.elasticsearch.search.sort.FieldSortBuilder;
import org.elasticsearch.search.sort.ScoreSortBuilder;
import org.elasticsearch.search.sort.SortOrder;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.threadpool.FixedExecutorBuilder;
import org.elasticsearch.threadpool.TestThreadPool;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.xpack.core.security.action.profile.Profile;
import org.elasticsearch.xpack.core.security.action.profile.SuggestProfilesRequest;
import org.elasticsearch.xpack.core.security.action.profile.SuggestProfilesRequestTests;
import org.elasticsearch.xpack.core.security.authc.Authentication;
import org.elasticsearch.xpack.core.security.authc.AuthenticationTests;
import org.elasticsearch.xpack.core.security.authc.Subject;
@ -38,6 +54,7 @@ import org.mockito.Mockito;
import java.io.IOException;
import java.time.Clock;
import java.time.Instant;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@ -47,7 +64,9 @@ import static org.elasticsearch.test.ActionListenerUtils.anyActionListener;
import static org.elasticsearch.xpack.security.Security.SECURITY_CRYPTO_THREAD_POOL_NAME;
import static org.elasticsearch.xpack.security.support.SecuritySystemIndices.SECURITY_PROFILE_ALIAS;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.instanceOf;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doAnswer;
@ -114,6 +133,9 @@ public class ProfileServiceTests extends ESTestCase {
);
this.client = mock(Client.class);
when(client.threadPool()).thenReturn(threadPool);
when(client.prepareSearch(SECURITY_PROFILE_ALIAS)).thenReturn(
new SearchRequestBuilder(client, SearchAction.INSTANCE).setIndices(SECURITY_PROFILE_ALIAS)
);
this.profileIndex = SecurityMocks.mockSecurityIndexManager(SECURITY_PROFILE_ALIAS);
this.profileService = new ProfileService(Settings.EMPTY, Clock.systemUTC(), client, profileIndex, threadPool);
}
@ -228,6 +250,62 @@ public class ProfileServiceTests extends ESTestCase {
assertThat(e3.getMessage(), containsString("differentiator is not a number"));
}
public void testBuildSearchRequest() {
final String name = randomAlphaOfLengthBetween(0, 8);
final int size = randomIntBetween(0, Integer.MAX_VALUE);
final SuggestProfilesRequest.Hint hint = SuggestProfilesRequestTests.randomHint();
final SuggestProfilesRequest suggestProfilesRequest = new SuggestProfilesRequest(Set.of(), name, size, hint);
final SearchRequest searchRequest = profileService.buildSearchRequest(suggestProfilesRequest);
final SearchSourceBuilder searchSourceBuilder = searchRequest.source();
assertThat(
searchSourceBuilder.sorts(),
equalTo(List.of(new ScoreSortBuilder(), new FieldSortBuilder("user_profile.last_synchronized").order(SortOrder.DESC)))
);
assertThat(searchSourceBuilder.size(), equalTo(size));
assertThat(searchSourceBuilder.query(), instanceOf(BoolQueryBuilder.class));
final BoolQueryBuilder query = (BoolQueryBuilder) searchSourceBuilder.query();
if (Strings.hasText(name)) {
assertThat(
query.must(),
equalTo(
List.of(
QueryBuilders.multiMatchQuery(
name,
"user_profile.user.username",
"user_profile.user.username._2gram",
"user_profile.user.username._3gram",
"user_profile.user.full_name",
"user_profile.user.full_name._2gram",
"user_profile.user.full_name._3gram",
"user_profile.user.email"
).type(MultiMatchQueryBuilder.Type.BOOL_PREFIX).fuzziness(Fuzziness.AUTO)
)
)
);
} else {
assertThat(query.must(), empty());
}
if (hint != null) {
final List<QueryBuilder> shouldQueries = new ArrayList<>(query.should());
if (hint.getUids() != null) {
assertThat(shouldQueries.remove(0), equalTo(QueryBuilders.termsQuery("user_profile.uid", hint.getUids())));
}
final Tuple<String, List<String>> label = hint.getSingleLabel();
if (label != null) {
final List<String> labelValues = label.v2();
assertThat(shouldQueries.remove(0), equalTo(QueryBuilders.termsQuery("user_profile.labels." + label.v1(), labelValues)));
}
assertThat(query.minimumShouldMatch(), equalTo("0"));
} else {
assertThat(query.should(), empty());
}
}
private void mockGetRequest(String uid, long lastSynchronized) {
final String source = SAMPLE_PROFILE_DOCUMENT_TEMPLATE.formatted(uid, lastSynchronized);

View File

@ -7,12 +7,14 @@
package org.elasticsearch.xpack.security.rest.action.profile;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.license.XPackLicenseState;
import org.elasticsearch.rest.RestRequest;
import org.elasticsearch.test.rest.FakeRestRequest;
import org.elasticsearch.test.rest.RestActionTestCase;
import org.elasticsearch.xcontent.NamedXContentRegistry;
import org.elasticsearch.xcontent.XContentType;
import org.elasticsearch.xpack.core.XPackSettings;
import org.elasticsearch.xpack.core.security.action.profile.SuggestProfilesRequest;
import org.elasticsearch.xpack.core.security.action.profile.SuggestProfilesResponse;
@ -20,8 +22,10 @@ import org.junit.Before;
import java.util.HashMap;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.atomic.AtomicReference;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.instanceOf;
import static org.mockito.Mockito.mock;
@ -29,6 +33,7 @@ import static org.mockito.Mockito.mock;
public class RestSuggestProfilesActionTests extends RestActionTestCase {
private Settings settings;
private XPackLicenseState licenseState;
private RestSuggestProfilesAction restSuggestProfilesAction;
private AtomicReference<SuggestProfilesRequest> requestHolder;
@Before
@ -36,7 +41,8 @@ public class RestSuggestProfilesActionTests extends RestActionTestCase {
settings = Settings.builder().put(XPackSettings.SECURITY_ENABLED.getKey(), true).build();
licenseState = mock(XPackLicenseState.class);
requestHolder = new AtomicReference<>();
controller().registerHandler(new RestSuggestProfilesAction(settings, licenseState));
restSuggestProfilesAction = new RestSuggestProfilesAction(settings, licenseState);
controller().registerHandler(restSuggestProfilesAction);
verifyingClient.setExecuteVerifier(((actionType, actionRequest) -> {
assertThat(actionRequest, instanceOf(SuggestProfilesRequest.class));
requestHolder.set((SuggestProfilesRequest) actionRequest);
@ -58,4 +64,41 @@ public class RestSuggestProfilesActionTests extends RestActionTestCase {
final SuggestProfilesRequest suggestProfilesRequest = requestHolder.get();
assertThat(suggestProfilesRequest.getName(), equalTo(expectedName));
}
public void testParsingDataParameter() {
final FakeRestRequest.Builder builder = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY).withMethod(
randomFrom(RestRequest.Method.GET, RestRequest.Method.POST)
).withPath("/_security/profile/_suggest");
if (randomBoolean()) {
builder.withParams(new HashMap<>(Map.of("data", "app1,app2,other*,app5")));
} else {
builder.withContent(new BytesArray("{\"data\": \"app1,app2,other*,app5\"}"), XContentType.JSON);
}
final FakeRestRequest restRequest = builder.build();
dispatchRequest(restRequest);
final SuggestProfilesRequest suggestProfilesRequest = requestHolder.get();
assertThat(suggestProfilesRequest.getDataKeys(), equalTo(Set.of("app1", "app2", "other*", "app5")));
}
public void testWillNotAllowDataInBothQueryParameterAndRequestBody() {
final FakeRestRequest restRequest = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY).withMethod(
randomFrom(RestRequest.Method.GET, RestRequest.Method.POST)
)
.withPath("/_security/profile/_suggest")
.withParams(new HashMap<>(Map.of("data", randomAlphaOfLengthBetween(3, 8))))
.withContent(new BytesArray("{\"data\": \"%s\"}".formatted(randomAlphaOfLengthBetween(3, 8))), XContentType.JSON)
.build();
final IllegalArgumentException e = expectThrows(
IllegalArgumentException.class,
() -> restSuggestProfilesAction.innerPrepareRequest(restRequest, verifyingClient)
);
assertThat(
e.getMessage(),
containsString("The [data] parameter must be specified in either request body or as query parameter, but not both")
);
}
}