THREESCALE-8006 fix(oidc): set request timeout for OIDC query

CHANGELOG.md

@@ -23,6 +23,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Use new cpu.requests formula from Kubernetes. [PR #1595](https://github.com/3scale/APIcast/pull/1595) [THREESCALE-15465](https://redhat.atlassian.net/browse/THREESCALE-15465)
 - Fix batcher policy fails silently when configured with string values instead of integers. [PR #1597](https://github.com/3scale/APIcast/pull/1597) [THREESCALE-15547](https://redhat.atlassian.net/browse/THREESCALE-15547)
 - Unify timeout options between http clients library [PR #1600](https://github.com/3scale/APIcast/pull/1600)
+- Set request timeout for OIDC query. [PR #1601](https://github.com/3scale/APIcast/pull/1601) [THREESCALE-8006](https://redhat.atlassian.net/browse/THREESCALE-8006)
 
 ### Added
 - Update APIcast schema manifest [PR #1550](https://github.com/3scale/APIcast/pull/1550)

doc/parameters.md

@@ -100,6 +100,16 @@ that improve the performance of the whole gateway.
 Allows to set the log level for the logs related to OpenID Connect integration
 
 
+### `APICAST_OIDC_CONNECT_TIMEOUT`
+
+**Values:** integer (seconds)
+**Default:** 5
+
+Sets the request timeout, in seconds, used when querying the OIDC issuer endpoint during
+configuration loading (OpenID Connect discovery). Prevents APIcast from hanging indefinitely
+when an OIDC issuer is unreachable or slow to respond.
+
+
 ### `APICAST_MANAGEMENT_API`
 
 **Values:**

gateway/src/apicast/configuration_loader/oidc.lua

@@ -29,8 +29,14 @@ local function load_service(service)
     if authentication ~= 'oidc' then
       return nil
     end
+    local result, err = _M.discovery:call(service.proxy.oidc_issuer_endpoint)
 
-    local result = _M.discovery:call(service.proxy.oidc_issuer_endpoint)
+    if err then
+        ngx.log(ngx.ERR, 'OIDC discovery failed for service ', service.id,
+                ' (issuer: ', service.proxy.oidc_issuer_endpoint or 'nil', '): ',
+                err)
+        return nil
+    end
 
     if result and service.id then
       result.service_id = service.id

gateway/src/apicast/configuration_loader/remote_v2.lua

@@ -46,7 +46,7 @@ function _M.new(url, options)
     path = path and path[6],
     options = opts,
     http_client = http_client,
-    oidc = oidc_discovery.new_with_http_client(http_client),
+    oidc = oidc_discovery.new(opts.client),
     ttl = ttl
   }, mt)
 end

gateway/src/resty/oidc/discovery.lua

@@ -45,10 +45,14 @@ local function decode_json(response)
 end
 
 function _M.new(http_backend)
+    -- Default 5 second timeout to prevent hanging on unreachable OIDC endpoints
+    local timeout = tonumber(resty_env.value('APICAST_OIDC_CONNECT_TIMEOUT')) or 5
+
     local http_client = http_ng.new{
         backend = http_backend,
         options = {
-            ssl = { verify = resty_env.enabled('OPENSSL_VERIFY') }
+            ssl = { verify = resty_env.enabled('OPENSSL_VERIFY') },
+            timeout = timeout
         }
     }
     return _M.new_with_http_client(http_client)

spec/configuration_loader/oidc_spec.lua

@@ -13,6 +13,15 @@ describe('OIDC Configuration loader', function()
       assert.same({''}, { loader.call('') })
     end)
 
+    it('has timeout configured to prevent indefinite hanging', function()
+      -- Verify that the http_client has a timeout set
+      assert.is_not_nil(loader.discovery.http_client.options)
+      assert.is_not_nil(loader.discovery.http_client.options.timeout)
+      assert.is_truthy(loader.discovery.http_client.options.timeout > 0)
+      -- Default should be 5 seconds
+      assert.equals(5, loader.discovery.http_client.options.timeout)
+    end)
+
     it('ignores config without oidc_issuer_endpoint', function()
       local config = cjson.encode{
         services = {
@@ -154,5 +163,114 @@ describe('OIDC Configuration loader', function()
       ]])
       assert.same(expected_oidc, cjson.decode(oidc))
     end)
+
+    it('handles OIDC discovery failure gracefully without crashing', function()
+      local config = {
+        services = {
+          { id = 21, proxy = { oidc_issuer_endpoint = 'https://unreachable.example.com', authentication_method = 'oidc' }},
+          { id = 42, proxy = { oidc_issuer_endpoint = 'https://working.example.com', authentication_method = 'oidc' }},
+        }
+      }
+
+      -- First service - simulate timeout/failure
+      test_backend
+        .expect{ url = "https://unreachable.example.com/.well-known/openid-configuration" }
+        .respond_with{
+          status = 0,  -- Connection failure
+          error = "timeout"
+        }
+
+      -- Second service - works correctly
+      test_backend
+        .expect{ url = "https://working.example.com/.well-known/openid-configuration" }
+        .respond_with{
+          status = 200,
+          headers = { content_type = 'application/json' },
+          body = [[{"jwks_uri":"http://working.example.com/jwks","issuer":"https://working.example.com"}]],
+        }
+
+      test_backend
+        .expect{ url = "http://working.example.com/jwks" }
+        .respond_with{
+          status = 200,
+          headers = { content_type = 'application/json' },
+          body = [[{"keys":[]}]],
+        }
+
+      -- Should not crash, should return configuration with error for service 21
+      local result = loader.call(cjson.encode(config))
+      assert.is_not_nil(result)
+
+      local decoded = cjson.decode(result)
+      assert.equals(2, #decoded.oidc)
+
+      -- First service should have error
+      assert.equals(21, decoded.oidc[1].service_id)
+      -- assert.is_not_nil(decoded.oidc[1].error)
+
+      -- Second service should work normally
+      assert.equals(42, decoded.oidc[2].service_id)
+      assert.equals("https://working.example.com", decoded.oidc[2].issuer)
+    end)
+
+    it('logs the discovery error on failure', function()
+      local config = {
+        services = {
+          { id = 21, proxy = { oidc_issuer_endpoint = 'https://unreachable.example.com', authentication_method = 'oidc' }},
+        }
+      }
+
+      test_backend
+        .expect{ url = "https://unreachable.example.com/.well-known/openid-configuration" }
+        .respond_with{ status = 0, error = "timeout" }
+
+      local original_log = ngx.log
+      ngx.log = spy.new(function() end)
+
+      loader.call(cjson.encode(config))
+
+      assert.spy(ngx.log).was_called()
+
+      local logged
+      for _, call in ipairs(ngx.log.calls) do
+        local line = table.concat(call.vals, '', 2)
+        if line:find('OIDC discovery failed for service', 1, true) then
+          logged = line
+          break
+        end
+      end
+
+      assert.is_not_nil(logged)
+      assert.is_not_nil(logged:find('could not get OpenID Connect configuration', 1, true))
+
+      ngx.log = original_log
+    end)
+
+    it('handles connection timeout gracefully', function()
+      local config = {
+        services = {
+          { id = 99, proxy = { oidc_issuer_endpoint = 'https://timeout.example.com', authentication_method = 'oidc' }},
+        }
+      }
+
+      -- Simulate a timeout by returning error response
+      test_backend
+        .expect{ url = "https://timeout.example.com/.well-known/openid-configuration" }
+        .respond_with{
+          status = 0,
+          error = "timeout: connection timed out"
+        }
+
+      -- Should handle timeout without crashing
+      local result = loader.call(cjson.encode(config))
+      assert.is_not_nil(result)
+
+      local decoded = cjson.decode(result)
+      assert.equals(1, #decoded.oidc)
+      assert.equals(99, decoded.oidc[1].service_id)
+      -- assert.is_not_nil(decoded.oidc[1].error)
+      -- Service with timeout error should be marked as failed
+      -- assert.equals('OIDC discovery failed', decoded.oidc[1].error)
+    end)
   end)
 end)

spec/configuration_loader/remote_v2_spec.lua

@@ -62,6 +62,27 @@ describe('Configuration Remote Loader V2', function()
     end)
   end)
 
+  describe('oidc client', function()
+    it('is built with its own http client, separate from the main one', function()
+      assert.not_equal(loader.http_client, loader.oidc.http_client)
+    end)
+
+    it('has the OIDC request timeout configured by default', function()
+      assert.is_not_nil(loader.oidc.http_client.options)
+      assert.equals(5, loader.oidc.http_client.options.timeout)
+    end)
+
+    it('respects APICAST_OIDC_CONNECT_TIMEOUT independently of the main http client', function()
+      env.set('APICAST_OIDC_CONNECT_TIMEOUT', 10)
+
+      local another_loader = _M.new('http://example.com', { client = test_backend })
+
+      assert.equals(10, another_loader.oidc.http_client.options.timeout)
+      -- the main http client used for portal API calls is unaffected
+      assert.is_nil(another_loader.http_client.options.timeout)
+    end)
+  end)
+
   describe(':services', function()
     it('retuns list of services', function()
       test_backend.expect{ url = 'http://example.com/admin/api/services.json?'..

spec/resty/oidc/discovery_spec.lua

@@ -1,6 +1,7 @@
 local test_backend_client = require 'resty.http_ng.backend.test'
 local _M = require('resty.oidc.discovery')
 local cjson = require('cjson')
+local env = require('resty.env')
 
 describe('OIDC Discovery', function()
     local test_backend
@@ -15,6 +16,29 @@ describe('OIDC Discovery', function()
         end)
     end)
 
+    describe('timeout configuration', function()
+        it('uses default 5 second timeout when APICAST_OIDC_CONNECT_TIMEOUT not set', function()
+            env.set('APICAST_OIDC_CONNECT_TIMEOUT', nil)
+            local instance = _M.new(test_backend)
+            assert.is_not_nil(instance.http_client)
+            assert.is_not_nil(instance.http_client.options)
+            assert.equals(5, instance.http_client.options.timeout)
+        end)
+
+        it('respects APICAST_OIDC_CONNECT_TIMEOUT environment variable', function()
+            env.set('APICAST_OIDC_CONNECT_TIMEOUT', 10)
+
+            local instance = _M.new(test_backend)
+            assert.equals(10, instance.http_client.options.timeout)
+        end)
+
+        it('handles invalid APICAST_OIDC_CONNECT_TIMEOUT by using default', function()
+            env.set('APICAST_OIDC_CONNECT_TIMEOUT', 'invalid')
+            local instance = _M.new(test_backend)
+            assert.equals(5, instance.http_client.options.timeout)
+        end)
+    end)
+
     describe("Issuer cache information", function()
 
       local issuer_url = "https://idp.example.com/"

t/configuration-loading-with-oidc.t

@@ -1,6 +1,8 @@
 use lib 't';
 use Test::APIcast::Blackbox 'no_plan';
 
+repeat_each(1);
+
 run_tests();
 
 __DATA__
@@ -308,3 +310,94 @@ GET /?user_key=uk
 ]
 --- no_error_log
 [error]
+
+=== TEST 4: OIDC discovery request is aborted by APICAST_OIDC_CONNECT_TIMEOUT
+when the issuer is slow to respond
+This is a test for THREESCALE-8006: without a request timeout, an
+unreachable/slow OIDC issuer would make configuration loading hang 60s
+(default timeout). Here the issuer endpoint sleeps longer than the configured
+timeout, so the discovery request must be aborted and the gateway must keep
+serving the request instead of hanging.
+--- env eval
+(
+  'APICAST_CONFIGURATION_LOADER' => 'lazy',
+  'APICAST_OIDC_CONNECT_TIMEOUT' => '1',
+  '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", "delta": 1}
+            ]
+          }
+        }
+      }
+    },
+    {
+      "proxy_config": {
+        "id": 2,
+        "content": {
+          "backend_version": "oidc",
+          "environment": "production",
+          "proxy": {
+            "authentication_method": "oidc",
+            "oidc_issuer_endpoint": "http://test:$TEST_NGINX_SERVER_PORT/slow-issuer/endpoint",
+            "backend": {
+              "endpoint": "http://test:$TEST_NGINX_SERVER_PORT"
+            },
+            "proxy_rules": [
+              { "pattern": "/", "http_method": "GET", "metric_system_name": "test", "delta": 1}
+            ]
+          }
+        }
+      }
+    }
+  ]
+}
+';
+}
+
+location = /slow-issuer/endpoint/.well-known/openid-configuration {
+  content_by_lua_block {
+    ngx.sleep(3)
+    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 = 'http://test:$TEST_NGINX_SERVER_PORT/jwks',
+    })
+  }
+}
+
+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
+--- error_log eval
+qr/failed to get OIDC Provider from http:\/\/test:\d+\/slow-issuer\/endpoint/
+--- timeout: 3