[java-identity-provider] 03/03: IDP-2480 - ReqPrincipalContext handles missing operator as "accepted"
Codeberg
noreply at shibboleth.net
Mon Sep 28 13:29:35 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/33f5f37e6628f4e49814083ceb05f315f5ceded1
commit 33f5f37e6628f4e49814083ceb05f315f5ceded1
Author: Scott Cantor <scott at restingparrotsoftware.com>
AuthorDate: Mon Sep 28 09:29:18 2026 -0400
IDP-2480 - ReqPrincipalContext handles missing operator as "accepted"
https://shibboleth.atlassian.net/browse/IDP-2480
Default in "exact" and ensure non-null operator.
---
.../idp/authn/AbstractCredentialValidator.java | 50 +++++++++++-----------
.../idp/authn/AbstractValidationAction.java | 40 ++++++++---------
.../authn/context/RequestedPrincipalContext.java | 27 ++++++++----
.../idp/authn/impl/FinalizeAuthentication.java | 3 +-
.../impl/tests/FinalizeAuthenticationTest.java | 6 +--
5 files changed, 63 insertions(+), 63 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..b5c120e23 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
@@ -58,11 +58,18 @@ import org.opensaml.messaging.context.BaseContext;
*/
public final class RequestedPrincipalContext extends BaseContext {
+ /**
+ * Default operator, matching SAML's default for expedience.
+ *
+ * @since 5.3.0
+ */
+ @Nonnull public static final String DEFAULT_OPERATOR = "exact";
+
/** The registry of predicate factories for custom principal evaluation. */
@Nonnull private PrincipalEvalPredicateFactoryRegistry evalRegistry;
/** Comparison operator specific to request protocol. */
- @Nullable private String operatorString;
+ @Nonnull private String operatorString;
/** The principals reflecting the request requirements. */
@Nonnull private List<Principal> requestedPrincipals;
@@ -76,6 +83,7 @@ public final class RequestedPrincipalContext extends BaseContext {
/** Constructor. */
public RequestedPrincipalContext() {
evalRegistry = new PrincipalEvalPredicateFactoryRegistry();
+ operatorString = DEFAULT_OPERATOR;
requestedPrincipals = CollectionSupport.emptyList();
}
@@ -111,7 +119,7 @@ public final class RequestedPrincipalContext extends BaseContext {
*
* @return comparison operator
*/
- @Nullable @NotEmpty public String getOperator() {
+ @Nonnull @NotEmpty public String getOperator() {
return operatorString;
}
@@ -124,7 +132,12 @@ public final class RequestedPrincipalContext extends BaseContext {
*/
@Nonnull public RequestedPrincipalContext setOperator(@Nullable @NotEmpty final String operator) {
- operatorString = StringSupport.trimOrNull(operator);
+ final String trimmed = StringSupport.trimOrNull(operator);
+ if (trimmed != null) {
+ operatorString = trimmed;
+ } else {
+ operatorString = DEFAULT_OPERATOR;
+ }
return this;
}
@@ -209,12 +222,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-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 =
diff --git a/idp-saml-impl/src/test/java/net/shibboleth/idp/authn/impl/tests/FinalizeAuthenticationTest.java b/idp-saml-impl/src/test/java/net/shibboleth/idp/authn/impl/tests/FinalizeAuthenticationTest.java
index c4f7c1533..83added60 100644
--- a/idp-saml-impl/src/test/java/net/shibboleth/idp/authn/impl/tests/FinalizeAuthenticationTest.java
+++ b/idp-saml-impl/src/test/java/net/shibboleth/idp/authn/impl/tests/FinalizeAuthenticationTest.java
@@ -199,11 +199,7 @@ public class FinalizeAuthenticationTest extends OpenSAMLInitBaseTestCase {
event = action.execute(src);
- ActionTestingSupport.assertProceedEvent(event);
- sc = prc.getSubcontext(SubjectContext.class);
- assert sc!= null;
- Assert.assertEquals(sc.getPrincipalName(), "foo");
- Assert.assertEquals(sc.getAuthenticationResults().size(), 1);
+ ActionTestingSupport.assertEvent(event, AuthnEventIds.REQUEST_UNSUPPORTED);
}
@Test public void testMultipleActive() {
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list