[java-identity-provider] branch main updated: IDP-1799 - X.509 certificate access should check jakarta attribute

Scott Cantor cantor.2 at osu.edu
Wed Apr 21 13:54:57 UTC 2021


This is an automated email from the git hooks/post-receive script.

scantor pushed a commit to branch main
in repository java-identity-provider.

View the commit online:
http://git.shibboleth.net/view/?p=java-identity-provider.git;a=commit;h=b53b8c843a0ea96de2f559ab861668143e8e084a

The following commit(s) were added to refs/heads/main by this push:
       new  b53b8c843 IDP-1799 - X.509 certificate access should check jakarta attribute
b53b8c843 is described below

commit b53b8c843a0ea96de2f559ab861668143e8e084a
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Wed Apr 21 09:54:54 2021 -0400

    IDP-1799 - X.509 certificate access should check jakarta attribute
    
    https://issues.shibboleth.net/jira/browse/IDP-1799
---
 .../impl/ExtractX509CertificateFromRequest.java     | 18 +++++++++++-------
 .../shibboleth/idp/authn/impl/X509AuthServlet.java  |  9 +++++++--
 .../shibboleth/idp/authn/impl/X509ProxyFilter.java  |  7 ++++++-
 .../impl/ExtractX509CertificateFromRequestTest.java | 21 ++++++++++++++++++++-
 4 files changed, 44 insertions(+), 11 deletions(-)

diff --git a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/ExtractX509CertificateFromRequest.java b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/ExtractX509CertificateFromRequest.java
index 8a5ea5aab..eda51fe6c 100644
--- a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/ExtractX509CertificateFromRequest.java
+++ b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/ExtractX509CertificateFromRequest.java
@@ -48,7 +48,6 @@ public class ExtractX509CertificateFromRequest extends AbstractExtractionAction
     @Nonnull private final Logger log = LoggerFactory.getLogger(ExtractX509CertificateFromRequest.class);
     
     /** {@inheritDoc} */
-    // CheckStyle: ReturnCount OFF
     @Override
     protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext,
             @Nonnull final AuthenticationContext authenticationContext) {
@@ -56,18 +55,23 @@ public class ExtractX509CertificateFromRequest extends AbstractExtractionAction
         final CertificateContext certCtx = new CertificateContext();
         authenticationContext.addSubcontext(certCtx, true);
         
-        final HttpServletRequest request = getHttpServletRequest();
-        if (request == null) {
+        final HttpServletRequest httpRequest = getHttpServletRequest();
+        if (httpRequest == null) {
             log.debug("{} Profile action does not contain an HttpServletRequest", getLogPrefix());
             ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.NO_CREDENTIALS);
             return;
         }
         
-        final X509Certificate[] certs =
-                (X509Certificate[]) request.getAttribute("javax.servlet.request.X509Certificate");
+        X509Certificate[] certs =
+                (X509Certificate[]) httpRequest.getAttribute("javax.servlet.request.X509Certificate");
+        if (certs == null || certs.length == 0) {
+            // Check for newer Jakarta variant.
+            // TODO: Once Jakarta is "common", probably reverse these checks.
+            certs = (X509Certificate[]) httpRequest.getAttribute("jakarta.servlet.request.X509Certificate");
+        }
         log.debug("{} {} X.509 Certificate(s) found in request", getLogPrefix(), certs != null ? certs.length : 0);
 
-        if (certs == null || certs.length < 1) {
+        if (certs == null || certs.length == 0) {
             ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.NO_CREDENTIALS);
             return;
         }
@@ -83,5 +87,5 @@ public class ExtractX509CertificateFromRequest extends AbstractExtractionAction
             certCtx.getIntermediates().add(certs[i]);
         }
     }
-    // CheckStyle: ReturnCount ON
+
 }
\ No newline at end of file
diff --git a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/X509AuthServlet.java b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/X509AuthServlet.java
index ab19a6aa0..515b16498 100644
--- a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/X509AuthServlet.java
+++ b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/X509AuthServlet.java
@@ -135,11 +135,16 @@ public class X509AuthServlet extends HttpServlet {
         try {
             final String key = ExternalAuthentication.startExternalAuthentication(httpRequest);
             
-            final X509Certificate[] certs =
+            X509Certificate[] certs =
                     (X509Certificate[]) httpRequest.getAttribute("javax.servlet.request.X509Certificate");
+            if (certs == null || certs.length == 0) {
+                // Check for newer Jakarta variant.
+                // TODO: Once Jakarta is "common", probably reverse these checks.
+                certs = (X509Certificate[]) httpRequest.getAttribute("jakarta.servlet.request.X509Certificate");
+            }
             log.debug("{} X.509 Certificate(s) found in request", certs != null ? certs.length : 0);
 
-            if (certs == null || certs.length < 1) {
+            if (certs == null || certs.length == 0) {
                 log.error("No X.509 Certificates found in request");
                 httpRequest.setAttribute(ExternalAuthentication.AUTHENTICATION_ERROR_KEY, AuthnEventIds.NO_CREDENTIALS);
                 ExternalAuthentication.finishExternalAuthentication(key, httpRequest, httpResponse);
diff --git a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/X509ProxyFilter.java b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/X509ProxyFilter.java
index ace3af8fc..b90c03f39 100644
--- a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/X509ProxyFilter.java
+++ b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/X509ProxyFilter.java
@@ -93,8 +93,11 @@ public class X509ProxyFilter implements Filter {
         final HttpServletRequest httpRequest = (HttpServletRequest) request;
                 
         try {
-            final X509Certificate[] certs =
+            X509Certificate[] certs =
                     (X509Certificate[]) request.getAttribute("javax.servlet.request.X509Certificate");
+            if (null == certs || 0 == certs.length) {
+                certs = (X509Certificate[]) request.getAttribute("jakarta.servlet.request.X509Certificate");
+            }
             
             if (null == certs || 0 == certs.length) {
                 final List<X509Certificate> proxyCerts = new ArrayList<>();
@@ -120,6 +123,8 @@ public class X509ProxyFilter implements Filter {
                 }
                 
                 if (!proxyCerts.isEmpty()) {
+                    // TODO: I guess we'd check the class name(s) we're using here to
+                    // know which attribute to populate?
                     request.setAttribute("javax.servlet.request.X509Certificate",
                             proxyCerts.toArray(new X509Certificate[proxyCerts.size()]));
                 }
diff --git a/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/ExtractX509CertificateFromRequestTest.java b/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/ExtractX509CertificateFromRequestTest.java
index 5eb902e5d..582de652f 100644
--- a/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/ExtractX509CertificateFromRequestTest.java
+++ b/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/ExtractX509CertificateFromRequestTest.java
@@ -105,7 +105,7 @@ public class ExtractX509CertificateFromRequestTest extends BaseAuthenticationCon
         ActionTestingSupport.assertEvent(event, AuthnEventIds.NO_CREDENTIALS);
     }
     
-    @Test public void testValid() throws Exception {
+    @Test public void testValidJava() throws Exception {
         
         final X509Certificate entityCert = X509Support.decodeCertificate(entityCertBase64);
         final X509Certificate otherCert1 = X509Support.decodeCertificate(otherCert1Base64);
@@ -124,4 +124,23 @@ public class ExtractX509CertificateFromRequestTest extends BaseAuthenticationCon
         Assert.assertSame(certCtx.getIntermediates().iterator().next(), otherCert1);
     }
     
+    @Test public void testValidJakarta() throws Exception {
+        
+        final X509Certificate entityCert = X509Support.decodeCertificate(entityCertBase64);
+        final X509Certificate otherCert1 = X509Support.decodeCertificate(otherCert1Base64);
+        
+        final X509Certificate[] certs = new X509Certificate[]{entityCert, otherCert1};
+        
+        ((MockHttpServletRequest) action.getHttpServletRequest()).setAttribute("jakarta.servlet.request.X509Certificate", certs);
+        
+        final Event event = action.execute(src);
+        ActionTestingSupport.assertProceedEvent(event);
+        final AuthenticationContext authCtx = prc.getSubcontext(AuthenticationContext.class);
+        final CertificateContext certCtx = authCtx.getSubcontext(CertificateContext.class);
+        Assert.assertNotNull(certCtx, "No CertificateContext attached");
+        
+        Assert.assertSame(certCtx.getCertificate(), entityCert);
+        Assert.assertSame(certCtx.getIntermediates().iterator().next(), otherCert1);
+    }
+
 }
\ No newline at end of file

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list