[java-identity-provider] branch main updated: IDP-2480 - Add logging to isAcceptable machinery in authn layer

Codeberg noreply at shibboleth.net
Wed Sep 23 17:41:13 UTC 2026


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

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

View the commit online:
https://codeberg.org/Shibboleth/java-identity-provider/commit/f692672008caf8de01b31284ded4c0e0f08e04a8

The following commit(s) were added to refs/heads/main by this push:
     new f69267200 IDP-2480 - Add logging to isAcceptable machinery in authn layer
f69267200 is described below

commit f692672008caf8de01b31284ded4c0e0f08e04a8
Author: Scott Cantor <scott at restingparrotsoftware.com>
AuthorDate: Wed Sep 23 13:40:33 2026 -0400

    IDP-2480 - Add logging to isAcceptable machinery in authn layer
    
    https://shibboleth.atlassian.net/browse/IDP-2480
    
    Push logging of null operator into registry to save work.
---
 .../idp/authn/AbstractCredentialValidator.java     | 50 +++++++++++-----------
 .../idp/authn/AbstractValidationAction.java        | 40 ++++++++---------
 .../authn/context/RequestedPrincipalContext.java   |  8 +---
 .../PrincipalEvalPredicateFactoryRegistry.java     | 12 ++++--
 .../idp/authn/impl/FinalizeAuthentication.java     |  3 +-
 5 files changed, 54 insertions(+), 59 deletions(-)

diff --git a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/AbstractCredentialValidator.java b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/AbstractCredentialValidator.java
index c0a1bc544..e7aa785da 100644
--- a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/AbstractCredentialValidator.java
+++ b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/AbstractCredentialValidator.java
@@ -199,35 +199,33 @@ public abstract class AbstractCredentialValidator extends AbstractIdentifiedInit
         
         if (subject != null && requestedPrincipalCtx != null) {
             final String operator = requestedPrincipalCtx.getOperator();
-            if (operator != null) {
-                log.debug("{} Request contains principal requirements, checking validator '{}' for compatibility",
-                        getLogPrefix(), configName);
-                for (final Principal p : requestedPrincipalCtx.getRequestedPrincipals()) {
-                    final PrincipalEvalPredicateFactory factory =
-                            requestedPrincipalCtx.getPrincipalEvalPredicateFactoryRegistry().lookup(
-                                    p.getClass(), operator);
-                    if (factory != null) {
-                        final PrincipalEvalPredicate predicate = factory.getPredicate(p);
-                        final PrincipalSupportingComponent wrapper = new PrincipalSupportingComponent() {
-                            @Nonnull
-                            public <T extends Principal> Set<T> getSupportedPrincipals(@Nonnull final Class<T> c) {
-                                Set<T> principals = subject.getPrincipals(c);
-                                assert principals!=null;
-                                return principals;
-                            }
-                        };
-                        if (predicate.test(wrapper)) {
-                            log.debug("{} Validator '{}' compatible with principal type '{}' and operator '{}'",
-                                    getLogPrefix(), configName, p.getClass(), requestedPrincipalCtx.getOperator());
-                            requestedPrincipalCtx.setMatchingPrincipal(predicate.getMatchingPrincipal());
-                            return true;
+            log.debug("{} Request contains principal requirements, checking validator '{}' for compatibility",
+                    getLogPrefix(), configName);
+            for (final Principal p : requestedPrincipalCtx.getRequestedPrincipals()) {
+                final PrincipalEvalPredicateFactory factory =
+                        requestedPrincipalCtx.getPrincipalEvalPredicateFactoryRegistry().lookup(
+                                p.getClass(), operator);
+                if (factory != null) {
+                    final PrincipalEvalPredicate predicate = factory.getPredicate(p);
+                    final PrincipalSupportingComponent wrapper = new PrincipalSupportingComponent() {
+                        @Nonnull
+                        public <T extends Principal> Set<T> getSupportedPrincipals(@Nonnull final Class<T> c) {
+                            Set<T> principals = subject.getPrincipals(c);
+                            assert principals!=null;
+                            return principals;
                         }
-                        log.debug("{} Validator '{}' not compatible with principal type '{}' and operator '{}'",
+                    };
+                    if (predicate.test(wrapper)) {
+                        log.debug("{} Validator '{}' compatible with principal type '{}' and operator '{}'",
                                 getLogPrefix(), configName, p.getClass(), requestedPrincipalCtx.getOperator());
-                    } else {
-                        log.debug("{} No comparison logic registered for principal type '{}' and operator '{}'",
-                                getLogPrefix(), p.getClass(), requestedPrincipalCtx.getOperator());
+                        requestedPrincipalCtx.setMatchingPrincipal(predicate.getMatchingPrincipal());
+                        return true;
                     }
+                    log.debug("{} Validator '{}' not compatible with principal type '{}' and operator '{}'",
+                            getLogPrefix(), configName, p.getClass(), requestedPrincipalCtx.getOperator());
+                } else {
+                    log.debug("{} No comparison logic registered for principal type '{}' and operator '{}'",
+                            getLogPrefix(), p.getClass(), requestedPrincipalCtx.getOperator());
                 }
             }
             
diff --git a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/AbstractValidationAction.java b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/AbstractValidationAction.java
index b7c43b13c..2a7dc3d52 100644
--- a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/AbstractValidationAction.java
+++ b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/AbstractValidationAction.java
@@ -337,31 +337,29 @@ public abstract class AbstractValidationAction extends AbstractAuthenticationAct
         final RequestedPrincipalContext rpCtx = authenticationContext.getSubcontext(RequestedPrincipalContext.class);
         if (rpCtx != null && !getSubject().getPrincipals().isEmpty()) {
             final String operator = rpCtx.getOperator();
-            if (operator != null) {
-                log.debug("{} Request contains principal requirements, evaluating for compatibility", getLogPrefix());
-                for (final Principal p : rpCtx.getRequestedPrincipals()) {
-                    final PrincipalEvalPredicateFactory factory =
-                            rpCtx.getPrincipalEvalPredicateFactoryRegistry().lookup(p.getClass(), operator);
-                    if (factory != null) {
-                        final PrincipalEvalPredicate predicate = factory.getPredicate(p);
-                        if (predicate.test(this)) {
-                            log.debug("{} Compatible with principal type '{}' and operator '{}'", getLogPrefix(),
-                                    p.getClass(), operator);
-                            rpCtx.setMatchingPrincipal(predicate.getMatchingPrincipal());
-                            return true;
-                        }
-                        log.debug("{} Not compatible with principal type '{}' and operator '{}'", getLogPrefix(),
+            log.debug("{} Request contains principal requirements, evaluating for compatibility", getLogPrefix());
+            for (final Principal p : rpCtx.getRequestedPrincipals()) {
+                final PrincipalEvalPredicateFactory factory =
+                        rpCtx.getPrincipalEvalPredicateFactoryRegistry().lookup(p.getClass(), operator);
+                if (factory != null) {
+                    final PrincipalEvalPredicate predicate = factory.getPredicate(p);
+                    if (predicate.test(this)) {
+                        log.debug("{} Compatible with principal type '{}' and operator '{}'", getLogPrefix(),
                                 p.getClass(), operator);
-                    } else {
-                        log.debug("{} No comparison logic registered for principal type '{}' and operator '{}'",
-                                getLogPrefix(), p.getClass(), operator);
+                        rpCtx.setMatchingPrincipal(predicate.getMatchingPrincipal());
+                        return true;
                     }
+                    log.debug("{} Not compatible with principal type '{}' and operator '{}'", getLogPrefix(),
+                            p.getClass(), operator);
+                } else {
+                    log.debug("{} No comparison logic registered for principal type '{}' and operator '{}'",
+                            getLogPrefix(), p.getClass(), operator);
                 }
-                
-                log.info("{} Skipping validator, not compatible with request's principal requirements", getLogPrefix());
-                ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.REQUEST_UNSUPPORTED);
-                return false;
             }
+            
+            log.info("{} Skipping validator, not compatible with request's principal requirements", getLogPrefix());
+            ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.REQUEST_UNSUPPORTED);
+            return false;
         }
         
         final Function<ProfileRequestContext,String> strategy = authenticationContext.getFixedEventLookupStrategy(); 
diff --git a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/context/RequestedPrincipalContext.java b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/context/RequestedPrincipalContext.java
index 376a1d74c..2938a8230 100644
--- a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/context/RequestedPrincipalContext.java
+++ b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/context/RequestedPrincipalContext.java
@@ -209,12 +209,8 @@ public final class RequestedPrincipalContext extends BaseContext {
      */
     @Nullable public PrincipalEvalPredicate getPredicate(@Nonnull final Principal principal) {
         
-        final String op = getOperator();
-        if (op != null) {
-            final PrincipalEvalPredicateFactory factory = evalRegistry.lookup(principal.getClass(), op);
-            return factory != null ? factory.getPredicate(principal) : null;
-        }
-        return null;
+        final PrincipalEvalPredicateFactory factory = evalRegistry.lookup(principal.getClass(), getOperator());
+        return factory != null ? factory.getPredicate(principal) : null;
     }
     
     /**
diff --git a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/principal/PrincipalEvalPredicateFactoryRegistry.java b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/principal/PrincipalEvalPredicateFactoryRegistry.java
index d65bbe943..62dd38c7a 100644
--- a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/principal/PrincipalEvalPredicateFactoryRegistry.java
+++ b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/principal/PrincipalEvalPredicateFactoryRegistry.java
@@ -101,10 +101,14 @@ public final class PrincipalEvalPredicateFactoryRegistry {
      * @return a corresponding predicate factory, or null
      */
     @Nullable public PrincipalEvalPredicateFactory lookup(@Nonnull final Class<? extends Principal> principalType,
-            @Nonnull @NotEmpty final String operator) {
+            @Nullable final String operator) {
         Constraint.isNotNull(principalType, "Principal subtype cannot be null");
-        final String trimmed =
-                Constraint.isNotNull(StringSupport.trimOrNull(operator), "Operator cannot be null or empty");
+        
+        final String trimmed = StringSupport.trimOrNull(operator);
+        if (trimmed == null) {
+            log.warn("Operator was null/empty, no predicate returned");
+            return null;
+        }
         
         final Pair<?,?> key = new Pair<>(principalType, trimmed);
         final PrincipalEvalPredicateFactory factory = registry.get(key);
@@ -113,7 +117,7 @@ public final class PrincipalEvalPredicateFactoryRegistry {
                     factory.getClass().getName(), principalType, trimmed);
             return factory;
         }
-        log.debug("Registry failed to locate predicate factory for principal type '{}' and operator '{}'",
+        log.info("Registry failed to locate predicate factory for principal type '{}' and operator '{}'",
                 principalType, trimmed);
         return null;
     }
diff --git a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/FinalizeAuthentication.java b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/FinalizeAuthentication.java
index cb80489ba..6dc7a5160 100644
--- a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/FinalizeAuthentication.java
+++ b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/FinalizeAuthentication.java
@@ -146,7 +146,7 @@ public class FinalizeAuthentication extends AbstractAuthenticationAction {
         // and the actual result produced may be a subset (and therefore could be an inadequate subset).
         final RequestedPrincipalContext requestedPrincipalCtx =
                 authenticationContext.getSubcontext(RequestedPrincipalContext.class);
-        if (requestedPrincipalCtx != null && requestedPrincipalCtx.getOperator() != null) {
+        if (requestedPrincipalCtx != null) {
             
             // If a matching principal is set, re-verify it. Normally this will work.
             final Principal match = requestedPrincipalCtx.getMatchingPrincipal();
@@ -237,7 +237,6 @@ public class FinalizeAuthentication extends AbstractAuthenticationAction {
         assert ar != null;
         for (final Principal p : requestedPrincipalCtx.getRequestedPrincipals()) {
             final String op = requestedPrincipalCtx.getOperator();
-            assert op != null;
             log.debug("{} Checking result for compatibility with operator '{}' and principal '{}'",
                     getLogPrefix(), op, p.getName());
             final PrincipalEvalPredicateFactory factory =

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


More information about the commits mailing list