From 951dc71325794ed1a7e2d2c5ce2b9a5ce7a6864b Mon Sep 17 00:00:00 2001 From: Dustin Sweigart Date: Wed, 15 Apr 2020 20:18:11 -0400 Subject: [PATCH] [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 | 10 ++- docs/configuration/access-control.md | 19 ++-- internal/authorization/authorizer.go | 20 +++-- internal/authorization/authorizer_test.go | 87 +++++++++++++------ internal/configuration/reader_test.go | 2 +- .../configuration/schema/access_control.go | 14 +-- .../configuration/test_resources/config.yml | 9 +- .../handler_extended_configuration_test.go | 36 ++++---- internal/handlers/handler_firstfactor_test.go | 12 +-- internal/handlers/handler_verify_test.go | 4 +- internal/mocks/mock_authelia_ctx.go | 16 ++-- 11 files changed, 146 insertions(+), 83 deletions(-) diff --git a/config.template.yml b/config.template.yml index fe787065d..ca5718360 100644 --- a/config.template.yml +++ b/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 diff --git a/docs/configuration/access-control.md b/docs/configuration/access-control.md index a0bb97d84..465b4b1bc 100644 --- a/docs/configuration/access-control.md +++ b/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 @@ -129,4 +130,4 @@ access_control: - "^/users/john/.*$" subject: "user:john" policy: two_factor -``` \ No newline at end of file +``` diff --git a/internal/authorization/authorizer.go b/internal/authorization/authorizer.go index 030d81e4d..8212f80ec 100644 --- a/internal/authorization/authorizer.go +++ b/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 diff --git a/internal/authorization/authorizer_test.go b/internal/authorization/authorizer_test.go index 8ea0d0165..38f787c02 100644 --- a/internal/authorization/authorizer_test.go +++ b/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]+$"}, }). diff --git a/internal/configuration/reader_test.go b/internal/configuration/reader_test.go index 92e0ce9ec..b21c8802d 100644 --- a/internal/configuration/reader_test.go +++ b/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) } diff --git a/internal/configuration/schema/access_control.go b/internal/configuration/schema/access_control.go index e497ecefd..a2253794a 100644 --- a/internal/configuration/schema/access_control.go +++ b/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 { diff --git a/internal/configuration/test_resources/config.yml b/internal/configuration/test_resources/config.yml index c4fcc35d6..bec861529 100644 --- a/internal/configuration/test_resources/config.yml +++ b/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: diff --git a/internal/handlers/handler_extended_configuration_test.go b/internal/handlers/handler_extended_configuration_test.go index 1065899b8..e109a6f31 100644 --- a/internal/handlers/handler_extended_configuration_test.go +++ b/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", }, }, }) diff --git a/internal/handlers/handler_firstfactor_test.go b/internal/handlers/handler_firstfactor_test.go index 7d8901716..22e51a2bc 100644 --- a/internal/handlers/handler_firstfactor_test.go +++ b/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", }, }, }) diff --git a/internal/handlers/handler_verify_test.go b/internal/handlers/handler_verify_test.go index 1fed911a7..3c86258cf 100644 --- a/internal/handlers/handler_verify_test.go +++ b/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, }}, }) diff --git a/internal/mocks/mock_authelia_ctx.go b/internal/mocks/mock_authelia_ctx.go index f481ff430..947611c57 100644 --- a/internal/mocks/mock_authelia_ctx.go +++ b/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{}