This is an automated email from the ASF dual-hosted git repository.
lmccay pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/knox.git
The following commit(s) were added to refs/heads/master by this push:
new 11a9b6e KNOX-2467 - Enable Jetty's X-Forwarded Header Support (#382)
11a9b6e is described below
commit 11a9b6e47465ddc5d374d2fcd444b915cb1bd32d
Author: lmccay <[email protected]>
AuthorDate: Sun Nov 1 20:12:09 2020 -0500
KNOX-2467 - Enable Jetty's X-Forwarded Header Support (#382)
* KNOX-2467 - Enable Jetty's X-Forwarded Header Support
---
.../provider/federation/jwt/JWTMessages.java | 3 +
.../jwt/filter/SSOCookieFederationFilter.java | 49 +++++++------
.../provider/federation/SSOCookieProviderTest.java | 82 ++++++++++++++++++++--
.../org/apache/knox/gateway/GatewayServer.java | 4 ++
.../gateway/config/impl/GatewayConfigImpl.java | 6 ++
.../apache/knox/gateway/config/GatewayConfig.java | 7 ++
.../org/apache/knox/gateway/GatewayTestConfig.java | 4 ++
.../apache/knox/gateway/GatewayBasicFuncTest.java | 19 ++---
8 files changed, 135 insertions(+), 39 deletions(-)
diff --git
a/gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/JWTMessages.java
b/gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/JWTMessages.java
index 72696c0..e5ce601 100644
---
a/gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/JWTMessages.java
+++
b/gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/JWTMessages.java
@@ -58,4 +58,7 @@ public interface JWTMessages {
@Message( level = MessageLevel.INFO, text = "Path {0} is configured as
unauthenticated path, letting the request {1} through" )
void unauthenticatedPathBypass(String path, String uri);
+
+ @Message( level = MessageLevel.WARN, text = "Unable to vderive
authentication provider url: {0}" )
+ void failedToDeriveAuthenticationProviderUrl(@StackTrace( level =
MessageLevel.ERROR) Exception e);
}
diff --git
a/gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/SSOCookieFederationFilter.java
b/gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/SSOCookieFederationFilter.java
index db56f55..7cf2804 100644
---
a/gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/SSOCookieFederationFilter.java
+++
b/gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/SSOCookieFederationFilter.java
@@ -38,6 +38,8 @@ import javax.servlet.http.Cookie;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import java.io.IOException;
+import java.net.MalformedURLException;
+import java.net.URL;
import java.nio.charset.StandardCharsets;
import java.text.ParseException;
import java.util.HashSet;
@@ -220,14 +222,18 @@ public class SSOCookieFederationFilter extends
AbstractJWTFilter {
* @return url to use as login url for redirect
*/
protected String constructLoginURL(HttpServletRequest request) {
+ String providerURL = null;
String delimiter = "?";
if (authenticationProviderUrl == null) {
- authenticationProviderUrl =
deriveDefaultAuthenticationProviderUrl(request);
+ providerURL = deriveDefaultAuthenticationProviderUrl(request);
}
- if (authenticationProviderUrl.contains("?")) {
+ else {
+ providerURL = authenticationProviderUrl;
+ }
+ if (providerURL.contains("?")) {
delimiter = "&";
}
- return authenticationProviderUrl + delimiter
+ return providerURL + delimiter
+ ORIGINAL_URL_QUERY_PARAM
+ request.getRequestURL().append(getOriginalQueryString(request));
}
@@ -239,31 +245,28 @@ public class SSOCookieFederationFilter extends
AbstractJWTFilter {
* @return url that is based on KnoxSSO endpoint
*/
public String deriveDefaultAuthenticationProviderUrl(HttpServletRequest
request) {
+ String providerURL = null;
String scheme;
String host;
int port;
- if (!beingProxied(request)) {
- scheme = request.getScheme();
- host = request.getServerName();
- port = request.getServerPort();
- }
- else {
- scheme = request.getHeader(X_FORWARDED_PROTO);
- host = request.getHeader(X_FORWARDED_HOST);
- port = Integer.parseInt(request.getHeader(X_FORWARDED_PORT));
- }
- StringBuilder sb = new StringBuilder(scheme);
- sb.append("://").append(host);
- if (!host.contains(":")) {
- sb.append(':').append(port);
- }
- sb.append('/').append(gatewayPath).append("/knoxsso/api/v1/websso");
+ try {
+ URL url = new URL(request.getRequestURL().toString());
+ scheme = url.getProtocol();
+ host = url.getHost();
+ port = url.getPort();
- return sb.toString();
- }
+ StringBuilder sb = new StringBuilder(scheme);
+ sb.append("://").append(host);
+ if (!host.contains(":") && port != -1) {
+ sb.append(':').append(port);
+ }
+ sb.append('/').append(gatewayPath).append("/knoxsso/api/v1/websso");
+ providerURL = sb.toString();
+ } catch (MalformedURLException e) {
+ LOGGER.failedToDeriveAuthenticationProviderUrl(e);
+ }
- private boolean beingProxied(HttpServletRequest request) {
- return (request.getHeader(X_FORWARDED_HOST) != null);
+ return providerURL;
}
private String getOriginalQueryString(HttpServletRequest request) {
diff --git
a/gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/SSOCookieProviderTest.java
b/gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/SSOCookieProviderTest.java
index 720ed06..b2a49db 100644
---
a/gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/SSOCookieProviderTest.java
+++
b/gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/SSOCookieProviderTest.java
@@ -173,21 +173,26 @@ public class SSOCookieProviderTest extends
AbstractJWTFilterTest {
String providerURL = ((TestSSOCookieFederationProvider)
handler).deriveDefaultAuthenticationProviderUrl(request);
Assert.assertNotNull("LoginURL should not be null.", providerURL);
- Assert.assertEquals(providerURL,
"https://localhost:8443/gateway/knoxsso/api/v1/websso";);
+ Assert.assertEquals(providerURL,
"https://localhost:8888/gateway/knoxsso/api/v1/websso";);
String loginURL = ((TestSSOCookieFederationProvider)
handler).constructLoginURL(request);
Assert.assertNotNull("LoginURL should not be null.", loginURL);
- Assert.assertEquals(loginURL,
"https://localhost:8443/gateway/knoxsso/api/v1/websso?originalUrl="; +
SERVICE_URL);
+ Assert.assertEquals(loginURL,
"https://localhost:8888/gateway/knoxsso/api/v1/websso?originalUrl="; +
SERVICE_URL);
}
@Test
public void testProxiedDefaultAuthenticationProviderURL() throws Exception {
+ // after KNOX-2467 enables jetty's xforwarded support this test has been
+ // changed to expect the X-Forwarded Headers to be resolved by
+ // httpRequest.getRequestURL instead of explicitly checking the headers
+ // ourselves. Leaving the headers set to show this is a proxied request
+ // but they really have no bearing on the results.
Properties props = new Properties();
props.setProperty("gateway.path", "gateway");
handler.init(new TestFilterConfig(props));
HttpServletRequest request =
EasyMock.createNiceMock(HttpServletRequest.class);
- EasyMock.expect(request.getRequestURL()).andReturn(new
StringBuffer(SERVICE_URL)).anyTimes();
+ EasyMock.expect(request.getRequestURL()).andReturn(new
StringBuffer("https://remotehost:8443/resource";)).anyTimes();
EasyMock.expect(request.getHeader(SSOCookieFederationFilter.X_FORWARDED_PROTO)).andReturn("https").anyTimes();
EasyMock.expect(request.getHeader(SSOCookieFederationFilter.X_FORWARDED_HOST)).andReturn("remotehost:8443").anyTimes();
EasyMock.expect(request.getHeader(SSOCookieFederationFilter.X_FORWARDED_PORT)).andReturn("8443").anyTimes();
@@ -199,17 +204,22 @@ public class SSOCookieProviderTest extends
AbstractJWTFilterTest {
String loginURL = ((TestSSOCookieFederationProvider)
handler).constructLoginURL(request);
Assert.assertNotNull("LoginURL should not be null.", loginURL);
- Assert.assertEquals(loginURL,
"https://remotehost:8443/gateway/knoxsso/api/v1/websso?originalUrl="; +
SERVICE_URL);
+ Assert.assertEquals(loginURL,
"https://remotehost:8443/gateway/knoxsso/api/v1/websso?originalUrl="; +
"https://remotehost:8443/resource";);
}
@Test
- public void
testProxiedDefaultAuthenticationProviderURLWithoutPortInHostHeader() throws
Exception {
+ public void
testProxiedDefaultAuthenticationProviderURLWithoutNonGatewayAppPath() throws
Exception {
+ // after KNOX-2467 enables jetty's xforwarded support this test has been
+ // changed to expect the X-Forwarded Headers to be resolved by
+ // httpRequest.getRequestURL instead of explicitly checking the headers
+ // ourselves. Leaving the headers set to show this is a proxied request
+ // but they really have no bearing on the results.
Properties props = new Properties();
props.setProperty("gateway.path", "notgateway");
handler.init(new TestFilterConfig(props));
HttpServletRequest request =
EasyMock.createNiceMock(HttpServletRequest.class);
- EasyMock.expect(request.getRequestURL()).andReturn(new
StringBuffer(SERVICE_URL)).anyTimes();
+ EasyMock.expect(request.getRequestURL()).andReturn(new
StringBuffer("https://remotehost:8443/resource";)).anyTimes();
EasyMock.expect(request.getHeader(SSOCookieFederationFilter.X_FORWARDED_PROTO)).andReturn("https").anyTimes();
EasyMock.expect(request.getHeader(SSOCookieFederationFilter.X_FORWARDED_HOST)).andReturn("remotehost").anyTimes();
EasyMock.expect(request.getHeader(SSOCookieFederationFilter.X_FORWARDED_PORT)).andReturn("8443").anyTimes();
@@ -221,7 +231,65 @@ public class SSOCookieProviderTest extends
AbstractJWTFilterTest {
String loginURL = ((TestSSOCookieFederationProvider)
handler).constructLoginURL(request);
Assert.assertNotNull("LoginURL should not be null.", loginURL);
- Assert.assertEquals(loginURL,
"https://remotehost:8443/notgateway/knoxsso/api/v1/websso?originalUrl="; +
SERVICE_URL);
+ Assert.assertEquals(loginURL,
"https://remotehost:8443/notgateway/knoxsso/api/v1/websso?originalUrl="; +
"https://remotehost:8443/resource";);
+ }
+
+ @Test
+ public void
testProxiedDefaultAuthenticationProviderURLWithoutPortInHostHeader() throws
Exception {
+ // after KNOX-2467 enables jetty's xforwarded support this test has been
+ // changed to expect the X-Forwarded Headers to be resolved by
+ // httpRequest.getRequestURL instead of explicitly checking the headers
+ // ourselves. Leaving the headers set to show this is a proxied request
+ // but they really have no bearing on the results.
+ Properties props = new Properties();
+ props.setProperty("gateway.path", "notgateway");
+ handler.init(new TestFilterConfig(props));
+
+ HttpServletRequest request =
EasyMock.createNiceMock(HttpServletRequest.class);
+ EasyMock.expect(request.getRequestURL()).andReturn(new
StringBuffer("https://remotehost/resource";)).anyTimes();
+
EasyMock.expect(request.getHeader(SSOCookieFederationFilter.X_FORWARDED_PROTO)).andReturn("https").anyTimes();
+
EasyMock.expect(request.getHeader(SSOCookieFederationFilter.X_FORWARDED_HOST)).andReturn("remotehost").anyTimes();
+ EasyMock.replay(request);
+
+ String providerURL = ((TestSSOCookieFederationProvider)
handler).deriveDefaultAuthenticationProviderUrl(request);
+ Assert.assertNotNull("LoginURL should not be null.", providerURL);
+ Assert.assertEquals(providerURL,
"https://remotehost/notgateway/knoxsso/api/v1/websso";);
+
+ String loginURL = ((TestSSOCookieFederationProvider)
handler).constructLoginURL(request);
+ Assert.assertNotNull("LoginURL should not be null.", loginURL);
+ Assert.assertEquals(loginURL,
"https://remotehost/notgateway/knoxsso/api/v1/websso?originalUrl="; +
"https://remotehost/resource";);
+ }
+
+ @Test
+ public void
testProxiedDefaultAuthenticationProviderURLWithoutMismatchInXForwardedHeader()
throws Exception {
+ // after KNOX-2467 enables jetty's xforwarded support this test has been
+ // changed to expect the X-Forwarded Headers to be resolved by
+ // httpRequest.getRequestURL instead of explicitly checking the headers
+ // ourselves. Leaving the headers set to show this is a proxied request
+ // but they really have no bearing on the results.
+
+ // this is an odd test but we want to make sure that the removed code
+ // that explicitly handled incoming xforwarded headers in the redirect
+ // url is actuall removed and all handling of xforwarded is handled by
+ // servlet container.
+ Properties props = new Properties();
+ props.setProperty("gateway.path", "notgateway");
+ handler.init(new TestFilterConfig(props));
+
+ HttpServletRequest request =
EasyMock.createNiceMock(HttpServletRequest.class);
+ EasyMock.expect(request.getRequestURL()).andReturn(new
StringBuffer("https://remotehost/resource";)).anyTimes();
+
EasyMock.expect(request.getHeader(SSOCookieFederationFilter.X_FORWARDED_PROTO)).andReturn("http").anyTimes();
+
EasyMock.expect(request.getHeader(SSOCookieFederationFilter.X_FORWARDED_HOST)).andReturn("larryhost").anyTimes();
+
EasyMock.expect(request.getHeader(SSOCookieFederationFilter.X_FORWARDED_PORT)).andReturn("7777").anyTimes();
+ EasyMock.replay(request);
+
+ String providerURL = ((TestSSOCookieFederationProvider)
handler).deriveDefaultAuthenticationProviderUrl(request);
+ Assert.assertNotNull("LoginURL should not be null.", providerURL);
+ Assert.assertEquals(providerURL,
"https://remotehost/notgateway/knoxsso/api/v1/websso";);
+
+ String loginURL = ((TestSSOCookieFederationProvider)
handler).constructLoginURL(request);
+ Assert.assertNotNull("LoginURL should not be null.", loginURL);
+ Assert.assertEquals(loginURL,
"https://remotehost/notgateway/knoxsso/api/v1/websso?originalUrl="; +
"https://remotehost/resource";);
}
@Override
diff --git
a/gateway-server/src/main/java/org/apache/knox/gateway/GatewayServer.java
b/gateway-server/src/main/java/org/apache/knox/gateway/GatewayServer.java
index f92e2d7..dab2710 100644
--- a/gateway-server/src/main/java/org/apache/knox/gateway/GatewayServer.java
+++ b/gateway-server/src/main/java/org/apache/knox/gateway/GatewayServer.java
@@ -386,6 +386,10 @@ public class GatewayServer {
for (ConnectionFactory x :
networkConnector.getConnectionFactories()) {
if (x instanceof HttpConnectionFactory) {
((HttpConnectionFactory)
x).getHttpConfiguration().setSendServerVersion(config.isGatewayServerHeaderEnabled());
+ if (config.isGatewayServerIncomingXForwardedSupportEnabled()) {
+ // Add support for X-Forwarded headers
+ ((HttpConnectionFactory)
x).getHttpConfiguration().addCustomizer(new
org.eclipse.jetty.server.ForwardedRequestCustomizer());
+ }
}
}
if (networkConnector.getName() == null) {
diff --git
a/gateway-server/src/main/java/org/apache/knox/gateway/config/impl/GatewayConfigImpl.java
b/gateway-server/src/main/java/org/apache/knox/gateway/config/impl/GatewayConfigImpl.java
index f2c33f3..4efcd13 100644
---
a/gateway-server/src/main/java/org/apache/knox/gateway/config/impl/GatewayConfigImpl.java
+++
b/gateway-server/src/main/java/org/apache/knox/gateway/config/impl/GatewayConfigImpl.java
@@ -266,6 +266,7 @@ public class GatewayConfigImpl extends Configuration
implements GatewayConfig {
private static final String KNOX_HOMEPAGE_HIDDEN_TOPOLOGIES =
"knox.homepage.hidden.topologies";
private static final Set<String> KNOX_HOMEPAGE_HIDDEN_TOPOLOGIES_DEFAULT =
new HashSet<>(Arrays.asList("admin", "manager", "knoxsso", "metadata",
"homepage"));
private static final String KNOX_HOMEPAGE_LOGOUT_ENABLED =
"knox.homepage.logout.enabled";
+ private static final String KNOX_INCOMING_XFORWARDED_ENABLED =
"gateway.incoming.xforwarded.enabled";
public GatewayConfigImpl() {
init();
@@ -1193,4 +1194,9 @@ public class GatewayConfigImpl extends Configuration
implements GatewayConfig {
public long getKeystoreCacheEntryTimeToLiveInMinutes() {
return getLong(KEYSTORE_CACHE_ENTRY_TTL, DEFAULT_KEYSTORE_CACHE_ENTRY_TTL);
}
+
+ @Override
+ public boolean isGatewayServerIncomingXForwardedSupportEnabled() {
+ return getBoolean(KNOX_INCOMING_XFORWARDED_ENABLED, true);
+ }
}
diff --git
a/gateway-spi/src/main/java/org/apache/knox/gateway/config/GatewayConfig.java
b/gateway-spi/src/main/java/org/apache/knox/gateway/config/GatewayConfig.java
index 124d968..41b2071 100644
---
a/gateway-spi/src/main/java/org/apache/knox/gateway/config/GatewayConfig.java
+++
b/gateway-spi/src/main/java/org/apache/knox/gateway/config/GatewayConfig.java
@@ -717,4 +717,11 @@ public interface GatewayConfig {
* @return the time - in minutes - an entry should be live (i.e. must not
expire) in keystore cache
*/
long getKeystoreCacheEntryTimeToLiveInMinutes();
+
+ /**
+ * Indicates whether the embedded Jetty Server support for X-Forwarded
Headers should
+ * be enabled.
+ * @return true if incoming X-Forwarded headers are enabled
+ */
+ boolean isGatewayServerIncomingXForwardedSupportEnabled();
}
diff --git
a/gateway-test-release-utils/src/main/java/org/apache/knox/gateway/GatewayTestConfig.java
b/gateway-test-release-utils/src/main/java/org/apache/knox/gateway/GatewayTestConfig.java
index 6b5b466..2b50450 100644
---
a/gateway-test-release-utils/src/main/java/org/apache/knox/gateway/GatewayTestConfig.java
+++
b/gateway-test-release-utils/src/main/java/org/apache/knox/gateway/GatewayTestConfig.java
@@ -833,4 +833,8 @@ public class GatewayTestConfig extends Configuration
implements GatewayConfig {
return 0;
}
+ @Override
+ public boolean isGatewayServerIncomingXForwardedSupportEnabled() {
+ return true;
+ }
}
diff --git
a/gateway-test/src/test/java/org/apache/knox/gateway/GatewayBasicFuncTest.java
b/gateway-test/src/test/java/org/apache/knox/gateway/GatewayBasicFuncTest.java
index a77524c..d395427 100644
---
a/gateway-test/src/test/java/org/apache/knox/gateway/GatewayBasicFuncTest.java
+++
b/gateway-test/src/test/java/org/apache/knox/gateway/GatewayBasicFuncTest.java
@@ -3463,10 +3463,6 @@ public class GatewayBasicFuncTest {
String port = "8889";
String scheme = "https";
- InetSocketAddress gatewayAddress = driver.gateway.getAddresses()[0];
- String gatewayHostName = gatewayAddress.getHostName();
- String gatewayAddrName = InetAddress.getByName( gatewayHostName
).getHostAddress();
-
//Test rewriting of body with X-Forwarded headers (using storm)
String resourceName = "storm/topology-id.json";
String path = "/api/v1/topology/WordCount-1-1424792039";
@@ -3478,7 +3474,12 @@ public class GatewayBasicFuncTest {
.header("X-Forwarded-Proto", scheme)
.header("X-Forwarded-Port", port)
.header("X-Forwarded-Context", "/gateway/cluster")
- .header("X-Forwarded-Server", Matchers.is(oneOf( gatewayHostName,
gatewayAddrName ) ))
+ // Since KNOX-2467 enables Jetty's X-Forwarded handling
+ // the following is no longer true and while possibly accurate
+ // in terms of the most recent proxy hostname, it should
+ // represent the host of the X-Forwarded-Host and does not here
+ //.header("X-Forwarded-Server", Matchers.is(oneOf( gatewayHostName,
gatewayAddrName ) ))
+ .header("X-Forwarded-Server", host)
.header("X-Forwarded-For", Matchers.containsString("what, boo"))
.pathInfo(path)
.queryParam("user.name", username)
@@ -3518,7 +3519,7 @@ public class GatewayBasicFuncTest {
.header("X-Forwarded-Proto", scheme)
.header("X-Forwarded-Port", port)
.header("X-Forwarded-Context", "/gateway/cluster")
- .header("X-Forwarded-Server", Matchers.is(oneOf( gatewayHostName,
gatewayAddrName ) ))
+ .header("X-Forwarded-Server", host)
.header("X-Forwarded-For", Matchers.containsString("what, boo"))
.pathInfo(path)
.queryParam("user.name", username)
@@ -3534,7 +3535,7 @@ public class GatewayBasicFuncTest {
.header("X-Forwarded-Host", host)
.header("X-Forwarded-Proto", scheme)
.header("X-Forwarded-Port", port)
- .header("X-Forwarded-Server", "what")
+ .header("X-Forwarded-Server", host)
.header("X-Forwarded-For", "what, boo")
.then()
// .log().all()
@@ -3561,7 +3562,7 @@ public class GatewayBasicFuncTest {
.header("X-Forwarded-Proto", scheme)
.header("X-Forwarded-Port", port)
.header("X-Forwarded-Context", "/gateway/cluster")
- .header("X-Forwarded-Server", Matchers.is(oneOf( gatewayHostName,
gatewayAddrName ) ))
+ .header("X-Forwarded-Server", host)
.header("X-Forwarded-For", Matchers.containsString("what, boo"))
.queryParam("op", "CREATE")
.queryParam( "user.name", username )
@@ -3575,7 +3576,7 @@ public class GatewayBasicFuncTest {
.header("X-Forwarded-Host", host)
.header("X-Forwarded-Proto", scheme)
.header("X-Forwarded-Port", port)
- .header("X-Forwarded-Server", "what")
+ .header("X-Forwarded-Server", host)
.header("X-Forwarded-For", "what, boo")
.queryParam( "op", "CREATE" )
.then()
