Skip to content

Commit c81f6d7

Browse files
elizabeth-devGacko
authored andcommitted
Template: Fix faulty CORS headers handling.
1 parent 9569a76 commit c81f6d7

File tree

5 files changed

+290
-379
lines changed

5 files changed

+290
-379
lines changed

internal/ingress/controller/template/template.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,7 @@ var funcMap = text_template.FuncMap{
300300
"getenv": os.Getenv,
301301
"contains": strings.Contains,
302302
"split": strings.Split,
303+
"join": strings.Join,
303304
"hasPrefix": strings.HasPrefix,
304305
"hasSuffix": strings.HasSuffix,
305306
"trimSpace": strings.TrimSpace,
@@ -1688,10 +1689,10 @@ func buildOriginRegex(origin string) string {
16881689

16891690
func buildCorsOriginRegex(corsOrigins []string) string {
16901691
if len(corsOrigins) == 1 && corsOrigins[0] == "*" {
1691-
return "set $http_origin *;\nset $cors 'true';"
1692+
return ".*"
16921693
}
16931694

1694-
originsRegex := "if ($http_origin ~* ("
1695+
originsRegex := "("
16951696
for i, origin := range corsOrigins {
16961697
originTrimmed := strings.TrimSpace(origin)
16971698
if originTrimmed != "" {
@@ -1702,6 +1703,6 @@ func buildCorsOriginRegex(corsOrigins []string) string {
17021703
}
17031704
}
17041705
}
1705-
originsRegex += ")$ ) { set $cors 'true'; }"
1706+
originsRegex += ")"
17061707
return originsRegex
17071708
}

internal/ingress/controller/template/template_test.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1971,3 +1971,36 @@ func TestCleanConf(t *testing.T) {
19711971
t.Errorf("cleanConf result don't match with expected: %s", diff)
19721972
}
19731973
}
1974+
1975+
func TestBuildCorsOriginRegex(t *testing.T) {
1976+
origins := []string{"http://foo.bar "}
1977+
1978+
result := buildCorsOriginRegex(origins)
1979+
1980+
expected := `((http://foo\.bar))`
1981+
if result != expected {
1982+
t.Errorf("expected '%v' but returned '%v'", expected, result)
1983+
}
1984+
}
1985+
1986+
func TestBuildCorsOriginRegexWithMultipleOrigins(t *testing.T) {
1987+
origins := []string{" http://foo.bar", "http://*.bar"}
1988+
1989+
result := buildCorsOriginRegex(origins)
1990+
1991+
expected := `((http://foo\.bar)|(http://[A-Za-z0-9\-]+\.bar))`
1992+
if result != expected {
1993+
t.Errorf("expected '%v' but returned '%v'", expected, result)
1994+
}
1995+
}
1996+
1997+
func TestBuildCorsOriginRegexWithWildcard(t *testing.T) {
1998+
origins := []string{"*"}
1999+
2000+
result := buildCorsOriginRegex(origins)
2001+
2002+
expected := `.*`
2003+
if result != expected {
2004+
t.Errorf("expected '%v' but returned '%v'", expected, result)
2005+
}
2006+
}
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
function handle_cors(req) {
2+
const originsRegex = new RegExp(`${req.variables.cors_origins_regex}$`, 'i');
3+
4+
if (originsRegex.test(req.headersIn['Origin'])) {
5+
const allowedOrigins = req.variables.cors_allowed_origins.split(',');
6+
7+
req.headersOut['Access-Control-Allow-Origin'] = allowedOrigins.length === 1 && allowedOrigins[0] === '*' ? '*' : req.headersIn['Origin'];
8+
req.headersOut['Access-Control-Allow-Methods'] = req.variables.cors_allow_methods;
9+
req.headersOut['Access-Control-Allow-Headers'] = req.variables.cors_allow_headers;
10+
req.headersOut['Access-Control-Max-Age'] = req.variables.cors_max_age;
11+
if (req.variables.cors_allow_credentials) req.headersOut['Access-Control-Allow-Credentials'] = req.variables.cors_allow_credentials;
12+
if (req.variables.cors_expose_headers) req.headersOut['Access-Control-Expose-Headers'] = req.variables.cors_expose_headers;
13+
14+
if (req.method === 'OPTIONS') {
15+
req.headersOut['Content-Type'] = 'text/plain charset=UTF-8';
16+
req.headersOut['Content-Length'] = '0';
17+
}
18+
}
19+
}
20+
21+
export default {handle_cors};

rootfs/etc/nginx/template/nginx.tmpl

Lines changed: 15 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
# setup custom paths that do not require root access
1313
pid {{ .PID }};
1414

15+
load_module /etc/nginx/modules/ngx_http_js_module.so;
16+
1517
{{ if $cfg.UseGeoIP2 }}
1618
load_module /etc/nginx/modules/ngx_http_geoip2_module.so;
1719
{{ end }}
@@ -74,6 +76,8 @@ http {
7476

7577
init_worker_by_lua_file /etc/nginx/lua/ngx_conf_init_worker.lua;
7678

79+
js_import njs_handle_cors from /etc/nginx/js/nginx/ngx_handle_cors.js;
80+
7781
{{/* Enable the real_ip module only if we use either X-Forwarded headers or Proxy Protocol. */}}
7882
{{/* we use the value of the real IP for the geo_ip module */}}
7983
{{ if or (or $cfg.UseForwardedHeaders $cfg.UseProxyProtocol) $cfg.EnableRealIP }}
@@ -872,33 +876,19 @@ stream {
872876
{{/* CORS support from https://michielkalkman.com/snippets/nginx-cors-open-configuration.html */}}
873877
{{ define "CORS" }}
874878
{{ $cors := .CorsConfig }}
875-
# Cors Preflight methods needs additional options and different Return Code
876-
{{ if $cors.CorsAllowOrigin }}
877-
{{ buildCorsOriginRegex $cors.CorsAllowOrigin }}
878-
{{ end }}
879-
if ($request_method = 'OPTIONS') {
880-
set $cors ${cors}options;
881-
}
882879

883-
if ($cors = "true") {
884-
more_set_headers 'Access-Control-Allow-Origin: $http_origin';
885-
{{ if $cors.CorsAllowCredentials }} more_set_headers 'Access-Control-Allow-Credentials: {{ $cors.CorsAllowCredentials }}'; {{ end }}
886-
more_set_headers 'Access-Control-Allow-Methods: {{ $cors.CorsAllowMethods }}';
887-
more_set_headers 'Access-Control-Allow-Headers: {{ $cors.CorsAllowHeaders }}';
888-
{{ if not (empty $cors.CorsExposeHeaders) }} more_set_headers 'Access-Control-Expose-Headers: {{ $cors.CorsExposeHeaders }}'; {{ end }}
889-
more_set_headers 'Access-Control-Max-Age: {{ $cors.CorsMaxAge }}';
890-
}
880+
set $cors_origins_regex '{{ buildCorsOriginRegex $cors.CorsAllowOrigin }}';
881+
set $cors_allowed_origins '{{ join $cors.CorsAllowOrigin "," }}';
882+
set $cors_allow_methods '{{ $cors.CorsAllowMethods }}';
883+
set $cors_allow_headers '{{ $cors.CorsAllowHeaders }}';
884+
set $cors_max_age '{{ $cors.CorsMaxAge }}';
885+
{{ if $cors.CorsAllowCredentials }} set $cors_allow_credentials {{ $cors.CorsAllowCredentials }}; {{ end }}
886+
{{ if not (empty $cors.CorsExposeHeaders) }} set $cors_expose_headers '{{ $cors.CorsExposeHeaders }}'; {{ end }}
887+
888+
js_header_filter njs_handle_cors.handle_cors;
891889

892-
if ($cors = "trueoptions") {
893-
more_set_headers 'Access-Control-Allow-Origin: $http_origin';
894-
{{ if $cors.CorsAllowCredentials }} more_set_headers 'Access-Control-Allow-Credentials: {{ $cors.CorsAllowCredentials }}'; {{ end }}
895-
more_set_headers 'Access-Control-Allow-Methods: {{ $cors.CorsAllowMethods }}';
896-
more_set_headers 'Access-Control-Allow-Headers: {{ $cors.CorsAllowHeaders }}';
897-
{{ if not (empty $cors.CorsExposeHeaders) }} more_set_headers 'Access-Control-Expose-Headers: {{ $cors.CorsExposeHeaders }}'; {{ end }}
898-
more_set_headers 'Access-Control-Max-Age: {{ $cors.CorsMaxAge }}';
899-
more_set_headers 'Content-Type: text/plain charset=UTF-8';
900-
more_set_headers 'Content-Length: 0';
901-
return 204;
890+
if ($request_method = 'OPTIONS') {
891+
return 204;
902892
}
903893
{{ end }}
904894

0 commit comments

Comments
 (0)