[THREESCALE-1289] Fix error when loading a config with only some services using OIDC

CHANGELOG.md

@@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Fix `APICAST_PROXY_HTTPS_PASSWORD_FILE` and `APICAST_PROXY_HTTPS_SESSION_REUSE` parameters for Mutual SSL [PR #927](https://github.com/3scale/apicast/pull/927)
 - The "allow" mode of the caching policy now accepts the request when it's authorization is not cached [PR #934](https://github.com/3scale/apicast/pull/934), [THREESCALE-1396](https://issues.jboss.org/browse/THREESCALE-1396)
 - When using SSL certs with path-based routing enabled, now APIcast falls backs to host-based routing instead of crashing [PR #938](https://github.com/3scale/apicast/pull/938), [THREESCALE-1430](https://issues.jboss.org/browse/THREESCALE-1430)
+- Fixed error that happened when loading certain configurations that use OIDC [PR #940](https://github.com/3scale/apicast/pull/940), [THREESCALE-1289](https://issues.jboss.org/browse/THREESCALE-1289)
 
 ### Added
 

gateway/src/apicast/configuration_loader/oidc.lua

@@ -33,7 +33,9 @@ function _M.call(...)
         local oidc = array(config.oidc)
 
         for i,service in ipairs(config.services or empty) do
-            oidc[i] = oidc[i] or load_service(service)
+            -- Assign false instead of nil to avoid sparse arrays. cjson raises
+            -- an error by default when converting sparse arrays.
+            oidc[i] = oidc[i] or load_service(service) or false
         end
 
         config.oidc = oidc

spec/configuration_loader/oidc_spec.lua

@@ -54,5 +54,25 @@ describe('OIDC Configuration loader', function()
 
       assert.same([[{"services":[{"id":21,"proxy":{"oidc_issuer_endpoint":"https:\/\/user:pass@example.com"}}],"oidc":[{"issuer":"https:\/\/example.com","config":{"jwks_uri":"http:\/\/example.com\/jwks","issuer":"https:\/\/example.com"},"keys":{}}]}]], oidc)
     end)
+
+    -- This is a regression test. cjson crashed when parsing a config where
+    -- only some of the services have OIDC enabled. In particular, it crashed
+    -- when it tried to convert into JSON a "sparse array":
+    -- https://www.kyne.com.au/~mark/software/lua-cjson-manual.html#encode_sparse_array
+    -- The easiest way to create a sparse array with the default cjson config
+    -- is to create a table that has 11 positions and only the last one is !=
+    -- false/nil.
+    it('works correctly when only some of the services have OIDC enabled', function()
+      local oidc = {}
+      for _=1, 10 do table.insert(oidc, false) end
+      oidc[11] = { issuer = "https://example.com" }
+
+      local services = {}
+      for i=1, 11 do table.insert(services, { id = i }) end
+
+      local config = { services = services, oidc = oidc }
+
+      loader.call(cjson.encode(config))
+    end)
   end)
 end)

t/configuration-loading-boot-remote.t

@@ -186,86 +186,3 @@ 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

t/configuration-loading-with-oidc.t

@@ -0,0 +1,221 @@
+use lib 't';
+use Test::APIcast::Blackbox 'no_plan';
+
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: 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.
+--- env eval
+(
+  'APICAST_CONFIGURATION_LOADER' => 'lazy',
+  'THREESCALE_PORTAL_ENDPOINT' => "http://test:$ENV{TEST_NGINX_SERVER_PORT}"
+)
+--- upstream env
+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": {
+      "id": 1,
+      "backend_version": 1,
+      "proxy": {
+        "api_backend": "http://test:$TEST_NGINX_SERVER_PORT/api/",
+        "backend": {
+          "endpoint": "http://test:$TEST_NGINX_SERVER_PORT"
+        },
+        "proxy_rules": [
+          {
+            "pattern": "/",
+            "http_method": "GET",
+            "metric_system_name": "test"
+          }
+        ]
+      }
+    }
+  }
+}
+';
+}
+
+location = /admin/api/services/11/proxy/configs/production/latest.json {
+echo '{ "proxy_config": { "content": { "proxy": { "oidc_issuer_endpoint": "http://test:$TEST_NGINX_SERVER_PORT/issuer/endpoint" } } } }';
+}
+
+location /transactions/authrep.xml {
+  content_by_lua_block {
+    ngx.exit(200)
+  }
+}
+
+location /api/ {
+  echo 'yay, api backend';
+}
+--- request
+GET /?user_key=uk
+--- error_code: 200
+--- response_body
+yay, api backend
+
+=== TEST 2: load a config where only some of the services have an OIDC configuration (with a portal endpoint with path)
+This test is almost the same as the previous one. The only difference is that,
+in this one, THREESCALE_PORTAL_ENDPOINT has a path.
+--- env eval
+(
+  'APICAST_CONFIGURATION_LOADER' => 'lazy',
+  'THREESCALE_PORTAL_ENDPOINT' => "http://test:$ENV{TEST_NGINX_SERVER_PORT}/config"
+)
+--- upstream env
+location = /config/production.json {
+echo '
+{
+  "proxy_configs": [
+    {
+      "proxy_config": {
+        "id": 1,
+        "content": {
+          "backend_version": 1,
+          "environment": "production",
+          "proxy": {
+            "hosts": [
+              "localhost"
+            ],
+            "api_backend": "http://test:$TEST_NGINX_SERVER_PORT/api/",
+            "backend": {
+              "endpoint": "http://test:$TEST_NGINX_SERVER_PORT"
+            },
+            "proxy_rules": [
+              { "pattern": "/", "http_method": "GET", "metric_system_name": "test" }
+            ]
+          }
+        }
+      }
+    },
+    { "proxy_config": { "content": {} } },
+    { "proxy_config": { "content": {} } },
+    { "proxy_config": { "content": {} } },
+    { "proxy_config": { "content": {} } },
+    { "proxy_config": { "content": {} } },
+    { "proxy_config": { "content": {} } },
+    { "proxy_config": { "content": {} } },
+    { "proxy_config": { "content": {} } },
+    { "proxy_config": { "content": {} } },
+    {
+      "proxy_config": {
+        "id": 2,
+        "content": {
+          "backend_version": "oidc",
+          "environment": "production",
+          "proxy": {
+            "oidc_issuer_endpoint": "http://test:$TEST_NGINX_SERVER_PORT/issuer/endpoint",
+            "backend": {
+              "endpoint": "http://test:$TEST_NGINX_SERVER_PORT"
+            },
+            "proxy_rules": [
+              { "pattern": "/", "http_method": "GET", "metric_system_name": "test" }
+            ]
+          }
+        }
+      }
+    }
+  ]
+}
+';
+}
+
+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 /transactions/authrep.xml {
+  content_by_lua_block {
+    ngx.exit(200)
+  }
+}
+
+location /api/ {
+  echo 'yay, api backend';
+}
+--- request
+GET /?user_key=uk
+--- error_code: 200
+--- response_body
+yay, api backend

t/management.t

@@ -164,7 +164,7 @@ include $TEST_NGINX_MANAGEMENT_CONFIG;
 --- request
 POST /boot
 --- response_body
-{"status":"ok","config":{"services":[{"id":42}],"oidc":{}}}
+{"status":"ok","config":{"services":[{"id":42}],"oidc":[false]}}
 --- error_code: 200
 --- udp_listen random_port env chomp
 $TEST_NGINX_RANDOM_PORT
@@ -193,8 +193,8 @@ location = /test {
 --- request
 POST /test
 --- response_body
-{"status":"ok","config":{"services":[{"id":42}],"oidc":{}}}
-{"status":"ok","config":{"services":[{"id":42}],"oidc":{}}}
+{"status":"ok","config":{"services":[{"id":42}],"oidc":[false]}}
+{"status":"ok","config":{"services":[{"id":42}],"oidc":[false]}}
 --- error_code: 200
 --- udp_listen random_port env chomp
 $TEST_NGINX_RANDOM_PORT