diff --git a/docs/changelog/88346.yaml b/docs/changelog/88346.yaml new file mode 100644 index 0000000000000..ca2537f28a5a9 --- /dev/null +++ b/docs/changelog/88346.yaml @@ -0,0 +1,5 @@ +pr: 88346 +summary: Updatable API keys - noop check +area: Security +type: enhancement +issues: [] diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/RealmDomain.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/RealmDomain.java index 8863953dc844d..53de14b5b68bb 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/RealmDomain.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/RealmDomain.java @@ -14,6 +14,7 @@ import org.elasticsearch.xcontent.ParseField; import org.elasticsearch.xcontent.ToXContentObject; import org.elasticsearch.xcontent.XContentBuilder; +import org.elasticsearch.xcontent.XContentParser; import java.io.IOException; import java.util.List; @@ -48,6 +49,10 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws return builder; } + public static RealmDomain fromXContent(final XContentParser parser) { + return REALM_DOMAIN_PARSER.apply(parser, null); + } + @Override public String toString() { return "RealmDomain{" + "name='" + name + '\'' + ", realms=" + realms + '}'; diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/AuthenticationTestHelper.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/AuthenticationTestHelper.java index 8cb8b684a64ab..1bbcec5d92ce8 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/AuthenticationTestHelper.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/AuthenticationTestHelper.java @@ -90,6 +90,36 @@ public static User randomUser() { ); } + public static User userWithRandomMetadataAndDetails(final String username, final String... roles) { + return new User( + username, + roles, + ESTestCase.randomFrom(ESTestCase.randomAlphaOfLengthBetween(1, 10), null), + // Not a very realistic email address, but we don't validate this nor rely on correct format, so keeping it simple + ESTestCase.randomFrom(ESTestCase.randomAlphaOfLengthBetween(1, 10), null), + randomUserMetadata(), + true + ); + } + + public static Map randomUserMetadata() { + return ESTestCase.randomFrom( + Map.of( + "employee_id", + ESTestCase.randomAlphaOfLength(5), + "number", + 1, + "numbers", + List.of(1, 3, 5), + "extra", + Map.of("favorite pizza", "hawaii", "age", 42) + ), + Map.of(ESTestCase.randomAlphaOfLengthBetween(3, 8), ESTestCase.randomAlphaOfLengthBetween(3, 8)), + Map.of(), + null + ); + } + public static RealmDomain randomDomain(boolean includeInternal) { final Supplier randomRealmTypeSupplier = randomRealmTypeSupplier(includeInternal); final Set domainRealms = new HashSet<>( diff --git a/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/ApiKeyRestIT.java b/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/ApiKeyRestIT.java index c7f89106338cd..516d3b3d7a3a2 100644 --- a/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/ApiKeyRestIT.java +++ b/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/ApiKeyRestIT.java @@ -198,7 +198,7 @@ public void testGrantApiKeyWithOnlyManageOwnApiKeyPrivilegeFails() throws IOExce public void testUpdateApiKey() throws IOException { final var apiKeyName = "my-api-key-name"; - final Map apiKeyMetadata = Map.of("not", "returned"); + final Map apiKeyMetadata = Map.of("not", "returned"); final Map createApiKeyRequestBody = Map.of("name", apiKeyName, "metadata", apiKeyMetadata); final Request createApiKeyRequest = new Request("POST", "_security/api_key"); @@ -215,7 +215,7 @@ public void testUpdateApiKey() throws IOException { assertThat(apiKeyId, not(emptyString())); assertThat(apiKeyEncoded, not(emptyString())); - doTestUpdateApiKey(apiKeyName, apiKeyId, apiKeyEncoded); + doTestUpdateApiKey(apiKeyName, apiKeyId, apiKeyEncoded, apiKeyMetadata); } public void testGrantTargetCanUpdateApiKey() throws IOException { @@ -240,7 +240,7 @@ public void testGrantTargetCanUpdateApiKey() throws IOException { assertThat(apiKeyId, not(emptyString())); assertThat(apiKeyEncoded, not(emptyString())); - doTestUpdateApiKey(apiKeyName, apiKeyId, apiKeyEncoded); + doTestUpdateApiKey(apiKeyName, apiKeyId, apiKeyEncoded, null); } public void testGrantorCannotUpdateApiKeyOfGrantTarget() throws IOException { @@ -283,18 +283,26 @@ private void doTestAuthenticationWithApiKey(final String apiKeyName, final Strin assertThat(authenticate, hasEntry("api_key", Map.of("id", apiKeyId, "name", apiKeyName))); } - private void doTestUpdateApiKey(String apiKeyName, String apiKeyId, String apiKeyEncoded) throws IOException { + private void doTestUpdateApiKey( + final String apiKeyName, + final String apiKeyId, + final String apiKeyEncoded, + final Map oldMetadata + ) throws IOException { final var updateApiKeyRequest = new Request("PUT", "_security/api_key/" + apiKeyId); - final Map expectedApiKeyMetadata = Map.of("not", "returned (changed)", "foo", "bar"); - final Map updateApiKeyRequestBody = Map.of("metadata", expectedApiKeyMetadata); + final boolean updated = randomBoolean(); + final Map expectedApiKeyMetadata = updated ? Map.of("not", "returned (changed)", "foo", "bar") : oldMetadata; + final Map updateApiKeyRequestBody = expectedApiKeyMetadata == null + ? Map.of() + : Map.of("metadata", expectedApiKeyMetadata); updateApiKeyRequest.setJsonEntity(XContentTestUtils.convertToXContent(updateApiKeyRequestBody, XContentType.JSON).utf8ToString()); final Response updateApiKeyResponse = doUpdateUsingRandomAuthMethod(updateApiKeyRequest); assertOK(updateApiKeyResponse); final Map updateApiKeyResponseMap = responseAsMap(updateApiKeyResponse); - assertTrue((Boolean) updateApiKeyResponseMap.get("updated")); - expectMetadata(apiKeyId, expectedApiKeyMetadata); + assertEquals(updated, updateApiKeyResponseMap.get("updated")); + expectMetadata(apiKeyId, expectedApiKeyMetadata == null ? Map.of() : expectedApiKeyMetadata); // validate authentication still works after update doTestAuthenticationWithApiKey(apiKeyName, apiKeyId, apiKeyEncoded); } diff --git a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java index 36d60d6e25fd3..a2624cc1fdca1 100644 --- a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java +++ b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java @@ -28,11 +28,13 @@ import org.elasticsearch.client.RestClient; import org.elasticsearch.client.internal.Client; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.settings.SecureString; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.EsExecutors; import org.elasticsearch.common.util.concurrent.EsRejectedExecutionException; import org.elasticsearch.common.util.set.Sets; +import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.core.TimeValue; import org.elasticsearch.core.Tuple; import org.elasticsearch.rest.RestStatus; @@ -43,6 +45,8 @@ import org.elasticsearch.test.XContentTestUtils; import org.elasticsearch.test.rest.ObjectPath; import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.xcontent.XContentBuilder; +import org.elasticsearch.xcontent.XContentFactory; import org.elasticsearch.xcontent.XContentType; import org.elasticsearch.xpack.core.XPackSettings; import org.elasticsearch.xpack.core.security.action.ClearSecurityCacheAction; @@ -137,6 +141,13 @@ public class ApiKeyIntegTests extends SecurityIntegTestCase { private static final long DELETE_INTERVAL_MILLIS = 100L; private static final int CRYPTO_THREAD_POOL_QUEUE_SIZE = 10; + private static final RoleDescriptor DEFAULT_API_KEY_ROLE_DESCRIPTOR = new RoleDescriptor( + "role", + new String[] { "monitor" }, + null, + null + ); + @Override public Settings nodeSettings(int nodeOrdinal, Settings otherSettings) { return Settings.builder() @@ -1435,6 +1446,7 @@ public void testSecurityIndexStateChangeWillInvalidateApiKeyCaches() throws Exce public void testUpdateApiKey() throws ExecutionException, InterruptedException, IOException { final Tuple> createdApiKey = createApiKey(TEST_USER_NAME, null); final var apiKeyId = createdApiKey.v1().getId(); + final Map oldMetadata = createdApiKey.v2(); final var newRoleDescriptors = randomRoleDescriptors(); final boolean nullRoleDescriptors = newRoleDescriptors == null; // Role descriptor corresponding to SecuritySettingsSource.TEST_ROLE_YML @@ -1449,11 +1461,14 @@ public void testUpdateApiKey() throws ExecutionException, InterruptedException, ); final var request = new UpdateApiKeyRequest(apiKeyId, newRoleDescriptors, ApiKeyTests.randomMetadata()); - final PlainActionFuture listener = new PlainActionFuture<>(); - final UpdateApiKeyResponse response = executeUpdateApiKey(TEST_USER_NAME, request, listener); + final UpdateApiKeyResponse response = executeUpdateApiKey(TEST_USER_NAME, request); assertNotNull(response); - assertTrue(response.isUpdated()); + // In this test, non-null roleDescriptors always result in an update since they either update the role name, or associated + // privileges. As such null descriptors (plus matching or null metadata) is the only way we can get a noop here + final boolean metadataChanged = request.getMetadata() != null && false == request.getMetadata().equals(oldMetadata); + final boolean isUpdated = nullRoleDescriptors == false || metadataChanged; + assertEquals(isUpdated, response.isUpdated()); final PlainActionFuture getListener = new PlainActionFuture<>(); client().filterWithHeader( @@ -1475,31 +1490,37 @@ public void testUpdateApiKey() throws ExecutionException, InterruptedException, final var updatedApiKeyDoc = getApiKeyDocument(apiKeyId); expectMetadataForApiKey(expectedMetadata, updatedApiKeyDoc); expectRoleDescriptorsForApiKey("limited_by_role_descriptors", expectedLimitedByRoleDescriptors, updatedApiKeyDoc); - if (nullRoleDescriptors) { - // Default role descriptor assigned to api key in `createApiKey` - final var expectedRoleDescriptor = new RoleDescriptor("role", new String[] { "monitor" }, null, null); - expectRoleDescriptorsForApiKey("role_descriptors", List.of(expectedRoleDescriptor), updatedApiKeyDoc); - - // Create user action unauthorized because we did not update key role; it only has `monitor` cluster priv - final Map authorizationHeaders = Collections.singletonMap( - "Authorization", - "ApiKey " + getBase64EncodedApiKeyValue(createdApiKey.v1().getId(), createdApiKey.v1().getKey()) - ); + final var expectedRoleDescriptors = nullRoleDescriptors ? List.of(DEFAULT_API_KEY_ROLE_DESCRIPTOR) : newRoleDescriptors; + expectRoleDescriptorsForApiKey("role_descriptors", expectedRoleDescriptors, updatedApiKeyDoc); + final Map expectedCreator = new HashMap<>(); + expectedCreator.put("principal", TEST_USER_NAME); + expectedCreator.put("full_name", null); + expectedCreator.put("email", null); + expectedCreator.put("metadata", Map.of()); + expectedCreator.put("realm_type", "file"); + expectedCreator.put("realm", "file"); + expectCreatorForApiKey(expectedCreator, updatedApiKeyDoc); + + // Check if update resulted in API key role going from `monitor` to `all` cluster privilege and assert that action that requires + // `all` is authorized or denied accordingly + final boolean hasAllClusterPrivilege = expectedRoleDescriptors.stream() + .filter(rd -> Arrays.asList(rd.getClusterPrivileges()).contains("all")) + .toList() + .isEmpty() == false; + final var authorizationHeaders = Collections.singletonMap( + "Authorization", + "ApiKey " + getBase64EncodedApiKeyValue(createdApiKey.v1().getId(), createdApiKey.v1().getKey()) + ); + if (hasAllClusterPrivilege) { + createUserWithRunAsRole(authorizationHeaders); + } else { ExecutionException e = expectThrows(ExecutionException.class, () -> createUserWithRunAsRole(authorizationHeaders)); assertThat(e.getMessage(), containsString("unauthorized")); assertThat(e.getCause(), instanceOf(ElasticsearchSecurityException.class)); - } else { - expectRoleDescriptorsForApiKey("role_descriptors", newRoleDescriptors, updatedApiKeyDoc); - // Create user action authorized because we updated key role to `all` cluster priv - final var authorizationHeaders = Collections.singletonMap( - "Authorization", - "ApiKey " + getBase64EncodedApiKeyValue(createdApiKey.v1().getId(), createdApiKey.v1().getKey()) - ); - createUserWithRunAsRole(authorizationHeaders); } } - public void testUpdateApiKeyAutoUpdatesUserRoles() throws IOException, ExecutionException, InterruptedException { + public void testUpdateApiKeyAutoUpdatesUserFields() throws IOException, ExecutionException, InterruptedException { // Create separate native realm user and role for user role change test final var nativeRealmUser = randomAlphaOfLengthBetween(5, 10); final var nativeRealmRole = randomAlphaOfLengthBetween(5, 10); @@ -1536,13 +1557,39 @@ public void testUpdateApiKeyAutoUpdatesUserRoles() throws IOException, Execution newClusterPrivileges.toArray(new String[0]) ); - // Update API key - final PlainActionFuture listener = new PlainActionFuture<>(); - final UpdateApiKeyResponse response = executeUpdateApiKey(nativeRealmUser, UpdateApiKeyRequest.usingApiKeyId(apiKeyId), listener); + UpdateApiKeyResponse response = executeUpdateApiKey(nativeRealmUser, UpdateApiKeyRequest.usingApiKeyId(apiKeyId)); assertNotNull(response); assertTrue(response.isUpdated()); expectRoleDescriptorsForApiKey("limited_by_role_descriptors", Set.of(roleDescriptorAfterUpdate), getApiKeyDocument(apiKeyId)); + + // Update user role name only + final RoleDescriptor roleDescriptorWithNewName = putRoleWithClusterPrivileges( + randomValueOtherThan(nativeRealmRole, () -> randomAlphaOfLength(10)), + // Keep old privileges + newClusterPrivileges.toArray(new String[0]) + ); + final User updatedUser = AuthenticationTestHelper.userWithRandomMetadataAndDetails( + nativeRealmUser, + roleDescriptorWithNewName.getName() + ); + updateUser(updatedUser); + + // Update API key + response = executeUpdateApiKey(nativeRealmUser, UpdateApiKeyRequest.usingApiKeyId(apiKeyId)); + + assertNotNull(response); + assertTrue(response.isUpdated()); + final Map updatedApiKeyDoc = getApiKeyDocument(apiKeyId); + expectRoleDescriptorsForApiKey("limited_by_role_descriptors", Set.of(roleDescriptorWithNewName), updatedApiKeyDoc); + final Map expectedCreator = new HashMap<>(); + expectedCreator.put("principal", updatedUser.principal()); + expectedCreator.put("full_name", updatedUser.fullName()); + expectedCreator.put("email", updatedUser.email()); + expectedCreator.put("metadata", updatedUser.metadata()); + expectedCreator.put("realm_type", "native"); + expectedCreator.put("realm", "index"); + expectCreatorForApiKey(expectedCreator, updatedApiKeyDoc); } public void testUpdateApiKeyNotFoundScenarios() throws ExecutionException, InterruptedException { @@ -1552,8 +1599,7 @@ public void testUpdateApiKeyNotFoundScenarios() throws ExecutionException, Inter final var request = new UpdateApiKeyRequest(apiKeyId, List.of(expectedRoleDescriptor), ApiKeyTests.randomMetadata()); // Validate can update own API key - final PlainActionFuture listener = new PlainActionFuture<>(); - final UpdateApiKeyResponse response = executeUpdateApiKey(TEST_USER_NAME, request, listener); + final UpdateApiKeyResponse response = executeUpdateApiKey(TEST_USER_NAME, request); assertNotNull(response); assertTrue(response.isUpdated()); @@ -1646,7 +1692,7 @@ public void testInvalidUpdateApiKeyScenarios() throws ExecutionException, Interr } } - public void testUpdateApiKeyAccountsForSecurityDomains() throws ExecutionException, InterruptedException { + public void testUpdateApiKeyAccountsForSecurityDomains() throws ExecutionException, InterruptedException, IOException { final Tuple> createdApiKey = createApiKey(TEST_USER_NAME, null); final var apiKeyId = createdApiKey.v1().getId(); @@ -1679,6 +1725,147 @@ public void testUpdateApiKeyAccountsForSecurityDomains() throws ExecutionExcepti assertNotNull(response); assertTrue(response.isUpdated()); + final Map expectedCreator = new HashMap<>(); + expectedCreator.put("principal", TEST_USER_NAME); + expectedCreator.put("full_name", null); + expectedCreator.put("email", null); + expectedCreator.put("metadata", Map.of()); + expectedCreator.put("realm_type", authenticatingRealm.getType()); + expectedCreator.put("realm", authenticatingRealm.getName()); + final XContentBuilder builder = realmDomain.toXContent(XContentFactory.jsonBuilder(), null); + expectedCreator.put("realm_domain", XContentHelper.convertToMap(BytesReference.bytes(builder), false, XContentType.JSON).v2()); + expectCreatorForApiKey(expectedCreator, getApiKeyDocument(apiKeyId)); + } + + public void testNoopUpdateApiKey() throws ExecutionException, InterruptedException, IOException { + final Tuple> createdApiKey = createApiKey(TEST_USER_NAME, null); + final var apiKeyId = createdApiKey.v1().getId(); + + final var initialRequest = new UpdateApiKeyRequest( + apiKeyId, + List.of(new RoleDescriptor(randomAlphaOfLength(10), new String[] { "all" }, null, null)), + ApiKeyTests.randomMetadata() + ); + UpdateApiKeyResponse response = executeUpdateApiKey(TEST_USER_NAME, initialRequest); + assertNotNull(response); + // First update is not noop, because role descriptors changed and possibly metadata + assertTrue(response.isUpdated()); + + // Update with same request is a noop and does not clear cache + authenticateWithApiKey(apiKeyId, createdApiKey.v1().getKey()); + final var serviceWithNameForDoc1 = Arrays.stream(internalCluster().getNodeNames()) + .map(n -> internalCluster().getInstance(ApiKeyService.class, n)) + .filter(s -> s.getDocCache().get(apiKeyId) != null) + .findFirst() + .orElseThrow(); + final int count = serviceWithNameForDoc1.getDocCache().count(); + response = executeUpdateApiKey(TEST_USER_NAME, initialRequest); + assertNotNull(response); + assertFalse(response.isUpdated()); + assertEquals(count, serviceWithNameForDoc1.getDocCache().count()); + + // Update with empty request is a noop + response = executeUpdateApiKey(TEST_USER_NAME, UpdateApiKeyRequest.usingApiKeyId(apiKeyId)); + assertNotNull(response); + assertFalse(response.isUpdated()); + + // Update with different role descriptors is not a noop + final List newRoleDescriptors = List.of( + randomValueOtherThanMany( + rd -> (RoleDescriptorRequestValidator.validate(rd) != null) && initialRequest.getRoleDescriptors().contains(rd) == false, + () -> RoleDescriptorTests.randomRoleDescriptor(false) + ), + randomValueOtherThanMany( + rd -> (RoleDescriptorRequestValidator.validate(rd) != null) && initialRequest.getRoleDescriptors().contains(rd) == false, + () -> RoleDescriptorTests.randomRoleDescriptor(false) + ) + ); + response = executeUpdateApiKey(TEST_USER_NAME, new UpdateApiKeyRequest(apiKeyId, newRoleDescriptors, null)); + assertNotNull(response); + assertTrue(response.isUpdated()); + + // Update with re-ordered role descriptors is a noop + response = executeUpdateApiKey( + TEST_USER_NAME, + new UpdateApiKeyRequest(apiKeyId, List.of(newRoleDescriptors.get(1), newRoleDescriptors.get(0)), null) + ); + assertNotNull(response); + assertFalse(response.isUpdated()); + + // Update with different metadata is not a noop + response = executeUpdateApiKey( + TEST_USER_NAME, + new UpdateApiKeyRequest( + apiKeyId, + null, + randomValueOtherThanMany(md -> md == null || md.equals(initialRequest.getMetadata()), ApiKeyTests::randomMetadata) + ) + ); + assertNotNull(response); + assertTrue(response.isUpdated()); + + // Update with different creator info is not a noop + // First, ensure that the user role descriptors alone do *not* cause an update, so we can test that we correctly perform the noop + // check when we update creator info + final ServiceWithNodeName serviceWithNodeName = getServiceWithNodeName(); + PlainActionFuture listener = new PlainActionFuture<>(); + // Role descriptor corresponding to SecuritySettingsSource.TEST_ROLE_YML, i.e., should not result in update + final Set oldUserRoleDescriptors = Set.of( + new RoleDescriptor( + TEST_ROLE, + new String[] { "ALL" }, + new RoleDescriptor.IndicesPrivileges[] { + RoleDescriptor.IndicesPrivileges.builder().indices("*").allowRestrictedIndices(true).privileges("ALL").build() }, + null + ) + ); + serviceWithNodeName.service() + .updateApiKey( + Authentication.newRealmAuthentication( + new User(TEST_USER_NAME, TEST_ROLE), + new Authentication.RealmRef("file", "file", serviceWithNodeName.nodeName()) + ), + UpdateApiKeyRequest.usingApiKeyId(apiKeyId), + oldUserRoleDescriptors, + listener + ); + response = listener.get(); + assertNotNull(response); + assertFalse(response.isUpdated()); + final User updatedUser = AuthenticationTestHelper.userWithRandomMetadataAndDetails(TEST_USER_NAME, TEST_ROLE); + final RealmConfig.RealmIdentifier creatorRealmOnCreatedApiKey = new RealmConfig.RealmIdentifier(FileRealmSettings.TYPE, "file"); + final boolean noUserChanges = updatedUser.equals(new User(TEST_USER_NAME, TEST_ROLE)); + final Authentication.RealmRef realmRef; + if (randomBoolean() || noUserChanges) { + final RealmConfig.RealmIdentifier otherRealmInDomain = AuthenticationTestHelper.randomRealmIdentifier(true); + final var realmDomain = new RealmDomain( + ESTestCase.randomAlphaOfLengthBetween(3, 8), + Set.of(creatorRealmOnCreatedApiKey, otherRealmInDomain) + ); + // Using other realm from domain should result in update + realmRef = new Authentication.RealmRef( + otherRealmInDomain.getName(), + otherRealmInDomain.getType(), + serviceWithNodeName.nodeName(), + realmDomain + ); + } else { + realmRef = new Authentication.RealmRef( + creatorRealmOnCreatedApiKey.getName(), + creatorRealmOnCreatedApiKey.getType(), + serviceWithNodeName.nodeName() + ); + } + final var authentication = randomValueOtherThanMany( + Authentication::isApiKey, + () -> AuthenticationTestHelper.builder().user(updatedUser).realmRef(realmRef).build() + ); + listener = new PlainActionFuture<>(); + serviceWithNodeName.service() + .updateApiKey(authentication, UpdateApiKeyRequest.usingApiKeyId(apiKeyId), oldUserRoleDescriptors, listener); + response = listener.get(); + assertNotNull(response); + assertTrue(response.isUpdated()); } public void testUpdateApiKeyClearsApiKeyDocCache() throws IOException, ExecutionException, InterruptedException { @@ -1720,7 +1907,12 @@ public void testUpdateApiKeyClearsApiKeyDocCache() throws IOException, Execution final Client client = client().filterWithHeader( Collections.singletonMap("Authorization", basicAuthHeaderValue(ES_TEST_ROOT_USER, TEST_PASSWORD_SECURE_STRING)) ); - client.execute(UpdateApiKeyAction.INSTANCE, new UpdateApiKeyRequest(apiKey1.v1(), List.of(), null), listener); + client.execute( + UpdateApiKeyAction.INSTANCE, + // Set metadata to ensure update + new UpdateApiKeyRequest(apiKey1.v1(), List.of(), Map.of(randomAlphaOfLength(5), randomAlphaOfLength(10))), + listener + ); final var response = listener.get(); assertNotNull(response); assertTrue(response.isUpdated()); @@ -1741,7 +1933,7 @@ public void testUpdateApiKeyClearsApiKeyDocCache() throws IOException, Execution } private List randomRoleDescriptors() { - int caseNo = randomIntBetween(0, 2); + int caseNo = randomIntBetween(0, 3); return switch (caseNo) { case 0 -> List.of(new RoleDescriptor(randomAlphaOfLength(10), new String[] { "all" }, null, null)); case 1 -> List.of( @@ -1752,6 +1944,15 @@ private List randomRoleDescriptors() { ) ); case 2 -> null; + // vary default role descriptor assigned to created API keys by name only + case 3 -> List.of( + new RoleDescriptor( + randomValueOtherThan(DEFAULT_API_KEY_ROLE_DESCRIPTOR.getName(), () -> randomAlphaOfLength(10)), + DEFAULT_API_KEY_ROLE_DESCRIPTOR.getClusterPrivileges(), + DEFAULT_API_KEY_ROLE_DESCRIPTOR.getIndicesPrivileges(), + DEFAULT_API_KEY_ROLE_DESCRIPTOR.getRunAs() + ) + ); default -> throw new IllegalStateException("unexpected case no"); }; } @@ -1774,6 +1975,13 @@ private void expectMetadataForApiKey(final Map expectedMetadata, assertThat("for api key doc " + actualRawApiKeyDoc, actualMetadata, equalTo(expectedMetadata)); } + private void expectCreatorForApiKey(final Map expectedCreator, final Map actualRawApiKeyDoc) { + assertNotNull(actualRawApiKeyDoc); + @SuppressWarnings("unchecked") + final var actualCreator = (Map) actualRawApiKeyDoc.get("creator"); + assertThat("for api key doc " + actualRawApiKeyDoc, actualCreator, equalTo(expectedCreator)); + } + @SuppressWarnings("unchecked") private void expectRoleDescriptorsForApiKey( final String roleDescriptorType, @@ -1940,12 +2148,17 @@ private void verifyGetResponse( } private Tuple> createApiKey(String user, TimeValue expiration) { - final Tuple, List>> res = createApiKeys(user, 1, expiration, "monitor"); + final Tuple, List>> res = createApiKeys( + user, + 1, + expiration, + DEFAULT_API_KEY_ROLE_DESCRIPTOR.getClusterPrivileges() + ); return new Tuple<>(res.v1().get(0), res.v2().get(0)); } private Tuple, List>> createApiKeys(int noOfApiKeys, TimeValue expiration) { - return createApiKeys(ES_TEST_ROOT_USER, noOfApiKeys, expiration, "monitor"); + return createApiKeys(ES_TEST_ROOT_USER, noOfApiKeys, expiration, DEFAULT_API_KEY_ROLE_DESCRIPTOR.getClusterPrivileges()); } private Tuple, List>> createApiKeys( @@ -1996,7 +2209,7 @@ private Tuple, List>> createApiKe List> metadatas = new ArrayList<>(noOfApiKeys); List responses = new ArrayList<>(); for (int i = 0; i < noOfApiKeys; i++) { - final RoleDescriptor descriptor = new RoleDescriptor("role", clusterPrivileges, null, null); + final RoleDescriptor descriptor = new RoleDescriptor(DEFAULT_API_KEY_ROLE_DESCRIPTOR.getName(), clusterPrivileges, null, null); Client client = client().filterWithHeader(headers); final Map metadata = ApiKeyTests.randomMetadata(); metadatas.add(metadata); @@ -2041,13 +2254,27 @@ private void createNativeRealmUser( assertTrue(putUserResponse.created()); } + private void updateUser(User user) throws ExecutionException, InterruptedException { + final PutUserRequest putUserRequest = new PutUserRequest(); + putUserRequest.username(user.principal()); + putUserRequest.roles(user.roles()); + putUserRequest.metadata(user.metadata()); + putUserRequest.fullName(user.fullName()); + putUserRequest.email(user.email()); + final PlainActionFuture listener = new PlainActionFuture<>(); + final Client client = client().filterWithHeader( + Map.of("Authorization", basicAuthHeaderValue(ES_TEST_ROOT_USER, TEST_PASSWORD_SECURE_STRING)) + ); + client.execute(PutUserAction.INSTANCE, putUserRequest, listener); + final PutUserResponse putUserResponse = listener.get(); + assertFalse(putUserResponse.created()); + } + private RoleDescriptor putRoleWithClusterPrivileges(final String nativeRealmRoleName, String... clusterPrivileges) throws InterruptedException, ExecutionException { final PutRoleRequest putRoleRequest = new PutRoleRequest(); putRoleRequest.name(nativeRealmRoleName); - for (final String clusterPrivilege : clusterPrivileges) { - putRoleRequest.cluster(clusterPrivilege); - } + putRoleRequest.cluster(clusterPrivileges); final PlainActionFuture roleListener = new PlainActionFuture<>(); client().filterWithHeader(Map.of("Authorization", basicAuthHeaderValue(ES_TEST_ROOT_USER, TEST_PASSWORD_SECURE_STRING))) .execute(PutRoleAction.INSTANCE, putRoleRequest, roleListener); @@ -2066,11 +2293,9 @@ private Client getClientForRunAsUser() { ); } - private UpdateApiKeyResponse executeUpdateApiKey( - final String username, - final UpdateApiKeyRequest request, - final PlainActionFuture listener - ) throws InterruptedException, ExecutionException { + private UpdateApiKeyResponse executeUpdateApiKey(final String username, final UpdateApiKeyRequest request) throws InterruptedException, + ExecutionException { + final var listener = new PlainActionFuture(); final Client client = client().filterWithHeader( Collections.singletonMap("Authorization", basicAuthHeaderValue(username, TEST_PASSWORD_SECURE_STRING)) ); 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 fe30462e9b8d6..883ad3ca98c19 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 @@ -119,6 +119,7 @@ import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; @@ -279,27 +280,27 @@ public void invalidateAll() { * Asynchronously creates a new API key based off of the request and authentication * @param authentication the authentication that this api key should be based off of * @param request the request to create the api key included any permission restrictions - * @param userRoles the user's actual roles that we always enforce + * @param userRoleDescriptors the user's actual roles that we always enforce * @param listener the listener that will be used to notify of completion */ public void createApiKey( Authentication authentication, CreateApiKeyRequest request, - Set userRoles, + Set userRoleDescriptors, ActionListener listener ) { ensureEnabled(); if (authentication == null) { listener.onFailure(new IllegalArgumentException("authentication must be provided")); } else { - createApiKeyAndIndexIt(authentication, request, userRoles, listener); + createApiKeyAndIndexIt(authentication, request, userRoleDescriptors, listener); } } private void createApiKeyAndIndexIt( Authentication authentication, CreateApiKeyRequest request, - Set roleDescriptorSet, + Set userRoleDescriptors, ActionListener listener ) { final Instant created = clock.instant(); @@ -313,7 +314,7 @@ private void createApiKeyAndIndexIt( apiKeyHashChars, request.getName(), authentication, - roleDescriptorSet, + userRoleDescriptors, created, expiration, request.getRoleDescriptors(), @@ -358,7 +359,7 @@ private void createApiKeyAndIndexIt( public void updateApiKey( final Authentication authentication, final UpdateApiKeyRequest request, - final Set userRoles, + final Set userRoleDescriptors, final ActionListener listener ) { ensureEnabled(); @@ -386,10 +387,7 @@ public void updateApiKey( validateCurrentApiKeyDocForUpdate(apiKeyId, authentication, versionedDoc.doc()); - executeBulkRequest( - buildBulkRequestForUpdate(versionedDoc, authentication, request, userRoles), - ActionListener.wrap(bulkResponse -> translateResponseAndClearCache(apiKeyId, bulkResponse, listener), listener::onFailure) - ); + doUpdateApiKey(authentication, request, userRoleDescriptors, versionedDoc, listener); }, listener::onFailure)); } @@ -418,10 +416,10 @@ static XContentBuilder newDocument( char[] apiKeyHashChars, String name, Authentication authentication, - Set userRoles, + Set userRoleDescriptors, Instant created, Instant expiration, - List keyRoles, + List keyRoleDescriptors, Version version, @Nullable Map metadata ) throws IOException { @@ -433,8 +431,8 @@ static XContentBuilder newDocument( .field("api_key_invalidated", false); addApiKeyHash(builder, apiKeyHashChars); - addRoleDescriptors(builder, keyRoles); - addLimitedByRoleDescriptors(builder, userRoles); + addRoleDescriptors(builder, keyRoleDescriptors); + addLimitedByRoleDescriptors(builder, userRoleDescriptors); builder.field("name", name).field("version", version.id).field("metadata_flattened", metadata); addCreator(builder, authentication); @@ -442,14 +440,22 @@ static XContentBuilder newDocument( return builder.endObject(); } - static XContentBuilder buildUpdatedDocument( + // package private for testing + + /** + * @return `null` if the update is a noop, i.e., if no changes to `currentApiKeyDoc` are required + */ + XContentBuilder maybeBuildUpdatedDocument( final ApiKeyDoc currentApiKeyDoc, + final Version targetDocVersion, final Authentication authentication, - final Set userRoles, - final List keyRoles, - final Version version, - final Map metadata + final UpdateApiKeyRequest request, + final Set userRoleDescriptors ) throws IOException { + if (isNoop(currentApiKeyDoc, targetDocVersion, authentication, request, userRoleDescriptors)) { + return null; + } + final XContentBuilder builder = XContentFactory.jsonBuilder(); builder.startObject() .field("doc_type", "api_key") @@ -459,6 +465,7 @@ static XContentBuilder buildUpdatedDocument( addApiKeyHash(builder, currentApiKeyDoc.hash.toCharArray()); + final List keyRoles = request.getRoleDescriptors(); if (keyRoles != null) { logger.trace(() -> format("Building API key doc with updated role descriptors [{}]", keyRoles)); addRoleDescriptors(builder, keyRoles); @@ -467,14 +474,15 @@ static XContentBuilder buildUpdatedDocument( builder.rawField("role_descriptors", currentApiKeyDoc.roleDescriptorsBytes.streamInput(), XContentType.JSON); } - addLimitedByRoleDescriptors(builder, userRoles); + addLimitedByRoleDescriptors(builder, userRoleDescriptors); - builder.field("name", currentApiKeyDoc.name).field("version", version.id); + builder.field("name", currentApiKeyDoc.name).field("version", targetDocVersion.id); assert currentApiKeyDoc.metadataFlattened == null || MetadataUtils.containsReservedMetadata( XContentHelper.convertToMap(currentApiKeyDoc.metadataFlattened, false, XContentType.JSON).v2() ) == false : "API key doc to be updated contains reserved metadata"; + final Map metadata = request.getMetadata(); if (metadata != null) { logger.trace(() -> format("Building API key doc with updated metadata [{}]", metadata)); builder.field("metadata_flattened", metadata); @@ -493,6 +501,90 @@ static XContentBuilder buildUpdatedDocument( return builder.endObject(); } + private boolean isNoop( + final ApiKeyDoc apiKeyDoc, + final Version targetDocVersion, + final Authentication authentication, + final UpdateApiKeyRequest request, + final Set userRoleDescriptors + ) { + if (apiKeyDoc.version != targetDocVersion.id) { + return false; + } + + final Map currentCreator = apiKeyDoc.creator; + final var user = authentication.getEffectiveSubject().getUser(); + final var sourceRealm = authentication.getEffectiveSubject().getRealm(); + if (false == (Objects.equals(user.principal(), currentCreator.get("principal")) + && Objects.equals(user.fullName(), currentCreator.get("full_name")) + && Objects.equals(user.email(), currentCreator.get("email")) + && Objects.equals(user.metadata(), currentCreator.get("metadata")) + && Objects.equals(sourceRealm.getName(), currentCreator.get("realm")) + && Objects.equals(sourceRealm.getType(), currentCreator.get("realm_type")))) { + return false; + } + if (sourceRealm.getDomain() != null) { + if (currentCreator.get("realm_domain") == null) { + return false; + } + @SuppressWarnings("unchecked") + final var currentRealmDomain = RealmDomain.fromXContent( + XContentHelper.mapToXContentParser( + XContentParserConfiguration.EMPTY, + (Map) currentCreator.get("realm_domain") + ) + ); + if (sourceRealm.getDomain().equals(currentRealmDomain) == false) { + return false; + } + } else { + if (currentCreator.get("realm_domain") != null) { + return false; + } + } + + final Map newMetadata = request.getMetadata(); + if (newMetadata != null) { + if (apiKeyDoc.metadataFlattened == null) { + return false; + } + final Map currentMetadata = XContentHelper.convertToMap(apiKeyDoc.metadataFlattened, false, XContentType.JSON) + .v2(); + if (newMetadata.equals(currentMetadata) == false) { + return false; + } + } + + final List newRoleDescriptors = request.getRoleDescriptors(); + if (newRoleDescriptors != null) { + final List currentRoleDescriptors = parseRoleDescriptorsBytes( + request.getId(), + apiKeyDoc.roleDescriptorsBytes, + RoleReference.ApiKeyRoleType.ASSIGNED + ); + if (false == (newRoleDescriptors.size() == currentRoleDescriptors.size() + && Set.copyOf(newRoleDescriptors).containsAll(new HashSet<>(currentRoleDescriptors)))) { + return false; + } + } + + assert userRoleDescriptors != null; + // There is an edge case here when we update an 7.x API key that has a `LEGACY_SUPERUSER_ROLE_DESCRIPTOR` role descriptor: + // `parseRoleDescriptorsBytes` automatically transforms it to `ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR`. As such, when we + // perform the noop check on `ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR` we will treat it as a noop even though the actual + // role descriptor bytes on the API key are different, and correspond to `LEGACY_SUPERUSER_ROLE_DESCRIPTOR`. + // + // This does *not* present a functional issue, since whenever a `LEGACY_SUPERUSER_ROLE_DESCRIPTOR` is loaded at authentication time, + // it is likewise automatically transformed to `ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR`. + final List currentLimitedByRoleDescriptors = parseRoleDescriptorsBytes( + request.getId(), + apiKeyDoc.limitedByRoleDescriptorsBytes, + RoleReference.ApiKeyRoleType.LIMITED_BY + ); + return (userRoleDescriptors.size() == currentLimitedByRoleDescriptors.size() + && userRoleDescriptors.containsAll(currentLimitedByRoleDescriptors)); + } + void tryAuthenticate(ThreadContext ctx, ApiKeyCredentials credentials, ActionListener> listener) { if (false == isEnabled()) { listener.onResponse(AuthenticationResult.notHandled()); @@ -971,6 +1063,68 @@ public void logRemovedField(String parserName, Supplier locati } } + private void doUpdateApiKey( + final Authentication authentication, + final UpdateApiKeyRequest request, + final Set userRoleDescriptors, + final VersionedApiKeyDoc currentVersionedDoc, + final ActionListener listener + ) throws IOException { + logger.trace( + "Building update request for API key doc [{}] with seqNo [{}] and primaryTerm [{}]", + request.getId(), + currentVersionedDoc.seqNo(), + currentVersionedDoc.primaryTerm() + ); + final var targetDocVersion = clusterService.state().nodes().getMinNodeVersion(); + final var currentDocVersion = Version.fromId(currentVersionedDoc.doc().version); + assert currentDocVersion.onOrBefore(targetDocVersion) : "current API key doc version must be on or before target version"; + if (currentDocVersion.before(targetDocVersion)) { + logger.debug( + "API key update for [{}] will update version from [{}] to [{}]", + request.getId(), + currentDocVersion, + targetDocVersion + ); + } + + final XContentBuilder builder = maybeBuildUpdatedDocument( + currentVersionedDoc.doc(), + targetDocVersion, + authentication, + request, + userRoleDescriptors + ); + final boolean isNoop = builder == null; + if (isNoop) { + logger.debug("Detected noop update request for API key [{}]. Skipping index request.", request.getId()); + listener.onResponse(new UpdateApiKeyResponse(false)); + return; + } + + final IndexRequest indexRequest = client.prepareIndex(SECURITY_MAIN_ALIAS) + .setId(request.getId()) + .setSource(builder) + .setIfSeqNo(currentVersionedDoc.seqNo()) + .setIfPrimaryTerm(currentVersionedDoc.primaryTerm()) + .setOpType(DocWriteRequest.OpType.INDEX) + .request(); + logger.trace("Executing index request to update API key [{}]", request.getId()); + securityIndex.prepareIndexIfNeededThenExecute( + listener::onFailure, + () -> executeAsyncWithOrigin( + client.threadPool().getThreadContext(), + SECURITY_ORIGIN, + client.prepareBulk().add(indexRequest).setRefreshPolicy(RefreshPolicy.WAIT_UNTIL).request(), + ActionListener.wrap( + bulkResponse -> translateResponseAndClearCache(request.getId(), bulkResponse, listener), + listener::onFailure + ), + client::bulk + ) + ); + } + /** * Invalidate API keys for given realm, user name, API key name and id. * @param realmNames realm names @@ -1235,63 +1389,11 @@ private static VersionedApiKeyDoc singleDoc(final String apiKeyId, final Collect return elements.iterator().next(); } - private BulkRequest buildBulkRequestForUpdate( - final VersionedApiKeyDoc versionedDoc, - final Authentication authentication, - final UpdateApiKeyRequest request, - final Set userRoles - ) throws IOException { - logger.trace( - "Building update request for API key doc [{}] with seqNo [{}] and primaryTerm [{}]", - request.getId(), - versionedDoc.seqNo(), - versionedDoc.primaryTerm() - ); - final var currentDocVersion = Version.fromId(versionedDoc.doc().version); - final var targetDocVersion = clusterService.state().nodes().getMinNodeVersion(); - assert currentDocVersion.onOrBefore(targetDocVersion) : "current API key doc version must be on or before target version"; - if (currentDocVersion.before(targetDocVersion)) { - logger.debug( - "API key update for [{}] will update version from [{}] to [{}]", - request.getId(), - currentDocVersion, - targetDocVersion - ); - } - final var bulkRequestBuilder = client.prepareBulk(); - bulkRequestBuilder.add( - client.prepareIndex(SECURITY_MAIN_ALIAS) - .setId(request.getId()) - .setSource( - buildUpdatedDocument( - versionedDoc.doc(), - authentication, - userRoles, - request.getRoleDescriptors(), - targetDocVersion, - request.getMetadata() - ) - ) - .setIfSeqNo(versionedDoc.seqNo()) - .setIfPrimaryTerm(versionedDoc.primaryTerm()) - .setOpType(DocWriteRequest.OpType.INDEX) - .request() - ); - bulkRequestBuilder.setRefreshPolicy(RefreshPolicy.WAIT_UNTIL); - return bulkRequestBuilder.request(); - } - - private void executeBulkRequest(final BulkRequest bulkRequest, final ActionListener listener) { - securityIndex.prepareIndexIfNeededThenExecute( - listener::onFailure, - () -> executeAsyncWithOrigin(client.threadPool().getThreadContext(), SECURITY_ORIGIN, bulkRequest, listener, client::bulk) - ); - } - - private static void addLimitedByRoleDescriptors(final XContentBuilder builder, final Set userRoles) throws IOException { - assert userRoles != null; + private static void addLimitedByRoleDescriptors(final XContentBuilder builder, final Set limitedByRoleDescriptors) + throws IOException { + assert limitedByRoleDescriptors != null; builder.startObject("limited_by_role_descriptors"); - for (RoleDescriptor descriptor : userRoles) { + for (RoleDescriptor descriptor : limitedByRoleDescriptors) { builder.field(descriptor.getName(), (contentBuilder, params) -> descriptor.toXContent(contentBuilder, params, true)); } builder.endObject(); 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 181dcf8211283..ae4fa08bc0806 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 @@ -51,6 +51,7 @@ import org.elasticsearch.test.ClusterServiceUtils; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.MockLogAppender; +import org.elasticsearch.test.VersionUtils; import org.elasticsearch.test.XContentTestUtils; import org.elasticsearch.threadpool.FixedExecutorBuilder; import org.elasticsearch.threadpool.TestThreadPool; @@ -68,12 +69,14 @@ import org.elasticsearch.xpack.core.security.action.apikey.CreateApiKeyResponse; import org.elasticsearch.xpack.core.security.action.apikey.GetApiKeyResponse; import org.elasticsearch.xpack.core.security.action.apikey.InvalidateApiKeyResponse; +import org.elasticsearch.xpack.core.security.action.apikey.UpdateApiKeyRequest; import org.elasticsearch.xpack.core.security.authc.Authentication; import org.elasticsearch.xpack.core.security.authc.Authentication.RealmRef; import org.elasticsearch.xpack.core.security.authc.AuthenticationField; import org.elasticsearch.xpack.core.security.authc.AuthenticationResult; import org.elasticsearch.xpack.core.security.authc.AuthenticationTestHelper; import org.elasticsearch.xpack.core.security.authc.AuthenticationTests; +import org.elasticsearch.xpack.core.security.authc.RealmDomain; import org.elasticsearch.xpack.core.security.authc.support.AuthenticationContextSerializer; import org.elasticsearch.xpack.core.security.authc.support.Hasher; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; @@ -1671,91 +1674,135 @@ public void testValidateApiKeyDocBeforeUpdate() throws IOException { assertThat(ex.getMessage(), containsString("cannot update legacy API key [" + apiKeyId + "] without name")); } - public void testBuildUpdatedDocument() throws IOException { + public void testMaybeBuildUpdatedDocument() throws IOException { final var apiKey = randomAlphaOfLength(16); final var hasher = getFastStoredHashAlgoForTests(); final char[] hash = hasher.hash(new SecureString(apiKey.toCharArray())); - - final var oldApiKeyDoc = buildApiKeyDoc(hash, randomBoolean() ? -1 : Instant.now().toEpochMilli(), false); - - final Set newUserRoles = randomBoolean() ? Set.of() : Set.of(RoleDescriptorTests.randomRoleDescriptor()); - - final boolean nullKeyRoles = randomBoolean(); - final List newKeyRoles; - if (nullKeyRoles) { - newKeyRoles = null; - } else { - newKeyRoles = List.of(RoleDescriptorTests.randomRoleDescriptor()); - } - - final var metadata = ApiKeyTests.randomMetadata(); - final var version = Version.CURRENT; - final var authentication = randomValueOtherThanMany( + final var oldAuthentication = randomValueOtherThanMany( Authentication::isApiKey, - () -> AuthenticationTestHelper.builder().user(new User("user", "role")).build(false) - ); - - final var keyDocSource = ApiKeyService.buildUpdatedDocument( - oldApiKeyDoc, - authentication, - newUserRoles, - newKeyRoles, - version, - metadata + () -> AuthenticationTestHelper.builder() + .user(AuthenticationTestHelper.userWithRandomMetadataAndDetails("user", "role")) + .build(false) ); - final var updatedApiKeyDoc = ApiKeyDoc.fromXContent( - XContentHelper.createParser(XContentParserConfiguration.EMPTY, BytesReference.bytes(keyDocSource), XContentType.JSON) + final Set oldUserRoles = randomSet(0, 3, RoleDescriptorTests::randomRoleDescriptor); + final List oldKeyRoles = randomList(3, RoleDescriptorTests::randomRoleDescriptor); + final Map oldMetadata = ApiKeyTests.randomMetadata(); + final Version oldVersion = VersionUtils.randomVersion(random()); + final ApiKeyDoc oldApiKeyDoc = ApiKeyDoc.fromXContent( + XContentHelper.createParser( + XContentParserConfiguration.EMPTY, + BytesReference.bytes( + ApiKeyService.newDocument( + hash, + randomAlphaOfLength(10), + oldAuthentication, + oldUserRoles, + Instant.now(), + randomBoolean() ? null : Instant.now(), + oldKeyRoles, + oldVersion, + oldMetadata + ) + ), + XContentType.JSON + ) ); - assertEquals(oldApiKeyDoc.docType, updatedApiKeyDoc.docType); - assertEquals(oldApiKeyDoc.name, updatedApiKeyDoc.name); - assertEquals(oldApiKeyDoc.hash, updatedApiKeyDoc.hash); - assertEquals(oldApiKeyDoc.expirationTime, updatedApiKeyDoc.expirationTime); - assertEquals(oldApiKeyDoc.creationTime, updatedApiKeyDoc.creationTime); - assertEquals(oldApiKeyDoc.invalidated, updatedApiKeyDoc.invalidated); - - final var service = createApiKeyService(Settings.EMPTY); - final var actualUserRoles = service.parseRoleDescriptorsBytes( - "", - updatedApiKeyDoc.limitedByRoleDescriptorsBytes, - RoleReference.ApiKeyRoleType.LIMITED_BY - ); - assertEquals(newUserRoles.size(), actualUserRoles.size()); - assertEquals(new HashSet<>(newUserRoles), new HashSet<>(actualUserRoles)); + final boolean changeUserRoles = randomBoolean(); + final boolean changeKeyRoles = randomBoolean(); + final boolean changeMetadata = randomBoolean(); + final boolean changeVersion = randomBoolean(); + final boolean changeCreator = randomBoolean(); + final Set newUserRoles = changeUserRoles + ? randomValueOtherThan(oldUserRoles, () -> randomSet(0, 3, RoleDescriptorTests::randomRoleDescriptor)) + : oldUserRoles; + final List newKeyRoles = changeKeyRoles + ? randomValueOtherThan(oldKeyRoles, () -> randomList(0, 3, RoleDescriptorTests::randomRoleDescriptor)) + : (randomBoolean() ? oldKeyRoles : null); + final Map newMetadata = changeMetadata + ? randomValueOtherThanMany(md -> md == null || md.equals(oldMetadata), ApiKeyTests::randomMetadata) + : (randomBoolean() ? oldMetadata : null); + final Version newVersion = changeVersion + ? randomValueOtherThan(oldVersion, () -> VersionUtils.randomVersion(random())) + : oldVersion; + final Authentication newAuthentication = changeCreator + ? randomValueOtherThanMany( + (auth -> auth.isApiKey() || auth.getEffectiveSubject().getUser().equals(oldAuthentication.getEffectiveSubject().getUser())), + () -> AuthenticationTestHelper.builder() + .user(AuthenticationTestHelper.userWithRandomMetadataAndDetails("user", "role")) + .build(false) + ) + : oldAuthentication; + final var request = new UpdateApiKeyRequest(randomAlphaOfLength(10), newKeyRoles, newMetadata); + final var service = createApiKeyService(); - final var actualKeyRoles = service.parseRoleDescriptorsBytes( - "", - updatedApiKeyDoc.roleDescriptorsBytes, - RoleReference.ApiKeyRoleType.ASSIGNED + final XContentBuilder builder = service.maybeBuildUpdatedDocument( + oldApiKeyDoc, + newVersion, + newAuthentication, + request, + newUserRoles ); - if (nullKeyRoles) { - assertEquals( - service.parseRoleDescriptorsBytes("", oldApiKeyDoc.roleDescriptorsBytes, RoleReference.ApiKeyRoleType.ASSIGNED), - actualKeyRoles - ); - } else { - assertEquals(newKeyRoles.size(), actualKeyRoles.size()); - assertEquals(new HashSet<>(newKeyRoles), new HashSet<>(actualKeyRoles)); - } - if (metadata == null) { - assertEquals(oldApiKeyDoc.metadataFlattened, updatedApiKeyDoc.metadataFlattened); - } else { - assertEquals(metadata, XContentHelper.convertToMap(updatedApiKeyDoc.metadataFlattened, true, XContentType.JSON).v2()); - } - assertEquals(authentication.getEffectiveSubject().getUser().principal(), updatedApiKeyDoc.creator.getOrDefault("principal", null)); - assertEquals(authentication.getEffectiveSubject().getUser().fullName(), updatedApiKeyDoc.creator.getOrDefault("fullName", null)); - assertEquals(authentication.getEffectiveSubject().getUser().email(), updatedApiKeyDoc.creator.getOrDefault("email", null)); - assertEquals(authentication.getEffectiveSubject().getUser().metadata(), updatedApiKeyDoc.creator.getOrDefault("metadata", null)); - RealmRef realm = authentication.getEffectiveSubject().getRealm(); - assertEquals(realm.getName(), updatedApiKeyDoc.creator.getOrDefault("realm", null)); - assertEquals(realm.getType(), updatedApiKeyDoc.creator.getOrDefault("realm_type", null)); - if (realm.getDomain() != null) { - @SuppressWarnings("unchecked") - final var actualDomain = (Map) updatedApiKeyDoc.creator.getOrDefault("realm_domain", null); - assertEquals(realm.getDomain().name(), actualDomain.get("name")); + final boolean noop = (changeCreator || changeMetadata || changeKeyRoles || changeUserRoles || changeVersion) == false; + if (noop) { + assertNull(builder); } else { - assertFalse(updatedApiKeyDoc.creator.containsKey("realm_domain")); + final ApiKeyDoc updatedApiKeyDoc = ApiKeyDoc.fromXContent( + XContentHelper.createParser(XContentParserConfiguration.EMPTY, BytesReference.bytes(builder), XContentType.JSON) + ); + assertEquals(oldApiKeyDoc.docType, updatedApiKeyDoc.docType); + assertEquals(oldApiKeyDoc.name, updatedApiKeyDoc.name); + assertEquals(oldApiKeyDoc.hash, updatedApiKeyDoc.hash); + assertEquals(oldApiKeyDoc.expirationTime, updatedApiKeyDoc.expirationTime); + assertEquals(oldApiKeyDoc.creationTime, updatedApiKeyDoc.creationTime); + assertEquals(oldApiKeyDoc.invalidated, updatedApiKeyDoc.invalidated); + assertEquals(newVersion.id, updatedApiKeyDoc.version); + final var actualUserRoles = service.parseRoleDescriptorsBytes( + "", + updatedApiKeyDoc.limitedByRoleDescriptorsBytes, + RoleReference.ApiKeyRoleType.LIMITED_BY + ); + assertEquals(newUserRoles.size(), actualUserRoles.size()); + assertEquals(new HashSet<>(newUserRoles), new HashSet<>(actualUserRoles)); + final var actualKeyRoles = service.parseRoleDescriptorsBytes( + "", + updatedApiKeyDoc.roleDescriptorsBytes, + RoleReference.ApiKeyRoleType.ASSIGNED + ); + if (changeKeyRoles == false) { + assertEquals( + service.parseRoleDescriptorsBytes("", oldApiKeyDoc.roleDescriptorsBytes, RoleReference.ApiKeyRoleType.ASSIGNED), + actualKeyRoles + ); + } else { + assertEquals(newKeyRoles.size(), actualKeyRoles.size()); + assertEquals(new HashSet<>(newKeyRoles), new HashSet<>(actualKeyRoles)); + } + if (changeMetadata == false) { + assertEquals(oldApiKeyDoc.metadataFlattened, updatedApiKeyDoc.metadataFlattened); + } else { + assertEquals(newMetadata, XContentHelper.convertToMap(updatedApiKeyDoc.metadataFlattened, true, XContentType.JSON).v2()); + } + assertEquals(newAuthentication.getEffectiveSubject().getUser().principal(), updatedApiKeyDoc.creator.get("principal")); + assertEquals(newAuthentication.getEffectiveSubject().getUser().fullName(), updatedApiKeyDoc.creator.get("full_name")); + assertEquals(newAuthentication.getEffectiveSubject().getUser().email(), updatedApiKeyDoc.creator.get("email")); + assertEquals(newAuthentication.getEffectiveSubject().getUser().metadata(), updatedApiKeyDoc.creator.get("metadata")); + final RealmRef realm = newAuthentication.getEffectiveSubject().getRealm(); + assertEquals(realm.getName(), updatedApiKeyDoc.creator.get("realm")); + assertEquals(realm.getType(), updatedApiKeyDoc.creator.get("realm_type")); + if (realm.getDomain() != null) { + @SuppressWarnings("unchecked") + final var actualRealmDomain = RealmDomain.fromXContent( + XContentHelper.mapToXContentParser( + XContentParserConfiguration.EMPTY, + (Map) updatedApiKeyDoc.creator.get("realm_domain") + ) + ); + assertEquals(realm.getDomain(), actualRealmDomain); + } else { + assertFalse(updatedApiKeyDoc.creator.containsKey("realm_domain")); + } } } diff --git a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/api_key/30_update.yml b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/api_key/30_update.yml index 013d28113521b..73ff3fba19b46 100644 --- a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/api_key/30_update.yml +++ b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/api_key/30_update.yml @@ -223,7 +223,7 @@ teardown: Authorization: "Basic YXBpX2tleV91c2VyXzE6eC1wYWNrLXRlc3QtcGFzc3dvcmQ=" # api_key_user_1 security.update_api_key: id: "$user1_key_id" - - match: { updated: true } + - match: { updated: false } # Check metadata did not change - do: