Merge pull request #764 from Mashape/fix/https

Added "accept_http_if_already_terminated" to SSL and OAuth 2.0

kong/plugins/oauth2/access.lua

@@ -76,10 +76,15 @@ local function get_redirect_uri(client_id)
   return client and client.redirect_uri or nil, client
 end
 
-local function is_https()
-  local forwarded_proto_header = ngx.req.get_headers()["x-forwarded-proto"]
+local HTTPS = "https"
 
-  return ngx.var.scheme:lower() == "https" or (forwarded_proto_header and forwarded_proto_header:lower() == "https")
+local function is_https(conf)
+  local result = ngx.var.scheme:lower() == HTTPS
+  if not result and conf.accept_http_if_already_terminated then
+    local forwarded_proto_header = ngx.req.get_headers()["x-forwarded-proto"]
+    result = forwarded_proto_header and forwarded_proto_header:lower() == HTTPS
+  end
+  return result
 end
 
 local function retrieve_parameters()
@@ -113,7 +118,7 @@ local function authorize(conf)
   local state = parameters[STATE]
   local redirect_uri, client
 
-  if not is_https() then
+  if not is_https(conf) then
     response_params = {[ERROR] = "access_denied", error_description = "You must use HTTPS"}
   else
     if conf.provision_key ~= parameters.provision_key then
@@ -214,7 +219,7 @@ local function issue_token(conf)
   local parameters = retrieve_parameters() --TODO: Also from authorization header
   local state = parameters[STATE]
 
-  if not is_https() then
+  if not is_https(conf) then
     response_params = {[ERROR] = "access_denied", error_description = "You must use HTTPS"}
   else
     local grant_type = parameters[GRANT_TYPE]

kong/plugins/oauth2/schema.lua

@@ -26,6 +26,7 @@ return {
     enable_implicit_grant = { required = true, type = "boolean", default = false },
     enable_client_credentials = { required = true, type = "boolean", default = false },
     enable_password_grant = { required = true, type = "boolean", default = false },
-    hide_credentials = { type = "boolean", default = false }
+    hide_credentials = { type = "boolean", default = false },
+    accept_http_if_already_terminated = { required = false, type = "boolean", default = false }
   }
 }

kong/plugins/ssl/access.lua

@@ -2,8 +2,19 @@ local responses = require "kong.tools.responses"
 
 local _M = {}
 
+local HTTPS = "https"
+
+local function is_https(conf)
+  local result = ngx.var.scheme:lower() == HTTPS
+  if not result and conf.accept_http_if_already_terminated then
+    local forwarded_proto_header = ngx.req.get_headers()["x-forwarded-proto"]
+    result = forwarded_proto_header and forwarded_proto_header:lower() == HTTPS
+  end
+  return result
+end
+
 function _M.execute(conf)
-  if conf.only_https and ngx.var.scheme:lower() ~= "https" then
+  if conf.only_https and not is_https(conf) then
     ngx.header["connection"] = { "Upgrade" }
     ngx.header["upgrade"] = "TLS/1.0, HTTP/1.1"
     return responses.send(426, {message="Please use HTTPS protocol"})

kong/plugins/ssl/schema.lua

@@ -23,6 +23,7 @@ return {
     cert = { required = true, type = "string", func = validate_cert },
     key = { required = true, type = "string", func = validate_key },
     only_https = { required = false, type = "boolean", default = false },
+    accept_http_if_already_terminated = { required = false, type = "boolean", default = false },
 
     -- Internal use
     _cert_der_cache = { type = "string", immutable = true },

spec/plugins/oauth2/access_spec.lua

@@ -44,7 +44,8 @@ describe("Authentication Plugin", function()
         { name = "tests-oauth2-with-path", request_host = "mockbin-path.com", upstream_url = "http://mockbin.com", request_path = "/somepath/" },
         { name = "tests-oauth2-with-hide-credentials", request_host = "oauth2_3.com", upstream_url = "http://mockbin.com" },
         { name = "tests-oauth2-client-credentials", request_host = "oauth2_4.com", upstream_url = "http://mockbin.com" },
-        { name = "tests-oauth2-password-grant", request_host = "oauth2_5.com", upstream_url = "http://mockbin.com" }
+        { name = "tests-oauth2-password-grant", request_host = "oauth2_5.com", upstream_url = "http://mockbin.com" },
+        { name = "tests-oauth2-accept_http_if_already_terminated", request_host = "oauth2_6.com", upstream_url = "http://mockbin.com" },
       },
       consumer = {
         { username = "auth_tests_consumer" }
@@ -54,7 +55,8 @@ describe("Authentication Plugin", function()
         { name = "oauth2", config = { scopes = { "email", "profile" }, mandatory_scope = true, provision_key = "provision123", token_expiration = 5, enable_implicit_grant = true }, __api = 2 },
         { name = "oauth2", config = { scopes = { "email", "profile" }, mandatory_scope = true, provision_key = "provision123", token_expiration = 5, enable_implicit_grant = true, hide_credentials = true }, __api = 3 },
         { name = "oauth2", config = { scopes = { "email", "profile" }, mandatory_scope = true, provision_key = "provision123", token_expiration = 5, enable_client_credentials = true, enable_authorization_code = false }, __api = 4 },
-        { name = "oauth2", config = { scopes = { "email", "profile" }, mandatory_scope = true, provision_key = "provision123", token_expiration = 5, enable_password_grant = true, enable_authorization_code = false }, __api = 5 }
+        { name = "oauth2", config = { scopes = { "email", "profile" }, mandatory_scope = true, provision_key = "provision123", token_expiration = 5, enable_password_grant = true, enable_authorization_code = false }, __api = 5 },
+        { name = "oauth2", config = { scopes = { "email", "profile", "user.email" }, mandatory_scope = true, provision_key = "provision123", token_expiration = 5, enable_implicit_grant = true, accept_http_if_already_terminated = true }, __api = 6 },
       },
       oauth2_credential = {
         { client_id = "clientid123", client_secret = "secret123", redirect_uri = "http://google.com/kong", name="testapp", __consumer = 1 }
@@ -163,14 +165,23 @@ describe("Authentication Plugin", function()
         assert.are.equal("You must use HTTPS", body.error_description)
       end)
 
-      it("should return success when under HTTP and X-Forwarded-Proto header is set to HTTPS", function()
-        local response, status = http_client.post(PROXY_URL.."/oauth2/authorize", { provision_key = "provision123", authenticated_userid = "id123", client_id = "clientid123", scope = "email", response_type = "code" }, {host = "oauth2.com", ["X-Forwarded-Proto"] = "https"})
+      it("should work when not under HTTPS but accept_http_if_already_terminated is true", function()
+        local response, status = http_client.post(PROXY_URL.."/oauth2/authorize", { provision_key = "provision123", authenticated_userid = "id123", client_id = "clientid123", scope = "email", response_type = "code" }, {host = "oauth2_6.com", ["X-Forwarded-Proto"] = "https"})
         local body = cjson.decode(response)
         assert.are.equal(200, status)
         assert.are.equal(1, utils.table_size(body))
         assert.truthy(rex.match(body.redirect_uri, "^http://google\\.com/kong\\?code=[\\w]{32,32}$"))
       end)
 
+      it("should fail when not under HTTPS and accept_http_if_already_terminated is false", function()
+        local response, status = http_client.post(PROXY_URL.."/oauth2/authorize", { provision_key = "provision123", authenticated_userid = "id123", client_id = "clientid123", scope = "email", response_type = "code" }, {host = "oauth2.com", ["X-Forwarded-Proto"] = "https"})
+        local body = cjson.decode(response)
+        assert.are.equal(400, status)
+        assert.are.equal(2, utils.table_size(body))
+        assert.are.equal("access_denied", body.error)
+        assert.are.equal("You must use HTTPS", body.error_description)
+      end)
+
       it("should return success", function()
         local response, status = http_client.post(PROXY_SSL_URL.."/oauth2/authorize", { provision_key = "provision123", authenticated_userid = "id123", client_id = "clientid123", scope = "email", response_type = "code" }, {host = "oauth2.com"})
         local body = cjson.decode(response)

spec/plugins/ssl/access_spec.lua

@@ -18,11 +18,13 @@ describe("SSL Plugin", function()
       api = {
         { name = "ssl-test", request_host = "ssl1.com", upstream_url = "http://mockbin.com" },
         { name = "ssl-test2", request_host = "ssl2.com", upstream_url = "http://mockbin.com" },
-        { name = "ssl-test3", request_host = "ssl3.com", upstream_url = "http://mockbin.com" }
+        { name = "ssl-test3", request_host = "ssl3.com", upstream_url = "http://mockbin.com" },
+        { name = "ssl-test4", request_host = "ssl4.com", upstream_url = "http://mockbin.com" },
       },
       plugin = {
         { name = "ssl", config = { cert = ssl_fixtures.cert, key = ssl_fixtures.key }, __api = 1 },
-        { name = "ssl", config = { cert = ssl_fixtures.cert, key = ssl_fixtures.key, only_https = true }, __api = 2 }
+        { name = "ssl", config = { cert = ssl_fixtures.cert, key = ssl_fixtures.key, only_https = true }, __api = 2 },
+        { name = "ssl", config = { cert = ssl_fixtures.cert, key = ssl_fixtures.key, only_https = true, accept_http_if_already_terminated = true }, __api = 4 }
       }
     }
 
@@ -86,6 +88,21 @@ describe("SSL Plugin", function()
       assert.are.equal(200, status)
     end)
 
+    it("should block request with https in x-forwarded-proto but no accept_if_already_terminated", function()
+      local _, status = http_client.get(STUB_GET_URL, nil, { host = "ssl2.com", ["x-forwarded-proto"] = "https" })
+      assert.are.equal(426, status)
+    end)
+
+    it("should not block request with https", function()
+      local _, status = http_client.get(STUB_GET_URL, nil, { host = "ssl4.com", ["x-forwarded-proto"] = "https" })
+      assert.are.equal(200, status)
+    end)
+
+    it("should not block request with https in x-forwarded-proto but accept_if_already_terminated", function()
+      local _, status = http_client.get(STUB_GET_URL, nil, { host = "ssl4.com", ["x-forwarded-proto"] = "https" })
+      assert.are.equal(200, status)
+    end)
+
   end)
 
   describe("should work with curl", function()