From e784a72735a1389db148867a2cb32b2e0e2d4674 Mon Sep 17 00:00:00 2001 From: James Elliott Date: Wed, 24 May 2023 19:23:46 +1000 Subject: [PATCH] test(authorization): add missing tests (#5478) Signed-off-by: James Elliott --- .../authorization/access_control_domain.go | 4 +- .../access_control_domain_test.go | 64 ++++++++++++++++ .../access_control_query_test.go | 62 ++++++++++++++++ .../authorization/access_control_rule_test.go | 33 +++++++++ internal/authorization/regexp.go | 12 +-- internal/authorization/regexp_test.go | 42 +++++++++++ internal/authorization/types_test.go | 30 ++++++++ internal/authorization/util_test.go | 74 +++++++++++++++++++ 8 files changed, 311 insertions(+), 10 deletions(-) create mode 100644 internal/authorization/access_control_domain_test.go create mode 100644 internal/authorization/access_control_query_test.go create mode 100644 internal/authorization/access_control_rule_test.go create mode 100644 internal/authorization/regexp_test.go diff --git a/internal/authorization/access_control_domain.go b/internal/authorization/access_control_domain.go index 463e3293d..21fe96b80 100644 --- a/internal/authorization/access_control_domain.go +++ b/internal/authorization/access_control_domain.go @@ -66,13 +66,13 @@ func (m AccessControlDomainMatcher) IsMatch(domain string, subject Subject) (mat return strings.HasSuffix(domain, m.Name) case m.UserWildcard: if subject.IsAnonymous() && strings.HasSuffix(domain, m.Name) { - return true + return len(domain) > len(m.Name) } return domain == fmt.Sprintf("%s%s", subject.Username, m.Name) case m.GroupWildcard: if subject.IsAnonymous() && strings.HasSuffix(domain, m.Name) { - return true + return len(domain) > len(m.Name) } i := strings.Index(domain, ".") diff --git a/internal/authorization/access_control_domain_test.go b/internal/authorization/access_control_domain_test.go new file mode 100644 index 000000000..19c4a908c --- /dev/null +++ b/internal/authorization/access_control_domain_test.go @@ -0,0 +1,64 @@ +package authorization + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestAccessControlDomain_IsMatch(t *testing.T) { + testCases := []struct { + name string + have *AccessControlDomainMatcher + domain string + subject Subject + expected bool + }{ + { + "ShouldMatchDomainSuffixUserWildcard", + &AccessControlDomainMatcher{ + Name: "-user.domain.com", + UserWildcard: true, + }, + "a-user.domain.com", + Subject{}, + true, + }, + { + "ShouldMatchDomainSuffixGroupWildcard", + &AccessControlDomainMatcher{ + Name: "-group.domain.com", + GroupWildcard: true, + }, + "a-group.domain.com", + Subject{}, + true, + }, + { + "ShouldNotMatchExactDomainWithUserWildcard", + &AccessControlDomainMatcher{ + Name: "-user.domain.com", + UserWildcard: true, + }, + "-user.domain.com", + Subject{}, + false, + }, + { + "ShouldMatchWildcard", + &AccessControlDomainMatcher{ + Name: "user.domain.com", + Wildcard: true, + }, + "abc.user.domain.com", + Subject{}, + true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.expected, tc.have.IsMatch(tc.domain, tc.subject)) + }) + } +} diff --git a/internal/authorization/access_control_query_test.go b/internal/authorization/access_control_query_test.go new file mode 100644 index 000000000..12f40620f --- /dev/null +++ b/internal/authorization/access_control_query_test.go @@ -0,0 +1,62 @@ +package authorization + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/authelia/authelia/v4/internal/configuration/schema" +) + +func TestNewAccessControlQuery(t *testing.T) { + testCases := []struct { + name string + have [][]schema.ACLQueryRule + expected []AccessControlQuery + matches [][]Object + }{ + { + "ShouldSkipInvalidTypeEqual", + [][]schema.ACLQueryRule{ + { + {Operator: operatorEqual, Key: "example", Value: 1}, + }, + }, + []AccessControlQuery{{Rules: []ObjectMatcher(nil)}}, + [][]Object{{{}}}, + }, + { + "ShouldSkipInvalidTypePattern", + [][]schema.ACLQueryRule{ + { + {Operator: operatorPattern, Key: "example", Value: 1}, + }, + }, + []AccessControlQuery{{Rules: []ObjectMatcher(nil)}}, + [][]Object{{{}}}, + }, + { + "ShouldSkipInvalidOperator", + [][]schema.ACLQueryRule{ + { + {Operator: "nop", Key: "example", Value: 1}, + }, + }, + []AccessControlQuery{{Rules: []ObjectMatcher(nil)}}, + [][]Object{{{}}}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + actual := NewAccessControlQuery(tc.have) + assert.Equal(t, tc.expected, actual) + + for i, rule := range actual { + for _, object := range tc.matches[i] { + assert.True(t, rule.IsMatch(object)) + } + } + }) + } +} diff --git a/internal/authorization/access_control_rule_test.go b/internal/authorization/access_control_rule_test.go new file mode 100644 index 000000000..68e7b54e7 --- /dev/null +++ b/internal/authorization/access_control_rule_test.go @@ -0,0 +1,33 @@ +package authorization + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestAccessControlRule_MatchesSubjectExact(t *testing.T) { + testCases := []struct { + name string + have *AccessControlRule + subject Subject + expected bool + }{ + { + "ShouldNotMatchAnonymous", + &AccessControlRule{ + Subjects: []AccessControlSubjects{ + {[]SubjectMatcher{schemaSubjectToACLSubject("user:john")}}, + }, + }, + Subject{}, + false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.expected, tc.have.MatchesSubjectExact(tc.subject)) + }) + } +} diff --git a/internal/authorization/regexp.go b/internal/authorization/regexp.go index f26c5fcbf..9c3c0d73b 100644 --- a/internal/authorization/regexp.go +++ b/internal/authorization/regexp.go @@ -16,7 +16,8 @@ type RegexpGroupStringSubjectMatcher struct { // IsMatch returns true if the underlying pattern matches the input given the subject. func (r RegexpGroupStringSubjectMatcher) IsMatch(input string, subject Subject) (match bool) { - if !r.Pattern.MatchString(input) { + matches := r.Pattern.FindStringSubmatch(input) + if matches == nil { return false } @@ -24,16 +25,11 @@ func (r RegexpGroupStringSubjectMatcher) IsMatch(input string, subject Subject) return true } - matches := r.Pattern.FindAllStringSubmatch(input, -1) - if matches == nil { + if r.SubexpNameUser != -1 && !strings.EqualFold(subject.Username, matches[r.SubexpNameUser]) { return false } - if r.SubexpNameUser != -1 && !strings.EqualFold(subject.Username, matches[0][r.SubexpNameUser]) { - return false - } - - if r.SubexpNameGroup != -1 && !utils.IsStringInSliceFold(matches[0][r.SubexpNameGroup], subject.Groups) { + if r.SubexpNameGroup != -1 && !utils.IsStringInSliceFold(matches[r.SubexpNameGroup], subject.Groups) { return false } diff --git a/internal/authorization/regexp_test.go b/internal/authorization/regexp_test.go new file mode 100644 index 000000000..a47567b0e --- /dev/null +++ b/internal/authorization/regexp_test.go @@ -0,0 +1,42 @@ +package authorization + +import ( + "regexp" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestRegexpGroupStringSubjectMatcher_IsMatch(t *testing.T) { + testCases := []struct { + name string + have *RegexpGroupStringSubjectMatcher + input string + subject Subject + expected bool + }{ + { + "Abc", + &RegexpGroupStringSubjectMatcher{ + MustCompileRegexNoPtr(`^(?P[a-zA-Z0-9]+)\.regex.com$`), + 1, + 0, + }, + "example.com", + Subject{Username: "a-user"}, + false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.expected, tc.have.IsMatch(tc.input, tc.subject)) + }) + } +} + +func MustCompileRegexNoPtr(input string) regexp.Regexp { + out := regexp.MustCompile(input) + + return *out +} diff --git a/internal/authorization/types_test.go b/internal/authorization/types_test.go index 00ced8aef..1c990cf63 100644 --- a/internal/authorization/types_test.go +++ b/internal/authorization/types_test.go @@ -85,3 +85,33 @@ func TestShouldCleanURL(t *testing.T) { }) } } + +func TestRuleMatchResult_IsPotentialMatch(t *testing.T) { + testCases := []struct { + name string + have RuleMatchResult + expected bool + }{ + { + "ShouldNotMatch", + RuleMatchResult{}, + false, + }, + { + "ShouldMatch", + RuleMatchResult{nil, true, true, true, true, true, true, true, false}, + true, + }, + { + "ShouldMatchExact", + RuleMatchResult{nil, true, true, true, true, true, true, true, true}, + false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.expected, tc.have.IsPotentialMatch()) + }) + } +} diff --git a/internal/authorization/util_test.go b/internal/authorization/util_test.go index ede91d086..4313d0ba6 100644 --- a/internal/authorization/util_test.go +++ b/internal/authorization/util_test.go @@ -2,6 +2,7 @@ package authorization import ( "net" + "regexp" "testing" "github.com/stretchr/testify/assert" @@ -217,3 +218,76 @@ func TestIsAuthLevelSufficient(t *testing.T) { assert.False(t, IsAuthLevelSufficient(authentication.OneFactor, TwoFactor)) assert.True(t, IsAuthLevelSufficient(authentication.TwoFactor, TwoFactor)) } + +func TestStringSliceToRegexpSlice(t *testing.T) { + testCases := []struct { + name string + have []string + expected []regexp.Regexp + err string + }{ + { + "ShouldNotParseBadRegex", + []string{`\q`}, + []regexp.Regexp(nil), + "error parsing regexp: invalid escape sequence: `\\q`", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + actual, theError := stringSliceToRegexpSlice(tc.have) + + assert.Equal(t, tc.expected, actual) + + if tc.err == "" { + assert.NoError(t, theError) + } else { + assert.EqualError(t, theError, tc.err) + } + }) + } +} + +func TestSchemaNetworksToACL(t *testing.T) { + testCases := []struct { + name string + have []string + globals map[string][]*net.IPNet + cache map[string]*net.IPNet + expected []*net.IPNet + }{ + { + "ShouldLoadFromCache", + []string{"192.168.0.0/24"}, + nil, + map[string]*net.IPNet{"192.168.0.0/24": MustParseCIDR("192.168.0.0/24")}, + []*net.IPNet{MustParseCIDR("192.168.0.0/24")}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + if tc.globals == nil { + tc.globals = map[string][]*net.IPNet{} + } + + if tc.cache == nil { + tc.cache = map[string]*net.IPNet{} + } + + actual := schemaNetworksToACL(tc.have, tc.globals, tc.cache) + + assert.Equal(t, tc.expected, actual) + }) + } +} + +func MustParseCIDR(input string) *net.IPNet { + _, out, err := net.ParseCIDR(input) + if err != nil { + panic(err) + } + + return out +}