From 4350add12cc5d89aef7a0ca0d6badf29d71cdd09 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Tue, 14 Jul 2020 16:08:32 +1000 Subject: [PATCH] Allow null name when deserialising API key document (#59485) (#59496) 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. --- .../xpack/security/authc/ApiKeyService.java | 15 ++++++------- .../security/authc/ApiKeyServiceTests.java | 22 ++++++++++++++++--- 2 files changed, 26 insertions(+), 11 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java index acf5ebfbc38..b96ed7312e7 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java @@ -1040,11 +1040,11 @@ public class ApiKeyService { 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 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(); @@ -1054,11 +1054,10 @@ public class ApiKeyService { 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 creator; @@ -1068,10 +1067,10 @@ public class ApiKeyService { 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 creator) { diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java index 17cd00e7f6e..ff1d3e41999 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java @@ -800,19 +800,19 @@ public class ApiKeyServiceTests extends ESTestCase { 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 class ApiKeyServiceTests extends ESTestCase { assertEquals("bar", ((Map)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();