[java-idp-plugin-duo] branch main updated: JDUO-83 - Integrate a username collection view into existing Duo flow
Scott Cantor
cantor.2 at osu.edu
Tue Jan 16 16:15:15 UTC 2024
This is an automated email from the git hooks/post-receive script.
scantor pushed a commit to branch main
in repository java-idp-plugin-duo.
View the commit online:
http://git.shibboleth.net/view/?p=java-idp-plugin-duo.git;a=commit;h=f0b33d74fa9df23b05c18a53fb0202931d4a5ecf
The following commit(s) were added to refs/heads/main by this push:
new f0b33d74 JDUO-83 - Integrate a username collection view into existing Duo flow
f0b33d74 is described below
commit f0b33d74fa9df23b05c18a53fb0202931d4a5ecf
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Tue Jan 16 11:15:12 2024 -0500
JDUO-83 - Integrate a username collection view into existing Duo flow
https://shibboleth.atlassian.net/browse/JDUO-83
Move username cookie mgmt to enrollment checking action.
---
.../plugin/authn/duo/DefaultDuoCleanupHook.java | 79 ----------------------
.../duo/impl/CheckPasswordlessEnrollment.java | 66 +++++++++++++++---
.../flows/authn/DuoOIDC/duo-oidc-authn-beans.xml | 6 +-
.../duo/impl/CheckPasswordlessEnrollmentTest.java | 24 +++++++
4 files changed, 82 insertions(+), 93 deletions(-)
diff --git a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DefaultDuoCleanupHook.java b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DefaultDuoCleanupHook.java
index 0e12bbc3..cfcfdf52 100644
--- a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DefaultDuoCleanupHook.java
+++ b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DefaultDuoCleanupHook.java
@@ -16,24 +16,14 @@ package net.shibboleth.idp.plugin.authn.duo;
import java.util.function.Consumer;
-import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import org.opensaml.profile.context.ProfileRequestContext;
-import org.slf4j.Logger;
-
-import com.google.common.net.UrlEscapers;
import net.shibboleth.idp.authn.context.AuthenticationContext;
import net.shibboleth.idp.plugin.authn.duo.context.DuoOIDCAuthenticationContext;
import net.shibboleth.idp.plugin.authn.duo.context.DuoPasswordlessContext;
-import net.shibboleth.shared.annotation.constraint.NotEmpty;
import net.shibboleth.shared.component.AbstractInitializableComponent;
-import net.shibboleth.shared.net.CookieManager;
-import net.shibboleth.shared.primitive.LoggerFactory;
-import net.shibboleth.shared.primitive.StringSupport;
-import net.shibboleth.shared.security.DataSealer;
-import net.shibboleth.shared.security.DataSealerException;
/**
* A default cleanup hook for the DuoOIDC flow that handles both standard
@@ -43,18 +33,6 @@ import net.shibboleth.shared.security.DataSealerException;
*/
public class DefaultDuoCleanupHook extends AbstractInitializableComponent implements Consumer<ProfileRequestContext> {
- /** Class logger. */
- @Nonnull private final Logger log = LoggerFactory.getLogger(DefaultDuoCleanupHook.class);
-
- /** Username cookie name. */
- @Nullable @NotEmpty private String cookieName;
-
- /** Optional cookie manager to use. */
- @Nullable private CookieManager cookieManager;
-
- /** Optional data sealer to use. */
- @Nullable private DataSealer dataSealer;
-
/** Whether to remove a {@link DuoPasswordlessContext} if present. */
private boolean removePasswordlessContext;
@@ -63,39 +41,6 @@ public class DefaultDuoCleanupHook extends AbstractInitializableComponent implem
removePasswordlessContext = true;
}
- /**
- * Set cookie name to use for cached username.
- *
- * @param name cookie name
- */
- public void setCookieName(@Nullable final String name) {
- checkSetterPreconditions();
-
- cookieName = StringSupport.trimOrNull(name);
- }
-
- /**
- * Sets optional {@link CookieManager} to use.
- *
- * @param manager cookie manager
- */
- public void setCookieManager(@Nullable final CookieManager manager) {
- checkSetterPreconditions();
-
- cookieManager = manager;
- }
-
- /**
- * Sets optional {@link DataSealer} to use.
- *
- * @param sealer data sealer
- */
- public void setDataSealer(@Nullable final DataSealer sealer) {
- checkSetterPreconditions();
-
- dataSealer = sealer;
- }
-
/**
* Sets whether to remove a {@link DuoPasswordlessContext} if present.
*
@@ -107,7 +52,6 @@ public class DefaultDuoCleanupHook extends AbstractInitializableComponent implem
removePasswordlessContext = flag;
}
-// Checkstyle: CyclomaticComplexity OFF
/** {@inheritDoc} */
public void accept(@Nullable final ProfileRequestContext input) {
checkComponentActive();
@@ -127,30 +71,7 @@ public class DefaultDuoCleanupHook extends AbstractInitializableComponent implem
if (removePasswordlessContext) {
passwordlessCtx.removeFromParent();
}
-
- final String localCookieName = cookieName;
- if (authnCtx.isResultCacheable()) {
- if (cookieManager != null && dataSealer != null && localCookieName != null) {
- String wrapped = passwordlessCtx.getUsername();
- if (wrapped != null) {
- try {
- assert dataSealer != null;
- wrapped = dataSealer.wrap(wrapped);
- assert cookieManager != null;
- cookieManager.addCookie(localCookieName,
- UrlEscapers.urlFormParameterEscaper().escape(wrapped));
- } catch (final DataSealerException e) {
- wrapped = null;
- log.warn("Error sealing username cookie", e);
- }
- }
- }
- } else if (cookieManager != null && localCookieName != null) {
- cookieManager.unsetCookie(localCookieName);
- }
}
}
-// Checkstyle: CyclomaticComplexity ON
-
}
\ No newline at end of file
diff --git a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/CheckPasswordlessEnrollment.java b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/CheckPasswordlessEnrollment.java
index 742e9510..b7948d65 100644
--- a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/CheckPasswordlessEnrollment.java
+++ b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/CheckPasswordlessEnrollment.java
@@ -29,6 +29,8 @@ import org.opensaml.profile.action.EventIds;
import org.opensaml.profile.context.ProfileRequestContext;
import org.slf4j.Logger;
+import com.google.common.net.UrlEscapers;
+
import jakarta.servlet.http.HttpServletRequest;
import net.shibboleth.idp.authn.AbstractExtractionAction;
import net.shibboleth.idp.authn.AuthnEventIds;
@@ -252,6 +254,8 @@ public class CheckPasswordlessEnrollment extends AbstractExtractionAction {
protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext,
@Nonnull final AuthenticationContext authenticationContext) {
+ setCacheability(authenticationContext);
+
boolean usernameChanged = false;
boolean exitLoop = false;
@@ -319,6 +323,10 @@ public class CheckPasswordlessEnrollment extends AbstractExtractionAction {
passwordlessCondition.test(profileRequestContext, passwordlessContext.getUsername()));
log.debug("{} Username '{}' found to be {} of passwordless attempt", getLogPrefix(),
passwordlessContext.getUsername(), passwordlessContext.isEnrolled() ? "capable" : "incapable");
+
+ // Upodate cookie if needed.
+ manageCookie(authenticationContext);
+
} else {
log.debug("{} Username not updated, leaving DuoPasswordlessContext unchanged", getLogPrefix());
}
@@ -329,6 +337,55 @@ public class CheckPasswordlessEnrollment extends AbstractExtractionAction {
}
// Checkstyle: CyclomaticComplexity|MethodLength ON
+ /**
+ * Establishes result cacheability from form field.
+ *
+ * @param authenticationContext authentication context
+ */
+ private void setCacheability(@Nonnull final AuthenticationContext authenticationContext) {
+
+ final HttpServletRequest request = getHttpServletRequest();
+ if (request != null) {
+ final String donotcache = request.getParameter(ssoBypassFieldName);
+ if (donotcache != null && "1".equals(donotcache)) {
+ log.debug("{} Recording do-not-cache instruction in authentication context", getLogPrefix());
+ authenticationContext.setResultCacheable(false);
+ } else {
+ authenticationContext.setResultCacheable(true);
+ }
+ }
+ }
+
+ /**
+ * Set or unset cokie based on enrollment status and result cacheability.
+ *
+ * @param authenticationContext authentication context
+ */
+ private void manageCookie(@Nonnull final AuthenticationContext authenticationContext) {
+ if (passwordlessContext.isEnrolled()) {
+ final String localCookieName = cookieName;
+ if (authenticationContext.isResultCacheable()) {
+ if (cookieManager != null && dataSealer != null && localCookieName != null) {
+ String wrapped = passwordlessContext.getUsername();
+ if (wrapped != null) {
+ try {
+ assert dataSealer != null;
+ wrapped = dataSealer.wrap(wrapped);
+ assert cookieManager != null;
+ cookieManager.addCookie(localCookieName,
+ UrlEscapers.urlFormParameterEscaper().escape(wrapped));
+ } catch (final DataSealerException e) {
+ wrapped = null;
+ log.warn("Error sealing username cookie", e);
+ }
+ }
+ }
+ } else if (cookieManager != null && localCookieName != null) {
+ cookieManager.unsetCookie(localCookieName);
+ }
+ }
+ }
+
/**
* Gets the username from a form submission.
*
@@ -344,16 +401,7 @@ public class CheckPasswordlessEnrollment extends AbstractExtractionAction {
final HttpServletRequest request = getHttpServletRequest();
if (request != null) {
- final String donotcache = request.getParameter(ssoBypassFieldName);
- if (donotcache != null && "1".equals(donotcache)) {
- log.debug("{} Recording do-not-cache instruction in authentication context", getLogPrefix());
- authenticationContext.setResultCacheable(false);
- } else {
- authenticationContext.setResultCacheable(true);
- }
-
// TODO: Convert to 2 parameter version once API moves to 5.1.
-
final String s = applyTransforms(request.getParameter(usernameFieldName));
return duoUsernameRemappingStrategy != null
diff --git a/idp-duo-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/DuoOIDC/duo-oidc-authn-beans.xml b/idp-duo-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/DuoOIDC/duo-oidc-authn-beans.xml
index eac9c29a..3f4d7263 100644
--- a/idp-duo-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/DuoOIDC/duo-oidc-authn-beans.xml
+++ b/idp-duo-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/DuoOIDC/duo-oidc-authn-beans.xml
@@ -290,11 +290,7 @@
<value>iat</value>
</util:set>
- <bean id="shibboleth.authn.DuoOIDC.DefaultCleanupHook"
- class="net.shibboleth.idp.plugin.authn.duo.DefaultDuoCleanupHook"
- p:dataSealer="#{'%{idp.authn.usernameCookieName:}'.trim().isEmpty() ? null : getObject('shibboleth.DataSealer')}"
- p:cookieManager="#{'%{idp.authn.usernameCookieName:}'.trim().isEmpty() ? null : getObject('shibboleth.PersistentCookieManager')}"
- p:cookieName="%{idp.authn.usernameCookieName:}" />
+ <bean id="shibboleth.authn.DuoOIDC.DefaultCleanupHook" class="net.shibboleth.idp.plugin.authn.duo.DefaultDuoCleanupHook" />
<bean id="ValidateDuoTokenAuthenticationResult" scope="prototype"
class="net.shibboleth.idp.plugin.authn.duo.impl.ValidateDuoTokenAuthenticationResult"
diff --git a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/CheckPasswordlessEnrollmentTest.java b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/CheckPasswordlessEnrollmentTest.java
index f389d7b5..32f88b2a 100644
--- a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/CheckPasswordlessEnrollmentTest.java
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/CheckPasswordlessEnrollmentTest.java
@@ -51,6 +51,7 @@ import net.shibboleth.shared.annotation.constraint.NotEmpty;
import net.shibboleth.shared.collection.CollectionSupport;
import net.shibboleth.shared.component.ComponentInitializationException;
import net.shibboleth.shared.net.CookieManager;
+import net.shibboleth.shared.net.URISupport;
import net.shibboleth.shared.security.DataSealer;
import net.shibboleth.shared.security.DataSealerException;
import net.shibboleth.shared.security.impl.BasicKeystoreKeyStrategy;
@@ -166,6 +167,11 @@ public class CheckPasswordlessEnrollmentTest extends AbstractDuoActionTest {
final DuoPasswordlessContext duoCtx = authCtx.ensureSubcontext(DuoPasswordlessContext.class);
Assert.assertEquals(duoCtx.getUsername(), "foo");
Assert.assertTrue(duoCtx.isEnrolled());
+
+ final Cookie cookie = ensureMockResponse().getCookie(COOKIE_NAME);
+ assert cookie != null;
+ Assert.assertNull(cookie.getValue());
+ Assert.assertEquals(cookie.getMaxAge(), 0);
}
@Test public void testFromCookie() throws DataSealerException {
@@ -183,6 +189,15 @@ public class CheckPasswordlessEnrollmentTest extends AbstractDuoActionTest {
final DuoPasswordlessContext duoCtx = authCtx.ensureSubcontext(DuoPasswordlessContext.class);
Assert.assertEquals(duoCtx.getUsername(), "foo");
Assert.assertTrue(duoCtx.isEnrolled());
+
+ final Cookie cookie2 = ensureMockResponse().getCookie(COOKIE_NAME);
+ assert cookie2 != null;
+ Assert.assertEquals(cookie2.getMaxAge(), 300);
+ String unwrapped = URISupport.doURLDecode(cookie2.getValue());
+ if (unwrapped != null) {
+ unwrapped = dataSealer.unwrap(unwrapped);
+ }
+ Assert.assertEquals(unwrapped, "foo");
}
@Test public void testFromSession() throws DataSealerException {
@@ -204,6 +219,15 @@ public class CheckPasswordlessEnrollmentTest extends AbstractDuoActionTest {
final DuoPasswordlessContext duoCtx = authCtx.ensureSubcontext(DuoPasswordlessContext.class);
Assert.assertEquals(duoCtx.getUsername(), "foo");
Assert.assertTrue(duoCtx.isEnrolled());
+
+ final Cookie cookie2 = ensureMockResponse().getCookie(COOKIE_NAME);
+ assert cookie2 != null;
+ Assert.assertEquals(cookie2.getMaxAge(), 300);
+ String unwrapped = URISupport.doURLDecode(cookie2.getValue());
+ if (unwrapped != null) {
+ unwrapped = dataSealer.unwrap(unwrapped);
+ }
+ Assert.assertEquals(unwrapped, "foo");
}
@Nonnull private MockHttpServletRequest ensureMockRequest() {
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list