Skip to content

Commit 1fe7fe4

Browse files
elizabeth-devGacko
authored andcommitted
Template: Fix faulty CORS headers handling.
1 parent 98e761a commit 1fe7fe4

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,
@@ -1689,10 +1690,10 @@ func buildOriginRegex(origin string) string {
16891690

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

1695-
originsRegex := "if ($http_origin ~* ("
1696+
originsRegex := "("
16961697
for i, origin := range corsOrigins {
16971698
originTrimmed := strings.TrimSpace(origin)
16981699
if originTrimmed != "" {
@@ -1703,6 +1704,6 @@ func buildCorsOriginRegex(corsOrigins []string) string {
17031704
}
17041705
}
17051706
}
1706-
originsRegex += ")$ ) { set $cors 'true'; }"
1707+
originsRegex += ")"
17071708
return originsRegex
17081709
}

internal/ingress/controller/template/template_test.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1984,3 +1984,36 @@ func TestCleanConf(t *testing.T) {
19841984
t.Errorf("cleanConf result don't match with expected: %s", diff)
19851985
}
19861986
}
1987+
1988+
func TestBuildCorsOriginRegex(t *testing.T) {
1989+
origins := []string{"http://foo.bar "}
1990+
1991+
result := buildCorsOriginRegex(origins)
1992+
1993+
expected := `((http://foo\.bar))`
1994+
if result != expected {
1995+
t.Errorf("expected '%v' but returned '%v'", expected, result)
1996+
}
1997+
}
1998+
1999+
func TestBuildCorsOriginRegexWithMultipleOrigins(t *testing.T) {
2000+
origins := []string{" http://foo.bar", "http://*.bar"}
2001+
2002+
result := buildCorsOriginRegex(origins)
2003+
2004+
expected := `((http://foo\.bar)|(http://[A-Za-z0-9\-]+\.bar))`
2005+
if result != expected {
2006+
t.Errorf("expected '%v' but returned '%v'", expected, result)
2007+
}
2008+
}
2009+
2010+
func TestBuildCorsOriginRegexWithWildcard(t *testing.T) {
2011+
origins := []string{"*"}
2012+
2013+
result := buildCorsOriginRegex(origins)
2014+
2015+
expected := `.*`
2016+
if result != expected {
2017+
t.Errorf("expected '%v' but returned '%v'", expected, result)
2018+
}
2019+
}
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)