From f056f9430536280454d9f8220ef28ca86fdabc07 Mon Sep 17 00:00:00 2001
From: Cameron Moore <moorereason@gmail.com>
Date: Wed, 2 Jan 2019 16:50:23 -0600
Subject: [PATCH] Allow multiple values for ip-whitelist

Allow the value of ip-whitelist to consist of multiple space-separated
addresses or CIDRs.

Updates #290
---
 hook/hook.go      | 43 +++++++++++++++++++------------------------
 hook/hook_test.go | 26 +++++++++++++++++++++++++-
 2 files changed, 44 insertions(+), 25 deletions(-)

diff --git a/hook/hook.go b/hook/hook.go
index d8e4635..665a510 100644
--- a/hook/hook.go
+++ b/hook/hook.go
@@ -168,41 +168,36 @@ func CheckScalrSignature(headers map[string]interface{}, body []byte, signingKey
 func CheckIPWhitelist(remoteAddr string, ipRange string) (bool, error) {
 	// Extract IP address from remote address.
 
-	ip := remoteAddr
+	// IPv6 addresses will likely be surrounded by [].
+	ip := strings.Trim(remoteAddr, " []")
 
-	if strings.LastIndex(remoteAddr, ":") != -1 {
-		ip = remoteAddr[0:strings.LastIndex(remoteAddr, ":")]
+	if i := strings.LastIndex(ip, ":"); i != -1 {
+		ip = ip[:i]
 	}
 
-	ip = strings.TrimSpace(ip)
-
-	// IPv6 addresses will likely be surrounded by [], so don't forget to remove those.
-
-	if strings.HasPrefix(ip, "[") && strings.HasSuffix(ip, "]") {
-		ip = ip[1 : len(ip)-1]
-	}
-
-	parsedIP := net.ParseIP(strings.TrimSpace(ip))
-
+	parsedIP := net.ParseIP(ip)
 	if parsedIP == nil {
 		return false, fmt.Errorf("invalid IP address found in remote address '%s'", remoteAddr)
 	}
 
-	// Extract IP range in CIDR form.  If a single IP address is provided, turn it into CIDR form.
+	for _, r := range strings.Fields(ipRange) {
+		// Extract IP range in CIDR form.  If a single IP address is provided, turn it into CIDR form.
 
-	ipRange = strings.TrimSpace(ipRange)
+		if !strings.Contains(r, "/") {
+			r = r + "/32"
+		}
 
-	if !strings.Contains(ipRange, "/") {
-		ipRange = ipRange + "/32"
+		_, cidr, err := net.ParseCIDR(r)
+		if err != nil {
+			return false, err
+		}
+
+		if cidr.Contains(parsedIP) {
+			return true, nil
+		}
 	}
 
-	_, cidr, err := net.ParseCIDR(ipRange)
-
-	if err != nil {
-		return false, err
-	}
-
-	return cidr.Contains(parsedIP), nil
+	return false, nil
 }
 
 // ReplaceParameter replaces parameter value with the passed value in the passed map
diff --git a/hook/hook_test.go b/hook/hook_test.go
index 1794a7c..e0c1c26 100644
--- a/hook/hook_test.go
+++ b/hook/hook_test.go
@@ -108,6 +108,30 @@ func TestCheckScalrSignature(t *testing.T) {
 	}
 }
 
+var checkIPWhitelistTests = []struct {
+	addr    string
+	ipRange string
+	expect  bool
+	ok      bool
+}{
+	{"[ 10.0.0.1:1234 ] ", "  10.0.0.1 ", true, true},
+	{"[ 10.0.0.1:1234 ] ", "  10.0.0.0 ", false, true},
+	{"[ 10.0.0.1:1234 ] ", "  10.0.0.1 10.0.0.1 ", true, true},
+	{"[ 10.0.0.1:1234 ] ", "  10.0.0.0/31 ", true, true},
+	{" [2001:db8:1:2::1:1234] ", "  2001:db8:1::/48 ", true, true},
+	{" [2001:db8:1:2::1:1234] ", "  2001:db8:1::/48 2001:db8:1::/64", true, true},
+	{" [2001:db8:1:2::1:1234] ", "  2001:db8:1::/64 ", false, true},
+}
+
+func TestCheckIPWhitelist(t *testing.T) {
+	for _, tt := range checkIPWhitelistTests {
+		result, err := CheckIPWhitelist(tt.addr, tt.ipRange)
+		if (err == nil) != tt.ok || result != tt.expect {
+			t.Errorf("ip whitelist test failed {%q, %q}:\nwant {expect:%#v, ok:%#v},\ngot {result:%#v, ok:%#v}", tt.addr, tt.ipRange, tt.expect, tt.ok, result, err)
+		}
+	}
+}
+
 var extractParameterTests = []struct {
 	s      string
 	params interface{}
@@ -129,7 +153,7 @@ var extractParameterTests = []struct {
 	{"a.501.b", map[string]interface{}{"a": []interface{}{map[string]interface{}{"b": "y"}, map[string]interface{}{"b": "z"}}}, "", false}, // non-existent slice index
 	{"a.502.b", map[string]interface{}{"a": []interface{}{}}, "", false},                                                                   // non-existent slice index
 	{"a.b.503", map[string]interface{}{"a": map[string]interface{}{"b": []interface{}{"x", "y", "z"}}}, "", false},                         // trailing, non-existent slice index
-	{"a.b", interface{}("a"), "", false},                                                                                                   // non-map, non-slice input
+	{"a.b", interface{}("a"), "", false}, // non-map, non-slice input
 }
 
 func TestExtractParameter(t *testing.T) {