KNOX-2973 - Fix redirect URI when host and port are query params of originalUrl

gateway-service-knoxsso/src/main/java/org/apache/knox/gateway/service/knoxsso/WebSSOResource.java

@@ -347,7 +347,7 @@ private Response getAuthenticationToken(int statusCode) {
     return Response.seeOther(location).entity("{ \"redirectTo\" : " + original + " }").build();
   }
 
-  private String getOriginalUrlFromQueryParams() {
+  protected String getOriginalUrlFromQueryParams() {
     String original = request.getParameter(ORIGINAL_URL_REQUEST_PARAM);
     StringBuilder buf = new StringBuilder(original);
 
@@ -362,7 +362,8 @@ private String getOriginalUrlFromQueryParams() {
           && !original.contains(entry.getKey() + "=")
           && !ssoExpectedparams.contains(entry.getKey())) {
 
-        if(first) {
+        /* Only add ? if not already present. See KNOX-2973 */
+        if(first && (buf.lastIndexOf("?") == -1) ) {
           buf.append('?');
           first = false;
         }

gateway-service-knoxsso/src/test/java/org/apache/knox/gateway/service/knoxsso/WebSSOResourceTest.java

@@ -771,6 +771,114 @@ public void testConcurrentSessionLimitHit() throws Exception {
     Assert.assertThrows(WebApplicationException.class, webSSOResponse::doPost);
   }
 
+  /**
+   * Test for cases where originalUrl has host and port parameters.
+   * @throws Exception
+   */
+  @Test
+  public void testGetOriginalUrlFromQueryParamsWithHostPort() throws Exception {
+    /* &port=1001 is missing in the original URL because it gets stripped
+    out during request.getParameter(ORIGINAL_URL_REQUEST_PARAM) operation */
+    final String ORIG_URL_VALUE = "https://local.site/gateway/sandbox/hbase/webui/master?host=local.site";
+    final String ORIGINAL_URL= "originalUrl="+ORIG_URL_VALUE;
+    final String PORT_VALUE = "1001";
+    Map<String, String[]> params = new HashMap<>();
+    params.put("originalUrl", new String[] {ORIG_URL_VALUE});
+    params.put("port", new String[] {PORT_VALUE});
+
+    HttpServletRequest req = EasyMock.createNiceMock(HttpServletRequest.class);
+    EasyMock.expect(req.getParameter("originalUrl")).andReturn(ORIGINAL_URL);
+    EasyMock.expect(req.getParameterMap()).andReturn(params);
+    EasyMock.expect(req.getParameterMap()).andReturn(Collections.emptyMap());
+    EasyMock.expect(req.getServletContext()).andReturn(context).anyTimes();
+
+    Principal principal = EasyMock.createNiceMock(Principal.class);
+    EasyMock.expect(principal.getName()).andReturn("alice").anyTimes();
+    EasyMock.expect(req.getUserPrincipal()).andReturn(principal).anyTimes();
+
+    EasyMock.replay(req);
+
+    WebSSOResource webSSOResource = new WebSSOResource();
+
+
+    webSSOResource.request = req;
+    String result = webSSOResource.getOriginalUrlFromQueryParams();
+
+    /* make sure there no additional '?' at the end of ORIG_URL_VALUE */
+    assertEquals(ORIGINAL_URL+"&port="+PORT_VALUE, result);
+  }
+
+  /**
+   * Test for cases where originalUrl has host parameter.
+   * @throws Exception
+   */
+  @Test
+  public void testGetOriginalUrlFromQueryParamsWithHost() throws Exception {
+    final String ORIG_URL_VALUE = "https://local.site/gateway/sandbox/hbase/webui/master?host=local.site";
+    final String ORIGINAL_URL= "originalUrl="+ORIG_URL_VALUE;
+    Map<String, String[]> params = new HashMap<>();
+    params.put("originalUrl", new String[] {ORIG_URL_VALUE});
+
+    HttpServletRequest req = EasyMock.createNiceMock(HttpServletRequest.class);
+    EasyMock.expect(req.getParameter("originalUrl")).andReturn(ORIGINAL_URL);
+    EasyMock.expect(req.getParameterMap()).andReturn(params);
+    EasyMock.expect(req.getParameterMap()).andReturn(Collections.emptyMap());
+    EasyMock.expect(req.getServletContext()).andReturn(context).anyTimes();
+
+    Principal principal = EasyMock.createNiceMock(Principal.class);
+    EasyMock.expect(principal.getName()).andReturn("alice").anyTimes();
+    EasyMock.expect(req.getUserPrincipal()).andReturn(principal).anyTimes();
+
+    EasyMock.replay(req);
+
+    WebSSOResource webSSOResource = new WebSSOResource();
+
+
+    webSSOResource.request = req;
+    String result = webSSOResource.getOriginalUrlFromQueryParams();
+
+    /* make sure there no additional '?' at the end of ORIG_URL_VALUE */
+    assertEquals(ORIGINAL_URL, result);
+  }
+
+
+  /**
+   * Test for cases where originalUrl does not have host and port parameters.
+   * normal case.
+   * @throws Exception
+   */
+  @Test
+  public void testGetOriginalUrlFromQueryParams() throws Exception {
+    /* &port=1001 is missing in the original URL because it gets stripped
+    out during request.getParameter(ORIGINAL_URL_REQUEST_PARAM) operation */
+    final String ORIG_URL_VALUE = "https://local.site/gateway/sandbox/hbase/webui/master";
+    final String ORIGINAL_URL= "originalUrl="+ORIG_URL_VALUE;
+    Map<String, String[]> params = new HashMap<>();
+    params.put("originalUrl", new String[] {ORIG_URL_VALUE});
+
+    HttpServletRequest req = EasyMock.createNiceMock(HttpServletRequest.class);
+    EasyMock.expect(req.getParameter("originalUrl")).andReturn(ORIGINAL_URL);
+    EasyMock.expect(req.getParameterMap()).andReturn(params);
+    EasyMock.expect(req.getParameterMap()).andReturn(Collections.emptyMap());
+    EasyMock.expect(req.getServletContext()).andReturn(context).anyTimes();
+
+    Principal principal = EasyMock.createNiceMock(Principal.class);
+    EasyMock.expect(principal.getName()).andReturn("alice").anyTimes();
+    EasyMock.expect(req.getUserPrincipal()).andReturn(principal).anyTimes();
+
+    EasyMock.replay(req);
+
+    WebSSOResource webSSOResource = new WebSSOResource();
+
+
+    webSSOResource.request = req;
+    String result = webSSOResource.getOriginalUrlFromQueryParams();
+
+    /* make sure there no additional '?' at the end of ORIG_URL_VALUE */
+    assertEquals(ORIGINAL_URL, result);
+  }
+
+
   /**
    * A wrapper for HttpServletResponseWrapper to store the cookies
    */