From cb71d7ea094df31f2e7847b101c7c52d83df8c91 Mon Sep 17 00:00:00 2001 From: Zadkiel Aharonian Date: Tue, 10 Oct 2023 14:19:56 +0200 Subject: [PATCH] fix: add 'vary: origin' header if not set by backend Signed-off-by: GitHub --- .../nginx-configuration/annotations.md | 4 + .../ingress/controller/template/template.go | 8 ++ rootfs/etc/nginx/template/nginx.tmpl | 12 +- test/e2e/annotations/cors.go | 130 +++++++++++++++++- 4 files changed, 152 insertions(+), 2 deletions(-) diff --git a/docs/user-guide/nginx-configuration/annotations.md b/docs/user-guide/nginx-configuration/annotations.md index 7cd34d344..072f09d2c 100755 --- a/docs/user-guide/nginx-configuration/annotations.md +++ b/docs/user-guide/nginx-configuration/annotations.md @@ -388,6 +388,10 @@ CORS can be controlled with the following annotations: !!! note For more information please see [https://enable-cors.org](https://enable-cors.org/server_nginx.html) +#### Handling the `Vary: Origin` Header + +When CORS support is enabled and the allowed origins contains either a wildcard host or more than one entry, the server will automatically add a `Vary: Origin` header to the response if the backend does not explicitly set one. The `Vary` header is used to indicate that the response may vary depending on the value of the `Origin` header in the request. This is important for caching and ensuring that responses are correctly handled based on the origin of the request. + ### HTTP2 Push Preload. Enables automatic conversion of preload links specified in the “Link” response header fields into push requests. diff --git a/internal/ingress/controller/template/template.go b/internal/ingress/controller/template/template.go index 7410ce6e0..0102eb651 100644 --- a/internal/ingress/controller/template/template.go +++ b/internal/ingress/controller/template/template.go @@ -283,6 +283,7 @@ var funcMap = text_template.FuncMap{ "shouldLoadAuthDigestModule": shouldLoadAuthDigestModule, "buildServerName": buildServerName, "buildCorsOriginRegex": buildCorsOriginRegex, + "isCorsVaryHeaderNeeded": isCorsVaryHeaderNeeded, } // escapeLiteralDollar will replace the $ character with ${literal_dollar} @@ -1754,3 +1755,10 @@ func buildCorsOriginRegex(corsOrigins []string) string { originsRegex += ")$ ) { set $cors 'true'; }" return originsRegex } + +func isCorsVaryHeaderNeeded(corsOrigins []string) bool { + hasMultipleOrigins := len(corsOrigins) > 1 + hasAnyOrigin := len(corsOrigins) == 1 && corsOrigins[0] == "*" + hasSingleDynamicOrigin := len(corsOrigins) == 1 && strings.Contains(corsOrigins[0], "*") + return hasMultipleOrigins || hasAnyOrigin || hasSingleDynamicOrigin +} diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl index 4454a4710..51f6f12c8 100644 --- a/rootfs/etc/nginx/template/nginx.tmpl +++ b/rootfs/etc/nginx/template/nginx.tmpl @@ -441,6 +441,14 @@ http { {{ end }} + # CORS with multiple allowed origin need to serve the 'Vary: Origin' header + # See https://github.com/kubernetes/ingress-nginx/issues/8469#issuecomment-1195714565 + # Only set it's value if it is not already set by the backend + map $upstream_http_vary $cors_vary { + default "$upstream_http_vary"; + '' "Origin"; + } + # Create a variable that contains the literal $ character. # This works because the geo module will not resolve variables. geo $literal_dollar { @@ -954,7 +962,8 @@ stream { more_set_headers 'Access-Control-Allow-Headers: {{ $cors.CorsAllowHeaders }}'; {{ if not (empty $cors.CorsExposeHeaders) }} more_set_headers 'Access-Control-Expose-Headers: {{ $cors.CorsExposeHeaders }}'; {{ end }} more_set_headers 'Access-Control-Max-Age: {{ $cors.CorsMaxAge }}'; - } + {{ if isCorsVaryHeaderNeeded $cors.CorsAllowOrigin }} more_set_headers 'Vary: $cors_vary'; {{ end }} + } if ($cors = "trueoptions") { more_set_headers 'Access-Control-Allow-Origin: $http_origin'; @@ -963,6 +972,7 @@ stream { more_set_headers 'Access-Control-Allow-Headers: {{ $cors.CorsAllowHeaders }}'; {{ if not (empty $cors.CorsExposeHeaders) }} more_set_headers 'Access-Control-Expose-Headers: {{ $cors.CorsExposeHeaders }}'; {{ end }} more_set_headers 'Access-Control-Max-Age: {{ $cors.CorsMaxAge }}'; + {{ if isCorsVaryHeaderNeeded $cors.CorsAllowOrigin }} more_set_headers 'Vary: $cors_vary'; {{ end }} more_set_headers 'Content-Type: text/plain charset=UTF-8'; more_set_headers 'Content-Length: 0'; return 204; diff --git a/test/e2e/annotations/cors.go b/test/e2e/annotations/cors.go index a14a5761f..ba9344352 100644 --- a/test/e2e/annotations/cors.go +++ b/test/e2e/annotations/cors.go @@ -31,7 +31,7 @@ const ( ) var _ = framework.DescribeAnnotation("cors-*", func() { - f := framework.NewDefaultFramework("cors") + f := framework.NewDefaultFramework("cors", framework.WithHTTPBunEnabled()) ginkgo.BeforeEach(func() { f.NewEchoDeployment(framework.WithDeploymentReplicas(2)) @@ -96,6 +96,134 @@ var _ = framework.DescribeAnnotation("cors-*", func() { }) }) + ginkgo.It("should include vary header - multiple origins", func() { + host := corsHost + origin := originHost + annotations := map[string]string{ + "nginx.ingress.kubernetes.io/enable-cors": "true", + "nginx.ingress.kubernetes.io/cors-allow-origin": "http://origin.com:8080, http://cors.foo.com", + } + + ing := framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, annotations) + f.EnsureIngress(ing) + + f.WaitForNginxServer(host, + func(server string) bool { + return strings.Contains(server, "more_set_headers 'Vary: $cors_vary';") + }) + + f.HTTPTestClient(). + GET("/"). + WithHeader("Host", host). + WithHeader("Origin", origin). + Expect(). + Headers(). + ValueEqual("Vary", []string{"Origin"}) + }) + + ginkgo.It("should include vary header - any origin", func() { + host := corsHost + origin := originHost + annotations := map[string]string{ + "nginx.ingress.kubernetes.io/enable-cors": "true", + "nginx.ingress.kubernetes.io/cors-allow-origin": "*", + } + + ing := framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, annotations) + f.EnsureIngress(ing) + + f.WaitForNginxServer(host, + func(server string) bool { + return strings.Contains(server, "more_set_headers 'Vary: $cors_vary';") + }) + + f.HTTPTestClient(). + GET("/"). + WithHeader("Host", host). + WithHeader("Origin", origin). + Expect(). + Headers(). + ValueEqual("Vary", []string{"Origin"}) + }) + + ginkgo.It("should include vary header - single dynamic origin", func() { + host := corsHost + origin := "http://foo.origin.cors.com" + annotations := map[string]string{ + "nginx.ingress.kubernetes.io/enable-cors": "true", + "nginx.ingress.kubernetes.io/cors-allow-origin": "http://*.origin.cors.com", + } + + ing := framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, annotations) + f.EnsureIngress(ing) + + f.WaitForNginxServer(host, + func(server string) bool { + return strings.Contains(server, "more_set_headers 'Vary: $cors_vary';") + }) + + f.HTTPTestClient(). + GET("/"). + WithHeader("Host", host). + WithHeader("Origin", origin). + Expect(). + Headers(). + ValueEqual("Vary", []string{"Origin"}) + }) + + ginkgo.It("should not include vary header - single static origin", func() { + host := corsHost + origin := originHost + annotations := map[string]string{ + "nginx.ingress.kubernetes.io/enable-cors": "true", + "nginx.ingress.kubernetes.io/cors-allow-origin": originHost, + } + + ing := framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, annotations) + f.EnsureIngress(ing) + + f.WaitForNginxServer(host, + func(server string) bool { + return !strings.Contains(server, "more_set_headers 'Vary: $cors_vary';") + }) + + f.HTTPTestClient(). + GET("/"). + WithHeader("Host", host). + WithHeader("Origin", origin). + Expect(). + Headers(). + NotContainsKey("Vary") + }) + + ginkgo.It("should preserve upstream vary headers", func() { + host := corsHost + origin := originHost + annotations := map[string]string{ + "nginx.ingress.kubernetes.io/enable-cors": "true", + "nginx.ingress.kubernetes.io/cors-allow-origin": "*", + } + + f.NewHttpbunDeployment(framework.WithDeploymentName("cors-service")) + f.EnsureIngress(framework.NewSingleIngress( + host, + "/", + host, + f.Namespace, + framework.HTTPBunService, + 80, + annotations)) + + f.HTTPTestClient(). + GET("/response-headers"). + WithQuery("Vary", "Origin, X-My-Custom-Header"). + WithHeader("Host", host). + WithHeader("Origin", origin). + Expect(). + Headers(). + ValueEqual("Vary", []string{"Origin, X-My-Custom-Header"}) + }) + ginkgo.It("should disable cors allow credentials", func() { host := corsHost annotations := map[string]string{