Make the name of the client certificate attribute which is mapped to roles configurable

[michalvavrik]

closes: #39364

[michalvavrik]

There is difference betweenJDK 17 and JDK21 as far as SAN list is concerned (17 doesn't parse ASN1). I'll fix the CI failure.

[sberyozkin]

Hi @michalvavrik, it is done very comprehensively which is a trademark of your work, it is totally ready to go, and I think having String instead of the enum keeps options open for custom certificate attributes being used as well, though I see now it required you to do some extra parsing.

However, I'd like to suggest that you review which of SAN properties do we really want to support out of the box immediately. We don't usually do some wildcard kind of mappings from the identity to roles, as some of these attributes allow, like mapping any certificate from a given IP address to roles, etc. IMHO only those SAN attributes which can uniquely identify the subject should stay for now. At the very least, trimming them a bit will likely simplify your code and we can grow it further. Think about it please

[michalvavrik]

However, I'd like to suggest that you review which of SAN properties do we really want to support out of the box immediately. We don't usually do some wildcard kind of mappings from the identity to roles, as some of these attributes allow, like mapping any certificate from a given IP address to roles, etc. IMHO only those SAN attributes which can uniquely identify the subject should stay for now. At the very least, trimming them a bit will likely simplify your code and we can grow it further. Think about it please

Sure. I can see users mapping roles from:

• email (RFC822)
• URI (judging by Microsoft tags I googled https://techcommunity.microsoft.com/t5/ask-the-directory-services-team/preview-of-san-uri-for-certificate-strong-mapping-for-kb5014754/ba-p/3789785 they can be used for auth)
• other name (any; same Microsoft link shows they put user principal there)

@cescoffier @sberyozkin please let's share your thoughts on what SANs should be supported, it will limit number of iterations and reworks.

[sberyozkin]

I'd keep those 3 you listed, those which are used in some setups and are unique enough to identity a single subject

[michalvavrik]

I'd keep those 3 you listed, those which are used in some setups and are unique enough to identity a single subject

I have dropped all SANs but URI, ANY and RFC822. IMO we need to wait for @cescoffier feedback when he finds a time.

[sberyozkin]

Sounds good, thanks @michalvavrik. Starting with these SAN attributes is reasonable IMHO

[sberyozkin]

LGTM, waiting for @cescoffier to review as well

[sberyozkin]

@michalvavrik By the way, I forgot to ask, can we use CN instead of DN_CN, the latter option reads quite technical, and clarity in docs it is part of the distinguished name ? So any attribute without SAN prefix or other extension 's prefix belongs to DN... It is not a big deal though, I'll let you decide, thanks

[michalvavrik]

@michalvavrik By the way, I forgot to ask, can we use CN instead of DN_CN, the latter option reads quite technical, and clarity in docs it is part of the distinguished name ? So any attribute without SAN prefix or other extension 's prefix belongs to DN... It is not a big deal though, I'll let you decide, thanks

It's very easy for me to change it, actually it will simplify situation. I don't mind to changing it. In the future, if someone asks for something else than DN/SN, we will need to keep convention that only RDN is not prefixed. Let me change it.

[quarkus-bot]

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit e6441a5.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

---

Flaky tests - Develocity

⚙️ JVM Tests - JDK 21

📦 extensions/smallrye-reactive-messaging-kafka/deployment

✖ io.quarkus.smallrye.reactivemessaging.kafka.deployment.dev.KafkaDevServicesDevModeTestCase.sseStream - History

• Assertion condition defined as a Lambda expression in io.quarkus.smallrye.reactivemessaging.kafka.deployment.dev.KafkaDevServicesDevModeTestCase Expecting size of: [] to be greater than or equal to 2 but was 0 within 10 seconds. - org.awaitility.core.ConditionTimeoutException

org.awaitility.core.ConditionTimeoutException: 
Assertion condition defined as a Lambda expression in io.quarkus.smallrye.reactivemessaging.kafka.deployment.dev.KafkaDevServicesDevModeTestCase 
Expecting size of:
  []
to be greater than or equal to 2 but was 0 within 10 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:119)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:31)

📦 integration-tests/reactive-messaging-kafka

✖ io.quarkus.it.kafka.KafkaConnectorTest.testFruits - History

• Assertion condition defined as a Lambda expression in io.quarkus.it.kafka.KafkaConnectorTest expected: <6> but was: <5> within 10 seconds. - org.awaitility.core.ConditionTimeoutException

org.awaitility.core.ConditionTimeoutException: Assertion condition defined as a Lambda expression in io.quarkus.it.kafka.KafkaConnectorTest expected: <6> but was: <5> within 10 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:119)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:31)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:1006)
	at org.awaitility.core.ConditionFactory.untilAsserted(ConditionFactory.java:790)
	at io.quarkus.it.kafka.KafkaConnectorTest.testFruits(KafkaConnectorTest.java:63)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)

[michalvavrik]

hey @cescoffier , if you have any RFCs in the future when you find a time, just write them down and I'll address them ASAP. Thanks