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

@@ -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)
 

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.

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
 

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

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