Merge pull request #29 from Unicon/3.2.3

fixed the embedded Id validation issue

README.md

@@ -127,11 +127,13 @@ See [here](https://github.com/Unicon/shib-cas-authn3/releases/).
 
 Developer Notes
 -------------------------------------------------------------
+The project distributables can be built using `./gradlew`. The artifacts will be in `build/distributions`.
+
 This project includes a Docker environment to assist with development/testing. 
 
 To build and execute: `./gradlew clean; ./gradlew up`
 Then browse to: `https://idptestbed/idp/profile/SAML2/Unsolicited/SSO?providerId=https://sp.idptestbed/shibboleth`
 
 > You'll need a `hosts` file entry that points `idptestbed` to your Docker server's IP address. 
 
-The IdP only had a session of 1 minute (to test expired session/conversation key issues), so login into CAS Server quickly.
+The IdP only has a session of 1 minute (to test expired session/conversation key issues), so login into CAS Server quickly.

build.gradle

@@ -3,9 +3,10 @@ apply plugin: 'idea'
 apply plugin: 'java'
 apply plugin: 'maven'
 
-defaultTasks 'clean', 'build'
+defaultTasks 'clean', 'distZip', 'distTar'
 
-sourceCompatibility = 1.7
+sourceCompatibility = 1.8
+targetCompatibility = 1.8
 
 group = 'net.unicon'
 version = project.version
@@ -99,7 +100,7 @@ task prepImageFiles() {
     }
 }
 
-task buildImage(type:Exec) {
+task buildImage(type: Exec) {
     dependsOn prepImageFiles
 
     doFirst {
@@ -110,29 +111,30 @@ task buildImage(type:Exec) {
     commandLine 'docker-compose', 'build'
 }
 
-task up(type:Exec) {
+task up(type: Exec) {
     dependsOn buildImage
 
     workingDir 'build/docker'
     commandLine 'docker-compose', 'up', '-d'
 }
 
-task stop(type:Exec) {
+task stop(type: Exec) {
     workingDir 'build/docker'
     commandLine 'docker-compose', 'kill'
 }
 
-task down(type:Exec) {
+task down(type: Exec) {
     dependsOn stop
 
     workingDir 'build/docker'
     commandLine 'docker-compose', 'rm', '-f'
 }
 
-clean {
-    dependsOn down
+if (new File('./build/docker/').exists()) {
+    clean.dependsOn down
 }
 
+
 task wrapper(type: Wrapper) {
     gradleVersion = project.gradleVersion
 }

gradle.properties

@@ -1,6 +1,6 @@
 gradleVersion=3.3
 
-version=3.2.2
+version=3.2.3
 
 casClientVersion=3.4.1
 commonLangVersion=2.5

src/main/java/net/unicon/idp/externalauth/ShibcasAuthServlet.java

@@ -35,6 +35,7 @@
  *
  * @author [email protected]
  * @author [email protected]
+ * @author aremmes (GitHub)
  */
 @WebServlet(name = "ShibcasAuthServlet", urlPatterns = {"/Authn/ExtCas/*"})
 public class ShibcasAuthServlet extends HttpServlet {
@@ -93,7 +94,7 @@ private void validateCasTicket(final HttpServletRequest request, final HttpServl
                                    final String authenticationKey, final boolean force) throws ExternalAuthenticationException, IOException {
         try {
             ticketValidator.setRenew(force);
-            String serviceUrl = constructServiceUrl(request, response);
+            String serviceUrl = constructServiceUrl(request, response, true);
             logger.debug("validating ticket: {} with service url: {}", ticket, serviceUrl);
 
             Assertion assertion = ticketValidator.validate(ticket, serviceUrl);
@@ -245,7 +246,7 @@ private void buildTranslators(Environment environment) {
     /**
      * Use the CAS CommonUtils to build the CAS Service URL.
      */
-    private String constructServiceUrl(final HttpServletRequest request, final HttpServletResponse response) {
+    protected String constructServiceUrl(final HttpServletRequest request, final HttpServletResponse response) {
         String serviceUrl = CommonUtils.constructServiceUrl(request, response, null, serverName, serviceParameterName, artifactParameterName, true);
 
         if ("embed".equalsIgnoreCase(entityIdLocation)) {
@@ -255,6 +256,18 @@ private String constructServiceUrl(final HttpServletRequest request, final HttpS
         return serviceUrl;
     }
 
+    /**
+      * Like the above, but with a flag indicating whether we're validating a service ticket,
+      * in which case we should not modify the service URL returned by CAS CommonUtils; this
+      * avoids appending the entity ID twice when entityIdLocation=embed, since the ID is already
+      * embedded in the string during validation.
+      */
+    protected String constructServiceUrl(final HttpServletRequest request, final HttpServletResponse response, final boolean isValidatingTicket ) {
+        return isValidatingTicket
+                ? CommonUtils.constructServiceUrl(request, response, null, serverName, serviceParameterName, artifactParameterName, true)
+                : constructServiceUrl(request, response);
+    }
+
     private void loadErrorPage(final HttpServletRequest request, final HttpServletResponse response) {
         RequestDispatcher requestDispatcher = request.getRequestDispatcher("/no-conversation-state.jsp");
         try {

src/test/java/net/unicon/idp/externalauth/ShibcasAuthServletTest.java

@@ -26,6 +26,7 @@
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
+import static org.junit.Assert.assertEquals;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyString;
 import static org.mockito.Matchers.eq;
@@ -37,11 +38,13 @@ public class ShibcasAuthServletTest {
     private String CONVERSATION = "conversation=e1s1";
     private String CONVERSATION_TICKET = "conversation=e1s1&ticket=ST-1234-123456789-a";
     private String CONVERSATION_TICKET_GATEWAY_ATTEMPTED = "conversation=e1s1&ticket=ST-1234-123456789-a&gatewayAttempted=true";
+    private String CONVERSATION_TICKET_EMBEDDED_ENTITYID = "conversation=e1s1&ticket=ST-1234-123456789-a&entityId=http://test.edu/sp";
     private String E1S1 = "E1S1";
     private String JDOE = "jdoe";
     private String TICKET = "ST-1234-123456789-a";
     private String URL_WITH_CONVERSATION = "https://shibserver.example.edu/idp/Authn/ExtCas?conversation=e1s1";
     private String URL_WITH_CONVERSATION_GATEWAY_ATTEMPTED = "https://shibserver.example.edu/idp/Authn/ExtCas?conversation=e1s1&gatewayAttempted=true";
+    private String URL_WITH_CONVERSATION_EMBEDDED_ENTITYID = "https://shibserver.example.edu/idp/Authn/ExtCas?conversation=e1s1&entityId=http%3A%2F%2Ftest.edu%2Fsp";
 
     @Test
     public void testDoGetStandard() throws Exception {
@@ -213,6 +216,36 @@ public void testDoGetPassiveAndForced() throws Exception {
     }
 
 
+    @Test
+    public void testConstructServiceUrlAppend() throws Exception {
+        //Mock some objects.
+        HttpServletRequest request = createDoGetHttpServletRequest(CONVERSATION_TICKET, TICKET, "false");
+        HttpServletResponse response = createMockHttpServletResponse();
+
+        //Prep our object
+        ShibcasAuthServlet shibcasAuthServlet = createShibcasAuthServlet();
+
+        String result = shibcasAuthServlet.constructServiceUrl(request, response);
+
+        assertEquals("https://shibserver.example.edu/idp/Authn/ExtCas?conversation=e1s1", result);
+    }
+
+    @Test
+    public void testConstructServiceUrlEmbed() throws Exception {
+        //Mock some objects.
+        HttpServletRequest request = createDoGetHttpServletRequest(CONVERSATION_TICKET + "&entityId=http://test.edu/sp", TICKET, "false");
+        HttpServletResponse response = createMockHttpServletResponse();
+
+        //Prep our object
+        ShibcasAuthServlet shibcasAuthServlet = createShibcasAuthServlet();
+        shibcasAuthServlet.init(createMockServletConfig("embed"));
+
+        String result = shibcasAuthServlet.constructServiceUrl(request, response, true);
+
+        assertEquals("https://shibserver.example.edu/idp/Authn/ExtCas?conversation=e1s1&entityId=http%3A%2F%2Ftest.edu%2Fsp", result);
+    }
+
+
     @Test
     public void testStartLoginRequestStandard() throws Exception {
         HttpServletRequest request = createMockHttpServletRequest();
@@ -380,6 +413,7 @@ private HttpServletResponse createMockHttpServletResponse() {
 
         BDDMockito.given(response.encodeURL(URL_WITH_CONVERSATION)).willReturn(URL_WITH_CONVERSATION);
         BDDMockito.given(response.encodeURL(URL_WITH_CONVERSATION_GATEWAY_ATTEMPTED)).willReturn(URL_WITH_CONVERSATION_GATEWAY_ATTEMPTED);
+        BDDMockito.given(response.encodeURL(URL_WITH_CONVERSATION_EMBEDDED_ENTITYID)).willReturn(URL_WITH_CONVERSATION_EMBEDDED_ENTITYID);
 
         return response;
     }