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 0c67b94bca8af..0a7566746607c 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 @@ -82,6 +82,7 @@ import org.elasticsearch.xpack.core.security.authc.file.FileRealmSettings; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; import org.elasticsearch.xpack.core.security.authz.privilege.ClusterPrivilegeResolver; +import org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore; import org.elasticsearch.xpack.core.security.user.User; import org.elasticsearch.xpack.security.authz.RoleDescriptorTests; import org.elasticsearch.xpack.security.transport.filter.IPFilter; @@ -1871,6 +1872,43 @@ public void testNoopUpdateApiKey() throws ExecutionException, InterruptedExcepti assertTrue(response.isUpdated()); } + public void testUpdateApiKeyAutoUpdatesLegacySuperuserRoleDescriptor() throws ExecutionException, InterruptedException, IOException { + final Tuple> createdApiKey = createApiKey(TEST_USER_NAME, null); + final var apiKeyId = createdApiKey.v1().getId(); + final ServiceWithNodeName serviceWithNodeName = getServiceWithNodeName(); + final Authentication authentication = Authentication.newRealmAuthentication( + new User(TEST_USER_NAME, TEST_ROLE), + new Authentication.RealmRef("file", "file", serviceWithNodeName.nodeName()) + ); + final Set legacySuperuserRoleDescriptor = Set.of(ApiKeyService.LEGACY_SUPERUSER_ROLE_DESCRIPTOR); + PlainActionFuture listener = new PlainActionFuture<>(); + // Force set user role descriptors to 7.x legacy superuser role descriptors + serviceWithNodeName.service() + .updateApiKey(authentication, UpdateApiKeyRequest.usingApiKeyId(apiKeyId), legacySuperuserRoleDescriptor, listener); + UpdateApiKeyResponse response = listener.get(); + assertNotNull(response); + assertTrue(response.isUpdated()); + expectRoleDescriptorsForApiKey("limited_by_role_descriptors", legacySuperuserRoleDescriptor, getApiKeyDocument(apiKeyId)); + + final Set currentSuperuserRoleDescriptors = Set.of(ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR); + PlainActionFuture listener2 = new PlainActionFuture<>(); + serviceWithNodeName.service() + .updateApiKey(authentication, UpdateApiKeyRequest.usingApiKeyId(apiKeyId), currentSuperuserRoleDescriptors, listener2); + response = listener2.get(); + assertNotNull(response); + // The first request is not a noop because we are auto-updating the legacy role descriptors to 8.x role descriptors + assertTrue(response.isUpdated()); + expectRoleDescriptorsForApiKey("limited_by_role_descriptors", currentSuperuserRoleDescriptors, getApiKeyDocument(apiKeyId)); + + PlainActionFuture listener3 = new PlainActionFuture<>(); + serviceWithNodeName.service() + .updateApiKey(authentication, UpdateApiKeyRequest.usingApiKeyId(apiKeyId), currentSuperuserRoleDescriptors, listener3); + response = listener3.get(); + assertNotNull(response); + // Second update is noop because role descriptors were auto-updated by the previous request + assertFalse(response.isUpdated()); + } + public void testUpdateApiKeyClearsApiKeyDocCache() throws IOException, ExecutionException, InterruptedException { final List services = Arrays.stream(internalCluster().getNodeNames()) .map(n -> new ServiceWithNodeName(internalCluster().getInstance(ApiKeyService.class, n), n)) 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 883ad3ca98c19..ab420c30229f3 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,7 +119,6 @@ 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; @@ -445,6 +444,7 @@ static XContentBuilder newDocument( /** * @return `null` if the update is a noop, i.e., if no changes to `currentApiKeyDoc` are required */ + @Nullable XContentBuilder maybeBuildUpdatedDocument( final ApiKeyDoc currentApiKeyDoc, final Version targetDocVersion, @@ -560,26 +560,21 @@ private boolean isNoop( final List currentRoleDescriptors = parseRoleDescriptorsBytes( request.getId(), apiKeyDoc.roleDescriptorsBytes, - RoleReference.ApiKeyRoleType.ASSIGNED + false ); if (false == (newRoleDescriptors.size() == currentRoleDescriptors.size() - && Set.copyOf(newRoleDescriptors).containsAll(new HashSet<>(currentRoleDescriptors)))) { + && Set.copyOf(newRoleDescriptors).containsAll(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 + // We want the 7.x `LEGACY_SUPERUSER_ROLE_DESCRIPTOR` role descriptor to be returned here to auto-update + // `LEGACY_SUPERUSER_ROLE_DESCRIPTOR` to `ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR`, when we update a 7.x API key. + false ); return (userRoleDescriptors.size() == currentLimitedByRoleDescriptors.size() && userRoleDescriptors.containsAll(currentLimitedByRoleDescriptors)); @@ -705,6 +700,14 @@ public List parseRoleDescriptorsBytes( final String apiKeyId, BytesReference bytesReference, RoleReference.ApiKeyRoleType roleType + ) { + return parseRoleDescriptorsBytes(apiKeyId, bytesReference, roleType == RoleReference.ApiKeyRoleType.LIMITED_BY); + } + + private List parseRoleDescriptorsBytes( + final String apiKeyId, + BytesReference bytesReference, + final boolean replaceLegacySuperuserRoleDescriptor ) { if (bytesReference == null) { return Collections.emptyList(); @@ -728,9 +731,7 @@ public List parseRoleDescriptorsBytes( } catch (IOException e) { throw new UncheckedIOException(e); } - return roleType == RoleReference.ApiKeyRoleType.LIMITED_BY - ? maybeReplaceSuperuserRoleDescriptor(apiKeyId, roleDescriptors) - : roleDescriptors; + return replaceLegacySuperuserRoleDescriptor ? maybeReplaceSuperuserRoleDescriptor(apiKeyId, roleDescriptors) : roleDescriptors; } // package private for tests