From fbcf205029bdbe59e8ada9805e14ce4d30266d4d Mon Sep 17 00:00:00 2001 From: supraja01 Date: Thu, 19 Oct 2017 17:05:44 -0700 Subject: [PATCH] provision to check access_token under custom header name in oauth2 along with migration scripts --- kong/plugins/oauth2/access.lua | 10 +- kong/plugins/oauth2/migrations/cassandra.lua | 22 +++- kong/plugins/oauth2/migrations/postgres.lua | 22 +++- kong/plugins/oauth2/schema.lua | 1 + spec/03-plugins/26-oauth2/01-schema_spec.lua | 20 ++- spec/03-plugins/26-oauth2/03-access_spec.lua | 130 ++++++++++++++++++- 6 files changed, 196 insertions(+), 9 deletions(-) diff --git a/kong/plugins/oauth2/access.lua b/kong/plugins/oauth2/access.lua index f296dc06b2d4..1eb79d3bf169 100644 --- a/kong/plugins/oauth2/access.lua +++ b/kong/plugins/oauth2/access.lua @@ -218,9 +218,9 @@ local function authorize(conf) }) end -local function retrieve_client_credentials(parameters) +local function retrieve_client_credentials(parameters, conf) local client_id, client_secret, from_authorization_header - local authorization_header = ngx.req.get_headers()["authorization"] + local authorization_header = ngx.req.get_headers()[conf.auth_header_name] if parameters[CLIENT_ID] and parameters[CLIENT_SECRET] then client_id = parameters[CLIENT_ID] client_secret = parameters[CLIENT_SECRET] @@ -271,7 +271,7 @@ local function issue_token(conf) response_params = {[ERROR] = "unsupported_grant_type", error_description = "Invalid " .. GRANT_TYPE} end - local client_id, client_secret, from_authorization_header = retrieve_client_credentials(parameters) + local client_id, client_secret, from_authorization_header = retrieve_client_credentials(parameters, conf) -- Check client_id and redirect_uri local allowed_redirect_uris, client = get_redirect_uri(client_id) @@ -405,7 +405,7 @@ local function parse_access_token(conf) local found_in = {} local result = retrieve_parameters()["access_token"] if not result then - local authorization = ngx.req.get_headers()["authorization"] + local authorization = ngx.req.get_headers()[conf.auth_header_name] if authorization then local parts = {} for v in authorization:gmatch("%S+") do -- Split by space @@ -420,7 +420,7 @@ local function parse_access_token(conf) if conf.hide_credentials then if found_in.authorization_header then - ngx.req.clear_header("authorization") + ngx.req.clear_header(conf.auth_header_name) else -- Remove from querystring local parameters = ngx.req.get_uri_args() diff --git a/kong/plugins/oauth2/migrations/cassandra.lua b/kong/plugins/oauth2/migrations/cassandra.lua index c721d30ad8dd..5817d0430cc7 100644 --- a/kong/plugins/oauth2/migrations/cassandra.lua +++ b/kong/plugins/oauth2/migrations/cassandra.lua @@ -1,3 +1,5 @@ +local plugin_config_iterator = require("kong.dao.migrations.helpers").plugin_config_iterator + return { { name = "2015-08-03-132400_init_oauth2", @@ -151,5 +153,23 @@ return { end end end - } + }, + { + name = "2017-10-19-set_auth_header_name_default", + up = function(_, _, dao) + for ok, config, update in plugin_config_iterator(dao, "oauth2") do + if not ok then + return config + end + if config.auth_header_name == nil then + config.auth_header_name = "authorization" + local _, err = update(config) + if err then + return err + end + end + end + end, + down = function(_, _, dao) end -- not implemented + }, } diff --git a/kong/plugins/oauth2/migrations/postgres.lua b/kong/plugins/oauth2/migrations/postgres.lua index cd4aa25cc107..220b37bf97b1 100644 --- a/kong/plugins/oauth2/migrations/postgres.lua +++ b/kong/plugins/oauth2/migrations/postgres.lua @@ -1,3 +1,5 @@ +local plugin_config_iterator = require("kong.dao.migrations.helpers").plugin_config_iterator + return { { name = "2015-08-03-132400_init_oauth2", @@ -164,5 +166,23 @@ return { down = [[ ALTER TABLE oauth2_credentials ADD CONSTRAINT oauth2_credentials_client_secret_key UNIQUE(client_secret); ]], - } + }, + { + name = "2017-10-19-set_auth_header_name_default", + up = function(_, _, dao) + for ok, config, update in plugin_config_iterator(dao, "oauth2") do + if not ok then + return config + end + if config.auth_header_name == nil then + config.auth_header_name = "authorization" + local _, err = update(config) + if err then + return err + end + end + end + end, + down = function(_, _, dao) end -- not implemented + }, } diff --git a/kong/plugins/oauth2/schema.lua b/kong/plugins/oauth2/schema.lua index f5314117a386..f558bb907612 100644 --- a/kong/plugins/oauth2/schema.lua +++ b/kong/plugins/oauth2/schema.lua @@ -31,6 +31,7 @@ return { accept_http_if_already_terminated = { required = false, type = "boolean", default = false }, anonymous = {type = "string", default = "", func = check_user}, global_credentials = {type = "boolean", default = false}, + auth_header_name = {required = false, type = "string", default = "authorization"}, }, self_check = function(schema, plugin_t, dao, is_update) if not plugin_t.enable_authorization_code and not plugin_t.enable_implicit_grant diff --git a/spec/03-plugins/26-oauth2/01-schema_spec.lua b/spec/03-plugins/26-oauth2/01-schema_spec.lua index 867b4b253a8c..41eeb9e604f4 100644 --- a/spec/03-plugins/26-oauth2/01-schema_spec.lua +++ b/spec/03-plugins/26-oauth2/01-schema_spec.lua @@ -28,7 +28,25 @@ describe("Plugin: oauth2 (schema)", function() assert.truthy(t.provision_key) assert.equal("hello", t.provision_key) end) - + it("sets default `auth_header_name` when not given", function() + local t = {enable_authorization_code = true, mandatory_scope = true, scopes = {"email", "info"}} + local ok, errors = validate_entity(t, oauth2_schema) + assert.True(ok) + assert.is_nil(errors) + assert.truthy(t.provision_key) + assert.equal(32, t.provision_key:len()) + assert.equal("authorization", t.auth_header_name) + end) + it("does not set default value for `auth_header_name` when it is given", function() + local t = {enable_authorization_code = true, mandatory_scope = true, scopes = {"email", "info"}, provision_key = "hello", + auth_header_name="custom_header_name"} + local ok, errors = validate_entity(t, oauth2_schema) + assert.True(ok) + assert.is_nil(errors) + assert.truthy(t.provision_key) + assert.equal("hello", t.provision_key) + assert.equal("custom_header_name", t.auth_header_name) + end) describe("errors", function() it("requires at least one flow", function() local ok, _, err = validate_entity({}, oauth2_schema) diff --git a/spec/03-plugins/26-oauth2/03-access_spec.lua b/spec/03-plugins/26-oauth2/03-access_spec.lua index 684bfde659b5..2a8a804d2f6c 100644 --- a/spec/03-plugins/26-oauth2/03-access_spec.lua +++ b/spec/03-plugins/26-oauth2/03-access_spec.lua @@ -96,6 +96,13 @@ describe("Plugin: oauth2 (access)", function() name = "testapp3", consumer_id = consumer.id }) + assert(helpers.dao.oauth2_credentials:insert { + client_id = "clientid1011", + client_secret = "secret1011", + redirect_uri = "http://google.com/kong", + name = "testapp31", + consumer_id = consumer.id + }) local api1 = assert(helpers.dao.apis:insert { name = "api-1", @@ -302,7 +309,45 @@ describe("Plugin: oauth2 (access)", function() anonymous = utils.uuid(), -- a non existing consumer }, }) - + local api11 = assert(helpers.dao.apis:insert { + name = "oauth2_11.com", + hosts = { "oauth2_11.com" }, + upstream_url = helpers.mock_upstream_url, + }) + assert(helpers.dao.plugins:insert { + name = "oauth2", + api_id = api11.id, + config = { + scopes = { "email", "profile", "user.email" }, + enable_authorization_code = true, + mandatory_scope = true, + provision_key = "provision123", + token_expiration = 7, + enable_implicit_grant = true, + global_credentials = true, + auth_header_name = "custom_header_name", + }, + }) + local api12 = assert(helpers.dao.apis:insert { + name = "oauth2_12.com", + hosts = { "oauth2_12.com" }, + upstream_url = helpers.mock_upstream_url, + }) + assert(helpers.dao.plugins:insert { + name = "oauth2", + api_id = api12.id, + config = { + scopes = { "email", "profile", "user.email" }, + enable_authorization_code = true, + mandatory_scope = true, + provision_key = "provision123", + token_expiration = 7, + enable_implicit_grant = true, + global_credentials = true, + auth_header_name = "custom_header_name", + hide_credentials = true, + }, + }) assert(helpers.start_kong({ trusted_ips = "127.0.0.1", nginx_conf = "spec/fixtures/custom_nginx.template", @@ -805,6 +850,35 @@ describe("Plugin: oauth2 (access)", function() assert.are.equal(5, data[1].expires_in) assert.falsy(data[1].refresh_token) end) + it("returns success and the token should have the right expiration when a custom header is passed", function() + local res = assert(proxy_ssl_client:send { + method = "POST", + path = "/oauth2/authorize", + body = { + provision_key = "provision123", + authenticated_userid = "id123", + client_id = "clientid1011", + scope = "email", + response_type = "token" + }, + headers = { + ["Host"] = "oauth2_11.com", + ["Content-Type"] = "application/json" + } + }) + local body = cjson.decode(assert.res_status(200, res)) + assert.is_table(ngx.re.match(body.redirect_uri, "^http://google\\.com/kong\\#access_token=[\\w]{32,32}&expires_in=[\\d]+&token_type=bearer$")) + + local iterator, err = ngx.re.gmatch(body.redirect_uri, "^http://google\\.com/kong\\#access_token=([\\w]{32,32})&expires_in=[\\d]+&token_type=bearer$") + assert.is_nil(err) + local m, err = iterator() + assert.is_nil(err) + local data = helpers.dao.oauth2_tokens:find_all {access_token = m[1]} + assert.are.equal(1, #data) + assert.are.equal(m[1], data[1].access_token) + assert.are.equal(7, data[1].expires_in) + assert.falsy(data[1].refresh_token) + end) it("returns success and store authenticated user properties", function() local res = assert(proxy_ssl_client:send { method = "POST", @@ -1695,6 +1769,32 @@ describe("Plugin: oauth2 (access)", function() }) assert.res_status(200, res) end) + it("work when a correct access_token is being sent in the custom header", function() + local token = provision_token("oauth2_11.com",nil,"clientid1011","secret1011") + + local res = assert(proxy_ssl_client:send { + method = "GET", + path = "/request", + headers = { + ["Host"] = "oauth2_11.com", + ["custom_header_name"] = "bearer " .. token.access_token, + } + }) + assert.res_status(200, res) + end) + it("fail when a correct access_token is being sent in the wrong header", function() + local token = provision_token("oauth2_11.com",nil,"clientid1011","secret1011") + + local res = assert(proxy_ssl_client:send { + method = "GET", + path = "/request", + headers = { + ["Host"] = "oauth2_11.com", + ["authorization"] = "bearer " .. token.access_token, + } + }) + assert.res_status(401, res) + end) it("does not work when requesting a different API", function() local token = provision_token() @@ -2124,6 +2224,19 @@ describe("Plugin: oauth2 (access)", function() local body = cjson.decode(assert.res_status(200, res)) assert.is_nil(body.uri_args.access_token) end) + it("hides credentials in the querystring for api with custom header", function() + local token = provision_token("oauth2_12.com",nil,"clientid1011","secret1011") + + local res = assert(proxy_client:send { + method = "GET", + path = "/request?access_token=" .. token.access_token, + headers = { + ["Host"] = "oauth2_12.com" + } + }) + local body = cjson.decode(assert.res_status(200, res)) + assert.is_nil(body.uri_args.access_token) + end) it("does not hide credentials in the header", function() local token = provision_token() @@ -2152,6 +2265,21 @@ describe("Plugin: oauth2 (access)", function() local body = cjson.decode(assert.res_status(200, res)) assert.is_nil(body.headers.authorization) end) + it("hides credentials in the custom header", function() + local token = provision_token("oauth2_12.com",nil,"clientid1011","secret1011") + + local res = assert(proxy_client:send { + method = "GET", + path = "/request", + headers = { + ["Host"] = "oauth2_12.com", + ["custom_header_name"] = "bearer " .. token.access_token + } + }) + local body = cjson.decode(assert.res_status(200, res)) + assert.is_nil(body.headers.authorization) + assert.is_nil(body.headers.custom_header_name) + end) it("does not abort when the request body is a multipart form upload", function() local token = provision_token("oauth2_3.com")