From 082fb166a24138bf7f09f7519c1cd579558cd8ed Mon Sep 17 00:00:00 2001 From: Traefiker Bot <30906710+traefiker@users.noreply.github.com> Date: Thu, 5 Mar 2020 08:18:04 +0100 Subject: [PATCH] Rework access control origin configuration --- docs/content/middlewares/headers.md | 40 +++-- docs/content/migration/v2.md | 8 + .../dynamic-configuration/docker-labels.yml | 2 + .../reference/dynamic-configuration/file.toml | 1 + .../reference/dynamic-configuration/file.yaml | 3 + .../reference/dynamic-configuration/kv-ref.md | 2 + .../marathon-labels.json | 2 + integration/fixtures/headers/cors.toml | 2 +- pkg/config/dynamic/fixtures/sample.toml | 1 + pkg/config/dynamic/middlewares.go | 6 +- pkg/config/label/label_test.go | 10 ++ pkg/middlewares/headers/headers.go | 150 +++++++++--------- pkg/middlewares/headers/headers_test.go | 92 +++++++---- pkg/provider/kv/kv_test.go | 6 + 14 files changed, 203 insertions(+), 122 deletions(-) diff --git a/docs/content/middlewares/headers.md b/docs/content/middlewares/headers.md index a5932c864..18bed41c2 100644 --- a/docs/content/middlewares/headers.md +++ b/docs/content/middlewares/headers.md @@ -197,7 +197,7 @@ This functionality allows for more advanced security features to quickly be set. ```yaml tab="Docker" labels: - "traefik.http.middlewares.testheader.headers.accesscontrolallowmethods=GET,OPTIONS,PUT" - - "traefik.http.middlewares.testheader.headers.accesscontrolalloworigin=origin-list-or-null" + - "traefik.http.middlewares.testheader.headers.accesscontrolalloworiginlist=https://foo.bar.org,https://example.org" - "traefik.http.middlewares.testheader.headers.accesscontrolmaxage=100" - "traefik.http.middlewares.testheader.headers.addvaryheader=true" ``` @@ -213,14 +213,16 @@ spec: - "GET" - "OPTIONS" - "PUT" - accessControlAllowOrigin: "origin-list-or-null" + accessControlAllowOriginList: + - "https://foo.bar.org" + - "https://example.org" accessControlMaxAge: 100 addVaryHeader: "true" ``` ```yaml tab="Consul Catalog" - "traefik.http.middlewares.testheader.headers.accesscontrolallowmethods=GET,OPTIONS,PUT" -- "traefik.http.middlewares.testheader.headers.accesscontrolalloworigin=origin-list-or-null" +- "traefik.http.middlewares.testheader.headers.accesscontrolalloworiginlist=https://foo.bar.org,https://example.org" - "traefik.http.middlewares.testheader.headers.accesscontrolmaxage=100" - "traefik.http.middlewares.testheader.headers.addvaryheader=true" ``` @@ -228,7 +230,7 @@ spec: ```json tab="Marathon" "labels": { "traefik.http.middlewares.testheader.headers.accesscontrolallowmethods": "GET,OPTIONS,PUT", - "traefik.http.middlewares.testheader.headers.accesscontrolalloworigin": "origin-list-or-null", + "traefik.http.middlewares.testheader.headers.accesscontrolalloworiginlist": "https://foo.bar.org,https://example.org", "traefik.http.middlewares.testheader.headers.accesscontrolmaxage": "100", "traefik.http.middlewares.testheader.headers.addvaryheader": "true" } @@ -237,7 +239,7 @@ spec: ```yaml tab="Rancher" labels: - "traefik.http.middlewares.testheader.headers.accesscontrolallowmethods=GET,OPTIONS,PUT" - - "traefik.http.middlewares.testheader.headers.accesscontrolalloworigin=origin-list-or-null" + - "traefik.http.middlewares.testheader.headers.accesscontrolalloworiginlist=https://foo.bar.org,https://example.org" - "traefik.http.middlewares.testheader.headers.accesscontrolmaxage=100" - "traefik.http.middlewares.testheader.headers.addvaryheader=true" ``` @@ -246,7 +248,7 @@ labels: [http.middlewares] [http.middlewares.testHeader.headers] accessControlAllowMethods= ["GET", "OPTIONS", "PUT"] - accessControlAllowOrigin = "origin-list-or-null" + accessControlAllowOriginList = ["https://foo.bar.org","https://example.org"] accessControlMaxAge = 100 addVaryHeader = true ``` @@ -260,7 +262,9 @@ http: - GET - OPTIONS - PUT - accessControlAllowOrigin: "origin-list-or-null" + accessControlAllowOriginList: + - https://foo.bar.org + - https://example.org accessControlMaxAge: 100 addVaryHeader: true ``` @@ -295,14 +299,22 @@ The `accessControlAllowHeaders` indicates which header field names can be used a The `accessControlAllowMethods` indicates which methods can be used during requests. -### `accessControlAllowOrigin` +### `accessControlAllowOriginList` -The `accessControlAllowOrigin` indicates whether a resource can be shared by returning different values. -The three options for this value are: +The `accessControlAllowOriginList` indicates whether a resource can be shared by returning different values. -- `origin-list-or-null` -- `*` -- `null` +A wildcard origin `*` can also be configured, and will match all requests. +If this value is set by a backend server, it will be overwritten by Traefik + +This value can contains a list of allowed origins. + +More information including how to use the settings can be found on: + +- [Mozilla.org](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Origin) +- [w3](https://www.w3.org/TR/cors/#access-control-allow-origin-response-header) +- [IETF](https://tools.ietf.org/html/rfc6454#section-7.1) + +Traefik no longer supports the null value, as it is [no longer recommended as a return value](https://w3c.github.io/webappsec-cors-for-developers/#avoid-returning-access-control-allow-origin-null). ### `accessControlExposeHeaders` @@ -314,7 +326,7 @@ The `accessControlMaxAge` indicates how long a preflight request can be cached. ### `addVaryHeader` -The `addVaryHeader` is used in conjunction with `accessControlAllowOrigin` to determine whether the vary header should be added or modified to demonstrate that server responses can differ beased on the value of the origin header. +The `addVaryHeader` is used in conjunction with `accessControlAllowOriginList` to determine whether the vary header should be added or modified to demonstrate that server responses can differ based on the value of the origin header. ### `allowedHosts` diff --git a/docs/content/migration/v2.md b/docs/content/migration/v2.md index c7a58dd18..d9ab4fa27 100644 --- a/docs/content/migration/v2.md +++ b/docs/content/migration/v2.md @@ -100,3 +100,11 @@ rules: ``` After having both resources applied, Traefik will work properly. + +## v2.1 to v2.2 + +### Headers middleware: accessControlAllowOrigin + +`accessControlAllowOrigin` is deprecated. +This field will be removed in future 2.x releases. +Please configure your allowed origins in `accessControlAllowOriginList` instead. diff --git a/docs/content/reference/dynamic-configuration/docker-labels.yml b/docs/content/reference/dynamic-configuration/docker-labels.yml index 26be2a93f..f5986f431 100644 --- a/docs/content/reference/dynamic-configuration/docker-labels.yml +++ b/docs/content/reference/dynamic-configuration/docker-labels.yml @@ -34,6 +34,7 @@ - "traefik.http.middlewares.middleware10.headers.accesscontrolallowheaders=foobar, foobar" - "traefik.http.middlewares.middleware10.headers.accesscontrolallowmethods=foobar, foobar" - "traefik.http.middlewares.middleware10.headers.accesscontrolalloworigin=foobar" +- "traefik.http.middlewares.middleware10.headers.accesscontrolalloworiginlist=foobar, foobar" - "traefik.http.middlewares.middleware10.headers.accesscontrolexposeheaders=foobar, foobar" - "traefik.http.middlewares.middleware10.headers.accesscontrolmaxage=42" - "traefik.http.middlewares.middleware10.headers.addvaryheader=true" @@ -134,6 +135,7 @@ - "traefik.http.routers.router1.tls.domains[1].main=foobar" - "traefik.http.routers.router1.tls.domains[1].sans=foobar, foobar" - "traefik.http.routers.router1.tls.options=foobar" +- "traefik.http.services.service01.loadbalancer.healthcheck.followredirects=true" - "traefik.http.services.service01.loadbalancer.healthcheck.headers.name0=foobar" - "traefik.http.services.service01.loadbalancer.healthcheck.headers.name1=foobar" - "traefik.http.services.service01.loadbalancer.healthcheck.hostname=foobar" diff --git a/docs/content/reference/dynamic-configuration/file.toml b/docs/content/reference/dynamic-configuration/file.toml index 8fa8c21ee..680e22a06 100644 --- a/docs/content/reference/dynamic-configuration/file.toml +++ b/docs/content/reference/dynamic-configuration/file.toml @@ -147,6 +147,7 @@ accessControlAllowHeaders = ["foobar", "foobar"] accessControlAllowMethods = ["foobar", "foobar"] accessControlAllowOrigin = "foobar" + accessControlAllowOriginList = ["foobar", "foobar"] accessControlExposeHeaders = ["foobar", "foobar"] accessControlMaxAge = 42 addVaryHeader = true diff --git a/docs/content/reference/dynamic-configuration/file.yaml b/docs/content/reference/dynamic-configuration/file.yaml index a7e946551..deafc992a 100644 --- a/docs/content/reference/dynamic-configuration/file.yaml +++ b/docs/content/reference/dynamic-configuration/file.yaml @@ -170,6 +170,9 @@ http: - foobar - foobar accessControlAllowOrigin: foobar + accessControlAllowOriginList: + - foobar + - foobar accessControlExposeHeaders: - foobar - foobar diff --git a/docs/content/reference/dynamic-configuration/kv-ref.md b/docs/content/reference/dynamic-configuration/kv-ref.md index 318d9cc4d..2ce8804e9 100644 --- a/docs/content/reference/dynamic-configuration/kv-ref.md +++ b/docs/content/reference/dynamic-configuration/kv-ref.md @@ -41,6 +41,8 @@ | `traefik/http/middlewares/Middleware10/headers/accessControlAllowMethods/0` | `foobar` | | `traefik/http/middlewares/Middleware10/headers/accessControlAllowMethods/1` | `foobar` | | `traefik/http/middlewares/Middleware10/headers/accessControlAllowOrigin` | `foobar` | +| `traefik/http/middlewares/Middleware10/headers/accessControlAllowOriginList/0` | `foobar` | +| `traefik/http/middlewares/Middleware10/headers/accessControlAllowOriginList/1` | `foobar` | | `traefik/http/middlewares/Middleware10/headers/accessControlExposeHeaders/0` | `foobar` | | `traefik/http/middlewares/Middleware10/headers/accessControlExposeHeaders/1` | `foobar` | | `traefik/http/middlewares/Middleware10/headers/accessControlMaxAge` | `42` | diff --git a/docs/content/reference/dynamic-configuration/marathon-labels.json b/docs/content/reference/dynamic-configuration/marathon-labels.json index cb465730d..0a125f331 100644 --- a/docs/content/reference/dynamic-configuration/marathon-labels.json +++ b/docs/content/reference/dynamic-configuration/marathon-labels.json @@ -34,6 +34,7 @@ "traefik.http.middlewares.middleware10.headers.accesscontrolallowheaders": "foobar, foobar", "traefik.http.middlewares.middleware10.headers.accesscontrolallowmethods": "foobar, foobar", "traefik.http.middlewares.middleware10.headers.accesscontrolalloworigin": "foobar", +"traefik.http.middlewares.middleware10.headers.accesscontrolalloworiginlist": "foobar, foobar", "traefik.http.middlewares.middleware10.headers.accesscontrolexposeheaders": "foobar, foobar", "traefik.http.middlewares.middleware10.headers.accesscontrolmaxage": "42", "traefik.http.middlewares.middleware10.headers.addvaryheader": "true", @@ -132,6 +133,7 @@ "traefik.http.routers.router1.tls.domains[1].main": "foobar", "traefik.http.routers.router1.tls.domains[1].sans": "foobar, foobar", "traefik.http.routers.router1.tls.options": "foobar", +"traefik.http.services.service01.loadbalancer.healthcheck.followredirects": "true", "traefik.http.services.service01.loadbalancer.healthcheck.headers.name0": "foobar", "traefik.http.services.service01.loadbalancer.healthcheck.headers.name1": "foobar", "traefik.http.services.service01.loadbalancer.healthcheck.hostname": "foobar", diff --git a/integration/fixtures/headers/cors.toml b/integration/fixtures/headers/cors.toml index 263966e39..3b0622e96 100644 --- a/integration/fixtures/headers/cors.toml +++ b/integration/fixtures/headers/cors.toml @@ -28,7 +28,7 @@ [http.middlewares] [http.middlewares.cors.headers] accessControlAllowMethods= ["GET", "OPTIONS", "PUT"] - accessControlAllowOrigin = "origin-list-or-null" + accessControlAllowOriginList = ["https://foo.bar.org"] accessControlMaxAge = 100 addVaryHeader = true diff --git a/pkg/config/dynamic/fixtures/sample.toml b/pkg/config/dynamic/fixtures/sample.toml index 8ff8bbd53..c75d53188 100644 --- a/pkg/config/dynamic/fixtures/sample.toml +++ b/pkg/config/dynamic/fixtures/sample.toml @@ -367,6 +367,7 @@ accessControlAllowHeaders = ["foobar", "foobar"] accessControlAllowMethods = ["foobar", "foobar"] accessControlAllowOrigin = "foobar" + accessControlAllowOriginList = ["foobar", "foobar"] accessControlExposeHeaders = ["foobar", "foobar"] accessControlMaxAge = 42 addVaryHeader = true diff --git a/pkg/config/dynamic/middlewares.go b/pkg/config/dynamic/middlewares.go index 9de48d8c7..e01480e97 100644 --- a/pkg/config/dynamic/middlewares.go +++ b/pkg/config/dynamic/middlewares.go @@ -158,7 +158,9 @@ type Headers struct { // AccessControlAllowMethods must be used in response to a preflight request with Access-Control-Request-Method set. AccessControlAllowMethods []string `json:"accessControlAllowMethods,omitempty" toml:"accessControlAllowMethods,omitempty" yaml:"accessControlAllowMethods,omitempty"` // AccessControlAllowOrigin Can be "origin-list-or-null" or "*". From (https://www.w3.org/TR/cors/#access-control-allow-origin-response-header) - AccessControlAllowOrigin string `json:"accessControlAllowOrigin,omitempty" toml:"accessControlAllowOrigin,omitempty" yaml:"accessControlAllowOrigin,omitempty"` + AccessControlAllowOrigin string `json:"accessControlAllowOrigin,omitempty" toml:"accessControlAllowOrigin,omitempty" yaml:"accessControlAllowOrigin,omitempty"` // Deprecated + // AccessControlAllowOriginList is a list of allowable origins. Can also be a wildcard origin "*". + AccessControlAllowOriginList []string `json:"accessControlAllowOriginList,omitempty" toml:"accessControlAllowOriginList,omitempty" yaml:"accessControlAllowOriginList,omitempty"` // AccessControlExposeHeaders sets valid headers for the response. AccessControlExposeHeaders []string `json:"accessControlExposeHeaders,omitempty" toml:"accessControlExposeHeaders,omitempty" yaml:"accessControlExposeHeaders,omitempty"` // AccessControlMaxAge sets the time that a preflight request may be cached. @@ -200,7 +202,7 @@ func (h *Headers) HasCorsHeadersDefined() bool { return h != nil && (h.AccessControlAllowCredentials || len(h.AccessControlAllowHeaders) != 0 || len(h.AccessControlAllowMethods) != 0 || - h.AccessControlAllowOrigin != "" || + len(h.AccessControlAllowOriginList) != 0 || len(h.AccessControlExposeHeaders) != 0 || h.AccessControlMaxAge != 0 || h.AddVaryHeader) diff --git a/pkg/config/label/label_test.go b/pkg/config/label/label_test.go index 56d73a753..0fe71e318 100644 --- a/pkg/config/label/label_test.go +++ b/pkg/config/label/label_test.go @@ -47,6 +47,7 @@ func TestDecodeConfiguration(t *testing.T) { "traefik.http.middlewares.Middleware8.headers.accesscontrolallowheaders": "X-foobar, X-fiibar", "traefik.http.middlewares.Middleware8.headers.accesscontrolallowmethods": "GET, PUT", "traefik.http.middlewares.Middleware8.headers.accesscontrolalloworigin": "foobar", + "traefik.http.middlewares.Middleware8.headers.accesscontrolalloworiginList": "foobar, fiibar", "traefik.http.middlewares.Middleware8.headers.accesscontrolexposeheaders": "X-foobar, X-fiibar", "traefik.http.middlewares.Middleware8.headers.accesscontrolmaxage": "200", "traefik.http.middlewares.Middleware8.headers.addvaryheader": "true", @@ -516,6 +517,10 @@ func TestDecodeConfiguration(t *testing.T) { "PUT", }, AccessControlAllowOrigin: "foobar", + AccessControlAllowOriginList: []string{ + "foobar", + "fiibar", + }, AccessControlExposeHeaders: []string{ "X-foobar", "X-fiibar", @@ -964,6 +969,10 @@ func TestEncodeConfiguration(t *testing.T) { "PUT", }, AccessControlAllowOrigin: "foobar", + AccessControlAllowOriginList: []string{ + "foobar", + "fiibar", + }, AccessControlExposeHeaders: []string{ "X-foobar", "X-fiibar", @@ -1118,6 +1127,7 @@ func TestEncodeConfiguration(t *testing.T) { "traefik.HTTP.Middlewares.Middleware8.Headers.AccessControlAllowHeaders": "X-foobar, X-fiibar", "traefik.HTTP.Middlewares.Middleware8.Headers.AccessControlAllowMethods": "GET, PUT", "traefik.HTTP.Middlewares.Middleware8.Headers.AccessControlAllowOrigin": "foobar", + "traefik.HTTP.Middlewares.Middleware8.Headers.AccessControlAllowOriginList": "foobar, fiibar", "traefik.HTTP.Middlewares.Middleware8.Headers.AccessControlExposeHeaders": "X-foobar, X-fiibar", "traefik.HTTP.Middlewares.Middleware8.Headers.AccessControlMaxAge": "200", "traefik.HTTP.Middlewares.Middleware8.Headers.AddVaryHeader": "true", diff --git a/pkg/middlewares/headers/headers.go b/pkg/middlewares/headers/headers.go index 0fce61796..4868c8cfd 100644 --- a/pkg/middlewares/headers/headers.go +++ b/pkg/middlewares/headers/headers.go @@ -20,20 +20,31 @@ const ( typeName = "Headers" ) +func handleDeprecation(ctx context.Context, cfg *dynamic.Headers) { + if cfg.AccessControlAllowOrigin != "" { + log.FromContext(ctx).Warn("accessControlAllowOrigin is deprecated, please use accessControlAllowOriginList instead.") + cfg.AccessControlAllowOriginList = append(cfg.AccessControlAllowOriginList, cfg.AccessControlAllowOrigin) + cfg.AccessControlAllowOrigin = "" + } +} + type headers struct { name string handler http.Handler } // New creates a Headers middleware. -func New(ctx context.Context, next http.Handler, config dynamic.Headers, name string) (http.Handler, error) { +func New(ctx context.Context, next http.Handler, cfg dynamic.Headers, name string) (http.Handler, error) { // HeaderMiddleware -> SecureMiddleWare -> next - logger := log.FromContext(middlewares.GetLoggerCtx(ctx, name, typeName)) + mCtx := middlewares.GetLoggerCtx(ctx, name, typeName) + logger := log.FromContext(mCtx) logger.Debug("Creating middleware") - hasSecureHeaders := config.HasSecureHeadersDefined() - hasCustomHeaders := config.HasCustomHeadersDefined() - hasCorsHeaders := config.HasCorsHeadersDefined() + handleDeprecation(mCtx, &cfg) + + hasSecureHeaders := cfg.HasSecureHeadersDefined() + hasCustomHeaders := cfg.HasCustomHeadersDefined() + hasCorsHeaders := cfg.HasCorsHeadersDefined() if !hasSecureHeaders && !hasCustomHeaders && !hasCorsHeaders { return nil, errors.New("headers configuration not valid") @@ -43,14 +54,14 @@ func New(ctx context.Context, next http.Handler, config dynamic.Headers, name st nextHandler := next if hasSecureHeaders { - logger.Debug("Setting up secureHeaders from %v", config) - handler = newSecure(next, config) + logger.Debug("Setting up secureHeaders from %v", cfg) + handler = newSecure(next, cfg) nextHandler = handler } if hasCustomHeaders || hasCorsHeaders { - logger.Debug("Setting up customHeaders/Cors from %v", config) - handler = NewHeader(nextHandler, config) + logger.Debug("Setting up customHeaders/Cors from %v", cfg) + handler = NewHeader(nextHandler, cfg) } return &headers{ @@ -73,29 +84,29 @@ type secureHeader struct { } // newSecure constructs a new secure instance with supplied options. -func newSecure(next http.Handler, headers dynamic.Headers) *secureHeader { +func newSecure(next http.Handler, cfg dynamic.Headers) *secureHeader { opt := secure.Options{ - BrowserXssFilter: headers.BrowserXSSFilter, - ContentTypeNosniff: headers.ContentTypeNosniff, - ForceSTSHeader: headers.ForceSTSHeader, - FrameDeny: headers.FrameDeny, - IsDevelopment: headers.IsDevelopment, - SSLRedirect: headers.SSLRedirect, - SSLForceHost: headers.SSLForceHost, - SSLTemporaryRedirect: headers.SSLTemporaryRedirect, - STSIncludeSubdomains: headers.STSIncludeSubdomains, - STSPreload: headers.STSPreload, - ContentSecurityPolicy: headers.ContentSecurityPolicy, - CustomBrowserXssValue: headers.CustomBrowserXSSValue, - CustomFrameOptionsValue: headers.CustomFrameOptionsValue, - PublicKey: headers.PublicKey, - ReferrerPolicy: headers.ReferrerPolicy, - SSLHost: headers.SSLHost, - AllowedHosts: headers.AllowedHosts, - HostsProxyHeaders: headers.HostsProxyHeaders, - SSLProxyHeaders: headers.SSLProxyHeaders, - STSSeconds: headers.STSSeconds, - FeaturePolicy: headers.FeaturePolicy, + BrowserXssFilter: cfg.BrowserXSSFilter, + ContentTypeNosniff: cfg.ContentTypeNosniff, + ForceSTSHeader: cfg.ForceSTSHeader, + FrameDeny: cfg.FrameDeny, + IsDevelopment: cfg.IsDevelopment, + SSLRedirect: cfg.SSLRedirect, + SSLForceHost: cfg.SSLForceHost, + SSLTemporaryRedirect: cfg.SSLTemporaryRedirect, + STSIncludeSubdomains: cfg.STSIncludeSubdomains, + STSPreload: cfg.STSPreload, + ContentSecurityPolicy: cfg.ContentSecurityPolicy, + CustomBrowserXssValue: cfg.CustomBrowserXSSValue, + CustomFrameOptionsValue: cfg.CustomFrameOptionsValue, + PublicKey: cfg.PublicKey, + ReferrerPolicy: cfg.ReferrerPolicy, + SSLHost: cfg.SSLHost, + AllowedHosts: cfg.AllowedHosts, + HostsProxyHeaders: cfg.HostsProxyHeaders, + SSLProxyHeaders: cfg.SSLProxyHeaders, + STSSeconds: cfg.STSSeconds, + FeaturePolicy: cfg.FeaturePolicy, } return &secureHeader{ @@ -119,13 +130,16 @@ type Header struct { } // NewHeader constructs a new header instance from supplied frontend header struct. -func NewHeader(next http.Handler, headers dynamic.Headers) *Header { - hasCustomHeaders := headers.HasCustomHeadersDefined() - hasCorsHeaders := headers.HasCorsHeadersDefined() +func NewHeader(next http.Handler, cfg dynamic.Headers) *Header { + hasCustomHeaders := cfg.HasCustomHeadersDefined() + hasCorsHeaders := cfg.HasCorsHeadersDefined() + + ctx := log.With(context.Background(), log.Str(log.MiddlewareType, typeName)) + handleDeprecation(ctx, &cfg) return &Header{ next: next, - headers: &headers, + headers: &cfg, hasCustomHeaders: hasCustomHeaders, hasCorsHeaders: hasCorsHeaders, } @@ -159,29 +173,6 @@ func (s *Header) modifyCustomRequestHeaders(req *http.Request) { } } -// preRequestModifyCorsResponseHeaders sets during request processing time, -// all the CORS response headers that we already know that are supposed to be set, -// and which do not depend on a later state of the response. -// One notable example of a header that can only be modified later on is "Vary", -// And this is set in the post-response response modifier method -func (s *Header) preRequestModifyCorsResponseHeaders(rw http.ResponseWriter, req *http.Request) { - originHeader := req.Header.Get("Origin") - allowOrigin := s.getAllowOrigin(originHeader) - - if allowOrigin != "" { - rw.Header().Set("Access-Control-Allow-Origin", allowOrigin) - } - - if s.headers.AccessControlAllowCredentials { - rw.Header().Set("Access-Control-Allow-Credentials", "true") - } - - if len(s.headers.AccessControlExposeHeaders) > 0 { - exposeHeaders := strings.Join(s.headers.AccessControlExposeHeaders, ",") - rw.Header().Set("Access-Control-Expose-Headers", exposeHeaders) - } -} - // PostRequestModifyResponseHeaders set or delete response headers. // This method is called AFTER the response is generated from the backend // and can merge/override headers from the backend response. @@ -194,6 +185,25 @@ func (s *Header) PostRequestModifyResponseHeaders(res *http.Response) error { res.Header.Set(header, value) } } + + if res != nil && res.Request != nil { + originHeader := res.Request.Header.Get("Origin") + allowed, match := s.isOriginAllowed(originHeader) + + if allowed { + res.Header.Set("Access-Control-Allow-Origin", match) + } + } + + if s.headers.AccessControlAllowCredentials { + res.Header.Set("Access-Control-Allow-Credentials", "true") + } + + if len(s.headers.AccessControlExposeHeaders) > 0 { + exposeHeaders := strings.Join(s.headers.AccessControlExposeHeaders, ",") + res.Header.Set("Access-Control-Expose-Headers", exposeHeaders) + } + if !s.headers.AddVaryHeader { return nil } @@ -241,30 +251,24 @@ func (s *Header) processCorsHeaders(rw http.ResponseWriter, req *http.Request) b rw.Header().Set("Access-Control-Allow-Methods", allowMethods) } - allowOrigin := s.getAllowOrigin(originHeader) - - if allowOrigin != "" { - rw.Header().Set("Access-Control-Allow-Origin", allowOrigin) + allowed, match := s.isOriginAllowed(originHeader) + if allowed { + rw.Header().Set("Access-Control-Allow-Origin", match) } rw.Header().Set("Access-Control-Max-Age", strconv.Itoa(int(s.headers.AccessControlMaxAge))) return true } - s.preRequestModifyCorsResponseHeaders(rw, req) return false } -func (s *Header) getAllowOrigin(header string) string { - switch s.headers.AccessControlAllowOrigin { - case "origin-list-or-null": - if len(header) == 0 { - return "null" +func (s *Header) isOriginAllowed(origin string) (bool, string) { + for _, item := range s.headers.AccessControlAllowOriginList { + if item == "*" || item == origin { + return true, item } - return header - case "*": - return "*" - default: - return "" } + + return false, "" } diff --git a/pkg/middlewares/headers/headers_test.go b/pkg/middlewares/headers/headers_test.go index e335399dc..f9db565ce 100644 --- a/pkg/middlewares/headers/headers_test.go +++ b/pkg/middlewares/headers/headers_test.go @@ -202,9 +202,9 @@ func TestCORSPreflights(t *testing.T) { { desc: "Test Simple Preflight", header: NewHeader(emptyHandler, dynamic.Headers{ - AccessControlAllowMethods: []string{"GET", "OPTIONS", "PUT"}, - AccessControlAllowOrigin: "origin-list-or-null", - AccessControlMaxAge: 600, + AccessControlAllowMethods: []string{"GET", "OPTIONS", "PUT"}, + AccessControlAllowOriginList: []string{"https://foo.bar.org"}, + AccessControlMaxAge: 600, }), requestHeaders: map[string][]string{ "Access-Control-Request-Headers": {"origin"}, @@ -220,9 +220,9 @@ func TestCORSPreflights(t *testing.T) { { desc: "Wildcard origin Preflight", header: NewHeader(emptyHandler, dynamic.Headers{ - AccessControlAllowMethods: []string{"GET", "OPTIONS", "PUT"}, - AccessControlAllowOrigin: "*", - AccessControlMaxAge: 600, + AccessControlAllowMethods: []string{"GET", "OPTIONS", "PUT"}, + AccessControlAllowOriginList: []string{"*"}, + AccessControlMaxAge: 600, }), requestHeaders: map[string][]string{ "Access-Control-Request-Headers": {"origin"}, @@ -239,7 +239,7 @@ func TestCORSPreflights(t *testing.T) { desc: "Allow Credentials Preflight", header: NewHeader(emptyHandler, dynamic.Headers{ AccessControlAllowMethods: []string{"GET", "OPTIONS", "PUT"}, - AccessControlAllowOrigin: "*", + AccessControlAllowOriginList: []string{"*"}, AccessControlAllowCredentials: true, AccessControlMaxAge: 600, }), @@ -258,10 +258,10 @@ func TestCORSPreflights(t *testing.T) { { desc: "Allow Headers Preflight", header: NewHeader(emptyHandler, dynamic.Headers{ - AccessControlAllowMethods: []string{"GET", "OPTIONS", "PUT"}, - AccessControlAllowOrigin: "*", - AccessControlAllowHeaders: []string{"origin", "X-Forwarded-For"}, - AccessControlMaxAge: 600, + AccessControlAllowMethods: []string{"GET", "OPTIONS", "PUT"}, + AccessControlAllowOriginList: []string{"*"}, + AccessControlAllowHeaders: []string{"origin", "X-Forwarded-For"}, + AccessControlMaxAge: 600, }), requestHeaders: map[string][]string{ "Access-Control-Request-Headers": {"origin"}, @@ -278,10 +278,10 @@ func TestCORSPreflights(t *testing.T) { { desc: "No Request Headers Preflight", header: NewHeader(emptyHandler, dynamic.Headers{ - AccessControlAllowMethods: []string{"GET", "OPTIONS", "PUT"}, - AccessControlAllowOrigin: "*", - AccessControlAllowHeaders: []string{"origin", "X-Forwarded-For"}, - AccessControlMaxAge: 600, + AccessControlAllowMethods: []string{"GET", "OPTIONS", "PUT"}, + AccessControlAllowOriginList: []string{"*"}, + AccessControlAllowHeaders: []string{"origin", "X-Forwarded-For"}, + AccessControlMaxAge: 600, }), requestHeaders: map[string][]string{ "Access-Control-Request-Method": {"GET", "OPTIONS"}, @@ -352,6 +352,12 @@ func TestCORSResponses(t *testing.T) { emptyHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}) nonEmptyHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Vary", "Testing") }) existingOriginHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Vary", "Origin") }) + existingAccessControlAllowOriginHandlerSet := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Access-Control-Allow-Origin", "http://foo.bar.org") + }) + existingAccessControlAllowOriginHandlerAdd := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Add("Access-Control-Allow-Origin", "http://foo.bar.org") + }) testCases := []struct { desc string @@ -362,7 +368,7 @@ func TestCORSResponses(t *testing.T) { { desc: "Test Simple Request", header: NewHeader(emptyHandler, dynamic.Headers{ - AccessControlAllowOrigin: "origin-list-or-null", + AccessControlAllowOriginList: []string{"https://foo.bar.org"}, }), requestHeaders: map[string][]string{ "Origin": {"https://foo.bar.org"}, @@ -374,7 +380,7 @@ func TestCORSResponses(t *testing.T) { { desc: "Wildcard origin Request", header: NewHeader(emptyHandler, dynamic.Headers{ - AccessControlAllowOrigin: "*", + AccessControlAllowOriginList: []string{"*"}, }), requestHeaders: map[string][]string{ "Origin": {"https://foo.bar.org"}, @@ -386,12 +392,10 @@ func TestCORSResponses(t *testing.T) { { desc: "Empty origin Request", header: NewHeader(emptyHandler, dynamic.Headers{ - AccessControlAllowOrigin: "origin-list-or-null", + AccessControlAllowOriginList: []string{"https://foo.bar.org"}, }), requestHeaders: map[string][]string{}, - expected: map[string][]string{ - "Access-Control-Allow-Origin": {"null"}, - }, + expected: map[string][]string{}, }, { desc: "Not Defined origin Request", @@ -402,7 +406,7 @@ func TestCORSResponses(t *testing.T) { { desc: "Allow Credentials Request", header: NewHeader(emptyHandler, dynamic.Headers{ - AccessControlAllowOrigin: "*", + AccessControlAllowOriginList: []string{"*"}, AccessControlAllowCredentials: true, }), requestHeaders: map[string][]string{ @@ -416,8 +420,8 @@ func TestCORSResponses(t *testing.T) { { desc: "Expose Headers Request", header: NewHeader(emptyHandler, dynamic.Headers{ - AccessControlAllowOrigin: "*", - AccessControlExposeHeaders: []string{"origin", "X-Forwarded-For"}, + AccessControlAllowOriginList: []string{"*"}, + AccessControlExposeHeaders: []string{"origin", "X-Forwarded-For"}, }), requestHeaders: map[string][]string{ "Origin": {"https://foo.bar.org"}, @@ -430,8 +434,8 @@ func TestCORSResponses(t *testing.T) { { desc: "Test Simple Request with Vary Headers", header: NewHeader(emptyHandler, dynamic.Headers{ - AccessControlAllowOrigin: "origin-list-or-null", - AddVaryHeader: true, + AccessControlAllowOriginList: []string{"https://foo.bar.org"}, + AddVaryHeader: true, }), requestHeaders: map[string][]string{ "Origin": {"https://foo.bar.org"}, @@ -444,8 +448,8 @@ func TestCORSResponses(t *testing.T) { { desc: "Test Simple Request with Vary Headers and non-empty response", header: NewHeader(nonEmptyHandler, dynamic.Headers{ - AccessControlAllowOrigin: "origin-list-or-null", - AddVaryHeader: true, + AccessControlAllowOriginList: []string{"https://foo.bar.org"}, + AddVaryHeader: true, }), requestHeaders: map[string][]string{ "Origin": {"https://foo.bar.org"}, @@ -458,8 +462,8 @@ func TestCORSResponses(t *testing.T) { { desc: "Test Simple Request with Vary Headers and existing vary:origin response", header: NewHeader(existingOriginHandler, dynamic.Headers{ - AccessControlAllowOrigin: "origin-list-or-null", - AddVaryHeader: true, + AccessControlAllowOriginList: []string{"https://foo.bar.org"}, + AddVaryHeader: true, }), requestHeaders: map[string][]string{ "Origin": {"https://foo.bar.org"}, @@ -470,6 +474,29 @@ func TestCORSResponses(t *testing.T) { }, }, { + desc: "Test Simple Request with non-empty response: set ACAO", + header: NewHeader(existingAccessControlAllowOriginHandlerSet, dynamic.Headers{ + AccessControlAllowOriginList: []string{"*"}, + }), + requestHeaders: map[string][]string{ + "Origin": {"https://foo.bar.org"}, + }, + expected: map[string][]string{ + "Access-Control-Allow-Origin": {"*"}, + }, + }, + { + desc: "Test Simple Request with non-empty response: add ACAO", + header: NewHeader(existingAccessControlAllowOriginHandlerAdd, dynamic.Headers{ + AccessControlAllowOriginList: []string{"*"}, + }), + requestHeaders: map[string][]string{ + "Origin": {"https://foo.bar.org"}, + }, + expected: map[string][]string{ + "Access-Control-Allow-Origin": {"*"}, + }, + }, { desc: "Test Simple CustomRequestHeaders Not Hijacked by CORS", header: NewHeader(emptyHandler, dynamic.Headers{ CustomRequestHeaders: map[string]string{"foo": "bar"}, @@ -487,10 +514,11 @@ func TestCORSResponses(t *testing.T) { t.Run(test.desc, func(t *testing.T) { req := testhelpers.MustNewRequest(http.MethodGet, "/foo", nil) req.Header = test.requestHeaders - rw := httptest.NewRecorder() test.header.ServeHTTP(rw, req) - err := test.header.PostRequestModifyResponseHeaders(rw.Result()) + res := rw.Result() + res.Request = req + err := test.header.PostRequestModifyResponseHeaders(res) require.NoError(t, err) assert.Equal(t, test.expected, rw.Result().Header) }) diff --git a/pkg/provider/kv/kv_test.go b/pkg/provider/kv/kv_test.go index 7f9e013c5..10de5d799 100644 --- a/pkg/provider/kv/kv_test.go +++ b/pkg/provider/kv/kv_test.go @@ -94,6 +94,8 @@ func Test_buildConfiguration(t *testing.T) { "traefik/http/middlewares/Middleware09/headers/accessControlAllowHeaders/0": "foobar", "traefik/http/middlewares/Middleware09/headers/accessControlAllowHeaders/1": "foobar", "traefik/http/middlewares/Middleware09/headers/accessControlAllowOrigin": "foobar", + "traefik/http/middlewares/Middleware09/headers/accessControlAllowOriginList/0": "foobar", + "traefik/http/middlewares/Middleware09/headers/accessControlAllowOriginList/1": "foobar", "traefik/http/middlewares/Middleware09/headers/contentTypeNosniff": "true", "traefik/http/middlewares/Middleware09/headers/accessControlAllowCredentials": "true", "traefik/http/middlewares/Middleware09/headers/featurePolicy": "foobar", @@ -543,6 +545,10 @@ func Test_buildConfiguration(t *testing.T) { "foobar", }, AccessControlAllowOrigin: "foobar", + AccessControlAllowOriginList: []string{ + "foobar", + "foobar", + }, AccessControlExposeHeaders: []string{ "foobar", "foobar",