Просмотр исходного кода

Coerce blank fields to null in ApiKey requests (#66240)

API Key request objects (`InvalidateApiKeyRequest` and
`GetApiKeyRequest`) support multiple key-selection parameters such as
realm-name, user-name, key-id and key-name.

The specified behaviour is that if any of these are _blank_ then they
act as a wildcard and do not restrict the search criteria.

This change moves the "is blank" logic into the constructor for these
requests so that there is a single consistent way to determine blank
(`Strings.hasText(arg) == false`) and all usage of these fields can
rely on the getter returning either `null` or a _real value_, and
never a non-null blank.

Relates: #62916
Tim Vernum 4 лет назад
Родитель
Сommit
fcf28c3197

+ 12 - 8
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequest.java

@@ -36,10 +36,10 @@ public final class GetApiKeyRequest extends ActionRequest {
 
     public GetApiKeyRequest(StreamInput in) throws IOException {
         super(in);
-        realmName = in.readOptionalString();
-        userName = in.readOptionalString();
-        apiKeyId = in.readOptionalString();
-        apiKeyName = in.readOptionalString();
+        realmName = textOrNull(in.readOptionalString());
+        userName = textOrNull(in.readOptionalString());
+        apiKeyId = textOrNull(in.readOptionalString());
+        apiKeyName = textOrNull(in.readOptionalString());
         if (in.getVersion().onOrAfter(Version.V_7_4_0)) {
             ownedByAuthenticatedUser = in.readOptionalBoolean();
         } else {
@@ -49,13 +49,17 @@ public final class GetApiKeyRequest extends ActionRequest {
 
     public GetApiKeyRequest(@Nullable String realmName, @Nullable String userName, @Nullable String apiKeyId,
                             @Nullable String apiKeyName, boolean ownedByAuthenticatedUser) {
-        this.realmName = realmName;
-        this.userName = userName;
-        this.apiKeyId = apiKeyId;
-        this.apiKeyName = apiKeyName;
+        this.realmName = textOrNull(realmName);
+        this.userName = textOrNull(userName);
+        this.apiKeyId = textOrNull(apiKeyId);
+        this.apiKeyName = textOrNull(apiKeyName);
         this.ownedByAuthenticatedUser = ownedByAuthenticatedUser;
     }
 
+    private static String textOrNull(@Nullable String arg) {
+        return Strings.hasText(arg) ? arg : null;
+    }
+
     public String getRealmName() {
         return realmName;
     }

+ 12 - 8
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/InvalidateApiKeyRequest.java

@@ -38,15 +38,15 @@ public final class InvalidateApiKeyRequest extends ActionRequest {
 
     public InvalidateApiKeyRequest(StreamInput in) throws IOException {
         super(in);
-        realmName = in.readOptionalString();
-        userName = in.readOptionalString();
+        realmName = textOrNull(in.readOptionalString());
+        userName = textOrNull(in.readOptionalString());
         if (in.getVersion().onOrAfter(Version.V_7_10_0)) {
             ids = in.readOptionalStringArray();
         } else {
             final String id = in.readOptionalString();
             ids = Strings.hasText(id) == false ? null : new String[] { id };
         }
-        name = in.readOptionalString();
+        name = textOrNull(in.readOptionalString());
         if (in.getVersion().onOrAfter(Version.V_7_4_0)) {
             ownedByAuthenticatedUser = in.readOptionalBoolean();
         } else {
@@ -65,17 +65,21 @@ public final class InvalidateApiKeyRequest extends ActionRequest {
             throw new IllegalArgumentException("Must use either [id] or [ids], not both at the same time");
         }
 
-        this.realmName = realmName;
-        this.userName = userName;
-        if (id != null) {
-            this.ids = new String[] {id};
+        this.realmName = textOrNull(realmName);
+        this.userName = textOrNull(userName);
+        if (Strings.hasText(id)) {
+            this.ids = new String[]{id};
         } else {
             this.ids = ids;
         }
-        this.name = name;
+        this.name = textOrNull(name);
         this.ownedByAuthenticatedUser = ownedByAuthenticatedUser;
     }
 
+    private static String textOrNull(@Nullable String arg) {
+        return Strings.hasText(arg) ? arg : null;
+    }
+
     public String getRealmName() {
         return realmName;
     }

+ 17 - 0
x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java

@@ -17,11 +17,13 @@ import org.elasticsearch.test.ESTestCase;
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
+import java.util.function.Supplier;
 
 import static org.elasticsearch.test.VersionUtils.randomVersionBetween;
 import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.nullValue;
 
 public class GetApiKeyRequestTests extends ESTestCase {
 
@@ -144,6 +146,21 @@ public class GetApiKeyRequestTests extends ESTestCase {
         }
     }
 
+    public void testEmptyStringsAreCoercedToNull() {
+        Supplier<String> randomBlankString = () -> " ".repeat(randomIntBetween(0, 5));
+        final GetApiKeyRequest request = new GetApiKeyRequest(
+            randomBlankString.get(), // realm name
+            randomBlankString.get(), // user name
+            randomBlankString.get(), // key id
+            randomBlankString.get(), // key name
+            randomBoolean() // owned by user
+        );
+        assertThat(request.getRealmName(), nullValue());
+        assertThat(request.getUserName(), nullValue());
+        assertThat(request.getApiKeyId(), nullValue());
+        assertThat(request.getApiKeyName(), nullValue());
+    }
+
     private static String randomNullOrEmptyString() {
         return randomBoolean() ? "" : null;
     }

+ 23 - 6
x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/InvalidateApiKeyRequestTests.java

@@ -18,6 +18,7 @@ import org.elasticsearch.test.ESTestCase;
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
+import java.util.function.Supplier;
 
 import static org.elasticsearch.test.VersionUtils.getPreviousVersion;
 import static org.elasticsearch.test.VersionUtils.randomVersionBetween;
@@ -25,6 +26,7 @@ import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.nullValue;
 
 public class InvalidateApiKeyRequestTests extends ESTestCase {
 
@@ -36,7 +38,7 @@ public class InvalidateApiKeyRequestTests extends ESTestCase {
                 randomAlphaOfLength(12),
                 randomFrom(randomNullOrEmptyString(), randomAlphaOfLength(8)),
                 false,
-                new String[] { randomAlphaOfLength(12) }));
+                new String[]{randomAlphaOfLength(12)}));
         assertThat(e.getMessage(), containsString("Must use either [id] or [ids], not both at the same time"));
     }
 
@@ -47,7 +49,7 @@ public class InvalidateApiKeyRequestTests extends ESTestCase {
             null,
             randomFrom(randomNullOrEmptyString(), randomAlphaOfLength(8)),
             false,
-            new String[] {});
+            new String[]{});
         ActionRequestValidationException validationException = invalidateApiKeyRequest.validate();
         assertNotNull(validationException);
         assertThat(validationException.getMessage(), containsString("Field [ids] cannot be an empty array"));
@@ -58,7 +60,7 @@ public class InvalidateApiKeyRequestTests extends ESTestCase {
             null,
             randomFrom(randomNullOrEmptyString(), randomAlphaOfLength(8)),
             false,
-            new String[] {randomAlphaOfLength(12), null});
+            new String[]{randomAlphaOfLength(12), null});
         validationException = invalidateApiKeyRequest.validate();
         assertNotNull(validationException);
 
@@ -66,6 +68,21 @@ public class InvalidateApiKeyRequestTests extends ESTestCase {
             + "but got blank id at index position: [1]"));
     }
 
+    public void testEmptyStringsAreCoercedToNull() {
+        Supplier<String> randomBlankString = () -> " ".repeat(randomIntBetween(0, 5));
+        final InvalidateApiKeyRequest request = new InvalidateApiKeyRequest(
+            randomBlankString.get(), // realm name
+            randomBlankString.get(), // user name
+            randomBlankString.get(), // key id
+            randomBlankString.get(), // key name
+            randomBoolean() // owned by user
+        );
+        assertThat(request.getRealmName(), nullValue());
+        assertThat(request.getUserName(), nullValue());
+        assertThat(request.getIds(), nullValue());
+        assertThat(request.getName(), nullValue());
+    }
+
     public void testRequestValidation() {
         InvalidateApiKeyRequest request = InvalidateApiKeyRequest.usingApiKeyId(randomAlphaOfLength(5), randomBoolean());
         ActionRequestValidationException ve = request.validate();
@@ -112,7 +129,7 @@ public class InvalidateApiKeyRequestTests extends ESTestCase {
                 out.writeOptionalString(user);
                 if (out.getVersion().onOrAfter(Version.V_7_10_0)) {
                     if (Strings.hasText(apiKeyId)) {
-                        out.writeOptionalStringArray(new String[] { apiKeyId });
+                        out.writeOptionalStringArray(new String[]{apiKeyId});
                     } else {
                         out.writeOptionalStringArray(null);
                     }
@@ -148,7 +165,7 @@ public class InvalidateApiKeyRequestTests extends ESTestCase {
 
         for (int caseNo = 0; caseNo < inputs.length; caseNo++) {
             try (ByteArrayOutputStream bos = new ByteArrayOutputStream();
-                    OutputStreamStreamOutput osso = new OutputStreamStreamOutput(bos)) {
+                 OutputStreamStreamOutput osso = new OutputStreamStreamOutput(bos)) {
                 final Version streamVersion = randomVersionBetween(random(), Version.V_7_4_0, getPreviousVersion(Version.V_7_10_0));
                 Dummy d = new Dummy(inputs[caseNo]);
                 osso.setVersion(streamVersion);
@@ -218,7 +235,7 @@ public class InvalidateApiKeyRequestTests extends ESTestCase {
             null,
             randomFrom(randomNullOrEmptyString(), randomAlphaOfLength(8)),
             false,
-            new String[] { randomAlphaOfLength(12), randomAlphaOfLength(12) });
+            new String[]{randomAlphaOfLength(12), randomAlphaOfLength(12)});
         ByteArrayOutputStream outBuffer = new ByteArrayOutputStream();
         OutputStreamStreamOutput out = new OutputStreamStreamOutput(outBuffer);
         out.setVersion(randomVersionBetween(random(), Version.V_7_4_0, getPreviousVersion(Version.V_7_10_0)));