Skip to content

Commit 735212b

Browse files
GUIGUI
committed
Fixes for redirect rewriting and port handling.
Various fixes around HTTP/HTTPS redirect rewriting behavior from API backends. Custom ports were previously not being included in the rewritten redirects. This should now be fixed so the custom ports (as well as overridden ports are now taken into account). This was an existing production issue, and not something specific to this postgres branch we're currently working on.
1 parent 3e44b1a commit 735212b

File tree

5 files changed

+147
-5
lines changed

5 files changed

+147
-5
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
### Fixed
66

77
- **Fix URL handling for query strings containing "api\_key":** It was possible that API Umbrella was stripping the string "api\_key" from inside URLs before passing requests to the API backend in some unexpected cases. The `api_key` query parameter should still be stripped, but other instances of "api\_key" elsewhere in the URL (for example as a value, like `?foo=api_key`), are now retained.
8+
- **Fix redirect rewriting when operating on custom ports:** If API Umbrella was running on custom HTTP or HTTP ports, redirects from API backends may not have been to the correct port.
89

910
## 0.14.4 (2017-07-15)
1011

‎src/api-umbrella/proxy/middleware/https_validator.lua

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,10 @@ return function(settings, user)
1010
-- continue.
1111
return nil
1212
else
13+
if settings["redirect_https"] then
14+
return ngx.redirect(httpsify_current_url(), ngx.HTTP_MOVED_PERMANENTLY)
15+
end
16+
1317
local mode = settings["require_https"]
1418
if not mode or mode == "optional" then
1519
-- Continue if https isn't required.

‎src/api-umbrella/proxy/middleware/rewrite_response.lua

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,39 @@ local function rewrite_redirects()
9090
local changed = false
9191

9292
if host_matches then
93-
parsed["authority"] = matched_api["frontend_host"]
94-
parsed["host"] = nil
93+
-- For wildcard hosts, keep the same host as on the incoming request. For
94+
-- all others, use the frontend host declared on the API.
95+
local host
96+
if matched_api["frontend_host"] == "*" then
97+
host = ngx.ctx.host_normalized
98+
else
99+
host = matched_api["_frontend_host_normalized"]
100+
end
101+
102+
local scheme = parsed["scheme"]
103+
if scheme == "http" and config["override_public_http_proto"] then
104+
scheme = config["override_public_http_proto"]
105+
elseif scheme == "https" and config["override_public_https_proto"] then
106+
scheme = config["override_public_https_proto"]
107+
end
108+
109+
local port
110+
if scheme == "https" then
111+
port = config["override_public_https_port"] or config["https_port"]
112+
if port == 443 then
113+
port = nil
114+
end
115+
elseif scheme == "http" then
116+
port = config["override_public_http_port"] or config["http_port"]
117+
if port == 80 then
118+
port = nil
119+
end
120+
end
121+
122+
parsed["scheme"] = scheme
123+
parsed["host"] = host
124+
parsed["port"] = port
125+
parsed["authority"] = nil
95126
changed = true
96127
end
97128

‎test/proxy/response_rewriting/test_redirects.rb

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,12 @@ def test_absolute_leaves_unknown_domain
3636

3737
def test_absolute_rewrites_backend_domain
3838
response = make_redirect_request("http://example.com/hello")
39-
assert_equal("http://frontend.foo/hello?api_key=#{api_key}", response.headers["location"])
39+
assert_equal("http://frontend.foo:9080/hello?api_key=#{api_key}", response.headers["location"])
40+
end
41+
42+
def test_absolute_rewrites_backend_domain_https
43+
response = make_redirect_request("https://example.com/hello")
44+
assert_equal("https://frontend.foo:9081/hello?api_key=#{api_key}", response.headers["location"])
4045
end
4146

4247
def test_absolute_requires_full_domain_match
@@ -46,7 +51,7 @@ def test_absolute_requires_full_domain_match
4651

4752
def test_absolute_rewrites_frontend_prefix_path
4853
response = make_redirect_request("http://example.com/backend-prefix/")
49-
assert_equal("http://frontend.foo/#{unique_test_class_id}/front/end/path/?api_key=#{api_key}", response.headers["location"])
54+
assert_equal("http://frontend.foo:9080/#{unique_test_class_id}/front/end/path/?api_key=#{api_key}", response.headers["location"])
5055
end
5156

5257
def test_relative_unknown_path_leaves_query_params
@@ -61,7 +66,7 @@ def test_relative_rewrite_keeps_query_params
6166

6267
def test_absolute_rewrite_keeps_query_params
6368
response = make_redirect_request("http://example.com/?some=param&and=another")
64-
assert_equal("http://frontend.foo/?some=param&and=another&api_key=#{api_key}", response.headers["location"])
69+
assert_equal("http://frontend.foo:9080/?some=param&and=another&api_key=#{api_key}", response.headers["location"])
6570
end
6671

6772
def test_leaves_empty_redirect

‎test/proxy/test_forwarded_port_headers.rb

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,14 @@ def setup
2222
Admin.delete_all
2323
FactoryBot.create(:admin)
2424
override_config_set(@default_config, ["--router", "--web"])
25+
prepend_api_backends([
26+
{
27+
:frontend_host => "frontend.foo",
28+
:backend_host => "example.com",
29+
:servers => [{ :host => "127.0.0.1", :port => 9444 }],
30+
:url_matches => [{ :frontend_prefix => "/#{unique_test_class_id}/front/end/path", :backend_prefix => "/backend-prefix" }],
31+
},
32+
])
2533
end
2634
end
2735

@@ -46,6 +54,12 @@ def test_default_headers
4654
when :admin_oauth2_http
4755
assert_response_code(301, response)
4856
assert_equal("https://127.0.0.1:9081/admins/auth/google_oauth2", response.headers["Location"])
57+
when :api_backend_redirect_http
58+
assert_response_code(302, response)
59+
assert_equal("http://frontend.foo:9080/hello?api_key=#{api_key}", response.headers["Location"])
60+
when :api_backend_redirect_https
61+
assert_response_code(302, response)
62+
assert_equal("https://frontend.foo:9081/hello?api_key=#{api_key}", response.headers["Location"])
4963
when :website_https
5064
assert_response_code(200, response)
5165
when :website_http
@@ -78,6 +92,12 @@ def test_forwarded_port
7892
when :admin_oauth2_http
7993
assert_response_code(301, response)
8094
assert_equal("https://127.0.0.1:9081/admins/auth/google_oauth2", response.headers["Location"])
95+
when :api_backend_redirect_http
96+
assert_response_code(302, response)
97+
assert_equal("http://frontend.foo:9080/hello?api_key=#{api_key}", response.headers["Location"])
98+
when :api_backend_redirect_https
99+
assert_response_code(302, response)
100+
assert_equal("https://frontend.foo:9081/hello?api_key=#{api_key}", response.headers["Location"])
81101
when :website_https
82102
assert_response_code(200, response)
83103
when :website_http
@@ -105,6 +125,12 @@ def test_forwarded_proto_http
105125
when :admin_oauth2_https, :admin_oauth2_http
106126
assert_response_code(301, response)
107127
assert_equal("https://127.0.0.1:9081/admins/auth/google_oauth2", response.headers["Location"])
128+
when :api_backend_redirect_http
129+
assert_response_code(302, response)
130+
assert_equal("http://frontend.foo:9080/hello?api_key=#{api_key}", response.headers["Location"])
131+
when :api_backend_redirect_https
132+
assert_response_code(302, response)
133+
assert_equal("https://frontend.foo:9081/hello?api_key=#{api_key}", response.headers["Location"])
108134
when :website_https, :website_http
109135
assert_response_code(301, response)
110136
assert_equal("https://127.0.0.1:9081/", response.headers["Location"])
@@ -130,6 +156,12 @@ def test_forwarded_proto_https
130156
when :admin_oauth2_http
131157
assert_response_code(302, response)
132158
assert_oauth2_redirect_uri("https://127.0.0.1:9080/admins/auth/google_oauth2/callback", response)
159+
when :api_backend_redirect_http
160+
assert_response_code(302, response)
161+
assert_equal("http://frontend.foo:9080/hello?api_key=#{api_key}", response.headers["Location"])
162+
when :api_backend_redirect_https
163+
assert_response_code(302, response)
164+
assert_equal("https://frontend.foo:9081/hello?api_key=#{api_key}", response.headers["Location"])
133165
when :website_https, :website_http
134166
assert_response_code(200, response)
135167
when :website_signup_https, :website_signup_http
@@ -151,6 +183,12 @@ def test_forwarded_proto_http_and_port
151183
when :admin_oauth2_https, :admin_oauth2_http
152184
assert_response_code(301, response)
153185
assert_equal("https://127.0.0.1:9081/admins/auth/google_oauth2", response.headers["Location"])
186+
when :api_backend_redirect_http
187+
assert_response_code(302, response)
188+
assert_equal("http://frontend.foo:9080/hello?api_key=#{api_key}", response.headers["Location"])
189+
when :api_backend_redirect_https
190+
assert_response_code(302, response)
191+
assert_equal("https://frontend.foo:9081/hello?api_key=#{api_key}", response.headers["Location"])
154192
when :website_https, :website_http
155193
assert_response_code(301, response)
156194
assert_equal("https://127.0.0.1:9081/", response.headers["Location"])
@@ -173,6 +211,12 @@ def test_forwarded_proto_https_and_port
173211
when :admin_oauth2_https, :admin_oauth2_http
174212
assert_response_code(302, response)
175213
assert_oauth2_redirect_uri("https://127.0.0.1:1111/admins/auth/google_oauth2/callback", response)
214+
when :api_backend_redirect_http
215+
assert_response_code(302, response)
216+
assert_equal("http://frontend.foo:9080/hello?api_key=#{api_key}", response.headers["Location"])
217+
when :api_backend_redirect_https
218+
assert_response_code(302, response)
219+
assert_equal("https://frontend.foo:9081/hello?api_key=#{api_key}", response.headers["Location"])
176220
when :website_https, :website_http
177221
assert_response_code(200, response)
178222
when :website_signup_https, :website_signup_http
@@ -202,6 +246,12 @@ def test_override_public_http_port
202246
when :admin_oauth2_http
203247
assert_response_code(301, response)
204248
assert_equal("https://127.0.0.1:9081/admins/auth/google_oauth2", response.headers["Location"])
249+
when :api_backend_redirect_http
250+
assert_response_code(302, response)
251+
assert_equal("http://frontend.foo:2222/hello?api_key=#{api_key}", response.headers["Location"])
252+
when :api_backend_redirect_https
253+
assert_response_code(302, response)
254+
assert_equal("https://frontend.foo:9081/hello?api_key=#{api_key}", response.headers["Location"])
205255
when :website_https
206256
assert_response_code(200, response)
207257
when :website_http
@@ -238,6 +288,12 @@ def test_override_public_https_port
238288
when :admin_oauth2_http
239289
assert_response_code(301, response)
240290
assert_equal("https://127.0.0.1:3333/admins/auth/google_oauth2", response.headers["Location"])
291+
when :api_backend_redirect_http
292+
assert_response_code(302, response)
293+
assert_equal("http://frontend.foo:9080/hello?api_key=#{api_key}", response.headers["Location"])
294+
when :api_backend_redirect_https
295+
assert_response_code(302, response)
296+
assert_equal("https://frontend.foo:3333/hello?api_key=#{api_key}", response.headers["Location"])
241297
when :website_https
242298
assert_response_code(200, response)
243299
when :website_http
@@ -274,6 +330,12 @@ def test_override_public_http_proto
274330
when :admin_oauth2_http
275331
assert_response_code(302, response)
276332
assert_oauth2_redirect_uri("https://127.0.0.1:9080/admins/auth/google_oauth2/callback", response)
333+
when :api_backend_redirect_http
334+
assert_response_code(302, response)
335+
assert_equal("https://frontend.foo:9081/hello?api_key=#{api_key}", response.headers["Location"])
336+
when :api_backend_redirect_https
337+
assert_response_code(302, response)
338+
assert_equal("https://frontend.foo:9081/hello?api_key=#{api_key}", response.headers["Location"])
277339
when :website_https
278340
assert_response_code(301, response)
279341
assert_equal("https://127.0.0.1:9081/", response.headers["Location"])
@@ -310,6 +372,12 @@ def test_override_public_https_proto
310372
when :admin_oauth2_http
311373
assert_response_code(302, response)
312374
assert_oauth2_redirect_uri("https://127.0.0.1:9080/admins/auth/google_oauth2/callback", response)
375+
when :api_backend_redirect_http
376+
assert_response_code(302, response)
377+
assert_equal("http://frontend.foo:9080/hello?api_key=#{api_key}", response.headers["Location"])
378+
when :api_backend_redirect_https
379+
assert_response_code(302, response)
380+
assert_equal("http://frontend.foo:9080/hello?api_key=#{api_key}", response.headers["Location"])
313381
when :website_https
314382
assert_response_code(301, response)
315383
assert_equal("https://127.0.0.1:9081/", response.headers["Location"])
@@ -347,6 +415,12 @@ def test_override_public_ports_defaults
347415
when :admin_oauth2_http
348416
assert_response_code(301, response)
349417
assert_equal("https://127.0.0.1/admins/auth/google_oauth2", response.headers["Location"])
418+
when :api_backend_redirect_http
419+
assert_response_code(302, response)
420+
assert_equal("http://frontend.foo/hello?api_key=#{api_key}", response.headers["Location"])
421+
when :api_backend_redirect_https
422+
assert_response_code(302, response)
423+
assert_equal("https://frontend.foo/hello?api_key=#{api_key}", response.headers["Location"])
350424
when :website_https
351425
assert_response_code(200, response)
352426
when :website_http
@@ -380,6 +454,9 @@ def test_override_public_ports_and_proto_ssl_terminator
380454
when :admin_oauth2_https, :admin_oauth2_http
381455
assert_response_code(302, response)
382456
assert_oauth2_redirect_uri("https://127.0.0.1/admins/auth/google_oauth2/callback", response)
457+
when :api_backend_redirect_http, :api_backend_redirect_https
458+
assert_response_code(302, response)
459+
assert_equal("https://frontend.foo/hello?api_key=#{api_key}", response.headers["Location"])
383460
when :website_https, :website_http
384461
assert_response_code(200, response)
385462
when :website_signup_https, :website_signup_http
@@ -405,6 +482,8 @@ def make_requests(headers)
405482
:admin_http => admin_http_request(headers),
406483
:admin_oauth2_https => admin_oauth2_https_request(headers),
407484
:admin_oauth2_http => admin_oauth2_http_request(headers),
485+
:api_backend_redirect_http => api_backend_redirect_http(headers),
486+
:api_backend_redirect_https => api_backend_redirect_https(headers),
408487
:website_https => website_https_request(headers),
409488
:website_http => website_http_request(headers),
410489
:website_signup_https => website_signup_https_request(headers),
@@ -428,6 +507,28 @@ def admin_oauth2_http_request(headers = {})
428507
Typhoeus.get("http://127.0.0.1:9080/admins/auth/google_oauth2", keyless_http_options.deep_merge(:headers => headers))
429508
end
430509

510+
def api_backend_redirect_http(headers = {})
511+
Typhoeus.get("http://127.0.0.1:9080/#{unique_test_class_id}/front/end/path/redirect", http_options.deep_merge({
512+
:headers => {
513+
"Host" => "frontend.foo",
514+
},
515+
:params => {
516+
:to => "http://example.com/hello",
517+
},
518+
}).deep_merge(:headers => headers))
519+
end
520+
521+
def api_backend_redirect_https(headers = {})
522+
Typhoeus.get("http://127.0.0.1:9080/#{unique_test_class_id}/front/end/path/redirect", http_options.deep_merge({
523+
:headers => {
524+
"Host" => "frontend.foo",
525+
},
526+
:params => {
527+
:to => "https://example.com/hello",
528+
},
529+
}).deep_merge(:headers => headers))
530+
end
531+
431532
def website_https_request(headers = {})
432533
Typhoeus.get("https://127.0.0.1:9081/", keyless_http_options.deep_merge(:headers => headers))
433534
end

0 commit comments

Comments
 (0)