diff --git a/src/apisix/plugins/bk-auth-verify.lua b/src/apisix/plugins/bk-auth-verify.lua index 3d06c73..d1f7927 100644 --- a/src/apisix/plugins/bk-auth-verify.lua +++ b/src/apisix/plugins/bk-auth-verify.lua @@ -118,11 +118,25 @@ local function get_auth_params_from_request(ctx, authorization_keys) if err ~= nil then return nil, err elseif auth_params ~= nil then + -- 记录认证参数位置，便于统计哪些请求将认证参数放到请求参数，推动优化 + ctx.var.auth_params_location = "header" return auth_params, nil end + if not ctx.var.bk_api_auth:allow_get_auth_params_from_parameters() then + -- 不允许从请求参数获取认证参数，直接返回 + return {}, nil + end + -- from the querystring and body - return get_auth_params_from_parameters(ctx, authorization_keys), nil + auth_params = get_auth_params_from_parameters(ctx, authorization_keys) + + if not pl_types.is_empty(auth_params) then + -- 记录认证参数位置，便于统计哪些请求将认证参数放到请求参数，推动优化 + ctx.var.auth_params_location = "params" + end + + return auth_params end -- utils end @@ -174,6 +188,8 @@ function _M.rewrite(conf, ctx) -- luacheck: no unused ctx.var.bk_user = user ctx.var.bk_app_code = app["app_code"] ctx.var.bk_username = user["username"] + -- 记录认证参数位置，便于统计哪些请求将认证参数放到请求参数，推动优化 + ctx.var.auth_params_location = ctx.var.auth_params_location or "" end if _TEST then -- luacheck: ignore diff --git a/src/apisix/plugins/bk-auth-verify/app-account-verifier.lua b/src/apisix/plugins/bk-auth-verify/app-account-verifier.lua index a456613..7780eae 100644 --- a/src/apisix/plugins/bk-auth-verify/app-account-verifier.lua +++ b/src/apisix/plugins/bk-auth-verify/app-account-verifier.lua @@ -20,6 +20,7 @@ local app_account_utils = require("apisix.plugins.bk-auth-verify.app-account-uti local bk_app_define = require("apisix.plugins.bk-define.app") local bk_cache = require("apisix.plugins.bk-cache.init") local setmetatable = setmetatable +local string = string local _M = {} @@ -46,6 +47,14 @@ function _M.verify_app(self) end if not pl_types.is_empty(self.app_secret) then + -- check the length before call bkauth apis + if string.len(self.app_code) > 32 then + return bk_app_define.new_anonymous_app("app code cannot be longer than 32 characters") + end + if string.len(self.app_secret) > 128 then + return bk_app_define.new_anonymous_app("app secret cannot be longer than 128 characters") + end + return self:verify_by_app_secret() end diff --git a/src/apisix/plugins/bk-define/context-api-bkauth.lua b/src/apisix/plugins/bk-define/context-api-bkauth.lua index ce8d578..9387d80 100644 --- a/src/apisix/plugins/bk-define/context-api-bkauth.lua +++ b/src/apisix/plugins/bk-define/context-api-bkauth.lua @@ -112,6 +112,8 @@ function ContextApiBkAuth.new(bk_api_auth) api_type = bk_api_auth.api_type, unfiltered_sensitive_keys = bk_api_auth.unfiltered_sensitive_keys or {}, include_system_headers_mapping = include_system_headers_mapping, + allow_auth_from_params = bk_api_auth.allow_auth_from_params, + allow_delete_sensitive_params = bk_api_auth.allow_delete_sensitive_params, uin_conf = UinConf.new(bk_api_auth.uin_conf), rtx_conf = RtxConf.new(bk_api_auth.rtx_conf), user_conf = UserConf.new(bk_api_auth.user_conf), @@ -123,6 +125,13 @@ function ContextApiBkAuth.get_api_type(self) return self.api_type end +---Allow get auth_params from request parameters, such as querystring, body +---@return boolean +function ContextApiBkAuth.allow_get_auth_params_from_parameters(self) + -- 默认允许从参数获取认证信息 + return self.allow_auth_from_params ~= false +end + ---Get the unfiltered sensitive keys. ---@return table function ContextApiBkAuth.get_unfiltered_sensitive_keys(self) @@ -147,8 +156,9 @@ end ---Filter the sensitive params or not, do the filter if api_type is not ESB. ---@return boolean -function ContextApiBkAuth.is_filter_sensitive_params(self) - return self.api_type ~= ESB +function ContextApiBkAuth.should_delete_sensitive_params(self) + -- 非 esb，且允许删除敏感参数（默认允许删除） + return self.api_type ~= ESB and self.allow_delete_sensitive_params ~= false end function ContextApiBkAuth.is_user_type_uin(self) diff --git a/src/apisix/plugins/bk-delete-sensitive.lua b/src/apisix/plugins/bk-delete-sensitive.lua index b45df2b..8084bf1 100644 --- a/src/apisix/plugins/bk-delete-sensitive.lua +++ b/src/apisix/plugins/bk-delete-sensitive.lua @@ -34,6 +34,7 @@ local core = require("apisix.core") local bk_core = require("apisix.plugins.bk-core.init") local ngx = ngx -- luacheck: ignore local ipairs = ipairs +local tostring = tostring local plugin_name = "bk-delete-sensitive" @@ -55,7 +56,6 @@ function _M.check_schema(conf) return core.schema.check(schema, conf) end - ---Delete the sensitive parameters in the request header, uri args and body, ---it will check the first, then do the modification. ---@param ctx apisix.Context @@ -103,6 +103,15 @@ local function delete_sensitive_params(ctx, sensitive_keys, unfiltered_sensitive ::continue:: end + if ctx.var.auth_params_location == "header" and (query_changed or form_changed or body_changed) then + core.log.warn( + "auth params are present in both header and request parameters, request_id: " .. + tostring(ctx.var.bk_request_id) + ) + -- 记录认证参数位置，便于统计哪些请求将认证参数放到请求参数，推动优化 + ctx.var.auth_params_location = "header_and_params" + end + if check_query and query_changed then core.request.set_uri_args(ctx, uri_args) end @@ -127,7 +136,7 @@ local function delete_sensitive_headers() end function _M.rewrite(conf, ctx) -- luacheck: no unused - if ctx.var.bk_api_auth and ctx.var.bk_api_auth:is_filter_sensitive_params() then + if ctx.var.bk_api_auth and ctx.var.bk_api_auth:should_delete_sensitive_params() then delete_sensitive_params( ctx, bk_core.config.get_sensitive_keys(), ctx.var.bk_api_auth:get_unfiltered_sensitive_keys() ) diff --git a/src/apisix/plugins/bk-stage-context.lua b/src/apisix/plugins/bk-stage-context.lua index 5d4ef21..59cc957 100644 --- a/src/apisix/plugins/bk-stage-context.lua +++ b/src/apisix/plugins/bk-stage-context.lua @@ -15,7 +15,6 @@ -- We undertake not to change the open source license (MIT license) applicable -- to the current version of the project delivered to anyone in the future. -- - local core = require("apisix.core") local bk_core = require("apisix.plugins.bk-core.init") local context_api_bkauth = require("apisix.plugins.bk-define.context-api-bkauth") @@ -63,6 +62,14 @@ local schema = { type = "string", }, }, + allow_auth_from_params = { + type = "boolean", + default = true, + }, + allow_delete_sensitive_params = { + type = "boolean", + default = true, + }, uin_conf = { type = "object", properties = { @@ -132,20 +139,17 @@ local _M = { schema = schema, } - ---@param jwt_private_key string: JWT private key (encoded in Base64) ----@return string|nil, string: The decoded JWT private key, or nil and error message if the decoding fails +---@return string|nil, string|nil: The decoded JWT private key, or nil and error message if the decoding fails local function decode_jwt_private_key(jwt_private_key) local decoded_private_key = ngx_decode_base64(jwt_private_key) if not decoded_private_key then core.log.error("failed to decode jwt_private_key with base64. jwt_private_key=", jwt_private_key) return nil, "failed to decode jwt_private_key with base64" end - return decoded_private_key + return decoded_private_key, nil end - - ---@param conf table: The user-provided configuration for the plugin instance ---@return boolean, string|nil: true and nil if the configuration is valid, false and error message if not function _M.check_schema(conf) @@ -165,7 +169,6 @@ function _M.check_schema(conf) return true end - ---@param conf table: The user-provided configuration for the plugin instance ---@param ctx api.Context: The request context function _M.rewrite(conf, ctx) diff --git a/src/apisix/t/README.md b/src/apisix/t/README.md index 9a9c2ab..4908d82 100644 --- a/src/apisix/t/README.md +++ b/src/apisix/t/README.md @@ -33,4 +33,4 @@ - [apisix source code : t](https://github.com/apache/apisix/tree/master/t) - [test-nginx](https://github.com/openresty/test-nginx) - [test-nginx doc: user guide](https://openresty.gitbooks.io/programming-openresty/content/testing/index.html) - +- [Test::Nginx::Socket](https://metacpan.org/pod/Test::Nginx::Socket) diff --git a/src/apisix/t/bk-auth-validate.t b/src/apisix/t/bk-auth-validate.t new file mode 100644 index 0000000..77f7fc4 --- /dev/null +++ b/src/apisix/t/bk-auth-validate.t @@ -0,0 +1,52 @@ +# +# TencentBlueKing is pleased to support the open source community by making +# 蓝鲸智云 - API 网关(BlueKing - APIGateway) available. +# Copyright (C) 2017 THL A29 Limited, a Tencent company. All rights reserved. +# Licensed under the MIT License (the "License"); you may not use this file except +# in compliance with the License. You may obtain a copy of the License at +# +# http://opensource.org/licenses/MIT +# +# Unless required by applicable law or agreed to in writing, software distributed under +# the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, +# either express or implied. See the License for the specific language governing permissions and +# limitations under the License. +# +# We undertake not to change the open source license (MIT license) applicable +# to the current version of the project delivered to anyone in the future. +# + +use t::APISIX 'no_plan'; + +repeat_each(1); +no_long_string(); +no_root_location(); +no_shuffle(); + +add_block_preprocessor(sub { + my ($block) = @_; + + if (!defined $block->request) { + $block->set_value("request", "GET /t"); + } +}); + +run_tests; + +__DATA__ + +=== TEST 1: sanity +--- config + location /t { + content_by_lua_block { + local plugin = require("apisix.plugins.bk-auth-validate") + local ok, err = plugin.check_schema({}) + if not ok then + ngx.say(err) + end + + ngx.say("done") + } + } +--- response_body +done diff --git a/src/apisix/t/bk-auth-verify.t b/src/apisix/t/bk-auth-verify.t new file mode 100644 index 0000000..1243d67 --- /dev/null +++ b/src/apisix/t/bk-auth-verify.t @@ -0,0 +1,234 @@ +# +# TencentBlueKing is pleased to support the open source community by making +# 蓝鲸智云 - API 网关(BlueKing - APIGateway) available. +# Copyright (C) 2017 THL A29 Limited, a Tencent company. All rights reserved. +# Licensed under the MIT License (the "License"); you may not use this file except +# in compliance with the License. You may obtain a copy of the License at +# +# http://opensource.org/licenses/MIT +# +# Unless required by applicable law or agreed to in writing, software distributed under +# the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, +# either express or implied. See the License for the specific language governing permissions and +# limitations under the License. +# +# We undertake not to change the open source license (MIT license) applicable +# to the current version of the project delivered to anyone in the future. +# + +use t::APISIX 'no_plan'; + +repeat_each(1); +no_long_string(); +no_root_location(); +no_shuffle(); + +add_block_preprocessor(sub { + my ($block) = @_; + + if (!defined $block->request) { + $block->set_value("request", "GET /t"); + } +}); + +run_tests; + +__DATA__ + +=== TEST 1: sanity +--- config + location /t { + content_by_lua_block { + local plugin = require("apisix.plugins.bk-auth-verify") + local ok, err = plugin.check_schema({}) + if not ok then + ngx.say(err) + end + + ngx.say("done") + } + } +--- response_body +done + +=== TEST 2: enable plugin to test auth_params_location, allow_auth_from_params=True +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/routes/1', + ngx.HTTP_PUT, + [[{ + "plugins": { + "bk-stage-context": { + "bk_gateway_name": "demo", + "bk_stage_name": "prod", + "jwt_private_key": "dGhpcyBpcyBhIGZha2Ugand0IHByaXZhdGUga2V5", + "bk_api_auth": { + "api_type": 10, + "allow_auth_from_params": true, + "unfiltered_sensitive_keys": {} + } + }, + "bk-resource-context": { + "bk_resource_name": "echo", + "bk_resource_auth": { + "verified_app_required": false, + "verified_user_required": false, + "resource_perm_required": false, + "skip_user_verification": true + } + }, + "bk-auth-verify": {}, + "file-logger": { + "path": "file.log", + "log_format": { + "auth_location": "$auth_params_location" + } + } + }, + "upstream": { + "nodes": { + "127.0.0.1:1980": 1 + }, + "type": "roundrobin" + }, + "uri": "/hello" + }]] + ) + + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- response_body +passed + +=== TEST 3: auth_params_location maybe header, params +--- config + location /t { + content_by_lua_block { + os.remove("file.log") + + local core = require("apisix.core") + local t = require("lib.test_admin").test + -- auth-params in header + t("/hello", ngx.HTTP_GET, nil, nil, { + ["x-bkapi-authorization"] = '{"bk_app_code": "demo"}' + }) + -- auth-params in params + t("/hello?bk_app_secret=secret", ngx.HTTP_GET) + + local fd, err = io.open("file.log", "r") + + if not fd then + core.log.error("failed to open file: file.log, error info: ", err) + return + end + + local line1 = fd:read() + local line2 = fd:read() + + local new_line1 = core.json.decode(line1) + local new_line2 = core.json.decode(line2) + ngx.say(new_line1.auth_location) + ngx.say(new_line2.auth_location) + } + } +--- response_body +header +params + + +=== TEST 4: enable plugin to test auth_params_location, allow_auth_from_params=False +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/routes/1', + ngx.HTTP_PUT, + [[{ + "plugins": { + "bk-stage-context": { + "bk_gateway_name": "demo", + "bk_stage_name": "prod", + "jwt_private_key": "dGhpcyBpcyBhIGZha2Ugand0IHByaXZhdGUga2V5", + "bk_api_auth": { + "api_type": 10, + "allow_auth_from_params": false, + "unfiltered_sensitive_keys": {} + } + }, + "bk-resource-context": { + "bk_resource_name": "echo", + "bk_resource_auth": { + "verified_app_required": false, + "verified_user_required": false, + "resource_perm_required": false, + "skip_user_verification": true + } + }, + "bk-auth-verify": {}, + "file-logger": { + "path": "file.log", + "log_format": { + "auth_location": "$auth_params_location" + } + } + }, + "upstream": { + "nodes": { + "127.0.0.1:1980": 1 + }, + "type": "roundrobin" + }, + "uri": "/hello" + }]] + ) + + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- response_body +passed + +=== TEST 5: auth_params_location maybe header +--- config + location /t { + content_by_lua_block { + os.remove("file.log") + + local core = require("apisix.core") + local t = require("lib.test_admin").test + -- auth-params in header + t("/hello", ngx.HTTP_GET, nil, nil, { + ["x-bkapi-authorization"] = '{"bk_app_code": "demo"}' + }) + -- auth-params in params + t("/hello?bk_app_secret=secret", ngx.HTTP_GET) + + local fd, err = io.open("file.log", "r") + + if not fd then + core.log.error("failed to open file: file.log, error info: ", err) + return + end + + local line1 = fd:read() + local line2 = fd:read() + + local new_line1 = core.json.decode(line1) + local new_line2 = core.json.decode(line2) + ngx.say(new_line1.auth_location) + -- it's empty string, add prefix for test + ngx.say("foo" .. new_line2.auth_location) + } + } +--- response_body +header +foo diff --git a/src/apisix/t/bk-delete-sensitive.t b/src/apisix/t/bk-delete-sensitive.t new file mode 100644 index 0000000..873c70d --- /dev/null +++ b/src/apisix/t/bk-delete-sensitive.t @@ -0,0 +1,347 @@ +# +# TencentBlueKing is pleased to support the open source community by making +# 蓝鲸智云 - API 网关(BlueKing - APIGateway) available. +# Copyright (C) 2017 THL A29 Limited, a Tencent company. All rights reserved. +# Licensed under the MIT License (the "License"); you may not use this file except +# in compliance with the License. You may obtain a copy of the License at +# +# http://opensource.org/licenses/MIT +# +# Unless required by applicable law or agreed to in writing, software distributed under +# the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, +# either express or implied. See the License for the specific language governing permissions and +# limitations under the License. +# +# We undertake not to change the open source license (MIT license) applicable +# to the current version of the project delivered to anyone in the future. +# + +use t::APISIX 'no_plan'; + +repeat_each(1); +no_long_string(); +no_root_location(); +no_shuffle(); + +add_block_preprocessor(sub { + my ($block) = @_; + + if (!defined $block->request) { + $block->set_value("request", "GET /t"); + } +}); + +run_tests; + +__DATA__ + +=== TEST 1: sanity +--- config + location /t { + content_by_lua_block { + local plugin = require("apisix.plugins.bk-delete-sensitive") + local ok, err = plugin.check_schema({}) + if not ok then + ngx.say(err) + end + + ngx.say("done") + } + } +--- response_body +done + +=== TEST 2: enable bk-delete-sensitive plugin using admin api +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/routes/1', + ngx.HTTP_PUT, + [[{ + "plugins": { + "bk-stage-context": { + "bk_gateway_name": "demo", + "bk_stage_name": "prod", + "jwt_private_key": "dGhpcyBpcyBhIGZha2Ugand0IHByaXZhdGUga2V5", + "bk_api_auth": { + "api_type": 10, + "unfiltered_sensitive_keys": {} + } + }, + "bk-delete-sensitive": {} + }, + "upstream": { + "nodes": { + "127.0.0.1:1980": 1 + }, + "type": "roundrobin" + }, + "uri": "/echo" + }]] + ) + + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- response_body +passed + +=== TEST 3: delete header x-bkapi-authorization +--- request +GET /echo +--- more_headers +x-bkapi-authorization: {"bk_app_code": "demo", "bk_app_secret": "secret"} +foo: bar +--- response_headers +!x-bkapi-authorization +foo: bar + +=== TEST 4: delete sensitive params +--- request +POST /echo +{"bk_app_code": "demo", "bk_app_secret": "secret", "access_token": "token"} +--- response_body chomp +{"bk_app_code":"demo"} + +=== TEST 5: enable plugin, api_type=0, will not delete sensitive params +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/routes/1', + ngx.HTTP_PUT, + [[{ + "plugins": { + "bk-stage-context": { + "bk_gateway_name": "demo", + "bk_stage_name": "prod", + "jwt_private_key": "dGhpcyBpcyBhIGZha2Ugand0IHByaXZhdGUga2V5", + "bk_api_auth": { + "api_type": 0, + "unfiltered_sensitive_keys": {} + } + }, + "bk-delete-sensitive": {} + }, + "upstream": { + "nodes": { + "127.0.0.1:1980": 1 + }, + "type": "roundrobin" + }, + "uri": "/echo" + }]] + ) + + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- response_body +passed + +=== TEST 6: api_type=0, will not delete sensitive params +--- request +POST /echo +{"bk_app_code":"demo", "bk_app_secret":"secret"} +--- response_body chomp +{"bk_app_code":"demo", "bk_app_secret":"secret"} + +=== TEST 7: enable plugin, api_type=10 and unfiltered_sensitive_keys exist, will not delete sensitive params +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/routes/1', + ngx.HTTP_PUT, + [[{ + "plugins": { + "bk-stage-context": { + "bk_gateway_name": "demo", + "bk_stage_name": "prod", + "jwt_private_key": "dGhpcyBpcyBhIGZha2Ugand0IHByaXZhdGUga2V5", + "bk_api_auth": { + "api_type": 10, + "unfiltered_sensitive_keys": ["access_token"] + } + }, + "bk-delete-sensitive": {} + }, + "upstream": { + "nodes": { + "127.0.0.1:1980": 1 + }, + "type": "roundrobin" + }, + "uri": "/echo" + }]] + ) + + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- response_body +passed + +=== TEST 8: unfiltered_sensitive_keys exist, will not delete sensitive params +--- request +POST /echo +{"access_token":"token"} +--- response_body chomp +{"access_token":"token"} + +=== TEST 9: enable plugin, allow_delete_sensitive_params=false, will not delete sensitive params +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/routes/1', + ngx.HTTP_PUT, + [[{ + "plugins": { + "bk-stage-context": { + "bk_gateway_name": "demo", + "bk_stage_name": "prod", + "jwt_private_key": "dGhpcyBpcyBhIGZha2Ugand0IHByaXZhdGUga2V5", + "bk_api_auth": { + "api_type": 10, + "allow_delete_sensitive_params": false, + "unfiltered_sensitive_keys": {} + } + }, + "bk-delete-sensitive": {} + }, + "upstream": { + "nodes": { + "127.0.0.1:1980": 1 + }, + "type": "roundrobin" + }, + "uri": "/echo" + }]] + ) + + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- response_body +passed + +=== TEST 10: allow_delete_sensitive_params=false, will not delete sensitive params +--- request +POST /echo +{"bk_app_code":"demo", "bk_app_secret":"secret", "access_token":"token"} +--- response_body chomp +{"bk_app_code":"demo", "bk_app_secret":"secret", "access_token":"token"} + + +=== TEST 11: enable plugin to test auth_params_location +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/routes/1', + ngx.HTTP_PUT, + [[{ + "plugins": { + "bk-stage-context": { + "bk_gateway_name": "demo", + "bk_stage_name": "prod", + "jwt_private_key": "dGhpcyBpcyBhIGZha2Ugand0IHByaXZhdGUga2V5", + "bk_api_auth": { + "api_type": 10, + "allow_delete_sensitive_params": true, + "unfiltered_sensitive_keys": {} + } + }, + "bk-resource-context": { + "bk_resource_name": "echo", + "bk_resource_auth": { + "verified_app_required": false, + "verified_user_required": false, + "resource_perm_required": false, + "skip_user_verification": true + } + }, + "bk-auth-verify": {}, + "bk-delete-sensitive": {}, + "file-logger": { + "path": "file.log", + "log_format": { + "auth_location": "$auth_params_location" + } + } + }, + "upstream": { + "nodes": { + "127.0.0.1:1980": 1 + }, + "type": "roundrobin" + }, + "uri": "/hello" + }]] + ) + + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- response_body +passed + +=== TEST 12: auth_params_location maybe changed to header_and_params if sensitive params exist in request params +--- config + location /t { + content_by_lua_block { + os.remove("file.log") + + local core = require("apisix.core") + local t = require("lib.test_admin").test + -- auth-params in header, and sensitive params exist in request params + t("/hello?bk_app_code=demo&bk_app_secret=secret", ngx.HTTP_GET, nil, nil, { + ["x-bkapi-authorization"] = '{"bk_app_code": "demo"}' + }) + -- auth-params in header, but sensitive params do not exist in request params + t("/hello?bk_app_code=demo", ngx.HTTP_GET, nil, nil, { + ["x-bkapi-authorization"] = '{"bk_app_code": "demo"}' + }) + -- auth-params only in params + t("/hello?bk_app_secret=secret", ngx.HTTP_GET) + + local fd, err = io.open("file.log", "r") + + if not fd then + core.log.error("failed to open file: file.log, error info: ", err) + return + end + + local line1 = fd:read() + local line2 = fd:read() + local line3 = fd:read() + + local new_line1 = core.json.decode(line1) + local new_line2 = core.json.decode(line2) + local new_line3 = core.json.decode(line3) + ngx.say(new_line1.auth_location) + ngx.say(new_line2.auth_location) + ngx.say(new_line3.auth_location) + } + } +--- response_body +header_and_params +header +params diff --git a/src/apisix/t/bk-stage-context.t b/src/apisix/t/bk-stage-context.t new file mode 100644 index 0000000..5ef3f34 --- /dev/null +++ b/src/apisix/t/bk-stage-context.t @@ -0,0 +1,84 @@ +# +# TencentBlueKing is pleased to support the open source community by making +# 蓝鲸智云 - API 网关(BlueKing - APIGateway) available. +# Copyright (C) 2017 THL A29 Limited, a Tencent company. All rights reserved. +# Licensed under the MIT License (the "License"); you may not use this file except +# in compliance with the License. You may obtain a copy of the License at +# +# http://opensource.org/licenses/MIT +# +# Unless required by applicable law or agreed to in writing, software distributed under +# the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, +# either express or implied. See the License for the specific language governing permissions and +# limitations under the License. +# +# We undertake not to change the open source license (MIT license) applicable +# to the current version of the project delivered to anyone in the future. +# + +use t::APISIX 'no_plan'; + +repeat_each(1); +no_long_string(); +no_shuffle(); +no_root_location(); + +add_block_preprocessor(sub { + my ($block) = @_; + + if (!defined $block->request) { + $block->set_value("request", "GET /t"); + } +}); + +run_tests; + +__DATA__ + +=== TEST 1: sanity +--- config + location /t { + content_by_lua_block { + local plugin = require("apisix.plugins.bk-stage-context") + local ok, err = plugin.check_schema({ + bk_gateway_name = "demo", + bk_stage_name = "prod", + jwt_private_key = "dGhpcyBpcyBhIGZha2Ugand0IHByaXZhdGUga2V5", + bk_api_auth = { + api_type = 10 + } + }) + if not ok then + ngx.say(err) + end + + ngx.say("done") + } + } +--- response_body +done + +=== TEST 2: with allow_auth_from_params/allow_delete_sensitive_params +--- config + location /t { + content_by_lua_block { + local plugin = require("apisix.plugins.bk-stage-context") + local ok, err = plugin.check_schema({ + bk_gateway_name = "demo", + bk_stage_name = "prod", + jwt_private_key = "dGhpcyBpcyBhIGZha2Ugand0IHByaXZhdGUga2V5", + bk_api_auth = { + api_type = 10, + allow_auth_from_params = false, + allow_delete_sensitive_params = false, + } + }) + if not ok then + ngx.say(err) + end + + ngx.say("done") + } + } +--- response_body +done diff --git a/src/apisix/tests/bk-auth-verify/test-app-account-verifier.lua b/src/apisix/tests/bk-auth-verify/test-app-account-verifier.lua index 9ed25f9..069210c 100644 --- a/src/apisix/tests/bk-auth-verify/test-app-account-verifier.lua +++ b/src/apisix/tests/bk-auth-verify/test-app-account-verifier.lua @@ -83,6 +83,36 @@ describe( end ) + it( + "app_code length is greather 32", function() + local auth_params = auth_params_mod.new({ + bk_app_code = "123456789012345678901234567890123", + bk_app_secret = "world", + }) + local verifier = app_account_verifier_mod.new(auth_params) + + local app = verifier:verify_app() + assert.is_equal(app.app_code, "") + assert.is_false(app.verified) + assert.is_equal(app.valid_error_message, "app code cannot be longer than 32 characters") + end + ) + + it( + "app_secret length is greather 128", function() + local auth_params = auth_params_mod.new({ + bk_app_code = "hello", + bk_app_secret = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + }) + local verifier = app_account_verifier_mod.new(auth_params) + + local app = verifier:verify_app() + assert.is_equal(app.app_code, "") + assert.is_false(app.verified) + assert.is_equal(app.valid_error_message, "app secret cannot be longer than 128 characters") + end + ) + it( "app secret is not empty", function() auth_params = auth_params_mod.new( diff --git a/src/apisix/tests/bk-define/test-context-api-bkauth.lua b/src/apisix/tests/bk-define/test-context-api-bkauth.lua index 6db7abf..0ff4b50 100644 --- a/src/apisix/tests/bk-define/test-context-api-bkauth.lua +++ b/src/apisix/tests/bk-define/test-context-api-bkauth.lua @@ -15,7 +15,6 @@ -- We undertake not to change the open source license (MIT license) applicable -- to the current version of the project delivered to anyone in the future. -- - local context_api_bkauth = require("apisix.plugins.bk-define.context-api-bkauth") describe( @@ -151,6 +150,19 @@ describe( end ) + it( + "allow_get_auth_params_from_parameters", function() + bk_api_auth.allow_auth_from_params = nil + assert.is_true(bk_api_auth:allow_get_auth_params_from_parameters()) + + bk_api_auth.allow_auth_from_params = true + assert.is_true(bk_api_auth:allow_get_auth_params_from_parameters()) + + bk_api_auth.allow_auth_from_params = false + assert.is_false(bk_api_auth:allow_get_auth_params_from_parameters()) + end + ) + it( "get_uin_conf", function() assert.is_same( @@ -202,11 +214,59 @@ describe( ) it( - "is_filter_sensitive_params", function() - assert.is_true(bk_api_auth:is_filter_sensitive_params()) - - bk_api_auth.api_type = 0 - assert.is_false(bk_api_auth:is_filter_sensitive_params()) + "should_delete_sensitive_params", function() + local data = { + -- esb + { + params = { + api_type = 0, + allow_delete_sensitive_params = nil, + }, + expected = false, + }, + { + params = { + api_type = 0, + allow_delete_sensitive_params = true, + }, + expected = false, + }, + { + params = { + api_type = 0, + allow_delete_sensitive_params = false, + }, + expected = false, + }, + -- normal + { + params = { + api_type = 10, + allow_delete_sensitive_params = nil, + }, + expected = true, + }, + { + params = { + api_type = 10, + allow_delete_sensitive_params = true, + }, + expected = true, + }, + { + params = { + api_type = 10, + allow_delete_sensitive_params = false, + }, + expected = false, + }, + } + for _, item in ipairs(data) do + bk_api_auth.api_type = item.params.api_type + bk_api_auth.allow_delete_sensitive_params = item.params.allow_delete_sensitive_params + + assert.is_equal(bk_api_auth:should_delete_sensitive_params(), item.expected) + end end ) diff --git a/src/apisix/tests/test-bk-auth-verify.lua b/src/apisix/tests/test-bk-auth-verify.lua index 2754d81..d6a756d 100644 --- a/src/apisix/tests/test-bk-auth-verify.lua +++ b/src/apisix/tests/test-bk-auth-verify.lua @@ -15,7 +15,6 @@ -- We undertake not to change the open source license (MIT license) applicable -- to the current version of the project delivered to anyone in the future. -- - local core = require("apisix.core") local bk_core = require("apisix.plugins.bk-core.init") local bk_auth_verify_init = require("apisix.plugins.bk-auth-verify.init") @@ -38,6 +37,7 @@ describe( local json_body local form_data local multipart_form_data + local ctx before_each( function() @@ -46,6 +46,9 @@ describe( json_body = nil form_data = nil multipart_form_data = nil + ctx = { + var = {}, + } stub( ngx.req, "get_method", function() @@ -261,6 +264,7 @@ describe( "get_auth_params_from_request", function() it( "from header", function() + ctx.var.bk_api_auth = context_api_bkauth.new({}) stub( core.request, "header", core.json.encode( { @@ -270,7 +274,7 @@ describe( ) ) - local auth_params = plugin._get_auth_params_from_request({}, authorization_keys) + local auth_params, err = plugin._get_auth_params_from_request(ctx, authorization_keys) assert.is_same( auth_params, { bk_app_code = "my-app", @@ -278,25 +282,83 @@ describe( a = "b", } ) + assert.is_nil(err) + assert.is_same(ctx.var.auth_params_location, "header") core.request.header:revert() end ) it( - "from parameters", function() + "from header with error", function() + ctx.var.bk_api_auth = context_api_bkauth.new({}) + stub(core.request, "header", "no-valid-json") + + local auth_params, err = plugin._get_auth_params_from_request(ctx, authorization_keys) + assert.is_nil(auth_params) + assert.is_not_nil(err) + assert.is_nil(ctx.var.auth_params_location) + + core.request.header:revert() + end + ) + + it( + "from parameters with auth params", function() + ctx.var.bk_api_auth = context_api_bkauth.new({}) stub( core.request, "get_uri_args", { bk_app_code = "my-app", a = "b", } ) - local auth_params = plugin._get_auth_params_from_request({}, authorization_keys) + local auth_params, err = plugin._get_auth_params_from_request(ctx, authorization_keys) assert.is_same( auth_params, { bk_app_code = "my-app", } ) + assert.is_nil(err) + assert.is_same(ctx.var.auth_params_location, "params") + + core.request.get_uri_args:revert() + end + ) + + it( + "from parameters without auth params", function() + ctx.var.bk_api_auth = context_api_bkauth.new({}) + stub( + core.request, "get_uri_args", { + a = "b", + } + ) + local auth_params, err = plugin._get_auth_params_from_request(ctx, authorization_keys) + assert.is_same(auth_params, {}) + assert.is_nil(err) + assert.is_nil(ctx.var.auth_params_location) + + core.request.get_uri_args:revert() + end + ) + + it( + "do not allow get auth params from parameters", function() + ctx.var.bk_api_auth = context_api_bkauth.new( + { + allow_auth_from_params = false, + } + ) + stub( + core.request, "get_uri_args", { + bk_app_code = "my-app", + a = "b", + } + ) + local auth_params, err = plugin._get_auth_params_from_request(ctx, authorization_keys) + assert.is_same(auth_params, {}) + assert.is_nil(err) + assert.is_nil(ctx.var.auth_params_location) core.request.get_uri_args:revert() end @@ -304,7 +366,8 @@ describe( it( "provide no valid inputs", function() - local auth_params, err = plugin._get_auth_params_from_request({}, authorization_keys) + ctx.var.bk_api_auth = context_api_bkauth.new({}) + local auth_params, err = plugin._get_auth_params_from_request(ctx, authorization_keys) assert.are_same(auth_params, {}) assert.is_nil(err) end @@ -319,12 +382,14 @@ describe( it( "with no auth params", function() - local app, user = plugin.verify({ - var = { - bk_api_auth = bk_api_auth, - bk_resource_auth = bk_resource_auth, - }, - }) + local app, user = plugin.verify( + { + var = { + bk_api_auth = bk_api_auth, + bk_resource_auth = bk_resource_auth, + }, + } + ) assert.is_equal(app.app_code, "") assert.is_false(app.verified) -- The legacy verifier was triggered by default. @@ -345,12 +410,14 @@ describe( "with invalid JSON auth_params in header", function() stub(core.request, "header", "not valid json") - local app, user = plugin.verify({ - var = { - bk_api_auth = bk_api_auth, - bk_resource_auth = bk_resource_auth, - }, - }) + local app, user = plugin.verify( + { + var = { + bk_api_auth = bk_api_auth, + bk_resource_auth = bk_resource_auth, + }, + } + ) assert.is_equal(app.app_code, "") assert.is_false(app.verified) assert.is_equal( @@ -504,6 +571,7 @@ describe( ) assert.is_same(ctx.var.bk_app_code, "my-app") assert.is_same(ctx.var.bk_username, "admin") + assert.is_same(ctx.var.auth_params_location, "") plugin.verify:revert() end diff --git a/src/apisix/tests/test-bk-delete-sensitive.lua b/src/apisix/tests/test-bk-delete-sensitive.lua index 4c42ea6..f4409ac 100644 --- a/src/apisix/tests/test-bk-delete-sensitive.lua +++ b/src/apisix/tests/test-bk-delete-sensitive.lua @@ -15,7 +15,6 @@ -- We undertake not to change the open source license (MIT license) applicable -- to the current version of the project delivered to anyone in the future. -- - local core = require("apisix.core") local bk_core = require("apisix.plugins.bk-core.init") local context_api_bkauth = require("apisix.plugins.bk-define.context-api-bkauth") @@ -34,10 +33,14 @@ describe( "access_token", } local unfiltered_sensitive_keys + local ctx before_each( function() unfiltered_sensitive_keys = {} + ctx = { + var = {}, + } stub(ngx.req, "clear_header") stub( @@ -83,9 +86,9 @@ describe( bk_app_secret = "fake-secret", foo = "bar", } - plugin._delete_sensitive_params({}, sensitive_keys, unfiltered_sensitive_keys) + plugin._delete_sensitive_params(ctx, sensitive_keys, unfiltered_sensitive_keys) assert.stub(core.request.set_uri_args).was_called_with( - {}, { + ctx, { foo = "bar", } ) @@ -98,7 +101,7 @@ describe( uri_args = { foo = "bar", } - plugin._delete_sensitive_params({}, sensitive_keys, unfiltered_sensitive_keys) + plugin._delete_sensitive_params(ctx, sensitive_keys, unfiltered_sensitive_keys) assert.stub(core.request.set_uri_args).was_not_called() assert.stub(ngx.req.set_body_data).was_not_called() end @@ -110,7 +113,7 @@ describe( bk_app_secret = "fake-secret", foo = "bar", } - plugin._delete_sensitive_params({}, sensitive_keys, unfiltered_sensitive_keys) + plugin._delete_sensitive_params(ctx, sensitive_keys, unfiltered_sensitive_keys) assert.stub(core.request.set_uri_args).was_not_called() assert.stub(ngx.req.set_body_data).was_called_with("foo=bar") end @@ -122,7 +125,7 @@ describe( a = "b", foo = "bar", } - plugin._delete_sensitive_params({}, sensitive_keys, unfiltered_sensitive_keys) + plugin._delete_sensitive_params(ctx, sensitive_keys, unfiltered_sensitive_keys) assert.stub(core.request.set_uri_args).was_not_called() assert.stub(ngx.req.set_body_data).was_not_called() end @@ -134,7 +137,7 @@ describe( bk_app_secret = "fake-secret", foo = "bar", } - plugin._delete_sensitive_params({}, sensitive_keys, unfiltered_sensitive_keys) + plugin._delete_sensitive_params(ctx, sensitive_keys, unfiltered_sensitive_keys) assert.stub(core.request.set_uri_args).was_not_called() assert.stub(ngx.req.set_body_data).was_called_with("{\"foo\":\"bar\"}") end @@ -145,7 +148,7 @@ describe( json_body = { foo = "bar", } - plugin._delete_sensitive_params({}, sensitive_keys, unfiltered_sensitive_keys) + plugin._delete_sensitive_params(ctx, sensitive_keys, unfiltered_sensitive_keys) assert.stub(core.request.set_uri_args).was_not_called() assert.stub(ngx.req.set_body_data).was_not_called() end @@ -163,9 +166,9 @@ describe( access_token = "fake-token", foo = "bar", } - plugin._delete_sensitive_params({}, sensitive_keys, unfiltered_sensitive_keys) + plugin._delete_sensitive_params(ctx, sensitive_keys, unfiltered_sensitive_keys) assert.stub(core.request.set_uri_args).was_called_with( - {}, { + ctx, { foo = "bar", } ) @@ -185,9 +188,9 @@ describe( access_token = "fake-token", foo = "bar", } - plugin._delete_sensitive_params({}, sensitive_keys, unfiltered_sensitive_keys) + plugin._delete_sensitive_params(ctx, sensitive_keys, unfiltered_sensitive_keys) assert.stub(core.request.set_uri_args).was_called_with( - {}, { + ctx, { foo = "bar", } ) @@ -205,15 +208,36 @@ describe( access_token = "fake-token", foo = "bar", } - plugin._delete_sensitive_params({}, sensitive_keys, unfiltered_sensitive_keys) + plugin._delete_sensitive_params(ctx, sensitive_keys, unfiltered_sensitive_keys) assert.stub(core.request.set_uri_args).was_called_with( - {}, { + ctx, { bk_app_secret = "fake-secret", foo = "bar", } ) end ) + + it( + "auth_params_location", function() + ctx.var.auth_params_location = "header" + + uri_args = { + bk_app_secret = "fake-secret", + foo = "bar", + } + plugin._delete_sensitive_params(ctx, sensitive_keys, unfiltered_sensitive_keys) + assert.is_same(ctx.var.auth_params_location, "header_and_params") + + ctx.var.auth_params_location = "" + uri_args = { + bk_app_secret = "fake-secret", + foo = "bar", + } + plugin._delete_sensitive_params(ctx, sensitive_keys, unfiltered_sensitive_keys) + assert.is_same(ctx.var.auth_params_location, "") + end + ) end ) @@ -232,16 +256,6 @@ describe( context( "rewrite", function() - local ctx - - before_each( - function() - ctx = { - var = {}, - } - end - ) - it( "delete sensitive params", function() ctx.var.bk_api_auth = context_api_bkauth.new( @@ -266,18 +280,28 @@ describe( it( "does not need to delete sensitive params", function() - ctx.var.bk_api_auth = context_api_bkauth.new( - { - api_type = 0, - } - ) uri_args = { bk_app_secret = "fake-secret", foo = "bar", } + -- esb + ctx.var.bk_api_auth = context_api_bkauth.new( + { + api_type = 0, + } + ) plugin.rewrite({}, ctx) + assert.stub(core.request.set_uri_args).was_not_called() + -- allow_delete_sensitive_params = false + ctx.var.bk_api_auth = context_api_bkauth.new( + { + api_type = 10, + allow_delete_sensitive_params = false, + } + ) + plugin.rewrite({}, ctx) assert.stub(core.request.set_uri_args).was_not_called() end ) diff --git a/src/apisix/tests/test-bk-stage-context.lua b/src/apisix/tests/test-bk-stage-context.lua index e467454..d5463dd 100644 --- a/src/apisix/tests/test-bk-stage-context.lua +++ b/src/apisix/tests/test-bk-stage-context.lua @@ -15,8 +15,6 @@ -- We undertake not to change the open source license (MIT license) applicable -- to the current version of the project delivered to anyone in the future. -- - -local core = require("apisix.core") local plugin = require("apisix.plugins.bk-stage-context") describe( @@ -102,6 +100,8 @@ describe( assert.is_equal(ctx.var.jwt_private_key, "test") assert.is_equal(ctx.var.bk_resource_name, ctx.route_name) assert.is_equal(ctx.var.bk_service_name, ctx.service_name) + assert.is_true(ctx.var.bk_api_auth.allow_auth_from_params) + assert.is_true(ctx.var.bk_api_auth.allow_delete_sensitive_params) end ) end diff --git a/src/apisix/typing.lua b/src/apisix/typing.lua index b6664b3..08c3e25 100644 --- a/src/apisix/typing.lua +++ b/src/apisix/typing.lua @@ -15,7 +15,6 @@ -- We undertake not to change the open source license (MIT license) applicable -- to the current version of the project delivered to anyone in the future. -- - ---@class nginx.ContextVar nginx 内建变量 --- Embedded Variables --- see https://nginx.org/en/docs/http/ngx_http_core_module.html#variables @@ -155,6 +154,7 @@ ---@field upstream_uri_without_args string 后端 uri ---@field bk_apigw_error table 网关返回的错误信息 ---@field bk_concurrency_limit_key string 并发限制 key; 仅在 bk-concurrency-limit 中使用 +---@field auth_params_location string 认证参数位置 --- ---@class bk_gateway.LogContextVar 日志相关的上下文变量，仅在 log 阶段可用 ---@field bk_log_request_timestamp integer|nil 日志请求秒级时间戳 diff --git a/src/build/patches/006_use_encoded_uri_for_radixtree_match.patch b/src/build/patches/006_use_encoded_uri_for_radixtree_match.patch new file mode 100644 index 0000000..2400dd2 --- /dev/null +++ b/src/build/patches/006_use_encoded_uri_for_radixtree_match.patch @@ -0,0 +1,14 @@ +diff --git a/apisix/http/route.lua b/apisix/http/route.lua +index d475646b..bc97ef82 100644 +--- a/apisix/http/route.lua ++++ b/apisix/http/route.lua +@@ -111,7 +111,8 @@ function _M.match_uri(uri_router, match_opts, api_ctx) + match_opts.vars = api_ctx.var + match_opts.matched = core.tablepool.fetch("matched_route_record", 0, 4) + +- local ok = uri_router:dispatch(api_ctx.var.uri, match_opts, api_ctx, match_opts) ++ local encoded_uri = core.utils.uri_safe_encode(api_ctx.var.uri) ++ local ok = uri_router:dispatch(encoded_uri, match_opts, api_ctx, match_opts) + return ok + end +