Skip to content

Conversation

@sjweixel
Copy link
Contributor

Set certificate to respond to CertificateRequest for ldaps and starttls requests when LDAP server requires client certificate verification.

Set certificate to respond to CertificateRequest for ldaps and starttls requests when LDAP server requires client certificate verification.
@sjweixel
Copy link
Contributor Author

Please review.

@mposolda
Copy link
Contributor

@sjweixel Thanks for the PR! My vote is that we don't drive by the use of system properties, but instead we properly Keycloak SPI mechanism for this. Perhaps we can add 2 additional optional configuration options like "keystore-file" and "keystore-password" to the TrustStore provider and document it here https://www.keycloak.org/docs/latest/server_installation/index.html#_truststore ?

Could you please send the mail to keycloak-dev mailing list for this topic? This will make sure that we reach bigger audience and have feedback from more people for this topic.

@mposolda mposolda self-assigned this Aug 31, 2021
@mposolda mposolda added the status/needs-discussion PR needs discussion on developer mailing list label Aug 31, 2021
@mposolda
Copy link
Contributor

@sjweixel Did you had a chance to look at this?

@sjweixel
Copy link
Contributor Author

My goal is to make setting the certificate for CertificateRequest easily configurable in keycloak-containers. I think it makes sense in to add these settings in truststore. I just need to be sure that keycloak-containers be able to be modified so CertificateRequest certificate will be as easy to setup in keycloak-containers as setting a tls certificate. I may not have time to look further into this for at least a month.

@mposolda
Copy link
Contributor

@sjweixel Sure, that is fine. FYI. there is another related PR #7365 . If we manage to have that PR merged, this issue will be automatically addressed too IMO.

@sjweixel sjweixel requested a review from a team as a code owner February 21, 2023 23:38
@mposolda
Copy link
Contributor

mposolda commented Aug 7, 2023

@sjweixel Sorry for late response. I am closing as we do not want to use system properties for setup of the keystore. As mentioned above, there is another PR #7365, which has more flexibility and which we might consider to include.

@mposolda mposolda closed this Aug 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status/needs-discussion PR needs discussion on developer mailing list

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants