From a4edf2132038500d1346ff59dbb72ddf567e16f6 Mon Sep 17 00:00:00 2001 From: James Elliott Date: Tue, 18 Oct 2022 19:14:34 +1100 Subject: [PATCH] fix(authorization): subject wildcard domain rule not matching (#4187) This fixes an issue where the subject wildcard domain rules (those containing {user} and {group}) are not considered matches even though they may be once a user authenticates. Fixes #4186 --- .../authorization/access_control_domain.go | 24 ++++++++++++------- internal/authorization/access_control_rule.go | 2 +- internal/authorization/authorizer_test.go | 10 ++++---- internal/authorization/util.go | 12 ++++++---- 4 files changed, 30 insertions(+), 18 deletions(-) diff --git a/internal/authorization/access_control_domain.go b/internal/authorization/access_control_domain.go index d19bdf3dd..463e3293d 100644 --- a/internal/authorization/access_control_domain.go +++ b/internal/authorization/access_control_domain.go @@ -9,7 +9,7 @@ import ( ) // NewAccessControlDomain creates a new SubjectObjectMatcher that matches the domain as a basic string. -func NewAccessControlDomain(domain string) AccessControlDomain { +func NewAccessControlDomain(domain string) (subjcets bool, rule AccessControlDomain) { m := &AccessControlDomainMatcher{} domain = strings.ToLower(domain) @@ -19,15 +19,15 @@ func NewAccessControlDomain(domain string) AccessControlDomain { m.Name = domain[1:] case strings.HasPrefix(domain, "{user}"): m.UserWildcard = true - m.Name = domain[7:] + m.Name = domain[6:] case strings.HasPrefix(domain, "{group}"): m.GroupWildcard = true - m.Name = domain[8:] + m.Name = domain[7:] default: m.Name = domain } - return AccessControlDomain{m} + return m.UserWildcard || m.GroupWildcard, AccessControlDomain{m} } // NewAccessControlDomainRegex creates a new SubjectObjectMatcher that matches the domain either in a basic way or @@ -65,11 +65,19 @@ func (m AccessControlDomainMatcher) IsMatch(domain string, subject Subject) (mat case m.Wildcard: return strings.HasSuffix(domain, m.Name) case m.UserWildcard: - return domain == fmt.Sprintf("%s.%s", subject.Username, m.Name) - case m.GroupWildcard: - prefix, suffix := domainToPrefixSuffix(domain) + if subject.IsAnonymous() && strings.HasSuffix(domain, m.Name) { + return true + } - return suffix == m.Name && utils.IsStringInSliceFold(prefix, subject.Groups) + return domain == fmt.Sprintf("%s%s", subject.Username, m.Name) + case m.GroupWildcard: + if subject.IsAnonymous() && strings.HasSuffix(domain, m.Name) { + return true + } + + i := strings.Index(domain, ".") + + return domain[i:] == m.Name && utils.IsStringInSliceFold(domain[:i], subject.Groups) default: return strings.EqualFold(domain, m.Name) } diff --git a/internal/authorization/access_control_rule.go b/internal/authorization/access_control_rule.go index 7b2989e8a..f95189025 100644 --- a/internal/authorization/access_control_rule.go +++ b/internal/authorization/access_control_rule.go @@ -22,7 +22,6 @@ func NewAccessControlRules(config schema.AccessControlConfiguration) (rules []*A func NewAccessControlRule(pos int, rule schema.ACLRule, networksMap map[string][]*net.IPNet, networksCacheMap map[string]*net.IPNet) *AccessControlRule { r := &AccessControlRule{ Position: pos, - Domains: schemaDomainsToACL(rule.Domains), Methods: schemaMethodsToACL(rule.Methods), Networks: schemaNetworksToACL(rule.Networks, networksMap, networksCacheMap), Subjects: schemaSubjectsToACL(rule.Subjects), @@ -33,6 +32,7 @@ func NewAccessControlRule(pos int, rule schema.ACLRule, networksMap map[string][ r.HasSubjects = true } + ruleAddDomain(rule.Domains, r) ruleAddDomainRegex(rule.DomainsRegex, r) ruleAddResources(rule.Resources, r) diff --git a/internal/authorization/authorizer_test.go b/internal/authorization/authorizer_test.go index ff303f9d5..c1047a637 100644 --- a/internal/authorization/authorizer_test.go +++ b/internal/authorization/authorizer_test.go @@ -151,17 +151,17 @@ func (s *AuthorizerSuite) TestShouldCheckDynamicDomainRules() { WithDefaultPolicy(deny). WithRule(schema.ACLRule{ Domains: []string{"{user}.example.com"}, - Policy: bypass, + Policy: oneFactor, }). WithRule(schema.ACLRule{ Domains: []string{"{group}.example.com"}, - Policy: bypass, + Policy: oneFactor, }). Build() - tester.CheckAuthorizations(s.T(), UserWithGroups, "https://john.example.com/", "GET", Bypass) - tester.CheckAuthorizations(s.T(), UserWithGroups, "https://dev.example.com/", "GET", Bypass) - tester.CheckAuthorizations(s.T(), UserWithGroups, "https://admins.example.com/", "GET", Bypass) + tester.CheckAuthorizations(s.T(), UserWithGroups, "https://john.example.com/", "GET", OneFactor) + tester.CheckAuthorizations(s.T(), UserWithGroups, "https://dev.example.com/", "GET", OneFactor) + tester.CheckAuthorizations(s.T(), UserWithGroups, "https://admins.example.com/", "GET", OneFactor) tester.CheckAuthorizations(s.T(), UserWithGroups, "https://othergroup.example.com/", "GET", Denied) } diff --git a/internal/authorization/util.go b/internal/authorization/util.go index cfcd1a5f2..50be55e48 100644 --- a/internal/authorization/util.go +++ b/internal/authorization/util.go @@ -70,12 +70,16 @@ func schemaSubjectToACLSubject(subjectRule string) (subject SubjectMatcher) { return nil } -func schemaDomainsToACL(domainRules []string) (domains []AccessControlDomain) { +func ruleAddDomain(domainRules []string, rule *AccessControlRule) { for _, domainRule := range domainRules { - domains = append(domains, NewAccessControlDomain(domainRule)) - } + subjects, r := NewAccessControlDomain(domainRule) - return domains + rule.Domains = append(rule.Domains, r) + + if !rule.HasSubjects && subjects { + rule.HasSubjects = true + } + } } func ruleAddDomainRegex(exps []regexp.Regexp, rule *AccessControlRule) {