[java-idp-plugin-duo] branch master updated: JDUO-2 - SWF execution key handling in Controller
Phil Smart
philip.smart at jisc.ac.uk
Tue Aug 4 14:34:43 UTC 2020
This is an automated email from the git hooks/post-receive script.
philsmart pushed a commit to branch master
in repository java-idp-plugin-duo.
View the commit online:
http://git.shibboleth.net/view/?p=java-idp-plugin-duo.git;a=commit;h=b15eb728a4487da53aac6e29e22b6c1d36c3b3a7
The following commit(s) were added to refs/heads/master by this push:
new b15eb72 JDUO-2 - SWF execution key handling in Controller
b15eb72 is described below
commit b15eb728a4487da53aac6e29e22b6c1d36c3b3a7
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Tue Aug 4 15:34:14 2020 +0100
JDUO-2 - SWF execution key handling in Controller
- Add execution key as a component of the state parameter
https://issues.shibboleth.net/jira/browse/JDUO-2
---
.../idp/plugin/authn/duo/DuoClientException.java | 2 +-
.../{DuoClientException.java => DuoException.java} | 12 +--
.../idp/plugin/authn/duo/DuoRegistryException.java | 2 +-
.../authn/duo/impl/DuoOIDCAuthnController.java | 77 ++++++++++----------
.../idp/plugin/authn/duo/impl/DuoSupport.java | 85 ++++++++++++++++++++--
.../flows/authn/DuoOIDC/duo-oidc-authn-beans.xml | 3 -
.../flows/authn/DuoOIDC/duo-oidc-authn-flow.xml | 2 +-
.../plugin/authn/duo/impl/DuoAuthnFlowTest.java | 8 +-
.../authn/duo/impl/DuoOIDCAuthnControllerTest.java | 54 ++++++++------
.../idp/plugin/authn/duo/impl/DuoSupportTest.java | 27 +++++++
.../duo/impl/ValidateDuoResponseStateTest.java | 10 +--
.../authn/duo/sdk/impl/DuoSDKClientFactory.java | 5 ++
12 files changed, 199 insertions(+), 88 deletions(-)
diff --git a/idp-duo-api/src/main/java/net/shbboleth/idp/plugin/authn/duo/DuoClientException.java b/idp-duo-api/src/main/java/net/shbboleth/idp/plugin/authn/duo/DuoClientException.java
index 89cab99..ab8fa56 100644
--- a/idp-duo-api/src/main/java/net/shbboleth/idp/plugin/authn/duo/DuoClientException.java
+++ b/idp-duo-api/src/main/java/net/shbboleth/idp/plugin/authn/duo/DuoClientException.java
@@ -23,7 +23,7 @@ import javax.annotation.concurrent.ThreadSafe;
* An exception to signal an error condition during execution of a Duo client.
*/
@ThreadSafe
-public class DuoClientException extends Exception{
+public class DuoClientException extends DuoException{
/** Serial UID. */
private static final long serialVersionUID = -2380145079984333546L;
diff --git a/idp-duo-api/src/main/java/net/shbboleth/idp/plugin/authn/duo/DuoClientException.java b/idp-duo-api/src/main/java/net/shbboleth/idp/plugin/authn/duo/DuoException.java
similarity index 81%
copy from idp-duo-api/src/main/java/net/shbboleth/idp/plugin/authn/duo/DuoClientException.java
copy to idp-duo-api/src/main/java/net/shbboleth/idp/plugin/authn/duo/DuoException.java
index 89cab99..65beb15 100644
--- a/idp-duo-api/src/main/java/net/shbboleth/idp/plugin/authn/duo/DuoClientException.java
+++ b/idp-duo-api/src/main/java/net/shbboleth/idp/plugin/authn/duo/DuoException.java
@@ -20,10 +20,10 @@ package net.shbboleth.idp.plugin.authn.duo;
import javax.annotation.concurrent.ThreadSafe;
/**
- * An exception to signal an error condition during execution of a Duo client.
+ * An exception to signal a general Duo error.
*/
@ThreadSafe
-public class DuoClientException extends Exception{
+public class DuoException extends Exception{
/** Serial UID. */
private static final long serialVersionUID = -2380145079984333546L;
@@ -32,7 +32,7 @@ public class DuoClientException extends Exception{
* Constructor.
*
*/
- public DuoClientException() {
+ public DuoException() {
super();
}
@@ -43,7 +43,7 @@ public class DuoClientException extends Exception{
* @param message exception message
* @param cause exception to be wrapped by this one
*/
- public DuoClientException(final String message, final Throwable cause) {
+ public DuoException(final String message, final Throwable cause) {
super(message, cause);
}
@@ -53,7 +53,7 @@ public class DuoClientException extends Exception{
*
* @param message exception message
*/
- public DuoClientException(final String message) {
+ public DuoException(final String message) {
super(message);
}
@@ -63,7 +63,7 @@ public class DuoClientException extends Exception{
*
* @param cause exception to be wrapped by this one
*/
- public DuoClientException(final Throwable cause) {
+ public DuoException(final Throwable cause) {
super(cause);
}
diff --git a/idp-duo-api/src/main/java/net/shbboleth/idp/plugin/authn/duo/DuoRegistryException.java b/idp-duo-api/src/main/java/net/shbboleth/idp/plugin/authn/duo/DuoRegistryException.java
index c5c2a56..af2ac44 100644
--- a/idp-duo-api/src/main/java/net/shbboleth/idp/plugin/authn/duo/DuoRegistryException.java
+++ b/idp-duo-api/src/main/java/net/shbboleth/idp/plugin/authn/duo/DuoRegistryException.java
@@ -20,7 +20,7 @@ package net.shbboleth.idp.plugin.authn.duo;
/**
* Indicates an error during registry processing.
*/
-public class DuoRegistryException extends Exception{
+public class DuoRegistryException extends DuoException{
/** Default serialUID. */
diff --git a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoOIDCAuthnController.java b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoOIDCAuthnController.java
index c73276e..1fd9bc0 100644
--- a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoOIDCAuthnController.java
+++ b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoOIDCAuthnController.java
@@ -35,6 +35,7 @@ import org.springframework.stereotype.Controller;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RequestMapping;
import net.shbboleth.idp.plugin.authn.duo.DuoClientException;
+import net.shbboleth.idp.plugin.authn.duo.DuoException;
import net.shbboleth.idp.plugin.authn.duo.DuoOIDCClient;
import net.shbboleth.idp.plugin.authn.duo.DuoOIDCClientRegistry;
import net.shbboleth.idp.plugin.authn.duo.DuoOIDCIntegration;
@@ -76,7 +77,6 @@ public class DuoOIDCAuthnController extends AbstractInitializableComponent{
/** Lookup strategy to locate the Duo authentication context. */
@Nonnull private Function<ProfileRequestContext,DuoOIDCAuthenticationContext> duoContextLookupStrategy;
-
/** Constructor. */
public DuoOIDCAuthnController() {
@@ -95,24 +95,20 @@ public class DuoOIDCAuthnController extends AbstractInitializableComponent{
duoContextLookupStrategy = Constraint.isNotNull(strategy, "DuoContextLookuplookup strategy cannot be null");
}
-
+ /**
+ * Start the Duo ODIC authorization code flow.
+ *
+ * @param httpRequest servlet request
+ * @param httpResponse servlet response
+ *
+ * @throws ExternalAuthenticationException if an error occurs
+ * @throws IOException if an I/O error occurs
+ */
@GetMapping("/authorize")
public void authorizationRequest(@Nonnull final HttpServletRequest httpRequest,
@Nonnull final HttpServletResponse httpResponse) throws ExternalAuthenticationException, IOException {
- final String key = ExternalAuthentication.startExternalAuthentication(httpRequest);
-
- //put the key in the session, do not create a session, should be one.
- //TODO: where to best stash the key so we can get it back easily. This can NOT be added
- //to the session (like this) safely, as only the last SWF execution key for the same user session
- //would be stored.
- final HttpSession session = httpRequest.getSession(false);
- if (session == null) {
- throw new ExternalAuthenticationException("Request is not associated with an existing Http Session!");
- }
- session.setAttribute(KEY_SESSION_ATTRIBUTE, key);
- log.info("Duo OIDC session has key '{}",session.getAttribute(KEY_SESSION_ATTRIBUTE));
-
+ final String key = ExternalAuthentication.startExternalAuthentication(httpRequest);
final ProfileRequestContext prc = ExternalAuthentication.getProfileRequestContext(key, httpRequest);
final DuoOIDCAuthenticationContext duoContext = duoContextLookupStrategy.apply(prc);
@@ -124,8 +120,8 @@ public class DuoOIDCAuthnController extends AbstractInitializableComponent{
}
final DuoOIDCIntegration integration = duoContext.getIntegration();
- log.trace("Starting Duo 2FA for integration client '{}' and user '{}'",integration.getClientId(),
- duoContext.getUsername());
+ log.trace("Starting Duo 2FA for integration client '{}' and user '{}'",
+ integration.getClientId(), duoContext.getUsername());
try {
final DuoOIDCClient client = duoContext.getClient();
if (client == null) {
@@ -133,8 +129,10 @@ public class DuoOIDCAuthnController extends AbstractInitializableComponent{
}
//generate state, stash in the context for checking on return.
//TODO: could use a type of replay cache and storage service?
- final String state = DuoSupport.generateState(32);
- duoContext.setRequestState(state);
+ final String nonce = DuoSupport.generateNonce(32);
+ final String state = DuoSupport.generateState(nonce, key);
+ //store only the nonce as the request state. The key is only used to resume the flow.
+ duoContext.setRequestState(nonce);
final String authURL = client.createAuthUrl(duoContext.getUsername(), state);
httpResponse.sendRedirect(authURL);
@@ -142,10 +140,18 @@ public class DuoOIDCAuthnController extends AbstractInitializableComponent{
httpRequest.setAttribute(ExternalAuthentication.AUTHENTICATION_EXCEPTION_KEY, e);
ExternalAuthentication.finishExternalAuthentication(key, httpRequest, httpResponse);
}
- //what happens here?
-
+
}
+ /**
+ * The redirect_uri endpoint for accepting an authorization code and resuming the flow execution.
+ *
+ * @param httpRequest servlet request
+ * @param httpResponse servlet response
+ *
+ * @throws ExternalAuthenticationException if an error occurs
+ * @throws IOException if an I/O error occurs
+ */
@GetMapping("/duo-callback")
public void authorizationCallback(@Nonnull final HttpServletRequest httpRequest,
@Nonnull final HttpServletResponse httpResponse) throws ExternalAuthenticationException, IOException {
@@ -155,21 +161,18 @@ public class DuoOIDCAuthnController extends AbstractInitializableComponent{
if (state == null || code == null) {
throw new ExternalAuthenticationException("Duo response must contain a 'code' and 'state' parameter");
}
- final HttpSession session = httpRequest.getSession(false);
- if (session == null) {
- throw new ExternalAuthenticationException("Request is not associated with an existing Http Session, unable"
- + " to resume the flow execution");
- }
-
- final Object keyObject = session.getAttribute(KEY_SESSION_ATTRIBUTE);
- if (keyObject == null || !(keyObject instanceof String)) {
- throw new ExternalAuthenticationException("Flow execution key could not be found in the "
- + "Http session, unable to resume the flow execution");
- }
- log.info("Duo OIDC session has key '{}",session.getAttribute(KEY_SESSION_ATTRIBUTE));
- //from this point onwards, any error can be added back into the flow.
- final String key = StringUtils.trim((String)keyObject);
-
+
+ String key;
+ String nonce;
+ try {
+ key = DuoSupport.extractKeyFromState(state);
+ nonce = DuoSupport.extractNonceFromState(state);
+ } catch (final DuoException e) {
+ throw new ExternalAuthenticationException("Flow execution key component could not be found in the "
+ + "returned state, unable to resume the flow execution",e);
+ }
+ log.trace("Duo OIDC callback has flow execution key '{}'",key);
+
final ProfileRequestContext prc = ExternalAuthentication.getProfileRequestContext(key, httpRequest);
final DuoOIDCAuthenticationContext duoContext = duoContextLookupStrategy.apply(prc);
@@ -181,7 +184,7 @@ public class DuoOIDCAuthnController extends AbstractInitializableComponent{
}
duoContext.setAuthorizationCode(code);
- duoContext.setResponseState(state);
+ duoContext.setResponseState(nonce);
ExternalAuthentication.finishExternalAuthentication(key, httpRequest, httpResponse);
}
diff --git a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoSupport.java b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoSupport.java
index 6ffccf8..14292ed 100644
--- a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoSupport.java
+++ b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoSupport.java
@@ -20,7 +20,12 @@ package net.shibboleth.idp.plugin.authn.duo.impl;
import java.security.SecureRandom;
import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+import org.apache.commons.codec.DecoderException;
+import org.apache.commons.codec.binary.Hex;
+
+import net.shbboleth.idp.plugin.authn.duo.DuoException;
import net.shibboleth.utilities.java.support.logic.Constraint;
/**
@@ -35,14 +40,14 @@ public final class DuoSupport {
}
/**
- * Generates a random identifier to be used as the state variable in Duo 2FA requests.
+ * Generates a random identifier to be used as a nonce.
*
- * @param length the length of the parameter, Duo requires a minimum 32 character state value.
+ * @param length the length of the parameter, minimum allowed is 22.
*
- * @return the randomly generated state value.
+ * @return the randomly generated nonce value.
*/
- @Nonnull public static String generateState(@Nonnull final Integer length) {
- Constraint.isGreaterThan(31, length, "State must be at least 32 characters");
+ @Nonnull public static String generateNonce(@Nonnull final Integer length) {
+ Constraint.isGreaterThan(22, length, "State must be at least 22 characters");
final SecureRandom secureRandom = new SecureRandom();
final StringBuilder sb = new StringBuilder();
while(sb.length() < length){
@@ -50,5 +55,75 @@ public final class DuoSupport {
}
return sb.toString().substring(0, length);
}
+
+
+ /**
+ * <p>Generate a state parameter from a nonce component and an execution key component.</p>
+ *
+ * <p>The nonce is separated from the key by a dot e.g. {@literal <nonce>.<keyHex>}.</p>
+ *
+ * <p>The nonce is assumed to be already encoded in its transmission format. The key is
+ * hex encoded before it is combined with the nonce.</p>
+ *
+ * @param nonceHex the nonce component.
+ * @param key the key component. The key is hex encoded before it is added to the generated state.
+ *
+ * @return the combined state component.
+ */
+ @Nonnull public static String generateState(@Nonnull final String nonce, @Nonnull final String key) {
+ Constraint.isNotNull(nonce, "NonceHex key can not be null");
+ Constraint.isNotNull(key, "Webflow execution key can not be null");
+
+ final String keyHex = Hex.encodeHexString(key.getBytes());
+ return nonce+"."+keyHex;
+ }
+
+ /**
+ * Extract the key component from the state. The key is hex encoded and separated from the
+ * nonce value by a dot.
+ *
+ * @param state the state which contains both the nonce and the key dot separated.
+ *
+ * @return the key extracted from the state and hex decoded.
+ *
+ * @throws DuoException if the key component can not be found, or hex decoding fails.
+ */
+ @Nonnull public static String extractKeyFromState(@Nonnull final String state) throws DuoException{
+ Constraint.isNotNull(state, "State can not be null");
+
+ final String[] stateSplit = state.split("\\.");
+ if (stateSplit.length!=2) {
+ throw new DuoException("State does not contain the key component");
+ }
+ final String hexKey = stateSplit[1];
+ try {
+ //should we check it is sensible?
+ return new String(Hex.decodeHex(hexKey));
+ } catch (final DecoderException e) {
+ throw new DuoException("Can not hex decode key",e);
+ }
+
+ }
+
+ /**
+ * Extract the nonce component from the state. The nonce is separated from the key
+ * by a dot, and is assumed to be the first value of the pair.
+ *
+ * @param state the state which contains both the nonce and the key dot separated.
+ *
+ * @return the nonce extracted from the state.
+ *
+ * @throws DuoException if the nonce component can not be found.
+ */
+ public static String extractNonceFromState(@Nonnull final String state) throws DuoException{
+ Constraint.isNotNull(state, "State can not be null");
+
+ final String[] stateSplit = state.split("\\.");
+ if (stateSplit.length!=2) {
+ throw new DuoException("State does not contain the nonce component");
+ }
+ final String nonce = stateSplit[0];
+ return nonce;
+ }
}
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 d4ad6ab..139e68f 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
@@ -35,9 +35,6 @@
<!-- Default username comes from previous c14n or session. -->
<bean id="shibboleth.authn.duo.OIDC.UsernameLookupStrategy"
class="net.shibboleth.idp.session.context.navigate.CanonicalUsernameLookupStrategy" />
-
- <!-- FIXME: For current SWF flow tests the client registry and factory beans from postconfig.xml
- need to be copied into here, will fix. -->
<!-- Duo Client factory and bean registry -->
<bean id="shibboleth.authn.duo.OIDC.clientRegistry" scope="singleton"
diff --git a/idp-duo-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/DuoOIDC/duo-oidc-authn-flow.xml b/idp-duo-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/DuoOIDC/duo-oidc-authn-flow.xml
index 74a3a03..4b68345 100644
--- a/idp-duo-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/DuoOIDC/duo-oidc-authn-flow.xml
+++ b/idp-duo-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/DuoOIDC/duo-oidc-authn-flow.xml
@@ -7,7 +7,7 @@
<!-- TODO: throws an AuthnException if the endpoint is not healthy, no backoff etc. -->
<action-state id="CheckDuoOIDCAuthAPI">
<evaluate expression="PopulateDuoAuthenticationContext" />
- <evaluate expression="HealthCheckDuoOIDCAuthAPI" />
+ <evaluate expression="HealthCheckDuoOIDCAuthAPI" />
<evaluate expression="'proceed'" />
<transition on="proceed" to="Duo2FAAuthorizationRequest" />
</action-state>
diff --git a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoAuthnFlowTest.java b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoAuthnFlowTest.java
index 5efa3c6..d8c6ee3 100644
--- a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoAuthnFlowTest.java
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoAuthnFlowTest.java
@@ -23,8 +23,6 @@ import java.util.Map;
import javax.annotation.Nonnull;
-import org.springframework.core.io.ClassPathResource;
-import org.springframework.core.io.Resource;
import org.junit.Test;
import org.opensaml.profile.context.ProfileRequestContext;
import org.slf4j.Logger;
@@ -294,10 +292,10 @@ public class DuoAuthnFlowTest extends AbstractAuthnXmlFlowExecutionTests {
final ProfileRequestContext prc = buildProfileRequestContext(false);
//add a DuoContext
final DuoOIDCAuthenticationContext duoContext = new DuoOIDCAuthenticationContext();
- final String state = DuoSupport.generateState(32);
+ final String nonce = DuoSupport.generateNonce(32);
duoContext.setAuthorizationCode("adummycode");
- duoContext.setRequestState(state);
- duoContext.setResponseState(state);
+ duoContext.setRequestState(nonce);
+ duoContext.setResponseState(nonce);
duoContext.setUsername("jdoe");
final DefaultDuoOIDCIntegration integ = new DefaultDuoOIDCIntegration();
diff --git a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoOIDCAuthnControllerTest.java b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoOIDCAuthnControllerTest.java
index 2c29e06..dec90f3 100644
--- a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoOIDCAuthnControllerTest.java
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoOIDCAuthnControllerTest.java
@@ -27,6 +27,7 @@ import static org.testng.Assert.assertTrue;
import javax.annotation.Nonnull;
import javax.servlet.ServletContext;
+import org.apache.commons.codec.binary.Hex;
import org.mockito.Mockito;
import org.opensaml.profile.action.EventIds;
import org.opensaml.profile.context.ProfileRequestContext;
@@ -69,6 +70,7 @@ import net.shibboleth.idp.plugin.authn.util.mock.IdPPropertyConfigurer;
import net.shibboleth.idp.session.IdPSession;
import net.shibboleth.idp.session.context.SessionContext;
import net.shibboleth.idp.ui.context.RelyingPartyUIContext;
+import net.shibboleth.utilities.java.support.annotation.constraint.NonnullAfterInit;
/**
* Tests for the {@link DuoOIDCAuthnController}.
@@ -80,6 +82,18 @@ public class DuoOIDCAuthnControllerTest extends AbstractTestNGSpringContextTests
/** Duo host the integration uses, and redirects should goto.*/
@Nonnull private final String API_HOST = "duo.host.com";
+
+ /** SWF key.*/
+ @Nonnull private final String KEY = "e1s1";
+
+ /** Duo OIDC nonce component of the state parameter.*/
+ @Nonnull private String NONCE = "8821febf7b3237fc3974f76a75f37e8c";
+
+ /** Duo OIDC state parameter.*/
+ @NonnullAfterInit private String state;
+
+ /** Duo OIDC authz code.*/
+ @Nonnull private final String CODE = "db87f45083db5aea16eb45b7ace685c8";
/** Class logger. */
@Nonnull private final Logger log = LoggerFactory.getLogger(DuoOIDCAuthnControllerTest.class);
@@ -93,8 +107,6 @@ public class DuoOIDCAuthnControllerTest extends AbstractTestNGSpringContextTests
/** The web application context loaded by the test framework. */
@Nonnull @Autowired private WebApplicationContext webApplicationContext;
- /** The mock http session.*/
- @Nonnull private MockHttpSession session;
/**
* Setup.
@@ -104,11 +116,12 @@ public class DuoOIDCAuthnControllerTest extends AbstractTestNGSpringContextTests
@BeforeMethod
public void setUp() throws Exception {
+ state = NONCE+"."+Hex.encodeHexString(KEY.getBytes());
+
// check controller is instantiated.
final DuoOIDCAuthnController controller = webApplicationContext.getBean(DuoOIDCAuthnController.class);
assertNotNull(controller);
- session = new MockHttpSession();
mockMvc = MockMvcBuilders.webAppContextSetup(webApplicationContext).build();
//add and mock attributes of the servlet context.
@@ -123,12 +136,10 @@ public class DuoOIDCAuthnControllerTest extends AbstractTestNGSpringContextTests
@Test
public void testSuccessfulAuthorizeRequest() throws Exception {
- MvcResult result = mockMvc.perform(get("/Authn/Duo/2FA/authorize").session(session).param("conversation", "e1s1")).andDo(print())
+ MvcResult result = mockMvc.perform(get("/Authn/Duo/2FA/authorize").param("conversation", "e1s1")).andDo(print())
.andExpect(status().is3xxRedirection()).andReturn();
assertNotNull(result.getResponse().getHeader("Location"));
assertTrue(result.getResponse().getHeader("Location").contains(API_HOST));
- //assert the key is in the session
- assertNotNull(session.getAttribute(DuoOIDCAuthnController.KEY_SESSION_ATTRIBUTE));
}
@@ -140,22 +151,19 @@ public class DuoOIDCAuthnControllerTest extends AbstractTestNGSpringContextTests
*/
@Test
public void testSuccessfulCallback() throws Exception {
-
- session.setAttribute(DuoOIDCAuthnController.KEY_SESSION_ATTRIBUTE, "e1s1");
+
mockMvc.perform(get("/Authn/Duo/2FA/duo-callback").
- session(session).
- param("code", "db87f45083db5aea16eb45b7ace685c8").
- param("state",
- "8821febf7b3237fc3974f76a75f37e8c")).
+ param("code", CODE).
+ param("state",state)).
andDo(print()).
andExpect(status().
is3xxRedirection());
- //check the duo context is populated correctly.
-
+ //check the duo context is populated correctly.
DuoOIDCAuthenticationContext duoContext = extractDuoContext();
- assertEquals("db87f45083db5aea16eb45b7ace685c8",duoContext.getAuthorizationCode());
- assertEquals("8821febf7b3237fc3974f76a75f37e8c", duoContext.getResponseState());
+ assertEquals(CODE,duoContext.getAuthorizationCode());
+ //the state we aim to preserve is just the nonce component of the state parameter
+ assertEquals(NONCE, duoContext.getResponseState());
}
@@ -169,12 +177,10 @@ public class DuoOIDCAuthnControllerTest extends AbstractTestNGSpringContextTests
public void testCallbackNoDuoAuthenticationContext() throws Exception {
removeDuoContext();
- session.setAttribute(DuoOIDCAuthnController.KEY_SESSION_ATTRIBUTE, "e1s1");
- final MvcResult result = mockMvc.perform(get("/Authn/Duo/2FA/duo-callback").
- session(session)
- .param("code", "db87f45083db5aea16eb45b7ace685c8").
- param("state",
- "8821febf7b3237fc3974f76a75f37e8c")).
+
+ final MvcResult result = mockMvc.perform(get("/Authn/Duo/2FA/duo-callback")
+ .param("code", CODE).
+ param("state", state)).
andDo(print()).
andExpect(status().
is3xxRedirection()).
@@ -199,7 +205,7 @@ public class DuoOIDCAuthnControllerTest extends AbstractTestNGSpringContextTests
try {
mockMvc.perform(get("/Authn/Duo/2FA/duo-callback").param("state",
- "8821febf7b3237fc3974f76a75f37e8c")).andDo(print());
+ state)).andDo(print());
} catch (final NestedServletException e) {
assertTrue(e.getCause() instanceof ExternalAuthenticationException);
}
@@ -215,7 +221,7 @@ public class DuoOIDCAuthnControllerTest extends AbstractTestNGSpringContextTests
public void testCallbackNoState() throws Exception {
try {
mockMvc.perform(get("/Authn/Duo/2FA/duo-callback").
- param("code", "db87f45083db5aea16eb45b7ace685c8")).
+ param("code", CODE)).
andDo(print());
} catch (final NestedServletException e) {
diff --git a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoSupportTest.java b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoSupportTest.java
new file mode 100644
index 0000000..6ef73bb
--- /dev/null
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoSupportTest.java
@@ -0,0 +1,27 @@
+package net.shibboleth.idp.plugin.authn.duo.impl;
+
+import static org.junit.Assert.assertEquals;
+
+import org.testng.annotations.Test;
+
+import net.shbboleth.idp.plugin.authn.duo.DuoException;
+
+/**
+ * Tests for the DuoSupport class.
+ */
+public class DuoSupportTest {
+
+ @Test public void testCreateAndExtractKeyAndNonceFromState() throws DuoException {
+
+ final String key = "e1s1";
+ final String nonce = DuoSupport.generateNonce(32);
+ final String state = DuoSupport.generateState(nonce, key);
+ final String extractedKey = DuoSupport.extractKeyFromState(state);
+ final String extractedNonce = DuoSupport.extractNonceFromState(state);
+ assertEquals(key,extractedKey);
+ assertEquals(nonce,extractedNonce);
+
+ }
+
+
+}
diff --git a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoResponseStateTest.java b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoResponseStateTest.java
index 23a28f9..66b17ca 100644
--- a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoResponseStateTest.java
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoResponseStateTest.java
@@ -47,9 +47,9 @@ public class ValidateDuoResponseStateTest extends AbstractDuoActionTest{
addDuoContext();
addDuoIntegrationToContext();
addAttemptedFlow("authn/DuoOIDC");
- final String state = DuoSupport.generateState(32);
- dc.setRequestState(state);
- dc.setResponseState(state);
+ final String nonce = DuoSupport.generateNonce(32);
+ dc.setRequestState(nonce);
+ dc.setResponseState(nonce);
action.initialize();
final Event event = action.execute(src);
//null event is success.
@@ -73,7 +73,7 @@ public class ValidateDuoResponseStateTest extends AbstractDuoActionTest{
addDuoContext();
addDuoIntegrationToContext();
addAttemptedFlow("authn/DuoOIDC");
- dc.setRequestState(DuoSupport.generateState(32));
+ dc.setRequestState(DuoSupport.generateNonce(32));
action.initialize();
final Event event = action.execute(src);
assertEventId(event,AuthnEventIds.AUTHN_EXCEPTION);
@@ -85,7 +85,7 @@ public class ValidateDuoResponseStateTest extends AbstractDuoActionTest{
addDuoContext();
addDuoIntegrationToContext();
addAttemptedFlow("authn/DuoOIDC");
- dc.setResponseState(DuoSupport.generateState(32));
+ dc.setResponseState(DuoSupport.generateNonce(32));
action.initialize();
final Event event = action.execute(src);
assertEventId(event,AuthnEventIds.AUTHN_EXCEPTION);
diff --git a/idp-duo-native-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/sdk/impl/DuoSDKClientFactory.java b/idp-duo-native-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/sdk/impl/DuoSDKClientFactory.java
index 6c73fb9..26bbcaf 100644
--- a/idp-duo-native-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/sdk/impl/DuoSDKClientFactory.java
+++ b/idp-duo-native-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/sdk/impl/DuoSDKClientFactory.java
@@ -5,6 +5,9 @@ import java.util.List;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
import net.shbboleth.idp.plugin.authn.duo.DuoClientException;
import net.shbboleth.idp.plugin.authn.duo.DuoOIDCClient;
import net.shbboleth.idp.plugin.authn.duo.DuoOIDCClientFactory;
@@ -16,6 +19,8 @@ import net.shbboleth.idp.plugin.authn.duo.DuoOIDCIntegration;
*/
public class DuoSDKClientFactory implements DuoOIDCClientFactory{
+ /** Class logger. */
+ @Nonnull private final Logger log = LoggerFactory.getLogger(DuoSDKClientFactory.class);
/** List of CA Certificate pins. If null, the clients default set are used.*/
@Nullable private List<String> caCerts;
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list