[java-identity-provider] branch main updated: Reimplement internal X.509 support as a CredentialValidator for reuse.

Scott Cantor cantor.2 at osu.edu
Mon Dec 20 16:11:00 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=19f38a987078be22b4d2d4c70dc27d96280e0d81

The following commit(s) were added to refs/heads/main by this push:
     new 19f38a987 Reimplement internal X.509 support as a CredentialValidator for reuse.
19f38a987 is described below

commit 19f38a987078be22b4d2d4c70dc27d96280e0d81
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Mon Dec 20 11:10:57 2021 -0500

    Reimplement internal X.509 support as a CredentialValidator for reuse.
---
 .../impl/ExtractX509CertificateFromRequest.java    |   6 +-
 ...ava => X509CertificateCredentialValidator.java} | 147 +++++++++++----------
 ...=> X509CertificateCredentialValidatorTest.java} |  43 +++---
 .../idp/flows/authn/x509-internal-authn-beans.xml  |  14 +-
 .../idp/flows/authn/x509-internal-authn-flow.xml   |   1 +
 5 files changed, 111 insertions(+), 100 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 eda51fe6c..ac6e45e75 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
@@ -57,7 +57,7 @@ public class ExtractX509CertificateFromRequest extends AbstractExtractionAction
         
         final HttpServletRequest httpRequest = getHttpServletRequest();
         if (httpRequest == null) {
-            log.debug("{} Profile action does not contain an HttpServletRequest", getLogPrefix());
+            log.warn("{} Profile action does not contain an HttpServletRequest", getLogPrefix());
             ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.NO_CREDENTIALS);
             return;
         }
@@ -69,13 +69,15 @@ public class ExtractX509CertificateFromRequest extends AbstractExtractionAction
             // 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 == 0) {
+            log.info("{} No X.509 certificate found in request", getLogPrefix());
             ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.NO_CREDENTIALS);
             return;
         }
 
+        log.debug("{} {} X.509 Certificate(s) found in request", getLogPrefix(), certs.length);
+        
         final X509Certificate cert = certs[0];
         log.debug("{} End-entity X.509 certificate found with subject '{}', issued by '{}'", getLogPrefix(),
                 cert.getSubjectDN().getName(), cert.getIssuerDN().getName());
diff --git a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/ValidateX509Certificate.java b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/X509CertificateCredentialValidator.java
similarity index 57%
rename from idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/ValidateX509Certificate.java
rename to idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/X509CertificateCredentialValidator.java
index 25fadc35a..c6b6f7149 100644
--- a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/ValidateX509Certificate.java
+++ b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/X509CertificateCredentialValidator.java
@@ -19,21 +19,24 @@ package net.shibboleth.idp.authn.impl;
 
 import java.security.cert.Certificate;
 import java.security.cert.X509Certificate;
+import java.util.function.Function;
 
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import javax.security.auth.Subject;
+import javax.security.auth.login.LoginException;
 
-import net.shibboleth.idp.authn.AbstractValidationAction;
+import net.shibboleth.idp.authn.AbstractCredentialValidator;
 import net.shibboleth.idp.authn.AuthnEventIds;
 import net.shibboleth.idp.authn.context.AuthenticationContext;
 import net.shibboleth.idp.authn.context.CertificateContext;
-import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
+import net.shibboleth.idp.authn.context.UsernamePasswordContext;
+import net.shibboleth.utilities.java.support.annotation.constraint.ThreadSafeAfterInit;
 import net.shibboleth.utilities.java.support.component.ComponentSupport;
+import net.shibboleth.utilities.java.support.logic.Constraint;
 import net.shibboleth.utilities.java.support.resolver.CriteriaSet;
 
-import org.opensaml.profile.action.ActionSupport;
-import org.opensaml.profile.action.EventIds;
+import org.opensaml.messaging.context.navigate.ChildContextLookup;
 import org.opensaml.profile.context.ProfileRequestContext;
 import org.opensaml.security.SecurityException;
 import org.opensaml.security.trust.TrustEngine;
@@ -43,41 +46,41 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 /**
- * An action that checks for a {@link CertificateContext} containing {@link X509Certificate} objects, and
- * directly produces an {@link net.shibboleth.idp.authn.AuthenticationResult} based on that identity, after
- * optionally validating the certificate(s) against a {@link TrustEngine}.
- *  
- * @event {@link EventIds#PROCEED_EVENT_ID}
- * @event {@link AuthnEventIds#INVALID_CREDENTIALS}
- * @event {@link AuthnEventIds#NO_CREDENTIALS}
- * @pre <pre>ProfileRequestContext.getSubcontext(AuthenticationContext.class).getAttemptedFlow() != null</pre>
- * @post If AuthenticationContext.getSubcontext(CertificateContext.class) != null, then
- * an {@link net.shibboleth.idp.authn.AuthenticationResult} is saved to the {@link AuthenticationContext} on a
- * successful validation. On a failure, the
- * {@link AbstractValidationAction#handleError(ProfileRequestContext, AuthenticationContext, Exception, String)}
- * method is called.
+ * A credential validator that validates an X.509 certificate.
+ * 
+ * @since 4.2.0
  */
-public class ValidateX509Certificate extends AbstractValidationAction {
-
-    /** Default prefix for metrics. */
-    @Nonnull @NotEmpty private static final String DEFAULT_METRIC_NAME = "net.shibboleth.idp.authn.x509";
-
+ at ThreadSafeAfterInit
+public class X509CertificateCredentialValidator extends AbstractCredentialValidator {
+    
     /** Class logger. */
-    @Nonnull private final Logger log = LoggerFactory.getLogger(ValidateX509Certificate.class);
+    @Nonnull private final Logger log = LoggerFactory.getLogger(X509CertificateCredentialValidator.class);
+
+    /** Lookup strategy for cert context. */
+    @Nonnull private Function<AuthenticationContext,CertificateContext> certContextLookupStrategy;
 
     /** Optional trust engine to validate certificates against. */
     @Nullable private TrustEngine<? super X509Credential> trustEngine;
     
-    /** CertificateContext containing the credentials to validate. */
-    @Nullable private CertificateContext certContext;
-    
     /** Whether to save the certificate in the Java Subject's public credentials. */
     private boolean saveCertificateToCredentialSet;
     
     /** Constructor. */
-    public ValidateX509Certificate() {
-        setMetricName(DEFAULT_METRIC_NAME);
-        saveCertificateToCredentialSet = true;
+    public X509CertificateCredentialValidator() {
+        certContextLookupStrategy = new ChildContextLookup<>(CertificateContext.class);
+    }
+    
+    /**
+     * Set the lookup strategy to locate the {@link UsernamePasswordContext}.
+     * 
+     * @param strategy lookup strategy
+     */
+    public void setCertificateContextLookupStrategy(
+            @Nonnull final Function<AuthenticationContext,CertificateContext> strategy) {
+        ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+        
+        certContextLookupStrategy = Constraint.isNotNull(strategy,
+                "CertificateContextLookupStrategy cannot be null");
     }
     
     /**
@@ -97,45 +100,30 @@ public class ValidateX509Certificate extends AbstractValidationAction {
      * <p>Defaults to true</p>
      * 
      * @param flag flag to set
-     * 
-     * @since 4.1.0
      */
     public void setSaveCertificateToCredentialSet(final boolean flag) {
         ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
         
         saveCertificateToCredentialSet = flag;
     }
-    
+
+// Checkstyle: CyclomaticComplexity OFF
     /** {@inheritDoc} */
     @Override
-    protected boolean doPreExecute(@Nonnull final ProfileRequestContext profileRequestContext,
-            @Nonnull final AuthenticationContext authenticationContext) {
-        
-        if (!super.doPreExecute(profileRequestContext, authenticationContext)) {
-            return false;
-        }
-        
-        certContext = authenticationContext.getSubcontext(CertificateContext.class);
+    @Nullable protected Subject doValidate(@Nonnull final ProfileRequestContext profileRequestContext,
+            @Nonnull final AuthenticationContext authenticationContext,
+            @Nullable final WarningHandler warningHandler,
+            @Nullable final ErrorHandler errorHandler) throws Exception {
+
+        final CertificateContext certContext = certContextLookupStrategy.apply(authenticationContext);
         if (certContext == null) {
-            log.info("{} No CertificateContext available within authentication context", getLogPrefix());
-            handleError(profileRequestContext, authenticationContext, AuthnEventIds.NO_CREDENTIALS,
-                    AuthnEventIds.NO_CREDENTIALS);
-            return false;
+            log.debug("{} No CertificateContext available within authentication context", getLogPrefix());
+            return null;
         } else if (certContext.getCertificate() == null || !(certContext.getCertificate() instanceof X509Certificate)) {
-            log.info("{} No X.509 certificate available within CertificateContext", getLogPrefix());
-            handleError(profileRequestContext, authenticationContext, AuthnEventIds.NO_CREDENTIALS,
-                    AuthnEventIds.NO_CREDENTIALS);
-            return false;
+            log.debug("{} No X.509 certificate available within CertificateContext", getLogPrefix());
+            return null;
         }
-        
-        return true;
-    }
 
-    /** {@inheritDoc} */
-    @Override
-    protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext,
-            @Nonnull final AuthenticationContext authenticationContext) {
-        
         if (trustEngine != null) {
             log.debug("{} Attempting to validate certificate using trust engine", getLogPrefix());
             try {
@@ -153,16 +141,20 @@ public class ValidateX509Certificate extends AbstractValidationAction {
                     log.debug("{} Trust engine validated X.509 certificate", getLogPrefix());
                 } else {
                     log.warn("{} Trust engine failed to validate X.509 certificate", getLogPrefix());
-                    handleError(profileRequestContext, authenticationContext, AuthnEventIds.INVALID_CREDENTIALS,
-                            AuthnEventIds.INVALID_CREDENTIALS);
-                    recordFailure(profileRequestContext);
-                    return;
+                    final LoginException e = new LoginException(AuthnEventIds.INVALID_CREDENTIALS);
+                    if (errorHandler != null) {
+                        errorHandler.handleError(profileRequestContext, authenticationContext, e,
+                                AuthnEventIds.INVALID_CREDENTIALS);
+                    }
+                    throw e;
                 }
             } catch (final SecurityException e) {
                 log.error("{} Exception raised by trust engine", getLogPrefix(), e);
-                handleError(profileRequestContext, authenticationContext, e, AuthnEventIds.INVALID_CREDENTIALS);
-                recordFailure(profileRequestContext);
-                return;
+                if (errorHandler != null) {
+                    errorHandler.handleError(profileRequestContext, authenticationContext, e,
+                            AuthnEventIds.INVALID_CREDENTIALS);
+                }
+                throw e;
             }
         } else {
             log.debug("{} No trust engine configured, certificate will be trusted", getLogPrefix());
@@ -170,19 +162,28 @@ public class ValidateX509Certificate extends AbstractValidationAction {
 
         log.info("{} Login by '{}' succeeded", getLogPrefix(),
                 ((X509Certificate) certContext.getCertificate()).getSubjectX500Principal().getName());
-        recordSuccess(profileRequestContext);
-        buildAuthenticationResult(profileRequestContext, authenticationContext);
-        ActionSupport.buildProceedEvent(profileRequestContext);
+        
+        return populateSubject((X509Certificate) certContext.getCertificate());
     }
-
-    /** {@inheritDoc} */
-    @Override
-    @Nonnull protected Subject populateSubject(@Nonnull final Subject subject) {
-        subject.getPrincipals().add(((X509Certificate) certContext.getCertificate()).getSubjectX500Principal());
+// Checkstyle: CyclomaticComplexity ON
+    
+    /**
+     * Builds a subject with "standard" content from the validation.
+     *
+     * @param certificate the certificate validated
+     * 
+     * @return the decorated subject
+     */
+    @Nonnull protected Subject populateSubject(@Nonnull final X509Certificate certificate) {
+       
+        final Subject subject = new Subject();
+       
+        subject.getPrincipals().add(certificate.getSubjectX500Principal());
         if (saveCertificateToCredentialSet) {
-            subject.getPublicCredentials().add(certContext.getCertificate());
+            subject.getPublicCredentials().add(certificate);
         }
-        return subject;
+       
+        return super.populateSubject(subject);
     }
 
 }
\ No newline at end of file
diff --git a/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/ValidateX509CertificateTest.java b/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/X509CertificateCredentialValidatorTest.java
similarity index 91%
rename from idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/ValidateX509CertificateTest.java
rename to idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/X509CertificateCredentialValidatorTest.java
index 58ec8f39a..734aa48b8 100644
--- a/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/ValidateX509CertificateTest.java
+++ b/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/X509CertificateCredentialValidatorTest.java
@@ -20,9 +20,9 @@ package net.shibboleth.idp.authn.impl;
 
 import java.security.cert.CertificateException;
 import java.security.cert.X509Certificate;
+import java.util.Collections;
 
 import javax.security.auth.x500.X500Principal;
-import javax.servlet.http.HttpServletRequest;
 
 import net.shibboleth.idp.authn.AuthnEventIds;
 import net.shibboleth.idp.authn.context.AuthenticationContext;
@@ -43,8 +43,8 @@ import org.testng.Assert;
 import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
-/** {@link ValidateX509Certificate} unit test. */
-public class ValidateX509CertificateTest extends BaseAuthenticationContextTest {
+/** {@link X509CertificateCredentialValidator} unit test. */
+public class X509CertificateCredentialValidatorTest extends BaseAuthenticationContextTest {
     
     private String entityCertBase64 = 
             "MIIDjDCCAnSgAwIBAgIBKjANBgkqhkiG9w0BAQUFADAtMRIwEAYDVQQKEwlJbnRl" +
@@ -91,13 +91,18 @@ public class ValidateX509CertificateTest extends BaseAuthenticationContextTest {
             "Zy+LbvWg3urUkiDjMcB6nGImmEfDSxRdybitcMwbwL26z2WOpwL3llm3mcCydKXg" +
             "Xt8IQhfDhOZOHWckeD2tStnJRP/cqBgO62/qirw=";
     
-    private ValidateX509Certificate action; 
+    private X509CertificateCredentialValidator validator;
+    private ValidateCredentials action;
     
     @BeforeMethod public void setUp() throws ComponentInitializationException {
         super.setUp();
         
-        action = new ValidateX509Certificate();
-        action.setHttpServletRequest((HttpServletRequest) src.getExternalContext().getNativeRequest());
+        validator = new X509CertificateCredentialValidator();
+        validator.setId("x509");
+        
+        action = new ValidateCredentials();
+        action.setValidators(Collections.singletonList(validator));
+        action.setHttpServletRequest(new MockHttpServletRequest());
         action.initialize();
     }
 
@@ -106,10 +111,13 @@ public class ValidateX509CertificateTest extends BaseAuthenticationContextTest {
         ActionTestingSupport.assertEvent(event, AuthnEventIds.INVALID_AUTHN_CTX);
     }
     
-    @Test public void testMissingCert() {
+    @Test public void testMissingCert() throws ComponentInitializationException {
         prc.getSubcontext(AuthenticationContext.class).setAttemptedFlow(authenticationFlows.get(0));
+        
+        validator.initialize();
+        
         final Event event = action.execute(src);
-        ActionTestingSupport.assertEvent(event, AuthnEventIds.NO_CREDENTIALS);
+        ActionTestingSupport.assertEvent(event, AuthnEventIds.REQUEST_UNSUPPORTED);
     }
 
     @Test public void testNoTrustEngine() throws ComponentInitializationException, CertificateException {
@@ -120,7 +128,9 @@ public class ValidateX509CertificateTest extends BaseAuthenticationContextTest {
         
         final AuthenticationContext ac = prc.getSubcontext(AuthenticationContext.class);
         ac.setAttemptedFlow(authenticationFlows.get(0));
-        
+
+        validator.initialize();
+
         doExtract();
         
         final Event event = action.execute(src);
@@ -137,11 +147,8 @@ public class ValidateX509CertificateTest extends BaseAuthenticationContextTest {
         final CredentialResolver resolver = new StaticCredentialResolver(new BasicX509Credential(entityCert));
         final TrustEngine<X509Credential> engine = new ExplicitX509CertificateTrustEngine(resolver);
         
-        action = new ValidateX509Certificate();
-        action.setTrustEngine(engine);
-        action.setHttpServletRequest((HttpServletRequest) src.getExternalContext().getNativeRequest());
-        
-        action.initialize();
+        validator.setTrustEngine(engine);
+        validator.initialize();
 
         ((MockHttpServletRequest) action.getHttpServletRequest()).setAttribute("javax.servlet.request.X509Certificate", certs);
 
@@ -167,15 +174,11 @@ public class ValidateX509CertificateTest extends BaseAuthenticationContextTest {
         final CredentialResolver resolver = new StaticCredentialResolver(new BasicX509Credential(otherCert1));
         final TrustEngine<X509Credential> engine = new ExplicitX509CertificateTrustEngine(resolver);
         
-        action = new ValidateX509Certificate();
-        action.setTrustEngine(engine);
-        action.setHttpServletRequest((HttpServletRequest) src.getExternalContext().getNativeRequest());
+        validator.setTrustEngine(engine);
+        validator.initialize();
         
-        action.initialize();
-
         ((MockHttpServletRequest) action.getHttpServletRequest()).setAttribute("javax.servlet.request.X509Certificate", certs);
 
-        
         final AuthenticationContext ac = prc.getSubcontext(AuthenticationContext.class);
         ac.setAttemptedFlow(authenticationFlows.get(0));
         
diff --git a/idp-conf-impl/src/main/resources/net/shibboleth/idp/flows/authn/x509-internal-authn-beans.xml b/idp-conf-impl/src/main/resources/net/shibboleth/idp/flows/authn/x509-internal-authn-beans.xml
index 52ee86c27..a101bbe0e 100644
--- a/idp-conf-impl/src/main/resources/net/shibboleth/idp/flows/authn/x509-internal-authn-beans.xml
+++ b/idp-conf-impl/src/main/resources/net/shibboleth/idp/flows/authn/x509-internal-authn-beans.xml
@@ -25,11 +25,15 @@
         p:httpServletRequest-ref="shibboleth.HttpServletRequest" />
         
     <bean id="ValidateX509Certificate"
-        class="net.shibboleth.idp.authn.impl.ValidateX509Certificate" scope="prototype"
-        p:trustEngine="#{getObject('shibboleth.authn.X509.TrustEngine')}"
-        p:saveCertificateToCredentialSet="%{idp.authn.X509Internal.saveCertificateToCredentialSet:true}"
-        p:addDefaultPrincipals="#{getObject('shibboleth.authn.X509.addDefaultPrincipals') ?: %{idp.authn.X509Internal.addDefaultPrincipals:true}}"
-        p:resultCachingPredicate="#{getObject('shibboleth.authn.X509.resultCachingPredicate')}" />
+            class="net.shibboleth.idp.authn.impl.ValidateCredentials" scope="prototype"
+            p:addDefaultPrincipals="#{getObject('shibboleth.authn.X509.addDefaultPrincipals') ?: %{idp.authn.X509Internal.addDefaultPrincipals:true}}"
+            p:resultCachingPredicate="#{getObject('shibboleth.authn.X509.resultCachingPredicate')}">
+        <property name="validators">
+            <bean id="x509" class="net.shibboleth.idp.authn.impl.X509CertificateCredentialValidator"
+                p:trustEngine="#{getObject('shibboleth.authn.X509.TrustEngine')}"
+                p:saveCertificateToCredentialSet="%{idp.authn.X509Internal.saveCertificateToCredentialSet:true}" />
+        </property>    
+    </bean>
 
     <bean id="PopulateSubjectCanonicalizationContext"
         class="net.shibboleth.idp.authn.impl.PopulateSubjectCanonicalizationContext" scope="prototype"
diff --git a/idp-conf-impl/src/main/resources/net/shibboleth/idp/flows/authn/x509-internal-authn-flow.xml b/idp-conf-impl/src/main/resources/net/shibboleth/idp/flows/authn/x509-internal-authn-flow.xml
index 29b5bda52..efa6b5214 100644
--- a/idp-conf-impl/src/main/resources/net/shibboleth/idp/flows/authn/x509-internal-authn-flow.xml
+++ b/idp-conf-impl/src/main/resources/net/shibboleth/idp/flows/authn/x509-internal-authn-flow.xml
@@ -27,6 +27,7 @@
     <global-transitions>
         <transition on="NoCredentials" to="ReselectFlow" />
         <transition on="InvalidCredentials" to="ReselectFlow" />
+        <transition on="RequestUnsupported" to="ReselectFlow" />
     </global-transitions>
 
     <bean-import resource="x509-internal-authn-beans.xml" />

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


More information about the commits mailing list