KNOX-2906 - SSO whitelist checker should cut off path segments and query params (#754)
diff --git a/gateway-service-knoxsso/src/main/java/org/apache/knox/gateway/service/knoxsso/WebSSOResource.java b/gateway-service-knoxsso/src/main/java/org/apache/knox/gateway/service/knoxsso/WebSSOResource.java
index afbca92..531524d 100644
--- a/gateway-service-knoxsso/src/main/java/org/apache/knox/gateway/service/knoxsso/WebSSOResource.java
+++ b/gateway-service-knoxsso/src/main/java/org/apache/knox/gateway/service/knoxsso/WebSSOResource.java
@@ -22,11 +22,8 @@
import static org.apache.knox.gateway.services.GatewayServices.GATEWAY_CLUSTER_ATTRIBUTE;
import java.io.IOException;
-import java.io.UnsupportedEncodingException;
import java.net.URI;
import java.net.URISyntaxException;
-import java.net.URLDecoder;
-import java.nio.charset.StandardCharsets;
import java.security.Principal;
import java.util.ArrayList;
import java.util.Arrays;
@@ -252,14 +249,7 @@
// If there is a whitelist defined, then the original URL must be validated against it.
// If there is no whitelist, then everything is valid.
if (whitelist != null) {
- String decodedOriginal = null;
- try {
- decodedOriginal = URLDecoder.decode(original, StandardCharsets.UTF_8.name());
- } catch (UnsupportedEncodingException e) {
- //
- }
-
- validRedirect = RegExUtils.checkWhitelist(whitelist, (decodedOriginal != null ? decodedOriginal : original));
+ validRedirect = RegExUtils.checkBaseUrlAgainstWhitelist(whitelist, original);
}
if (!validRedirect) {
diff --git a/gateway-service-knoxsso/src/test/java/org/apache/knox/gateway/service/knoxsso/WebSSOResourceTest.java b/gateway-service-knoxsso/src/test/java/org/apache/knox/gateway/service/knoxsso/WebSSOResourceTest.java
index 1d8940b..33be1c7 100644
--- a/gateway-service-knoxsso/src/test/java/org/apache/knox/gateway/service/knoxsso/WebSSOResourceTest.java
+++ b/gateway-service-knoxsso/src/test/java/org/apache/knox/gateway/service/knoxsso/WebSSOResourceTest.java
@@ -148,6 +148,29 @@
"/local/resource/"));
}
+ @Test
+ public void testWhitelistMatchingAgainstBaseURL() {
+ Assert.assertTrue("Failed to match whitelist",
+ RegExUtils.checkBaseUrlAgainstWhitelist("^https?:\\/\\/(.*KNOX_GW_DOMAIN)(?::[0-9]+)?(?:\\/.*)?$",
+ "https://KNOX_GW_DOMAIN"));
+ Assert.assertTrue("Failed to match whitelist",
+ RegExUtils.checkBaseUrlAgainstWhitelist("^https?:\\/\\/(.*KNOX_GW_DOMAIN)(?::[0-9]+)?(?:\\/.*)?$",
+ "https://KNOX_GW_DOMAIN?a=1&b=2"));
+ Assert.assertTrue("Failed to match whitelist",
+ RegExUtils.checkBaseUrlAgainstWhitelist("^https?:\\/\\/(.*KNOX_GW_DOMAIN)(?::[0-9]+)?(?:\\/.*)?$",
+ "https://KNOX_GW_DOMAIN?a=1&b=2"));
+ Assert.assertTrue("Failed to match whitelist",
+ RegExUtils.checkBaseUrlAgainstWhitelist("^https?:\\/\\/(.*KNOX_GW_DOMAIN)(?::[0-9]+)?(?:\\/.*)?$",
+ "https://KNOX_GW_DOMAIN/path1/path2/path/3?a=1&b=2"));
+ Assert.assertFalse("Inappropriately matched whitelist",
+ RegExUtils.checkBaseUrlAgainstWhitelist("^https?:\\/\\/(.*KNOX_GW_DOMAIN)(?::[0-9]+)?(?:\\/.*)?$",
+ "https://google.com?https://KNOX_GW_DOMAIN"));
+ Assert.assertFalse("Inappropriately matched whitelist",
+ RegExUtils.checkBaseUrlAgainstWhitelist("^https?:\\/\\/(.*KNOX_GW_DOMAIN)(?::[0-9]+)?(?:\\/.*)?$",
+ "https://google.com/https://KNOX_GW_DOMAIN"));
+ }
+
+
private void configureCommonExpectations(Map<String, String> contextExpectations) throws Exception {
configureCommonExpectations(contextExpectations, false, false, true);
}
diff --git a/gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/GatewayDispatchFilter.java b/gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/GatewayDispatchFilter.java
index f54c639..2b56a06 100644
--- a/gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/GatewayDispatchFilter.java
+++ b/gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/GatewayDispatchFilter.java
@@ -32,12 +32,7 @@
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import java.io.IOException;
-import java.io.UnsupportedEncodingException;
-import java.net.MalformedURLException;
import java.net.URISyntaxException;
-import java.net.URL;
-import java.net.URLDecoder;
-import java.nio.charset.StandardCharsets;
import java.util.Collections;
import java.util.HashMap;
import java.util.Locale;
@@ -145,26 +140,10 @@
}
if (whitelist != null) {
- String requestURI = request.getRequestURI();
-
- String decodedURL = requestURI;
- try {
- decodedURL = URLDecoder.decode(requestURI, StandardCharsets.UTF_8.name());
- } catch (UnsupportedEncodingException e) {
- //
- }
- String baseUrl;
- try {
- URL url = new URL(decodedURL);
- baseUrl = new URL(url.getProtocol(), url.getHost(), url.getPort(), "").toString();
- } catch (MalformedURLException e) {
- throw new RuntimeException(e);
- }
-
- isAllowed = RegExUtils.checkWhitelist(whitelist, baseUrl);
+ isAllowed = RegExUtils.checkBaseUrlAgainstWhitelist(whitelist, request.getRequestURI());
if (!isAllowed) {
- LOG.dispatchDisallowed(requestURI);
+ LOG.dispatchDisallowed(request.getRequestURI());
}
}
diff --git a/gateway-util-common/src/main/java/org/apache/knox/gateway/util/RegExUtils.java b/gateway-util-common/src/main/java/org/apache/knox/gateway/util/RegExUtils.java
index 9ebd1d7..7a644fd 100644
--- a/gateway-util-common/src/main/java/org/apache/knox/gateway/util/RegExUtils.java
+++ b/gateway-util-common/src/main/java/org/apache/knox/gateway/util/RegExUtils.java
@@ -17,6 +17,11 @@
*/
package org.apache.knox.gateway.util;
+import java.io.UnsupportedEncodingException;
+import java.net.MalformedURLException;
+import java.net.URL;
+import java.net.URLDecoder;
+import java.nio.charset.StandardCharsets;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
@@ -40,4 +45,21 @@
return false;
}
+ public static boolean checkBaseUrlAgainstWhitelist(String whitelist, String fullUrl) {
+ String decodedURL = fullUrl;
+ try {
+ decodedURL = URLDecoder.decode(fullUrl, StandardCharsets.UTF_8.name());
+ } catch (UnsupportedEncodingException e) {
+ //
+ }
+ String baseUrl;
+ try {
+ URL url = new URL(decodedURL);
+ baseUrl = new URL(url.getProtocol(), url.getHost(), url.getPort(), "").toString();
+ } catch (MalformedURLException e) {
+ throw new RuntimeException(e);
+ }
+ return checkWhitelist(whitelist, baseUrl);
+ }
+
}