From b8c2bb92a5854b19392acee32b368e020bea19c4 Mon Sep 17 00:00:00 2001 From: wklken Date: Thu, 21 Dec 2023 11:43:52 +0800 Subject: [PATCH 1/4] refactor(bk-delete-sensitive): remove useless core.log.warn (#72) --- src/apisix/plugins/bk-delete-sensitive.lua | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/apisix/plugins/bk-delete-sensitive.lua b/src/apisix/plugins/bk-delete-sensitive.lua index 8084bf1..fa1a9c4 100644 --- a/src/apisix/plugins/bk-delete-sensitive.lua +++ b/src/apisix/plugins/bk-delete-sensitive.lua @@ -34,7 +34,6 @@ 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" @@ -104,10 +103,6 @@ local function delete_sensitive_params(ctx, sensitive_keys, unfiltered_sensitive 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 From 1c4f71fc0e8013d80f0309ccd046a248b33e6732 Mon Sep 17 00:00:00 2001 From: wklken Date: Tue, 26 Dec 2023 16:57:59 +0800 Subject: [PATCH 2/4] =?UTF-8?q?fix(bk-define):=20bkauth=20check=20verified?= =?UTF-8?q?=5Fapp=5Frequired=20should=20be=20true=20whi=E2=80=A6=20(#74)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../bk-define/context-resource-bkauth.lua | 3 +- .../test-context-resource-bkauth.lua | 43 +++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/src/apisix/plugins/bk-define/context-resource-bkauth.lua b/src/apisix/plugins/bk-define/context-resource-bkauth.lua index 1c048cb..f298541 100644 --- a/src/apisix/plugins/bk-define/context-resource-bkauth.lua +++ b/src/apisix/plugins/bk-define/context-resource-bkauth.lua @@ -44,7 +44,8 @@ function _M.get_verified_user_required(self) end function _M.get_resource_perm_required(self) - return self.resource_perm_required + -- only verified_app_required is true, the bk_app_code is verified, then the resource perm is meaningful + return self.verified_app_required and self.resource_perm_required end function _M.get_skip_user_verification(self) diff --git a/src/apisix/tests/bk-define/test-context-resource-bkauth.lua b/src/apisix/tests/bk-define/test-context-resource-bkauth.lua index 8beecce..e5abe22 100644 --- a/src/apisix/tests/bk-define/test-context-resource-bkauth.lua +++ b/src/apisix/tests/bk-define/test-context-resource-bkauth.lua @@ -60,6 +60,49 @@ describe( ) end ) + + it( + "resource perm, verified_app_required is false", function() + bk_resource_auth = context_resource_bkauth.new( + { + verified_app_required = false, + verified_user_required = true, + resource_perm_required = true, + skip_user_verification = true, + } + ) + + assert.is_false(bk_resource_auth:get_resource_perm_required()) + end + ) + it( + "resource perm, resource_perm_required is false", function() + bk_resource_auth = context_resource_bkauth.new( + { + verified_app_required = true, + verified_user_required = true, + resource_perm_required = false, + skip_user_verification = true, + } + ) + + assert.is_false(bk_resource_auth:get_resource_perm_required()) + end + ) + it( + "resource perm, both true", function() + bk_resource_auth = context_resource_bkauth.new( + { + verified_app_required = true, + verified_user_required = true, + resource_perm_required = true, + skip_user_verification = true, + } + ) + + assert.is_true(bk_resource_auth:get_resource_perm_required()) + end + ) end ) end From 96ba7ff931d2ed9000acd94448c6656e1f0ea563 Mon Sep 17 00:00:00 2001 From: wklken Date: Wed, 27 Dec 2023 14:40:46 +0800 Subject: [PATCH 3/4] fix(ci/dockerfile.apisix-test-busted): build image fail (#76) --- src/apisix/ci/Dockerfile.apisix-test-busted | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/apisix/ci/Dockerfile.apisix-test-busted b/src/apisix/ci/Dockerfile.apisix-test-busted index fd207c9..b800c9b 100644 --- a/src/apisix/ci/Dockerfile.apisix-test-busted +++ b/src/apisix/ci/Dockerfile.apisix-test-busted @@ -3,7 +3,7 @@ FROM apache/apisix:$APISIX_VERSION-centos # in github action, change the source of yum is not ok # note: uncomment below if it's slow to build image -# RUN mv /etc/yum.repos.d/CentOS-Base.repo /etc/yum.repos.d/CentOS-Base.repo.backup && \ +# RUN mv /etc/yum.repos.d/CentOS-Base.repo /etc/yum.repos.d/CentOS-Base.repo.backup && \ # curl -o /etc/yum.repos.d/CentOS-Base.repo http://mirrors.cloud.tencent.com/repo/centos7_base.repo && \ # yum clean all @@ -11,6 +11,8 @@ RUN yum install -y sudo make gcc curl wget unzip git valgrind ARG APISIX_VERSION RUN curl https://raw.githubusercontent.com/apache/apisix/${APISIX_VERSION}/utils/linux-install-luarocks.sh | bash +# lock the version of luasystem, otherwise the busted won't be installed success +RUN luarocks install https://luarocks.org/manifests/lunarmodules/luasystem-0.2.1-0.rockspec RUN luarocks install https://github.com/lunarmodules/busted/releases/download/v2.1.1/busted-2.1.1-1.rockspec COPY ci/requirements-dev-0.rockspec / From 63af8b1eebf5699c9b0aab071235f70430f35645 Mon Sep 17 00:00:00 2001 From: hanyajun <1581532052@qq.com> Date: Wed, 27 Dec 2023 15:34:25 +0800 Subject: [PATCH 4/4] fix: fix bkauth request add request-id (#75) --- src/apisix/plugins/bk-components/bkauth.lua | 55 +++++++++++++------ .../tests/bk-components/test-bkauth.lua | 12 +++- 2 files changed, 50 insertions(+), 17 deletions(-) diff --git a/src/apisix/plugins/bk-components/bkauth.lua b/src/apisix/plugins/bk-components/bkauth.lua index 63a5903..b841857 100644 --- a/src/apisix/plugins/bk-components/bkauth.lua +++ b/src/apisix/plugins/bk-components/bkauth.lua @@ -17,6 +17,7 @@ -- local pl_types = require("pl.types") local http = require("resty.http") +local uuid = require("resty.jit-uuid") local core = require("apisix.core") local bk_core = require("apisix.plugins.bk-core.init") @@ -48,6 +49,8 @@ function _M.verify_app_secret(app_code, app_secret) local http_client = http.new() http_client:set_timeout(BKAUTH_TIMEOUT_MS) + + local request_id = uuid.generate_v4() local res, err = http_client:request_uri( url, { method = "POST", @@ -57,16 +60,19 @@ function _M.verify_app_secret(app_code, app_secret) } ), ssl_verify = false, + headers = { ["X-Bk-App-Code"] = _M.app_code, ["X-Bk-App-Secret"] = _M.app_secret, + ["X-Request-Id"] = request_id, ["Content-Type"] = "application/json", }, } ) if not (res and res.body) then - err = string_format("failed to request third-party api, url: %s, err: %s, response: nil", url, err) + err = string_format("failed to request third-party api, url: %s, request_id: %s, err: %s, response: nil", url, + request_id, err) core.log.error(err) return nil, err end @@ -83,24 +89,28 @@ function _M.verify_app_secret(app_code, app_secret) if result == nil then core.log.error( string_format( - "failed to request %s, response is not valid json, status: %s, response: %s", url, res.status, res.body + "failed to request %s, request_id: %s, response is not valid json, status: %s, response: %s", url, + request_id, res.status, res.body ) ) return nil, string_format( - "failed to request third-party api, response is not valid json, url: %s, status: %s", url, res.status + "failed to request third-party api, response is not valid json, url: %s, request_id: %s, status: %s", url, + request_id, res.status ) end if result.code ~= 0 or res.status ~= 200 then core.log.error( string_format( - "failed to request %s, result.code!=0 or status!=200, status: %s, response: %s", url, res.status, + "failed to request %s, request_id: %s, result.code!=0 or status!=200, status: %s, response: %s", url, + request_id, res.status, res.body ) ) return nil, string_format( - "failed to request third-party api, bkauth error message: %s, url: %s, status: %s, code: %s", - result.message, url, res.status, result.code + "failed to request third-party api, bkauth error message: %s, url: %s, \ + request_id: %s, status: %s, code: %s", + result.message, url, request_id, res.status, result.code ) end @@ -119,6 +129,7 @@ function _M.list_app_secrets(app_code) local http_client = http.new() http_client:set_timeout(BKAUTH_TIMEOUT_MS) + local request_id = uuid.generate_v4() local res, err = http_client:request_uri( url, { method = "GET", @@ -126,13 +137,15 @@ function _M.list_app_secrets(app_code) headers = { ["X-Bk-App-Code"] = _M.app_code, ["X-Bk-App-Secret"] = _M.app_secret, + ["X-Request-Id"] = request_id, ["Content-Type"] = "application/x-www-form-urlencoded", }, } ) if not (res and res.body) then - err = string_format("failed to request third-party api, url: %s, err: %s, response: nil", url, err) + err = string_format("failed to request third-party api, url: %s, request_id: %s, err: %s, response: nil", url, + request_id, err) core.log.error(err) return nil, err end @@ -148,24 +161,28 @@ function _M.list_app_secrets(app_code) if result == nil then core.log.error( string_format( - "failed to request %s, response is not valid json, status: %s, response: %s", url, res.status, res.body + "failed to request %s, request_id: %s, response is not valid json, status: %s, response: %s", url, + request_id, res.status, res.body ) ) return nil, string_format( - "failed to request third-party api, response is not valid json, url: %s, status: %s", url, res.status + "failed to request third-party api, response is not valid json, url: %s, request_id: %s, status: %s", url, + request_id, res.status ) end if result.code ~= 0 or res.status ~= 200 then core.log.error( string_format( - "failed to request %s, result.code!=0 or status!=200, status: %s, response: %s", url, res.status, + "failed to request %s, request_id: %s, result.code!=0 or status!=200, status: %s, response: %s", url, + request_id, res.status, res.body ) ) return nil, string_format( - "failed to request third-party api, bkauth error message: %s, url: %s, status: %s, code: %s", - result.message, url, res.status, result.code + "failed to request third-party api, bkauth error message: %s, url: %s,\ + request_id: %s, status: %s, code: %s", + result.message, url, request_id, res.status, result.code ) end @@ -188,6 +205,7 @@ function _M.verify_access_token(access_token) local http_client = http.new() http_client:set_timeout(BKAUTH_TIMEOUT_MS) + local request_id = uuid.generate_v4() local res, err = http_client:request_uri( url, { method = "POST", @@ -200,6 +218,7 @@ function _M.verify_access_token(access_token) headers = { ["X-Bk-App-Code"] = _M.app_code, ["X-Bk-App-Secret"] = _M.app_secret, + ["X-Request-Id"] = request_id, -- ["Authorization"] = "Bearer " .. self.bkauth_access_token ["Content-Type"] = "application/json", }, @@ -207,7 +226,8 @@ function _M.verify_access_token(access_token) ) if not (res and res.body) then - err = string_format("failed to request third-party api, url: %s, err: %s, response: nil", url, err) + err = string_format("failed to request third-party api, url: %s, request_id: %s, err: %s, response: nil", url, + request_id, err) core.log.error(err) return nil, err end @@ -216,17 +236,20 @@ function _M.verify_access_token(access_token) if result == nil then core.log.error( string_format( - "failed to request %s, response is not valid json, status: %s, response: %s", url, res.status, res.body + "failed to request %s, request_id: %s, response is not valid json, status: %s, response: %s", url, + request_id, res.status, res.body ) ) return nil, string_format( - "failed to request third-party api, response is not valid json, url: %s, status: %s", url, res.status + "failed to request third-party api, response is not valid json, url: %s, request_id: %s, status: %s", url, + request_id, res.status ) end if result.code ~= 0 or res.status ~= 200 then return nil, string_format( - "bkauth error message: %s, url: %s, status: %s, code: %s", result.message, url, res.status, result.code + "bkauth error message: %s, url: %s, request_id: %s, status: %s, code: %s", + result.message, url, request_id, res.status, result.code ) end diff --git a/src/apisix/tests/bk-components/test-bkauth.lua b/src/apisix/tests/bk-components/test-bkauth.lua index e1329c0..0ebd3fb 100644 --- a/src/apisix/tests/bk-components/test-bkauth.lua +++ b/src/apisix/tests/bk-components/test-bkauth.lua @@ -21,7 +21,6 @@ local bkauth = require("apisix.plugins.bk-components.bkauth") describe( "bkauth", function() - local response, response_err before_each( @@ -59,6 +58,7 @@ describe( local result, err = bkauth.verify_app_secret("fake-app-code", "fake-app-secret") assert.is_nil(result) assert.is_true(core.string.has_prefix(err, "failed to request third-party api")) + assert.is_true(core.string.find(err, "request_id") ~= nil) end ) @@ -93,6 +93,7 @@ describe( local result, err = bkauth.verify_app_secret("fake-app-code", "fake-app-secret") assert.is_nil(result) assert.is_true(core.string.has_prefix(err, "failed to request third-party api")) + assert.is_true(core.string.find(err, "request_id") ~= nil) end ) @@ -115,6 +116,7 @@ describe( local result, err = bkauth.verify_app_secret("fake-app-code", "fake-app-secret") assert.is_nil(result) assert.is_true(core.string.has_prefix(err, "failed to request third-party api")) + assert.is_true(core.string.find(err, "request_id") ~= nil) end ) @@ -174,6 +176,7 @@ describe( local result, err = bkauth.list_app_secrets("fake-app-code") assert.is_nil(result) assert.is_true(core.string.has_prefix(err, "failed to request third-party api")) + assert.is_true(core.string.find(err, "request_id") ~= nil) end ) it( @@ -210,6 +213,7 @@ describe( local result, err = bkauth.list_app_secrets("fake-app-code") assert.is_nil(result) assert.is_true(core.string.has_prefix(err, "failed to request third-party api")) + assert.is_true(core.string.find(err, "request_id") ~= nil) end ) @@ -232,6 +236,7 @@ describe( local result, err = bkauth.list_app_secrets("fake-app-code") assert.is_nil(result) assert.is_true(core.string.has_prefix(err, "failed to request third-party api")) + assert.is_true(core.string.find(err, "request_id") ~= nil) end ) @@ -282,6 +287,7 @@ describe( local result, err = bkauth.verify_access_token("fake-token") assert.is_nil(result) assert.is_true(core.string.has_prefix(err, "failed to request third-party api")) + assert.is_true(core.string.find(err, "request_id") ~= nil) end ) @@ -295,6 +301,7 @@ describe( local result, err = bkauth.verify_access_token("fake-token") assert.is_nil(result) assert.is_true(core.string.has_prefix(err, "failed to request third-party api")) + assert.is_true(core.string.find(err, "request_id") ~= nil) end ) @@ -309,6 +316,7 @@ describe( local result, err = bkauth.verify_access_token("fake-token") assert.is_nil(result) assert.is_true(core.string.has_prefix(err, "failed to request third-party api")) + assert.is_true(core.string.find(err, "request_id") ~= nil) end ) @@ -328,6 +336,7 @@ describe( local result, err = bkauth.verify_access_token("fake-token") assert.is_nil(result) assert.is_true(core.string.has_prefix(err, "bkauth error message: error")) + assert.is_true(core.string.find(err, "request_id") ~= nil) end ) @@ -347,6 +356,7 @@ describe( local result, err = bkauth.verify_access_token("fake-token") assert.is_nil(result) assert.is_true(core.string.has_prefix(err, "bkauth error message: error")) + assert.is_true(core.string.find(err, "request_id") ~= nil) end )