[java-identity-provider] branch main updated: IDP-1796 - Selection of c14n flow doesn't log enough information
Scott Cantor
cantor.2 at osu.edu
Tue Apr 13 12:52:47 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=086b9684ea03387cbb55cb2605ac453b1516e263
The following commit(s) were added to refs/heads/main by this push:
new 086b9684e IDP-1796 - Selection of c14n flow doesn't log enough information
086b9684e is described below
commit 086b9684ea03387cbb55cb2605ac453b1516e263
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Tue Apr 13 08:52:44 2021 -0400
IDP-1796 - Selection of c14n flow doesn't log enough information
https://issues.shibboleth.net/jira/browse/IDP-1796
---
.../impl/SelectSubjectCanonicalizationFlow.java | 6 ++--
.../authn/impl/SimpleSubjectCanonicalization.java | 29 ++++++++++-------
.../authn/impl/X500SubjectCanonicalization.java | 5 +--
.../saml/nameid/impl/NameIDCanonicalization.java | 35 +++++++++-----------
.../impl/NameIdentifierCanonicalization.java | 38 ++++++++++------------
5 files changed, 58 insertions(+), 55 deletions(-)
diff --git a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/SelectSubjectCanonicalizationFlow.java b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/SelectSubjectCanonicalizationFlow.java
index 6439f84b9..746131aa7 100644
--- a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/SelectSubjectCanonicalizationFlow.java
+++ b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/SelectSubjectCanonicalizationFlow.java
@@ -103,8 +103,10 @@ public class SelectSubjectCanonicalizationFlow extends AbstractSubjectCanonicali
if (flow.test(profileRequestContext)) {
return flow;
}
- log.debug("{} Canonicalization flow {} was not applicable to this request", getLogPrefix(),
- flow.getId());
+ log.debug("{} Canonicalization flow {} was not applicable: {}", getLogPrefix(), flow.getId(),
+ c14nContext.getException() != null ? c14nContext.getException().getMessage()
+ : "reason unknown");
+ c14nContext.setException(null);
// Note that we don't exclude this flow from possible future selection, since one flow
// could in theory do partial work and change the context such that this flow then applies.
diff --git a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/SimpleSubjectCanonicalization.java b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/SimpleSubjectCanonicalization.java
index 19a07f138..8944711c6 100644
--- a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/SimpleSubjectCanonicalization.java
+++ b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/SimpleSubjectCanonicalization.java
@@ -31,6 +31,8 @@ import net.shibboleth.idp.authn.principal.UsernamePrincipal;
import org.opensaml.profile.action.ActionSupport;
import org.opensaml.profile.context.ProfileRequestContext;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* An action that operates on a {@link SubjectCanonicalizationContext} child of the current
@@ -45,6 +47,9 @@ import org.opensaml.profile.context.ProfileRequestContext;
*/
public class SimpleSubjectCanonicalization extends AbstractSubjectCanonicalizationAction {
+ /** Class logger. */
+ @Nonnull private final Logger log = LoggerFactory.getLogger(SelectAuthenticationFlow.class);
+
/** Supplies logic for pre-execute test. */
@Nonnull private final ActivationCondition embeddedPredicate;
@@ -114,23 +119,23 @@ public class SimpleSubjectCanonicalization extends AbstractSubjectCanonicalizati
usernames = null;
}
- if (duringAction) {
- if (usernames == null || usernames.isEmpty()) {
- c14nContext.setException(
- new SubjectCanonicalizationException("No UsernamePrincipals were found"));
+ if (usernames == null || usernames.isEmpty()) {
+ c14nContext.setException(
+ new SubjectCanonicalizationException("No UsernamePrincipals were found"));
+ if (duringAction) {
ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.INVALID_SUBJECT);
- return false;
- } else if (usernames.size() > 1) {
- c14nContext.setException(
- new SubjectCanonicalizationException("Multiple UsernamePrincipals were found"));
+ }
+ return false;
+ } else if (usernames.size() > 1) {
+ c14nContext.setException(
+ new SubjectCanonicalizationException("Multiple UsernamePrincipals were found"));
+ if (duringAction) {
ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.INVALID_SUBJECT);
- return false;
}
-
- return true;
+ return false;
}
- return usernames != null && usernames.size() == 1;
+ return true;
}
}
diff --git a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/X500SubjectCanonicalization.java b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/X500SubjectCanonicalization.java
index 33f1c9abf..486263e56 100644
--- a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/X500SubjectCanonicalization.java
+++ b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/X500SubjectCanonicalization.java
@@ -256,11 +256,12 @@ public class X500SubjectCanonicalization extends AbstractSubjectCanonicalization
}
}
+ c14nContext.setException(new SubjectCanonicalizationException(
+ "Neither a single X509Certificate nor X500Principal were found"));
if (duringAction) {
- c14nContext.setException(new SubjectCanonicalizationException(
- "Neither a single X509Certificate nor X500Principal were found"));
ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.INVALID_SUBJECT);
}
+
return false;
}
diff --git a/idp-saml-impl/src/main/java/net/shibboleth/idp/saml/nameid/impl/NameIDCanonicalization.java b/idp-saml-impl/src/main/java/net/shibboleth/idp/saml/nameid/impl/NameIDCanonicalization.java
index 0aa6ad60e..2628fd087 100644
--- a/idp-saml-impl/src/main/java/net/shibboleth/idp/saml/nameid/impl/NameIDCanonicalization.java
+++ b/idp-saml-impl/src/main/java/net/shibboleth/idp/saml/nameid/impl/NameIDCanonicalization.java
@@ -211,31 +211,28 @@ public class NameIDCanonicalization extends AbstractSubjectCanonicalizationActio
nameIDs = c14nContext.getSubject().getPrincipals(NameIDPrincipal.class);
}
- if (duringAction) {
- if (nameIDs == null || nameIDs.isEmpty()) {
- c14nContext.setException(new SubjectCanonicalizationException("No NameIDPrincipals were found"));
+ if (nameIDs == null || nameIDs.isEmpty()) {
+ c14nContext.setException(new SubjectCanonicalizationException("No NameIDPrincipals were found"));
+ if (duringAction) {
ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.INVALID_SUBJECT);
- return false;
- } else if (nameIDs.size() > 1) {
- c14nContext.setException(
- new SubjectCanonicalizationException("Multiple NameIDPrincipals were found"));
+ }
+ return false;
+ } else if (nameIDs.size() > 1) {
+ c14nContext.setException(
+ new SubjectCanonicalizationException("Multiple NameIDPrincipals were found"));
+ if (duringAction) {
ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.INVALID_SUBJECT);
- return false;
- } else if (!formatMatches(nameIDs.iterator().next().getNameID().getFormat(), c14nContext)) {
- c14nContext.setException(new SubjectCanonicalizationException("Format not supported"));
+ }
+ return false;
+ } else if (!formatMatches(nameIDs.iterator().next().getNameID().getFormat(), c14nContext)) {
+ c14nContext.setException(new SubjectCanonicalizationException("Format not supported"));
+ if (duringAction) {
ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.INVALID_SUBJECT);
- return false;
}
-
- return true;
- }
-
- // Not in an action, so do the same but without the context side effects
- if (nameIDs == null || nameIDs.size() != 1) {
return false;
}
-
- return formatMatches(nameIDs.iterator().next().getNameID().getFormat(), c14nContext);
+
+ return true;
}
}
diff --git a/idp-saml-impl/src/main/java/net/shibboleth/idp/saml/nameid/impl/NameIdentifierCanonicalization.java b/idp-saml-impl/src/main/java/net/shibboleth/idp/saml/nameid/impl/NameIdentifierCanonicalization.java
index 98818360b..59eb2f80f 100644
--- a/idp-saml-impl/src/main/java/net/shibboleth/idp/saml/nameid/impl/NameIdentifierCanonicalization.java
+++ b/idp-saml-impl/src/main/java/net/shibboleth/idp/saml/nameid/impl/NameIdentifierCanonicalization.java
@@ -212,32 +212,30 @@ public class NameIdentifierCanonicalization extends AbstractSubjectCanonicalizat
nameIdentifiers = c14nContext.getSubject().getPrincipals(NameIdentifierPrincipal.class);
}
- if (duringAction) {
- if (nameIdentifiers == null || nameIdentifiers.isEmpty()) {
- c14nContext.setException(
- new SubjectCanonicalizationException("No NameIdentifierPrincipals were found"));
+ if (nameIdentifiers == null || nameIdentifiers.isEmpty()) {
+ c14nContext.setException(
+ new SubjectCanonicalizationException("No NameIdentifierPrincipals were found"));
+ if (duringAction) {
ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.INVALID_SUBJECT);
- return false;
- } else if (nameIdentifiers.size() > 1) {
- c14nContext.setException(
- new SubjectCanonicalizationException("Multiple NameIdentifierPrincipals were found"));
+ }
+ return false;
+ } else if (nameIdentifiers.size() > 1) {
+ c14nContext.setException(
+ new SubjectCanonicalizationException("Multiple NameIdentifierPrincipals were found"));
+ if (duringAction) {
ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.INVALID_SUBJECT);
- return false;
- } else if (!formatMatches(nameIdentifiers.iterator().next().getNameIdentifier().getFormat(),
- c14nContext)) {
- c14nContext.setException(new SubjectCanonicalizationException("Format not supported"));
+ }
+ return false;
+ } else if (!formatMatches(nameIdentifiers.iterator().next().getNameIdentifier().getFormat(),
+ c14nContext)) {
+ c14nContext.setException(new SubjectCanonicalizationException("Format not supported"));
+ if (duringAction) {
ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.INVALID_SUBJECT);
- return false;
}
-
- return true;
- }
-
- // Not in an action, so do the same but without the context side effects
- if (nameIdentifiers == null || nameIdentifiers.size() != 1) {
return false;
}
- return formatMatches(nameIdentifiers.iterator().next().getNameIdentifier().getFormat(), c14nContext);
+
+ return true;
}
}
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list