Skip to content

Commit

Permalink
KAFKA-15243: Set decoded user names to DescribeUserScramCredentialsRe…
Browse files Browse the repository at this point in the history 
…sponse (apache#14094)


Reviewers: Manikumar Reddy <manikumar.reddy@gmail.com>
  • Loading branch information
@sciclon2
sciclon2 committed Jul 26, 2023
1 parent ff390ab commit 6d81698
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 20 deletions.
2 changes: 1 addition & 1 deletion core/src/main/scala/kafka/server/ZkAdminManager.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
50 changes: 31 additions & 19 deletions core/src/test/scala/unit/kafka/server/AlterUserScramCredentialsRequestTest.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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");
Expand Down

0 comments on commit 6d81698

Please sign in to comment.