Improve the clients/find API response

AGHTechDoc.md

@@ -940,7 +940,7 @@ Error response (Client not found):
 ### API: Find clients by IP
 
 This method returns the list of clients (manual and auto-clients) matching the IP list.
-For auto-clients only `name`, `ids`, `whois_info`, `disallowed` fields are set.  Other fields are empty.
+For auto-clients only `name`, `ids`, `whois_info`, `disallowed`, and `disallowed_rule` fields are set.  Other fields are empty.
 
 Request:
 
@@ -967,17 +967,15 @@ Response:
 				...
 			}
 
-			"disallowed": "..."
+			"disallowed": false,
+			"disallowed_rule": "..."
 		}
 	}
 	...
 	]
 
-`disallowed`:
-* "": IP is allowed
-* not "", e.g. "127.0.0.0/24" - IP is disallowed by "disallowed IP list", and the string contains the matched rule (IP or CIDR)
-* "not-in-allowed-list" - IP is disallowed by "allowed IP list"
-
+* `disallowed` - whether the client's IP is blocked or not.
+* `disallowed_rule` - the rule due to which the client is disallowed. If `disallowed` is `true`, and this string is empty - it means that the client IP is disallowed by the "allowed IP list", i.e. it is not included in allowed.
 
 ## DNS general settings
 

dnsforward/access.go

@@ -80,6 +80,9 @@ func processIPCIDRArray(dst *map[string]bool, dstIPNet *[]net.IPNet, src []strin
 }
 
 // IsBlockedIP - return TRUE if this client should be blocked
+// Returns the item from the "disallowedClients" list that lead to blocking IP.
+// If it returns TRUE and an empty string, it means that the "allowedClients" is not empty,
+// but the ip does not belong to it.
 func (a *accessCtx) IsBlockedIP(ip string) (bool, string) {
 	a.lock.Lock()
 	defer a.lock.Unlock()
@@ -99,7 +102,7 @@ func (a *accessCtx) IsBlockedIP(ip string) (bool, string) {
 			}
 		}
 
-		return true, "not-in-allowed-list"
+		return true, ""
 	}
 
 	_, ok := a.disallowedClients[ip]

dnsforward/access_test.go

@@ -0,0 +1,73 @@
+package dnsforward
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestIsBlockedIPAllowed(t *testing.T) {
+	a := &accessCtx{}
+	assert.True(t, a.Init([]string{"1.1.1.1", "2.2.0.0/16"}, nil, nil) == nil)
+
+	disallowed, disallowedRule := a.IsBlockedIP("1.1.1.1")
+	assert.False(t, disallowed)
+	assert.Equal(t, "", disallowedRule)
+
+	disallowed, disallowedRule = a.IsBlockedIP("1.1.1.2")
+	assert.True(t, disallowed)
+	assert.Equal(t, "", disallowedRule)
+
+	disallowed, disallowedRule = a.IsBlockedIP("2.2.1.1")
+	assert.False(t, disallowed)
+	assert.Equal(t, "", disallowedRule)
+
+	disallowed, disallowedRule = a.IsBlockedIP("2.3.1.1")
+	assert.True(t, disallowed)
+	assert.Equal(t, "", disallowedRule)
+}
+
+func TestIsBlockedIPDisallowed(t *testing.T) {
+	a := &accessCtx{}
+	assert.True(t, a.Init(nil, []string{"1.1.1.1", "2.2.0.0/16"}, nil) == nil)
+
+	disallowed, disallowedRule := a.IsBlockedIP("1.1.1.1")
+	assert.True(t, disallowed)
+	assert.Equal(t, "1.1.1.1", disallowedRule)
+
+	disallowed, disallowedRule = a.IsBlockedIP("1.1.1.2")
+	assert.False(t, disallowed)
+	assert.Equal(t, "", disallowedRule)
+
+	disallowed, disallowedRule = a.IsBlockedIP("2.2.1.1")
+	assert.True(t, disallowed)
+	assert.Equal(t, "2.2.0.0/16", disallowedRule)
+
+	disallowed, disallowedRule = a.IsBlockedIP("2.3.1.1")
+	assert.False(t, disallowed)
+	assert.Equal(t, "", disallowedRule)
+}
+
+func TestIsBlockedIPBlockedDomain(t *testing.T) {
+	a := &accessCtx{}
+	assert.True(t, a.Init(nil, nil, []string{"host1",
+		"host2",
+		"*.host.com",
+		"||host3.com^",
+	}) == nil)
+
+	// match by "host2.com"
+	assert.True(t, a.IsBlockedDomain("host1"))
+	assert.True(t, a.IsBlockedDomain("host2"))
+	assert.True(t, !a.IsBlockedDomain("host3"))
+
+	// match by wildcard "*.host.com"
+	assert.True(t, !a.IsBlockedDomain("host.com"))
+	assert.True(t, a.IsBlockedDomain("asdf.host.com"))
+	assert.True(t, a.IsBlockedDomain("qwer.asdf.host.com"))
+	assert.True(t, !a.IsBlockedDomain("asdf.zhost.com"))
+
+	// match by wildcard "||host3.com^"
+	assert.True(t, a.IsBlockedDomain("host3.com"))
+	assert.True(t, a.IsBlockedDomain("asdf.host3.com"))
+}

dnsforward/dnsforward_test.go

@@ -874,58 +874,6 @@ func publicKey(priv interface{}) interface{} {
 	}
 }
 
-func TestIsBlockedIPAllowed(t *testing.T) {
-	a := &accessCtx{}
-	assert.True(t, a.Init([]string{"1.1.1.1", "2.2.0.0/16"}, nil, nil) == nil)
-
-	disallowed, _ := a.IsBlockedIP("1.1.1.1")
-	assert.False(t, disallowed)
-	disallowed, _ = a.IsBlockedIP("1.1.1.2")
-	assert.True(t, disallowed)
-	disallowed, _ = a.IsBlockedIP("2.2.1.1")
-	assert.False(t, disallowed)
-	disallowed, _ = a.IsBlockedIP("2.3.1.1")
-	assert.True(t, disallowed)
-}
-
-func TestIsBlockedIPDisallowed(t *testing.T) {
-	a := &accessCtx{}
-	assert.True(t, a.Init(nil, []string{"1.1.1.1", "2.2.0.0/16"}, nil) == nil)
-
-	disallowed, _ := a.IsBlockedIP("1.1.1.1")
-	assert.True(t, disallowed)
-	disallowed, _ = a.IsBlockedIP("1.1.1.2")
-	assert.False(t, disallowed)
-	disallowed, _ = a.IsBlockedIP("2.2.1.1")
-	assert.True(t, disallowed)
-	disallowed, _ = a.IsBlockedIP("2.3.1.1")
-	assert.False(t, disallowed)
-}
-
-func TestIsBlockedIPBlockedDomain(t *testing.T) {
-	a := &accessCtx{}
-	assert.True(t, a.Init(nil, nil, []string{"host1",
-		"host2",
-		"*.host.com",
-		"||host3.com^",
-	}) == nil)
-
-	// match by "host2.com"
-	assert.True(t, a.IsBlockedDomain("host1"))
-	assert.True(t, a.IsBlockedDomain("host2"))
-	assert.True(t, !a.IsBlockedDomain("host3"))
-
-	// match by wildcard "*.host.com"
-	assert.True(t, !a.IsBlockedDomain("host.com"))
-	assert.True(t, a.IsBlockedDomain("asdf.host.com"))
-	assert.True(t, a.IsBlockedDomain("qwer.asdf.host.com"))
-	assert.True(t, !a.IsBlockedDomain("asdf.zhost.com"))
-
-	// match by wildcard "||host3.com^"
-	assert.True(t, a.IsBlockedDomain("host3.com"))
-	assert.True(t, a.IsBlockedDomain("asdf.host3.com"))
-}
-
 func TestValidateUpstream(t *testing.T) {
 	invalidUpstreams := []string{"1.2.3.4.5",
 		"123.3.7m",

home/clients.go

@@ -80,7 +80,8 @@ type clientsContainer struct {
 	// dhcpServer is used for looking up clients IP addresses by MAC addresses
 	dhcpServer *dhcpd.Server
 
-	DNSServer *dnsforward.Server
+	// dnsServer is used for checking clients IP status access list status
+	dnsServer *dnsforward.Server
 
 	autoHosts *util.AutoHosts // get entries from system hosts-files
 

home/clients_http.go

@@ -24,10 +24,14 @@ type clientJSON struct {
 
 	WhoisInfo map[string]interface{} `json:"whois_info"`
 
-	// * "": IP is allowed
-	// * not "", e.g. "127.0.0.0/24" - IP is disallowed by "disallowed IP list", and the string contains the matched rule (IP or CIDR)
-	// * "not-in-allowed-list" - IP is disallowed by "allowed IP list"
-	Disallowed string `json:"disallowed"`
+	// Disallowed - if true -- client's IP is not disallowed
+	// Otherwise, it is blocked.
+	Disallowed bool `json:"disallowed"`
+
+	// DisallowedRule - the rule due to which the client is disallowed
+	// If Disallowed is true, and this string is empty - it means that the client IP
+	// is disallowed by the "allowed IP list", i.e. it is not included in allowed.
+	DisallowedRule string `json:"disallowed_rule"`
 }
 
 type clientHostJSON struct {
@@ -45,7 +49,7 @@ type clientListJSON struct {
 }
 
 // respond with information about configured clients
-func (clients *clientsContainer) handleGetClients(w http.ResponseWriter, r *http.Request) {
+func (clients *clientsContainer) handleGetClients(w http.ResponseWriter, _ *http.Request) {
 	data := clientListJSON{}
 
 	clients.lock.Lock()
@@ -257,20 +261,12 @@ func (clients *clientsContainer) handleFindClient(w http.ResponseWriter, r *http
 			}
 			cj := clientHostToJSON(ip, ch)
 
-			disallowed, disallowedRule := clients.DNSServer.IsBlockedIP(ip)
-			if disallowed {
-				cj.Disallowed = disallowedRule
-			}
-
+			cj.Disallowed, cj.DisallowedRule = clients.dnsServer.IsBlockedIP(ip)
 			el[ip] = cj
 		} else {
 			cj := clientToJSON(&c)
 
-			disallowed, disallowedRule := clients.DNSServer.IsBlockedIP(ip)
-			if disallowed {
-				cj.Disallowed = disallowedRule
-			}
-
+			cj.Disallowed, cj.DisallowedRule = clients.dnsServer.IsBlockedIP(ip)
 			el[ip] = cj
 		}
 

home/dns.go

@@ -70,7 +70,7 @@ func initDNSServer() error {
 		p.DHCPServer = Context.dhcpServer
 	}
 	Context.dnsServer = dnsforward.NewServer(p)
-	Context.clients.DNSServer = Context.dnsServer
+	Context.clients.dnsServer = Context.dnsServer
 	dnsConfig := generateServerConfig()
 	err = Context.dnsServer.Prepare(&dnsConfig)
 	if err != nil {

openapi/openapi.yaml

@@ -1796,11 +1796,15 @@ components:
                     items:
                         $ref: "#/components/schemas/WhoisInfo"
                 disallowed:
+                    type: boolean
                     description: >
-                        * "": IP is allowed
-                        * not "", e.g. "127.0.0.0/24" - IP is disallowed by "disallowed IP list", and the string contains the matched rule (IP or CIDR)
-                        * "not-in-allowed-list" - IP is disallowed by "allowed IP list"
+                        Whether the client's IP is blocked or not.
+                disallowed_rule:
                     type: string
+                    description: >
+                        The rule due to which the client is disallowed.
+                        If `disallowed` is `true`, and this string is empty - it means that
+                        the client IP is disallowed by the "allowed IP list", i.e. it is not included in allowed list.
 
         WhoisInfo:
             type: object