More test #145

jenkinsci · Nov 17, 2019 · 935fab2 · 935fab2
1 parent bbf0f67
commit 935fab2
Show file tree

Hide file tree

Showing 4 changed files with 130 additions and 9 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,14 @@
 # Generic Webhook Plugin Changelog
 Changelog of Generic Webhook Plugin.
+## Unreleased
+### GitHub [#145](https://github.com/jenkinsci/generic-webhook-trigger-plugin/pull/145) Fix whitelist host validation  
+
+**More test #145**
+
+
+[fde8a59ff1ec255](https://github.com/jenkinsci/generic-webhook-trigger-plugin/commit/fde8a59ff1ec255) Tomas Bjerre *2019-11-17 13:13:04*
+
+
 ## 1.64 (2019-11-15 18:55:35)
 ### GitHub [#145](https://github.com/jenkinsci/generic-webhook-trigger-plugin/pull/145) Fix whitelist host validation  
 

diff --git a/src/main/java/org/jenkinsci/plugins/gwt/whitelist/HostVerifier.java b/src/main/java/org/jenkinsci/plugins/gwt/whitelist/HostVerifier.java
@@ -16,8 +16,6 @@ static boolean whitelistVerified(
         return true;
       case CIDR:
         return verifyCidr(remoteHost, whitelistHost);
-      case RANGE:
-        return verifyCidr(remoteHost, whitelistHost);
       case STATIC:
         return verifyStatic(remoteHost, whitelistHost);
       default:

diff --git a/src/main/java/org/jenkinsci/plugins/gwt/whitelist/WhitelistHost.java b/src/main/java/org/jenkinsci/plugins/gwt/whitelist/WhitelistHost.java
@@ -10,7 +10,6 @@ public class WhitelistHost {
 
   public static enum HOST_TYPE {
     CIDR,
-    RANGE,
     STATIC,
     ANY;
   }
@@ -34,32 +33,39 @@ public WhitelistHost(final String string) throws WhitelistException {
 
     String[] hostParts = whitelistHost.split("/");
 
+    boolean isCidrRange = false;
     if (hostParts.length == 2) {
       hostType = HOST_TYPE.CIDR;
     } else {
       hostParts = whitelistHost.split("-");
       if (hostParts.length == 2) {
-        hostType = HOST_TYPE.RANGE;
+        isCidrRange = true;
+        hostType = HOST_TYPE.CIDR;
       }
     }
 
-    if (hostType == HOST_TYPE.CIDR || hostType == HOST_TYPE.RANGE) {
-      final int leftValueLength = InetAddresses.forString(hostParts[0]).getAddress().length;
+    if (hostType == HOST_TYPE.CIDR) {
+      int leftValueLength;
+      try {
+        leftValueLength = InetAddresses.forString(hostParts[0]).getAddress().length;
+      } catch (final IllegalArgumentException e) {
+        throw new WhitelistException(whitelistHost + " is not an Ipv4 string literal.");
+      }
       if (leftValueLength == 4) {
         try {
           this.rangeIpv4 = Ipv4Range.parse(whitelistHost);
         } catch (final IllegalArgumentException e) {
-          throw new WhitelistException(whitelistHost + " is not an Ipv4 string literal.");
+          throw new WhitelistException(whitelistHost + " cannot be parsed as Ipv4 string literal.");
         }
       } else if (leftValueLength == 16) {
         try {
           this.rangeIpv6 = Ipv6Range.parse(whitelistHost);
         } catch (final IllegalArgumentException e) {
-          throw new WhitelistException(whitelistHost + " is not an Ipv6 string literal.");
+          throw new WhitelistException(whitelistHost + " cannot be parsed as Ipv6 string literal.");
         }
       }
 
-      if (hostType == HOST_TYPE.RANGE) {
+      if (isCidrRange) {
         final String leftValue = hostParts[0];
         final String rightValue = hostParts[1];
 

diff --git a/src/test/java/org/jenkinsci/plugins/gwt/whitelist/WhitelistVerifierTest.java b/src/test/java/org/jenkinsci/plugins/gwt/whitelist/WhitelistVerifierTest.java
@@ -1,6 +1,7 @@
 package org.jenkinsci.plugins.gwt.whitelist;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.fail;
 import static org.jenkinsci.plugins.gwt.whitelist.WhitelistVerifier.doVerifyWhitelist;
 
 import java.util.ArrayList;
@@ -139,6 +140,113 @@ public void testThatHostCanBeVerifiedWithSupportedNotations() {
         .isTrue();
   }
 
+  @Test
+  public void testThatHostCanBeVerifiedWithCidr() {
+    final WhitelistItem whitelistItem = new WhitelistItem("2.2.3.0/24");
+    whitelistItem.setHmacEnabled(false);
+
+    final Map<String, List<String>> headers = new HashMap<>();
+    final String postContent = "";
+
+    final boolean enabled = true;
+    final Whitelist whitelist = new Whitelist(enabled, Arrays.asList(whitelistItem));
+
+    assertThat(testDoVerifyWhitelist("2.2.2.255", headers, postContent, whitelist)).isFalse();
+    assertThat(testDoVerifyWhitelist("2.2.3.0", headers, postContent, whitelist)).isTrue();
+    assertThat(testDoVerifyWhitelist("2.2.3.255", headers, postContent, whitelist)).isTrue();
+    assertThat(testDoVerifyWhitelist("2.2.4.0", headers, postContent, whitelist)).isFalse();
+  }
+
+  @Test
+  public void testThatHostCanBeVerifiedWithRanges() {
+    final WhitelistItem whitelistItem = new WhitelistItem("3.2.3.5-3.2.3.10");
+    whitelistItem.setHmacEnabled(false);
+
+    final Map<String, List<String>> headers = new HashMap<>();
+    final String postContent = "";
+
+    final boolean enabled = true;
+    final Whitelist whitelist = new Whitelist(enabled, Arrays.asList(whitelistItem));
+
+    assertThat(testDoVerifyWhitelist("3.2.3.4", headers, postContent, whitelist)).isFalse();
+    assertThat(testDoVerifyWhitelist("3.2.3.5", headers, postContent, whitelist)).isTrue();
+    assertThat(testDoVerifyWhitelist("3.2.3.10", headers, postContent, whitelist)).isTrue();
+    assertThat(testDoVerifyWhitelist("3.2.3.11", headers, postContent, whitelist)).isFalse();
+  }
+
+  @Test
+  public void testThatInvalidRangeThrowsException() {
+    final WhitelistItem whitelistItem = new WhitelistItem("3.2.3.a-3.2.3.10");
+    whitelistItem.setHmacEnabled(false);
+
+    final Map<String, List<String>> headers = new HashMap<>();
+    final String postContent = "";
+
+    final boolean enabled = true;
+    final Whitelist whitelist = new Whitelist(enabled, Arrays.asList(whitelistItem));
+
+    try {
+      doVerifyWhitelist("1.1.1.1", headers, postContent, whitelist);
+      fail("No exception");
+    } catch (final WhitelistException e) {
+      assertThat(e.getMessage()).contains("3.2.3.a-3.2.3.10 is not an Ipv4 string literal.");
+    }
+  }
+
+  @Test
+  public void testThatInvalidCidrThrowsException() {
+    final WhitelistItem whitelistItem = new WhitelistItem("3.2.3.1/a");
+    whitelistItem.setHmacEnabled(false);
+
+    final Map<String, List<String>> headers = new HashMap<>();
+    final String postContent = "";
+
+    final boolean enabled = true;
+    final Whitelist whitelist = new Whitelist(enabled, Arrays.asList(whitelistItem));
+
+    try {
+      doVerifyWhitelist("1.1.1.1", headers, postContent, whitelist);
+      fail("No exception");
+    } catch (final WhitelistException e) {
+      assertThat(e.getMessage()).contains("3.2.3.1/a cannot be parsed as Ipv4 string literal");
+    }
+  }
+
+  @Test
+  public void testThatInvalidStaticThrowsException() {
+    final WhitelistItem whitelistItem = new WhitelistItem("3.2.3.a");
+    whitelistItem.setHmacEnabled(false);
+
+    final Map<String, List<String>> headers = new HashMap<>();
+    final String postContent = "";
+
+    final boolean enabled = true;
+    final Whitelist whitelist = new Whitelist(enabled, Arrays.asList(whitelistItem));
+
+    try {
+      doVerifyWhitelist("1.1.1.1", headers, postContent, whitelist);
+      fail("No exception");
+    } catch (final WhitelistException e) {
+      assertThat(e.getMessage()).contains("3.2.3.a is not a valid IP string literal");
+    }
+  }
+
+  @Test
+  public void testThatHostCanBeVerifiedWithStaticIp() {
+    final WhitelistItem whitelistItem = new WhitelistItem("4.2.3.5");
+    whitelistItem.setHmacEnabled(false);
+
+    final Map<String, List<String>> headers = new HashMap<>();
+    final String postContent = "";
+
+    final boolean enabled = true;
+    final Whitelist whitelist = new Whitelist(enabled, Arrays.asList(whitelistItem));
+
+    assertThat(testDoVerifyWhitelist("4.2.3.4", headers, postContent, whitelist)).isFalse();
+    assertThat(testDoVerifyWhitelist("4.2.3.5", headers, postContent, whitelist)).isTrue();
+    assertThat(testDoVerifyWhitelist("4.2.3.6", headers, postContent, whitelist)).isFalse();
+  }
+
   private boolean testDoVerifyWhitelist(
       final String remoteHost,
       final Map<String, List<String>> headers,