[java-idp-plugin-duo] branch main updated: JDUO-24 - Duo's WebSDK now requests the authorization code as 'duo_code'

Phil Smart philip.smart at jisc.ac.uk
Tue Apr 20 10:52:55 UTC 2021


This is an automated email from the git hooks/post-receive script.

philsmart 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=bf0a95b545a2f352471ff39545b082683eb7038b

The following commit(s) were added to refs/heads/main by this push:
       new  bf0a95b   JDUO-24 - Duo's WebSDK now requests the authorization code as 'duo_code'
bf0a95b is described below

commit bf0a95b545a2f352471ff39545b082683eb7038b
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Tue Apr 20 11:52:51 2021 +0100

    JDUO-24 - Duo's WebSDK now requests the authorization code as 'duo_code'
    
    Updated the websdk to 1.1.1 and reverted duo_code back to code.
    
    https://issues.shibboleth.net/jira/browse/JDUO-24
---
 .../authn/duo/impl/DuoOIDCAuthnController.java     | 19 +----
 .../authn/duo/impl/DuoOIDCAuthnControllerTest.java | 87 +++++++++++-----------
 .../authn/duo/sdk/impl/DuoSDKClientAdaptor.java    | 10 +--
 pom.xml                                            |  2 +-
 4 files changed, 51 insertions(+), 67 deletions(-)

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 c322256..9f193f0 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
@@ -68,12 +68,6 @@ public class DuoOIDCAuthnController extends AbstractInitializableComponent{
     /** The name of the Http parameter that stores the authorisation code.*/
     @Nonnull @NotEmpty public static final String CODE_PARAMETER = "code";
     
-    /** 
-     * The name of the Http parameter that stores the authorisation code
-     * when using the Duo WebSDK client.
-     */
-    @Nonnull @NotEmpty public static final String DUO_CODE_PARAMETER = "duo_code";
-    
     /** The name of the Http parameter that stores the state value.*/
     @Nonnull @NotEmpty public static final String STATE_PARAMETER = "state";    
     
@@ -192,17 +186,10 @@ public class DuoOIDCAuthnController extends AbstractInitializableComponent{
         
         final String code = httpRequest.getParameter(CODE_PARAMETER);
         final String state = httpRequest.getParameter(STATE_PARAMETER);
-        
-        //if duo's webSDK becomes OAuth2.0 complaint again, remove this code path
-        final String duoCode = httpRequest.getParameter(DUO_CODE_PARAMETER);
-        
-        if (state == null || (code == null && duoCode == null)) {
+               
+        if (state == null || code == null) {
             throw new ExternalAuthenticationException("Duo response must contain a 'code' and 'state' parameter");
         }
-        if (code != null && duoCode != null) {
-            throw new ExternalAuthenticationException("Duo response can not contain both a 'code' and "
-                    + "'duo_code' parameter");
-        }
        
         final String key;
         final String nonce;
@@ -224,7 +211,7 @@ public class DuoOIDCAuthnController extends AbstractInitializableComponent{
             return;
             
         }
-        duoContext.setAuthorizationCode(code != null ? code : duoCode);
+        duoContext.setAuthorizationCode(code);
         duoContext.setResponseState(nonce);
         ExternalAuthentication.finishExternalAuthentication(key, httpRequest, httpResponse);
         
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 046c084..669df02 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
@@ -125,50 +125,7 @@ public class DuoOIDCAuthnControllerTest extends AbstractTestNGSpringContextTests
         //add and mock attributes of the servlet context.
         exportServletContextAttributes();
     }
-    
-    /**
-     * Test specific to the Duo WebSDK's use of the duo_code parameter to store the
-     * OAuth2.0 auth code. 
-     * 
-     * @throws Exception on error.
-     */
-    @Test
-    public void testCallbackDuoCode() throws Exception {
-        
-        mockMvc.perform(get("/Authn/Duo/2FA/duo-callback").
-                param("duo_code", CODE).
-                param("state",state)).
-        andDo(print()).
-        andExpect(status().
-                is3xxRedirection());
-        
-        //check the duo context is populated correctly.       
-        DuoOIDCAuthenticationContext duoContext = extractDuoContext();
-        assertEquals(CODE,duoContext.getAuthorizationCode());
-        //the state we aim to preserve is just the nonce component of the state parameter
-        assertEquals(NONCE, duoContext.getResponseState());
-    }
-    
-    /**
-     * Test specific to the Duo WebSDK's use of the duo_code parameter to store the
-     * OAuth2.0 auth code. The controller should not accept both a duo_code and code
-     * parameter at the same time.
-     * 
-     * @throws Exception on error.
-     */
-    @Test
-    public void testCallbackDuoCodeAndCode() throws Exception {
-        try {
-        mockMvc.perform(get("/Authn/Duo/2FA/duo-callback").
-                param("duo_code", CODE).
-                param("code", CODE).
-                param("state",state)).
-        andDo(print());
-        } catch (final NestedServletException e) {
-            assertTrue(e.getCause() instanceof ExternalAuthenticationException); 
-        }
-            
-    }
+
 
     /**
      * Start a Duo 2FA request. 
@@ -187,6 +144,44 @@ public class DuoOIDCAuthnControllerTest extends AbstractTestNGSpringContextTests
         assertTrue(result.getResponse().getHeader("Location").contains(API_HOST));
 
     }
+    
+    /**
+     * Checks that a state response with no key component triggers and error.
+     * 
+     * @throws Exception on exception.
+     */
+    @Test
+    public void testIncorrectStateNoExecutionKey() throws Exception {
+        try {  
+            final String invalidState = NONCE+".";
+            mockMvc.perform(get("/Authn/Duo/2FA/duo-callback")
+                    .param("code", CODE)
+                    .param("state",invalidState))
+            .andDo(print());
+         }  catch (final NestedServletException e) {
+             assertTrue(e.getCause() instanceof ExternalAuthenticationException);
+             log.error("{}",e.getRootCause().getMessage());
+         }  
+    }
+    
+    /**
+     * Checks that a state response with no nonce component triggers and error.
+     * 
+     * @throws Exception on exception.
+     */
+    @Test
+    public void testIncorrectStateNoNonce() throws Exception {
+        try {  
+            final String invalidState = "."+KEY;
+            mockMvc.perform(get("/Authn/Duo/2FA/duo-callback")
+                    .param("code", CODE)
+                    .param("state",invalidState))
+            .andDo(print());
+         }  catch (final NestedServletException e) {
+             assertTrue(e.getCause() instanceof ExternalAuthenticationException);
+             log.error("{}",e.getRootCause().getMessage());
+         }  
+    }
 
     /**
      * Ensure the 2FA end controller adds the correct Duo response information into the 
@@ -205,7 +200,7 @@ public class DuoOIDCAuthnControllerTest extends AbstractTestNGSpringContextTests
                 is3xxRedirection());
         
         //check the duo context is populated correctly.       
-        DuoOIDCAuthenticationContext duoContext = extractDuoContext();
+        final DuoOIDCAuthenticationContext duoContext = extractDuoContext();
         assertEquals(CODE,duoContext.getAuthorizationCode());
         //the state we aim to preserve is just the nonce component of the state parameter
         assertEquals(NONCE, duoContext.getResponseState());
@@ -253,6 +248,7 @@ public class DuoOIDCAuthnControllerTest extends AbstractTestNGSpringContextTests
                     state)).andDo(print());
          }  catch (final NestedServletException e) {
              assertTrue(e.getCause() instanceof ExternalAuthenticationException);
+             log.error("{}",e.getMessage());
          }    
         
     }
@@ -271,6 +267,7 @@ public class DuoOIDCAuthnControllerTest extends AbstractTestNGSpringContextTests
            
         } catch (final NestedServletException e) {
             assertTrue(e.getCause() instanceof ExternalAuthenticationException); 
+            log.error("{}",e.getMessage());
         }
         
     }
diff --git a/idp-duo-sdk-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/sdk/impl/DuoSDKClientAdaptor.java b/idp-duo-sdk-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/sdk/impl/DuoSDKClientAdaptor.java
index 0574c60..a6f016d 100644
--- a/idp-duo-sdk-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/sdk/impl/DuoSDKClientAdaptor.java
+++ b/idp-duo-sdk-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/sdk/impl/DuoSDKClientAdaptor.java
@@ -100,12 +100,12 @@ public final class DuoSDKClientAdaptor extends AbstractDuoOIDCClient{
         try {
             if (caCerts == null) {
                 //will use the default certs in the Client if the caCerts are null
-                client = new Client(integration.getClientId(), integration.getSecretKey(),
-                    integration.getAPIHost(), integration.getRedirectURI());
+                client = new Client.Builder(integration.getClientId(), integration.getSecretKey(),
+                        integration.getAPIHost(), integration.getRedirectURI()).setUseDuoCodeAttribute(false).build();
             } else {
-                client = new Client(integration.getClientId(), integration.getSecretKey(),
-                        integration.getAPIHost(), integration.getRedirectURI(), 
-                        caCerts.toArray(new String[caCerts.size()]));
+                client = new Client.Builder(integration.getClientId(), integration.getSecretKey(),
+                        integration.getAPIHost(), integration.getRedirectURI()).setCACerts(caCerts.toArray(new String[caCerts.size()]))
+                        .setUseDuoCodeAttribute(false).build();
             }
         } catch (final DuoException e) {
             //wrap exception and throw
diff --git a/pom.xml b/pom.xml
index c25de4d..2ec84a2 100644
--- a/pom.xml
+++ b/pom.xml
@@ -22,7 +22,7 @@
     <properties>
         <idp.groupId>net.shibboleth.idp</idp.groupId>
         <idp.version>4.1.0</idp.version>
-        <duo.client.version>1.0.3</duo.client.version>
+        <duo.client.version>1.1.1</duo.client.version>
         <opensaml.groupId>org.opensaml</opensaml.groupId>
         <opensaml.version>4.1.0</opensaml.version>
         <java-support.version>8.2.0</java-support.version>      

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list