fix(validator): misleading warning for empty acl domains (#1898)

This fixes misleading errors for ACL rules with an empty list of domains. This also enables admins to have a default policy with zero ACL rules as long as the default policy is not deny or bypass. It also adds a rule number to all ACL rule related log messages which is the position in the YAML list plus 1. Lastly it adds comprehensive per rule HIT/MISS logging when Authelia trace logging is enabled. This trace logging includes the rule number.

1 files changed, 50 insertions, 11 deletions

diff --git a/internal/configuration/validator/access_control_test.go b/internal/configuration/validator/access_control_test.go
index 7105680bd..ed810edaf 100644
--- a/internal/configuration/validator/access_control_test.go
+++ b/internal/configuration/validator/access_control_test.go
@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/suite"
 
 	"github.com/authelia/authelia/internal/configuration/schema"
@@ -56,16 +57,42 @@ func (suite *AccessControl) TestShouldRaiseErrorInvalidNetworkGroupNetwork() {
 	suite.Assert().EqualError(suite.validator.Errors()[0], "Network [abc.def.ghi.jkl] from network group: internal must be a valid IP or CIDR")
 }
 
-func (suite *AccessControl) TestShouldRaiseErrorNoRulesDefined() {
-	suite.configuration.Rules = []schema.ACLRule{{}}
+func (suite *AccessControl) TestShouldRaiseErrorWithNoRulesDefined() {
+	suite.configuration.Rules = []schema.ACLRule{}
 
 	ValidateRules(suite.configuration, suite.validator)
 
 	suite.Assert().False(suite.validator.HasWarnings())
-	suite.Require().Len(suite.validator.Errors(), 2)
+	suite.Require().Len(suite.validator.Errors(), 1)
+
+	suite.Assert().EqualError(suite.validator.Errors()[0], "Default Policy [deny] is invalid, access control rules must be provided or a policy must either be 'one_factor' or 'two_factor'")
+}
+
+func (suite *AccessControl) TestShouldRaiseWarningWithNoRulesDefined() {
+	suite.configuration.Rules = []schema.ACLRule{}
+
+	suite.configuration.DefaultPolicy = twoFactorPolicy
+
+	ValidateRules(suite.configuration, suite.validator)
+
+	suite.Assert().False(suite.validator.HasErrors())
+	suite.Require().Len(suite.validator.Warnings(), 1)
+
+	suite.Assert().EqualError(suite.validator.Warnings()[0], "No access control rules have been defined so the default policy two_factor will be applied to all requests")
+}
+
+func (suite *AccessControl) TestShouldRaiseErrorsWithEmptyRules() {
+	suite.configuration.Rules = []schema.ACLRule{{}, {}}
+
+	ValidateRules(suite.configuration, suite.validator)
+
+	suite.Assert().False(suite.validator.HasWarnings())
+	suite.Require().Len(suite.validator.Errors(), 4)
 
-	suite.Assert().EqualError(suite.validator.Errors()[0], "No access control rules have been defined")
-	suite.Assert().EqualError(suite.validator.Errors()[1], "Policy [] for domain: [] is invalid, a policy must either be 'deny', 'two_factor', 'one_factor' or 'bypass'")
+	suite.Assert().EqualError(suite.validator.Errors()[0], "Rule #1 is invalid, a policy must have one or more domains")
+	suite.Assert().EqualError(suite.validator.Errors()[1], "Policy [] for rule #1 domain: [] is invalid, a policy must either be 'deny', 'two_factor', 'one_factor' or 'bypass'")
+	suite.Assert().EqualError(suite.validator.Errors()[2], "Rule #2 is invalid, a policy must have one or more domains")
+	suite.Assert().EqualError(suite.validator.Errors()[3], "Policy [] for rule #2 domain: [] is invalid, a policy must either be 'deny', 'two_factor', 'one_factor' or 'bypass'")
 }
 
 func (suite *AccessControl) TestShouldRaiseErrorInvalidPolicy() {
@@ -81,7 +108,7 @@ func (suite *AccessControl) TestShouldRaiseErrorInvalidPolicy() {
 	suite.Assert().False(suite.validator.HasWarnings())
 	suite.Require().Len(suite.validator.Errors(), 1)
 
-	suite.Assert().EqualError(suite.validator.Errors()[0], "Policy [invalid] for domain: [public.example.com] is invalid, a policy must either be 'deny', 'two_factor', 'one_factor' or 'bypass'")
+	suite.Assert().EqualError(suite.validator.Errors()[0], "Policy [invalid] for rule #1 domain: [public.example.com] is invalid, a policy must either be 'deny', 'two_factor', 'one_factor' or 'bypass'")
 }
 
 func (suite *AccessControl) TestShouldRaiseErrorInvalidNetwork() {
@@ -98,7 +125,7 @@ func (suite *AccessControl) TestShouldRaiseErrorInvalidNetwork() {
 	suite.Assert().False(suite.validator.HasWarnings())
 	suite.Require().Len(suite.validator.Errors(), 1)
 
-	suite.Assert().EqualError(suite.validator.Errors()[0], "Network [abc.def.ghi.jkl/32] for domain: [public.example.com] is not a valid network or network group")
+	suite.Assert().EqualError(suite.validator.Errors()[0], "Network [abc.def.ghi.jkl/32] for rule #1 domain: [public.example.com] is not a valid network or network group")
 }
 
 func (suite *AccessControl) TestShouldRaiseErrorInvalidMethod() {
@@ -115,7 +142,7 @@ func (suite *AccessControl) TestShouldRaiseErrorInvalidMethod() {
 	suite.Assert().False(suite.validator.HasWarnings())
 	suite.Require().Len(suite.validator.Errors(), 1)
 
-	suite.Assert().EqualError(suite.validator.Errors()[0], "Method HOP for domain: [public.example.com] is invalid, must be one of the following methods: GET, HEAD, POST, PUT, PATCH, DELETE, TRACE, CONNECT, OPTIONS")
+	suite.Assert().EqualError(suite.validator.Errors()[0], "Method HOP for rule #1 domain: [public.example.com] is invalid, must be one of the following methods: GET, HEAD, POST, PUT, PATCH, DELETE, TRACE, CONNECT, OPTIONS")
 }
 
 func (suite *AccessControl) TestShouldRaiseErrorInvalidResource() {
@@ -132,7 +159,7 @@ func (suite *AccessControl) TestShouldRaiseErrorInvalidResource() {
 	suite.Assert().False(suite.validator.HasWarnings())
 	suite.Require().Len(suite.validator.Errors(), 1)
 
-	suite.Assert().EqualError(suite.validator.Errors()[0], "Resource [^/(api.*] for domain: [public.example.com] is invalid, error parsing regexp: missing closing ): `^/(api.*`")
+	suite.Assert().EqualError(suite.validator.Errors()[0], "Resource [^/(api.*] for rule #1 domain: [public.example.com] is invalid, error parsing regexp: missing closing ): `^/(api.*`")
 }
 
 func (suite *AccessControl) TestShouldRaiseErrorInvalidSubject() {
@@ -151,10 +178,22 @@ func (suite *AccessControl) TestShouldRaiseErrorInvalidSubject() {
 	suite.Require().Len(suite.validator.Warnings(), 0)
 	suite.Require().Len(suite.validator.Errors(), 2)
 
-	suite.Assert().EqualError(suite.validator.Errors()[0], "Subject [invalid] for domain: [public.example.com] is invalid, must start with 'user:' or 'group:'")
-	suite.Assert().EqualError(suite.validator.Errors()[1], fmt.Sprintf(errAccessControlInvalidPolicyWithSubjects, domains, subjects))
+	suite.Assert().EqualError(suite.validator.Errors()[0], "Subject [invalid] for rule #1 domain: [public.example.com] is invalid, must start with 'user:' or 'group:'")
+	suite.Assert().EqualError(suite.validator.Errors()[1], fmt.Sprintf(errAccessControlInvalidPolicyWithSubjects, 1, domains, subjects))
 }
 
 func TestAccessControl(t *testing.T) {
 	suite.Run(t, new(AccessControl))
 }
+
+func TestShouldReturnCorrectResultsForValidNetworkGroups(t *testing.T) {
+	config := schema.AccessControlConfiguration{
+		Networks: schema.DefaultACLNetwork,
+	}
+
+	validNetwork := IsNetworkGroupValid(config, "internal")
+	invalidNetwork := IsNetworkGroupValid(config, "127.0.0.1")
+
+	assert.True(t, validNetwork)
+	assert.False(t, invalidNetwork)
+}