Allow CIDRs in allowed ips config

alexbrazier · Oct 30, 2019 · fa3d775 · fa3d775
1 parent 9d78c24
commit fa3d775
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 2 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -57,6 +57,16 @@ jobs:
           command: |
             cd frontend && yarn lint && echo -e "\033[0;32mESLint passed"
 
+  unit-test:
+    executor: default
+    steps:
+      - attach_workspace:
+          at: /go/src/github.com/alexbrazier
+      - run:
+          name: Run unit tests
+          command: |
+            cd api && go test ./...
+
   e2e-test:
     executor: browsers
     parallelism: 1

diff --git a/README.md b/README.md
@@ -99,7 +99,7 @@ POSTGRES_PASS=password HOSTS=localhost APP_URI=http://localhost:3000 go run serv
 | `AD_CLIENT_ID`              |          |                |                              | Azure AD client ID                                                                                     |
 | `AD_CLIENT_SECRET`          |          |                |                              | Azure AD client secret                                                                                 |
 | `SESSION_TOKEN`             |          |                |                              | Secret session token to store the user sessions                                                        |
-| `ALLOWED_IPS`               |          |                | 110.1.10.2,1.1.22.1          | IP addresses that are always allowed access, even with auth enabled                                    |
+| `ALLOWED_IPS`               |          |                | 110.1.10.2,1.1.22.0/24       | IP addresses or CIDRs that are always allowed access, even with auth enabled                           |
 | `ALLOW_FORWARDED_FOR`       |          | false          |                              | Retrieve origin IP from X-Forwarded-For header. Only enable if source is trusted, e.g. via Cloudfront  |
 | `FORWARDED_FOR_TRUST_LEVEL` |          | 1              |                              | Number of levels to trust X-Forwarded-For header - should map to number of proxies used                |
 

diff --git a/api/handler/ip_restrict.go b/api/handler/ip_restrict.go
@@ -6,12 +6,16 @@ import (
 
 	"github.com/alexbrazier/go-url/api/config"
 	"github.com/labstack/echo"
+	"net"
 )
 
 func ipAllowed(allowedIPs, ips []string) bool {
 	for _, allowedIP := range allowedIPs {
+		_, ipNet, err := net.ParseCIDR(allowedIP)
+		isCIDR := err == nil
 		for _, ip := range ips {
-			if strings.TrimSpace(ip) == allowedIP {
+			trimmedIp := strings.TrimSpace(ip)
+			if trimmedIp == allowedIP || (isCIDR && ipNet.Contains(net.ParseIP(trimmedIp))) {
 				return true
 			}
 		}

diff --git a/api/handler/ip_restrict_test.go b/api/handler/ip_restrict_test.go
@@ -0,0 +1,29 @@
+package handler
+
+import "testing"
+
+func TestIpAllowed(t *testing.T) {
+	tables := []struct {
+		allowedIps []string
+		actualIps  []string
+		allowed    bool
+	}{
+		{[]string{"10.1.1.1"}, []string{"10.1.1.1"}, true},             // same IP
+		{[]string{"10.1.1.1"}, []string{"10.1.1.0"}, false},            // different IP
+		{[]string{"10.1.1.1"}, []string{"10.1.1.0", "10.1.1.1"}, true}, // multiple IPs
+		{[]string{"10.1.1.1", "10.1.1.2"}, []string{"10.1.1.2"}, true}, // multiple allowedIPs
+		{[]string{"10.1.1.1/32"}, []string{"10.1.1.1"}, true},          // allowed CIDR
+		{[]string{"10.1.1.1/32"}, []string{"10.1.1.0"}, false},         // disallowed CIDR
+		{[]string{"10.1.1.1/28"}, []string{"10.1.1.0"}, false},         // first matching ip CIDR
+		{[]string{"10.1.1.1/28"}, []string{"10.1.1.15"}, false},        // last matching ip CIDR
+		{[]string{"10.1.1.1/28"}, []string{"10.1.1.16"}, false},        // first non matching matching ip CIDR
+		{[]string{"10.1.1.1/28"}, []string{"10.1.0.255"}, false},       // last non matching matching ip CIDR
+	}
+
+	for _, table := range tables {
+		allowed := ipAllowed(table.allowedIps, table.actualIps)
+		if allowed != table.allowed {
+			t.Errorf("Expected %t, with allowedIps: %q and actualIps: %q", table.allowed, table.allowedIps, table.actualIps)
+		}
+	}
+}