From 46f510d9bfcd5ad0586fd1fcca92a2136ed6056d Mon Sep 17 00:00:00 2001 From: An Tran Date: Mon, 24 Aug 2026 01:46:04 +1000 Subject: [PATCH] fix(oidc): set request timeout for OIDC query Set the connection timeout to 5s to prevent request hanging idenfinetely when an OIDC issuer is unreachable or slow to respond. --- CHANGELOG.md | 1 + doc/parameters.md | 10 +++ .../src/apicast/configuration_loader/oidc.lua | 8 +- gateway/src/resty/oidc/discovery.lua | 6 +- spec/configuration_loader/oidc_spec.lua | 85 +++++++++++++++++++ spec/resty/oidc/discovery_spec.lua | 24 ++++++ 6 files changed, 132 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 433a0e2df..9ab58dacc 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/doc/parameters.md b/doc/parameters.md index 852854361..59702471f 100644 --- a/doc/parameters.md +++ b/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:** diff --git a/gateway/src/apicast/configuration_loader/oidc.lua b/gateway/src/apicast/configuration_loader/oidc.lua index 28ea9389c..4b2a28d48 100644 --- a/gateway/src/apicast/configuration_loader/oidc.lua +++ b/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', '): ', + result) + return nil + end if result and service.id then result.service_id = service.id diff --git a/gateway/src/resty/oidc/discovery.lua b/gateway/src/resty/oidc/discovery.lua index a90fb4a7d..f0406ba62 100644 --- a/gateway/src/resty/oidc/discovery.lua +++ b/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) diff --git a/spec/configuration_loader/oidc_spec.lua b/spec/configuration_loader/oidc_spec.lua index ccf11f8f5..f08265e8f 100644 --- a/spec/configuration_loader/oidc_spec.lua +++ b/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,81 @@ 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('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) diff --git a/spec/resty/oidc/discovery_spec.lua b/spec/resty/oidc/discovery_spec.lua index 9d8f0fd81..12c1934e1 100644 --- a/spec/resty/oidc/discovery_spec.lua +++ b/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/"