Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added "accept_http_if_already_terminated" to SSL and OAuth 2.0 #764

Merged
merged 1 commit into from
Dec 2, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions kong/plugins/oauth2/access.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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]
Expand Down
3 changes: 2 additions & 1 deletion kong/plugins/oauth2/schema.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
}
}
13 changes: 12 additions & 1 deletion kong/plugins/ssl/access.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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"})
Expand Down
1 change: 1 addition & 0 deletions kong/plugins/ssl/schema.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
Expand Down
19 changes: 15 additions & 4 deletions spec/plugins/oauth2/access_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand All @@ -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 }
Expand Down Expand Up @@ -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)
Expand Down
21 changes: 19 additions & 2 deletions spec/plugins/ssl/access_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
}
}

Expand Down Expand Up @@ -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()
Expand Down