diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequest.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequest.java index feb4ea509d7be..f0d2c061d4625 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequest.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequest.java @@ -36,10 +36,10 @@ public GetApiKeyRequest() { 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 GetApiKeyRequest(StreamInput in) throws IOException { 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; } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/InvalidateApiKeyRequest.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/InvalidateApiKeyRequest.java index e7a10236c4909..1c74bc91b3773 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/InvalidateApiKeyRequest.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/InvalidateApiKeyRequest.java @@ -38,15 +38,15 @@ public InvalidateApiKeyRequest() { 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 InvalidateApiKeyRequest(@Nullable String realmName, @Nullable String user 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; } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java index f38920cf9cfd4..b4f359a61d397 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java @@ -17,11 +17,13 @@ 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 void testSerialization() throws IOException { } } + public void testEmptyStringsAreCoercedToNull() { + Supplier 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; } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/InvalidateApiKeyRequestTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/InvalidateApiKeyRequestTests.java index 1a12abd5cc52c..50e48c86b52df 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/InvalidateApiKeyRequestTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/InvalidateApiKeyRequestTests.java @@ -18,6 +18,7 @@ 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.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 void testCannotSpecifyBothIdAndIds() { 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 void testNonNullIdsCannotBeEmptyNorContainBlankId() { 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 void testNonNullIdsCannotBeEmptyNorContainBlankId() { 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 void testNonNullIdsCannotBeEmptyNorContainBlankId() { + "but got blank id at index position: [1]")); } + public void testEmptyStringsAreCoercedToNull() { + Supplier 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 void writeTo(StreamOutput out) throws IOException { 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 void writeTo(StreamOutput out) throws IOException { 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 void testSerializationWillThrowWhenMultipleIdsAndOldVersionStream() { 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)));