From 52102eea8c7379e0d34d9025ea72bebdcf639673 Mon Sep 17 00:00:00 2001 From: James Elliott Date: Wed, 19 Oct 2022 14:09:22 +1100 Subject: [PATCH] feat(authorization): query parameter filtering (#3990) This allows for advanced filtering of the query parameters in ACL's. Closes #2708 --- cmd/authelia-gen/cmd_code.go | 18 ++- .../configuration/security/access-control.md | 66 ++++++++ .../openid-connect/synology-dsm/index.md | 2 +- .../en/reference/guides/rule-operators.md | 134 ++++++++++++++++ .../authorization/access_control_query.go | 119 +++++++++++++++ internal/authorization/access_control_rule.go | 76 +++++++--- internal/authorization/authorizer.go | 13 +- internal/authorization/authorizer_test.go | 123 +++++++++++++++ internal/authorization/const.go | 9 ++ internal/authorization/types.go | 10 +- internal/commands/crypto_hash.go | 2 +- .../configuration/schema/access_control.go | 22 ++- internal/configuration/schema/keys.go | 4 + .../configuration/validator/access_control.go | 60 ++++++++ .../validator/access_control_test.go | 143 ++++++++++++++++++ internal/configuration/validator/const.go | 27 +++- 16 files changed, 784 insertions(+), 44 deletions(-) create mode 100644 docs/content/en/reference/guides/rule-operators.md create mode 100644 internal/authorization/access_control_query.go diff --git a/cmd/authelia-gen/cmd_code.go b/cmd/authelia-gen/cmd_code.go index 54db167e0..2d3e931ad 100644 --- a/cmd/authelia-gen/cmd_code.go +++ b/cmd/authelia-gen/cmd_code.go @@ -201,9 +201,18 @@ func containsType(needle reflect.Type, haystack []reflect.Type) (contains bool) return false } +//nolint:gocyclo func readTags(prefix string, t reflect.Type) (tags []string) { tags = make([]string, 0) + if t.Kind() != reflect.Struct { + if t.Kind() == reflect.Slice { + tags = append(tags, readTags(getKeyNameFromTagAndPrefix(prefix, "", true), t.Elem())...) + } + + return + } + for i := 0; i < t.NumField(); i++ { field := t.Field(i) @@ -223,13 +232,16 @@ func readTags(prefix string, t reflect.Type) (tags []string) { continue } case reflect.Slice: - if field.Type.Elem().Kind() == reflect.Struct { + switch field.Type.Elem().Kind() { + case reflect.Struct: if !containsType(field.Type.Elem(), decodedTypes) { tags = append(tags, getKeyNameFromTagAndPrefix(prefix, tag, false)) tags = append(tags, readTags(getKeyNameFromTagAndPrefix(prefix, tag, true), field.Type.Elem())...) continue } + case reflect.Slice: + tags = append(tags, readTags(getKeyNameFromTagAndPrefix(prefix, tag, true), field.Type.Elem())...) } case reflect.Ptr: switch field.Type.Elem().Kind() { @@ -268,6 +280,10 @@ func getKeyNameFromTagAndPrefix(prefix, name string, slice bool) string { } if slice { + if name == "" { + return fmt.Sprintf("%s[]", prefix) + } + return fmt.Sprintf("%s.%s[]", prefix, nameParts[0]) } diff --git a/docs/content/en/configuration/security/access-control.md b/docs/content/en/configuration/security/access-control.md index 2ac337eed..4be5546a2 100644 --- a/docs/content/en/configuration/security/access-control.md +++ b/docs/content/en/configuration/security/access-control.md @@ -42,6 +42,17 @@ access_control: - HEAD resources: - '^/api.*' + query: + - - operator: 'present' + key: 'secure' + - operator: 'absent' + key: 'insecure' + - - operator: 'pattern' + key: 'token' + value: '^(abc123|zyx789)$' + - operator: 'not pattern' + key: 'random' + value: '^(1|2)$' ``` ## Options @@ -416,6 +427,61 @@ access_control: - '^/api([/?].*)?$' ``` +#### query + +{{< confkey type="list(list(object))" required="no" >}} + +The query criteria is an advanced criteria which can allow configuration of rules that match specific query argument +keys against various rules. It's recommended to use [resources](#resources) rules instead for basic needs. + +The format of this rule is unique in as much as it is a list of lists. The logic behind this format is to allow for both +`OR` and `AND` logic. The first level of the list defines the `OR` logic, and the second level defines the `AND` logic. +Additionally each level of these lists does not have to be explicitly defined. + +##### key + +{{< confkey type="string" required="yes" >}} + +The query argument key to check. + +##### value + +{{< confkey type="string" required="situational" >}} + +The value to match against. This is required unless the operator is `absent` or `present`. It's recommended this value +is always quoted as per the examples. + +##### operator + +{{< confkey type="string" required="situational" >}} + +The rule operator for this rule. Valid operators can be found in the +[Rule Operators](../../reference/guides/rule-operators.md#operators) reference guide. + +If [key](#key) and [value](#value) are specified this defaults to `equal`, otherwise if [key](#key) is specified it +defaults to `present`. + + +##### Examples + +```yaml +access_control: + rules: + - domain: app.example.com + policy: bypass + query: + - - operator: 'present' + key: 'secure' + - operator: 'absent' + key: 'insecure' + - - operator: 'pattern' + key: 'token' + value: '^(abc123|zyx789)$' + - operator: 'not pattern' + key: 'random' + value: '^(1|2)$' +``` + ## Policies The policy of the first matching rule in the configured list decides the policy applied to the request, if no rule diff --git a/docs/content/en/integration/openid-connect/synology-dsm/index.md b/docs/content/en/integration/openid-connect/synology-dsm/index.md index bf6d430a7..fdbde09f6 100644 --- a/docs/content/en/integration/openid-connect/synology-dsm/index.md +++ b/docs/content/en/integration/openid-connect/synology-dsm/index.md @@ -2,7 +2,7 @@ title: "Synology DSM" description: "Integrating Synology DSM with the Authelia OpenID Connect Provider." lead: "" -date: 2022-06-15T17:51:47+10:00 +date: 2022-10-18T21:22:13+11:00 draft: false images: [] menu: diff --git a/docs/content/en/reference/guides/rule-operators.md b/docs/content/en/reference/guides/rule-operators.md new file mode 100644 index 000000000..4c0a68086 --- /dev/null +++ b/docs/content/en/reference/guides/rule-operators.md @@ -0,0 +1,134 @@ +--- +title: "Access Control Rule Guide" +description: "A reference guide on access control rule operators" +lead: "This section contains a reference guide on access control rule operators." +date: 2022-09-09T15:44:23+10:00 +draft: false +images: [] +menu: + reference: + parent: "guides" +weight: 220 +toc: true +--- + +## Operators + +Rule operators are effectively words which alter the behaviour of particular access control rules. The following table +is a guide on their use. + +| Operator | Effect | +|:-------------:|:--------------------------------------------------------------:| +| `equal` | Matches when the item value is equal to the provided value | +| `not equal` | Matches when the item value is not equal to the provided value | +| `present` | Matches when the item is present with any value | +| `absent` | Matches when the item is not present at all | +| `pattern` | Matches when the item matches the regex pattern | +| `not pattern` | Matches when the item doesn't match the regex pattern | + + +## Multi-level Logical Criteria + +Criteria which is described as multi-level logical criteria indicates that it is a list of lists. The first level i.e. +the list least indented to the right will be referred to the `OR-list`, and the list most indented to the right will be +referred to the `AND-list`. + +The OR-list matches if any of the criteria from it's AND-list's matches; in other words, a *__logical OR__*. The +AND-list matches if all of it's criteria matches the given request; in other words, a *__logical AND__*. + +In addition to these rules, if the AND-list only needs one item, it can be represented without the second level. + +### Examples + +#### List of Lists + +The following examples show various abstract examples to express a rule that matches either c, or a AND b; +i.e `(a AND b) OR (c)`. In relation to access control rules all of these should be treated the same. This format should +not be used for the configuration item type `list(list(object))`, see [List of List Objects](#list-of-list-objects) +instead. + +##### Fully Expressed + +```yaml +rule: + - - 'a' + - 'b' + - - 'c' +``` + +##### Omitted Level + +```yaml +rule: + - - 'a' + - 'b' + - 'c' +``` + +##### Compact + +```yaml +rule: + - ['a', 'b'] + - ['c'] +``` + +##### Compact with Omitted Level + +```yaml +rule: + - ['a', 'b'] + - 'c' +``` + +##### Super Compact + +```yaml +rule: [['a', 'b'], ['c']] +``` + +#### List of List Objects + +The following examples show various abstract examples that mirror the above rules however the AND-list is a list of +objects where the key is named `value`. This format should only be used for the configuration item type +`list(list(object))`, see [List of Lists](#list-of-lists) if you're not looking for a `list(list(object))` + +##### Fully Expressed + +```yaml +rule: + - - value: 'a' + - value: 'b' + - - value: 'c' +``` + +##### Omitted Level + +```yaml +rule: + - - 'a' + - 'b' + - value: 'c' +``` + +##### Compact + +```yaml +rule: + - ['a', 'b'] + - ['c'] +``` + +##### Compact with Omitted Level + +```yaml +rule: + - ['a', 'b'] + - 'c' +``` + +##### Super Compact + +```yaml +rule: [['a', 'b'], ['c']] +``` diff --git a/internal/authorization/access_control_query.go b/internal/authorization/access_control_query.go new file mode 100644 index 000000000..e83b08736 --- /dev/null +++ b/internal/authorization/access_control_query.go @@ -0,0 +1,119 @@ +package authorization + +import ( + "fmt" + "regexp" + + "github.com/authelia/authelia/v4/internal/configuration/schema" +) + +// NewAccessControlQuery creates a new AccessControlQuery rule type. +func NewAccessControlQuery(config [][]schema.ACLQueryRule) (rules []AccessControlQuery) { + if len(config) == 0 { + return nil + } + + for i := 0; i < len(config); i++ { + var rule []ObjectMatcher + + for j := 0; j < len(config[i]); j++ { + subRule, err := NewAccessControlQueryObjectMatcher(config[i][j]) + if err != nil { + continue + } + + rule = append(rule, subRule) + } + + rules = append(rules, AccessControlQuery{Rules: rule}) + } + + return rules +} + +// AccessControlQuery represents an ACL query args rule. +type AccessControlQuery struct { + Rules []ObjectMatcher +} + +// IsMatch returns true if this rule matches the object. +func (acq AccessControlQuery) IsMatch(object Object) (isMatch bool) { + for _, rule := range acq.Rules { + if !rule.IsMatch(object) { + return false + } + } + + return true +} + +// NewAccessControlQueryObjectMatcher creates a new ObjectMatcher rule type from a schema.ACLQueryRule. +func NewAccessControlQueryObjectMatcher(rule schema.ACLQueryRule) (matcher ObjectMatcher, err error) { + switch rule.Operator { + case operatorPresent, operatorAbsent: + return &AccessControlQueryMatcherPresent{key: rule.Key, present: rule.Operator == operatorPresent}, nil + case operatorEqual, operatorNotEqual: + if value, ok := rule.Value.(string); ok { + return &AccessControlQueryMatcherEqual{key: rule.Key, value: value, equal: rule.Operator == operatorEqual}, nil + } else { + return nil, fmt.Errorf("rule value is not a string and is instead %T", rule.Value) + } + case operatorPattern, operatorNotPattern: + if pattern, ok := rule.Value.(*regexp.Regexp); ok { + return &AccessControlQueryMatcherPattern{key: rule.Key, pattern: pattern, match: rule.Operator == operatorPattern}, nil + } else { + return nil, fmt.Errorf("rule value is not a *regexp.Regexp and is instead %T", rule.Value) + } + default: + return nil, fmt.Errorf("invalid operator: %s", rule.Operator) + } +} + +// AccessControlQueryMatcherEqual is a rule type that checks the equality of a query parameter. +type AccessControlQueryMatcherEqual struct { + key, value string + equal bool +} + +// IsMatch returns true if this rule matches the object. +func (acl AccessControlQueryMatcherEqual) IsMatch(object Object) (isMatch bool) { + switch { + case acl.equal: + return object.URL.Query().Get(acl.key) == acl.value + default: + return object.URL.Query().Get(acl.key) != acl.value + } +} + +// AccessControlQueryMatcherPresent is a rule type that checks the presence of a query parameter. +type AccessControlQueryMatcherPresent struct { + key string + present bool +} + +// IsMatch returns true if this rule matches the object. +func (acl AccessControlQueryMatcherPresent) IsMatch(object Object) (isMatch bool) { + switch { + case acl.present: + return object.URL.Query().Has(acl.key) + default: + return !object.URL.Query().Has(acl.key) + } +} + +// AccessControlQueryMatcherPattern is a rule type that checks a query parameter against regex. +type AccessControlQueryMatcherPattern struct { + key string + pattern *regexp.Regexp + match bool +} + +// IsMatch returns true if this rule matches the object. +func (acl AccessControlQueryMatcherPattern) IsMatch(object Object) (isMatch bool) { + switch { + case acl.match: + return acl.pattern.MatchString(object.URL.Query().Get(acl.key)) + default: + return !acl.pattern.MatchString(object.URL.Query().Get(acl.key)) + } +} diff --git a/internal/authorization/access_control_rule.go b/internal/authorization/access_control_rule.go index f95189025..9d86b80cf 100644 --- a/internal/authorization/access_control_rule.go +++ b/internal/authorization/access_control_rule.go @@ -22,6 +22,7 @@ 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, + Query: NewAccessControlQuery(rule.Query), Methods: schemaMethodsToACL(rule.Methods), Networks: schemaNetworksToACL(rule.Networks, networksMap, networksCacheMap), Subjects: schemaSubjectsToACL(rule.Subjects), @@ -46,6 +47,7 @@ type AccessControlRule struct { Position int Domains []AccessControlDomain Resources []AccessControlResource + Query []AccessControlQuery Methods []string Networks []*net.IPNet Subjects []AccessControlSubjects @@ -54,37 +56,42 @@ type AccessControlRule struct { // IsMatch returns true if all elements of an AccessControlRule match the object and subject. func (acr *AccessControlRule) IsMatch(subject Subject, object Object) (match bool) { - if !isMatchForDomains(subject, object, acr) { + if !acr.MatchesDomains(subject, object) { return false } - if !isMatchForResources(subject, object, acr) { + if !acr.MatchesResources(subject, object) { return false } - if !isMatchForMethods(object, acr) { + if !acr.MatchesQuery(object) { return false } - if !isMatchForNetworks(subject, acr) { + if !acr.MatchesMethods(object) { return false } - if !isMatchForSubjects(subject, acr) { + if !acr.MatchesNetworks(subject) { + return false + } + + if !acr.MatchesSubjects(subject) { return false } return true } -func isMatchForDomains(subject Subject, object Object, acl *AccessControlRule) (match bool) { +// MatchesDomains returns true if the rule matches the domains. +func (acr *AccessControlRule) MatchesDomains(subject Subject, object Object) (matches bool) { // If there are no domains in this rule then the domain condition is a match. - if len(acl.Domains) == 0 { + if len(acr.Domains) == 0 { return true } // Iterate over the domains until we find a match (return true) or until we exit the loop (return false). - for _, domain := range acl.Domains { + for _, domain := range acr.Domains { if domain.IsMatch(subject, object) { return true } @@ -93,14 +100,15 @@ func isMatchForDomains(subject Subject, object Object, acl *AccessControlRule) ( return false } -func isMatchForResources(subject Subject, object Object, acl *AccessControlRule) (match bool) { +// MatchesResources returns true if the rule matches the resources. +func (acr *AccessControlRule) MatchesResources(subject Subject, object Object) (matches bool) { // If there are no resources in this rule then the resource condition is a match. - if len(acl.Resources) == 0 { + if len(acr.Resources) == 0 { return true } // Iterate over the resources until we find a match (return true) or until we exit the loop (return false). - for _, resource := range acl.Resources { + for _, resource := range acr.Resources { if resource.IsMatch(subject, object) { return true } @@ -109,23 +117,42 @@ func isMatchForResources(subject Subject, object Object, acl *AccessControlRule) return false } -func isMatchForMethods(object Object, acl *AccessControlRule) (match bool) { - // If there are no methods in this rule then the method condition is a match. - if len(acl.Methods) == 0 { +// MatchesQuery returns true if the rule matches the query arguments. +func (acr *AccessControlRule) MatchesQuery(object Object) (match bool) { + // If there are no query rules in this rule then the query condition is a match. + if len(acr.Query) == 0 { return true } - return utils.IsStringInSlice(object.Method, acl.Methods) + // Iterate over the queries until we find a match (return true) or until we exit the loop (return false). + for _, query := range acr.Query { + if query.IsMatch(object) { + return true + } + } + + return false } -func isMatchForNetworks(subject Subject, acl *AccessControlRule) (match bool) { +// MatchesMethods returns true if the rule matches the method. +func (acr *AccessControlRule) MatchesMethods(object Object) (match bool) { + // If there are no methods in this rule then the method condition is a match. + if len(acr.Methods) == 0 { + return true + } + + return utils.IsStringInSlice(object.Method, acr.Methods) +} + +// MatchesNetworks returns true if the rule matches the networks. +func (acr *AccessControlRule) MatchesNetworks(subject Subject) (match bool) { // If there are no networks in this rule then the network condition is a match. - if len(acl.Networks) == 0 { + if len(acr.Networks) == 0 { return true } // Iterate over the networks until we find a match (return true) or until we exit the loop (return false). - for _, network := range acl.Networks { + for _, network := range acr.Networks { if network.Contains(subject.IP) { return true } @@ -134,25 +161,26 @@ func isMatchForNetworks(subject Subject, acl *AccessControlRule) (match bool) { return false } -// Same as isExactMatchForSubjects except it theoretically matches if subject is anonymous since they'd need to authenticate. -func isMatchForSubjects(subject Subject, acl *AccessControlRule) (match bool) { +// MatchesSubjects returns true if the rule matches the subjects. +func (acr *AccessControlRule) MatchesSubjects(subject Subject) (match bool) { if subject.IsAnonymous() { return true } - return isExactMatchForSubjects(subject, acl) + return acr.MatchesSubjectExact(subject) } -func isExactMatchForSubjects(subject Subject, acl *AccessControlRule) (match bool) { +// MatchesSubjectExact returns true if the rule matches the subjects exactly. +func (acr *AccessControlRule) MatchesSubjectExact(subject Subject) (match bool) { // If there are no subjects in this rule then the subject condition is a match. - if len(acl.Subjects) == 0 { + if len(acr.Subjects) == 0 { return true } else if subject.IsAnonymous() { return false } // Iterate over the subjects until we find a match (return true) or until we exit the loop (return false). - for _, subjectRule := range acl.Subjects { + for _, subjectRule := range acr.Subjects { if subjectRule.IsMatch(subject) { return true } diff --git a/internal/authorization/authorizer.go b/internal/authorization/authorizer.go index 6ce4dbbbc..a3546083f 100644 --- a/internal/authorization/authorizer.go +++ b/internal/authorization/authorizer.go @@ -88,12 +88,13 @@ func (p Authorizer) GetRuleMatchResults(subject Subject, object Object) (results Rule: rule, Skipped: skipped, - MatchDomain: isMatchForDomains(subject, object, rule), - MatchResources: isMatchForResources(subject, object, rule), - MatchMethods: isMatchForMethods(object, rule), - MatchNetworks: isMatchForNetworks(subject, rule), - MatchSubjects: isMatchForSubjects(subject, rule), - MatchSubjectsExact: isExactMatchForSubjects(subject, rule), + MatchDomain: rule.MatchesDomains(subject, object), + MatchResources: rule.MatchesResources(subject, object), + MatchQuery: rule.MatchesQuery(object), + MatchMethods: rule.MatchesMethods(object), + MatchNetworks: rule.MatchesNetworks(subject), + MatchSubjects: rule.MatchesSubjects(subject), + MatchSubjectsExact: rule.MatchesSubjectExact(subject), } skipped = skipped || results[i].IsMatch() diff --git a/internal/authorization/authorizer_test.go b/internal/authorization/authorizer_test.go index c1047a637..495a40f9a 100644 --- a/internal/authorization/authorizer_test.go +++ b/internal/authorization/authorizer_test.go @@ -208,6 +208,129 @@ func (s *AuthorizerSuite) TestShouldCheckFactorsPolicy() { tester.CheckAuthorizations(s.T(), UserWithGroups, "https://example.com/", "GET", Denied) } +func (s *AuthorizerSuite) TestShouldCheckQueryPolicy() { + tester := NewAuthorizerBuilder(). + WithDefaultPolicy(deny). + WithRule(schema.ACLRule{ + Domains: []string{"one.example.com"}, + Query: [][]schema.ACLQueryRule{ + { + { + Operator: operatorEqual, + Key: "test", + Value: "two", + }, + { + Operator: operatorAbsent, + Key: "admin", + }, + }, + { + { + Operator: operatorPresent, + Key: "public", + }, + }, + }, + Policy: oneFactor, + }). + WithRule(schema.ACLRule{ + Domains: []string{"two.example.com"}, + Query: [][]schema.ACLQueryRule{ + { + { + Operator: operatorEqual, + Key: "test", + Value: "one", + }, + }, + { + { + Operator: operatorEqual, + Key: "test", + Value: "two", + }, + }, + }, + Policy: twoFactor, + }). + WithRule(schema.ACLRule{ + Domains: []string{"three.example.com"}, + Query: [][]schema.ACLQueryRule{ + { + { + Operator: operatorNotEqual, + Key: "test", + Value: "one", + }, + { + Operator: operatorNotEqual, + Key: "test", + Value: "two", + }, + }, + }, + Policy: twoFactor, + }). + WithRule(schema.ACLRule{ + Domains: []string{"four.example.com"}, + Query: [][]schema.ACLQueryRule{ + { + { + Operator: operatorPattern, + Key: "test", + Value: regexp.MustCompile(`^(one|two|three)$`), + }, + }, + }, + Policy: twoFactor, + }). + WithRule(schema.ACLRule{ + Domains: []string{"five.example.com"}, + Query: [][]schema.ACLQueryRule{ + { + { + Operator: operatorNotPattern, + Key: "test", + Value: regexp.MustCompile(`^(one|two|three)$`), + }, + }, + }, + Policy: twoFactor, + }). + Build() + + testCases := []struct { + name, requestURL string + expected Level + }{ + {"ShouldDenyAbsentRule", "https://one.example.com/?admin=true", Denied}, + {"ShouldAllow1FAPresentRule", "https://one.example.com/?public=true", OneFactor}, + {"ShouldAllow1FAEqualRule", "https://one.example.com/?test=two", OneFactor}, + {"ShouldDenyAbsentRuleWithMatchingPresentRule", "https://one.example.com/?test=two&admin=true", Denied}, + {"ShouldAllow2FARuleWithOneMatchingEqual", "https://two.example.com/?test=one&admin=true", TwoFactor}, + {"ShouldAllow2FARuleWithAnotherMatchingEqual", "https://two.example.com/?test=two&admin=true", TwoFactor}, + {"ShouldDenyRuleWithNotMatchingEqual", "https://two.example.com/?test=three&admin=true", Denied}, + {"ShouldDenyRuleWithNotMatchingNotEqualAND1", "https://three.example.com/?test=one", Denied}, + {"ShouldDenyRuleWithNotMatchingNotEqualAND2", "https://three.example.com/?test=two", Denied}, + {"ShouldAllowRuleWithMatchingNotEqualAND", "https://three.example.com/?test=three", TwoFactor}, + {"ShouldAllowRuleWithMatchingPatternOne", "https://four.example.com/?test=one", TwoFactor}, + {"ShouldAllowRuleWithMatchingPatternTwo", "https://four.example.com/?test=two", TwoFactor}, + {"ShouldAllowRuleWithMatchingPatternThree", "https://four.example.com/?test=three", TwoFactor}, + {"ShouldDenyRuleWithNotMatchingPattern", "https://four.example.com/?test=five", Denied}, + {"ShouldAllowRuleWithMatchingNotPattern", "https://five.example.com/?test=five", TwoFactor}, + {"ShouldDenyRuleWithNotMatchingNotPatternOne", "https://five.example.com/?test=one", Denied}, + {"ShouldDenyRuleWithNotMatchingNotPatternTwo", "https://five.example.com/?test=two", Denied}, + {"ShouldDenyRuleWithNotMatchingNotPatternThree", "https://five.example.com/?test=three", Denied}, + } + + for _, tc := range testCases { + s.T().Run(tc.name, func(t *testing.T) { + tester.CheckAuthorizations(t, UserWithGroups, tc.requestURL, "GET", tc.expected) + }) + } +} + func (s *AuthorizerSuite) TestShouldCheckRulePrecedence() { tester := NewAuthorizerBuilder(). WithDefaultPolicy(deny). diff --git a/internal/authorization/const.go b/internal/authorization/const.go index dcf991563..afda76888 100644 --- a/internal/authorization/const.go +++ b/internal/authorization/const.go @@ -26,6 +26,15 @@ const ( deny = "deny" ) +const ( + operatorPresent = "present" + operatorAbsent = "absent" + operatorEqual = "equal" + operatorNotEqual = "not equal" + operatorPattern = "pattern" + operatorNotPattern = "not pattern" +) + const ( subexpNameUser = "User" subexpNameGroup = "Group" diff --git a/internal/authorization/types.go b/internal/authorization/types.go index d02a1081a..4e26a2db3 100644 --- a/internal/authorization/types.go +++ b/internal/authorization/types.go @@ -24,6 +24,11 @@ type SubjectObjectMatcher interface { IsMatch(subject Subject, object Object) (match bool) } +// ObjectMatcher is a matcher that takes an object. +type ObjectMatcher interface { + IsMatch(object Object) (match bool) +} + // Subject represents the identity of a user for the purposes of ACL matching. type Subject struct { Username string @@ -43,7 +48,7 @@ func (s Subject) IsAnonymous() bool { // Object represents a protected object for the purposes of ACL matching. type Object struct { - URL url.URL + URL *url.URL Domain string Path string @@ -63,7 +68,7 @@ func NewObjectRaw(targetURL *url.URL, method []byte) (object Object) { // NewObject creates a new Object type from a URL and a method header. func NewObject(targetURL *url.URL, method string) (object Object) { return Object{ - URL: *targetURL, + URL: targetURL, Domain: targetURL.Hostname(), Path: utils.URLPathFullClean(targetURL), Method: method, @@ -78,6 +83,7 @@ type RuleMatchResult struct { MatchDomain bool MatchResources bool + MatchQuery bool MatchMethods bool MatchNetworks bool MatchSubjects bool diff --git a/internal/commands/crypto_hash.go b/internal/commands/crypto_hash.go index 72eb8f7d9..5d80a4aac 100644 --- a/internal/commands/crypto_hash.go +++ b/internal/commands/crypto_hash.go @@ -480,7 +480,7 @@ func cmdCryptoHashGetPassword(cmd *cobra.Command, args []string, useArgs, useRan func hashReadPasswordWithPrompt(prompt string) (data []byte, err error) { fmt.Print(prompt) - if data, err = term.ReadPassword(int(syscall.Stdin)); err != nil { //nolint:unconvert // Conversion required. + if data, err = term.ReadPassword(int(syscall.Stdin)); err != nil { //nolint:unconvert,nolintlint if err.Error() == "inappropriate ioctl for device" { return nil, fmt.Errorf("the terminal doesn't appear to be interactive either use the '--password' flag or use an interactive terminal: %w", err) } diff --git a/internal/configuration/schema/access_control.go b/internal/configuration/schema/access_control.go index 7b12fe129..79e1c93fa 100644 --- a/internal/configuration/schema/access_control.go +++ b/internal/configuration/schema/access_control.go @@ -19,13 +19,21 @@ type ACLNetwork struct { // ACLRule represents one ACL rule entry. type ACLRule struct { - Domains []string `koanf:"domain"` - DomainsRegex []regexp.Regexp `koanf:"domain_regex"` - Policy string `koanf:"policy"` - Subjects [][]string `koanf:"subject"` - Networks []string `koanf:"networks"` - Resources []regexp.Regexp `koanf:"resources"` - Methods []string `koanf:"methods"` + Domains []string `koanf:"domain"` + DomainsRegex []regexp.Regexp `koanf:"domain_regex"` + Policy string `koanf:"policy"` + Subjects [][]string `koanf:"subject"` + Networks []string `koanf:"networks"` + Resources []regexp.Regexp `koanf:"resources"` + Methods []string `koanf:"methods"` + Query [][]ACLQueryRule `koanf:"query"` +} + +// ACLQueryRule represents the ACL query criteria. +type ACLQueryRule struct { + Operator string `koanf:"operator"` + Key string `koanf:"key"` + Value any `koanf:"value"` } // DefaultACLNetwork represents the default configuration related to access control network group configuration. diff --git a/internal/configuration/schema/keys.go b/internal/configuration/schema/keys.go index ddb262c40..ffcf2840f 100644 --- a/internal/configuration/schema/keys.go +++ b/internal/configuration/schema/keys.go @@ -148,6 +148,10 @@ var Keys = []string{ "access_control.rules[].networks", "access_control.rules[].resources", "access_control.rules[].methods", + "access_control.rules[].query[][].operator", + "access_control.rules[].query[][].key", + "access_control.rules[].query[][].value", + "access_control.rules[].query", "ntp.address", "ntp.version", "ntp.max_desync", diff --git a/internal/configuration/validator/access_control.go b/internal/configuration/validator/access_control.go index 3bd475cb2..9509c4cab 100644 --- a/internal/configuration/validator/access_control.go +++ b/internal/configuration/validator/access_control.go @@ -3,6 +3,7 @@ package validator import ( "fmt" "net" + "regexp" "strings" "github.com/authelia/authelia/v4/internal/authorization" @@ -103,6 +104,8 @@ func ValidateRules(config *schema.Configuration, validator *schema.StructValidat validateMethods(rulePosition, rule, validator) + validateQuery(i, rule, config, validator) + if rule.Policy == policyBypass { validateBypass(rulePosition, rule, validator) } @@ -149,3 +152,60 @@ func validateMethods(rulePosition int, rule schema.ACLRule, validator *schema.St } } } + +//nolint:gocyclo +func validateQuery(i int, rule schema.ACLRule, config *schema.Configuration, validator *schema.StructValidator) { + for j := 0; j < len(config.AccessControl.Rules[i].Query); j++ { + for k := 0; k < len(config.AccessControl.Rules[i].Query[j]); k++ { + if config.AccessControl.Rules[i].Query[j][k].Operator == "" { + if config.AccessControl.Rules[i].Query[j][k].Key != "" { + switch config.AccessControl.Rules[i].Query[j][k].Value { + case "", nil: + config.AccessControl.Rules[i].Query[j][k].Operator = operatorPresent + default: + config.AccessControl.Rules[i].Query[j][k].Operator = operatorEqual + } + } + } else if !utils.IsStringInSliceFold(config.AccessControl.Rules[i].Query[j][k].Operator, validACLRuleOperators) { + validator.Push(fmt.Errorf(errFmtAccessControlRuleQueryInvalid, ruleDescriptor(i+1, rule), config.AccessControl.Rules[i].Query[j][k].Operator, strings.Join(validACLRuleOperators, "', '"))) + } + + if config.AccessControl.Rules[i].Query[j][k].Key == "" { + validator.Push(fmt.Errorf(errFmtAccessControlRuleQueryInvalidNoValue, ruleDescriptor(i+1, rule), "key")) + } + + op := config.AccessControl.Rules[i].Query[j][k].Operator + + if op == "" { + continue + } + + switch v := config.AccessControl.Rules[i].Query[j][k].Value.(type) { + case nil: + if op != operatorAbsent && op != operatorPresent { + validator.Push(fmt.Errorf(errFmtAccessControlRuleQueryInvalidNoValueOperator, ruleDescriptor(i+1, rule), "value", op)) + } + case string: + switch op { + case operatorPresent, operatorAbsent: + if v != "" { + validator.Push(fmt.Errorf(errFmtAccessControlRuleQueryInvalidValue, ruleDescriptor(i+1, rule), "value", op)) + } + case operatorPattern, operatorNotPattern: + var ( + pattern *regexp.Regexp + err error + ) + + if pattern, err = regexp.Compile(v); err != nil { + validator.Push(fmt.Errorf(errFmtAccessControlRuleQueryInvalidValueParse, ruleDescriptor(i+1, rule), "value", err)) + } else { + config.AccessControl.Rules[i].Query[j][k].Value = pattern + } + } + default: + validator.Push(fmt.Errorf(errFmtAccessControlRuleQueryInvalidValueType, ruleDescriptor(i+1, rule), v)) + } + } + } +} diff --git a/internal/configuration/validator/access_control_test.go b/internal/configuration/validator/access_control_test.go index 42ac82258..7bf24d52f 100644 --- a/internal/configuration/validator/access_control_test.go +++ b/internal/configuration/validator/access_control_test.go @@ -201,6 +201,149 @@ func (suite *AccessControl) TestShouldRaiseErrorInvalidSubject() { suite.Assert().EqualError(suite.validator.Errors()[1], fmt.Sprintf(errAccessControlRuleBypassPolicyInvalidWithSubjects, ruleDescriptor(1, suite.config.AccessControl.Rules[0]))) } +func (suite *AccessControl) TestShouldSetQueryDefaults() { + domains := []string{"public.example.com"} + suite.config.AccessControl.Rules = []schema.ACLRule{ + { + Domains: domains, + Policy: "bypass", + Query: [][]schema.ACLQueryRule{ + { + {Operator: "", Key: "example"}, + }, + { + {Operator: "", Key: "example", Value: "test"}, + }, + }, + }, + { + Domains: domains, + Policy: "bypass", + Query: [][]schema.ACLQueryRule{ + { + {Operator: "pattern", Key: "a", Value: "^(x|y|z)$"}, + }, + }, + }, + } + + ValidateRules(suite.config, suite.validator) + + suite.Assert().Len(suite.validator.Warnings(), 0) + suite.Assert().Len(suite.validator.Errors(), 0) + + suite.Assert().Equal("present", suite.config.AccessControl.Rules[0].Query[0][0].Operator) + suite.Assert().Equal("equal", suite.config.AccessControl.Rules[0].Query[1][0].Operator) + + suite.Require().Len(suite.config.AccessControl.Rules, 2) + suite.Require().Len(suite.config.AccessControl.Rules[1].Query, 1) + suite.Require().Len(suite.config.AccessControl.Rules[1].Query[0], 1) + + t := ®exp.Regexp{} + + suite.Assert().IsType(t, suite.config.AccessControl.Rules[1].Query[0][0].Value) +} + +func (suite *AccessControl) TestShouldErrorOnInvalidRulesQuery() { + domains := []string{"public.example.com"} + suite.config.AccessControl.Rules = []schema.ACLRule{ + { + Domains: domains, + Policy: "bypass", + Query: [][]schema.ACLQueryRule{ + { + {Operator: "equal", Key: "example"}, + }, + }, + }, + { + Domains: domains, + Policy: "bypass", + Query: [][]schema.ACLQueryRule{ + { + {Operator: "present"}, + }, + }, + }, + { + Domains: domains, + Policy: "bypass", + Query: [][]schema.ACLQueryRule{ + { + {Operator: "present", Key: "a"}, + }, + }, + }, + { + Domains: domains, + Policy: "bypass", + Query: [][]schema.ACLQueryRule{ + { + {Operator: "absent", Key: "a"}, + }, + }, + }, + { + Domains: domains, + Policy: "bypass", + Query: [][]schema.ACLQueryRule{ + { + {}, + }, + }, + }, + { + Domains: domains, + Policy: "bypass", + Query: [][]schema.ACLQueryRule{ + { + {Operator: "not", Key: "a", Value: "a"}, + }, + }, + }, + { + Domains: domains, + Policy: "bypass", + Query: [][]schema.ACLQueryRule{ + { + {Operator: "pattern", Key: "a", Value: "(bad pattern"}, + }, + }, + }, + { + Domains: domains, + Policy: "bypass", + Query: [][]schema.ACLQueryRule{ + { + {Operator: "present", Key: "a", Value: "not good"}, + }, + }, + }, + { + Domains: domains, + Policy: "bypass", + Query: [][]schema.ACLQueryRule{ + { + {Operator: "present", Key: "a", Value: 5}, + }, + }, + }, + } + + ValidateRules(suite.config, suite.validator) + + suite.Assert().Len(suite.validator.Warnings(), 0) + suite.Require().Len(suite.validator.Errors(), 7) + + suite.Assert().EqualError(suite.validator.Errors()[0], "access control: rule #1 (domain 'public.example.com'): 'query' option 'value' is invalid: must have a value when the operator is 'equal'") + suite.Assert().EqualError(suite.validator.Errors()[1], "access control: rule #2 (domain 'public.example.com'): 'query' option 'key' is invalid: must have a value") + suite.Assert().EqualError(suite.validator.Errors()[2], "access control: rule #5 (domain 'public.example.com'): 'query' option 'key' is invalid: must have a value") + suite.Assert().EqualError(suite.validator.Errors()[3], "access control: rule #6 (domain 'public.example.com'): 'query' option 'operator' with value 'not' is invalid: must be one of 'present', 'absent', 'equal', 'not equal', 'pattern', 'not pattern'") + suite.Assert().EqualError(suite.validator.Errors()[4], "access control: rule #7 (domain 'public.example.com'): 'query' option 'value' is invalid: error parsing regexp: missing closing ): `(bad pattern`") + suite.Assert().EqualError(suite.validator.Errors()[5], "access control: rule #8 (domain 'public.example.com'): 'query' option 'value' is invalid: must not have a value when the operator is 'present'") + suite.Assert().EqualError(suite.validator.Errors()[6], "access control: rule #9 (domain 'public.example.com'): 'query' option 'value' is invalid: expected type was string but got int") +} + func TestAccessControl(t *testing.T) { suite.Run(t, new(AccessControl)) } diff --git a/internal/configuration/validator/const.go b/internal/configuration/validator/const.go index 448e02c23..adc19f36f 100644 --- a/internal/configuration/validator/const.go +++ b/internal/configuration/validator/const.go @@ -211,6 +211,18 @@ const ( "invalid: must start with 'user:' or 'group:'" errFmtAccessControlRuleMethodInvalid = "access control: rule %s: 'methods' option '%s' is " + "invalid: must be one of '%s'" + errFmtAccessControlRuleQueryInvalid = "access control: rule %s: 'query' option 'operator' with value '%s' is " + + "invalid: must be one of '%s'" + errFmtAccessControlRuleQueryInvalidNoValue = "access control: rule %s: 'query' option '%s' is " + + "invalid: must have a value" + errFmtAccessControlRuleQueryInvalidNoValueOperator = "access control: rule %s: 'query' option '%s' is " + + "invalid: must have a value when the operator is '%s'" + errFmtAccessControlRuleQueryInvalidValue = "access control: rule %s: 'query' option '%s' is " + + "invalid: must not have a value when the operator is '%s'" + errFmtAccessControlRuleQueryInvalidValueParse = "access control: rule %s: 'query' option '%s' is " + + "invalid: %w" + errFmtAccessControlRuleQueryInvalidValueType = "access control: rule %s: 'query' option 'value' is " + + "invalid: expected type was string but got %T" ) // Theme Error constants. @@ -317,9 +329,20 @@ var validRFC7231HTTPMethodVerbs = []string{"GET", "HEAD", "POST", "PUT", "PATCH" var validRFC4918HTTPMethodVerbs = []string{"COPY", "LOCK", "MKCOL", "MOVE", "PROPFIND", "PROPPATCH", "UNLOCK"} -var validACLHTTPMethodVerbs = append(validRFC7231HTTPMethodVerbs, validRFC4918HTTPMethodVerbs...) +const ( + operatorPresent = "present" + operatorAbsent = "absent" + operatorEqual = "equal" + operatorNotEqual = "not equal" + operatorPattern = "pattern" + operatorNotPattern = "not pattern" +) -var validACLRulePolicies = []string{policyBypass, policyOneFactor, policyTwoFactor, policyDeny} +var ( + validACLHTTPMethodVerbs = append(validRFC7231HTTPMethodVerbs, validRFC4918HTTPMethodVerbs...) + validACLRulePolicies = []string{policyBypass, policyOneFactor, policyTwoFactor, policyDeny} + validACLRuleOperators = []string{operatorPresent, operatorAbsent, operatorEqual, operatorNotEqual, operatorPattern, operatorNotPattern} +) var validDefault2FAMethods = []string{"totp", "webauthn", "mobile_push"}