[FEATURE] Support multiple domains and multiple subjects in ACLs (#869)

* added support for listing multiple domains and multiple subjects

* updated documentation to show use of multiple domains and subjects

* updated config.template.yml to display multiple domains as a list

* updated config.template.yml to display multiple subjects as a list

* updated docs/configuration/access-control.md to display multiple domains as a list

* updated docs/configuration/access-control.md to display multiple subjects as a list

* removed redundant check that always returned true

* Commentary definition for `weak`

config.template.yml

@@ -212,7 +212,10 @@ access_control:
       # Network based rule, if not provided any network matches.
       networks:
         - 192.168.1.0/24
-    - domain: secure.example.com
+
+    - domain:
+      - secure.example.com
+      - private.example.com
       policy: two_factor
 
     - domain: singlefactor.example.com
@@ -222,8 +225,11 @@ access_control:
     - domain: "mx2.mail.example.com"
       subject: "group:admins"
       policy: deny
+
     - domain: "*.example.com"
-      subject: "group:admins"
+      subject:
+        - "group:admins"
+        - "group:moderators"
       policy: two_factor
 
     # Rules applied to 'dev' group

docs/configuration/access-control.md

@@ -54,7 +54,7 @@ The domains defined in rules must obviously be either a subdomain of the domain
 protected by Authelia or the protected domain itself. In order to match multiple
 subdomains, the wildcard matcher character `*.` can be used as prefix of the domain.
 For instance, to define a rule for all subdomains of *example.com*, one would use
-`*.example.com` in the rule.
+`*.example.com` in the rule. A single rule can define multiple domains for matching.
 
 ## Resources
 
@@ -67,11 +67,8 @@ any one of them matches, the resource criteria of the rule matches.
 A subject is a representation of a user or a group of user for who the rule should apply.
 
 For a user with unique identifier `john`, the subject should be `user:john` and for a group
-uniquely identified by `developers`, the subject should be `group:developers`. Unlike resources
-there can be only one subject per rule. However, if multiple users or group must be matched by
-a rule, one can just duplicate the rule as many times as there are subjects.
-
-*Note: Any PR to make it a list instead of a single item is welcome.*
+uniquely identified by `developers`, the subject should be `group:developers`. Similar to resources
+and domains you can define multiple subjects in a single rule.
 
 ## Networks
 
@@ -104,7 +101,9 @@ access_control:
       networks:
       - 192.168.1.0/24
 
-    - domain: secure.example.com
+    - domain:
+      - secure.example.com
+      - private.example.com
       policy: two_factor
 
     - domain: singlefactor.example.com
@@ -115,7 +114,9 @@ access_control:
       policy: deny
 
     - domain: "*.example.com"
-      subject: "group:admins"
+      subject:
+        - "group:admins"
+        - "group:moderators"
       policy: two_factor
 
     - domain: dev.example.com

internal/authorization/authorizer.go

@@ -47,8 +47,16 @@ func selectMatchingSubjectRules(rules []schema.ACLRule, subject Subject) []schem
 	selectedRules := []schema.ACLRule{}
 
 	for _, rule := range rules {
-		if isSubjectMatching(subject, rule.Subject) && isIPMatching(subject.IP, rule.Networks) {
-			selectedRules = append(selectedRules, rule)
+		if len(rule.Subjects) > 0 {
+			for _, subjectRule := range rule.Subjects {
+				if isSubjectMatching(subject, subjectRule) && isIPMatching(subject.IP, rule.Networks) {
+					selectedRules = append(selectedRules, rule)
+				}
+			}
+		} else {
+			if isIPMatching(subject.IP, rule.Networks) {
+				selectedRules = append(selectedRules, rule)
+			}
 		}
 	}
 
@@ -59,9 +67,11 @@ func selectMatchingObjectRules(rules []schema.ACLRule, object Object) []schema.A
 	selectedRules := []schema.ACLRule{}
 
 	for _, rule := range rules {
-		if isDomainMatching(object.Domain, rule.Domain) &&
-			isPathMatching(object.Path, rule.Resources) {
-			selectedRules = append(selectedRules, rule)
+		for _, domain := range rule.Domains {
+			if isDomainMatching(object.Domain, domain) &&
+				isPathMatching(object.Path, rule.Resources) {
+				selectedRules = append(selectedRules, rule)
+			}
 		}
 	}
 	return selectedRules

internal/authorization/authorizer_test.go

@@ -105,8 +105,8 @@ func (s *AuthorizerSuite) TestShouldCheckMultiDomainRule() {
 	tester := NewAuthorizerBuilder().
 		WithDefaultPolicy("deny").
 		WithRule(schema.ACLRule{
-			Domain: "*.example.com",
-			Policy: "bypass",
+			Domains: []string{"*.example.com"},
+			Policy:  "bypass",
 		}).
 		Build()
 
@@ -118,20 +118,40 @@ func (s *AuthorizerSuite) TestShouldCheckMultiDomainRule() {
 	tester.CheckAuthorizations(s.T(), UserWithGroups, "https://public.example.co/", Denied)
 }
 
+func (s *AuthorizerSuite) TestShouldCheckMultipleDomainRule() {
+	tester := NewAuthorizerBuilder().
+		WithDefaultPolicy("deny").
+		WithRule(schema.ACLRule{
+			Domains: []string{"*.example.com", "other.com"},
+			Policy:  "bypass",
+		}).
+		Build()
+
+	tester.CheckAuthorizations(s.T(), UserWithGroups, "https://public.example.com/", Bypass)
+	tester.CheckAuthorizations(s.T(), UserWithGroups, "https://private.example.com/", Bypass)
+	tester.CheckAuthorizations(s.T(), UserWithGroups, "https://public.example.com/elsewhere", Bypass)
+	tester.CheckAuthorizations(s.T(), UserWithGroups, "https://example.com/", Denied)
+	tester.CheckAuthorizations(s.T(), UserWithGroups, "https://public.example.com.c/", Denied)
+	tester.CheckAuthorizations(s.T(), UserWithGroups, "https://public.example.co/", Denied)
+	tester.CheckAuthorizations(s.T(), UserWithGroups, "https://other.com/", Bypass)
+	tester.CheckAuthorizations(s.T(), UserWithGroups, "https://other.com/elsewhere", Bypass)
+	tester.CheckAuthorizations(s.T(), UserWithGroups, "https://private.other.com/", Denied)
+}
+
 func (s *AuthorizerSuite) TestShouldCheckFactorsPolicy() {
 	tester := NewAuthorizerBuilder().
 		WithDefaultPolicy("deny").
 		WithRule(schema.ACLRule{
-			Domain: "single.example.com",
-			Policy: "one_factor",
+			Domains: []string{"single.example.com"},
+			Policy:  "one_factor",
 		}).
 		WithRule(schema.ACLRule{
-			Domain: "protected.example.com",
-			Policy: "two_factor",
+			Domains: []string{"protected.example.com"},
+			Policy:  "two_factor",
 		}).
 		WithRule(schema.ACLRule{
-			Domain: "public.example.com",
-			Policy: "bypass",
+			Domains: []string{"public.example.com"},
+			Policy:  "bypass",
 		}).
 		Build()
 
@@ -145,17 +165,17 @@ func (s *AuthorizerSuite) TestShouldCheckRulePrecedence() {
 	tester := NewAuthorizerBuilder().
 		WithDefaultPolicy("deny").
 		WithRule(schema.ACLRule{
-			Domain:  "protected.example.com",
-			Policy:  "bypass",
-			Subject: "user:john",
+			Domains:  []string{"protected.example.com"},
+			Policy:   "bypass",
+			Subjects: []string{"user:john"},
 		}).
 		WithRule(schema.ACLRule{
-			Domain: "protected.example.com",
-			Policy: "one_factor",
+			Domains: []string{"protected.example.com"},
+			Policy:  "one_factor",
 		}).
 		WithRule(schema.ACLRule{
-			Domain: "*.example.com",
-			Policy: "two_factor",
+			Domains: []string{"*.example.com"},
+			Policy:  "two_factor",
 		}).
 		Build()
 
@@ -168,9 +188,9 @@ func (s *AuthorizerSuite) TestShouldCheckUserMatching() {
 	tester := NewAuthorizerBuilder().
 		WithDefaultPolicy("deny").
 		WithRule(schema.ACLRule{
-			Domain:  "protected.example.com",
-			Policy:  "bypass",
-			Subject: "user:john",
+			Domains:  []string{"protected.example.com"},
+			Policy:   "bypass",
+			Subjects: []string{"user:john"},
 		}).
 		Build()
 
@@ -182,9 +202,9 @@ func (s *AuthorizerSuite) TestShouldCheckGroupMatching() {
 	tester := NewAuthorizerBuilder().
 		WithDefaultPolicy("deny").
 		WithRule(schema.ACLRule{
-			Domain:  "protected.example.com",
-			Policy:  "bypass",
-			Subject: "group:admins",
+			Domains:  []string{"protected.example.com"},
+			Policy:   "bypass",
+			Subjects: []string{"group:admins"},
 		}).
 		Build()
 
@@ -192,21 +212,36 @@ func (s *AuthorizerSuite) TestShouldCheckGroupMatching() {
 	tester.CheckAuthorizations(s.T(), Bob, "https://protected.example.com/", Denied)
 }
 
+func (s *AuthorizerSuite) TestShouldCheckSubjectsMatching() {
+	tester := NewAuthorizerBuilder().
+		WithDefaultPolicy("deny").
+		WithRule(schema.ACLRule{
+			Domains:  []string{"protected.example.com"},
+			Policy:   "bypass",
+			Subjects: []string{"group:admins", "user:bob"},
+		}).
+		Build()
+
+	tester.CheckAuthorizations(s.T(), John, "https://protected.example.com/", Bypass)
+	tester.CheckAuthorizations(s.T(), Bob, "https://protected.example.com/", Bypass)
+	tester.CheckAuthorizations(s.T(), AnonymousUser, "https://protected.example.com/", Denied)
+}
+
 func (s *AuthorizerSuite) TestShouldCheckIPMatching() {
 	tester := NewAuthorizerBuilder().
 		WithDefaultPolicy("deny").
 		WithRule(schema.ACLRule{
-			Domain:   "protected.example.com",
+			Domains:  []string{"protected.example.com"},
 			Policy:   "bypass",
 			Networks: []string{"192.168.1.8", "10.0.0.8"},
 		}).
 		WithRule(schema.ACLRule{
-			Domain:   "protected.example.com",
+			Domains:  []string{"protected.example.com"},
 			Policy:   "one_factor",
 			Networks: []string{"10.0.0.7"},
 		}).
 		WithRule(schema.ACLRule{
-			Domain:   "net.example.com",
+			Domains:  []string{"net.example.com"},
 			Policy:   "two_factor",
 			Networks: []string{"10.0.0.0/8"},
 		}).
@@ -225,12 +260,12 @@ func (s *AuthorizerSuite) TestShouldCheckResourceMatching() {
 	tester := NewAuthorizerBuilder().
 		WithDefaultPolicy("deny").
 		WithRule(schema.ACLRule{
-			Domain:    "resource.example.com",
+			Domains:   []string{"resource.example.com"},
 			Policy:    "bypass",
 			Resources: []string{"^/bypass/[a-z]+$", "^/$", "embedded"},
 		}).
 		WithRule(schema.ACLRule{
-			Domain:    "resource.example.com",
+			Domains:   []string{"resource.example.com"},
 			Policy:    "one_factor",
 			Resources: []string{"^/one_factor/[a-z]+$"},
 		}).

internal/configuration/reader_test.go

@@ -40,5 +40,5 @@ func TestShouldParseConfigFile(t *testing.T) {
 	assert.Equal(t, "postgres_secret_from_env", config.Storage.PostgreSQL.Password)
 
 	assert.Equal(t, "deny", config.AccessControl.DefaultPolicy)
-	assert.Len(t, config.AccessControl.Rules, 11)
+	assert.Len(t, config.AccessControl.Rules, 12)
 }

internal/configuration/schema/access_control.go

@@ -7,10 +7,11 @@ import (
 )
 
 // ACLRule represent one ACL rule
+// "weak" coerces a single value into string slice
 type ACLRule struct {
-	Domain    string   `mapstructure:"domain"`
+	Domains   []string `mapstructure:"domain,weak"`
 	Policy    string   `mapstructure:"policy"`
-	Subject   string   `mapstructure:"subject"`
+	Subjects  []string `mapstructure:"subject,weak"`
 	Networks  []string `mapstructure:"networks"`
 	Resources []string `mapstructure:"resources"`
 }
@@ -33,7 +34,8 @@ func IsNetworkValid(network string) bool {
 
 // Validate validate an ACL Rule
 func (r *ACLRule) Validate(validator *StructValidator) {
-	if r.Domain == "" {
+
+	if len(r.Domains) == 0 {
 		validator.Push(fmt.Errorf("Domain must be provided"))
 	}
 
@@ -41,8 +43,10 @@ func (r *ACLRule) Validate(validator *StructValidator) {
 		validator.Push(fmt.Errorf("A policy must either be 'deny', 'two_factor', 'one_factor' or 'bypass'"))
 	}
 
-	if !IsSubjectValid(r.Subject) {
-		validator.Push(fmt.Errorf("A subject must start with 'user:' or 'group:'"))
+	for i, subject := range r.Subjects {
+		if !IsSubjectValid(subject) {
+			validator.Push(fmt.Errorf("Subject %d must start with 'user:' or 'group:'", i))
+		}
 	}
 
 	for i, network := range r.Networks {

internal/configuration/test_resources/config.yml

@@ -44,7 +44,7 @@ access_control:
     - domain: secure.example.com
       policy: two_factor
 
-    - domain: singlefactor.example.com
+    - domain: [singlefactor.example.com, onefactor.example.com]
       policy: one_factor
 
     # Rules applied to 'admins' group
@@ -69,6 +69,13 @@ access_control:
       subject: "user:john"
       policy: two_factor
 
+    # Rules applied to 'dev' group and user 'john'
+    - domain: dev.example.com
+      resources:
+        - "^/deny-all.*$"
+      subject: ["group:dev", "user:john"]
+      policy: denied
+
     # Rules applied to user 'harry'
     - domain: dev.example.com
       resources:

internal/handlers/handler_extended_configuration_test.go

@@ -68,16 +68,16 @@ func (s *SecondFactorAvailableMethodsFixture) TestShouldCheckSecondFactorIsDisab
 		DefaultPolicy: "bypass",
 		Rules: []schema.ACLRule{
 			{
-				Domain: "example.com",
-				Policy: "deny",
+				Domains: []string{"example.com"},
+				Policy:  "deny",
 			},
 			{
-				Domain: "abc.example.com",
-				Policy: "single_factor",
+				Domains: []string{"abc.example.com"},
+				Policy:  "single_factor",
 			},
 			{
-				Domain: "def.example.com",
-				Policy: "bypass",
+				Domains: []string{"def.example.com"},
+				Policy:  "bypass",
 			},
 		},
 	})
@@ -99,16 +99,16 @@ func (s *SecondFactorAvailableMethodsFixture) TestShouldCheckSecondFactorIsEnabl
 		DefaultPolicy: "two_factor",
 		Rules: []schema.ACLRule{
 			{
-				Domain: "example.com",
-				Policy: "deny",
+				Domains: []string{"example.com"},
+				Policy:  "deny",
 			},
 			{
-				Domain: "abc.example.com",
-				Policy: "single_factor",
+				Domains: []string{"abc.example.com"},
+				Policy:  "single_factor",
 			},
 			{
-				Domain: "def.example.com",
-				Policy: "bypass",
+				Domains: []string{"def.example.com"},
+				Policy:  "bypass",
 			},
 		},
 	})
@@ -130,16 +130,16 @@ func (s *SecondFactorAvailableMethodsFixture) TestShouldCheckSecondFactorIsEnabl
 		DefaultPolicy: "bypass",
 		Rules: []schema.ACLRule{
 			{
-				Domain: "example.com",
-				Policy: "deny",
+				Domains: []string{"example.com"},
+				Policy:  "deny",
 			},
 			{
-				Domain: "abc.example.com",
-				Policy: "two_factor",
+				Domains: []string{"abc.example.com"},
+				Policy:  "two_factor",
 			},
 			{
-				Domain: "def.example.com",
-				Policy: "bypass",
+				Domains: []string{"def.example.com"},
+				Policy:  "bypass",
 			},
 		},
 	})

internal/handlers/handler_firstfactor_test.go

@@ -281,8 +281,8 @@ func (s *FirstFactorRedirectionSuite) SetupTest() {
 	s.mock.Ctx.Configuration.AccessControl.DefaultPolicy = "bypass"
 	s.mock.Ctx.Configuration.AccessControl.Rules = []schema.ACLRule{
 		{
-			Domain: "default.local",
-			Policy: "one_factor",
+			Domains: []string{"default.local"},
+			Policy:  "one_factor",
 		},
 	}
 	s.mock.Ctx.Providers.Authorizer = authorization.NewAuthorizer(
@@ -377,12 +377,12 @@ func (s *FirstFactorRedirectionSuite) TestShouldReply200WhenUnsafeTargetURLProvi
 		DefaultPolicy: "one_factor",
 		Rules: []schema.ACLRule{
 			{
-				Domain: "test.example.com",
-				Policy: "one_factor",
+				Domains: []string{"test.example.com"},
+				Policy:  "one_factor",
 			},
 			{
-				Domain: "example.com",
-				Policy: "two_factor",
+				Domains: []string{"example.com"},
+				Policy:  "two_factor",
 			},
 		},
 	})

internal/handlers/handler_verify_test.go

@@ -170,8 +170,8 @@ func TestShouldCheckAuthorizationMatching(t *testing.T) {
 		authorizer := authorization.NewAuthorizer(schema.AccessControlConfiguration{
 			DefaultPolicy: "deny",
 			Rules: []schema.ACLRule{{
-				Domain: "test.example.com",
-				Policy: rule.Policy,
+				Domains: []string{"test.example.com"},
+				Policy:  rule.Policy,
 			}},
 		})
 

internal/mocks/mock_authelia_ctx.go

@@ -73,17 +73,17 @@ func NewMockAutheliaCtx(t *testing.T) *MockAutheliaCtx {
 	configuration.Session.Name = "authelia_session"
 	configuration.AccessControl.DefaultPolicy = "deny"
 	configuration.AccessControl.Rules = []schema.ACLRule{{
-		Domain: "bypass.example.com",
-		Policy: "bypass",
+		Domains: []string{"bypass.example.com"},
+		Policy:  "bypass",
 	}, {
-		Domain: "one-factor.example.com",
-		Policy: "one_factor",
+		Domains: []string{"one-factor.example.com"},
+		Policy:  "one_factor",
 	}, {
-		Domain: "two-factor.example.com",
-		Policy: "two_factor",
+		Domains: []string{"two-factor.example.com"},
+		Policy:  "two_factor",
 	}, {
-		Domain: "deny.example.com",
-		Policy: "deny",
+		Domains: []string{"deny.example.com"},
+		Policy:  "deny",
 	}}
 
 	providers := middlewares.Providers{}