From 9e0fd846520cf4268407f59ecbad976fb8a8f0af Mon Sep 17 00:00:00 2001 From: Yusheng Li Date: Fri, 31 May 2024 15:53:25 +0800 Subject: [PATCH] fix(openid-connect): fixed a bug where anonymous consumers may be cached as nil under a certain condition (#9271) --- .../kong-ee/fix-oidc-cache-corruption.yml | 3 + .../kong/plugins/openid-connect/cache.lua | 6 ++ .../spec/openid-connect/05-keycloak_spec.lua | 87 +++++++++++++++++++ 3 files changed, 96 insertions(+) create mode 100644 changelog/unreleased/kong-ee/fix-oidc-cache-corruption.yml diff --git a/changelog/unreleased/kong-ee/fix-oidc-cache-corruption.yml b/changelog/unreleased/kong-ee/fix-oidc-cache-corruption.yml new file mode 100644 index 000000000000..0bff19869b8b --- /dev/null +++ b/changelog/unreleased/kong-ee/fix-oidc-cache-corruption.yml @@ -0,0 +1,3 @@ +message: "**OpenID Connect**: Fixed a bug where anonymous consumers may be cached as nil under a certain condition." +type: bugfix +scope: Plugin diff --git a/plugins-ee/openid-connect/kong/plugins/openid-connect/cache.lua b/plugins-ee/openid-connect/kong/plugins/openid-connect/cache.lua index a01d32216bd3..7b703b62ef7e 100644 --- a/plugins-ee/openid-connect/kong/plugins/openid-connect/cache.lua +++ b/plugins-ee/openid-connect/kong/plugins/openid-connect/cache.lua @@ -1040,6 +1040,10 @@ function consumers.load(subject, anonymous, consumer_by, ttl, by_username_ignore local err for _, field_name in ipairs(field_names) do + if field_name == "id" and not utils.is_valid_uuid(subject) then + goto continue + end + local key if field_name == "id" then @@ -1057,6 +1061,8 @@ function consumers.load(subject, anonymous, consumer_by, ttl, by_username_ignore if consumer then return consumer end + + ::continue:: end return nil, err diff --git a/plugins-ee/openid-connect/spec/openid-connect/05-keycloak_spec.lua b/plugins-ee/openid-connect/spec/openid-connect/05-keycloak_spec.lua index a1806f8484cc..3bdc879a914e 100644 --- a/plugins-ee/openid-connect/spec/openid-connect/05-keycloak_spec.lua +++ b/plugins-ee/openid-connect/spec/openid-connect/05-keycloak_spec.lua @@ -3591,6 +3591,93 @@ for _, strategy in helpers.all_strategies() do end) end) + describe("FTI-5861 existing anonymous consumer should not be cached to nil", function() + local proxy_client, anonymous_test + lazy_setup(function() + local bp = helpers.get_db_utils(strategy, { + "routes", + "services", + "plugins", + }, { + PLUGIN_NAME, "key-auth" + }) + + local service = bp.services:insert { + path = "/anything" + } + local oidc_route = bp.routes:insert { + service = service, + paths = { "/oidc-test" }, + } + local keyauth_route = bp.routes:insert { + service = service, + paths = { "/keyauth-test" }, + } + anonymous_test = bp.consumers:insert { + username = "anonymous_test" + } + bp.plugins:insert { + route = oidc_route, + name = PLUGIN_NAME, + config = { + issuer = ISSUER_URL, + client_id = { + KONG_CLIENT_ID, + }, + client_secret = { + KONG_CLIENT_SECRET, + }, + auth_methods = { + "password" + }, + anonymous = anonymous_test.username, + }, + } + bp.plugins:insert { + route = keyauth_route, + name = "key-auth", + config = { + anonymous = anonymous_test.username, + }, + } + assert(helpers.start_kong({ + database = strategy, + nginx_conf = "spec/fixtures/custom_nginx.template", + plugins = "bundled," .. PLUGIN_NAME, + })) + end) + + lazy_teardown(function() + helpers.stop_kong() + end) + + before_each(function() + proxy_client = helpers.proxy_client() + end) + + after_each(function() + if proxy_client then + proxy_client:close() + end + end) + + it("key-auth plugin should pass after call oidc first", function () + local res = proxy_client:get("/oidc-test", { + headers = { + Authorization = "incorrectpw", + }, + }) + assert.response(res).has.status(200) + local anon_consumer = assert.request(res).has.header("x-anonymous-consumer") + assert.is_same(anon_consumer, "true") + local id = assert.request(res).has.header("x-consumer-id") + assert.equal(id, anonymous_test.id) + + res = proxy_client:get("/keyauth-test") + assert.response(res).has.status(200) + end) + end) + for _, c in ipairs({ { alg = "ES256",