From a530a5af643cf53e1fa91431f9912f5e840735e9 Mon Sep 17 00:00:00 2001 From: David Ortiz Date: Thu, 13 Sep 2018 11:08:19 +0200 Subject: [PATCH 1/3] t/configuration-loading-boot-remote: test case where only some services use OIDC --- t/configuration-loading-boot-remote.t | 83 +++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/t/configuration-loading-boot-remote.t b/t/configuration-loading-boot-remote.t index 45dcf9768..36914b425 100644 --- a/t/configuration-loading-boot-remote.t +++ b/t/configuration-loading-boot-remote.t @@ -186,3 +186,86 @@ echo ' --- request GET /t --- exit_code: 200 + +=== TEST 5: load a config where only some of the services have an OIDC configuration +This is a regression test. APIcast crashed when loading a config where only +some of the services used OIDC. +The reason is that we created an array of OIDC configs with +size=number_of_services. Let's say we have 100 services and only the 50th has an +OIDC config. In this case, we created this Lua table: +{ [50] = oidc_config_here }. +The problem is that cjson raises an error when trying to convert a sparse array +like that into JSON. Using the default cjson configuration, the minimum number +of elements to reproduce the error is 11. So in this test, we create 11 services +and assign an OIDC config only to the last one. Check +https://www.kyne.com.au/~mark/software/lua-cjson-manual.html#encode_sparse_array +for more details. +Now we assign to _false_ the elements of the array that do not have an OIDC +config, so this test should not crash. +--- main_config +env THREESCALE_PORTAL_ENDPOINT=http://127.0.0.1:$TEST_NGINX_SERVER_PORT; +env APICAST_CONFIGURATION_LOADER=boot; +env THREESCALE_DEPLOYMENT_ENV=production; +env PATH; +--- http_config + lua_package_path "$TEST_NGINX_LUA_PATH"; +--- config +location = /t { + content_by_lua_block { + local loader = require('apicast.configuration_loader.remote_v2') + ngx.say(assert(loader:call())) + } +} + +location = /admin/api/services.json { + echo ' + { + "services":[ + { "service": { "id":1 } }, + { "service": { "id":2 } }, + { "service": { "id":3 } }, + { "service": { "id":4 } }, + { "service": { "id":5 } }, + { "service": { "id":6 } }, + { "service": { "id":7 } }, + { "service": { "id":8 } }, + { "service": { "id":9 } }, + { "service": { "id":10 } }, + { "service": { "id":11 } } + ] + }'; +} + +location = /issuer/endpoint/.well-known/openid-configuration { + content_by_lua_block { + local base = "http://" .. ngx.var.host .. ':' .. ngx.var.server_port + ngx.header.content_type = 'application/json;charset=utf-8' + ngx.say(require('cjson').encode { + issuer = 'https://example.com/auth/realms/apicast', + id_token_signing_alg_values_supported = { 'RS256' }, + jwks_uri = base .. '/jwks', + }) + } +} + +location = /jwks { + content_by_lua_block { + ngx.header.content_type = 'application/json;charset=utf-8' + ngx.say([[ + { "keys": [ + { "kty":"RSA","kid":"somekid", + "n":"sKXP3pwND3rkQ1gx9nMb4By7bmWnHYo2kAAsFD5xq0IDn26zv64tjmuNBHpI6BmkLPk8mIo0B1E8MkxdKZeozQ","e":"AQAB" } + ] } + ]]) + } +} + +location ~ /admin/api/services/([0-9]|10)/proxy/configs/production/latest.json { +echo '{ "proxy_config": { "content": { } } }'; +} +location = /admin/api/services/11/proxy/configs/production/latest.json { +echo '{ "proxy_config": { "content": { "proxy": { "oidc_issuer_endpoint": "http://127.0.0.1:$TEST_NGINX_SERVER_PORT/issuer/endpoint" } } } }'; +} +--- request +GET /t +--- exit_code: 200 From a9dbdd9dc355ca5974361e5c77135f1ee3e00744 Mon Sep 17 00:00:00 2001 From: David Ortiz Date: Thu, 13 Sep 2018 11:10:07 +0200 Subject: [PATCH 2/3] configuration_loader/remote_v2: avoid sparse arrays of OIDC configs APIcast crashes when loading a config where only some of the services use OIDC. The reason is that we created an array of OIDC configs with size=number_of_services. Let's say we have 100 services and only the 50th has an OIDC config. In this case, we created this Lua table: { [50] = oidc_config_here }. The problem is that cjson raises an error when trying to convert a sparse array like that into JSON. Check https://www.kyne.com.au/~mark/software/lua-cjson-manual.html#encode_sparse_array for more details. Assigning false instead of nil solves the issue. --- .../src/apicast/configuration_loader/remote_v2.lua | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/gateway/src/apicast/configuration_loader/remote_v2.lua b/gateway/src/apicast/configuration_loader/remote_v2.lua index 5b5303d88..ddb0063f7 100644 --- a/gateway/src/apicast/configuration_loader/remote_v2.lua +++ b/gateway/src/apicast/configuration_loader/remote_v2.lua @@ -125,9 +125,10 @@ function _M:index(host) local service = configuration.parse_service(proxy_config.content) local oidc = self:oidc_issuer_configuration(service) - if oidc then - config.oidc[i] = oidc - end + -- Assign false instead of nil to avoid sparse arrays. cjson raises an + -- error by default when converting sparse arrays. + config.oidc[i] = oidc or false + config.services[i] = original_proxy_config.content end @@ -183,7 +184,11 @@ function _M:call(environment) for i=1, #configs do configs.services[i] = configs[i].content - configs.oidc[i] = configs[i].oidc + + -- Assign false instead of nil to avoid sparse arrays. cjson raises an + -- error by default when converting sparse arrays. + configs.oidc[i] = configs[i].oidc or false + configs[i] = nil end From edb8ee86dcd39f750de39f8b90f10fd22c1abb80 Mon Sep 17 00:00:00 2001 From: David Ortiz Date: Thu, 13 Sep 2018 11:21:34 +0200 Subject: [PATCH 3/3] CHANGELOG: add fix for configs where only some of the services use OIDC --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7cb2197fe..899229cc1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Invalid SNI when connecting to 3scale backend over HTTPS [THREESCALE-1269](https://issues.jboss.org/browse/THREESCALE-1269) - Fix handling --pid and --signal on the CLI [PR #880](https://github.com/3scale/apicast/pull/880) - Some policies did not have access to the vars exposed when using Liquid (`uri`, `path`, etc.) [PR #891](https://github.com/3scale/apicast/pull/891) +- Fix error when loading certain configurations that use OIDC [PR #893](https://github.com/3scale/apicast/pull/893) ### Added