From 735212b1dd77dab104541f53521a47e76c6ef063 Mon Sep 17 00:00:00 2001 From: Nick Muerdter Date: Sat, 25 Nov 2017 10:17:48 -0700 Subject: [PATCH] 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. --- CHANGELOG.md | 1 + .../proxy/middleware/https_validator.lua | 4 + .../proxy/middleware/rewrite_response.lua | 35 +++++- .../response_rewriting/test_redirects.rb | 11 +- test/proxy/test_forwarded_port_headers.rb | 101 ++++++++++++++++++ 5 files changed, 147 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3cda6a341..b5b5ae3b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Fixed - **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. +- **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. ## 0.14.4 (2017-07-15) diff --git a/src/api-umbrella/proxy/middleware/https_validator.lua b/src/api-umbrella/proxy/middleware/https_validator.lua index 979411561..afab09242 100644 --- a/src/api-umbrella/proxy/middleware/https_validator.lua +++ b/src/api-umbrella/proxy/middleware/https_validator.lua @@ -10,6 +10,10 @@ return function(settings, user) -- continue. return nil else + if settings["redirect_https"] then + return ngx.redirect(httpsify_current_url(), ngx.HTTP_MOVED_PERMANENTLY) + end + local mode = settings["require_https"] if not mode or mode == "optional" then -- Continue if https isn't required. diff --git a/src/api-umbrella/proxy/middleware/rewrite_response.lua b/src/api-umbrella/proxy/middleware/rewrite_response.lua index be05bf254..d615214a7 100644 --- a/src/api-umbrella/proxy/middleware/rewrite_response.lua +++ b/src/api-umbrella/proxy/middleware/rewrite_response.lua @@ -90,8 +90,39 @@ local function rewrite_redirects() local changed = false if host_matches then - parsed["authority"] = matched_api["frontend_host"] - parsed["host"] = nil + -- For wildcard hosts, keep the same host as on the incoming request. For + -- all others, use the frontend host declared on the API. + local host + if matched_api["frontend_host"] == "*" then + host = ngx.ctx.host_normalized + else + host = matched_api["_frontend_host_normalized"] + end + + local scheme = parsed["scheme"] + if scheme == "http" and config["override_public_http_proto"] then + scheme = config["override_public_http_proto"] + elseif scheme == "https" and config["override_public_https_proto"] then + scheme = config["override_public_https_proto"] + end + + local port + if scheme == "https" then + port = config["override_public_https_port"] or config["https_port"] + if port == 443 then + port = nil + end + elseif scheme == "http" then + port = config["override_public_http_port"] or config["http_port"] + if port == 80 then + port = nil + end + end + + parsed["scheme"] = scheme + parsed["host"] = host + parsed["port"] = port + parsed["authority"] = nil changed = true end diff --git a/test/proxy/response_rewriting/test_redirects.rb b/test/proxy/response_rewriting/test_redirects.rb index ccf24c132..54e8159df 100644 --- a/test/proxy/response_rewriting/test_redirects.rb +++ b/test/proxy/response_rewriting/test_redirects.rb @@ -36,7 +36,12 @@ def test_absolute_leaves_unknown_domain def test_absolute_rewrites_backend_domain response = make_redirect_request("http://example.com/hello") - assert_equal("http://frontend.foo/hello?api_key=#{api_key}", response.headers["location"]) + assert_equal("http://frontend.foo:9080/hello?api_key=#{api_key}", response.headers["location"]) + end + + def test_absolute_rewrites_backend_domain_https + response = make_redirect_request("https://example.com/hello") + assert_equal("https://frontend.foo:9081/hello?api_key=#{api_key}", response.headers["location"]) end def test_absolute_requires_full_domain_match @@ -46,7 +51,7 @@ def test_absolute_requires_full_domain_match def test_absolute_rewrites_frontend_prefix_path response = make_redirect_request("http://example.com/backend-prefix/") - assert_equal("http://frontend.foo/#{unique_test_class_id}/front/end/path/?api_key=#{api_key}", response.headers["location"]) + assert_equal("http://frontend.foo:9080/#{unique_test_class_id}/front/end/path/?api_key=#{api_key}", response.headers["location"]) end def test_relative_unknown_path_leaves_query_params @@ -61,7 +66,7 @@ def test_relative_rewrite_keeps_query_params def test_absolute_rewrite_keeps_query_params response = make_redirect_request("http://example.com/?some=param&and=another") - assert_equal("http://frontend.foo/?some=param&and=another&api_key=#{api_key}", response.headers["location"]) + assert_equal("http://frontend.foo:9080/?some=param&and=another&api_key=#{api_key}", response.headers["location"]) end def test_leaves_empty_redirect diff --git a/test/proxy/test_forwarded_port_headers.rb b/test/proxy/test_forwarded_port_headers.rb index b475752de..1b9adf9ec 100644 --- a/test/proxy/test_forwarded_port_headers.rb +++ b/test/proxy/test_forwarded_port_headers.rb @@ -22,6 +22,14 @@ def setup Admin.delete_all FactoryBot.create(:admin) override_config_set(@default_config, ["--router", "--web"]) + prepend_api_backends([ + { + :frontend_host => "frontend.foo", + :backend_host => "example.com", + :servers => [{ :host => "127.0.0.1", :port => 9444 }], + :url_matches => [{ :frontend_prefix => "/#{unique_test_class_id}/front/end/path", :backend_prefix => "/backend-prefix" }], + }, + ]) end end @@ -46,6 +54,12 @@ def test_default_headers when :admin_oauth2_http assert_response_code(301, response) assert_equal("https://127.0.0.1:9081/admins/auth/google_oauth2", response.headers["Location"]) + when :api_backend_redirect_http + assert_response_code(302, response) + assert_equal("http://frontend.foo:9080/hello?api_key=#{api_key}", response.headers["Location"]) + when :api_backend_redirect_https + assert_response_code(302, response) + assert_equal("https://frontend.foo:9081/hello?api_key=#{api_key}", response.headers["Location"]) when :website_https assert_response_code(200, response) when :website_http @@ -78,6 +92,12 @@ def test_forwarded_port when :admin_oauth2_http assert_response_code(301, response) assert_equal("https://127.0.0.1:9081/admins/auth/google_oauth2", response.headers["Location"]) + when :api_backend_redirect_http + assert_response_code(302, response) + assert_equal("http://frontend.foo:9080/hello?api_key=#{api_key}", response.headers["Location"]) + when :api_backend_redirect_https + assert_response_code(302, response) + assert_equal("https://frontend.foo:9081/hello?api_key=#{api_key}", response.headers["Location"]) when :website_https assert_response_code(200, response) when :website_http @@ -105,6 +125,12 @@ def test_forwarded_proto_http when :admin_oauth2_https, :admin_oauth2_http assert_response_code(301, response) assert_equal("https://127.0.0.1:9081/admins/auth/google_oauth2", response.headers["Location"]) + when :api_backend_redirect_http + assert_response_code(302, response) + assert_equal("http://frontend.foo:9080/hello?api_key=#{api_key}", response.headers["Location"]) + when :api_backend_redirect_https + assert_response_code(302, response) + assert_equal("https://frontend.foo:9081/hello?api_key=#{api_key}", response.headers["Location"]) when :website_https, :website_http assert_response_code(301, response) assert_equal("https://127.0.0.1:9081/", response.headers["Location"]) @@ -130,6 +156,12 @@ def test_forwarded_proto_https when :admin_oauth2_http assert_response_code(302, response) assert_oauth2_redirect_uri("https://127.0.0.1:9080/admins/auth/google_oauth2/callback", response) + when :api_backend_redirect_http + assert_response_code(302, response) + assert_equal("http://frontend.foo:9080/hello?api_key=#{api_key}", response.headers["Location"]) + when :api_backend_redirect_https + assert_response_code(302, response) + assert_equal("https://frontend.foo:9081/hello?api_key=#{api_key}", response.headers["Location"]) when :website_https, :website_http assert_response_code(200, response) when :website_signup_https, :website_signup_http @@ -151,6 +183,12 @@ def test_forwarded_proto_http_and_port when :admin_oauth2_https, :admin_oauth2_http assert_response_code(301, response) assert_equal("https://127.0.0.1:9081/admins/auth/google_oauth2", response.headers["Location"]) + when :api_backend_redirect_http + assert_response_code(302, response) + assert_equal("http://frontend.foo:9080/hello?api_key=#{api_key}", response.headers["Location"]) + when :api_backend_redirect_https + assert_response_code(302, response) + assert_equal("https://frontend.foo:9081/hello?api_key=#{api_key}", response.headers["Location"]) when :website_https, :website_http assert_response_code(301, response) assert_equal("https://127.0.0.1:9081/", response.headers["Location"]) @@ -173,6 +211,12 @@ def test_forwarded_proto_https_and_port when :admin_oauth2_https, :admin_oauth2_http assert_response_code(302, response) assert_oauth2_redirect_uri("https://127.0.0.1:1111/admins/auth/google_oauth2/callback", response) + when :api_backend_redirect_http + assert_response_code(302, response) + assert_equal("http://frontend.foo:9080/hello?api_key=#{api_key}", response.headers["Location"]) + when :api_backend_redirect_https + assert_response_code(302, response) + assert_equal("https://frontend.foo:9081/hello?api_key=#{api_key}", response.headers["Location"]) when :website_https, :website_http assert_response_code(200, response) when :website_signup_https, :website_signup_http @@ -202,6 +246,12 @@ def test_override_public_http_port when :admin_oauth2_http assert_response_code(301, response) assert_equal("https://127.0.0.1:9081/admins/auth/google_oauth2", response.headers["Location"]) + when :api_backend_redirect_http + assert_response_code(302, response) + assert_equal("http://frontend.foo:2222/hello?api_key=#{api_key}", response.headers["Location"]) + when :api_backend_redirect_https + assert_response_code(302, response) + assert_equal("https://frontend.foo:9081/hello?api_key=#{api_key}", response.headers["Location"]) when :website_https assert_response_code(200, response) when :website_http @@ -238,6 +288,12 @@ def test_override_public_https_port when :admin_oauth2_http assert_response_code(301, response) assert_equal("https://127.0.0.1:3333/admins/auth/google_oauth2", response.headers["Location"]) + when :api_backend_redirect_http + assert_response_code(302, response) + assert_equal("http://frontend.foo:9080/hello?api_key=#{api_key}", response.headers["Location"]) + when :api_backend_redirect_https + assert_response_code(302, response) + assert_equal("https://frontend.foo:3333/hello?api_key=#{api_key}", response.headers["Location"]) when :website_https assert_response_code(200, response) when :website_http @@ -274,6 +330,12 @@ def test_override_public_http_proto when :admin_oauth2_http assert_response_code(302, response) assert_oauth2_redirect_uri("https://127.0.0.1:9080/admins/auth/google_oauth2/callback", response) + when :api_backend_redirect_http + assert_response_code(302, response) + assert_equal("https://frontend.foo:9081/hello?api_key=#{api_key}", response.headers["Location"]) + when :api_backend_redirect_https + assert_response_code(302, response) + assert_equal("https://frontend.foo:9081/hello?api_key=#{api_key}", response.headers["Location"]) when :website_https assert_response_code(301, response) assert_equal("https://127.0.0.1:9081/", response.headers["Location"]) @@ -310,6 +372,12 @@ def test_override_public_https_proto when :admin_oauth2_http assert_response_code(302, response) assert_oauth2_redirect_uri("https://127.0.0.1:9080/admins/auth/google_oauth2/callback", response) + when :api_backend_redirect_http + assert_response_code(302, response) + assert_equal("http://frontend.foo:9080/hello?api_key=#{api_key}", response.headers["Location"]) + when :api_backend_redirect_https + assert_response_code(302, response) + assert_equal("http://frontend.foo:9080/hello?api_key=#{api_key}", response.headers["Location"]) when :website_https assert_response_code(301, response) assert_equal("https://127.0.0.1:9081/", response.headers["Location"]) @@ -347,6 +415,12 @@ def test_override_public_ports_defaults when :admin_oauth2_http assert_response_code(301, response) assert_equal("https://127.0.0.1/admins/auth/google_oauth2", response.headers["Location"]) + when :api_backend_redirect_http + assert_response_code(302, response) + assert_equal("http://frontend.foo/hello?api_key=#{api_key}", response.headers["Location"]) + when :api_backend_redirect_https + assert_response_code(302, response) + assert_equal("https://frontend.foo/hello?api_key=#{api_key}", response.headers["Location"]) when :website_https assert_response_code(200, response) when :website_http @@ -380,6 +454,9 @@ def test_override_public_ports_and_proto_ssl_terminator when :admin_oauth2_https, :admin_oauth2_http assert_response_code(302, response) assert_oauth2_redirect_uri("https://127.0.0.1/admins/auth/google_oauth2/callback", response) + when :api_backend_redirect_http, :api_backend_redirect_https + assert_response_code(302, response) + assert_equal("https://frontend.foo/hello?api_key=#{api_key}", response.headers["Location"]) when :website_https, :website_http assert_response_code(200, response) when :website_signup_https, :website_signup_http @@ -405,6 +482,8 @@ def make_requests(headers) :admin_http => admin_http_request(headers), :admin_oauth2_https => admin_oauth2_https_request(headers), :admin_oauth2_http => admin_oauth2_http_request(headers), + :api_backend_redirect_http => api_backend_redirect_http(headers), + :api_backend_redirect_https => api_backend_redirect_https(headers), :website_https => website_https_request(headers), :website_http => website_http_request(headers), :website_signup_https => website_signup_https_request(headers), @@ -428,6 +507,28 @@ def admin_oauth2_http_request(headers = {}) Typhoeus.get("http://127.0.0.1:9080/admins/auth/google_oauth2", keyless_http_options.deep_merge(:headers => headers)) end + def api_backend_redirect_http(headers = {}) + Typhoeus.get("http://127.0.0.1:9080/#{unique_test_class_id}/front/end/path/redirect", http_options.deep_merge({ + :headers => { + "Host" => "frontend.foo", + }, + :params => { + :to => "http://example.com/hello", + }, + }).deep_merge(:headers => headers)) + end + + def api_backend_redirect_https(headers = {}) + Typhoeus.get("http://127.0.0.1:9080/#{unique_test_class_id}/front/end/path/redirect", http_options.deep_merge({ + :headers => { + "Host" => "frontend.foo", + }, + :params => { + :to => "https://example.com/hello", + }, + }).deep_merge(:headers => headers)) + end + def website_https_request(headers = {}) Typhoeus.get("https://127.0.0.1:9081/", keyless_http_options.deep_merge(:headers => headers)) end