[java-identity-provider] branch main updated: IDP-1701 - Auditing failed requests with Subject log the wrong username
Scott Cantor
cantor.2 at osu.edu
Wed Jan 6 16:21:25 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=d9a7186b1e1a26773ddbf1e3bd68d83b8fbcf59b
The following commit(s) were added to refs/heads/main by this push:
new d9a7186b1 IDP-1701 - Auditing failed requests with Subject log the wrong username
d9a7186b1 is described below
commit d9a7186b1e1a26773ddbf1e3bd68d83b8fbcf59b
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Wed Jan 6 11:21:22 2021 -0500
IDP-1701 - Auditing failed requests with Subject log the wrong username
https://issues.shibboleth.net/jira/browse/IDP-1701
Redesign handling of inbound Subject cross-check.
---
.../idp/authn/context/AuthenticationContext.java | 35 +++++++++++++++++++++-
.../idp/authn/impl/FinalizeAuthentication.java | 21 ++++++-------
.../idp/authn/impl/FinalizeAuthenticationTest.java | 15 +++++-----
.../idp/flows/authn/authn-abstract-flow.xml | 2 ++
.../impl/InitializeAuthenticationContext.java | 13 ++++++++
5 files changed, 65 insertions(+), 21 deletions(-)
diff --git a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/context/AuthenticationContext.java b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/context/AuthenticationContext.java
index f0aa6cb19..f46d3b496 100644
--- a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/context/AuthenticationContext.java
+++ b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/context/AuthenticationContext.java
@@ -78,9 +78,12 @@ public final class AuthenticationContext extends BaseContext {
/** Whether authentication must not involve subject interaction. */
private boolean isPassive;
+ /** A normative hint some protocols support to indicate who the subject MUST be. */
+ @Nullable private String requiredName;
+
/** A non-normative hint some protocols support to indicate who the subject might be. */
@Nullable private String hintedName;
-
+
/** Allowed time since an {@link AuthenticationResult} was created that it can be reused for this request. */
@Nullable private Duration maxAge;
@@ -319,6 +322,35 @@ public final class AuthenticationContext extends BaseContext {
return this;
}
+ /**
+ * Get a normative hint provided by the request about the user's identity.
+ *
+ * <p>This <strong>MUST BE</strong> a trustworthy value, and can only be set through some
+ * normative protocol machinery or with the understanding that it is subsequently a governing
+ * constraint on the canonical result of every flow.</p>
+ *
+ * @return the mandatory username
+ */
+ @Nullable @NotEmpty public String getRequiredName() {
+ return requiredName;
+ }
+
+ /**
+ * Set a normative hint provided by the request about the user's identity.
+ *
+ * <p>This <strong>MUST BE</strong> a trustworthy value, and can only be set through some
+ * normative protocol machinery or with the understanding that it is subsequently a governing
+ * constraint on the canonical result of every flow.</p>
+ *
+ * @param name the required username
+ *
+ * @return this authentication context
+ */
+ @Nonnull public AuthenticationContext setRequiredName(@Nullable final String name) {
+ requiredName = name;
+ return this;
+ }
+
/**
* Get a non-normative hint provided by the request about the user's identity.
*
@@ -774,6 +806,7 @@ public final class AuthenticationContext extends BaseContext {
.add("initiationInstant", initiationInstant)
.add("isPassive", isPassive)
.add("forceAuthn", forceAuthn)
+ .add("requiredName", requiredName)
.add("hintedName", hintedName)
.add("maxAge", maxAge)
.add("potentialFlows", potentialFlows.keySet())
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 7f96bb08e..96e1f9e31 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
@@ -39,7 +39,6 @@ import net.shibboleth.idp.authn.principal.PrincipalEvalPredicate;
import net.shibboleth.idp.authn.principal.PrincipalEvalPredicateFactory;
import net.shibboleth.idp.authn.principal.PrincipalSupportingComponent;
import net.shibboleth.idp.authn.principal.ProxyAuthenticationPrincipal;
-import net.shibboleth.idp.profile.IdPEventIds;
import net.shibboleth.idp.profile.context.RelyingPartyContext;
import net.shibboleth.idp.session.context.SessionContext;
@@ -74,13 +73,12 @@ import org.slf4j.LoggerFactory;
*
* <p>Any {@link SubjectCanonicalizationContext} found will be removed.</p>
*
- * <p>If a {@link SubjectContext} already exists, then this action will validate that
- * the same principal name is represented by it, and signal a mismatch otherwise. This
- * is used in protocols that indicate normatively what the authenticated identity is
- * required to be.</p>
+ * <p>If {@link AuthenticationContext#getRequiredName()} is set, then this action will validate that
+ * the same principal name is represented by it, and signal a mismatch otherwise. This is used in
+ * protocols that indicate normatively what the authenticated identity is required to be.</p>
*
* @event {@link org.opensaml.profile.action.EventIds#PROCEED_EVENT_ID}
- * @event {@link IdPEventIds#INVALID_SUBJECT_CTX}
+ * @event {@link AuthnEventIds#INVALID_SUBJECT}
* @event {@link AuthnEventIds#INVALID_AUTHN_CTX}
* @event {@link AuthnEventIds#REQUEST_UNSUPPORTED}
*
@@ -186,16 +184,15 @@ public class FinalizeAuthentication extends AbstractAuthenticationAction {
@Nonnull final AuthenticationContext authenticationContext) {
if (canonicalPrincipalName != null) {
- final SubjectContext sc = profileRequestContext.getSubcontext(SubjectContext.class, true);
-
- // Check for an existing value.
- if (sc.getPrincipalName() != null && !canonicalPrincipalName.equals(sc.getPrincipalName())) {
+ if (authenticationContext.getRequiredName() != null &&
+ !canonicalPrincipalName.equals(authenticationContext.getRequiredName())) {
log.warn("{} Result of authentication ({}) does not match existing subject in context ({})",
- getLogPrefix(), canonicalPrincipalName, sc.getPrincipalName());
- ActionSupport.buildEvent(profileRequestContext, IdPEventIds.INVALID_SUBJECT_CTX);
+ getLogPrefix(), canonicalPrincipalName, authenticationContext.getRequiredName());
+ ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.INVALID_SUBJECT);
return;
}
+ final SubjectContext sc = profileRequestContext.getSubcontext(SubjectContext.class, true);
sc.setPrincipalName(canonicalPrincipalName);
final Map<String,AuthenticationResult> scResults = sc.getAuthenticationResults();
diff --git a/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/FinalizeAuthenticationTest.java b/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/FinalizeAuthenticationTest.java
index 6d4652ebd..2bdee7acb 100644
--- a/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/FinalizeAuthenticationTest.java
+++ b/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/FinalizeAuthenticationTest.java
@@ -33,7 +33,6 @@ import net.shibboleth.idp.authn.impl.testing.BaseAuthenticationContextTest;
import net.shibboleth.idp.authn.principal.ProxyAuthenticationPrincipal;
import net.shibboleth.idp.authn.principal.impl.ExactPrincipalEvalPredicateFactory;
import net.shibboleth.idp.authn.testing.TestPrincipal;
-import net.shibboleth.idp.profile.IdPEventIds;
import net.shibboleth.idp.profile.context.RelyingPartyContext;
import net.shibboleth.idp.profile.testing.ActionTestingSupport;
import net.shibboleth.idp.saml.saml2.profile.config.BrowserSSOProfileConfiguration;
@@ -65,18 +64,18 @@ public class FinalizeAuthenticationTest extends BaseAuthenticationContextTest {
}
@Test public void testMismatch() {
- prc.getSubcontext(SubjectContext.class, true).setPrincipalName("foo");
- prc.getSubcontext(SubjectCanonicalizationContext.class, true).setPrincipalName("bar");
-
+ final AuthenticationContext authCtx = prc.getSubcontext(AuthenticationContext.class);
+ authCtx.setRequiredName("foo");
+
final AuthenticationResult active = new AuthenticationResult("test2", new Subject());
active.getSubject().getPrincipals().add(new TestPrincipal("bar2"));
- final AuthenticationContext authCtx = prc.getSubcontext(AuthenticationContext.class);
authCtx.setAuthenticationResult(active);
-
+
+ prc.getSubcontext(SubjectCanonicalizationContext.class, true).setPrincipalName("bar");
+
final Event event = action.execute(src);
-
- ActionTestingSupport.assertEvent(event, IdPEventIds.INVALID_SUBJECT_CTX);
+ ActionTestingSupport.assertEvent(event, AuthnEventIds.INVALID_SUBJECT);
}
@Test public void testRequestUnsupported() {
diff --git a/idp-conf-impl/src/main/resources/net/shibboleth/idp/flows/authn/authn-abstract-flow.xml b/idp-conf-impl/src/main/resources/net/shibboleth/idp/flows/authn/authn-abstract-flow.xml
index c9bce6558..8a6784d0d 100644
--- a/idp-conf-impl/src/main/resources/net/shibboleth/idp/flows/authn/authn-abstract-flow.xml
+++ b/idp-conf-impl/src/main/resources/net/shibboleth/idp/flows/authn/authn-abstract-flow.xml
@@ -26,6 +26,7 @@
<end-state id="InvalidProfileConfiguration" />
<end-state id="InvalidRelyingPartyConfiguration" />
<end-state id="InvalidRelyingPartyContext" />
+ <end-state id="InvalidSubject" />
<end-state id="InvalidSubjectContext" />
<end-state id="InvalidSubjectCanonicalizationContext" />
<end-state id="NoCredentials" />
@@ -73,6 +74,7 @@
<transition on="InvalidProfileConfiguration" to="InvalidProfileConfiguration" />
<transition on="InvalidRelyingPartyConfiguration" to="InvalidRelyingPartyConfiguration" />
<transition on="InvalidRelyingPartyContext" to="InvalidRelyingPartyContext" />
+ <transition on="InvalidSubject" to="InvalidSubject" />
<transition on="InvalidSubjectContext" to="InvalidSubjectContext" />
<transition on="InvalidSubjectCanonicalizationContext" to="InvalidSubjectCanonicalizationContext" />
<transition on="NoCredentials" to="NoCredentials" />
diff --git a/idp-saml-impl/src/main/java/net/shibboleth/idp/saml/profile/impl/InitializeAuthenticationContext.java b/idp-saml-impl/src/main/java/net/shibboleth/idp/saml/profile/impl/InitializeAuthenticationContext.java
index 247c6db3a..ced457fbe 100644
--- a/idp-saml-impl/src/main/java/net/shibboleth/idp/saml/profile/impl/InitializeAuthenticationContext.java
+++ b/idp-saml-impl/src/main/java/net/shibboleth/idp/saml/profile/impl/InitializeAuthenticationContext.java
@@ -27,6 +27,7 @@ import javax.annotation.Nullable;
import net.shibboleth.idp.authn.config.navigate.ForceAuthnProfileConfigPredicate;
import net.shibboleth.idp.authn.context.AuthenticationContext;
+import net.shibboleth.idp.authn.context.SubjectContext;
import net.shibboleth.idp.profile.AbstractProfileAction;
import net.shibboleth.idp.profile.context.RelyingPartyContext;
import net.shibboleth.idp.saml.saml2.profile.config.BrowserSSOProfileConfiguration;
@@ -169,6 +170,7 @@ public class InitializeAuthenticationContext extends AbstractProfileAction {
return true;
}
+// Checkstyle: CyclomaticComplexity OFF
/** {@inheritDoc} */
@Override
protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext) {
@@ -181,6 +183,16 @@ public class InitializeAuthenticationContext extends AbstractProfileAction {
}
authnCtx.setForceAuthn(authnRequest.isForceAuthn());
authnCtx.setIsPassive(authnRequest.isPassive());
+
+ // On an inbound Subject, migrate the populated SubjectContext into the required name
+ // field in the new AuthenticationContext.
+ if (authnRequest.getSubject() != null && authnRequest.getSubject().getNameID() != null) {
+ final SubjectContext subjectCtx = profileRequestContext.getSubcontext(SubjectContext.class);
+ if (subjectCtx != null && subjectCtx.getPrincipalName() != null) {
+ authnCtx.setRequiredName(subjectCtx.getPrincipalName());
+ profileRequestContext.removeSubcontext(subjectCtx);
+ }
+ }
}
if (!authnCtx.isForceAuthn()) {
@@ -209,6 +221,7 @@ public class InitializeAuthenticationContext extends AbstractProfileAction {
log.debug("{} Created authentication context: {}", getLogPrefix(), authnCtx);
}
+// Checkstyle: CyclomaticComplexity OFF
/**
* Check an inbound {@link AuthnRequest} for a {@link Scoping} element.
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list