gerlowskija commented on code in PR #4474:
URL: https://github.com/apache/solr/pull/4474#discussion_r3605992499
##########
solr/core/src/java/org/apache/solr/security/CertAuthPlugin.java:
##########
@@ -38,6 +38,10 @@ public class CertAuthPlugin extends AuthenticationPlugin {
private static final String PARAM_PRINCIPAL_RESOLVER = "principalResolver";
private static final String PARAM_CLASS = "class";
private static final String PARAM_PARAMS = "params";
+ private static final String JAVAX_REQUEST_ATTRIBUTE_NAME =
Review Comment:
[0] We probably don't want to support reading both properties forever.
Could you put a TODO comment here nudging someone in the future to remove
support for the 'javax' variant on/after Solr 11
##########
solr/core/src/java/org/apache/solr/security/CertAuthPlugin.java:
##########
@@ -102,7 +106,10 @@ public boolean doAuthenticate(
HttpServletRequest request, HttpServletResponse response, FilterChain
filterChain)
throws Exception {
X509Certificate[] certs =
- (X509Certificate[])
request.getAttribute("javax.servlet.request.X509Certificate");
+ (X509Certificate[])
request.getAttribute(JAKARTA_REQUEST_ATTRIBUTE_NAME);
Review Comment:
[Q] Why support both property names in this PR?
AFAIK Solr doesn't aim to support folks swapping out core dependencies like
Jetty, so components like CertAuthPlugin only need to support a single Jetty
version at a time. Is the "fallback" logic serving some purpose that I'm
missing?
##########
changelog/unreleased/SOLR-18270-easy-fix-without-parametrization.yml:
##########
@@ -0,0 +1,8 @@
+title: Update looked up attribute for SSL authentication
Review Comment:
As I understand your JIRA description, CertAuthPlugin is broken entirely in
Solr 10, since it's looking at the wrong attribute name?
If that's correct, then this is a pretty serious bug you're fixing here! We
should tweak the wording to highlight it more strongly. Maybe something like:
```suggestion
title: CertAuthPlugin now uses correct Jetty attribute for certificate lookup
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
