From 6d81698ba4cb9c62185b0a50cc81ddea3047994d Mon Sep 17 00:00:00 2001 From: sciclon2 <74413315+sciclon2@users.noreply.github.com> Date: Wed, 26 Jul 2023 15:48:09 +0200 Subject: [PATCH] KAFKA-15243: Set decoded user names to DescribeUserScramCredentialsResponse (#14094) Reviewers: Manikumar Reddy --- .../scala/kafka/server/ZkAdminManager.scala | 2 +- ...AlterUserScramCredentialsRequestTest.scala | 50 ++++++++++++------- 2 files changed, 32 insertions(+), 20 deletions(-) diff --git a/core/src/main/scala/kafka/server/ZkAdminManager.scala b/core/src/main/scala/kafka/server/ZkAdminManager.scala index 31ab40430d7a..00ef867de9cd 100644 --- a/core/src/main/scala/kafka/server/ZkAdminManager.scala +++ b/core/src/main/scala/kafka/server/ZkAdminManager.scala @@ -848,7 +848,7 @@ class ZkAdminManager(val config: KafkaConfig, try { if (describingAllUsers) adminZkClient.fetchAllEntityConfigs(ConfigType.User).foreach { - case (user, properties) => addToResultsIfHasScramCredential(user, properties) } + case (user, properties) => addToResultsIfHasScramCredential(Sanitizer.desanitize(user), properties) } else { // describing specific users val illegalUsers = users.get.filter(_.isEmpty).toSet diff --git a/core/src/test/scala/unit/kafka/server/AlterUserScramCredentialsRequestTest.scala b/core/src/test/scala/unit/kafka/server/AlterUserScramCredentialsRequestTest.scala index 2503522eea2c..159524c92d1a 100644 --- a/core/src/test/scala/unit/kafka/server/AlterUserScramCredentialsRequestTest.scala +++ b/core/src/test/scala/unit/kafka/server/AlterUserScramCredentialsRequestTest.scala @@ -72,6 +72,7 @@ class AlterUserScramCredentialsRequestTest extends BaseRequestTest { private val saltBytes = "salt".getBytes(StandardCharsets.UTF_8) private val user1 = "user1" private val user2 = "user2" + private val user3 = "user3@user3.com" private val unknownUser = "unknownUser" @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumName) @@ -163,21 +164,21 @@ class AlterUserScramCredentialsRequestTest extends BaseRequestTest { val deletionUnknown1 = new AlterUserScramCredentialsRequestData.ScramCredentialDeletion().setName(user1).setMechanism(ScramMechanism.UNKNOWN.`type`) val deletionValid1 = new AlterUserScramCredentialsRequestData.ScramCredentialDeletion().setName(user1).setMechanism(ScramMechanism.SCRAM_SHA_256.`type`) val deletionUnknown2 = new AlterUserScramCredentialsRequestData.ScramCredentialDeletion().setName(user2).setMechanism(10.toByte) - val user3 = "user3" - val upsertionUnknown3 = new AlterUserScramCredentialsRequestData.ScramCredentialUpsertion().setName(user3).setMechanism(ScramMechanism.UNKNOWN.`type`) - .setIterations(8192).setSalt(saltBytes).setSaltedPassword(saltedPasswordBytes) - val upsertionValid3 = new AlterUserScramCredentialsRequestData.ScramCredentialUpsertion().setName(user3).setMechanism(ScramMechanism.SCRAM_SHA_256.`type`) - .setIterations(8192).setSalt(saltBytes).setSaltedPassword(saltedPasswordBytes) val user4 = "user4" - val upsertionUnknown4 = new AlterUserScramCredentialsRequestData.ScramCredentialUpsertion().setName(user4).setMechanism(10.toByte) + val upsertionUnknown4 = new AlterUserScramCredentialsRequestData.ScramCredentialUpsertion().setName(user4).setMechanism(ScramMechanism.UNKNOWN.`type`) + .setIterations(8192).setSalt(saltBytes).setSaltedPassword(saltedPasswordBytes) + val upsertionValid4 = new AlterUserScramCredentialsRequestData.ScramCredentialUpsertion().setName(user4).setMechanism(ScramMechanism.SCRAM_SHA_256.`type`) .setIterations(8192).setSalt(saltBytes).setSaltedPassword(saltedPasswordBytes) val user5 = "user5" - val upsertionUnknown5 = new AlterUserScramCredentialsRequestData.ScramCredentialUpsertion().setName(user5).setMechanism(ScramMechanism.UNKNOWN.`type`) + val upsertionUnknown5 = new AlterUserScramCredentialsRequestData.ScramCredentialUpsertion().setName(user5).setMechanism(10.toByte) + .setIterations(8192).setSalt(saltBytes).setSaltedPassword(saltedPasswordBytes) + val user6 = "user6" + val upsertionUnknown6 = new AlterUserScramCredentialsRequestData.ScramCredentialUpsertion().setName(user6).setMechanism(ScramMechanism.UNKNOWN.`type`) .setIterations(8192).setSalt(saltBytes).setSaltedPassword(saltedPasswordBytes) val request = new AlterUserScramCredentialsRequest.Builder( new AlterUserScramCredentialsRequestData() .setDeletions(util.Arrays.asList(deletionUnknown1, deletionValid1, deletionUnknown2)) - .setUpsertions(util.Arrays.asList(upsertionUnknown3, upsertionValid3, upsertionUnknown4, upsertionUnknown5))).build() + .setUpsertions(util.Arrays.asList(upsertionUnknown4, upsertionValid4, upsertionUnknown5, upsertionUnknown6))).build() val response = sendAlterUserScramCredentialsRequest(request) val results = response.data.results assertEquals(5, results.size) @@ -278,23 +279,30 @@ class AlterUserScramCredentialsRequestTest extends BaseRequestTest { .setIterations(8192) .setSalt(saltBytes) .setSaltedPassword(saltedPasswordBytes), + new AlterUserScramCredentialsRequestData.ScramCredentialUpsertion() + .setName(user3).setMechanism(ScramMechanism.SCRAM_SHA_512.`type`) + .setIterations(8192) + .setSalt(saltBytes) + .setSaltedPassword(saltedPasswordBytes), ))).build() val results1_1 = sendAlterUserScramCredentialsRequest(request1_1).data.results - assertEquals(2, results1_1.size) + assertEquals(3, results1_1.size) checkNoErrorsAlteringCredentials(results1_1) checkUserAppearsInAlterResults(results1_1, user1) checkUserAppearsInAlterResults(results1_1, user2) + checkUserAppearsInAlterResults(results1_1, user3) - // KRaft is eventually consistent so it is possible to call describe before + // KRaft is eventually consistent so it is possible to call describe before // the credential is propagated from the controller to the broker. - TestUtils.waitUntilTrue(() => describeAllWithNoTopLevelErrorConfirmed().data.results.size == 2, - "describeAllWithNoTopLevelErrorConfirmed does not see 2 users"); + TestUtils.waitUntilTrue(() => describeAllWithNoTopLevelErrorConfirmed().data.results.size == 3, + "describeAllWithNoTopLevelErrorConfirmed does not see 3 users"); // now describe them all val results2 = describeAllWithNoTopLevelErrorConfirmed().data.results - assertEquals(2, results2.size) + assertEquals(3, results2.size) checkUserHasTwoCredentials(results2, user1) checkForSingleSha512Iterations8192Credential(results2, user2) + checkForSingleSha512Iterations8192Credential(results2, user3) // now describe just one val request3 = new DescribeUserScramCredentialsRequest.Builder( @@ -347,25 +355,29 @@ class AlterUserScramCredentialsRequestTest extends BaseRequestTest { checkUserAppearsInAlterResults(results4, user1) checkUserAppearsInAlterResults(results4, user2) - TestUtils.waitUntilTrue(() => describeAllWithNoTopLevelErrorConfirmed().data.results.size == 1, - "describeAllWithNoTopLevelErrorConfirmed does not see only 1 user"); + TestUtils.waitUntilTrue(() => describeAllWithNoTopLevelErrorConfirmed().data.results.size == 2, + "describeAllWithNoTopLevelErrorConfirmed does not see only 2 users"); - // now describe them all, which should just yield 1 credential + // now describe them all, which should just yield 2 credentials val results5 = describeAllWithNoTopLevelErrorConfirmed().data.results - assertEquals(1, results5.size) + assertEquals(2, results5.size) checkForSingleSha512Iterations8192Credential(results5, user1) + checkForSingleSha512Iterations8192Credential(results5, user3) - // now delete the last one + // now delete user1 and user3 val request6 = new AlterUserScramCredentialsRequest.Builder( new AlterUserScramCredentialsRequestData() .setDeletions(util.Arrays.asList( new AlterUserScramCredentialsRequestData.ScramCredentialDeletion() .setName(user1).setMechanism(ScramMechanism.SCRAM_SHA_512.`type`), + new AlterUserScramCredentialsRequestData.ScramCredentialDeletion() + .setName(user3).setMechanism(ScramMechanism.SCRAM_SHA_512.`type`), ))).build() val results6 = sendAlterUserScramCredentialsRequest(request6).data.results - assertEquals(1, results6.size) + assertEquals(2, results6.size) checkNoErrorsAlteringCredentials(results6) checkUserAppearsInAlterResults(results6, user1) + checkUserAppearsInAlterResults(results6, user3) TestUtils.waitUntilTrue(() => describeAllWithNoTopLevelErrorConfirmed().data.results.size == 0, "describeAllWithNoTopLevelErrorConfirmed does not see empty user");