Skip to content

Commit

Permalink
Allowed methods (#444)
Browse files Browse the repository at this point in the history
* AllowedMethodsHandler to check allowed methods (between access control and scope)

* allowed_methods attribute for api and endpoint blocks

* wildcard for allowed_methods

* CORS preflight: Access-Control-Allow-Methods only set with requested method if method is allowed (per allowed_methods or default)

* Documentation for allowed_methods

* changelog entry

* little bit shorter code

* moved config file

* test cases for blocking request by not allowing any method

* test cases for invalid strings in allowed_methods

* upper-case methods only after regexp check

* our own added operations (method/endpoint) must be allowed, no need to check method

* removed now unused MustAddRoute() and allowedMethods

* don't allow TRACE, added test cases for CONNECT

* check methods at config load

* moved methodAllowed from CORS to CORSOptions
  • Loading branch information
johakoch authored Mar 21, 2022
1 parent 08d4c36 commit b613096
Show file tree
Hide file tree
Showing 20 changed files with 462 additions and 47 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ Unreleased changes are available as `avenga/couper:edge` container.
* [`backend_request`](./docs/REFERENCE.md#backend_request) and [`backend_response`](./docs/REFERENCE.md#backend_response) variables ([#430](https://github.com/avenga/couper/pull/430))
* `beta_scope_map` attribute for the [JWT Block](./docs/REFERENCE.md#jwt-block) ([#434](https://github.com/avenga/couper/pull/434))
* `saml` [error type](./docs/ERRORS.md#error-types) ([#424](https://github.com/avenga/couper/pull/424))
* `allowed_methods` attribute for the [API](./docs/REFERENCE.md#api-block) or [Endpoint Block](./docs/REFERENCE.md#endpoint-block) ([#444](https://github.com/avenga/couper/pull/444))

* **Changed**
* Automatically add the `private` directive to the response `Cache-Control` HTTP header field value for all resources protected by [JWT](./docs/REFERENCE.md#jwt-block) ([#418](https://github.com/avenga/couper/pull/418))
Expand Down
1 change: 1 addition & 0 deletions config/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ var _ Inline = &API{}
type API struct {
ErrorHandlerSetter
AccessControl []string `hcl:"access_control,optional"`
AllowedMethods []string `hcl:"allowed_methods,optional"`
BasePath string `hcl:"base_path,optional"`
CORS *CORS `hcl:"cors,block"`
DisableAccessControl []string `hcl:"disable_access_control,optional"`
Expand Down
6 changes: 6 additions & 0 deletions config/configload/endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,12 @@ func refineEndpoints(definedBackends Backends, endpoints config.Endpoints, check

endpointContent := bodyToContent(endpoint.Remain)

if check && endpoint.AllowedMethods != nil && len(endpoint.AllowedMethods) > 0 {
if err = validMethods(endpoint.AllowedMethods, &endpointContent.Attributes["allowed_methods"].Range); err != nil {
return err
}
}

proxies := endpointContent.Blocks.OfType(proxy)
requests := endpointContent.Blocks.OfType(request)

Expand Down
6 changes: 6 additions & 0 deletions config/configload/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,12 @@ func LoadConfig(body hcl.Body, src []byte, filename string) (*config.Couper, err
apiConfig.Name = apiBlock.Labels[0]
}

if apiConfig.AllowedMethods != nil && len(apiConfig.AllowedMethods) > 0 {
if err = validMethods(apiConfig.AllowedMethods, &bodyToContent(apiConfig.Remain).Attributes["allowed_methods"].Range); err != nil {
return nil, err
}
}

err := refineEndpoints(definedBackends, apiConfig.Endpoints, true)
if err != nil {
return nil, err
Expand Down
19 changes: 19 additions & 0 deletions config/configload/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,15 @@ package configload

import (
"fmt"
"regexp"

"github.com/hashicorp/hcl/v2"
)

// https://datatracker.ietf.org/doc/html/rfc7231#section-4
// https://datatracker.ietf.org/doc/html/rfc7230#section-3.2.6
var methodRegExp = regexp.MustCompile("^[!#$%&'*+\\-.\\^_`|~0-9a-zA-Z]+$")

func validLabelName(name string, hr *hcl.Range) error {
if !regexProxyRequestLabel.MatchString(name) {
return hcl.Diagnostics{&hcl.Diagnostic{
Expand Down Expand Up @@ -53,3 +58,17 @@ func verifyBodyAttributes(content *hcl.BodyContent) error {
}
return nil
}

func validMethods(methods []string, hr *hcl.Range) error {
for _, method := range methods {
if !methodRegExp.MatchString(method) {
return hcl.Diagnostics{&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "method contains invalid character(s)",
Subject: hr,
}}
}
}

return nil
}
1 change: 1 addition & 0 deletions config/endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ var _ Inline = &Endpoint{}
type Endpoint struct {
ErrorHandlerSetter
AccessControl []string `hcl:"access_control,optional"`
AllowedMethods []string `hcl:"allowed_methods,optional"`
DisableAccessControl []string `hcl:"disable_access_control,optional"`
ErrorFile string `hcl:"error_file,optional"`
Pattern string `hcl:"pattern,label"`
Expand Down
20 changes: 17 additions & 3 deletions config/runtime/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ func NewServerConfiguration(conf *config.Couper, log *logrus.Entry, memStore *ca
return nil, err
}

corsOptions, cerr := middleware.NewCORSOptions(whichCORS(srvConf, srvConf.Spa))
corsOptions, cerr := middleware.NewCORSOptions(whichCORS(srvConf, srvConf.Spa), nil)
if cerr != nil {
return nil, cerr
}
Expand Down Expand Up @@ -210,7 +210,7 @@ func NewServerConfiguration(conf *config.Couper, log *logrus.Entry, memStore *ca
return nil, err
}

corsOptions, cerr := middleware.NewCORSOptions(whichCORS(srvConf, srvConf.Files))
corsOptions, cerr := middleware.NewCORSOptions(whichCORS(srvConf, srvConf.Files), nil)
if cerr != nil {
return nil, cerr
}
Expand Down Expand Up @@ -320,6 +320,20 @@ func NewServerConfiguration(conf *config.Couper, log *logrus.Entry, memStore *ca
}

accessControl := newAC(srvConf, parentAPI)

allowedMethods := endpointConf.AllowedMethods
if allowedMethods == nil && parentAPI != nil {
// if allowed_methods in endpoint {} not defined, try allowed_methods in parent api {}
allowedMethods = parentAPI.AllowedMethods
}
notAllowedMethodsHandler := epOpts.ErrorTemplate.WithError(errors.MethodNotAllowed)
var allowedMethodsHandler *middleware.AllowedMethodsHandler
allowedMethodsHandler, err = middleware.NewAllowedMethodsHandler(allowedMethods, protectedHandler, notAllowedMethodsHandler)
if err != nil {
return nil, err
}
protectedHandler = allowedMethodsHandler

epHandler, err = configureProtectedHandler(accessControls, confCtx, accessControl,
config.NewAccessControl(endpointConf.AccessControl, endpointConf.DisableAccessControl),
&protectedOptions{
Expand All @@ -333,7 +347,7 @@ func NewServerConfiguration(conf *config.Couper, log *logrus.Entry, memStore *ca
return nil, err
}

corsOptions, err := middleware.NewCORSOptions(whichCORS(srvConf, parentAPI))
corsOptions, err := middleware.NewCORSOptions(whichCORS(srvConf, parentAPI), allowedMethodsHandler.MethodAllowed)
if err != nil {
return nil, err
}
Expand Down
4 changes: 4 additions & 0 deletions docs/REFERENCE.md
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ as json error with an error body payload. This can be customized via `error_file
|`base_path`|string|-|Configures the path prefix for all requests.|⚠ Must be unique if multiple `api` blocks are defined.| `base_path = "/v1"`|
| `error_file` |string|-|Location of the error file template.|-|`error_file = "./my_error_body.json"`|
| `access_control` |list|-|Sets predefined [Access Control](#access-control) for `api` block context.|⚠ Inherited by nested blocks.| `access_control = ["foo"]`|
| `allowed_methods` | tuple of string | `["*"]` == `["GET", "HEAD", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"]` | Sets allowed methods as _default_ for all contained endpoints. Requests with a method that is not allowed result in an error response with a `405 Method Not Allowed` status. | The default value `*` can be combined with additional methods. Methods are matched case-insensitively. `Access-Control-Allow-Methods` is only sent in response to a [CORS](#cors-block) preflight request, if the method requested by `Access-Control-Request-Method` is an allowed method. | `allowed_methods = ["GET", "POST"]` or `allowed_methods = ["*", "BREW"]` |
| `beta_scope` |string or object|-|Scope value required to use this API (see [error type](ERRORS.md#error-types) `beta_insufficient_scope`).|If the value is a string, the same scope value applies to all request methods. If there are different scope values for different request methods, use an object with the request methods as keys and string values. Methods not specified in this object are not permitted (see [error type](ERRORS.md#error-types) `beta_operation_denied`). `"*"` is the key for "all other methods". A value `""` means "no (additional) scope required".| `beta_scope = "read"` or `beta_scope = { post = "write", "*" = "" }`|
| `custom_log_fields` | map | - | Defines log fields for [Custom Logging](LOGS.md#custom-logging). | ⚠ Inherited by nested blocks. | - |

Expand All @@ -134,6 +135,7 @@ produce an explicit or implicit client response.
| `request_body_limit` | string | `64MiB` | Configures the maximum buffer size while accessing `request.form_body` or `request.json_body` content. | ⚠ Valid units are: `KiB, MiB, GiB` | `request_body_limit = "200KiB"` |
| `path` | string | - | Changeable part of the upstream URL. Changes the path suffix of the outgoing request. | - | - |
| `access_control` | list | - | Sets predefined [Access Control](#access-control) for `endpoint` block context. | - | `access_control = ["foo"]` |
| `allowed_methods` | tuple of string | `["*"]` == `["GET", "HEAD", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"]` | Sets allowed methods _overriding_ a default set in the containing `api` block. Requests with a method that is not allowed result in an error response with a `405 Method Not Allowed` status. | The default value `*` can be combined with additional methods. Methods are matched case-insensitively. `Access-Control-Allow-Methods` is only sent in response to a [CORS](#cors-block) preflight request, if the method requested by `Access-Control-Request-Method` is an allowed method. | `allowed_methods = ["GET", "POST"]` or `allowed_methods = ["*", "BREW"]` |
| `beta_scope` | string or object | - | Scope value required to use this endpoint (see [error type](ERRORS.md#error-types) `beta_insufficient_scope`). | If the value is a string, the same scope value applies to all request methods. If there are different scope values for different request methods, use an object with the request methods as keys and string values. Methods not specified in this object are not permitted (see [error type](ERRORS.md#error-types) `beta_operation_denied`). `"*"` is the key for "all other methods". A value `""` means "no (additional) scope required". | `beta_scope = "read"` or `beta_scope = { post = "write", "*" = "" }` |
| `custom_log_fields` | map | - | Defines log fields for [Custom Logging](LOGS.md#custom-logging). | ⚠ Inherited by nested blocks. | - |
| [Modifiers](#modifiers) | - | - | - | - | - |
Expand Down Expand Up @@ -281,6 +283,8 @@ The `cors` block configures the CORS (Cross-Origin Resource Sharing) behavior in
| `disable` | bool|`false`|Set to `true` to disable the inheritance of CORS from the [Server Block](#server-block) in [Files Block](#files-block), [SPA Block](#spa-block) and [API Block](#api-block) contexts.|-|-|
| `max_age` |[duration](#duration)|-|Indicates the time the information provided by the `Access-Control-Allow-Methods` and `Access-Control-Allow-Headers` response HTTP header fields.|⚠ Can be cached|`max_age = "1h"`|

**Note:** `Access-Control-Allow-Methods` is only sent in response to a CORS preflight request, if the method requested by `Access-Control-Request-Method` is an allowed method (see the `allowed_method` attribute for [`api`](#api-block) or [`endpoint`](#endpoint-block) blocks).

### OAuth2 CC Block

The `oauth2` block in the [Backend Block](#backend-block) context configures the OAuth2 Client Credentials flow to request a bearer token for the backend request.
Expand Down
1 change: 1 addition & 0 deletions errors/couper.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ var (
Endpoint = &Error{synopsis: "endpoint error", kinds: []string{"endpoint"}, httpStatus: http.StatusBadGateway}
Evaluation = &Error{synopsis: "expression evaluation error", kinds: []string{"evaluation"}, httpStatus: http.StatusInternalServerError}
Configuration = &Error{synopsis: "configuration error", kinds: []string{"configuration"}, httpStatus: http.StatusInternalServerError}
MethodNotAllowed = &Error{synopsis: "method not allowed error", httpStatus: http.StatusMethodNotAllowed}
Proxy = &Error{synopsis: "proxy error", httpStatus: http.StatusBadGateway}
Request = &Error{synopsis: "request error", httpStatus: http.StatusBadGateway}
RouteNotFound = &Error{synopsis: "route not found error", httpStatus: http.StatusNotFound}
Expand Down
68 changes: 68 additions & 0 deletions handler/middleware/allowed_methods.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
package middleware

import (
"net/http"
"strings"
)

var defaultAllowedMethods = []string{
http.MethodGet,
http.MethodHead,
http.MethodPost,
http.MethodPut,
http.MethodPatch,
http.MethodDelete,
http.MethodOptions,
}

var _ http.Handler = &AllowedMethodsHandler{}

type AllowedMethodsHandler struct {
allowedMethods map[string]struct{}
allowedHandler http.Handler
notAllowedHandler http.Handler
}

type methodAllowedFunc func(string) bool

func NewAllowedMethodsHandler(allowedMethods []string, allowedHandler, notAllowedHandler http.Handler) (*AllowedMethodsHandler, error) {
amh := &AllowedMethodsHandler{
allowedMethods: make(map[string]struct{}),
allowedHandler: allowedHandler,
notAllowedHandler: notAllowedHandler,
}
if allowedMethods == nil {
allowedMethods = defaultAllowedMethods
}
for _, method := range allowedMethods {
if method == "*" {
for _, m := range defaultAllowedMethods {
amh.allowedMethods[m] = struct{}{}
}
} else {
method = strings.ToUpper(method)
amh.allowedMethods[method] = struct{}{}
}
}

return amh, nil
}

func (a *AllowedMethodsHandler) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
if _, ok := a.allowedMethods[req.Method]; !ok {
a.notAllowedHandler.ServeHTTP(rw, req)
return
}

a.allowedHandler.ServeHTTP(rw, req)
}

func (a *AllowedMethodsHandler) MethodAllowed(method string) bool {
method = strings.TrimSpace(strings.ToUpper(method))
_, ok := a.allowedMethods[method]
return ok
}

func (a *AllowedMethodsHandler) Child() http.Handler {
return a.allowedHandler
}
8 changes: 6 additions & 2 deletions handler/middleware/cors.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,10 @@ type CORSOptions struct {
AllowedOrigins []string
AllowCredentials bool
MaxAge string
methodAllowed methodAllowedFunc
}

func NewCORSOptions(cors *config.CORS) (*CORSOptions, error) {
func NewCORSOptions(cors *config.CORS, methodAllowed methodAllowedFunc) (*CORSOptions, error) {
if cors == nil {
return nil, nil
}
Expand All @@ -49,6 +50,7 @@ func NewCORSOptions(cors *config.CORS) (*CORSOptions, error) {
AllowedOrigins: allowedOrigins,
AllowCredentials: cors.AllowCredentials,
MaxAge: corsMaxAge,
methodAllowed: methodAllowed,
}, nil
}

Expand Down Expand Up @@ -128,7 +130,9 @@ func (c *CORS) setCorsRespHeaders(headers http.Header, req *http.Request) {
// Reflect request header value
acrm := req.Header.Get("Access-Control-Request-Method")
if acrm != "" {
headers.Set("Access-Control-Allow-Methods", acrm)
if c.options.methodAllowed == nil || c.options.methodAllowed(acrm) {
headers.Set("Access-Control-Allow-Methods", acrm)
}
headers.Add("Vary", "Access-Control-Request-Method")
}
// Reflect request header value
Expand Down
Loading

0 comments on commit b613096

Please sign in to comment.