fix(authorization): regex subj doesn't redirect anon user (#4037)

This fixes an issue with the authorization policies where if the Domain Regex or Resources criteria would incorrectly return 403 Forbidden statuses instead of 302 Found statuses.

Co-authored-by: Amir Zarrinkafsh <nightah@me.com>
pull/4081/head
James Elliott 2022-09-26 14:33:08 +10:00 committed by GitHub
parent d8f8f74dce
commit 8cdf4a5624
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 122 additions and 60 deletions

View File

@ -32,7 +32,7 @@ func NewAccessControlDomain(domain string) AccessControlDomain {
// NewAccessControlDomainRegex creates a new SubjectObjectMatcher that matches the domain either in a basic way or
// dynamic User/Group subexpression group way.
func NewAccessControlDomainRegex(pattern regexp.Regexp) AccessControlDomain {
func NewAccessControlDomainRegex(pattern regexp.Regexp) (subjects bool, rule AccessControlDomain) {
var iuser, igroup = -1, -1
for i, group := range pattern.SubexpNames() {
@ -45,10 +45,10 @@ func NewAccessControlDomainRegex(pattern regexp.Regexp) AccessControlDomain {
}
if iuser != -1 || igroup != -1 {
return AccessControlDomain{RegexpGroupStringSubjectMatcher{pattern, iuser, igroup}}
return true, AccessControlDomain{RegexpGroupStringSubjectMatcher{pattern, iuser, igroup}}
}
return AccessControlDomain{RegexpStringSubjectMatcher{pattern}}
return false, AccessControlDomain{RegexpStringSubjectMatcher{pattern}}
}
// AccessControlDomainMatcher is the basic domain matcher.

View File

@ -5,7 +5,7 @@ import (
)
// NewAccessControlResource creates a AccessControlResource or AccessControlResourceGroup.
func NewAccessControlResource(pattern regexp.Regexp) AccessControlResource {
func NewAccessControlResource(pattern regexp.Regexp) (subjects bool, rule AccessControlResource) {
var iuser, igroup = -1, -1
for i, group := range pattern.SubexpNames() {
@ -18,10 +18,10 @@ func NewAccessControlResource(pattern regexp.Regexp) AccessControlResource {
}
if iuser != -1 || igroup != -1 {
return AccessControlResource{RegexpGroupStringSubjectMatcher{pattern, iuser, igroup}}
return true, AccessControlResource{RegexpGroupStringSubjectMatcher{pattern, iuser, igroup}}
}
return AccessControlResource{RegexpStringSubjectMatcher{pattern}}
return false, AccessControlResource{RegexpStringSubjectMatcher{pattern}}
}
// AccessControlResource represents an ACL resource that matches without named groups.

View File

@ -20,19 +20,29 @@ func NewAccessControlRules(config schema.AccessControlConfiguration) (rules []*A
// NewAccessControlRule parses a schema ACL and generates an internal ACL.
func NewAccessControlRule(pos int, rule schema.ACLRule, networksMap map[string][]*net.IPNet, networksCacheMap map[string]*net.IPNet) *AccessControlRule {
return &AccessControlRule{
Position: pos,
Domains: schemaDomainsToACL(rule.Domains, rule.DomainsRegex),
Resources: schemaResourcesToACL(rule.Resources),
Methods: schemaMethodsToACL(rule.Methods),
Networks: schemaNetworksToACL(rule.Networks, networksMap, networksCacheMap),
Subjects: schemaSubjectsToACL(rule.Subjects),
Policy: StringToLevel(rule.Policy),
r := &AccessControlRule{
Position: pos,
Domains: schemaDomainsToACL(rule.Domains),
Methods: schemaMethodsToACL(rule.Methods),
Networks: schemaNetworksToACL(rule.Networks, networksMap, networksCacheMap),
Subjects: schemaSubjectsToACL(rule.Subjects),
Policy: StringToLevel(rule.Policy),
}
if len(r.Subjects) != 0 {
r.HasSubjects = true
}
ruleAddDomainRegex(rule.DomainsRegex, r)
ruleAddResources(rule.Resources, r)
return r
}
// AccessControlRule controls and represents an ACL internally.
type AccessControlRule struct {
HasSubjects bool
Position int
Domains []AccessControlDomain
Resources []AccessControlResource

View File

@ -1,6 +1,8 @@
package authorization
import (
"github.com/sirupsen/logrus"
"github.com/authelia/authelia/v4/internal/configuration/schema"
"github.com/authelia/authelia/v4/internal/logging"
)
@ -10,15 +12,17 @@ type Authorizer struct {
defaultPolicy Level
rules []*AccessControlRule
mfa bool
configuration *schema.Configuration
config *schema.Configuration
log *logrus.Logger
}
// NewAuthorizer create an instance of authorizer with a given access control configuration.
func NewAuthorizer(configuration *schema.Configuration) (authorizer *Authorizer) {
// NewAuthorizer create an instance of authorizer with a given access control config.
func NewAuthorizer(config *schema.Configuration) (authorizer *Authorizer) {
authorizer = &Authorizer{
defaultPolicy: StringToLevel(configuration.AccessControl.DefaultPolicy),
rules: NewAccessControlRules(configuration.AccessControl),
configuration: configuration,
defaultPolicy: StringToLevel(config.AccessControl.DefaultPolicy),
rules: NewAccessControlRules(config.AccessControl),
config: config,
log: logging.Logger(),
}
if authorizer.defaultPolicy == TwoFactor {
@ -35,8 +39,8 @@ func NewAuthorizer(configuration *schema.Configuration) (authorizer *Authorizer)
}
}
if authorizer.configuration.IdentityProviders.OIDC != nil {
for _, client := range authorizer.configuration.IdentityProviders.OIDC.Clients {
if authorizer.config.IdentityProviders.OIDC != nil {
for _, client := range authorizer.config.IdentityProviders.OIDC.Clients {
if client.Policy == twoFactor {
authorizer.mfa = true
@ -54,24 +58,21 @@ func (p Authorizer) IsSecondFactorEnabled() bool {
}
// GetRequiredLevel retrieve the required level of authorization to access the object.
func (p Authorizer) GetRequiredLevel(subject Subject, object Object) (bool, Level) {
logger := logging.Logger()
logger.Debugf("Check authorization of subject %s and object %s (method %s).",
func (p Authorizer) GetRequiredLevel(subject Subject, object Object) (hasSubjects bool, level Level) {
p.log.Debugf("Check authorization of subject %s and object %s (method %s).",
subject.String(), object.String(), object.Method)
for _, rule := range p.rules {
if rule.IsMatch(subject, object) {
logger.Tracef(traceFmtACLHitMiss, "HIT", rule.Position, subject.String(), object.String(), object.Method)
p.log.Tracef(traceFmtACLHitMiss, "HIT", rule.Position, subject, object, object.Method)
return len(rule.Subjects) > 0, rule.Policy
return rule.HasSubjects, rule.Policy
}
logger.Tracef(traceFmtACLHitMiss, "MISS", rule.Position, subject.String(), object.String(), object.Method)
p.log.Tracef(traceFmtACLHitMiss, "MISS", rule.Position, subject, object, object.Method)
}
logger.Debugf("No matching rule for subject %s and url %s... Applying default policy.",
subject.String(), object.String())
p.log.Debugf("No matching rule for subject %s and url %s (method %s) applying default policy", subject, object, object.Method)
return false, p.defaultPolicy
}

View File

@ -278,7 +278,7 @@ func (s *AuthorizerSuite) TestShouldCheckDomainMatching() {
s.Require().Len(tester.rules[0].Domains, 1)
s.Assert().Equal("public.example.com", tester.configuration.AccessControl.Rules[0].Domains[0])
s.Assert().Equal("public.example.com", tester.config.AccessControl.Rules[0].Domains[0])
ruleMatcher0, ok := tester.rules[0].Domains[0].Matcher.(*AccessControlDomainMatcher)
s.Require().True(ok)
@ -289,7 +289,7 @@ func (s *AuthorizerSuite) TestShouldCheckDomainMatching() {
s.Require().Len(tester.rules[1].Domains, 1)
s.Assert().Equal("one-factor.example.com", tester.configuration.AccessControl.Rules[1].Domains[0])
s.Assert().Equal("one-factor.example.com", tester.config.AccessControl.Rules[1].Domains[0])
ruleMatcher1, ok := tester.rules[1].Domains[0].Matcher.(*AccessControlDomainMatcher)
s.Require().True(ok)
@ -300,7 +300,7 @@ func (s *AuthorizerSuite) TestShouldCheckDomainMatching() {
s.Require().Len(tester.rules[2].Domains, 1)
s.Assert().Equal("two-factor.example.com", tester.configuration.AccessControl.Rules[2].Domains[0])
s.Assert().Equal("two-factor.example.com", tester.config.AccessControl.Rules[2].Domains[0])
ruleMatcher2, ok := tester.rules[2].Domains[0].Matcher.(*AccessControlDomainMatcher)
s.Require().True(ok)
@ -311,7 +311,7 @@ func (s *AuthorizerSuite) TestShouldCheckDomainMatching() {
s.Require().Len(tester.rules[3].Domains, 1)
s.Assert().Equal("*.example.com", tester.configuration.AccessControl.Rules[3].Domains[0])
s.Assert().Equal("*.example.com", tester.config.AccessControl.Rules[3].Domains[0])
ruleMatcher3, ok := tester.rules[3].Domains[0].Matcher.(*AccessControlDomainMatcher)
s.Require().True(ok)
@ -322,7 +322,7 @@ func (s *AuthorizerSuite) TestShouldCheckDomainMatching() {
s.Require().Len(tester.rules[4].Domains, 1)
s.Assert().Equal("*.example.com", tester.configuration.AccessControl.Rules[4].Domains[0])
s.Assert().Equal("*.example.com", tester.config.AccessControl.Rules[4].Domains[0])
ruleMatcher4, ok := tester.rules[4].Domains[0].Matcher.(*AccessControlDomainMatcher)
s.Require().True(ok)
@ -375,7 +375,7 @@ func (s *AuthorizerSuite) TestShouldCheckDomainRegexMatching() {
s.Require().Len(tester.rules[0].Domains, 1)
s.Assert().Equal("^.*\\.example.com$", tester.configuration.AccessControl.Rules[0].DomainsRegex[0].String())
s.Assert().Equal("^.*\\.example.com$", tester.config.AccessControl.Rules[0].DomainsRegex[0].String())
ruleMatcher0, ok := tester.rules[0].Domains[0].Matcher.(RegexpStringSubjectMatcher)
s.Require().True(ok)
@ -383,7 +383,7 @@ func (s *AuthorizerSuite) TestShouldCheckDomainRegexMatching() {
s.Require().Len(tester.rules[1].Domains, 1)
s.Assert().Equal("^.*\\.example2.com$", tester.configuration.AccessControl.Rules[1].DomainsRegex[0].String())
s.Assert().Equal("^.*\\.example2.com$", tester.config.AccessControl.Rules[1].DomainsRegex[0].String())
ruleMatcher1, ok := tester.rules[1].Domains[0].Matcher.(RegexpStringSubjectMatcher)
s.Require().True(ok)
@ -391,7 +391,7 @@ func (s *AuthorizerSuite) TestShouldCheckDomainRegexMatching() {
s.Require().Len(tester.rules[2].Domains, 1)
s.Assert().Equal("^(?P<User>[a-zA-Z0-9]+)\\.regex.com$", tester.configuration.AccessControl.Rules[2].DomainsRegex[0].String())
s.Assert().Equal("^(?P<User>[a-zA-Z0-9]+)\\.regex.com$", tester.config.AccessControl.Rules[2].DomainsRegex[0].String())
ruleMatcher2, ok := tester.rules[2].Domains[0].Matcher.(RegexpGroupStringSubjectMatcher)
s.Require().True(ok)
@ -399,7 +399,7 @@ func (s *AuthorizerSuite) TestShouldCheckDomainRegexMatching() {
s.Require().Len(tester.rules[3].Domains, 1)
s.Assert().Equal("^group-(?P<Group>[a-zA-Z0-9]+)\\.regex.com$", tester.configuration.AccessControl.Rules[3].DomainsRegex[0].String())
s.Assert().Equal("^group-(?P<Group>[a-zA-Z0-9]+)\\.regex.com$", tester.config.AccessControl.Rules[3].DomainsRegex[0].String())
ruleMatcher3, ok := tester.rules[3].Domains[0].Matcher.(RegexpGroupStringSubjectMatcher)
s.Require().True(ok)
@ -407,7 +407,7 @@ func (s *AuthorizerSuite) TestShouldCheckDomainRegexMatching() {
s.Require().Len(tester.rules[4].Domains, 1)
s.Assert().Equal("^.*\\.(one|two).com$", tester.configuration.AccessControl.Rules[4].DomainsRegex[0].String())
s.Assert().Equal("^.*\\.(one|two).com$", tester.config.AccessControl.Rules[4].DomainsRegex[0].String())
ruleMatcher4, ok := tester.rules[4].Domains[0].Matcher.(RegexpStringSubjectMatcher)
s.Require().True(ok)
@ -454,30 +454,30 @@ func (s *AuthorizerSuite) TestShouldCheckResourceSubjectMatching() {
// Accessing an invalid users Personal page.
tester.CheckAuthorizations(s.T(), John, "https://id.example.com/invaliduser/personal", "GET", Denied)
tester.CheckAuthorizations(s.T(), Bob, "https://id.example.com/invaliduser/personal", "GET", Denied)
tester.CheckAuthorizations(s.T(), AnonymousUser, "https://id.example.com/invaliduser/personal", "GET", Denied)
tester.CheckAuthorizations(s.T(), AnonymousUser, "https://id.example.com/invaliduser/personal", "GET", OneFactor)
// Accessing another users Personal page.
tester.CheckAuthorizations(s.T(), John, "https://id.example.com/bob/personal", "GET", Denied)
tester.CheckAuthorizations(s.T(), AnonymousUser, "https://id.example.com/bob/personal", "GET", Denied)
tester.CheckAuthorizations(s.T(), AnonymousUser, "https://id.example.com/bob/personal", "GET", OneFactor)
tester.CheckAuthorizations(s.T(), John, "https://id.example.com/Bob/personal", "GET", Denied)
tester.CheckAuthorizations(s.T(), AnonymousUser, "https://id.example.com/Bob/personal", "GET", Denied)
tester.CheckAuthorizations(s.T(), AnonymousUser, "https://id.example.com/Bob/personal", "GET", OneFactor)
tester.CheckAuthorizations(s.T(), Bob, "https://id.example.com/john/personal", "GET", Denied)
tester.CheckAuthorizations(s.T(), AnonymousUser, "https://id.example.com/john/personal", "GET", Denied)
tester.CheckAuthorizations(s.T(), AnonymousUser, "https://id.example.com/john/personal", "GET", OneFactor)
tester.CheckAuthorizations(s.T(), Bob, "https://id.example.com/John/personal", "GET", Denied)
tester.CheckAuthorizations(s.T(), AnonymousUser, "https://id.example.com/John/personal", "GET", Denied)
tester.CheckAuthorizations(s.T(), AnonymousUser, "https://id.example.com/John/personal", "GET", OneFactor)
// Accessing a Group page.
tester.CheckAuthorizations(s.T(), John, "https://id.example.com/dev/group", "GET", OneFactor)
tester.CheckAuthorizations(s.T(), John, "https://id.example.com/admins/group", "GET", OneFactor)
tester.CheckAuthorizations(s.T(), Bob, "https://id.example.com/dev/group", "GET", Denied)
tester.CheckAuthorizations(s.T(), Bob, "https://id.example.com/admins/group", "GET", Denied)
tester.CheckAuthorizations(s.T(), AnonymousUser, "https://id.example.com/dev/group", "GET", Denied)
tester.CheckAuthorizations(s.T(), AnonymousUser, "https://id.example.com/admins/group", "GET", Denied)
tester.CheckAuthorizations(s.T(), AnonymousUser, "https://id.example.com/dev/group", "GET", OneFactor)
tester.CheckAuthorizations(s.T(), AnonymousUser, "https://id.example.com/admins/group", "GET", OneFactor)
// Accessing an invalid group's Group page.
tester.CheckAuthorizations(s.T(), John, "https://id.example.com/invalidgroup/group", "GET", Denied)
tester.CheckAuthorizations(s.T(), Bob, "https://id.example.com/invalidgroup/group", "GET", Denied)
tester.CheckAuthorizations(s.T(), AnonymousUser, "https://id.example.com/invalidgroup/group", "GET", Denied)
tester.CheckAuthorizations(s.T(), AnonymousUser, "https://id.example.com/invalidgroup/group", "GET", OneFactor)
s.Require().Len(tester.rules, 3)

View File

@ -36,4 +36,4 @@ var (
IdentitySubexpNames = []string{subexpNameUser, subexpNameGroup}
)
const traceFmtACLHitMiss = "ACL %s Position %d for subject %s and object %s (Method %s)"
const traceFmtACLHitMiss = "ACL %s Position %d for subject %s and object %s (method %s)"

View File

@ -16,6 +16,14 @@ 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) {
return false
}
if subject.IsAnonymous() {
return true
}
matches := r.Pattern.FindAllStringSubmatch(input, -1)
if matches == nil {
return false

View File

@ -70,24 +70,36 @@ func schemaSubjectToACLSubject(subjectRule string) (subject SubjectMatcher) {
return nil
}
func schemaDomainsToACL(domainRules []string, domainRegexRules []regexp.Regexp) (domains []AccessControlDomain) {
func schemaDomainsToACL(domainRules []string) (domains []AccessControlDomain) {
for _, domainRule := range domainRules {
domains = append(domains, NewAccessControlDomain(domainRule))
}
for _, domainRegexRule := range domainRegexRules {
domains = append(domains, NewAccessControlDomainRegex(domainRegexRule))
}
return domains
}
func schemaResourcesToACL(resourceRules []regexp.Regexp) (resources []AccessControlResource) {
for _, resourceRule := range resourceRules {
resources = append(resources, NewAccessControlResource(resourceRule))
}
func ruleAddDomainRegex(exps []regexp.Regexp, rule *AccessControlRule) {
for _, exp := range exps {
subjects, r := NewAccessControlDomainRegex(exp)
return resources
rule.Domains = append(rule.Domains, r)
if !rule.HasSubjects && subjects {
rule.HasSubjects = true
}
}
}
func ruleAddResources(exps []regexp.Regexp, rule *AccessControlRule) {
for _, exp := range exps {
subjects, r := NewAccessControlResource(exp)
rule.Resources = append(rule.Resources, r)
if !rule.HasSubjects && subjects {
rule.HasSubjects = true
}
}
}
func schemaMethodsToACL(methodRules []string) (methods []string) {

View File

@ -429,6 +429,37 @@ func TestShouldVerifyWrongCredentialsInBasicAuth(t *testing.T) {
"https://test.example.com", actualStatus, expStatus)
}
func TestShouldRedirectWithGroups(t *testing.T) {
mock := mocks.NewMockAutheliaCtx(t)
defer mock.Close()
mock.Ctx.Providers.Authorizer = authorization.NewAuthorizer(&schema.Configuration{
AccessControl: schema.AccessControlConfiguration{
DefaultPolicy: "deny",
Rules: []schema.ACLRule{
{
Domains: []string{"app.example.com"},
Policy: "one_factor",
Resources: []regexp.Regexp{
*regexp.MustCompile(`^/code-(?P<User>\w+)([/?].*)?$`),
},
},
},
},
})
mock.Ctx.Request.Header.Set("Accept", "text/html; charset=utf-8")
mock.Ctx.Request.Header.Set(fasthttp.HeaderXForwardedProto, "https")
mock.Ctx.Request.Header.Set(fasthttp.HeaderXForwardedHost, "app.example.com")
mock.Ctx.Request.Header.Set("X-Forwarded-Uri", "/code-test/login")
mock.Ctx.Request.SetRequestURI("/?rd=https://auth.mydomain.com")
VerifyGET(verifyGetCfg)(mock.Ctx)
assert.Equal(t, fasthttp.StatusFound, mock.Ctx.Response.StatusCode())
}
func TestShouldVerifyFailingPasswordCheckingInBasicAuth(t *testing.T) {
mock := mocks.NewMockAutheliaCtx(t)
defer mock.Close()