RPJosh
/
authelia
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

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.
pull/1914/head
James Elliott 2021-04-14 20:53:23 +10:00 committed by GitHub
parent a6ebf4ad4c
commit 1e30b00f7e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 136 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
internal/authorization/access_control_rule.go
View File

@ -11,16 +11,17 @@ import (
func NewAccessControlRules(config schema.AccessControlConfiguration) (rules []*AccessControlRule) { func NewAccessControlRules(config schema.AccessControlConfiguration) (rules []*AccessControlRule) {
networksMap, networksCacheMap := parseSchemaNetworks(config.Networks) networksMap, networksCacheMap := parseSchemaNetworks(config.Networks)
for _, schemaRule := range config.Rules { for i, schemaRule := range config.Rules {
rules = append(rules, NewAccessControlRule(schemaRule, networksMap, networksCacheMap)) rules = append(rules, NewAccessControlRule(i+1, schemaRule, networksMap, networksCacheMap))
} }
return rules return rules
} }
// NewAccessControlRule parses a schema ACL and generates an internal ACL. // NewAccessControlRule parses a schema ACL and generates an internal ACL.
func NewAccessControlRule(rule schema.ACLRule, networksMap map[string][]*net.IPNet, networksCacheMap map[string]*net.IPNet) *AccessControlRule { func NewAccessControlRule(pos int, rule schema.ACLRule, networksMap map[string][]*net.IPNet, networksCacheMap map[string]*net.IPNet) *AccessControlRule {
return &AccessControlRule{ return &AccessControlRule{
Position: pos,
Domains: schemaDomainsToACL(rule.Domains), Domains: schemaDomainsToACL(rule.Domains),
Resources: schemaResourcesToACL(rule.Resources), Resources: schemaResourcesToACL(rule.Resources),
Methods: schemaMethodsToACL(rule.Methods), Methods: schemaMethodsToACL(rule.Methods),
@ -32,6 +33,7 @@ func NewAccessControlRule(rule schema.ACLRule, networksMap map[string][]*net.IPN
// AccessControlRule controls and represents an ACL internally. // AccessControlRule controls and represents an ACL internally.
type AccessControlRule struct { type AccessControlRule struct {
Position int
Domains []AccessControlDomain Domains []AccessControlDomain
Resources []AccessControlResource Resources []AccessControlResource
Methods []string Methods []string

22
internal/authorization/authorizer.go
View File

@ -1,6 +1,8 @@
package authorization package authorization
import ( import (
"github.com/sirupsen/logrus"
"github.com/authelia/authelia/internal/configuration/schema" "github.com/authelia/authelia/internal/configuration/schema"
"github.com/authelia/authelia/internal/logging" "github.com/authelia/authelia/internal/logging"
) )
@ -13,6 +15,13 @@ type Authorizer struct {
// NewAuthorizer create an instance of authorizer with a given access control configuration. // NewAuthorizer create an instance of authorizer with a given access control configuration.
func NewAuthorizer(configuration schema.AccessControlConfiguration) *Authorizer { func NewAuthorizer(configuration schema.AccessControlConfiguration) *Authorizer {
if logging.Logger().IsLevelEnabled(logrus.TraceLevel) {
return &Authorizer{
defaultPolicy: PolicyToLevel(configuration.DefaultPolicy),
rules: NewAccessControlRules(configuration),
}
}
return &Authorizer{ return &Authorizer{
defaultPolicy: PolicyToLevel(configuration.DefaultPolicy), defaultPolicy: PolicyToLevel(configuration.DefaultPolicy),
rules: NewAccessControlRules(configuration), rules: NewAccessControlRules(configuration),
@ -35,17 +44,24 @@ func (p *Authorizer) IsSecondFactorEnabled() bool {
} }
// GetRequiredLevel retrieve the required level of authorization to access the object. // GetRequiredLevel retrieve the required level of authorization to access the object.
func (p *Authorizer) GetRequiredLevel(subject Subject, object Object) Level { func (p Authorizer) GetRequiredLevel(subject Subject, object Object) Level {
logger := logging.Logger() logger := logging.Logger()
logger.Tracef("Check authorization of subject %s and url %s.", subject.String(), object.String())
logger.Debugf("Check authorization of subject %s and object %s (method %s).",
subject.String(), object.String(), object.Method)
for _, rule := range p.rules { for _, rule := range p.rules {
if rule.IsMatch(subject, object) { if rule.IsMatch(subject, object) {
logger.Tracef(traceFmtACLHitMiss, "HIT", rule.Position, subject.String(), object.String(), object.Method)
return rule.Policy return rule.Policy
} }
logger.Tracef(traceFmtACLHitMiss, "MISS", rule.Position, subject.String(), object.String(), object.Method)
} }
logger.Tracef("No matching rule for subject %s and url %s... Applying default policy.", subject.String(), object.String()) logger.Debugf("No matching rule for subject %s and url %s... Applying default policy.",
subject.String(), object.String())
return p.defaultPolicy return p.defaultPolicy
} }

2
internal/authorization/const.go
View File

@ -16,3 +16,5 @@ const (
const userPrefix = "user:" const userPrefix = "user:"
const groupPrefix = "group:" const groupPrefix = "group:"
const traceFmtACLHitMiss = "ACL %s Position %d for subject %s and object %s (Method %s)"

62
internal/configuration/validator/access_control.go
View File

@ -12,7 +12,7 @@ import (
// IsPolicyValid check if policy is valid. // IsPolicyValid check if policy is valid.
func IsPolicyValid(policy string) (isValid bool) { func IsPolicyValid(policy string) (isValid bool) {
return policy == denyPolicy || policy == "one_factor" || policy == "two_factor" || policy == bypassPolicy return policy == denyPolicy || policy == oneFactorPolicy || policy == twoFactorPolicy || policy == bypassPolicy
} }
// IsResourceValid check if a resource is valid. // IsResourceValid check if a resource is valid.
@ -68,61 +68,75 @@ func ValidateAccessControl(configuration schema.AccessControlConfiguration, vali
// ValidateRules validates an ACL Rule configuration. // ValidateRules validates an ACL Rule configuration.
func ValidateRules(configuration schema.AccessControlConfiguration, validator *schema.StructValidator) { func ValidateRules(configuration schema.AccessControlConfiguration, validator *schema.StructValidator) {
for _, r := range configuration.Rules { if configuration.Rules == nil || len(configuration.Rules) == 0 {
if len(r.Domains) == 0 { if configuration.DefaultPolicy != oneFactorPolicy && configuration.DefaultPolicy != twoFactorPolicy {
validator.Push(fmt.Errorf("No access control rules have been defined")) validator.Push(fmt.Errorf("Default Policy [%s] is invalid, access control rules must be provided or a policy must either be 'one_factor' or 'two_factor'", configuration.DefaultPolicy))
return
} }
if !IsPolicyValid(r.Policy) { validator.PushWarning(fmt.Errorf("No access control rules have been defined so the default policy %s will be applied to all requests", configuration.DefaultPolicy))
validator.Push(fmt.Errorf("Policy [%s] for domain: %s is invalid, a policy must either be 'deny', 'two_factor', 'one_factor' or 'bypass'", r.Policy, r.Domains))
return
}
for i, rule := range configuration.Rules {
rulePosition := i + 1
if len(rule.Domains) == 0 {
validator.Push(fmt.Errorf("Rule #%d is invalid, a policy must have one or more domains", rulePosition))
} }
validateNetworks(r, configuration, validator) if !IsPolicyValid(rule.Policy) {
validator.Push(fmt.Errorf("Policy [%s] for rule #%d domain: %s is invalid, a policy must either be 'deny', 'two_factor', 'one_factor' or 'bypass'", rule.Policy, rulePosition, rule.Domains))
}
validateResources(r, validator) validateNetworks(rulePosition, rule, configuration, validator)
validateSubjects(r, validator) validateResources(rulePosition, rule, validator)
validateMethods(r, validator) validateSubjects(rulePosition, rule, validator)
if r.Policy == bypassPolicy && len(r.Subjects) != 0 { validateMethods(rulePosition, rule, validator)
validator.Push(fmt.Errorf(errAccessControlInvalidPolicyWithSubjects, r.Domains, r.Subjects))
if rule.Policy == bypassPolicy && len(rule.Subjects) != 0 {
validator.Push(fmt.Errorf(errAccessControlInvalidPolicyWithSubjects, rulePosition, rule.Domains, rule.Subjects))
} }
} }
} }
func validateNetworks(r schema.ACLRule, configuration schema.AccessControlConfiguration, validator *schema.StructValidator) { func validateNetworks(rulePosition int, rule schema.ACLRule, configuration schema.AccessControlConfiguration, validator *schema.StructValidator) {
for _, network := range r.Networks { for _, network := range rule.Networks {
if !IsNetworkValid(network) { if !IsNetworkValid(network) {
if !IsNetworkGroupValid(configuration, network) { if !IsNetworkGroupValid(configuration, network) {
validator.Push(fmt.Errorf("Network %s for domain: %s is not a valid network or network group", r.Networks, r.Domains)) validator.Push(fmt.Errorf("Network %s for rule #%d domain: %s is not a valid network or network group", rule.Networks, rulePosition, rule.Domains))
} }
} }
} }
} }
func validateResources(r schema.ACLRule, validator *schema.StructValidator) { func validateResources(rulePosition int, rule schema.ACLRule, validator *schema.StructValidator) {
for _, resource := range r.Resources { for _, resource := range rule.Resources {
if err := IsResourceValid(resource); err != nil { if err := IsResourceValid(resource); err != nil {
validator.Push(fmt.Errorf("Resource %s for domain: %s is invalid, %s", r.Resources, r.Domains, err)) validator.Push(fmt.Errorf("Resource %s for rule #%d domain: %s is invalid, %s", rule.Resources, rulePosition, rule.Domains, err))
} }
} }
} }
func validateSubjects(r schema.ACLRule, validator *schema.StructValidator) { func validateSubjects(rulePosition int, rule schema.ACLRule, validator *schema.StructValidator) {
for _, subjectRule := range r.Subjects { for _, subjectRule := range rule.Subjects {
for _, subject := range subjectRule { for _, subject := range subjectRule {
if !IsSubjectValid(subject) { if !IsSubjectValid(subject) {
validator.Push(fmt.Errorf("Subject %s for domain: %s is invalid, must start with 'user:' or 'group:'", subjectRule, r.Domains)) validator.Push(fmt.Errorf("Subject %s for rule #%d domain: %s is invalid, must start with 'user:' or 'group:'", subjectRule, rulePosition, rule.Domains))
} }
} }
} }
} }
func validateMethods(r schema.ACLRule, validator *schema.StructValidator) { func validateMethods(rulePosition int, rule schema.ACLRule, validator *schema.StructValidator) {
for _, method := range r.Methods { for _, method := range rule.Methods {
if !utils.IsStringInSliceFold(method, validRequestMethods) { if !utils.IsStringInSliceFold(method, validRequestMethods) {
validator.Push(fmt.Errorf("Method %s for domain: %s is invalid, must be one of the following methods: %s", method, r.Domains, strings.Join(validRequestMethods, ", "))) validator.Push(fmt.Errorf("Method %s for rule #%d domain: %s is invalid, must be one of the following methods: %s", method, rulePosition, rule.Domains, strings.Join(validRequestMethods, ", ")))
} }
} }
} }

61
internal/configuration/validator/access_control_test.go
View File

@ -4,6 +4,7 @@ import (
"fmt" "fmt"
"testing" "testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/suite" "github.com/stretchr/testify/suite"
"github.com/authelia/authelia/internal/configuration/schema" "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") 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() { func (suite *AccessControl) TestShouldRaiseErrorWithNoRulesDefined() {
suite.configuration.Rules = []schema.ACLRule{{}} suite.configuration.Rules = []schema.ACLRule{}
ValidateRules(suite.configuration, suite.validator) ValidateRules(suite.configuration, suite.validator)
suite.Assert().False(suite.validator.HasWarnings()) 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], "No access control rules have been defined") 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'")
suite.Assert().EqualError(suite.validator.Errors()[1], "Policy [] for domain: [] is invalid, a policy must either be 'deny', 'two_factor', 'one_factor' or 'bypass'") }
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], "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() { func (suite *AccessControl) TestShouldRaiseErrorInvalidPolicy() {
@ -81,7 +108,7 @@ func (suite *AccessControl) TestShouldRaiseErrorInvalidPolicy() {
suite.Assert().False(suite.validator.HasWarnings()) suite.Assert().False(suite.validator.HasWarnings())
suite.Require().Len(suite.validator.Errors(), 1) 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() { func (suite *AccessControl) TestShouldRaiseErrorInvalidNetwork() {
@ -98,7 +125,7 @@ func (suite *AccessControl) TestShouldRaiseErrorInvalidNetwork() {
suite.Assert().False(suite.validator.HasWarnings()) suite.Assert().False(suite.validator.HasWarnings())
suite.Require().Len(suite.validator.Errors(), 1) 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() { func (suite *AccessControl) TestShouldRaiseErrorInvalidMethod() {
@ -115,7 +142,7 @@ func (suite *AccessControl) TestShouldRaiseErrorInvalidMethod() {
suite.Assert().False(suite.validator.HasWarnings()) suite.Assert().False(suite.validator.HasWarnings())
suite.Require().Len(suite.validator.Errors(), 1) 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() { func (suite *AccessControl) TestShouldRaiseErrorInvalidResource() {
@ -132,7 +159,7 @@ func (suite *AccessControl) TestShouldRaiseErrorInvalidResource() {
suite.Assert().False(suite.validator.HasWarnings()) suite.Assert().False(suite.validator.HasWarnings())
suite.Require().Len(suite.validator.Errors(), 1) 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() { func (suite *AccessControl) TestShouldRaiseErrorInvalidSubject() {
@ -151,10 +178,22 @@ func (suite *AccessControl) TestShouldRaiseErrorInvalidSubject() {
suite.Require().Len(suite.validator.Warnings(), 0) suite.Require().Len(suite.validator.Warnings(), 0)
suite.Require().Len(suite.validator.Errors(), 2) 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()[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, domains, subjects)) suite.Assert().EqualError(suite.validator.Errors()[1], fmt.Sprintf(errAccessControlInvalidPolicyWithSubjects, 1, domains, subjects))
} }
func TestAccessControl(t *testing.T) { func TestAccessControl(t *testing.T) {
suite.Run(t, new(AccessControl)) 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)
}

4
internal/configuration/validator/configuration.go
View File

@ -72,9 +72,7 @@ func ValidateConfiguration(configuration *schema.Configuration, validator *schem
ValidateAccessControl(configuration.AccessControl, validator) ValidateAccessControl(configuration.AccessControl, validator)
if configuration.AccessControl.Rules != nil { ValidateRules(configuration.AccessControl, validator)
ValidateRules(configuration.AccessControl, validator)
}
ValidateSession(&configuration.Session, validator) ValidateSession(&configuration.Session, validator)

18
internal/configuration/validator/configuration_test.go
View File

@ -17,8 +17,12 @@ func newDefaultConfig() schema.Configuration {
config.LogLevel = "info" config.LogLevel = "info"
config.LogFormat = "text" config.LogFormat = "text"
config.JWTSecret = testJWTSecret config.JWTSecret = testJWTSecret
config.AuthenticationBackend.File = new(schema.FileAuthenticationBackendConfiguration) config.AuthenticationBackend.File = &schema.FileAuthenticationBackendConfiguration{
config.AuthenticationBackend.File.Path = "/a/path" Path: "/a/path",
}
config.AccessControl = schema.AccessControlConfiguration{
DefaultPolicy: "two_factor",
}
config.Session = schema.SessionConfiguration{ config.Session = schema.SessionConfiguration{
Domain: "example.com", Domain: "example.com",
Name: "authelia_session", Name: "authelia_session",
@ -98,6 +102,16 @@ func TestShouldAddDefaultAccessControl(t *testing.T) {
validator := schema.NewStructValidator() validator := schema.NewStructValidator()
config := newDefaultConfig() config := newDefaultConfig()
config.AccessControl.DefaultPolicy = ""
config.AccessControl.Rules = []schema.ACLRule{
{
Policy: "bypass",
Domains: []string{
"public.example.com",
},
},
}
ValidateConfiguration(&config, validator) ValidateConfiguration(&config, validator)
require.Len(t, validator.Errors(), 0) require.Len(t, validator.Errors(), 0)
assert.NotNil(t, config.AccessControl) assert.NotNil(t, config.AccessControl)

8
internal/configuration/validator/const.go
View File

@ -10,8 +10,10 @@ const (
errFilePHashing = "config key incorrect: authentication_backend.file.password_hashing should be authentication_backend.file.password" errFilePHashing = "config key incorrect: authentication_backend.file.password_hashing should be authentication_backend.file.password"
errFilePOptions = "config key incorrect: authentication_backend.file.password_options should be authentication_backend.file.password" errFilePOptions = "config key incorrect: authentication_backend.file.password_options should be authentication_backend.file.password"
denyPolicy = "deny" bypassPolicy = "bypass"
bypassPolicy = "bypass" oneFactorPolicy = "one_factor"
twoFactorPolicy = "two_factor"
denyPolicy = "deny"
argon2id = "argon2id" argon2id = "argon2id"
sha512 = "sha512" sha512 = "sha512"
@ -30,7 +32,7 @@ const (
testTLSCert = "/tmp/cert.pem" testTLSCert = "/tmp/cert.pem"
testTLSKey = "/tmp/key.pem" testTLSKey = "/tmp/key.pem"
errAccessControlInvalidPolicyWithSubjects = "Policy [bypass] for domain %s with subjects %s is invalid. It is " + errAccessControlInvalidPolicyWithSubjects = "Policy [bypass] for rule #%d domain %s with subjects %s is invalid. It is " +
"not supported to configure both policy bypass and subjects. For more information see: " + "not supported to configure both policy bypass and subjects. For more information see: " +
"https://www.authelia.com/docs/configuration/access-control.html#combining-subjects-and-the-bypass-policy" "https://www.authelia.com/docs/configuration/access-control.html#combining-subjects-and-the-bypass-policy"
) )