Minor code improvements
Signed-off-by: Dominic Schabel dominic.schabel@bosch-si.com
This commit is contained in:
@@ -100,7 +100,8 @@ public class DosFilter extends OncePerRequestFilter {
|
||||
|
||||
boolean processChain;
|
||||
|
||||
final String ip = IpUtil.getClientIpFromRequest(request, forwardHeader, true).getHost();
|
||||
final String ip = IpUtil.getClientIpFromRequest(request, forwardHeader).getHost();
|
||||
|
||||
if (checkIpFails(ip)) {
|
||||
processChain = handleMissingIpAddress(response);
|
||||
} else {
|
||||
@@ -121,7 +122,6 @@ public class DosFilter extends OncePerRequestFilter {
|
||||
if (processChain) {
|
||||
filterChain.doFilter(request, response);
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -44,7 +44,7 @@ public final class IpUtil {
|
||||
* {@link HttpServletRequest} by either the
|
||||
* {@link HttpHeaders#X_FORWARDED_FOR} or by the
|
||||
* {@link HttpServletRequest#getRemoteAddr()} methods.
|
||||
*
|
||||
*
|
||||
* @param request
|
||||
* the {@link HttpServletRequest} to determine the IP address
|
||||
* where this request has been sent from
|
||||
@@ -65,21 +65,23 @@ public final class IpUtil {
|
||||
* {@link HttpServletRequest} by either the
|
||||
* {@link HttpHeaders#X_FORWARDED_FOR} or by the
|
||||
* {@link HttpServletRequest#getRemoteAddr()} methods.
|
||||
*
|
||||
*
|
||||
* @param request
|
||||
* the {@link HttpServletRequest} to determine the IP address
|
||||
* where this request has been sent from
|
||||
* @param forwardHeader
|
||||
* the header name containing the IP address e.g. forwarded by a
|
||||
* proxy {@code x-forwarded-for}
|
||||
*
|
||||
* @param trackRemoteIp
|
||||
* to <code>true</code> if remote IP should be tracked.
|
||||
* @return the {@link URI} based IP address from the client which sent the
|
||||
* request
|
||||
*/
|
||||
public static URI getClientIpFromRequest(final HttpServletRequest request, final String forwardHeader,
|
||||
public static URI getClientIpFromRequest(final HttpServletRequest request, final String forwardHeader) {
|
||||
return getClientIpFromRequest(request, forwardHeader, true);
|
||||
}
|
||||
|
||||
private static URI getClientIpFromRequest(final HttpServletRequest request, final String forwardHeader,
|
||||
final boolean trackRemoteIp) {
|
||||
|
||||
String ip;
|
||||
|
||||
if (trackRemoteIp) {
|
||||
@@ -113,7 +115,7 @@ public final class IpUtil {
|
||||
|
||||
/**
|
||||
* Create a {@link URI} with scheme and host.
|
||||
*
|
||||
*
|
||||
* @param scheme
|
||||
* the scheme
|
||||
* @param host
|
||||
@@ -132,7 +134,7 @@ public final class IpUtil {
|
||||
|
||||
/**
|
||||
* Create a {@link URI} with amqp scheme and host.
|
||||
*
|
||||
*
|
||||
* @param host
|
||||
* the host
|
||||
* @param exchange
|
||||
@@ -147,7 +149,7 @@ public final class IpUtil {
|
||||
|
||||
/**
|
||||
* Create a {@link URI} with http scheme and host.
|
||||
*
|
||||
*
|
||||
* @param host
|
||||
* the host
|
||||
* @return the {@link URI}
|
||||
@@ -160,7 +162,7 @@ public final class IpUtil {
|
||||
|
||||
/**
|
||||
* Check if scheme contains http and uri ist not <code>null</code>.
|
||||
*
|
||||
*
|
||||
* @param uri
|
||||
* the uri
|
||||
* @return true = is http host false = not
|
||||
@@ -171,7 +173,7 @@ public final class IpUtil {
|
||||
|
||||
/**
|
||||
* Check if host scheme amqp and uri ist not <code>null</code>.
|
||||
*
|
||||
*
|
||||
* @param uri
|
||||
* the uri
|
||||
* @return true = is http host false = not
|
||||
@@ -183,7 +185,7 @@ public final class IpUtil {
|
||||
/**
|
||||
* Check if the IP address of that {@link URI} is known, i.e. not an AQMP
|
||||
* exchange in DMF case and not HIDDEN_IP in DDI case.
|
||||
*
|
||||
*
|
||||
* @param uri
|
||||
* the uri
|
||||
* @return <code>true</code> if IP address is actually known by the server
|
||||
|
||||
@@ -8,6 +8,7 @@
|
||||
*/
|
||||
package org.eclipse.hawkbit.util;
|
||||
|
||||
import static com.google.common.net.HttpHeaders.X_FORWARDED_FOR;
|
||||
import static org.fest.assertions.api.Assertions.assertThat;
|
||||
import static org.junit.Assert.assertEquals;
|
||||
import static org.junit.Assert.assertFalse;
|
||||
@@ -21,13 +22,13 @@ import java.net.URI;
|
||||
|
||||
import javax.servlet.http.HttpServletRequest;
|
||||
|
||||
import org.eclipse.hawkbit.security.HawkbitSecurityProperties;
|
||||
import org.eclipse.hawkbit.security.HawkbitSecurityProperties.Clients;
|
||||
import org.junit.Test;
|
||||
import org.junit.runner.RunWith;
|
||||
import org.mockito.Mock;
|
||||
import org.mockito.runners.MockitoJUnitRunner;
|
||||
|
||||
import com.google.common.net.HttpHeaders;
|
||||
|
||||
import ru.yandex.qatools.allure.annotations.Description;
|
||||
import ru.yandex.qatools.allure.annotations.Features;
|
||||
import ru.yandex.qatools.allure.annotations.Stories;
|
||||
@@ -37,69 +38,73 @@ import ru.yandex.qatools.allure.annotations.Stories;
|
||||
@Stories("IP Util Test")
|
||||
public class IpUtilTest {
|
||||
|
||||
private static final String KNOWN_REQUEST_HEADER = "bumlux";
|
||||
|
||||
@Mock
|
||||
private HttpServletRequest requestMock;
|
||||
|
||||
@Mock
|
||||
private Clients clientMock;
|
||||
|
||||
@Mock
|
||||
private HawkbitSecurityProperties securityPropertyMock;
|
||||
|
||||
@Test
|
||||
@Description("Tests create uri from request")
|
||||
public void getRemoteAddrFromRequestIfForwaredHeaderNotPresent() {
|
||||
// known values
|
||||
|
||||
final URI knownRemoteClientIP = IpUtil.createHttpUri("127.0.0.1");
|
||||
// mock
|
||||
when(requestMock.getHeader(HttpHeaders.X_FORWARDED_FOR)).thenReturn(null);
|
||||
when(requestMock.getHeader(X_FORWARDED_FOR)).thenReturn(null);
|
||||
when(requestMock.getRemoteAddr()).thenReturn(knownRemoteClientIP.getHost());
|
||||
|
||||
// test
|
||||
final URI remoteAddr = IpUtil.getClientIpFromRequest(requestMock, "bumlux", true);
|
||||
final URI remoteAddr = IpUtil.getClientIpFromRequest(requestMock, KNOWN_REQUEST_HEADER);
|
||||
|
||||
// verify
|
||||
assertThat(remoteAddr).as("The remote address should be as the known client IP address")
|
||||
.isEqualTo(knownRemoteClientIP);
|
||||
verify(requestMock, times(1)).getHeader("bumlux");
|
||||
verify(requestMock, times(1)).getHeader(KNOWN_REQUEST_HEADER);
|
||||
verify(requestMock, times(1)).getRemoteAddr();
|
||||
}
|
||||
|
||||
@Test
|
||||
@Description("Tests create uri from request with masked IP when IP tracking is disabled")
|
||||
public void maskRemoteAddrIfDisabled() {
|
||||
// known values
|
||||
|
||||
final URI knownRemoteClientIP = IpUtil.createHttpUri("***");
|
||||
// mock
|
||||
when(requestMock.getHeader(HttpHeaders.X_FORWARDED_FOR)).thenReturn(null);
|
||||
when(requestMock.getHeader(X_FORWARDED_FOR)).thenReturn(null);
|
||||
when(requestMock.getRemoteAddr()).thenReturn(knownRemoteClientIP.getHost());
|
||||
when(securityPropertyMock.getClients()).thenReturn(clientMock);
|
||||
when(clientMock.getRemoteIpHeader()).thenReturn(KNOWN_REQUEST_HEADER);
|
||||
when(clientMock.isTrackRemoteIp()).thenReturn(false);
|
||||
|
||||
// test
|
||||
final URI remoteAddr = IpUtil.getClientIpFromRequest(requestMock, "bumlux", false);
|
||||
final URI remoteAddr = IpUtil.getClientIpFromRequest(requestMock, securityPropertyMock);
|
||||
|
||||
// verify
|
||||
assertThat(remoteAddr).as("The remote address should be as the known client IP address")
|
||||
.isEqualTo(knownRemoteClientIP);
|
||||
verify(requestMock, times(0)).getHeader("bumlux");
|
||||
verify(requestMock, times(0)).getHeader(KNOWN_REQUEST_HEADER);
|
||||
verify(requestMock, times(0)).getRemoteAddr();
|
||||
}
|
||||
|
||||
@Test
|
||||
@Description("Tests create uri from x forward header")
|
||||
public void getRemoteAddrFromXForwardedForHeader() {
|
||||
// known values
|
||||
|
||||
final URI knownRemoteClientIP = IpUtil.createHttpUri("10.99.99.1");
|
||||
// mock
|
||||
when(requestMock.getHeader(HttpHeaders.X_FORWARDED_FOR)).thenReturn(knownRemoteClientIP.getHost());
|
||||
when(requestMock.getHeader(X_FORWARDED_FOR)).thenReturn(knownRemoteClientIP.getHost());
|
||||
when(requestMock.getRemoteAddr()).thenReturn(null);
|
||||
|
||||
// test
|
||||
final URI remoteAddr = IpUtil.getClientIpFromRequest(requestMock, "X-Forwarded-For", true);
|
||||
final URI remoteAddr = IpUtil.getClientIpFromRequest(requestMock, "X-Forwarded-For");
|
||||
|
||||
// verify
|
||||
assertThat(remoteAddr).as("The remote address should be as the known client IP address")
|
||||
.isEqualTo(knownRemoteClientIP);
|
||||
verify(requestMock, times(1)).getHeader(HttpHeaders.X_FORWARDED_FOR);
|
||||
verify(requestMock, times(1)).getHeader(X_FORWARDED_FOR);
|
||||
verify(requestMock, times(0)).getRemoteAddr();
|
||||
}
|
||||
|
||||
@Test
|
||||
@Description("Tests create http uri ipv4 and ipv6")
|
||||
public void testCreateHttpUri() {
|
||||
|
||||
final String ipv4 = "10.99.99.1";
|
||||
URI httpUri = IpUtil.createHttpUri(ipv4);
|
||||
assertHttpUri(ipv4, httpUri);
|
||||
@@ -111,7 +116,6 @@ public class IpUtilTest {
|
||||
final String ipv6 = "0:0:0:0:0:0:0:1";
|
||||
httpUri = IpUtil.createHttpUri(ipv6);
|
||||
assertHttpUri("[" + ipv6 + "]", httpUri);
|
||||
|
||||
}
|
||||
|
||||
private void assertHttpUri(final String host, final URI httpUri) {
|
||||
@@ -124,6 +128,7 @@ public class IpUtilTest {
|
||||
@Test
|
||||
@Description("Tests create amqp uri ipv4 and ipv6")
|
||||
public void testCreateAmqpUri() {
|
||||
|
||||
final String ipv4 = "10.99.99.1";
|
||||
URI amqpUri = IpUtil.createAmqpUri(ipv4, "path");
|
||||
assertAmqpUri(ipv4, amqpUri);
|
||||
@@ -138,6 +143,7 @@ public class IpUtilTest {
|
||||
}
|
||||
|
||||
private void assertAmqpUri(final String host, final URI amqpUri) {
|
||||
|
||||
assertTrue("The given URI is an AMQP scheme", IpUtil.isAmqpUri(amqpUri));
|
||||
assertFalse("The given URI is not an HTTP scheme", IpUtil.isHttpUri(amqpUri));
|
||||
assertEquals("The given host matches the URI host", host, amqpUri.getHost());
|
||||
@@ -148,17 +154,19 @@ public class IpUtilTest {
|
||||
@Test
|
||||
@Description("Tests create invalid uri")
|
||||
public void testCreateInvalidUri() {
|
||||
|
||||
final String host = "10.99.99.1";
|
||||
final URI testUri = IpUtil.createUri("test", host);
|
||||
|
||||
assertFalse("The given URI is not an AMQP address", IpUtil.isAmqpUri(testUri));
|
||||
assertFalse("The given URI is not an HTTP address", IpUtil.isHttpUri(testUri));
|
||||
assertEquals("The given host matches the URI host", host, testUri.getHost());
|
||||
|
||||
try {
|
||||
IpUtil.createUri(":/", host);
|
||||
fail("Missing expected IllegalArgumentException due invalid URI");
|
||||
} catch (final IllegalArgumentException e) {
|
||||
// expected
|
||||
}
|
||||
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user