From 4dce8f94962d3bd0099bbb202f76696a551d099b Mon Sep 17 00:00:00 2001 From: James Elliott Date: Fri, 5 Mar 2021 15:18:31 +1100 Subject: [PATCH] perf(authorizer): preload access control lists (#1640) * adjust session refresh to always occur (for disabled users) * feat: adds filtering option for Request Method in ACL's * simplify flow of internal/authorization/authorizer.go's methods * implement query string checking * utilize authorizer.Object fully * make matchers uniform * add tests * add missing request methods * add frontend enhancements to handle request method * add request method to 1FA Handler Suite * add internal ACL representations (preparsing) * expand on access_control next * add docs * remove unnecessary slice for network names and instead just use a plain string * add warning for ineffectual bypass policy (due to subjects) * add user/group wildcard support * fix(authorization): allow subject rules to match anonymous users * feat(api): add new params * docs(api): wording adjustments * test: add request method into testing and proxy docs * test: add several checks and refactor schema validation for ACL * test: add integration test for methods acl * refactor: apply suggestions from code review * docs(authorization): update description --- api/openapi.yml | 53 ++- docs/configuration/access-control.md | 118 +++++-- docs/deployment/supported-proxies/haproxy.md | 27 +- docs/deployment/supported-proxies/nginx.md | 6 +- .../authorization/access_control_domain.go | 32 ++ .../authorization/access_control_resource.go | 15 + internal/authorization/access_control_rule.go | 139 ++++++++ .../authorization/access_control_subjects.go | 55 ++++ internal/authorization/authorizer.go | 111 +------ internal/authorization/authorizer_test.go | 309 ++++++++++++++---- internal/authorization/const.go | 3 + internal/authorization/domain_matcher.go | 15 - internal/authorization/domain_matcher_test.go | 36 -- internal/authorization/ip_matcher.go | 66 ---- internal/authorization/ip_matcher_test.go | 38 --- internal/authorization/path_matcher.go | 19 -- internal/authorization/path_matcher_test.go | 24 -- internal/authorization/subject_matcher.go | 29 -- internal/authorization/types.go | 60 ++++ internal/authorization/types_test.go | 22 ++ internal/authorization/util.go | 177 ++++++++++ internal/authorization/util_test.go | 184 +++++++++++ .../configuration/schema/access_control.go | 7 +- .../configuration/validator/access_control.go | 74 +++-- .../validator/access_control_test.go | 35 +- .../configuration/validator/authentication.go | 4 - .../validator/authentication_test.go | 12 + internal/configuration/validator/const.go | 5 + .../validator/regulation_test.go | 14 + internal/handlers/handler_firstfactor.go | 2 +- internal/handlers/handler_firstfactor_test.go | 6 + internal/handlers/handler_verify.go | 49 ++- internal/handlers/handler_verify_test.go | 5 +- internal/handlers/response.go | 14 +- internal/handlers/types.go | 13 +- internal/middlewares/authelia_context.go | 13 +- internal/middlewares/const.go | 1 + internal/suites/Standalone/configuration.yml | 5 + .../example/compose/haproxy/haproxy.cfg | 10 + .../example/compose/nginx/portal/nginx.conf | 2 +- internal/suites/suite_standalone_test.go | 25 ++ internal/utils/certificates.go | 2 +- internal/utils/strings.go | 17 +- internal/utils/strings_test.go | 52 ++- web/src/hooks/RequestMethod.ts | 8 + web/src/services/FirstFactor.ts | 14 +- .../FirstFactor/FirstFactorForm.tsx | 4 +- web/src/views/LoginPortal/LoginPortal.tsx | 8 +- 48 files changed, 1420 insertions(+), 519 deletions(-) create mode 100644 internal/authorization/access_control_domain.go create mode 100644 internal/authorization/access_control_resource.go create mode 100644 internal/authorization/access_control_rule.go create mode 100644 internal/authorization/access_control_subjects.go delete mode 100644 internal/authorization/domain_matcher.go delete mode 100644 internal/authorization/domain_matcher_test.go delete mode 100644 internal/authorization/ip_matcher.go delete mode 100644 internal/authorization/ip_matcher_test.go delete mode 100644 internal/authorization/path_matcher.go delete mode 100644 internal/authorization/path_matcher_test.go delete mode 100644 internal/authorization/subject_matcher.go create mode 100644 internal/authorization/types.go create mode 100644 internal/authorization/types_test.go create mode 100644 internal/authorization/util.go create mode 100644 internal/authorization/util_test.go create mode 100644 web/src/hooks/RequestMethod.ts diff --git a/api/openapi.yml b/api/openapi.yml index 646f5dd6..dfc05e02 100644 --- a/api/openapi.yml +++ b/api/openapi.yml @@ -73,14 +73,9 @@ paths: summary: Verification description: The verify endpoint provides the ability to verify if a user has the necessary permissions to access a specified domain. parameters: - - name: X-Original-URL - in: header - description: Redirection URL - required: true - style: simple - explode: true - schema: - type: string + - $ref: '#/components/parameters/originalURLParam' + - $ref: '#/components/parameters/forwardedMethodParam' + - $ref: '#/components/parameters/authParam' responses: "200": description: Successful Operation @@ -115,14 +110,9 @@ paths: summary: Verification description: The verify endpoint provides the ability to verify if a user has the necessary permissions to access a specified domain. parameters: - - name: X-Original-URL - in: header - description: Redirection URL - required: true - style: simple - explode: true - schema: - type: string + - $ref: '#/components/parameters/originalURLParam' + - $ref: '#/components/parameters/forwardedMethodParam' + - $ref: '#/components/parameters/authParam' responses: "200": description: Successful Operation @@ -481,6 +471,34 @@ paths: security: - authelia_auth: [] components: + parameters: + originalURLParam: + name: X-Original-URL + in: header + description: Redirection URL + required: true + style: simple + explode: true + schema: + type: string + forwardedMethodParam: + name: X-Forwarded-Method + in: header + description: Request Method + required: false + style: simple + explode: true + schema: + type: string + enum: ["GET", "HEAD", "POST", "PUT", "PATCH", "DELETE", "TRACE", "CONNECT", "OPTIONS"] + authParam: + name: auth + in: query + description: Switch authorization header and prompt for basic auth + required: false + schema: + type: string + enum: ["basic"] schemas: handlers.configuration.ConfigurationBody: type: object @@ -517,6 +535,9 @@ components: targetURL: type: string example: https://home.example.com + requestMethod: + type: string + example: GET keepMeLoggedIn: type: boolean example: true diff --git a/docs/configuration/access-control.md b/docs/configuration/access-control.md index 34db849a..def8a463 100644 --- a/docs/configuration/access-control.md +++ b/docs/configuration/access-control.md @@ -8,7 +8,7 @@ nav_order: 1 # Access Control {: .no_toc } -## Access Control List +## Policies With **Authelia** you can define a list of rules that are going to be evaluated in sequential order when authorization is delegated to Authelia. @@ -16,8 +16,46 @@ sequential order when authorization is delegated to Authelia. The first matching rule of the list defines the policy applied to the resource, if no rule matches the resource a customizable default policy is applied. +### deny -## Access Control Rule +This is the policy applied by default, and is what we recommend as the default policy for all installs. Its effect +is literally to deny the user access to the resource. Additionally you can use this policy to conditionally deny +access in desired situations. Examples include denying access to an API that has no authentication mechanism built in. + +### bypass + +This policy skips all authentication and allows anyone to use the resource. This policy is not available with a rule +that includes a [subject](#Subjects) restriction because the minimum authentication level required to obtain information +about the subject is [one_factor](#one_factor). + +### one_factor + +This policy requires the user at minimum complete 1FA successfully (username and password). This means if they have +performed 2FA then they will be allowed to access the resource. + +### two_factor + +This policy requires the user to complete 2FA successfully. This is currently the highest level of authentication +policy available. + +## Default Policy + +The default policy is the policy applied when no other rule matches. It is recommended that this is configured to +[deny](#deny) for security reasons. Sites which you do not wish to secure with Authelia should not be configured to +perform authentication with Authelia at all. + +See [Policies](#policies) for more information. + +## Network Aliases + +The main networks section defines a list of network aliases, where the name matches a list of networks. These names can +be used in any [rule](#rules) instead of a literal network. This makes it easier to define a group of networks multiple +times. + +You can combine both literal networks and these aliases inside the [networks](#networks) section of a rule. See this +section for more details. + +## Rules A rule defines two things: @@ -26,29 +64,24 @@ A rule defines two things: The criteria are: -* domain: domain targeted by the request. -* resources: list of patterns that the path should match (one is sufficient). +* domain: domain or list of domains targeted by the request. +* resources: pattern or list of patterns that the path should match. * subject: the user or group of users to define the policy for. * networks: the network addresses, ranges (CIDR notation) or groups from where the request originates. +* methods: the http methods used in the request. -A rule is matched when all criteria of the rule match. +A rule is matched when all criteria of the rule match. Rules are evaluated in sequential order, and this is +particularly **important** for bypass rules. Bypass rules should generally appear near the top of the rules list. -## Policies +### Policy A policy represents the level of authentication the user needs to pass before being authorized to request the resource. -There exist 4 policies: +See [Policies](#policies) for more information. -* bypass: the resource is public as the user does not need any authentication to -get access to it. -* one_factor: the user needs to pass at least the first factor to get access to -the resource. -* two_factor: the user needs to pass two factors to get access to the resource. -* deny: the user does not have access to the resource. - -## Domains +### Domains 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 @@ -58,7 +91,12 @@ For instance, to define a rule for all subdomains of *example.com*, one would us These domains can be either listed in YAML-short form `["example1.com", "example2.com"]` or in YAML long-form as dashed list. -## Resources +Domain prefixes can also be dynamically match users or groups. For example you can have a +specific policy adjustment if the user or group matches the subdomain. For +example `{user}.example.com` or `{group}.example.com` check the users name or +groups against the subdomain. + +### Resources A rule can define multiple regular expressions for matching the path of the resource similar to the list of domains. If any one of them matches, the resource criteria of @@ -72,7 +110,7 @@ when you are using regular expressions, you enclose them between quotes. It's op it will likely save you a lot of debugging time. -## Subjects +### Subjects A subject is a representation of a user or a group of user for who the rule should apply. @@ -86,20 +124,52 @@ In summary, the first list level of subjects are evaluated using a logical `OR`, second level by a logical `AND`. The last example below reads as: the group is `dev` AND the username is `john` OR the group is `admins`. -## Networks +#### Combining subjects and the bypass policy + +A subject cannot be combined with the `bypass` policy since the minimum authentication level to identify a subject is +`one_factor`. Combining the `one_factor` policy with a subject is effectively the same as setting the policy to `bypass` +in the past. We have taken an opinionated stance on preventing this configuration as it could result in problematic +security scenarios with badly thought out configurations and cannot see a likely configuration scenario that would +require users to do this. If you have a scenario in mind please open an +[issue](https://github.com/authelia/authelia/issues/new) on GitHub. + +### Networks A list of network addresses, ranges (CIDR notation) or groups can be specified in a rule in order to apply different -policies when requests originate from different networks. +policies when requests originate from different networks. This list can contain both literal definitions of networks +and [network aliases](#network-aliases). -The main use case is when, lets say a resource should be exposed both on the Internet and from an +Main use cases for this rule option is to adjust the security requirements of a resource based on the location of +the user. For example lets say a resource should be exposed both on the Internet and from an authenticated VPN for instance. Passing a second factor a first time to get access to the VPN and a second time to get access to the application can sometimes be cumbersome if the endpoint is not considered overly sensitive. +An additional situation where this may be useful is if there is a specific network you wish to deny access +or require a higher level of authentication for; like a public machine network vs a company device network, or a +BYOD network. + Even if Authelia provides this flexibility, you might prefer a higher level of security and avoid this option entirely. You and only you can define your security policy and it's up to you to configure Authelia accordingly. +### Methods + +A list of HTTP request methods to apply the rule to. Valid values are GET, HEAD, POST, PUT, DELETE, +CONNECT, OPTIONS, and TRACE. Additional information about HTTP request methods can be found on the +[MDN](https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods). + +It's important to note this policy type is primarily intended for use when you wish to bypass authentication for +a specific request method. This is because there are several key limitations in what is possible to accomplish +without Authelia being a reverse proxy server. This rule type is discouraged unless you really know what you're +doing or you wish to setup a rule to bypass CORS preflight requests by bypassing for the OPTIONS method. + +For example, if you require authentication only for write events (POST, PATCH, DELETE, PUT), when a user who is not +currently authenticated tries to do one of these actions, they will be redirected to Authelia. Authelia will decide +what level is required for authentication, and then after the user authenticates it will redirect them to the original +URL where Authelia decided they needed to authenticate. So if the endpoint they are redirected to originally had +data sent as part of the request, this data is completely lost. Further if the endpoint expects the data or doesn't allow +GET request types, the user may be presented with an error leading to a bad user experience. ## Complete example @@ -119,6 +189,11 @@ access_control: - domain: public.example.com policy: bypass + - domain: "*.example.com" + policy: bypass + methods: + - OPTIONS + - domain: secure.example.com policy: one_factor networks: @@ -158,4 +233,7 @@ access_control: - ["group:dev", "user:john"] - "group:admins" policy: two_factor + + - domain: "{user}.example.com" + policy: bypass ``` diff --git a/docs/deployment/supported-proxies/haproxy.md b/docs/deployment/supported-proxies/haproxy.md index 735d78e4..170cd21e 100644 --- a/docs/deployment/supported-proxies/haproxy.md +++ b/docs/deployment/supported-proxies/haproxy.md @@ -102,8 +102,21 @@ frontend fe_http http-request set-var(req.scheme) str(http) if !{ ssl_fc } http-request set-var(req.questionmark) str(?) if { query -m found } + # These are optional if you wish to use the Methods rule in the access_control section. + #http-request set-var(req.method) str(CONNECT) if { method CONNECT } + #http-request set-var(req.method) str(GET) if { method GET } + #http-request set-var(req.method) str(HEAD) if { method HEAD } + #http-request set-var(req.method) str(OPTIONS) if { method OPTIONS } + #http-request set-var(req.method) str(POST) if { method POST } + #http-request set-var(req.method) str(TRACE) if { method TRACE } + #http-request set-var(req.method) str(PUT) if { method PUT } + #http-request set-var(req.method) str(PATCH) if { method PATCH } + #http-request set-var(req.method) str(DELETE) if { method DELETE } + #http-request set-header X-Forwarded-Method %[var(req.method)] + # Required headers http-request set-header X-Real-IP %[src] + http-request set-header X-Forwarded-Method %[var(req.method)] http-request set-header X-Forwarded-Proto %[var(req.scheme)] http-request set-header X-Forwarded-Host %[req.hdr(Host)] http-request set-header X-Forwarded-Uri %[path]%[var(req.questionmark)]%[query] @@ -179,7 +192,19 @@ frontend fe_http http-request set-var(req.scheme) str(https) if { ssl_fc } http-request set-var(req.scheme) str(http) if !{ ssl_fc } http-request set-var(req.questionmark) str(?) if { query -m found } - + + # These are optional if you wish to use the Methods rule in the access_control section. + #http-request set-var(req.method) str(CONNECT) if { method CONNECT } + #http-request set-var(req.method) str(GET) if { method GET } + #http-request set-var(req.method) str(HEAD) if { method HEAD } + #http-request set-var(req.method) str(OPTIONS) if { method OPTIONS } + #http-request set-var(req.method) str(POST) if { method POST } + #http-request set-var(req.method) str(TRACE) if { method TRACE } + #http-request set-var(req.method) str(PUT) if { method PUT } + #http-request set-var(req.method) str(PATCH) if { method PATCH } + #http-request set-var(req.method) str(DELETE) if { method DELETE } + #http-request set-header X-Forwarded-Method %[var(req.method)] + # Required headers http-request set-header X-Real-IP %[src] http-request set-header X-Forwarded-Proto %[var(req.scheme)] diff --git a/docs/deployment/supported-proxies/nginx.md b/docs/deployment/supported-proxies/nginx.md index 949bd7ee..2b238fe9 100644 --- a/docs/deployment/supported-proxies/nginx.md +++ b/docs/deployment/supported-proxies/nginx.md @@ -48,10 +48,11 @@ location /authelia { proxy_set_header Host $host; proxy_set_header X-Original-URL $scheme://$http_host$request_uri; proxy_set_header X-Real-IP $remote_addr; - proxy_set_header X-Forwarded-For $remote_addr; + proxy_set_header X-Forwarded-Method $request_method; proxy_set_header X-Forwarded-Proto $scheme; proxy_set_header X-Forwarded-Host $http_host; proxy_set_header X-Forwarded-Uri $request_uri; + proxy_set_header X-Forwarded-For $remote_addr; proxy_set_header X-Forwarded-Ssl on; proxy_redirect http:// $scheme://; proxy_http_version 1.1; @@ -208,10 +209,11 @@ location /authelia { proxy_set_header Host $host; proxy_set_header X-Original-URL $scheme://$http_host$request_uri; proxy_set_header X-Real-IP $remote_addr; - proxy_set_header X-Forwarded-For $remote_addr; + proxy_set_header X-Forwarded-Method $request_method; proxy_set_header X-Forwarded-Proto $scheme; proxy_set_header X-Forwarded-Host $http_host; proxy_set_header X-Forwarded-Uri $request_uri; + proxy_set_header X-Forwarded-For $remote_addr; proxy_set_header X-Forwarded-Ssl on; proxy_redirect http:// $scheme://; proxy_http_version 1.1; diff --git a/internal/authorization/access_control_domain.go b/internal/authorization/access_control_domain.go new file mode 100644 index 00000000..e018621a --- /dev/null +++ b/internal/authorization/access_control_domain.go @@ -0,0 +1,32 @@ +package authorization + +import ( + "fmt" + "strings" + + "github.com/authelia/authelia/internal/utils" +) + +// AccessControlDomain represents an ACL domain. +type AccessControlDomain struct { + Name string + Wildcard bool + UserWildcard bool + GroupWildcard bool +} + +// IsMatch returns true if the ACL domain matches the object domain. +func (acd AccessControlDomain) IsMatch(subject Subject, object Object) (match bool) { + switch { + case acd.Wildcard: + return strings.HasSuffix(object.Domain, acd.Name) + case acd.UserWildcard: + return object.Domain == fmt.Sprintf("%s.%s", subject.Username, acd.Name) + case acd.GroupWildcard: + prefix, suffix := domainToPrefixSuffix(object.Domain) + + return suffix == acd.Name && utils.IsStringInSliceFold(prefix, subject.Groups) + default: + return object.Domain == acd.Name + } +} diff --git a/internal/authorization/access_control_resource.go b/internal/authorization/access_control_resource.go new file mode 100644 index 00000000..3482b413 --- /dev/null +++ b/internal/authorization/access_control_resource.go @@ -0,0 +1,15 @@ +package authorization + +import ( + "regexp" +) + +// AccessControlResource represents an ACL resource. +type AccessControlResource struct { + Pattern *regexp.Regexp +} + +// IsMatch returns true if the ACL resource match the object path. +func (acr AccessControlResource) IsMatch(object Object) (match bool) { + return acr.Pattern.MatchString(object.Path) +} diff --git a/internal/authorization/access_control_rule.go b/internal/authorization/access_control_rule.go new file mode 100644 index 00000000..4b4e968e --- /dev/null +++ b/internal/authorization/access_control_rule.go @@ -0,0 +1,139 @@ +package authorization + +import ( + "net" + + "github.com/authelia/authelia/internal/configuration/schema" + "github.com/authelia/authelia/internal/utils" +) + +// NewAccessControlRules converts a schema.AccessControlConfiguration into an AccessControlRule slice. +func NewAccessControlRules(config schema.AccessControlConfiguration) (rules []*AccessControlRule) { + networksMap, networksCacheMap := parseSchemaNetworks(config.Networks) + + for _, schemaRule := range config.Rules { + rules = append(rules, NewAccessControlRule(schemaRule, networksMap, networksCacheMap)) + } + + return rules +} + +// NewAccessControlRule parses a schema ACL and generates an internal ACL. +func NewAccessControlRule(rule schema.ACLRule, networksMap map[string][]*net.IPNet, networksCacheMap map[string]*net.IPNet) *AccessControlRule { + return &AccessControlRule{ + Domains: schemaDomainsToACL(rule.Domains), + Resources: schemaResourcesToACL(rule.Resources), + Methods: schemaMethodsToACL(rule.Methods), + Networks: schemaNetworksToACL(rule.Networks, networksMap, networksCacheMap), + Subjects: schemaSubjectsToACL(rule.Subjects), + Policy: PolicyToLevel(rule.Policy), + } +} + +// AccessControlRule controls and represents an ACL internally. +type AccessControlRule struct { + Domains []AccessControlDomain + Resources []AccessControlResource + Methods []string + Networks []*net.IPNet + Subjects []AccessControlSubjects + Policy Level +} + +// 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) { + return false + } + + if !isMatchForResources(object, acr) { + return false + } + + if !isMatchForMethods(object, acr) { + return false + } + + if !isMatchForNetworks(subject, acr) { + return false + } + + if !isMatchForSubjects(subject, acr) { + return false + } + + return true +} + +func isMatchForDomains(subject Subject, object Object, acl *AccessControlRule) (match bool) { + // If there are no domains in this rule then the domain condition is a match. + if len(acl.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 { + if domain.IsMatch(subject, object) { + return true + } + } + + return false +} + +func isMatchForResources(object Object, acl *AccessControlRule) (match bool) { + // If there are no resources in this rule then the resource condition is a match. + if len(acl.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 { + if resource.IsMatch(object) { + return true + } + } + + 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 { + return true + } + + return utils.IsStringInSlice(object.Method, acl.Methods) +} + +func isMatchForNetworks(subject Subject, acl *AccessControlRule) (match bool) { + // If there are no networks in this rule then the network condition is a match. + if len(acl.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 { + if network.Contains(subject.IP) { + return true + } + } + + return false +} + +func isMatchForSubjects(subject Subject, acl *AccessControlRule) (match bool) { + // If there are no subjects in this rule then the subject condition is a match. + if len(acl.Subjects) == 0 || subject.IsAnonymous() { + return true + } + + // Iterate over the subjects until we find a match (return true) or until we exit the loop (return false). + for _, subjectRule := range acl.Subjects { + if subjectRule.IsMatch(subject) { + return true + } + } + + return false +} diff --git a/internal/authorization/access_control_subjects.go b/internal/authorization/access_control_subjects.go new file mode 100644 index 00000000..6e5efc7b --- /dev/null +++ b/internal/authorization/access_control_subjects.go @@ -0,0 +1,55 @@ +package authorization + +import ( + "github.com/authelia/authelia/internal/utils" +) + +// AccessControlSubject abstracts an ACL subject of type `group:` or `user:`. +type AccessControlSubject interface { + IsMatch(subject Subject) (match bool) +} + +// AccessControlSubjects represents an ACL subject. +type AccessControlSubjects struct { + Subjects []AccessControlSubject +} + +// AddSubject appends to the AccessControlSubjects based on a subject rule string. +func (acs *AccessControlSubjects) AddSubject(subjectRule string) { + subject := schemaSubjectToACLSubject(subjectRule) + + if subject != nil { + acs.Subjects = append(acs.Subjects, subject) + } +} + +// IsMatch returns true if the ACL subjects match the subject properties. +func (acs AccessControlSubjects) IsMatch(subject Subject) (match bool) { + for _, rule := range acs.Subjects { + if !rule.IsMatch(subject) { + return false + } + } + + return true +} + +// AccessControlUser represents an ACL subject of type `user:`. +type AccessControlUser struct { + Name string +} + +// IsMatch returns true if the AccessControlUser name matches the Subject username. +func (acu AccessControlUser) IsMatch(subject Subject) (match bool) { + return subject.Username == acu.Name +} + +// AccessControlGroup represents an ACL subject of type `group:`. +type AccessControlGroup struct { + Name string +} + +// IsMatch returns true if the AccessControlGroup name matches one of the groups of the Subject. +func (acg AccessControlGroup) IsMatch(subject Subject) (match bool) { + return utils.IsStringInSlice(acg.Name, subject.Groups) +} diff --git a/internal/authorization/authorizer.go b/internal/authorization/authorizer.go index 8d602250..1e0d654f 100644 --- a/internal/authorization/authorizer.go +++ b/internal/authorization/authorizer.go @@ -1,110 +1,32 @@ package authorization import ( - "fmt" - "net" - "net/url" - "strings" - "github.com/authelia/authelia/internal/configuration/schema" "github.com/authelia/authelia/internal/logging" ) -const userPrefix = "user:" -const groupPrefix = "group:" - // Authorizer the component in charge of checking whether a user can access a given resource. type Authorizer struct { - configuration schema.AccessControlConfiguration + defaultPolicy Level + rules []*AccessControlRule } // NewAuthorizer create an instance of authorizer with a given access control configuration. func NewAuthorizer(configuration schema.AccessControlConfiguration) *Authorizer { return &Authorizer{ - configuration: configuration, + defaultPolicy: PolicyToLevel(configuration.DefaultPolicy), + rules: NewAccessControlRules(configuration), } } -// Subject subject who to check access control for. -type Subject struct { - Username string - Groups []string - IP net.IP -} - -func (s Subject) String() string { - return fmt.Sprintf("username=%s groups=%s ip=%s", s.Username, strings.Join(s.Groups, ","), s.IP.String()) -} - -// Object object to check access control for. -type Object struct { - Domain string - Path string -} - -// selectMatchingSubjectRules take a set of rules and select only the rules matching the subject constraints. -func selectMatchingSubjectRules(rules []schema.ACLRule, networks []schema.ACLNetwork, subject Subject) []schema.ACLRule { - selectedRules := []schema.ACLRule{} - - for _, rule := range rules { - switch { - case len(rule.Subjects) > 0: - for _, subjectRule := range rule.Subjects { - if isSubjectMatching(subject, subjectRule) && isIPMatching(subject.IP, rule.Networks, networks) { - selectedRules = append(selectedRules, rule) - } - } - default: - if isIPMatching(subject.IP, rule.Networks, networks) { - selectedRules = append(selectedRules, rule) - } - } - } - - return selectedRules -} - -func selectMatchingObjectRules(rules []schema.ACLRule, object Object) []schema.ACLRule { - selectedRules := []schema.ACLRule{} - - for _, rule := range rules { - if isDomainMatching(object.Domain, rule.Domains) && isPathMatching(object.Path, rule.Resources) { - selectedRules = append(selectedRules, rule) - } - } - - return selectedRules -} - -func selectMatchingRules(rules []schema.ACLRule, networks []schema.ACLNetwork, subject Subject, object Object) []schema.ACLRule { - matchingRules := selectMatchingSubjectRules(rules, networks, subject) - return selectMatchingObjectRules(matchingRules, object) -} - -// PolicyToLevel converts a string policy to int authorization level. -func PolicyToLevel(policy string) Level { - switch policy { - case "bypass": - return Bypass - case "one_factor": - return OneFactor - case "two_factor": - return TwoFactor - case "deny": - return Denied - } - // By default the deny policy applies. - return Denied -} - // IsSecondFactorEnabled return true if at least one policy is set to second factor. func (p *Authorizer) IsSecondFactorEnabled() bool { - if PolicyToLevel(p.configuration.DefaultPolicy) == TwoFactor { + if p.defaultPolicy == TwoFactor { return true } - for _, r := range p.configuration.Rules { - if PolicyToLevel(r.Policy) == TwoFactor { + for _, rule := range p.rules { + if rule.Policy == TwoFactor { return true } } @@ -113,20 +35,17 @@ func (p *Authorizer) IsSecondFactorEnabled() bool { } // GetRequiredLevel retrieve the required level of authorization to access the object. -func (p *Authorizer) GetRequiredLevel(subject Subject, requestURL url.URL) Level { +func (p *Authorizer) GetRequiredLevel(subject Subject, object Object) Level { logger := logging.Logger() - logger.Tracef("Check authorization of subject %s and url %s.", subject.String(), requestURL.String()) + logger.Tracef("Check authorization of subject %s and url %s.", subject.String(), object.String()) - matchingRules := selectMatchingRules(p.configuration.Rules, p.configuration.Networks, subject, Object{ - Domain: requestURL.Hostname(), - Path: requestURL.Path, - }) - - if len(matchingRules) > 0 { - return PolicyToLevel(matchingRules[0].Policy) + for _, rule := range p.rules { + if rule.IsMatch(subject, object) { + return rule.Policy + } } - logger.Tracef("No matching rule for subject %s and url %s... Applying default policy.", subject.String(), requestURL.String()) + logger.Tracef("No matching rule for subject %s and url %s... Applying default policy.", subject.String(), object.String()) - return PolicyToLevel(p.configuration.DefaultPolicy) + return p.defaultPolicy } diff --git a/internal/authorization/authorizer_test.go b/internal/authorization/authorizer_test.go index 19d22606..3cb58c6d 100644 --- a/internal/authorization/authorizer_test.go +++ b/internal/authorization/authorizer_test.go @@ -25,13 +25,17 @@ func NewAuthorizerTester(config schema.AccessControlConfiguration) *AuthorizerTe } } -func (s *AuthorizerTester) CheckAuthorizations(t *testing.T, subject Subject, requestURI string, expectedLevel Level) { +func (s *AuthorizerTester) CheckAuthorizations(t *testing.T, subject Subject, requestURI, method string, expectedLevel Level) { url, _ := url.ParseRequestURI(requestURI) - level := s.GetRequiredLevel(Subject{ - Groups: subject.Groups, - Username: subject.Username, - IP: subject.IP, - }, *url) + + object := Object{ + Scheme: url.Scheme, + Domain: url.Hostname(), + Path: url.Path, + Method: method, + } + + level := s.GetRequiredLevel(subject, object) assert.Equal(t, expectedLevel, level) } @@ -80,24 +84,40 @@ var UserWithoutGroups = Subject{ var Bob = UserWithoutGroups +var UserWithIPv6Address = Subject{ + Username: "sam", + Groups: []string{}, + IP: net.ParseIP("fec0::1"), +} + +var Sam = UserWithIPv6Address + +var UserWithIPv6AddressAndGroups = Subject{ + Username: "sam", + Groups: []string{"dev", "admins"}, + IP: net.ParseIP("fec0::2"), +} + +var Sally = UserWithIPv6AddressAndGroups + func (s *AuthorizerSuite) TestShouldCheckDefaultBypassConfig() { tester := NewAuthorizerBuilder(). WithDefaultPolicy("bypass").Build() - tester.CheckAuthorizations(s.T(), AnonymousUser, "https://public.example.com/", Bypass) - tester.CheckAuthorizations(s.T(), UserWithGroups, "https://public.example.com/", Bypass) - tester.CheckAuthorizations(s.T(), UserWithoutGroups, "https://public.example.com/", Bypass) - tester.CheckAuthorizations(s.T(), UserWithoutGroups, "https://public.example.com/elsewhere", Bypass) + tester.CheckAuthorizations(s.T(), AnonymousUser, "https://public.example.com/", "GET", Bypass) + tester.CheckAuthorizations(s.T(), UserWithGroups, "https://public.example.com/", "GET", Bypass) + tester.CheckAuthorizations(s.T(), UserWithoutGroups, "https://public.example.com/", "GET", Bypass) + tester.CheckAuthorizations(s.T(), UserWithoutGroups, "https://public.example.com/elsewhere", "GET", Bypass) } func (s *AuthorizerSuite) TestShouldCheckDefaultDeniedConfig() { tester := NewAuthorizerBuilder(). WithDefaultPolicy("deny").Build() - tester.CheckAuthorizations(s.T(), AnonymousUser, "https://public.example.com/", Denied) - tester.CheckAuthorizations(s.T(), UserWithGroups, "https://public.example.com/", Denied) - tester.CheckAuthorizations(s.T(), UserWithoutGroups, "https://public.example.com/", Denied) - tester.CheckAuthorizations(s.T(), UserWithoutGroups, "https://public.example.com/elsewhere", Denied) + tester.CheckAuthorizations(s.T(), AnonymousUser, "https://public.example.com/", "GET", Denied) + tester.CheckAuthorizations(s.T(), UserWithGroups, "https://public.example.com/", "GET", Denied) + tester.CheckAuthorizations(s.T(), UserWithoutGroups, "https://public.example.com/", "GET", Denied) + tester.CheckAuthorizations(s.T(), UserWithoutGroups, "https://public.example.com/elsewhere", "GET", Denied) } func (s *AuthorizerSuite) TestShouldCheckMultiDomainRule() { @@ -109,12 +129,31 @@ func (s *AuthorizerSuite) TestShouldCheckMultiDomainRule() { }). 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://public.example.com/", "GET", Bypass) + tester.CheckAuthorizations(s.T(), UserWithGroups, "https://private.example.com/", "GET", Bypass) + tester.CheckAuthorizations(s.T(), UserWithGroups, "https://public.example.com/elsewhere", "GET", Bypass) + tester.CheckAuthorizations(s.T(), UserWithGroups, "https://example.com/", "GET", Denied) + tester.CheckAuthorizations(s.T(), UserWithGroups, "https://public.example.com.c/", "GET", Denied) + tester.CheckAuthorizations(s.T(), UserWithGroups, "https://public.example.co/", "GET", Denied) +} + +func (s *AuthorizerSuite) TestShouldCheckDynamicDomainRules() { + tester := NewAuthorizerBuilder(). + WithDefaultPolicy("deny"). + WithRule(schema.ACLRule{ + Domains: []string{"{user}.example.com"}, + Policy: "bypass", + }). + WithRule(schema.ACLRule{ + Domains: []string{"{group}.example.com"}, + Policy: "bypass", + }). + 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://othergroup.example.com/", "GET", Denied) } func (s *AuthorizerSuite) TestShouldCheckMultipleDomainRule() { @@ -126,15 +165,15 @@ func (s *AuthorizerSuite) TestShouldCheckMultipleDomainRule() { }). 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) + tester.CheckAuthorizations(s.T(), UserWithGroups, "https://public.example.com/", "GET", Bypass) + tester.CheckAuthorizations(s.T(), UserWithGroups, "https://private.example.com/", "GET", Bypass) + tester.CheckAuthorizations(s.T(), UserWithGroups, "https://public.example.com/elsewhere", "GET", Bypass) + tester.CheckAuthorizations(s.T(), UserWithGroups, "https://example.com/", "GET", Denied) + tester.CheckAuthorizations(s.T(), UserWithGroups, "https://public.example.com.c/", "GET", Denied) + tester.CheckAuthorizations(s.T(), UserWithGroups, "https://public.example.co/", "GET", Denied) + tester.CheckAuthorizations(s.T(), UserWithGroups, "https://other.com/", "GET", Bypass) + tester.CheckAuthorizations(s.T(), UserWithGroups, "https://other.com/elsewhere", "GET", Bypass) + tester.CheckAuthorizations(s.T(), UserWithGroups, "https://private.other.com/", "GET", Denied) } func (s *AuthorizerSuite) TestShouldCheckFactorsPolicy() { @@ -154,10 +193,10 @@ func (s *AuthorizerSuite) TestShouldCheckFactorsPolicy() { }). Build() - tester.CheckAuthorizations(s.T(), UserWithGroups, "https://public.example.com/", Bypass) - tester.CheckAuthorizations(s.T(), UserWithGroups, "https://protected.example.com/", TwoFactor) - tester.CheckAuthorizations(s.T(), UserWithGroups, "https://single.example.com/", OneFactor) - tester.CheckAuthorizations(s.T(), UserWithGroups, "https://example.com/", Denied) + tester.CheckAuthorizations(s.T(), UserWithGroups, "https://public.example.com/", "GET", Bypass) + tester.CheckAuthorizations(s.T(), UserWithGroups, "https://protected.example.com/", "GET", TwoFactor) + tester.CheckAuthorizations(s.T(), UserWithGroups, "https://single.example.com/", "GET", OneFactor) + tester.CheckAuthorizations(s.T(), UserWithGroups, "https://example.com/", "GET", Denied) } func (s *AuthorizerSuite) TestShouldCheckRulePrecedence() { @@ -178,9 +217,9 @@ func (s *AuthorizerSuite) TestShouldCheckRulePrecedence() { }). Build() - tester.CheckAuthorizations(s.T(), John, "https://protected.example.com/", Bypass) - tester.CheckAuthorizations(s.T(), Bob, "https://protected.example.com/", OneFactor) - tester.CheckAuthorizations(s.T(), John, "https://public.example.com/", TwoFactor) + tester.CheckAuthorizations(s.T(), John, "https://protected.example.com/", "GET", Bypass) + tester.CheckAuthorizations(s.T(), Bob, "https://protected.example.com/", "GET", OneFactor) + tester.CheckAuthorizations(s.T(), John, "https://public.example.com/", "GET", TwoFactor) } func (s *AuthorizerSuite) TestShouldCheckUserMatching() { @@ -188,13 +227,13 @@ func (s *AuthorizerSuite) TestShouldCheckUserMatching() { WithDefaultPolicy("deny"). WithRule(schema.ACLRule{ Domains: []string{"protected.example.com"}, - Policy: "bypass", + Policy: "one_factor", Subjects: [][]string{{"user:john"}}, }). Build() - tester.CheckAuthorizations(s.T(), John, "https://protected.example.com/", Bypass) - tester.CheckAuthorizations(s.T(), Bob, "https://protected.example.com/", Denied) + tester.CheckAuthorizations(s.T(), John, "https://protected.example.com/", "GET", OneFactor) + tester.CheckAuthorizations(s.T(), Bob, "https://protected.example.com/", "GET", Denied) } func (s *AuthorizerSuite) TestShouldCheckGroupMatching() { @@ -202,13 +241,13 @@ func (s *AuthorizerSuite) TestShouldCheckGroupMatching() { WithDefaultPolicy("deny"). WithRule(schema.ACLRule{ Domains: []string{"protected.example.com"}, - Policy: "bypass", + Policy: "one_factor", Subjects: [][]string{{"group:admins"}}, }). Build() - tester.CheckAuthorizations(s.T(), John, "https://protected.example.com/", Bypass) - tester.CheckAuthorizations(s.T(), Bob, "https://protected.example.com/", Denied) + tester.CheckAuthorizations(s.T(), John, "https://protected.example.com/", "GET", OneFactor) + tester.CheckAuthorizations(s.T(), Bob, "https://protected.example.com/", "GET", Denied) } func (s *AuthorizerSuite) TestShouldCheckSubjectsMatching() { @@ -216,14 +255,15 @@ func (s *AuthorizerSuite) TestShouldCheckSubjectsMatching() { WithDefaultPolicy("deny"). WithRule(schema.ACLRule{ Domains: []string{"protected.example.com"}, - Policy: "bypass", + Policy: "one_factor", 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) + tester.CheckAuthorizations(s.T(), John, "https://protected.example.com/", "GET", OneFactor) + tester.CheckAuthorizations(s.T(), Bob, "https://protected.example.com/", "GET", OneFactor) + tester.CheckAuthorizations(s.T(), Sam, "https://protected.example.com/", "GET", Denied) + tester.CheckAuthorizations(s.T(), AnonymousUser, "https://protected.example.com/", "GET", OneFactor) } func (s *AuthorizerSuite) TestShouldCheckMultipleSubjectsMatching() { @@ -231,14 +271,14 @@ func (s *AuthorizerSuite) TestShouldCheckMultipleSubjectsMatching() { WithDefaultPolicy("deny"). WithRule(schema.ACLRule{ Domains: []string{"protected.example.com"}, - Policy: "bypass", + Policy: "one_factor", Subjects: [][]string{{"group:admins", "user:bob"}, {"group:admins", "group:dev"}}, }). Build() - tester.CheckAuthorizations(s.T(), John, "https://protected.example.com/", Bypass) - tester.CheckAuthorizations(s.T(), Bob, "https://protected.example.com/", Denied) - tester.CheckAuthorizations(s.T(), AnonymousUser, "https://protected.example.com/", Denied) + tester.CheckAuthorizations(s.T(), John, "https://protected.example.com/", "GET", OneFactor) + tester.CheckAuthorizations(s.T(), Bob, "https://protected.example.com/", "GET", Denied) + tester.CheckAuthorizations(s.T(), AnonymousUser, "https://protected.example.com/", "GET", OneFactor) } func (s *AuthorizerSuite) TestShouldCheckIPMatching() { @@ -259,15 +299,63 @@ func (s *AuthorizerSuite) TestShouldCheckIPMatching() { Policy: "two_factor", Networks: []string{"10.0.0.0/8"}, }). + WithRule(schema.ACLRule{ + Domains: []string{"ipv6.example.com"}, + Policy: "two_factor", + Networks: []string{"fec0::1/64"}, + }). + WithRule(schema.ACLRule{ + Domains: []string{"ipv6-alt.example.com"}, + Policy: "two_factor", + Networks: []string{"fec0::1"}, + }). Build() - tester.CheckAuthorizations(s.T(), John, "https://protected.example.com/", Bypass) - tester.CheckAuthorizations(s.T(), Bob, "https://protected.example.com/", OneFactor) - tester.CheckAuthorizations(s.T(), AnonymousUser, "https://protected.example.com/", Denied) + tester.CheckAuthorizations(s.T(), John, "https://protected.example.com/", "GET", Bypass) + tester.CheckAuthorizations(s.T(), Bob, "https://protected.example.com/", "GET", OneFactor) + tester.CheckAuthorizations(s.T(), AnonymousUser, "https://protected.example.com/", "GET", Denied) - tester.CheckAuthorizations(s.T(), John, "https://net.example.com/", TwoFactor) - tester.CheckAuthorizations(s.T(), Bob, "https://net.example.com/", TwoFactor) - tester.CheckAuthorizations(s.T(), AnonymousUser, "https://net.example.com/", Denied) + tester.CheckAuthorizations(s.T(), John, "https://net.example.com/", "GET", TwoFactor) + tester.CheckAuthorizations(s.T(), Bob, "https://net.example.com/", "GET", TwoFactor) + tester.CheckAuthorizations(s.T(), AnonymousUser, "https://net.example.com/", "GET", Denied) + + tester.CheckAuthorizations(s.T(), Sally, "https://ipv6-alt.example.com/", "GET", Denied) + tester.CheckAuthorizations(s.T(), Sam, "https://ipv6-alt.example.com/", "GET", TwoFactor) + tester.CheckAuthorizations(s.T(), Sally, "https://ipv6.example.com/", "GET", TwoFactor) + tester.CheckAuthorizations(s.T(), Sam, "https://ipv6.example.com/", "GET", TwoFactor) +} + +func (s *AuthorizerSuite) TestShouldCheckMethodMatching() { + tester := NewAuthorizerBuilder(). + WithDefaultPolicy("deny"). + WithRule(schema.ACLRule{ + Domains: []string{"protected.example.com"}, + Policy: "bypass", + Methods: []string{"OPTIONS", "HEAD", "GET", "CONNECT", "TRACE"}, + }). + WithRule(schema.ACLRule{ + Domains: []string{"protected.example.com"}, + Policy: "one_factor", + Methods: []string{"PUT", "PATCH", "POST"}, + }). + WithRule(schema.ACLRule{ + Domains: []string{"protected.example.com"}, + Policy: "two_factor", + Methods: []string{"DELETE"}, + }). + Build() + + tester.CheckAuthorizations(s.T(), John, "https://protected.example.com/", "GET", Bypass) + tester.CheckAuthorizations(s.T(), Bob, "https://protected.example.com/", "OPTIONS", Bypass) + tester.CheckAuthorizations(s.T(), AnonymousUser, "https://protected.example.com/", "HEAD", Bypass) + tester.CheckAuthorizations(s.T(), John, "https://protected.example.com/", "CONNECT", Bypass) + tester.CheckAuthorizations(s.T(), Bob, "https://protected.example.com/", "TRACE", Bypass) + + tester.CheckAuthorizations(s.T(), John, "https://protected.example.com/", "PUT", OneFactor) + tester.CheckAuthorizations(s.T(), Bob, "https://protected.example.com/", "PATCH", OneFactor) + tester.CheckAuthorizations(s.T(), AnonymousUser, "https://protected.example.com/", "POST", OneFactor) + + tester.CheckAuthorizations(s.T(), AnonymousUser, "https://protected.example.com/", "DELETE", TwoFactor) } func (s *AuthorizerSuite) TestShouldCheckResourceMatching() { @@ -285,12 +373,109 @@ func (s *AuthorizerSuite) TestShouldCheckResourceMatching() { }). Build() - tester.CheckAuthorizations(s.T(), John, "https://resource.example.com/", Bypass) - tester.CheckAuthorizations(s.T(), John, "https://resource.example.com/bypass/abc", Bypass) - tester.CheckAuthorizations(s.T(), John, "https://resource.example.com/bypass/", Denied) - tester.CheckAuthorizations(s.T(), John, "https://resource.example.com/bypass/ABC", Denied) - tester.CheckAuthorizations(s.T(), John, "https://resource.example.com/one_factor/abc", OneFactor) - tester.CheckAuthorizations(s.T(), John, "https://resource.example.com/xyz/embedded/abc", Bypass) + tester.CheckAuthorizations(s.T(), John, "https://resource.example.com/", "GET", Bypass) + tester.CheckAuthorizations(s.T(), John, "https://resource.example.com/bypass/abc", "GET", Bypass) + tester.CheckAuthorizations(s.T(), John, "https://resource.example.com/bypass/", "GET", Denied) + tester.CheckAuthorizations(s.T(), John, "https://resource.example.com/bypass/ABC", "GET", Denied) + tester.CheckAuthorizations(s.T(), John, "https://resource.example.com/one_factor/abc", "GET", OneFactor) + tester.CheckAuthorizations(s.T(), John, "https://resource.example.com/xyz/embedded/abc", "GET", Bypass) +} + +// This test assures that rules without domains (not allowed by schema validator at this time) will pass validation correctly. +func (s *AuthorizerSuite) TestShouldMatchAnyDomainIfBlank() { + tester := NewAuthorizerBuilder(). + WithRule(schema.ACLRule{ + Policy: "bypass", + Methods: []string{"OPTIONS", "HEAD", "GET", "CONNECT", "TRACE"}, + }). + WithRule(schema.ACLRule{ + Policy: "one_factor", + Methods: []string{"PUT", "PATCH"}, + }). + WithRule(schema.ACLRule{ + Policy: "two_factor", + Methods: []string{"DELETE"}, + }). + Build() + + tester.CheckAuthorizations(s.T(), John, "https://one.domain-four.com", "GET", Bypass) + tester.CheckAuthorizations(s.T(), AnonymousUser, "https://one.domain-three.com", "GET", Bypass) + tester.CheckAuthorizations(s.T(), AnonymousUser, "https://one.domain-two.com", "OPTIONS", Bypass) + + tester.CheckAuthorizations(s.T(), John, "https://one.domain-four.com", "PUT", OneFactor) + tester.CheckAuthorizations(s.T(), AnonymousUser, "https://one.domain-three.com", "PATCH", OneFactor) + tester.CheckAuthorizations(s.T(), AnonymousUser, "https://one.domain-two.com", "PUT", OneFactor) + + tester.CheckAuthorizations(s.T(), John, "https://one.domain-four.com", "DELETE", TwoFactor) + tester.CheckAuthorizations(s.T(), AnonymousUser, "https://one.domain-three.com", "DELETE", TwoFactor) + tester.CheckAuthorizations(s.T(), AnonymousUser, "https://one.domain-two.com", "DELETE", TwoFactor) + + tester.CheckAuthorizations(s.T(), John, "https://one.domain-four.com", "POST", Denied) + tester.CheckAuthorizations(s.T(), AnonymousUser, "https://one.domain-three.com", "POST", Denied) + tester.CheckAuthorizations(s.T(), AnonymousUser, "https://one.domain-two.com", "POST", Denied) +} + +func (s *AuthorizerSuite) TestShouldMatchResourceWithSubjectRules() { + tester := NewAuthorizerBuilder(). + WithDefaultPolicy("deny"). + WithRule(schema.ACLRule{ + Domains: []string{"public.example.com"}, + Resources: []string{"^/admin/.*$"}, + Subjects: [][]string{{"group:admins"}}, + Policy: "one_factor", + }). + WithRule(schema.ACLRule{ + Domains: []string{"public.example.com"}, + Resources: []string{"^/admin/.*$"}, + Policy: "deny", + }). + WithRule(schema.ACLRule{ + Domains: []string{"public.example.com"}, + Policy: "bypass", + }). + WithRule(schema.ACLRule{ + Domains: []string{"public2.example.com"}, + Resources: []string{"^/admin/.*$"}, + Subjects: [][]string{{"group:admins"}}, + Policy: "bypass", + }). + WithRule(schema.ACLRule{ + Domains: []string{"public2.example.com"}, + Resources: []string{"^/admin/.*$"}, + Policy: "deny", + }). + WithRule(schema.ACLRule{ + Domains: []string{"public2.example.com"}, + Policy: "bypass", + }). + WithRule(schema.ACLRule{ + Domains: []string{"private.example.com"}, + Subjects: [][]string{{"group:admins"}}, + Policy: "two_factor", + }). + Build() + + tester.CheckAuthorizations(s.T(), John, "https://public.example.com", "GET", Bypass) + tester.CheckAuthorizations(s.T(), Bob, "https://public.example.com", "GET", Bypass) + tester.CheckAuthorizations(s.T(), AnonymousUser, "https://public.example.com", "GET", Bypass) + + tester.CheckAuthorizations(s.T(), John, "https://public.example.com/admin/index.html", "GET", OneFactor) + tester.CheckAuthorizations(s.T(), Bob, "https://public.example.com/admin/index.html", "GET", Denied) + tester.CheckAuthorizations(s.T(), AnonymousUser, "https://public.example.com/admin/index.html", "GET", OneFactor) + + tester.CheckAuthorizations(s.T(), John, "https://public2.example.com", "GET", Bypass) + tester.CheckAuthorizations(s.T(), Bob, "https://public2.example.com", "GET", Bypass) + tester.CheckAuthorizations(s.T(), AnonymousUser, "https://public2.example.com", "GET", Bypass) + + tester.CheckAuthorizations(s.T(), John, "https://public2.example.com/admin/index.html", "GET", Bypass) + tester.CheckAuthorizations(s.T(), Bob, "https://public2.example.com/admin/index.html", "GET", Denied) + + // This test returns this result since we validate the schema instead of validating it in code. + tester.CheckAuthorizations(s.T(), AnonymousUser, "https://public2.example.com/admin/index.html", "GET", Bypass) + + tester.CheckAuthorizations(s.T(), John, "https://private.example.com", "GET", TwoFactor) + tester.CheckAuthorizations(s.T(), Bob, "https://private.example.com", "GET", Denied) + tester.CheckAuthorizations(s.T(), AnonymousUser, "https://private.example.com", "GET", TwoFactor) } func (s *AuthorizerSuite) TestPolicyToLevel() { diff --git a/internal/authorization/const.go b/internal/authorization/const.go index 99093bd8..7e15ab56 100644 --- a/internal/authorization/const.go +++ b/internal/authorization/const.go @@ -13,3 +13,6 @@ const ( // Denied denied level. Denied Level = iota ) + +const userPrefix = "user:" +const groupPrefix = "group:" diff --git a/internal/authorization/domain_matcher.go b/internal/authorization/domain_matcher.go deleted file mode 100644 index c34c8249..00000000 --- a/internal/authorization/domain_matcher.go +++ /dev/null @@ -1,15 +0,0 @@ -package authorization - -import "strings" - -func isDomainMatching(domain string, domainRules []string) bool { - for _, domainRule := range domainRules { - if domain == domainRule { - return true - } else if strings.HasPrefix(domainRule, "*.") && strings.HasSuffix(domain, domainRule[1:]) { - return true - } - } - - return false -} diff --git a/internal/authorization/domain_matcher_test.go b/internal/authorization/domain_matcher_test.go deleted file mode 100644 index 2bf596e0..00000000 --- a/internal/authorization/domain_matcher_test.go +++ /dev/null @@ -1,36 +0,0 @@ -package authorization - -import ( - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestShouldMatchACLWithSingleDomain(t *testing.T) { - assert.True(t, isDomainMatching("example.com", []string{"example.com"})) - - assert.True(t, isDomainMatching("abc.example.com", []string{"*.example.com"})) - assert.True(t, isDomainMatching("abc.def.example.com", []string{"*.example.com"})) -} - -func TestShouldNotMatchACLWithSingleDomain(t *testing.T) { - assert.False(t, isDomainMatching("example.com", []string{"*.example.com"})) - // Character * must be followed by . to be valid. - assert.False(t, isDomainMatching("example.com", []string{"*example.com"})) - - assert.False(t, isDomainMatching("example.com", []string{"*.exampl.com"})) - - assert.False(t, isDomainMatching("example.com", []string{"*.other.net"})) - assert.False(t, isDomainMatching("example.com", []string{"*other.net"})) - assert.False(t, isDomainMatching("example.com", []string{"other.net"})) -} - -func TestShouldMatchACLWithMultipleDomains(t *testing.T) { - assert.True(t, isDomainMatching("example.com", []string{"*.example.com", "example.com"})) - assert.True(t, isDomainMatching("apple.example.com", []string{"*.example.com", "example.com"})) -} - -func TestShouldNotMatchACLWithMultipleDomains(t *testing.T) { - assert.False(t, isDomainMatching("example.com", []string{"*.example.com", "*example.com"})) - assert.False(t, isDomainMatching("apple.example.com", []string{"*example.com", "example.com"})) -} diff --git a/internal/authorization/ip_matcher.go b/internal/authorization/ip_matcher.go deleted file mode 100644 index d76c7c2f..00000000 --- a/internal/authorization/ip_matcher.go +++ /dev/null @@ -1,66 +0,0 @@ -package authorization - -import ( - "net" - "strings" - - "github.com/authelia/authelia/internal/configuration/schema" -) - -func selectMatchingNetworkGroups(networks []string, aclNetworks []schema.ACLNetwork) []schema.ACLNetwork { - selectedNetworkGroups := []schema.ACLNetwork{} - - for _, network := range networks { - for _, n := range aclNetworks { - for _, ng := range n.Name { - if network == ng { - selectedNetworkGroups = append(selectedNetworkGroups, n) - } - } - } - } - - return selectedNetworkGroups -} - -func isIPAddressOrCIDR(ip net.IP, network string) bool { - switch { - case ip.String() == network: - return true - case strings.Contains(network, "/"): - return parseCIDR(ip, network) - } - - return false -} - -func parseCIDR(ip net.IP, network string) bool { - _, ipNet, _ := net.ParseCIDR(network) - return ipNet.Contains(ip) -} - -// isIPMatching check whether user's IP is in one of the network ranges. -func isIPMatching(ip net.IP, networks []string, aclNetworks []schema.ACLNetwork) bool { - // If no network is provided in the rule, we match any network - if len(networks) == 0 { - return true - } - - matchingNetworkGroups := selectMatchingNetworkGroups(networks, aclNetworks) - - for _, network := range networks { - if net.ParseIP(network) == nil && !strings.Contains(network, "/") { - for _, n := range matchingNetworkGroups { - for _, network := range n.Networks { - if isIPAddressOrCIDR(ip, network) { - return true - } - } - } - } else if isIPAddressOrCIDR(ip, network) { - return true - } - } - - return false -} diff --git a/internal/authorization/ip_matcher_test.go b/internal/authorization/ip_matcher_test.go deleted file mode 100644 index ce6107e1..00000000 --- a/internal/authorization/ip_matcher_test.go +++ /dev/null @@ -1,38 +0,0 @@ -package authorization - -import ( - "net" - "testing" - - "github.com/stretchr/testify/assert" - - "github.com/authelia/authelia/internal/configuration/schema" -) - -func TestIPMatcher(t *testing.T) { - // Default policy is 'allow all ips' if no IP is defined - assert.True(t, isIPMatching(net.ParseIP("127.0.0.1"), []string{}, schema.DefaultACLNetwork)) - - assert.True(t, isIPMatching(net.ParseIP("127.0.0.1"), []string{"127.0.0.1"}, schema.DefaultACLNetwork)) - assert.False(t, isIPMatching(net.ParseIP("127.1"), []string{"127.0.0.1"}, schema.DefaultACLNetwork)) - assert.False(t, isIPMatching(net.ParseIP("not-an-ip"), []string{"127.0.0.1"}, schema.DefaultACLNetwork)) - - assert.False(t, isIPMatching(net.ParseIP("127.0.0.1"), []string{"10.0.0.1"}, schema.DefaultACLNetwork)) - assert.False(t, isIPMatching(net.ParseIP("127.0.0.1"), []string{"10.0.0.0/8"}, schema.DefaultACLNetwork)) - - assert.True(t, isIPMatching(net.ParseIP("10.230.5.1"), []string{"10.0.0.0/8"}, schema.DefaultACLNetwork)) - assert.True(t, isIPMatching(net.ParseIP("10.230.5.1"), []string{"192.168.0.0/24", "10.0.0.0/8"}, schema.DefaultACLNetwork)) - - // Test network groups - assert.True(t, isIPMatching(net.ParseIP("127.0.0.1"), []string{}, schema.DefaultACLNetwork)) - - assert.True(t, isIPMatching(net.ParseIP("127.0.0.1"), []string{"localhost"}, schema.DefaultACLNetwork)) - assert.False(t, isIPMatching(net.ParseIP("127.1"), []string{"localhost"}, schema.DefaultACLNetwork)) - assert.False(t, isIPMatching(net.ParseIP("not-an-ip"), []string{"localhost"}, schema.DefaultACLNetwork)) - - assert.False(t, isIPMatching(net.ParseIP("127.0.0.1"), []string{"internal"}, schema.DefaultACLNetwork)) - assert.False(t, isIPMatching(net.ParseIP("127.0.0.1"), []string{"internal"}, schema.DefaultACLNetwork)) - - assert.True(t, isIPMatching(net.ParseIP("10.230.5.1"), []string{"internal"}, schema.DefaultACLNetwork)) - assert.True(t, isIPMatching(net.ParseIP("10.230.5.1"), []string{"192.168.0.0/24", "internal"}, schema.DefaultACLNetwork)) -} diff --git a/internal/authorization/path_matcher.go b/internal/authorization/path_matcher.go deleted file mode 100644 index 6eca5c27..00000000 --- a/internal/authorization/path_matcher.go +++ /dev/null @@ -1,19 +0,0 @@ -package authorization - -import "regexp" - -func isPathMatching(path string, pathRegexps []string) bool { - // If there is no regexp patterns, it means that we match any path. - if len(pathRegexps) == 0 { - return true - } - - for _, pathRegexp := range pathRegexps { - match, _ := regexp.MatchString(pathRegexp, path) - if match { - return true - } - } - - return false -} diff --git a/internal/authorization/path_matcher_test.go b/internal/authorization/path_matcher_test.go deleted file mode 100644 index bfcd8a54..00000000 --- a/internal/authorization/path_matcher_test.go +++ /dev/null @@ -1,24 +0,0 @@ -package authorization - -import ( - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestPathMatcher(t *testing.T) { - // Matching any path if no regexp is provided - assert.True(t, isPathMatching("/", []string{})) - - assert.False(t, isPathMatching("/", []string{"^/api"})) - assert.True(t, isPathMatching("/api/test", []string{"^/api"})) - assert.False(t, isPathMatching("/api/test", []string{"^/api$"})) - assert.True(t, isPathMatching("/api", []string{"^/api$"})) - assert.True(t, isPathMatching("/api/test", []string{"^/api/?.*"})) - assert.True(t, isPathMatching("/apitest", []string{"^/api/?.*"})) - assert.True(t, isPathMatching("/api/test", []string{"^/api/.*"})) - assert.True(t, isPathMatching("/api/", []string{"^/api/.*"})) - assert.False(t, isPathMatching("/api", []string{"^/api/.*"})) - - assert.False(t, isPathMatching("/api", []string{"xyz", "^/api/.*"})) -} diff --git a/internal/authorization/subject_matcher.go b/internal/authorization/subject_matcher.go deleted file mode 100644 index 90f7aff9..00000000 --- a/internal/authorization/subject_matcher.go +++ /dev/null @@ -1,29 +0,0 @@ -package authorization - -import ( - "strings" - - "github.com/authelia/authelia/internal/utils" -) - -func isSubjectMatching(subject Subject, subjectRule []string) bool { - for _, ruleSubject := range subjectRule { - if strings.HasPrefix(ruleSubject, userPrefix) { - user := strings.Trim(ruleSubject[len(userPrefix):], " ") - if user == subject.Username { - continue - } - } - - if strings.HasPrefix(ruleSubject, groupPrefix) { - group := strings.Trim(ruleSubject[len(groupPrefix):], " ") - if utils.IsStringInSlice(group, subject.Groups) { - continue - } - } - - return false - } - - return true -} diff --git a/internal/authorization/types.go b/internal/authorization/types.go new file mode 100644 index 00000000..880254eb --- /dev/null +++ b/internal/authorization/types.go @@ -0,0 +1,60 @@ +package authorization + +import ( + "fmt" + "net" + "net/url" + "strings" +) + +// Subject represents the identity of a user for the purposes of ACL matching. +type Subject struct { + Username string + Groups []string + IP net.IP +} + +// String returns a string representation of the Subject. +func (s Subject) String() string { + return fmt.Sprintf("username=%s groups=%s ip=%s", s.Username, strings.Join(s.Groups, ","), s.IP.String()) +} + +// IsAnonymous returns true if the Subject username and groups are empty. +func (s Subject) IsAnonymous() bool { + return s.Username == "" && len(s.Groups) == 0 +} + +// Object represents a protected object for the purposes of ACL matching. +type Object struct { + Scheme string + Domain string + Path string + Method string +} + +// String is a string representation of the Object. +func (o Object) String() string { + return fmt.Sprintf("%s://%s%s", o.Scheme, o.Domain, o.Path) +} + +// NewObjectRaw creates a new Object type from a URL and a method header. +func NewObjectRaw(targetURL *url.URL, method []byte) (object Object) { + return NewObject(targetURL, string(method)) +} + +// NewObject creates a new Object type from a URL and a method header. +func NewObject(targetURL *url.URL, method string) (object Object) { + object = Object{ + Scheme: targetURL.Scheme, + Domain: targetURL.Hostname(), + Method: method, + } + + if targetURL.RawQuery == "" { + object.Path = targetURL.Path + } else { + object.Path = targetURL.Path + "?" + targetURL.RawQuery + } + + return object +} diff --git a/internal/authorization/types_test.go b/internal/authorization/types_test.go new file mode 100644 index 00000000..be39a922 --- /dev/null +++ b/internal/authorization/types_test.go @@ -0,0 +1,22 @@ +package authorization + +import ( + "net/url" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestShouldAppendQueryParamToURL(t *testing.T) { + targetURL, err := url.Parse("https://domain.example.com/api?type=none") + + require.NoError(t, err) + + object := NewObject(targetURL, "GET") + + assert.Equal(t, "domain.example.com", object.Domain) + assert.Equal(t, "GET", object.Method) + assert.Equal(t, "/api?type=none", object.Path) + assert.Equal(t, "https", object.Scheme) +} diff --git a/internal/authorization/util.go b/internal/authorization/util.go new file mode 100644 index 00000000..f2997b84 --- /dev/null +++ b/internal/authorization/util.go @@ -0,0 +1,177 @@ +package authorization + +import ( + "net" + "regexp" + "strings" + + "github.com/authelia/authelia/internal/configuration/schema" +) + +// PolicyToLevel converts a string policy to int authorization level. +func PolicyToLevel(policy string) Level { + switch policy { + case "bypass": + return Bypass + case "one_factor": + return OneFactor + case "two_factor": + return TwoFactor + case "deny": + return Denied + } + // By default the deny policy applies. + return Denied +} + +func schemaSubjectToACLSubject(subjectRule string) (subject AccessControlSubject) { + if strings.HasPrefix(subjectRule, userPrefix) { + user := strings.Trim(subjectRule[len(userPrefix):], " ") + + return AccessControlUser{Name: user} + } + + if strings.HasPrefix(subjectRule, groupPrefix) { + group := strings.Trim(subjectRule[len(groupPrefix):], " ") + + return AccessControlGroup{Name: group} + } + + return nil +} + +func schemaDomainsToACL(domainRules []string) (domains []AccessControlDomain) { + for _, domainRule := range domainRules { + domain := AccessControlDomain{} + + domainRule = strings.ToLower(domainRule) + + switch { + case strings.HasPrefix(domainRule, "*."): + domain.Wildcard = true + domain.Name = domainRule[1:] + case strings.HasPrefix(domainRule, "{user}"): + domain.UserWildcard = true + domain.Name = domainRule[7:] + case strings.HasPrefix(domainRule, "{group}"): + domain.GroupWildcard = true + domain.Name = domainRule[8:] + default: + domain.Name = domainRule + } + + domains = append(domains, domain) + } + + return domains +} + +func schemaResourcesToACL(resourceRules []string) (resources []AccessControlResource) { + for _, resourceRule := range resourceRules { + resources = append(resources, AccessControlResource{regexp.MustCompile(resourceRule)}) + } + + return resources +} + +func schemaMethodsToACL(methodRules []string) (methods []string) { + for _, method := range methodRules { + methods = append(methods, strings.ToUpper(method)) + } + + return methods +} + +func schemaNetworksToACL(networkRules []string, networksMap map[string][]*net.IPNet, networksCacheMap map[string]*net.IPNet) (networks []*net.IPNet) { + for _, network := range networkRules { + if _, ok := networksMap[network]; !ok { + if _, ok := networksCacheMap[network]; ok { + networks = append(networks, networksCacheMap[network]) + } else { + cidr, err := parseNetwork(network) + if err == nil { + networks = append(networks, cidr) + networksCacheMap[cidr.String()] = cidr + + if cidr.String() != network { + networksCacheMap[network] = cidr + } + } + } + } else { + networks = append(networks, networksMap[network]...) + } + } + + return networks +} + +func parseSchemaNetworks(schemaNetworks []schema.ACLNetwork) (networksMap map[string][]*net.IPNet, networksCacheMap map[string]*net.IPNet) { + // These maps store pointers to the net.IPNet values so we can reuse them efficiently. + // The networksMap contains the named networks as keys, the networksCacheMap contains the CIDR notations as keys. + networksMap = map[string][]*net.IPNet{} + networksCacheMap = map[string]*net.IPNet{} + + for _, aclNetwork := range schemaNetworks { + var networks []*net.IPNet + + for _, networkRule := range aclNetwork.Networks { + cidr, err := parseNetwork(networkRule) + if err == nil { + networks = append(networks, cidr) + networksCacheMap[cidr.String()] = cidr + + if cidr.String() != networkRule { + networksCacheMap[networkRule] = cidr + } + } + } + + if _, ok := networksMap[aclNetwork.Name]; len(networks) != 0 && !ok { + networksMap[aclNetwork.Name] = networks + } + } + + return networksMap, networksCacheMap +} + +func parseNetwork(networkRule string) (cidr *net.IPNet, err error) { + if !strings.Contains(networkRule, "/") { + ip := net.ParseIP(networkRule) + if ip.To4() != nil { + _, cidr, err = net.ParseCIDR(networkRule + "/32") + } else { + _, cidr, err = net.ParseCIDR(networkRule + "/128") + } + } else { + _, cidr, err = net.ParseCIDR(networkRule) + } + + return cidr, err +} + +func schemaSubjectsToACL(subjectRules [][]string) (subjects []AccessControlSubjects) { + for _, subjectRule := range subjectRules { + subject := AccessControlSubjects{} + + for _, subjectRuleItem := range subjectRule { + subject.AddSubject(subjectRuleItem) + } + + if len(subject.Subjects) != 0 { + subjects = append(subjects, subject) + } + } + + return subjects +} + +func domainToPrefixSuffix(domain string) (prefix, suffix string) { + parts := strings.Split(domain, ".") + + if len(parts) == 1 { + return "", parts[0] + } + + return parts[0], strings.Join(parts[1:], ".") +} diff --git a/internal/authorization/util_test.go b/internal/authorization/util_test.go new file mode 100644 index 00000000..db72d682 --- /dev/null +++ b/internal/authorization/util_test.go @@ -0,0 +1,184 @@ +package authorization + +import ( + "net" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/authelia/authelia/internal/configuration/schema" +) + +func TestShouldNotParseInvalidSubjects(t *testing.T) { + subjectsSchema := [][]string{{"groups:z"}, {"group:z", "users:b"}} + subjectsACL := schemaSubjectsToACL(subjectsSchema) + + require.Len(t, subjectsACL, 1) + + require.Len(t, subjectsACL[0].Subjects, 1) + + assert.True(t, subjectsACL[0].IsMatch(Subject{Username: "a", Groups: []string{"z"}})) +} + +func TestShouldSplitDomainCorrectly(t *testing.T) { + prefix, suffix := domainToPrefixSuffix("apple.example.com") + + assert.Equal(t, "apple", prefix) + assert.Equal(t, "example.com", suffix) + + prefix, suffix = domainToPrefixSuffix("example") + + assert.Equal(t, "", prefix) + assert.Equal(t, "example", suffix) + + prefix, suffix = domainToPrefixSuffix("example.com") + + assert.Equal(t, "example", prefix) + assert.Equal(t, "com", suffix) +} + +func TestShouldParseRuleNetworks(t *testing.T) { + schemaNetworks := []schema.ACLNetwork{ + { + Name: "desktop", + Networks: []string{ + "10.0.0.1", + }, + }, + { + Name: "lan", + Networks: []string{ + "10.0.0.0/8", + "172.16.0.0/12", + "192.168.0.0/16", + }, + }, + } + + _, firstNetwork, err := net.ParseCIDR("192.168.1.20/32") + require.NoError(t, err) + + networksMap, networksCacheMap := parseSchemaNetworks(schemaNetworks) + + assert.Len(t, networksCacheMap, 5) + + networks := []string{"192.168.1.20", "lan"} + + acl := schemaNetworksToACL(networks, networksMap, networksCacheMap) + + assert.Len(t, networksCacheMap, 7) + + require.Len(t, acl, 4) + assert.Equal(t, firstNetwork, acl[0]) + assert.Equal(t, networksMap["lan"][0], acl[1]) + assert.Equal(t, networksMap["lan"][1], acl[2]) + assert.Equal(t, networksMap["lan"][2], acl[3]) + + // Check they are the same memory address. + assert.True(t, networksMap["lan"][0] == acl[1]) + assert.True(t, networksMap["lan"][1] == acl[2]) + assert.True(t, networksMap["lan"][2] == acl[3]) + + assert.False(t, firstNetwork == acl[0]) +} + +func TestShouldParseACLNetworks(t *testing.T) { + schemaNetworks := []schema.ACLNetwork{ + { + Name: "test", + Networks: []string{ + "10.0.0.1", + }, + }, + { + Name: "second", + Networks: []string{ + "10.0.0.1", + }, + }, + { + Name: "duplicate", + Networks: []string{ + "10.0.0.1", + }, + }, + { + Name: "duplicate", + Networks: []string{ + "10.0.0.1", + }, + }, + { + Name: "ipv6", + Networks: []string{ + "fec0::1", + }, + }, + { + Name: "ipv6net", + Networks: []string{ + "fec0::1/64", + }, + }, + { + Name: "net", + Networks: []string{ + "10.0.0.0/8", + }, + }, + { + Name: "badnet", + Networks: []string{ + "bad/8", + }, + }, + } + + _, firstNetwork, err := net.ParseCIDR("10.0.0.1/32") + require.NoError(t, err) + + _, secondNetwork, err := net.ParseCIDR("10.0.0.0/8") + require.NoError(t, err) + + _, thirdNetwork, err := net.ParseCIDR("fec0::1/64") + require.NoError(t, err) + + _, fourthNetwork, err := net.ParseCIDR("fec0::1/128") + require.NoError(t, err) + + networksMap, networksCacheMap := parseSchemaNetworks(schemaNetworks) + + require.Len(t, networksMap, 6) + require.Contains(t, networksMap, "test") + require.Contains(t, networksMap, "second") + require.Contains(t, networksMap, "duplicate") + require.Contains(t, networksMap, "ipv6") + require.Contains(t, networksMap, "ipv6net") + require.Contains(t, networksMap, "net") + require.Len(t, networksMap["test"], 1) + + require.Len(t, networksCacheMap, 7) + require.Contains(t, networksCacheMap, "10.0.0.1") + require.Contains(t, networksCacheMap, "10.0.0.1/32") + require.Contains(t, networksCacheMap, "10.0.0.1/32") + require.Contains(t, networksCacheMap, "10.0.0.0/8") + require.Contains(t, networksCacheMap, "fec0::1") + require.Contains(t, networksCacheMap, "fec0::1/128") + require.Contains(t, networksCacheMap, "fec0::1/64") + + assert.Equal(t, firstNetwork, networksMap["test"][0]) + assert.Equal(t, secondNetwork, networksMap["net"][0]) + assert.Equal(t, thirdNetwork, networksMap["ipv6net"][0]) + assert.Equal(t, fourthNetwork, networksMap["ipv6"][0]) + + assert.Equal(t, firstNetwork, networksCacheMap["10.0.0.1"]) + assert.Equal(t, firstNetwork, networksCacheMap["10.0.0.1/32"]) + + assert.Equal(t, secondNetwork, networksCacheMap["10.0.0.0/8"]) + + assert.Equal(t, thirdNetwork, networksCacheMap["fec0::1/64"]) + + assert.Equal(t, fourthNetwork, networksCacheMap["fec0::1"]) + assert.Equal(t, fourthNetwork, networksCacheMap["fec0::1/128"]) +} diff --git a/internal/configuration/schema/access_control.go b/internal/configuration/schema/access_control.go index 9000fe1d..e2d5bbc7 100644 --- a/internal/configuration/schema/access_control.go +++ b/internal/configuration/schema/access_control.go @@ -9,7 +9,7 @@ type AccessControlConfiguration struct { // ACLNetwork represents one ACL network group entry; "weak" coerces a single value into slice. type ACLNetwork struct { - Name []string `mapstructure:"name,weak"` + Name string `mapstructure:"name"` Networks []string `mapstructure:"networks"` } @@ -20,16 +20,17 @@ type ACLRule struct { Subjects [][]string `mapstructure:"subject,weak"` Networks []string `mapstructure:"networks"` Resources []string `mapstructure:"resources"` + Methods []string `mapstructure:"methods"` } // DefaultACLNetwork represents the default configuration related to access control network group configuration. var DefaultACLNetwork = []ACLNetwork{ { - Name: []string{"localhost"}, + Name: "localhost", Networks: []string{"127.0.0.1"}, }, { - Name: []string{"internal"}, + Name: "internal", Networks: []string{"10.0.0.0/8"}, }, } diff --git a/internal/configuration/validator/access_control.go b/internal/configuration/validator/access_control.go index 28132244..8c770f3d 100644 --- a/internal/configuration/validator/access_control.go +++ b/internal/configuration/validator/access_control.go @@ -11,25 +11,25 @@ import ( ) // IsPolicyValid check if policy is valid. -func IsPolicyValid(policy string) bool { - return policy == denyPolicy || policy == "one_factor" || policy == "two_factor" || policy == "bypass" +func IsPolicyValid(policy string) (isValid bool) { + return policy == denyPolicy || policy == "one_factor" || policy == "two_factor" || policy == bypassPolicy } // IsResourceValid check if a resource is valid. -func IsResourceValid(resource string) error { - _, err := regexp.Compile(resource) +func IsResourceValid(resource string) (err error) { + _, err = regexp.Compile(resource) return err } // IsSubjectValid check if a subject is valid. -func IsSubjectValid(subject string) bool { +func IsSubjectValid(subject string) (isValid bool) { return subject == "" || strings.HasPrefix(subject, "user:") || strings.HasPrefix(subject, "group:") } // IsNetworkGroupValid check if a network group is valid. func IsNetworkGroupValid(configuration schema.AccessControlConfiguration, network string) bool { for _, networks := range configuration.Networks { - if !utils.IsStringInSlice(network, networks.Name) { + if network != networks.Name { continue } else { return true @@ -40,7 +40,7 @@ func IsNetworkGroupValid(configuration schema.AccessControlConfiguration, networ } // IsNetworkValid check if a network is valid. -func IsNetworkValid(network string) bool { +func IsNetworkValid(network string) (isValid bool) { if net.ParseIP(network) == nil { _, _, err := net.ParseCIDR(network) return err == nil @@ -77,26 +77,52 @@ func ValidateRules(configuration schema.AccessControlConfiguration, validator *s validator.Push(fmt.Errorf("Policy [%s] for domain: %s is invalid, a policy must either be 'deny', 'two_factor', 'one_factor' or 'bypass'", r.Policy, r.Domains)) } - for _, network := range r.Networks { - if !IsNetworkValid(network) { - if !IsNetworkGroupValid(configuration, network) { - validator.Push(fmt.Errorf("Network %s for domain: %s is not a valid network or network group", r.Networks, r.Domains)) - } - } - } + validateNetworks(r, configuration, validator) - for _, resource := range r.Resources { - if err := IsResourceValid(resource); err != nil { - validator.Push(fmt.Errorf("Resource %s for domain: %s is invalid, %s", r.Resources, r.Domains, err)) - } - } + validateResources(r, validator) - for _, subjectRule := range r.Subjects { - for _, subject := range subjectRule { - if !IsSubjectValid(subject) { - validator.Push(fmt.Errorf("Subject %s for domain: %s must start with 'user:' or 'group:'", subjectRule, r.Domains)) - } + validateSubjects(r, validator) + + validateMethods(r, validator) + + if r.Policy == bypassPolicy && len(r.Subjects) != 0 { + validator.Push(fmt.Errorf(errAccessControlInvalidPolicyWithSubjects, r.Domains, r.Subjects)) + } + } +} + +func validateNetworks(r schema.ACLRule, configuration schema.AccessControlConfiguration, validator *schema.StructValidator) { + for _, network := range r.Networks { + if !IsNetworkValid(network) { + if !IsNetworkGroupValid(configuration, network) { + validator.Push(fmt.Errorf("Network %s for domain: %s is not a valid network or network group", r.Networks, r.Domains)) } } } } + +func validateResources(r schema.ACLRule, validator *schema.StructValidator) { + for _, resource := range r.Resources { + if err := IsResourceValid(resource); err != nil { + validator.Push(fmt.Errorf("Resource %s for domain: %s is invalid, %s", r.Resources, r.Domains, err)) + } + } +} + +func validateSubjects(r schema.ACLRule, validator *schema.StructValidator) { + for _, subjectRule := range r.Subjects { + for _, subject := range subjectRule { + if !IsSubjectValid(subject) { + validator.Push(fmt.Errorf("Subject %s for domain: %s is invalid, must start with 'user:' or 'group:'", subjectRule, r.Domains)) + } + } + } +} + +func validateMethods(r schema.ACLRule, validator *schema.StructValidator) { + for _, method := range r.Methods { + if !utils.IsStringInSliceFold(method, validRequestMethods) { + validator.Push(fmt.Errorf("Method %s for domain: %s is invalid, must be one of the following methods: %s", method, r.Domains, strings.Join(validRequestMethods, ", "))) + } + } +} diff --git a/internal/configuration/validator/access_control_test.go b/internal/configuration/validator/access_control_test.go index c57e46a1..7105680b 100644 --- a/internal/configuration/validator/access_control_test.go +++ b/internal/configuration/validator/access_control_test.go @@ -1,6 +1,7 @@ package validator import ( + "fmt" "testing" "github.com/stretchr/testify/suite" @@ -42,7 +43,7 @@ func (suite *AccessControl) TestShouldRaiseErrorInvalidDefaultPolicy() { func (suite *AccessControl) TestShouldRaiseErrorInvalidNetworkGroupNetwork() { suite.configuration.Networks = []schema.ACLNetwork{ { - Name: []string{"internal"}, + Name: "internal", Networks: []string{"abc.def.ghi.jkl"}, }, } @@ -52,7 +53,7 @@ func (suite *AccessControl) TestShouldRaiseErrorInvalidNetworkGroupNetwork() { suite.Assert().False(suite.validator.HasWarnings()) suite.Require().Len(suite.validator.Errors(), 1) - suite.Assert().EqualError(suite.validator.Errors()[0], "Network [abc.def.ghi.jkl] from network group: [internal] must be a valid IP or CIDR") + suite.Assert().EqualError(suite.validator.Errors()[0], "Network [abc.def.ghi.jkl] from network group: internal must be a valid IP or CIDR") } func (suite *AccessControl) TestShouldRaiseErrorNoRulesDefined() { @@ -100,6 +101,23 @@ func (suite *AccessControl) TestShouldRaiseErrorInvalidNetwork() { suite.Assert().EqualError(suite.validator.Errors()[0], "Network [abc.def.ghi.jkl/32] for domain: [public.example.com] is not a valid network or network group") } +func (suite *AccessControl) TestShouldRaiseErrorInvalidMethod() { + suite.configuration.Rules = []schema.ACLRule{ + { + Domains: []string{"public.example.com"}, + Policy: "bypass", + Methods: []string{"GET", "HOP"}, + }, + } + + ValidateRules(suite.configuration, suite.validator) + + suite.Assert().False(suite.validator.HasWarnings()) + suite.Require().Len(suite.validator.Errors(), 1) + + suite.Assert().EqualError(suite.validator.Errors()[0], "Method HOP for domain: [public.example.com] is invalid, must be one of the following methods: GET, HEAD, POST, PUT, PATCH, DELETE, TRACE, CONNECT, OPTIONS") +} + func (suite *AccessControl) TestShouldRaiseErrorInvalidResource() { suite.configuration.Rules = []schema.ACLRule{ { @@ -118,20 +136,23 @@ func (suite *AccessControl) TestShouldRaiseErrorInvalidResource() { } func (suite *AccessControl) TestShouldRaiseErrorInvalidSubject() { + domains := []string{"public.example.com"} + subjects := [][]string{{"invalid"}} suite.configuration.Rules = []schema.ACLRule{ { - Domains: []string{"public.example.com"}, + Domains: domains, Policy: "bypass", - Subjects: [][]string{{"invalid"}}, + Subjects: subjects, }, } ValidateRules(suite.configuration, suite.validator) - suite.Assert().False(suite.validator.HasWarnings()) - suite.Require().Len(suite.validator.Errors(), 1) + suite.Require().Len(suite.validator.Warnings(), 0) + suite.Require().Len(suite.validator.Errors(), 2) - suite.Assert().EqualError(suite.validator.Errors()[0], "Subject [invalid] for domain: [public.example.com] must start with 'user:' or 'group:'") + suite.Assert().EqualError(suite.validator.Errors()[0], "Subject [invalid] for domain: [public.example.com] is invalid, must start with 'user:' or 'group:'") + suite.Assert().EqualError(suite.validator.Errors()[1], fmt.Sprintf(errAccessControlInvalidPolicyWithSubjects, domains, subjects)) } func TestAccessControl(t *testing.T) { diff --git a/internal/configuration/validator/authentication.go b/internal/configuration/validator/authentication.go index 98a07043..a0a6115e 100644 --- a/internal/configuration/validator/authentication.go +++ b/internal/configuration/validator/authentication.go @@ -195,10 +195,6 @@ func validateLdapAuthenticationBackend(configuration *schema.LDAPAuthenticationB } else if !strings.HasPrefix(configuration.GroupsFilter, "(") || !strings.HasSuffix(configuration.GroupsFilter, ")") { validator.Push(errors.New("The groups filter should contain enclosing parenthesis. For instance cn={input} should be (cn={input})")) } - - if configuration.UsernameAttribute == "" { - validator.Push(errors.New("Please provide a username attribute with `username_attribute`")) - } } func setDefaultImplementationActiveDirectoryLdapAuthenticationBackend(configuration *schema.LDAPAuthenticationBackendConfiguration) { diff --git a/internal/configuration/validator/authentication_test.go b/internal/configuration/validator/authentication_test.go index cf01525f..574a5b04 100644 --- a/internal/configuration/validator/authentication_test.go +++ b/internal/configuration/validator/authentication_test.go @@ -214,6 +214,18 @@ func (suite *LdapAuthenticationBackendSuite) TestShouldValidateCompleteConfigura suite.Assert().False(suite.validator.HasErrors()) } +func (suite *LdapAuthenticationBackendSuite) TestShouldValidateDefaultImplementationAndUsernameAttribute() { + suite.configuration.Ldap.Implementation = "" + suite.configuration.Ldap.UsernameAttribute = "" + ValidateAuthenticationBackend(&suite.configuration, suite.validator) + + suite.Assert().Equal(schema.LDAPImplementationCustom, suite.configuration.Ldap.Implementation) + + suite.Assert().Equal(suite.configuration.Ldap.UsernameAttribute, schema.DefaultLDAPAuthenticationBackendConfiguration.UsernameAttribute) + suite.Assert().False(suite.validator.HasWarnings()) + suite.Assert().False(suite.validator.HasErrors()) +} + func (suite *LdapAuthenticationBackendSuite) TestShouldRaiseErrorWhenImplementationIsInvalidMSAD() { suite.configuration.Ldap.Implementation = "masd" diff --git a/internal/configuration/validator/const.go b/internal/configuration/validator/const.go index 87242ab9..ca990b22 100644 --- a/internal/configuration/validator/const.go +++ b/internal/configuration/validator/const.go @@ -1,5 +1,7 @@ package validator +var validRequestMethods = []string{"GET", "HEAD", "POST", "PUT", "PATCH", "DELETE", "TRACE", "CONNECT", "OPTIONS"} + var validKeys = []string{ // Root Keys. "host", @@ -170,6 +172,7 @@ var specificErrorKeys = map[string]string{ } const denyPolicy = "deny" +const bypassPolicy = "bypass" const argon2id = "argon2id" const sha512 = "sha512" @@ -187,3 +190,5 @@ const testLDAPUser = "user" const testModeDisabled = "disable" const testTLSCert = "/tmp/cert.pem" const testTLSKey = "/tmp/key.pem" + +const errAccessControlInvalidPolicyWithSubjects = "Policy [bypass] for domain %s with subjects %s is invalid. It is not supported to configure both policy bypass and subjects. For more information see: https://www.authelia.com/docs/configuration/access-control.html#combining-subjects-and-the-bypass-policy" diff --git a/internal/configuration/validator/regulation_test.go b/internal/configuration/validator/regulation_test.go index 18915793..f30228df 100644 --- a/internal/configuration/validator/regulation_test.go +++ b/internal/configuration/validator/regulation_test.go @@ -38,8 +38,22 @@ func TestShouldRaiseErrorWhenFindTimeLessThanBanTime(t *testing.T) { config := newDefaultRegulationConfig() config.FindTime = "1m" config.BanTime = "10s" + ValidateRegulation(&config, validator) assert.Len(t, validator.Errors(), 1) assert.EqualError(t, validator.Errors()[0], "find_time cannot be greater than ban_time") } + +func TestShouldRaiseErrorOnBadDurationStrings(t *testing.T) { + validator := schema.NewStructValidator() + config := newDefaultRegulationConfig() + config.FindTime = "a year" + config.BanTime = "forever" + + ValidateRegulation(&config, validator) + + assert.Len(t, validator.Errors(), 2) + assert.EqualError(t, validator.Errors()[0], "Error occurred parsing regulation find_time string: Could not convert the input string of a year into a duration") + assert.EqualError(t, validator.Errors()[1], "Error occurred parsing regulation ban_time string: Could not convert the input string of forever into a duration") +} diff --git a/internal/handlers/handler_firstfactor.go b/internal/handlers/handler_firstfactor.go index d33dac2c..56201a8f 100644 --- a/internal/handlers/handler_firstfactor.go +++ b/internal/handlers/handler_firstfactor.go @@ -188,6 +188,6 @@ func FirstFactorPost(msInitialDelay time.Duration, delayEnabled bool) middleware successful = true - Handle1FAResponse(ctx, bodyJSON.TargetURL, userSession.Username, userSession.Groups) + Handle1FAResponse(ctx, bodyJSON.TargetURL, bodyJSON.RequestMethod, userSession.Username, userSession.Groups) } } diff --git a/internal/handlers/handler_firstfactor_test.go b/internal/handlers/handler_firstfactor_test.go index 4e10b4a6..b78c0de7 100644 --- a/internal/handlers/handler_firstfactor_test.go +++ b/internal/handlers/handler_firstfactor_test.go @@ -210,6 +210,7 @@ func (s *FirstFactorSuite) TestShouldAuthenticateUserWithRememberMeUnchecked() { s.mock.Ctx.Request.SetBodyString(`{ "username": "test", "password": "hello", + "requestMethod": "GET", "keepMeLoggedIn": false }`) FirstFactorPost(0, false)(s.mock.Ctx) @@ -253,6 +254,7 @@ func (s *FirstFactorSuite) TestShouldSaveUsernameFromAuthenticationBackendInSess s.mock.Ctx.Request.SetBodyString(`{ "username": "test", "password": "hello", + "requestMethod": "GET", "keepMeLoggedIn": true }`) FirstFactorPost(0, false)(s.mock.Ctx) @@ -323,6 +325,7 @@ func (s *FirstFactorRedirectionSuite) TestShouldRedirectToDefaultURLWhenNoTarget s.mock.Ctx.Request.SetBodyString(`{ "username": "test", "password": "hello", + "requestMethod": "GET", "keepMeLoggedIn": false }`) FirstFactorPost(0, false)(s.mock.Ctx) @@ -341,6 +344,7 @@ func (s *FirstFactorRedirectionSuite) TestShouldRedirectToDefaultURLWhenURLIsUns s.mock.Ctx.Request.SetBodyString(`{ "username": "test", "password": "hello", + "requestMethod": "GET", "keepMeLoggedIn": false, "targetURL": "http://notsafe.local" }`) @@ -362,6 +366,7 @@ func (s *FirstFactorRedirectionSuite) TestShouldReply200WhenNoTargetURLProvidedA s.mock.Ctx.Request.SetBodyString(`{ "username": "test", "password": "hello", + "requestMethod": "GET", "keepMeLoggedIn": false }`) @@ -392,6 +397,7 @@ func (s *FirstFactorRedirectionSuite) TestShouldReply200WhenUnsafeTargetURLProvi s.mock.Ctx.Request.SetBodyString(`{ "username": "test", "password": "hello", + "requestMethod": "GET", "keepMeLoggedIn": false }`) diff --git a/internal/handlers/handler_verify.go b/internal/handlers/handler_verify.go index d99af1c3..37ccfe48 100644 --- a/internal/handlers/handler_verify.go +++ b/internal/handlers/handler_verify.go @@ -98,12 +98,14 @@ func parseBasicAuth(header, auth string) (username, password string, err error) // isTargetURLAuthorized check whether the given user is authorized to access the resource. func isTargetURLAuthorized(authorizer *authorization.Authorizer, targetURL url.URL, - username string, userGroups []string, clientIP net.IP, authLevel authentication.Level) authorizationMatching { - level := authorizer.GetRequiredLevel(authorization.Subject{ - Username: username, - Groups: userGroups, - IP: clientIP, - }, targetURL) + username string, userGroups []string, clientIP net.IP, method []byte, authLevel authentication.Level) authorizationMatching { + level := authorizer.GetRequiredLevel( + authorization.Subject{ + Username: username, + Groups: userGroups, + IP: clientIP, + }, + authorization.NewObjectRaw(&targetURL, method)) switch { case level == authorization.Bypass: @@ -233,7 +235,7 @@ func verifySessionCookie(ctx *middlewares.AutheliaCtx, targetURL *url.URL, userS return userSession.Username, userSession.DisplayName, userSession.Groups, userSession.Emails, userSession.AuthenticationLevel, nil } -func handleUnauthorized(ctx *middlewares.AutheliaCtx, targetURL fmt.Stringer, isBasicAuth bool, username string) { +func handleUnauthorized(ctx *middlewares.AutheliaCtx, targetURL fmt.Stringer, isBasicAuth bool, username string, method []byte) { if isBasicAuth { ctx.Logger.Infof("Access to %s is not authorized to user %s, sending 401 response with basic auth header", targetURL.String(), username) ctx.ReplyUnauthorized() @@ -246,17 +248,28 @@ func handleUnauthorized(ctx *middlewares.AutheliaCtx, targetURL fmt.Stringer, is // endpoint to provide the URL of the login portal. The target URL of the user // is computed from X-Forwarded-* headers or X-Original-URL. rd := string(ctx.QueryArgs().Peek("rd")) + rm := string(method) + + friendlyMethod := "unknown" + + if rm != "" { + friendlyMethod = rm + } + if rd != "" { - redirectionURL := fmt.Sprintf("%s?rd=%s", rd, url.QueryEscape(targetURL.String())) - if strings.Contains(redirectionURL, "/%23/") { - ctx.Logger.Warn("Characters /%23/ have been detected in redirection URL. This is not needed anymore, please strip it") + redirectionURL := "" + + if rm != "" { + redirectionURL = fmt.Sprintf("%s?rd=%s&rm=%s", rd, url.QueryEscape(targetURL.String()), rm) + } else { + redirectionURL = fmt.Sprintf("%s?rd=%s", rd, url.QueryEscape(targetURL.String())) } - ctx.Logger.Infof("Access to %s is not authorized to user %s, redirecting to %s", targetURL.String(), username, redirectionURL) + ctx.Logger.Infof("Access to %s (method %s) is not authorized to user %s, redirecting to %s", targetURL.String(), friendlyMethod, username, redirectionURL) ctx.Redirect(redirectionURL, 302) ctx.SetBodyString(fmt.Sprintf("Found. Redirecting to %s", redirectionURL)) } else { - ctx.Logger.Infof("Access to %s is not authorized to user %s, sending 401 response", targetURL.String(), username) + ctx.Logger.Infof("Access to %s (method %s) is not authorized to user %s, sending 401 response", targetURL.String(), friendlyMethod, username) ctx.ReplyUnauthorized() } } @@ -475,6 +488,8 @@ func VerifyGet(cfg schema.AuthenticationBackendConfiguration) middlewares.Reques isBasicAuth, username, name, groups, emails, authLevel, err := verifyAuth(ctx, targetURL, refreshProfile, refreshProfileInterval) + method := ctx.XForwardedMethod() + if err != nil { ctx.Logger.Error(fmt.Sprintf("Error caught when verifying user authorization: %s", err)) @@ -483,20 +498,20 @@ func VerifyGet(cfg schema.AuthenticationBackendConfiguration) middlewares.Reques return } - handleUnauthorized(ctx, targetURL, isBasicAuth, username) + handleUnauthorized(ctx, targetURL, isBasicAuth, username, method) return } - authorization := isTargetURLAuthorized(ctx.Providers.Authorizer, *targetURL, username, - groups, ctx.RemoteIP(), authLevel) + authorized := isTargetURLAuthorized(ctx.Providers.Authorizer, *targetURL, username, + groups, ctx.RemoteIP(), method, authLevel) - switch authorization { + switch authorized { case Forbidden: ctx.Logger.Infof("Access to %s is forbidden to user %s", targetURL.String(), username) ctx.ReplyForbidden() case NotAuthorized: - handleUnauthorized(ctx, targetURL, isBasicAuth, username) + handleUnauthorized(ctx, targetURL, isBasicAuth, username, method) case Authorized: setForwardedHeaders(&ctx.Response.Header, username, name, groups, emails) } diff --git a/internal/handlers/handler_verify_test.go b/internal/handlers/handler_verify_test.go index eab127de..9f9c61a2 100644 --- a/internal/handlers/handler_verify_test.go +++ b/internal/handlers/handler_verify_test.go @@ -196,7 +196,7 @@ func TestShouldCheckAuthorizationMatching(t *testing.T) { username = testUsername } - matching := isTargetURLAuthorized(authorizer, *url, username, []string{}, net.ParseIP("127.0.0.1"), rule.AuthLevel) + matching := isTargetURLAuthorized(authorizer, *url, username, []string{}, net.ParseIP("127.0.0.1"), []byte("GET"), rule.AuthLevel) assert.Equal(t, rule.ExpectedMatching, matching, "policy=%s, authLevel=%v, expected=%v, actual=%v", rule.Policy, rule.AuthLevel, rule.ExpectedMatching, matching) } @@ -762,10 +762,11 @@ func TestShouldRedirectWhenSessionInactiveForTooLongAndRDParamProvided(t *testin mock.Ctx.QueryArgs().Add("rd", "https://login.example.com") mock.Ctx.Request.Header.Set("X-Original-URL", "https://two-factor.example.com") + mock.Ctx.Request.Header.Set("X-Forwarded-Method", "GET") VerifyGet(verifyGetCfg)(mock.Ctx) - assert.Equal(t, "Found. Redirecting to https://login.example.com?rd=https%3A%2F%2Ftwo-factor.example.com", + assert.Equal(t, "Found. Redirecting to https://login.example.com?rd=https%3A%2F%2Ftwo-factor.example.com&rm=GET", string(mock.Ctx.Response.Body())) assert.Equal(t, 302, mock.Ctx.Response.StatusCode()) diff --git a/internal/handlers/response.go b/internal/handlers/response.go index e85b9046..ebff9dbb 100644 --- a/internal/handlers/response.go +++ b/internal/handlers/response.go @@ -12,7 +12,7 @@ import ( ) // Handle1FAResponse handle the redirection upon 1FA authentication. -func Handle1FAResponse(ctx *middlewares.AutheliaCtx, targetURI string, username string, groups []string) { +func Handle1FAResponse(ctx *middlewares.AutheliaCtx, targetURI, requestMethod string, username string, groups []string) { if targetURI == "" { if !ctx.Providers.Authorizer.IsSecondFactorEnabled() && ctx.Configuration.DefaultRedirectionURL != "" { err := ctx.SetJSONBody(redirectResponse{Redirect: ctx.Configuration.DefaultRedirectionURL}) @@ -32,11 +32,13 @@ func Handle1FAResponse(ctx *middlewares.AutheliaCtx, targetURI string, username return } - requiredLevel := ctx.Providers.Authorizer.GetRequiredLevel(authorization.Subject{ - Username: username, - Groups: groups, - IP: ctx.RemoteIP(), - }, *targetURL) + requiredLevel := ctx.Providers.Authorizer.GetRequiredLevel( + authorization.Subject{ + Username: username, + Groups: groups, + IP: ctx.RemoteIP(), + }, + authorization.NewObject(targetURL, requestMethod)) ctx.Logger.Debugf("Required level for the URL %s is %d", targetURI, requiredLevel) diff --git a/internal/handlers/types.go b/internal/handlers/types.go index bcd21f62..85283d26 100644 --- a/internal/handlers/types.go +++ b/internal/handlers/types.go @@ -42,14 +42,15 @@ type signDuoRequestBody struct { TargetURL string `json:"targetURL"` } -// firstFactorBody represents the JSON body received by the endpoint. +// firstFactorRequestBody represents the JSON body received by the endpoint. type firstFactorRequestBody struct { - Username string `json:"username" valid:"required"` - Password string `json:"password" valid:"required"` - TargetURL string `json:"targetURL"` - // Cannot require this field because of https://github.com/asaskevich/govalidator/pull/329 + Username string `json:"username" valid:"required"` + Password string `json:"password" valid:"required"` + TargetURL string `json:"targetURL"` + RequestMethod string `json:"requestMethod"` + KeepMeLoggedIn *bool `json:"keepMeLoggedIn"` + // KeepMeLoggedIn: Cannot require this field because of https://github.com/asaskevich/govalidator/pull/329 // TODO(c.michaud): add required validation once the above PR is merged. - KeepMeLoggedIn *bool `json:"keepMeLoggedIn"` } // redirectResponse represent the response sent by the first factor endpoint diff --git a/internal/middlewares/authelia_context.go b/internal/middlewares/authelia_context.go index c4f8a227..1ba162e4 100644 --- a/internal/middlewares/authelia_context.go +++ b/internal/middlewares/authelia_context.go @@ -87,22 +87,27 @@ func (c *AutheliaCtx) ReplyForbidden() { c.RequestCtx.Error(fasthttp.StatusMessage(fasthttp.StatusForbidden), fasthttp.StatusForbidden) } -// XForwardedProto return the content of the header X-Forwarded-Proto. +// XForwardedProto return the content of the X-Forwarded-Proto header. func (c *AutheliaCtx) XForwardedProto() []byte { return c.RequestCtx.Request.Header.Peek(xForwardedProtoHeader) } -// XForwardedHost return the content of the header X-Forwarded-Host. +// XForwardedMethod return the content of the X-Forwarded-Method header. +func (c *AutheliaCtx) XForwardedMethod() []byte { + return c.RequestCtx.Request.Header.Peek(xForwardedMethodHeader) +} + +// XForwardedHost return the content of the X-Forwarded-Host header. func (c *AutheliaCtx) XForwardedHost() []byte { return c.RequestCtx.Request.Header.Peek(xForwardedHostHeader) } -// XForwardedURI return the content of the header X-Forwarded-URI. +// XForwardedURI return the content of the X-Forwarded-URI header. func (c *AutheliaCtx) XForwardedURI() []byte { return c.RequestCtx.Request.Header.Peek(xForwardedURIHeader) } -// XOriginalURL return the content of the header X-Original-URL. +// XOriginalURL return the content of the X-Original-URL header. func (c *AutheliaCtx) XOriginalURL() []byte { return c.RequestCtx.Request.Header.Peek(xOriginalURLHeader) } diff --git a/internal/middlewares/const.go b/internal/middlewares/const.go index 7565a963..e15cee80 100644 --- a/internal/middlewares/const.go +++ b/internal/middlewares/const.go @@ -3,6 +3,7 @@ package middlewares const jwtIssuer = "Authelia" const xForwardedProtoHeader = "X-Forwarded-Proto" +const xForwardedMethodHeader = "X-Forwarded-Method" const xForwardedHostHeader = "X-Forwarded-Host" const xForwardedURIHeader = "X-Forwarded-URI" diff --git a/internal/suites/Standalone/configuration.yml b/internal/suites/Standalone/configuration.yml index b22fe06e..3555ac81 100644 --- a/internal/suites/Standalone/configuration.yml +++ b/internal/suites/Standalone/configuration.yml @@ -35,6 +35,11 @@ access_control: - domain: public.example.com policy: bypass + - domain: secure.example.com + policy: bypass + methods: + - OPTIONS + - domain: secure.example.com policy: two_factor diff --git a/internal/suites/example/compose/haproxy/haproxy.cfg b/internal/suites/example/compose/haproxy/haproxy.cfg index edc9ec07..bf682ba0 100644 --- a/internal/suites/example/compose/haproxy/haproxy.cfg +++ b/internal/suites/example/compose/haproxy/haproxy.cfg @@ -28,6 +28,16 @@ frontend fe_http http-request set-var(req.scheme) str(https) if { ssl_fc } http-request set-var(req.scheme) str(http) if !{ ssl_fc } http-request set-var(req.questionmark) str(?) if { query -m found } + http-request set-var(req.method) str(CONNECT) if { method CONNECT } + http-request set-var(req.method) str(GET) if { method GET } + http-request set-var(req.method) str(HEAD) if { method HEAD } + http-request set-var(req.method) str(OPTIONS) if { method OPTIONS } + http-request set-var(req.method) str(POST) if { method POST } + http-request set-var(req.method) str(TRACE) if { method TRACE } + http-request set-var(req.method) str(PUT) if { method PUT } + http-request set-var(req.method) str(PATCH) if { method PATCH } + http-request set-var(req.method) str(DELETE) if { method DELETE } + http-request set-header X-Forwarded-Method %[var(req.method)] http-request set-header X-Real-IP %[src] http-request set-header X-Forwarded-Proto %[var(req.scheme)] diff --git a/internal/suites/example/compose/nginx/portal/nginx.conf b/internal/suites/example/compose/nginx/portal/nginx.conf index 5b8a09b4..cd1084d8 100644 --- a/internal/suites/example/compose/nginx/portal/nginx.conf +++ b/internal/suites/example/compose/nginx/portal/nginx.conf @@ -152,10 +152,10 @@ http { # See https://expressjs.com/en/guide/behind-proxies.html proxy_set_header X-Original-URL $scheme://$http_host$request_uri; + proxy_set_header X-Forwarded-Method $request_method; proxy_set_header X-Forwarded-Proto $scheme; proxy_set_header X-Forwarded-Host $http_host; proxy_set_header X-Forwarded-URI $request_uri; - proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; # Authelia can receive Proxy-Authorization to authenticate however most of the clients diff --git a/internal/suites/suite_standalone_test.go b/internal/suites/suite_standalone_test.go index edd4881a..2901d49c 100644 --- a/internal/suites/suite_standalone_test.go +++ b/internal/suites/suite_standalone_test.go @@ -103,6 +103,31 @@ func NewStandaloneSuite() *StandaloneSuite { return &StandaloneSuite{} } +func (s *StandaloneSuite) TestShouldRespectMethodsACL() { + req, err := http.NewRequest("GET", fmt.Sprintf("%s/api/verify?rd=%s", AutheliaBaseURL, GetLoginBaseURL()), nil) + s.Assert().NoError(err) + req.Header.Set("X-Forwarded-Method", "GET") + req.Header.Set("X-Forwarded-Proto", "https") + req.Header.Set("X-Forwarded-Host", fmt.Sprintf("secure.%s", BaseDomain)) + req.Header.Set("X-Forwarded-URI", "/") + + client := NewHTTPClient() + res, err := client.Do(req) + s.Assert().NoError(err) + s.Assert().Equal(res.StatusCode, 302) + body, err := ioutil.ReadAll(res.Body) + s.Assert().NoError(err) + + urlEncodedAdminURL := url.QueryEscape(SecureBaseURL + "/") + s.Assert().Equal(fmt.Sprintf("Found. Redirecting to %s?rd=%s&rm=GET", GetLoginBaseURL(), urlEncodedAdminURL), string(body)) + + req.Header.Set("X-Forwarded-Method", "OPTIONS") + + res, err = client.Do(req) + s.Assert().NoError(err) + s.Assert().Equal(res.StatusCode, 200) +} + // Standard case using nginx. func (s *StandaloneSuite) TestShouldVerifyAPIVerifyUnauthorize() { req, err := http.NewRequest("GET", fmt.Sprintf("%s/api/verify", AutheliaBaseURL), nil) diff --git a/internal/utils/certificates.go b/internal/utils/certificates.go index 251528e4..976ff983 100644 --- a/internal/utils/certificates.go +++ b/internal/utils/certificates.go @@ -32,7 +32,7 @@ func NewTLSConfig(config *schema.TLSConfig, defaultMinVersion uint16, certPool * func NewX509CertPool(directory string, config *schema.Configuration) (certPool *x509.CertPool, errors []error, nonFatalErrors []error) { certPool, err := x509.SystemCertPool() if err != nil { - nonFatalErrors = append(nonFatalErrors, fmt.Errorf("could not load system certificate pool which may result in untruested certificate issues: %v", err)) + nonFatalErrors = append(nonFatalErrors, fmt.Errorf("could not load system certificate pool which may result in untrusted certificate issues: %v", err)) certPool = x509.NewCertPool() } diff --git a/internal/utils/strings.go b/internal/utils/strings.go index abe9408e..a1e07e53 100644 --- a/internal/utils/strings.go +++ b/internal/utils/strings.go @@ -18,9 +18,9 @@ func IsStringAlphaNumeric(input string) bool { return true } -// IsStringInSlice checks if a single string is in an array of strings. -func IsStringInSlice(a string, list []string) (inSlice bool) { - for _, b := range list { +// IsStringInSlice checks if a single string is in a slice of strings. +func IsStringInSlice(a string, slice []string) (inSlice bool) { + for _, b := range slice { if b == a { return true } @@ -29,6 +29,17 @@ func IsStringInSlice(a string, list []string) (inSlice bool) { return false } +// IsStringInSliceFold checks if a single string is in a slice of strings but uses strings.EqualFold to compare them. +func IsStringInSliceFold(a string, slice []string) (inSlice bool) { + for _, b := range slice { + if strings.EqualFold(b, a) { + return true + } + } + + return false +} + // IsStringInSliceContains checks if a single string is in an array of strings. func IsStringInSliceContains(a string, list []string) (inSlice bool) { for _, b := range list { diff --git a/internal/utils/strings_test.go b/internal/utils/strings_test.go index ad73893d..793d42c9 100644 --- a/internal/utils/strings_test.go +++ b/internal/utils/strings_test.go @@ -9,7 +9,9 @@ import ( func TestShouldSplitIntoEvenStringsOfFour(t *testing.T) { input := testStringInput + arrayOfStrings := SliceString(input, 4) + assert.Equal(t, len(arrayOfStrings), 3) assert.Equal(t, "abcd", arrayOfStrings[0]) assert.Equal(t, "efgh", arrayOfStrings[1]) @@ -18,7 +20,9 @@ func TestShouldSplitIntoEvenStringsOfFour(t *testing.T) { func TestShouldSplitIntoEvenStringsOfOne(t *testing.T) { input := testStringInput + arrayOfStrings := SliceString(input, 1) + assert.Equal(t, 12, len(arrayOfStrings)) assert.Equal(t, "a", arrayOfStrings[0]) assert.Equal(t, "b", arrayOfStrings[1]) @@ -29,7 +33,9 @@ func TestShouldSplitIntoEvenStringsOfOne(t *testing.T) { func TestShouldSplitIntoUnevenStringsOfFour(t *testing.T) { input := testStringInput + "m" + arrayOfStrings := SliceString(input, 4) + assert.Equal(t, len(arrayOfStrings), 4) assert.Equal(t, "abcd", arrayOfStrings[0]) assert.Equal(t, "efgh", arrayOfStrings[1]) @@ -40,7 +46,9 @@ func TestShouldSplitIntoUnevenStringsOfFour(t *testing.T) { func TestShouldFindSliceDifferencesDelta(t *testing.T) { before := []string{"abc", "onetwothree"} after := []string{"abc", "xyz"} + added, removed := StringSlicesDelta(before, after) + require.Len(t, added, 1) require.Len(t, removed, 1) assert.Equal(t, "onetwothree", removed[0]) @@ -50,7 +58,9 @@ func TestShouldFindSliceDifferencesDelta(t *testing.T) { func TestShouldNotFindSliceDifferencesDelta(t *testing.T) { before := []string{"abc", "onetwothree"} after := []string{"abc", "onetwothree"} + added, removed := StringSlicesDelta(before, after) + require.Len(t, added, 0) require.Len(t, removed, 0) } @@ -58,34 +68,52 @@ func TestShouldNotFindSliceDifferencesDelta(t *testing.T) { func TestShouldFindSliceDifferences(t *testing.T) { a := []string{"abc", "onetwothree"} b := []string{"abc", "xyz"} - diff := IsStringSlicesDifferent(a, b) - assert.True(t, diff) + + assert.True(t, IsStringSlicesDifferent(a, b)) } func TestShouldNotFindSliceDifferences(t *testing.T) { a := []string{"abc", "onetwothree"} b := []string{"abc", "onetwothree"} - diff := IsStringSlicesDifferent(a, b) - assert.False(t, diff) + + assert.False(t, IsStringSlicesDifferent(a, b)) } func TestShouldFindSliceDifferenceWhenDifferentLength(t *testing.T) { a := []string{"abc", "onetwothree"} b := []string{"abc", "onetwothree", "more"} - diff := IsStringSlicesDifferent(a, b) - assert.True(t, diff) + + assert.True(t, IsStringSlicesDifferent(a, b)) } func TestShouldFindStringInSliceContains(t *testing.T) { a := "abc" - b := []string{"abc", "onetwothree"} - s := IsStringInSliceContains(a, b) - assert.True(t, s) + slice := []string{"abc", "onetwothree"} + + assert.True(t, IsStringInSliceContains(a, slice)) } func TestShouldNotFindStringInSliceContains(t *testing.T) { a := "xyz" - b := []string{"abc", "onetwothree"} - s := IsStringInSliceContains(a, b) - assert.False(t, s) + slice := []string{"abc", "onetwothree"} + + assert.False(t, IsStringInSliceContains(a, slice)) +} + +func TestShouldFindStringInSliceFold(t *testing.T) { + a := "xYz" + b := "AbC" + slice := []string{"XYz", "abc"} + + assert.True(t, IsStringInSliceFold(a, slice)) + assert.True(t, IsStringInSliceFold(b, slice)) +} + +func TestShouldNotFindStringInSliceFold(t *testing.T) { + a := "xyZ" + b := "ABc" + slice := []string{"cba", "zyx"} + + assert.False(t, IsStringInSliceFold(a, slice)) + assert.False(t, IsStringInSliceFold(b, slice)) } diff --git a/web/src/hooks/RequestMethod.ts b/web/src/hooks/RequestMethod.ts new file mode 100644 index 00000000..fc548cdc --- /dev/null +++ b/web/src/hooks/RequestMethod.ts @@ -0,0 +1,8 @@ +import queryString from "query-string"; +import { useLocation } from "react-router"; + +export function useRequestMethod() { + const location = useLocation(); + const queryParams = queryString.parse(location.search); + return queryParams && "rm" in queryParams ? (queryParams["rm"] as string) : undefined; +} diff --git a/web/src/services/FirstFactor.ts b/web/src/services/FirstFactor.ts index 3d9c2498..708dd82c 100644 --- a/web/src/services/FirstFactor.ts +++ b/web/src/services/FirstFactor.ts @@ -7,9 +7,16 @@ interface PostFirstFactorBody { password: string; keepMeLoggedIn: boolean; targetURL?: string; + requestMethod?: string; } -export async function postFirstFactor(username: string, password: string, rememberMe: boolean, targetURL?: string) { +export async function postFirstFactor( + username: string, + password: string, + rememberMe: boolean, + targetURL?: string, + requestMethod?: string, +) { const data: PostFirstFactorBody = { username, password, @@ -19,6 +26,11 @@ export async function postFirstFactor(username: string, password: string, rememb if (targetURL) { data.targetURL = targetURL; } + + if (requestMethod) { + data.requestMethod = requestMethod; + } + const res = await PostWithOptionalResponse(FirstFactorPath, data); return res ? res : ({} as SignInResponse); } diff --git a/web/src/views/LoginPortal/FirstFactor/FirstFactorForm.tsx b/web/src/views/LoginPortal/FirstFactor/FirstFactorForm.tsx index 4552c9c5..a9e8b900 100644 --- a/web/src/views/LoginPortal/FirstFactor/FirstFactorForm.tsx +++ b/web/src/views/LoginPortal/FirstFactor/FirstFactorForm.tsx @@ -7,6 +7,7 @@ import { useHistory } from "react-router"; import FixedTextField from "../../../components/FixedTextField"; import { useNotifications } from "../../../hooks/NotificationsContext"; import { useRedirectionURL } from "../../../hooks/RedirectionURL"; +import { useRequestMethod } from "../../../hooks/RequestMethod"; import LoginLayout from "../../../layouts/LoginLayout"; import { ResetPasswordStep1Route } from "../../../Routes"; import { postFirstFactor } from "../../../services/FirstFactor"; @@ -25,6 +26,7 @@ const FirstFactorForm = function (props: Props) { const style = useStyles(); const history = useHistory(); const redirectionURL = useRedirectionURL(); + const requestMethod = useRequestMethod(); const [rememberMe, setRememberMe] = useState(false); const [username, setUsername] = useState(""); @@ -60,7 +62,7 @@ const FirstFactorForm = function (props: Props) { props.onAuthenticationStart(); try { - const res = await postFirstFactor(username, password, rememberMe, redirectionURL); + const res = await postFirstFactor(username, password, rememberMe, redirectionURL, requestMethod); props.onAuthenticationSuccess(res ? res.redirect : undefined); } catch (err) { console.error(err); diff --git a/web/src/views/LoginPortal/LoginPortal.tsx b/web/src/views/LoginPortal/LoginPortal.tsx index 47a9a0ae..9601f8f2 100644 --- a/web/src/views/LoginPortal/LoginPortal.tsx +++ b/web/src/views/LoginPortal/LoginPortal.tsx @@ -5,6 +5,7 @@ import { Switch, Route, Redirect, useHistory, useLocation } from "react-router"; import { useConfiguration } from "../../hooks/Configuration"; import { useNotifications } from "../../hooks/NotificationsContext"; import { useRedirectionURL } from "../../hooks/RedirectionURL"; +import { useRequestMethod } from "../../hooks/RequestMethod"; import { useAutheliaState } from "../../hooks/State"; import { useUserPreferences as userUserInfo } from "../../hooks/UserInfo"; import { SecondFactorMethod } from "../../models/Methods"; @@ -31,6 +32,7 @@ const LoginPortal = function (props: Props) { const history = useHistory(); const location = useLocation(); const redirectionURL = useRedirectionURL(); + const requestMethod = useRequestMethod(); const { createErrorNotification } = useNotifications(); const [firstFactorDisabled, setFirstFactorDisabled] = useState(true); @@ -84,7 +86,9 @@ const LoginPortal = function (props: Props) { // Redirect to the correct stage if not enough authenticated useEffect(() => { if (state) { - const redirectionSuffix = redirectionURL ? `?rd=${encodeURIComponent(redirectionURL)}` : ""; + const redirectionSuffix = redirectionURL + ? `?rd=${encodeURIComponent(redirectionURL)}${requestMethod ? `&rm=${requestMethod}` : ""}` + : ""; if (state.authentication_level === AuthenticationLevel.Unauthenticated) { setFirstFactorDisabled(false); @@ -103,7 +107,7 @@ const LoginPortal = function (props: Props) { } } } - }, [state, redirectionURL, redirect, userInfo, setFirstFactorDisabled, configuration]); + }, [state, redirectionURL, requestMethod, redirect, userInfo, setFirstFactorDisabled, configuration]); const handleAuthSuccess = async (redirectionURL: string | undefined) => { if (redirectionURL) {