Allow null name when deserialising API key document (#59485)

API keys can be created without names using grant API key action. This is considered as a bug (#59484). Since the feature has already been released, we need to accomodate existing keys that are created with null names. This PR relaxes the parser logic so that a null name is accepted.
elastic · Jul 14, 2020 · c0fc9d0 · c0fc9d0
1 parent a51dda8
commit c0fc9d0
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 11 deletions.
diff --git a/...k/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java b/...k/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java
@@ -1030,11 +1030,11 @@ public static final class ApiKeyDoc {
             builder.declareLong(constructorArg(), new ParseField("creation_time"));
             builder.declareLongOrNull(constructorArg(), -1, new ParseField("expiration_time"));
             builder.declareBoolean(constructorArg(), new ParseField("api_key_invalidated"));
-            builder.declareString(optionalConstructorArg(), new ParseField("api_key_hash"));
-            builder.declareString(constructorArg(), new ParseField("name"));
+            builder.declareString(constructorArg(), new ParseField("api_key_hash"));
+            builder.declareStringOrNull(optionalConstructorArg(), new ParseField("name"));
             builder.declareInt(constructorArg(), new ParseField("version"));
             ObjectParserHelper<ApiKeyDoc, Void> parserHelper = new ObjectParserHelper<>();
-            parserHelper.declareRawObject(builder, optionalConstructorArg(), new ParseField("role_descriptors"));
+            parserHelper.declareRawObject(builder, constructorArg(), new ParseField("role_descriptors"));
             parserHelper.declareRawObject(builder, constructorArg(), new ParseField("limited_by_role_descriptors"));
             builder.declareObject(constructorArg(), (p, c) -> p.map(), new ParseField("creator"));
             PARSER = builder.build();
@@ -1044,11 +1044,10 @@ public static final class ApiKeyDoc {
         final long creationTime;
         final long expirationTime;
         final Boolean invalidated;
-        @Nullable
         final String hash;
+        @Nullable
         final String name;
         final int version;
-        @Nullable
         final BytesReference roleDescriptorsBytes;
         final BytesReference limitedByRoleDescriptorsBytes;
         final Map<String, Object> creator;
@@ -1058,10 +1057,10 @@ public ApiKeyDoc(
             long creationTime,
             long expirationTime,
             Boolean invalidated,
-            @Nullable String hash,
-            String name,
+            String hash,
+            @Nullable String name,
             int version,
-            @Nullable BytesReference roleDescriptorsBytes,
+            BytesReference roleDescriptorsBytes,
             BytesReference limitedByRoleDescriptorsBytes,
             Map<String, Object> creator) {
 

diff --git a/...gin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java b/...gin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java
@@ -800,19 +800,19 @@ public void testCachedApiKeyValidationWillNotBeBlockedByUnCachedApiKey() throws
 
     public void testApiKeyDocDeserialization() throws IOException {
         final String apiKeyDocumentSource =
-            "{\"doc_type\":\"api_key\",\"creation_time\":1591919944598,\"expiration_time\":null,\"api_key_invalidated\":false," +
+            "{\"doc_type\":\"api_key\",\"creation_time\":1591919944598,\"expiration_time\":1591919944599,\"api_key_invalidated\":false," +
                 "\"api_key_hash\":\"{PBKDF2}10000$abc\",\"role_descriptors\":{\"a\":{\"cluster\":[\"all\"]}}," +
                 "\"limited_by_role_descriptors\":{\"limited_by\":{\"cluster\":[\"all\"]," +
                 "\"metadata\":{\"_reserved\":true},\"type\":\"role\"}}," +
                 "\"name\":\"key-1\",\"version\":7000099," +
-                "\"creator\":{\"principal\":\"admin\",\"metadata\":{\"foo\":\"bar\"},\"realm\":\"file1\",\"realm_type\":\"file\"}}\n";
+                "\"creator\":{\"principal\":\"admin\",\"metadata\":{\"foo\":\"bar\"},\"realm\":\"file1\",\"realm_type\":\"file\"}}";
         final ApiKeyDoc apiKeyDoc = ApiKeyDoc.fromXContent(XContentHelper.createParser(NamedXContentRegistry.EMPTY,
             LoggingDeprecationHandler.INSTANCE,
             new BytesArray(apiKeyDocumentSource),
             XContentType.JSON));
         assertEquals("api_key", apiKeyDoc.docType);
         assertEquals(1591919944598L, apiKeyDoc.creationTime);
-        assertEquals(-1L, apiKeyDoc.expirationTime);
+        assertEquals(1591919944599L, apiKeyDoc.expirationTime);
         assertFalse(apiKeyDoc.invalidated);
         assertEquals("{PBKDF2}10000$abc", apiKeyDoc.hash);
         assertEquals("key-1", apiKeyDoc.name);
@@ -828,6 +828,22 @@ public void testApiKeyDocDeserialization() throws IOException {
         assertEquals("bar", ((Map<String, Object>)creator.get("metadata")).get("foo"));
     }
 
+    public void testApiKeyDocDeserializationWithNullValues() throws IOException {
+        final String apiKeyDocumentSource =
+            "{\"doc_type\":\"api_key\",\"creation_time\":1591919944598,\"expiration_time\":null,\"api_key_invalidated\":false," +
+                "\"api_key_hash\":\"{PBKDF2}10000$abc\",\"role_descriptors\":{}," +
+                "\"limited_by_role_descriptors\":{\"limited_by\":{\"cluster\":[\"all\"]}}," +
+                "\"name\":null,\"version\":7000099," +
+                "\"creator\":{\"principal\":\"admin\",\"metadata\":{},\"realm\":\"file1\"}}";
+        final ApiKeyDoc apiKeyDoc = ApiKeyDoc.fromXContent(XContentHelper.createParser(NamedXContentRegistry.EMPTY,
+            LoggingDeprecationHandler.INSTANCE,
+            new BytesArray(apiKeyDocumentSource),
+            XContentType.JSON));
+        assertEquals(-1L, apiKeyDoc.expirationTime);
+        assertNull(apiKeyDoc.name);
+        assertEquals(new BytesArray("{}"), apiKeyDoc.roleDescriptorsBytes);
+    }
+
     public static class Utils {
         private static final AuthenticationContextSerializer authenticationContextSerializer = new AuthenticationContextSerializer();