Skip to content

Commit

Permalink
Allow any protocol for cors origins (#11153)
Browse files Browse the repository at this point in the history
Co-authored-by: Ricardo Katz <[email protected]>
  • Loading branch information
iterion and rikatz authored Aug 31, 2024
1 parent 6ca67b5 commit 2cec241
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 10 deletions.
8 changes: 4 additions & 4 deletions docs/user-guide/nginx-configuration/annotations.md
Original file line number Diff line number Diff line change
Expand Up @@ -387,13 +387,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.

Expand Down
9 changes: 5 additions & 4 deletions internal/ingress/annotations/cors/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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?)+$`)
Expand Down Expand Up @@ -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),
Expand Down
36 changes: 34 additions & 2 deletions internal/ingress/annotations/cors/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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,"
Expand All @@ -203,3 +205,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)] = enableAnnotation

// 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)
}
}
29 changes: 29 additions & 0 deletions test/e2e/annotations/cors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"})
})
})

0 comments on commit 2cec241

Please sign in to comment.