From 7908a29a220d70f9eaed0cba54ed8aea85073003 Mon Sep 17 00:00:00 2001 From: Adam Sunderland Date: Fri, 22 Mar 2024 18:10:41 -0400 Subject: [PATCH 1/4] Allow any protocol for cors --- internal/ingress/annotations/cors/main.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/ingress/annotations/cors/main.go b/internal/ingress/annotations/cors/main.go index d6e92b34db..aa03c2d732 100644 --- a/internal/ingress/annotations/cors/main.go +++ b/internal/ingress/annotations/cors/main.go @@ -43,9 +43,9 @@ var ( // * Sets a group that can be (https?://)?*?.something.com:port? // * Allows this to be repeated as much as possible, and separated by comma // Otherwise it should be '*' - corsOriginRegexValidator = regexp.MustCompile(`^((((https?://)?(\*\.)?[A-Za-z0-9\-.]*(:\d+)?,?)+)|\*)?$`) + corsOriginRegexValidator = regexp.MustCompile(`^(((([a-z]+://)?(\*\.)?[A-Za-z0-9\-.]*(:\d+)?,?)+)|\*)?$`) // corsOriginRegex defines the regex for validation inside Parse - corsOriginRegex = regexp.MustCompile(`^(https?://(\*\.)?[A-Za-z0-9\-.]*(:\d+)?|\*)?$`) + corsOriginRegex = regexp.MustCompile(`^([a-z]+://(\*\.)?[A-Za-z0-9\-.]*(:\d+)?|\*)?$`) // Method must contain valid methods list (PUT, GET, POST, BLA) // May contain or not spaces between each verb corsMethodsRegex = regexp.MustCompile(`^([A-Za-z]+,?\s?)+$`) From 8c01758bdd039b1f3440d6c352967d188dd181a9 Mon Sep 17 00:00:00 2001 From: Adam Sunderland Date: Fri, 22 Mar 2024 18:16:27 -0400 Subject: [PATCH 2/4] Add a test for non-http protocol --- .../ingress/annotations/cors/main_test.go | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/internal/ingress/annotations/cors/main_test.go b/internal/ingress/annotations/cors/main_test.go index a69390a174..a924919576 100644 --- a/internal/ingress/annotations/cors/main_test.go +++ b/internal/ingress/annotations/cors/main_test.go @@ -203,3 +203,33 @@ func TestIngresCorsConfigAllowOriginWithTrailingComma(t *testing.T) { t.Errorf("expected %v but returned %v", expectedCorsAllowOrigins, nginxCors.CorsAllowOrigin) } } + +func TestIngressCorsConfigAllowOriginWithNonHttpProtocol(t *testing.T) { + ing := buildIngress() + + data := map[string]string{} + data[parser.GetAnnotationWithPrefix(corsEnableAnnotation)] = "true" + + // Include a trailing comma and an empty value between the commas. + data[parser.GetAnnotationWithPrefix(corsAllowOriginAnnotation)] = "test://localhost" + ing.SetAnnotations(data) + + corst, err := NewParser(&resolver.Mock{}).Parse(ing) + if err != nil { + t.Errorf("error parsing annotations: %v", err) + } + + nginxCors, ok := corst.(*Config) + if !ok { + t.Errorf("expected a Config type but returned %t", corst) + } + + if !nginxCors.CorsEnabled { + t.Errorf("expected %v but returned %v", data[parser.GetAnnotationWithPrefix(corsEnableAnnotation)], nginxCors.CorsEnabled) + } + + expectedCorsAllowOrigins := []string{"test://localhost"} + if !reflect.DeepEqual(nginxCors.CorsAllowOrigin, expectedCorsAllowOrigins) { + t.Errorf("expected %v but returned %v", expectedCorsAllowOrigins, nginxCors.CorsAllowOrigin) + } +} From 43243827594ac7205d2b411706778366675dcc82 Mon Sep 17 00:00:00 2001 From: Adam Sunderland Date: Sat, 23 Mar 2024 08:33:30 -0400 Subject: [PATCH 3/4] Update docs and add e2e test --- .../nginx-configuration/annotations.md | 8 ++--- internal/ingress/annotations/cors/main.go | 5 ++-- test/e2e/annotations/cors.go | 29 +++++++++++++++++++ 3 files changed, 36 insertions(+), 6 deletions(-) diff --git a/docs/user-guide/nginx-configuration/annotations.md b/docs/user-guide/nginx-configuration/annotations.md index 184c4993be..ff88e450d8 100755 --- a/docs/user-guide/nginx-configuration/annotations.md +++ b/docs/user-guide/nginx-configuration/annotations.md @@ -375,13 +375,13 @@ CORS can be controlled with the following annotations: * `nginx.ingress.kubernetes.io/cors-allow-origin`: Controls what's the accepted Origin for CORS. - This is a multi-valued field, separated by ','. It must follow this format: `http(s)://origin-site.com` or `http(s)://origin-site.com:port` + This is a multi-valued field, separated by ','. It must follow this format: `protocol://origin-site.com` or `protocol://origin-site.com:port` - Default: `*` - - Example: `nginx.ingress.kubernetes.io/cors-allow-origin: "https://origin-site.com:4443, http://origin-site.com, https://example.org:1199"` + - Example: `nginx.ingress.kubernetes.io/cors-allow-origin: "https://origin-site.com:4443, http://origin-site.com, myprotocol://example.org:1199"` - It also supports single level wildcard subdomains and follows this format: `http(s)://*.foo.bar`, `http(s)://*.bar.foo:8080` or `http(s)://*.abc.bar.foo:9000` - - Example: `nginx.ingress.kubernetes.io/cors-allow-origin: "https://*.origin-site.com:4443, http://*.origin-site.com, https://example.org:1199"` + It also supports single level wildcard subdomains and follows this format: `protocol://*.foo.bar`, `protocol://*.bar.foo:8080` or `protocol://*.abc.bar.foo:9000` + - Example: `nginx.ingress.kubernetes.io/cors-allow-origin: "https://*.origin-site.com:4443, http://*.origin-site.com, myprotocol://example.org:1199"` * `nginx.ingress.kubernetes.io/cors-allow-credentials`: Controls if credentials can be passed during CORS operations. diff --git a/internal/ingress/annotations/cors/main.go b/internal/ingress/annotations/cors/main.go index aa03c2d732..b815148203 100644 --- a/internal/ingress/annotations/cors/main.go +++ b/internal/ingress/annotations/cors/main.go @@ -78,8 +78,9 @@ var corsAnnotation = parser.Annotation{ Scope: parser.AnnotationScopeIngress, Risk: parser.AnnotationRiskMedium, Documentation: `This annotation controls what's the accepted Origin for CORS. - This is a multi-valued field, separated by ','. It must follow this format: http(s)://origin-site.com or http(s)://origin-site.com:port - It also supports single level wildcard subdomains and follows this format: http(s)://*.foo.bar, http(s)://*.bar.foo:8080 or http(s)://*.abc.bar.foo:9000`, + This is a multi-valued field, separated by ','. It must follow this format: protocol://origin-site.com or protocol://origin-site.com:port + It also supports single level wildcard subdomains and follows this format: https://*.foo.bar, http://*.bar.foo:8080 or myprotocol://*.abc.bar.foo:9000 + Protocol can be any lowercase string, like http, https, or mycustomprotocol.`, }, corsAllowHeadersAnnotation: { Validator: parser.ValidateRegex(parser.HeadersVariable, true), diff --git a/test/e2e/annotations/cors.go b/test/e2e/annotations/cors.go index a14a5761fd..58f4445f70 100644 --- a/test/e2e/annotations/cors.go +++ b/test/e2e/annotations/cors.go @@ -669,4 +669,33 @@ var _ = framework.DescribeAnnotation("cors-*", func() { Headers(). NotContainsKey("Access-Control-Allow-Origin") }) + + ginkgo.It("should allow - origins with non-http[s] protocols", func() { + host := corsHost + origin := "test://localhost" + origin2 := "tauri://localhost:3000" + annotations := map[string]string{ + "nginx.ingress.kubernetes.io/enable-cors": "true", + "nginx.ingress.kubernetes.io/cors-allow-origin": "test://localhost, tauri://localhost:3000", + } + + ing := framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, annotations) + f.EnsureIngress(ing) + + f.HTTPTestClient(). + GET("/"). + WithHeader("Host", host). + WithHeader("Origin", origin). + Expect(). + Status(http.StatusOK).Headers(). + ValueEqual("Access-Control-Allow-Origin", []string{"test://localhost"}) + + f.HTTPTestClient(). + GET("/"). + WithHeader("Host", host). + WithHeader("Origin", origin2). + Expect(). + Status(http.StatusOK).Headers(). + ValueEqual("Access-Control-Allow-Origin", []string{"tauri://localhost:3000"}) + }) }) From 1a22978f2ea324a1f404c7ecc7445468729bf579 Mon Sep 17 00:00:00 2001 From: Adam Sunderland Date: Wed, 27 Mar 2024 13:46:36 -0400 Subject: [PATCH 4/4] Extract repetitive string to constant --- internal/ingress/annotations/cors/main_test.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/internal/ingress/annotations/cors/main_test.go b/internal/ingress/annotations/cors/main_test.go index a924919576..dee36fcae8 100644 --- a/internal/ingress/annotations/cors/main_test.go +++ b/internal/ingress/annotations/cors/main_test.go @@ -27,6 +27,8 @@ import ( "k8s.io/ingress-nginx/internal/ingress/resolver" ) +const enableAnnotation = "true" + func buildIngress() *networking.Ingress { defaultBackend := networking.IngressBackend{ Service: &networking.IngressServiceBackend{ @@ -76,7 +78,7 @@ func TestIngressCorsConfigValid(t *testing.T) { data := map[string]string{} // Valid - data[parser.GetAnnotationWithPrefix(corsEnableAnnotation)] = "true" + data[parser.GetAnnotationWithPrefix(corsEnableAnnotation)] = enableAnnotation data[parser.GetAnnotationWithPrefix(corsAllowHeadersAnnotation)] = "DNT,X-CustomHeader, Keep-Alive,User-Agent" data[parser.GetAnnotationWithPrefix(corsAllowCredentialsAnnotation)] = "false" data[parser.GetAnnotationWithPrefix(corsAllowMethodsAnnotation)] = "GET, PATCH" @@ -178,7 +180,7 @@ func TestIngresCorsConfigAllowOriginWithTrailingComma(t *testing.T) { ing := buildIngress() data := map[string]string{} - data[parser.GetAnnotationWithPrefix(corsEnableAnnotation)] = "true" + data[parser.GetAnnotationWithPrefix(corsEnableAnnotation)] = enableAnnotation // Include a trailing comma and an empty value between the commas. data[parser.GetAnnotationWithPrefix(corsAllowOriginAnnotation)] = "https://origin123.test.com:4443, ,https://origin321.test.com:4443," @@ -208,7 +210,7 @@ func TestIngressCorsConfigAllowOriginWithNonHttpProtocol(t *testing.T) { ing := buildIngress() data := map[string]string{} - data[parser.GetAnnotationWithPrefix(corsEnableAnnotation)] = "true" + data[parser.GetAnnotationWithPrefix(corsEnableAnnotation)] = enableAnnotation // Include a trailing comma and an empty value between the commas. data[parser.GetAnnotationWithPrefix(corsAllowOriginAnnotation)] = "test://localhost"