From d9ad8ed68a4169364a0bf3cc40428b8fb175a29e Mon Sep 17 00:00:00 2001 From: Rui Engana <11096527+ruiengana@users.noreply.github.com> Date: Thu, 3 Jun 2021 19:04:09 +0100 Subject: [PATCH 01/14] added "introspection_cache_ignore" config option --- kong/plugins/oidc/schema.lua | 3 ++- kong/plugins/oidc/utils.lua | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/kong/plugins/oidc/schema.lua b/kong/plugins/oidc/schema.lua index 933bf200..94799f76 100644 --- a/kong/plugins/oidc/schema.lua +++ b/kong/plugins/oidc/schema.lua @@ -5,8 +5,9 @@ return { client_secret = { type = "string", required = true }, discovery = { type = "string", required = true, default = "https://.well-known/openid-configuration" }, introspection_endpoint = { type = "string", required = false }, - timeout = { type = "number", required = false }, introspection_endpoint_auth_method = { type = "string", required = false }, + introspection_cache_ignore = { type = "string", required = true, default = "no" }, + timeout = { type = "number", required = false }, bearer_only = { type = "string", required = true, default = "no" }, realm = { type = "string", required = true, default = "kong" }, redirect_uri = { type = "string" }, diff --git a/kong/plugins/oidc/utils.lua b/kong/plugins/oidc/utils.lua index 76044ec5..aaa777b2 100644 --- a/kong/plugins/oidc/utils.lua +++ b/kong/plugins/oidc/utils.lua @@ -50,8 +50,9 @@ function M.get_options(config, ngx) client_secret = config.client_secret, discovery = config.discovery, introspection_endpoint = config.introspection_endpoint, - timeout = config.timeout, introspection_endpoint_auth_method = config.introspection_endpoint_auth_method, + introspection_cache_ignore = config.introspection_cache_ignore, + timeout = config.timeout, bearer_only = config.bearer_only, realm = config.realm, redirect_uri = config.redirect_uri or M.get_redirect_uri(ngx), From 577aaf9a2cc7087055aae55bca789d48c7ba19e5 Mon Sep 17 00:00:00 2001 From: Cristian Chiru Date: Fri, 20 Aug 2021 11:51:35 +0300 Subject: [PATCH 02/14] Update kong-oidc-1.2.3-1.rockspec --- kong-oidc-1.2.3-1.rockspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kong-oidc-1.2.3-1.rockspec b/kong-oidc-1.2.3-1.rockspec index 0bf4fe2b..c69f7aeb 100644 --- a/kong-oidc-1.2.3-1.rockspec +++ b/kong-oidc-1.2.3-1.rockspec @@ -1,5 +1,5 @@ package = "kong-oidc" -version = "1.2.3-1" +version = "1.2.3-2" source = { url = "git://github.com/revomatico/kong-oidc", tag = "master", From 82ec13bec25ea50930f84883a8ea0f3a2e57a119 Mon Sep 17 00:00:00 2001 From: Cristian Chiru Date: Fri, 20 Aug 2021 11:52:03 +0300 Subject: [PATCH 03/14] Rename kong-oidc-1.2.3-1.rockspec to kong-oidc-1.2.3-2.rockspec --- kong-oidc-1.2.3-1.rockspec => kong-oidc-1.2.3-2.rockspec | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename kong-oidc-1.2.3-1.rockspec => kong-oidc-1.2.3-2.rockspec (100%) diff --git a/kong-oidc-1.2.3-1.rockspec b/kong-oidc-1.2.3-2.rockspec similarity index 100% rename from kong-oidc-1.2.3-1.rockspec rename to kong-oidc-1.2.3-2.rockspec From 4ee4c2cf72581a9049df7f3572dbe857a0627e16 Mon Sep 17 00:00:00 2001 From: Stian Haugland Date: Tue, 5 Oct 2021 09:48:37 +0200 Subject: [PATCH 04/14] Add scope validation for jwt tokens --- kong/plugins/oidc/handler.lua | 15 +++++++++++++++ kong/plugins/oidc/schema.lua | 1 + kong/plugins/oidc/utils.lua | 1 + 3 files changed, 17 insertions(+) diff --git a/kong/plugins/oidc/handler.lua b/kong/plugins/oidc/handler.lua index cbb51c8b..23df091e 100644 --- a/kong/plugins/oidc/handler.lua +++ b/kong/plugins/oidc/handler.lua @@ -118,6 +118,21 @@ function introspect(oidcConfig) local res, err if oidcConfig.use_jwks == "yes" then res, err = require("resty.openidc").bearer_jwt_verify(oidcConfig) + if err then + utils.exit(ngx.HTTP_UNAUTHORIZED, err, ngx.HTTP_UNAUTHORIZED) + end + if oidcConfig.validate_scope == "yes" then + local validScope = false + for scope in res.scope:gmatch("([^ ]+)") do + if scope == oidcConfig.scope then + validScope = true + break + end + end + if not validScope then + utils.exit(ngx.HTTP_FORBIDDEN,"Invalid scope",ngx.HTTP_FORBIDDEN) + end + end else res, err = require("resty.openidc").introspect(oidcConfig) end diff --git a/kong/plugins/oidc/schema.lua b/kong/plugins/oidc/schema.lua index 94799f76..0c6f7bbf 100644 --- a/kong/plugins/oidc/schema.lua +++ b/kong/plugins/oidc/schema.lua @@ -12,6 +12,7 @@ return { realm = { type = "string", required = true, default = "kong" }, redirect_uri = { type = "string" }, scope = { type = "string", required = true, default = "openid" }, + validate_scope = { type = "string", required = true, default = "no" }, response_type = { type = "string", required = true, default = "code" }, ssl_verify = { type = "string", required = true, default = "no" }, use_jwks = { type = "string", required = true, default = "no" }, diff --git a/kong/plugins/oidc/utils.lua b/kong/plugins/oidc/utils.lua index aaa777b2..b7cdf5c2 100644 --- a/kong/plugins/oidc/utils.lua +++ b/kong/plugins/oidc/utils.lua @@ -57,6 +57,7 @@ function M.get_options(config, ngx) realm = config.realm, redirect_uri = config.redirect_uri or M.get_redirect_uri(ngx), scope = config.scope, + validate_scope = config.validate_scope, response_type = config.response_type, ssl_verify = config.ssl_verify, use_jwks = config.use_jwks, From 3c93d3204797f498156d0759de5941a9e54c6669 Mon Sep 17 00:00:00 2001 From: Cristian Chiru Date: Tue, 25 Jan 2022 12:52:09 +0200 Subject: [PATCH 05/14] Update and rename kong-oidc-1.2.3-2.rockspec to kong-oidc-1.2.4-1.rockspec --- kong-oidc-1.2.3-2.rockspec => kong-oidc-1.2.4-1.rockspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename kong-oidc-1.2.3-2.rockspec => kong-oidc-1.2.4-1.rockspec (98%) diff --git a/kong-oidc-1.2.3-2.rockspec b/kong-oidc-1.2.4-1.rockspec similarity index 98% rename from kong-oidc-1.2.3-2.rockspec rename to kong-oidc-1.2.4-1.rockspec index c69f7aeb..f5de8482 100644 --- a/kong-oidc-1.2.3-2.rockspec +++ b/kong-oidc-1.2.4-1.rockspec @@ -1,5 +1,5 @@ package = "kong-oidc" -version = "1.2.3-2" +version = "1.2.4-1" source = { url = "git://github.com/revomatico/kong-oidc", tag = "master", From ee5637051f5de8d0cc8df9b3b9572002fa7dbbb2 Mon Sep 17 00:00:00 2001 From: Cristian Chiru Date: Tue, 25 Jan 2022 12:52:54 +0200 Subject: [PATCH 06/14] Bump lua-resty-openidc dep to 1.7.5-1 --- kong-oidc-1.2.4-1.rockspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kong-oidc-1.2.4-1.rockspec b/kong-oidc-1.2.4-1.rockspec index f5de8482..1718b9c6 100644 --- a/kong-oidc-1.2.4-1.rockspec +++ b/kong-oidc-1.2.4-1.rockspec @@ -22,7 +22,7 @@ description = { license = "Apache 2.0" } dependencies = { - "lua-resty-openidc ~> 1.7.4-1" + "lua-resty-openidc ~> 1.7.5-1" } build = { type = "builtin", From 32c0bcf20c2e23bdc8d9018b1ef09e773b6a1137 Mon Sep 17 00:00:00 2001 From: Rui Engana <11096527+ruiengana@users.noreply.github.com> Date: Mon, 14 Feb 2022 11:51:49 +0000 Subject: [PATCH 07/14] chore: refactor introspection scope validation --- kong/plugins/oidc/handler.lua | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/kong/plugins/oidc/handler.lua b/kong/plugins/oidc/handler.lua index 23df091e..de7e3dbe 100644 --- a/kong/plugins/oidc/handler.lua +++ b/kong/plugins/oidc/handler.lua @@ -118,21 +118,6 @@ function introspect(oidcConfig) local res, err if oidcConfig.use_jwks == "yes" then res, err = require("resty.openidc").bearer_jwt_verify(oidcConfig) - if err then - utils.exit(ngx.HTTP_UNAUTHORIZED, err, ngx.HTTP_UNAUTHORIZED) - end - if oidcConfig.validate_scope == "yes" then - local validScope = false - for scope in res.scope:gmatch("([^ ]+)") do - if scope == oidcConfig.scope then - validScope = true - break - end - end - if not validScope then - utils.exit(ngx.HTTP_FORBIDDEN,"Invalid scope",ngx.HTTP_FORBIDDEN) - end - end else res, err = require("resty.openidc").introspect(oidcConfig) end @@ -143,6 +128,19 @@ function introspect(oidcConfig) end return nil end + -- authorization - validate scope + if oidcConfig.validate_scope == "yes" then + local validScope = false + for scope in res.scope:gmatch("([^ ]+)") do + if scope == oidcConfig.scope then + validScope = true + break + end + end + if not validScope then + utils.exit(ngx.HTTP_FORBIDDEN,"Invalid scope",ngx.HTTP_FORBIDDEN) + end + end ngx.log(ngx.DEBUG, "OidcHandler introspect succeeded, requested path: " .. ngx.var.request_uri) return res end From 879e2bb1059bade0d90f8ef78fb49fcfc35f294c Mon Sep 17 00:00:00 2001 From: Cristian Chiru Date: Tue, 15 Feb 2022 13:55:59 +0200 Subject: [PATCH 08/14] Update and rename kong-oidc-1.2.4-1.rockspec to kong-oidc-1.2.4-2.rockspec --- kong-oidc-1.2.4-1.rockspec => kong-oidc-1.2.4-2.rockspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename kong-oidc-1.2.4-1.rockspec => kong-oidc-1.2.4-2.rockspec (98%) diff --git a/kong-oidc-1.2.4-1.rockspec b/kong-oidc-1.2.4-2.rockspec similarity index 98% rename from kong-oidc-1.2.4-1.rockspec rename to kong-oidc-1.2.4-2.rockspec index 1718b9c6..38dd4bdf 100644 --- a/kong-oidc-1.2.4-1.rockspec +++ b/kong-oidc-1.2.4-2.rockspec @@ -1,5 +1,5 @@ package = "kong-oidc" -version = "1.2.4-1" +version = "1.2.4-2" source = { url = "git://github.com/revomatico/kong-oidc", tag = "master", From 837c483447d38431f8f23d44da249858991d8e7f Mon Sep 17 00:00:00 2001 From: Rui Engana <11096527+ruiengana@users.noreply.github.com> Date: Fri, 1 Apr 2022 12:38:38 +0100 Subject: [PATCH 09/14] fix: check scope claim from introspection response --- kong/plugins/oidc/handler.lua | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/kong/plugins/oidc/handler.lua b/kong/plugins/oidc/handler.lua index de7e3dbe..83b6e32b 100644 --- a/kong/plugins/oidc/handler.lua +++ b/kong/plugins/oidc/handler.lua @@ -131,11 +131,13 @@ function introspect(oidcConfig) -- authorization - validate scope if oidcConfig.validate_scope == "yes" then local validScope = false - for scope in res.scope:gmatch("([^ ]+)") do - if scope == oidcConfig.scope then - validScope = true - break - end + if res.scope then + for scope in res.scope:gmatch("([^ ]+)") do + if scope == oidcConfig.scope then + validScope = true + break + end + end end if not validScope then utils.exit(ngx.HTTP_FORBIDDEN,"Invalid scope",ngx.HTTP_FORBIDDEN) From eee1fa9d7d0455982656a8887df5a9269c0f2881 Mon Sep 17 00:00:00 2001 From: Rui Engana <11096527+ruiengana@users.noreply.github.com> Date: Fri, 1 Apr 2022 12:41:27 +0100 Subject: [PATCH 10/14] remove comment and empty spaces --- kong/plugins/oidc/handler.lua | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/kong/plugins/oidc/handler.lua b/kong/plugins/oidc/handler.lua index 83b6e32b..422e2168 100644 --- a/kong/plugins/oidc/handler.lua +++ b/kong/plugins/oidc/handler.lua @@ -128,7 +128,6 @@ function introspect(oidcConfig) end return nil end - -- authorization - validate scope if oidcConfig.validate_scope == "yes" then local validScope = false if res.scope then @@ -137,7 +136,7 @@ function introspect(oidcConfig) validScope = true break end - end + end end if not validScope then utils.exit(ngx.HTTP_FORBIDDEN,"Invalid scope",ngx.HTTP_FORBIDDEN) From 66c0b92c85914befcbeefe6e026f7448f1cf70d9 Mon Sep 17 00:00:00 2001 From: Rui Engana <11096527+ruiengana@users.noreply.github.com> Date: Fri, 1 Apr 2022 17:53:06 +0100 Subject: [PATCH 11/14] added scope validation unit tests updated test framework to latest kong, postgres, keycloak images refactor utils.exit() to log errors --- .env | 6 +-- kong/plugins/oidc/handler.lua | 8 +-- kong/plugins/oidc/session.lua | 2 +- kong/plugins/oidc/utils.lua | 8 +-- test/docker/integration/Dockerfile | 5 +- test/docker/unit/Dockerfile | 10 ++-- test/unit/mockable_case.lua | 2 + test/unit/test_handler_mocking_openidc.lua | 61 +++++++++++++++++++--- 8 files changed, 79 insertions(+), 23 deletions(-) diff --git a/.env b/.env index 3af9ffcb..099ff02c 100644 --- a/.env +++ b/.env @@ -2,9 +2,9 @@ BUILD_IMG_NAME=nokia/kong-oidc INTEGRATION_PATH=test/docker/integration UNIT_PATH=test/docker/unit -KONG_BASE_TAG=:2.2.1-centos +KONG_BASE_TAG=:2.8.0-ubuntu KONG_TAG= -KONG_DB_TAG=:12 +KONG_DB_TAG=:14 KONG_DB_PORT=5432 KONG_DB_USER=kong KONG_DB_PW=kong @@ -13,7 +13,7 @@ KONG_SESSION_STORE_PORT=6379 KONG_HTTP_PROXY_PORT=8000 KONG_HTTP_ADMIN_PORT=8001 -KEYCLOAK_TAG=:4.8.3.Final +KEYCLOAK_TAG=:16.1.1 KEYCLOAK_PORT=8081 KEYCLOAK_USER=admin KEYCLOAK_PW=password diff --git a/kong/plugins/oidc/handler.lua b/kong/plugins/oidc/handler.lua index 422e2168..5cede4fa 100644 --- a/kong/plugins/oidc/handler.lua +++ b/kong/plugins/oidc/handler.lua @@ -101,13 +101,13 @@ function make_oidc(oidcConfig) if err then if err == 'unauthorized request' then - utils.exit(ngx.HTTP_UNAUTHORIZED, err, ngx.HTTP_UNAUTHORIZED) + utils.exit(ngx.HTTP_UNAUTHORIZED, err) else if oidcConfig.recovery_page_path then ngx.log(ngx.DEBUG, "Redirecting to recovery page: " .. oidcConfig.recovery_page_path) ngx.redirect(oidcConfig.recovery_page_path) end - utils.exit(ngx.HTTP_INTERNAL_SERVER_ERROR, err, ngx.HTTP_INTERNAL_SERVER_ERROR) + utils.exit(ngx.HTTP_INTERNAL_SERVER_ERROR, err) end end return res @@ -124,7 +124,7 @@ function introspect(oidcConfig) if err then if oidcConfig.bearer_only == "yes" then ngx.header["WWW-Authenticate"] = 'Bearer realm="' .. oidcConfig.realm .. '",error="' .. err .. '"' - utils.exit(ngx.HTTP_UNAUTHORIZED, err, ngx.HTTP_UNAUTHORIZED) + utils.exit(ngx.HTTP_UNAUTHORIZED, err) end return nil end @@ -139,7 +139,7 @@ function introspect(oidcConfig) end end if not validScope then - utils.exit(ngx.HTTP_FORBIDDEN,"Invalid scope",ngx.HTTP_FORBIDDEN) + utils.exit(ngx.HTTP_FORBIDDEN, 'Scope validation failed') end end ngx.log(ngx.DEBUG, "OidcHandler introspect succeeded, requested path: " .. ngx.var.request_uri) diff --git a/kong/plugins/oidc/session.lua b/kong/plugins/oidc/session.lua index 18875546..a3b96468 100644 --- a/kong/plugins/oidc/session.lua +++ b/kong/plugins/oidc/session.lua @@ -6,7 +6,7 @@ function M.configure(config) if config.session_secret then local decoded_session_secret = ngx.decode_base64(config.session_secret) if not decoded_session_secret then - utils.exit(500, "invalid OIDC plugin configuration, session secret could not be decoded", ngx.exit(ngx.HTTP_INTERNAL_SERVER_ERROR)) + utils.exit(ngx.HTTP_INTERNAL_SERVER_ERROR, 'Invalid plugin configuration, session secret could not be decoded') end ngx.var.session_secret = decoded_session_secret end diff --git a/kong/plugins/oidc/utils.lua b/kong/plugins/oidc/utils.lua index b7cdf5c2..272c1a1b 100644 --- a/kong/plugins/oidc/utils.lua +++ b/kong/plugins/oidc/utils.lua @@ -84,10 +84,10 @@ function M.get_options(config, ngx) } end -function M.exit(httpStatusCode, message, ngxCode) - ngx.status = httpStatusCode - ngx.say(message) - ngx.exit(ngxCode) +function M.exit(statusCode, message) + ngx.status = statusCode + kong.log.err(message) + ngx.exit(statusCode) end diff --git a/test/docker/integration/Dockerfile b/test/docker/integration/Dockerfile index 48d721c2..f6226135 100644 --- a/test/docker/integration/Dockerfile +++ b/test/docker/integration/Dockerfile @@ -7,12 +7,13 @@ ENV LUA_PATH /usr/local/share/lua/5.1/?.lua;/usr/local/kong-oidc/?.lua;; ENV LUA_CPATH /usr/local/lib/lua/5.1/?.so;; # Install unzip for luarocks, gcc for lua-cjson -RUN yum install -y unzip gcc curl +RUN apt update && apt install -y unzip gcc curl RUN luarocks install luacov RUN luarocks install luaunit RUN luarocks install lua-cjson +RUN luarocks install luaossl OPENSSL_DIR=/usr/local/kong CRYPTO_DIR=/usr/local/kong # Change openidc version when version in rockspec changes -RUN luarocks install lua-resty-openidc 1.7.4-1 +RUN luarocks install lua-resty-openidc 1.7.5-1 COPY . /usr/local/kong-oidc diff --git a/test/docker/unit/Dockerfile b/test/docker/unit/Dockerfile index 2bacadaf..ed414800 100644 --- a/test/docker/unit/Dockerfile +++ b/test/docker/unit/Dockerfile @@ -1,16 +1,20 @@ ARG KONG_BASE_TAG FROM kong${KONG_BASE_TAG} USER root + ENV LUA_PATH /usr/local/share/lua/5.1/?.lua;/usr/local/kong-oidc/?.lua # For lua-cjson ENV LUA_CPATH /usr/local/lib/lua/5.1/?.so # Install unzip for luarocks, gcc for lua-cjson -RUN echo "ip_resolve=4" >> /etc/yum.conf && yum install -y unzip gcc -# Change openidc version when version in rockspec changes -RUN luarocks install lua-resty-openidc 1.7.4-1 +RUN apt update && apt install -y unzip gcc curl RUN luarocks install luacov RUN luarocks install luaunit +RUN luarocks install lua-cjson +RUN luarocks install luaossl OPENSSL_DIR=/usr/local/kong CRYPTO_DIR=/usr/local/kong + +# Change openidc version when version in rockspec changes +RUN luarocks install lua-resty-openidc 1.7.5-1 WORKDIR /usr/local/kong-oidc diff --git a/test/unit/mockable_case.lua b/test/unit/mockable_case.lua index ebc90699..78d0e9ad 100644 --- a/test/unit/mockable_case.lua +++ b/test/unit/mockable_case.lua @@ -9,6 +9,8 @@ function MockableCase:setUp() DEBUG = "debug", ERR = "error", HTTP_UNAUTHORIZED = 401, + HTTP_FORBIDDEN = 403, + HTTP_INTERNAL_SERVER_ERROR = 500, ctx = {}, header = {}, var = {request_uri = "/"}, diff --git a/test/unit/test_handler_mocking_openidc.lua b/test/unit/test_handler_mocking_openidc.lua index e5b18969..fad410b9 100644 --- a/test/unit/test_handler_mocking_openidc.lua +++ b/test/unit/test_handler_mocking_openidc.lua @@ -214,14 +214,63 @@ function TestHandler:test_introspect_bearer_token_and_property_mapping() ngx.encode_base64 = function(x) return "x" end local headers = {} - ngx.req.set_header = function(h, v) - headers[h] = v - end + kong.service.request.set_header = function(name, value) headers[name] = value end - self.handler:access({introspection_endpoint = "x", bearer_only = "yes", use_jwks = "yes", mappings = {'foo:X-Foo', 'incorrect', 'not:present'}}) + self.handler:access({introspection_endpoint = "x", bearer_only = "yes", use_jwks = "yes", disable_userinfo_header = "yes", header_names = {'X-Foo', 'present'}, header_claims = {'foo', 'not'}}) lu.assertEquals(headers["X-Foo"], 'bar') - lu.assertTrue(self:log_contains("not present on token")) - lu.assertTrue(self:log_contains("Ignoring incorrect configuration")) + lu.assertNil(headers["present"]) +end + +function TestHandler:test_introspect_bearer_token_and_incorrect_property_mapping() + self.module_resty.openidc.bearer_jwt_verify = function(opts) + return {foo = "bar"}, false + end + ngx.req.get_headers = function() return {Authorization = "Bearer xxx"} end + + ngx.encode_base64 = function(x) return "x" end + + local headers = {} + kong.service.request.set_header = function(name, value) headers[name] = value end + + self.handler:access({introspection_endpoint = "x", bearer_only = "yes", use_jwks = "yes", disable_userinfo_header = "yes", header_names = {'X-Foo'}, header_claims = {'foo', 'incorrect'}}) + lu.assertNil(headers["X-Foo"]) +end + +function TestHandler:test_introspect_bearer_token_and_scope_nok() + self.module_resty.openidc.bearer_jwt_verify = function(opts) + return {scope = "foo"}, false + end + ngx.req.get_headers = function() return {Authorization = "Bearer xxx"} end + + ngx.encode_base64 = function(x) return "x" end + + self.handler:access({introspection_endpoint = "x", bearer_only = "yes", use_jwks = "yes", userinfo_header_name = "X-Userinfo", validate_scope = "yes", scope = "bar"}) + lu.assertEquals(ngx.status, ngx.HTTP_FORBIDDEN) +end + +function TestHandler:test_introspect_bearer_token_and_empty_scope_nok() + self.module_resty.openidc.bearer_jwt_verify = function(opts) + return {foo = "bar"}, false + end + ngx.req.get_headers = function() return {Authorization = "Bearer xxx"} end + + ngx.encode_base64 = function(x) return "x" end + + self.handler:access({introspection_endpoint = "x", bearer_only = "yes", use_jwks = "yes", userinfo_header_name = "X-Userinfo", validate_scope = "yes", scope = "bar"}) + lu.assertEquals(ngx.status, ngx.HTTP_FORBIDDEN) +end + +function TestHandler:test_introspect_bearer_token_and_scope_ok() + self.module_resty.openidc.bearer_jwt_verify = function(opts) + return {scope = "foo bar"}, false + end + ngx.req.get_headers = function() return {Authorization = "Bearer xxx"} end + + ngx.encode_base64 = function(x) return "x" end + + self.handler:access({introspection_endpoint = "x", bearer_only = "yes", use_jwks = "yes", userinfo_header_name = "X-Userinfo", validate_scope = "yes", scope = "bar"}) + lu.assertNotEquals(ngx.status, ngx.HTTP_FORBIDDEN) + lu.assertNotEquals(ngx.status, ngx.HTTP_INTERNAL_SERVER_ERROR) end lu.run() From 79550c4b7ca4b5f563face565e88df0add3a62c5 Mon Sep 17 00:00:00 2001 From: Cristian Chiru Date: Fri, 1 Apr 2022 20:10:50 +0300 Subject: [PATCH 12/14] Update and rename kong-oidc-1.2.4-2.rockspec to kong-oidc-1.2.4-3.rockspec --- kong-oidc-1.2.4-2.rockspec => kong-oidc-1.2.4-3.rockspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename kong-oidc-1.2.4-2.rockspec => kong-oidc-1.2.4-3.rockspec (98%) diff --git a/kong-oidc-1.2.4-2.rockspec b/kong-oidc-1.2.4-3.rockspec similarity index 98% rename from kong-oidc-1.2.4-2.rockspec rename to kong-oidc-1.2.4-3.rockspec index 38dd4bdf..3368503f 100644 --- a/kong-oidc-1.2.4-2.rockspec +++ b/kong-oidc-1.2.4-3.rockspec @@ -1,5 +1,5 @@ package = "kong-oidc" -version = "1.2.4-2" +version = "1.2.4-3" source = { url = "git://github.com/revomatico/kong-oidc", tag = "master", From 24bcb72d0e535762ea8f93a5ccf714c8ae3d842d Mon Sep 17 00:00:00 2001 From: Rui Engana <11096527+ruiengana@users.noreply.github.com> Date: Sun, 3 Apr 2022 09:43:56 +0100 Subject: [PATCH 13/14] Align error response bodies with official plugins Fix unit tests --- kong/plugins/oidc/handler.lua | 9 +++-- kong/plugins/oidc/session.lua | 3 +- kong/plugins/oidc/utils.lua | 38 +++++++------------ test/unit/mockable_case.lua | 5 +++ test/unit/test_handler_mocking_openidc.lua | 44 ++++++++-------------- test/unit/test_header_claims.lua | 3 +- test/unit/test_introspect.lua | 10 ++--- 7 files changed, 45 insertions(+), 67 deletions(-) diff --git a/kong/plugins/oidc/handler.lua b/kong/plugins/oidc/handler.lua index 5cede4fa..44dfd590 100644 --- a/kong/plugins/oidc/handler.lua +++ b/kong/plugins/oidc/handler.lua @@ -101,13 +101,13 @@ function make_oidc(oidcConfig) if err then if err == 'unauthorized request' then - utils.exit(ngx.HTTP_UNAUTHORIZED, err) + return kong.response.error(ngx.HTTP_UNAUTHORIZED) else if oidcConfig.recovery_page_path then ngx.log(ngx.DEBUG, "Redirecting to recovery page: " .. oidcConfig.recovery_page_path) ngx.redirect(oidcConfig.recovery_page_path) end - utils.exit(ngx.HTTP_INTERNAL_SERVER_ERROR, err) + return kong.response.error(ngx.HTTP_INTERNAL_SERVER_ERROR) end end return res @@ -124,7 +124,7 @@ function introspect(oidcConfig) if err then if oidcConfig.bearer_only == "yes" then ngx.header["WWW-Authenticate"] = 'Bearer realm="' .. oidcConfig.realm .. '",error="' .. err .. '"' - utils.exit(ngx.HTTP_UNAUTHORIZED, err) + return kong.response.error(ngx.HTTP_UNAUTHORIZED) end return nil end @@ -139,7 +139,8 @@ function introspect(oidcConfig) end end if not validScope then - utils.exit(ngx.HTTP_FORBIDDEN, 'Scope validation failed') + kong.log.err("Scope validation failed") + return kong.response.error(ngx.HTTP_FORBIDDEN) end end ngx.log(ngx.DEBUG, "OidcHandler introspect succeeded, requested path: " .. ngx.var.request_uri) diff --git a/kong/plugins/oidc/session.lua b/kong/plugins/oidc/session.lua index a3b96468..e4943192 100644 --- a/kong/plugins/oidc/session.lua +++ b/kong/plugins/oidc/session.lua @@ -6,7 +6,8 @@ function M.configure(config) if config.session_secret then local decoded_session_secret = ngx.decode_base64(config.session_secret) if not decoded_session_secret then - utils.exit(ngx.HTTP_INTERNAL_SERVER_ERROR, 'Invalid plugin configuration, session secret could not be decoded') + kong.log.err("Invalid plugin configuration, session secret could not be decoded") + return kong.response.error(ngx.HTTP_INTERNAL_SERVER_ERROR) end ngx.var.session_secret = decoded_session_secret end diff --git a/kong/plugins/oidc/utils.lua b/kong/plugins/oidc/utils.lua index 272c1a1b..4e577db6 100644 --- a/kong/plugins/oidc/utils.lua +++ b/kong/plugins/oidc/utils.lua @@ -84,13 +84,6 @@ function M.get_options(config, ngx) } end -function M.exit(statusCode, message) - ngx.status = statusCode - kong.log.err(message) - ngx.exit(statusCode) -end - - -- Function set_consumer is derived from the following kong auth plugins: -- https://github.com/Kong/kong/blob/2.2.0/kong/plugins/ldap-auth/access.lua -- https://github.com/Kong/kong/blob/2.2.0/kong/plugins/oauth2/access.lua @@ -99,39 +92,36 @@ end local function set_consumer(consumer, credential) kong.client.authenticate(consumer, credential) - local set_header = kong.service.request.set_header - local clear_header = kong.service.request.clear_header - if consumer and consumer.id then - set_header(constants.HEADERS.CONSUMER_ID, consumer.id) + kong.service.request.set_header(constants.HEADERS.CONSUMER_ID, consumer.id) else - clear_header(constants.HEADERS.CONSUMER_ID) + kong.service.request.clear_header(constants.HEADERS.CONSUMER_ID) end if consumer and consumer.custom_id then - set_header(constants.HEADERS.CONSUMER_CUSTOM_ID, consumer.custom_id) + kong.service.request.set_header(constants.HEADERS.CONSUMER_CUSTOM_ID, consumer.custom_id) else - clear_header(constants.HEADERS.CONSUMER_CUSTOM_ID) + kong.service.request.clear_header(constants.HEADERS.CONSUMER_CUSTOM_ID) end if consumer and consumer.username then - set_header(constants.HEADERS.CONSUMER_USERNAME, consumer.username) + kong.service.request.set_header(constants.HEADERS.CONSUMER_USERNAME, consumer.username) else - clear_header(constants.HEADERS.CONSUMER_USERNAME) + kong.service.request.clear_header(constants.HEADERS.CONSUMER_USERNAME) end if credential and credential.sub then - set_header(constants.HEADERS.CREDENTIAL_IDENTIFIER, credential.sub) + kong.service.request.set_header(constants.HEADERS.CREDENTIAL_IDENTIFIER, credential.sub) else - clear_header(constants.HEADERS.CREDENTIAL_IDENTIFIER) + kong.service.request.clear_header(constants.HEADERS.CREDENTIAL_IDENTIFIER) end - clear_header(constants.HEADERS.CREDENTIAL_USERNAME) + kong.service.request.clear_header(constants.HEADERS.CREDENTIAL_USERNAME) if credential then - clear_header(constants.HEADERS.ANONYMOUS) + kong.service.request.clear_header(constants.HEADERS.ANONYMOUS) else - set_header(constants.HEADERS.ANONYMOUS, true) + kong.service.request.set_header(constants.HEADERS.ANONYMOUS, true) end end @@ -141,13 +131,13 @@ function M.injectAccessToken(accessToken, headerName, bearerToken) if (bearerToken) then token = formatAsBearerToken(token) end - ngx.req.set_header(headerName, token) + kong.service.request.set_header(headerName, token) end function M.injectIDToken(idToken, headerName) ngx.log(ngx.DEBUG, "Injecting " .. headerName) local tokenStr = cjson.encode(idToken) - ngx.req.set_header(headerName, ngx.encode_base64(tokenStr)) + kong.service.request.set_header(headerName, ngx.encode_base64(tokenStr)) end function M.setCredentials(user) @@ -160,7 +150,7 @@ end function M.injectUser(user, headerName) ngx.log(ngx.DEBUG, "Injecting " .. headerName) local userinfo = cjson.encode(user) - ngx.req.set_header(headerName, ngx.encode_base64(userinfo)) + kong.service.request.set_header(headerName, ngx.encode_base64(userinfo)) end function M.injectGroups(user, claim) diff --git a/test/unit/mockable_case.lua b/test/unit/mockable_case.lua index 78d0e9ad..613b7799 100644 --- a/test/unit/mockable_case.lua +++ b/test/unit/mockable_case.lua @@ -46,6 +46,11 @@ function MockableCase:setUp() set_header = function(...) end } }, + response = { + error = function(status) + ngx.status = status + end + }, log = { err = function(...) end }, diff --git a/test/unit/test_handler_mocking_openidc.lua b/test/unit/test_handler_mocking_openidc.lua index fad410b9..47ae6ec5 100644 --- a/test/unit/test_handler_mocking_openidc.lua +++ b/test/unit/test_handler_mocking_openidc.lua @@ -38,9 +38,7 @@ function TestHandler:test_authenticate_ok_with_userinfo() end local headers = {} - ngx.req.set_header = function(h, v) - headers[h] = v - end + kong.service.request.set_header = function(name, value) headers[name] = value end self.handler:access({userinfo_header_name = 'X-Userinfo'}) lu.assertTrue(self:log_contains("calling authenticate")) @@ -54,9 +52,7 @@ function TestHandler:test_authenticate_ok_with_no_accesstoken() end local headers = {} - ngx.req.set_header = function(h, v) - headers[h] = v - end + kong.service.request.set_header = function(name, value) headers[name] = value end self.handler:access({disable_id_token_header = "yes"}) lu.assertTrue(self:log_contains("calling authenticate")) @@ -65,28 +61,24 @@ end function TestHandler:test_authenticate_ok_with_accesstoken() self.module_resty.openidc.authenticate = function(opts) - return {id_token = { sub = "sub" } , access_token = "ACCESS_TOKEN"}, true + return {id_token = { sub = "sub" } , access_token = "ACCESS_TOKEN"}, false end local headers = {} - ngx.req.set_header = function(h, v) - headers[h] = v - end + kong.service.request.set_header = function(name, value) headers[name] = value end - self.handler:access({access_token_header_name = 'X-Access-Token', disable_id_token_header = "yes"}) + self.handler:access({access_token_header_name = 'X-Access-Token', disable_id_token_header = "yes"}) lu.assertTrue(self:log_contains("calling authenticate")) lu.assertEquals(headers['X-Access-Token'], "ACCESS_TOKEN") end function TestHandler:test_authenticate_ok_with_no_idtoken() self.module_resty.openidc.authenticate = function(opts) - return {}, true + return {}, false end local headers = {} - ngx.req.set_header = function(h, v) - headers[h] = v - end + kong.service.request.set_header = function(name, value) headers[name] = value end self.handler:access({}) lu.assertTrue(self:log_contains("calling authenticate")) @@ -95,7 +87,7 @@ end function TestHandler:test_authenticate_ok_with_idtoken() self.module_resty.openidc.authenticate = function(opts) - return {id_token = {sub = "sub"}}, true + return {id_token = {sub = "sub"}}, false end ngx.encode_base64 = function(x) @@ -103,9 +95,7 @@ function TestHandler:test_authenticate_ok_with_idtoken() end local headers = {} - ngx.req.set_header = function(h, v) - headers[h] = v - end + kong.service.request.set_header = function(name, value) headers[name] = value end self.handler:access({id_token_header_name = 'X-ID-Token'}) lu.assertTrue(self:log_contains("calling authenticate")) @@ -124,9 +114,9 @@ end function TestHandler:test_authenticate_nok_deny() self.module_resty.openidc.authenticate = function(opts) if opts.unauth_action == "deny" then - return nil, "unauthorized request" - end - return {}, true + return nil, "unauthorized request" + end + return {}, true end self.handler:access({unauth_action = "deny"}) @@ -163,9 +153,7 @@ function TestHandler:test_introspect_ok_with_userinfo() end local headers = {} - ngx.req.set_header = function(h, v) - headers[h] = v - end + kong.service.request.set_header = function(name, value) headers[name] = value end self.handler:access({introspection_endpoint = "x", userinfo_header_name = "X-Userinfo"}) lu.assertTrue(self:log_contains("introspect succeeded")) @@ -183,11 +171,9 @@ function TestHandler:test_bearer_only_with_good_token() end local headers = {} - ngx.req.set_header = function(h, v) - headers[h] = v - end - self.handler:access({introspection_endpoint = "x", bearer_only = "yes", realm = "kong", userinfo_header_name = "X-Userinfo"}) + kong.service.request.set_header = function(name, value) headers[name] = value end + self.handler:access({introspection_endpoint = "x", bearer_only = "yes", realm = "kong", userinfo_header_name = "X-Userinfo"}) lu.assertTrue(self:log_contains("introspect succeeded")) lu.assertEquals(headers['X-Userinfo'], "eyJzdWIiOiJzdWIifQ==") end diff --git a/test/unit/test_header_claims.lua b/test/unit/test_header_claims.lua index d7f03eca..07fea5f9 100644 --- a/test/unit/test_header_claims.lua +++ b/test/unit/test_header_claims.lua @@ -21,8 +21,7 @@ function TestHandler:test_header_add() self.module_resty.openidc.authenticate = function(opts) return { user = {sub = "sub", email = "ghost@localhost"}, id_token = { sub = "sub", aud = "aud123"} }, false end - local headers - headers = {} + local headers = {} kong.service.request.set_header = function(name, value) headers[name] = value end self.handler:access({ disable_id_token_header = "yes", disable_userinfo_header = "yes", diff --git a/test/unit/test_introspect.lua b/test/unit/test_introspect.lua index 31949334..062da8be 100644 --- a/test/unit/test_introspect.lua +++ b/test/unit/test_introspect.lua @@ -28,10 +28,8 @@ function TestIntrospect:test_access_token_exists() end local headers = {} - ngx.req.set_header = function(h, v) - headers[h] = v - end - + kong.service.request.set_header = function(name, value) headers[name] = value end + self.handler:access({introspection_endpoint = "x", userinfo_header_name = "X-Userinfo"}) lu.assertTrue(self:log_contains("introspect succeeded")) lu.assertEquals(headers['X-Userinfo'], "eyJzdWIiOiJzdWIifQ==") @@ -49,9 +47,7 @@ function TestIntrospect:test_no_authorization_header() ngx.req.get_headers = function() return {} end local headers = {} - ngx.req.set_header = function(h, v) - headers[h] = v - end + kong.service.request.set_header = function(name, value) headers[name] = value end self.handler:access({introspection_endpoint = "x", userinfo_header_name = "X-Userinfo"}) lu.assertFalse(self:log_contains(self.mocked_ngx.ERR)) From 4c6903f15d0d2303fce726dd709e1e513408c7d2 Mon Sep 17 00:00:00 2001 From: Cristian Chiru Date: Sun, 3 Apr 2022 12:36:53 +0300 Subject: [PATCH 14/14] Update and rename kong-oidc-1.2.4-3.rockspec to kong-oidc-1.2.4-4.rockspec --- kong-oidc-1.2.4-3.rockspec => kong-oidc-1.2.4-4.rockspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename kong-oidc-1.2.4-3.rockspec => kong-oidc-1.2.4-4.rockspec (98%) diff --git a/kong-oidc-1.2.4-3.rockspec b/kong-oidc-1.2.4-4.rockspec similarity index 98% rename from kong-oidc-1.2.4-3.rockspec rename to kong-oidc-1.2.4-4.rockspec index 3368503f..3c281cd8 100644 --- a/kong-oidc-1.2.4-3.rockspec +++ b/kong-oidc-1.2.4-4.rockspec @@ -1,5 +1,5 @@ package = "kong-oidc" -version = "1.2.4-3" +version = "1.2.4-4" source = { url = "git://github.com/revomatico/kong-oidc", tag = "master",