diff --git a/CHANGELOG.md b/CHANGELOG.md index 9666c1b26..c5e85fec0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Fixed - Correct FAPI header to `x-fapi-interaction-id` [PR #1557](https://github.com/3scale/APIcast/pull/1557) [THREESCALE-11957](https://issues.redhat.com/browse/THREESCALE-11957) +- Only validate oidc setting if authentication method is set to oidc [PR #1568](https://github.com/3scale/APIcast/pull/1568) [THREESCALE-11441](https://issues.redhat.com/browse/THREESCALE-11441) ### Added - Update APIcast schema manifest [PR #1550](https://github.com/3scale/APIcast/pull/1550) diff --git a/gateway/src/apicast/configuration_loader/oidc.lua b/gateway/src/apicast/configuration_loader/oidc.lua index 8241e7420..1997388b1 100644 --- a/gateway/src/apicast/configuration_loader/oidc.lua +++ b/gateway/src/apicast/configuration_loader/oidc.lua @@ -21,13 +21,22 @@ _M.discovery = require('resty.oidc.discovery').new() local function load_service(service) if not service or not service.proxy then return nil end - local result = _M.discovery:call(service.proxy.oidc_issuer_endpoint) + local proxy = service.proxy - if result and service.id then - result.service_id = service.id + -- Only fetch OIDC configuration if authentication method is set to 'oidc' + local authentication = proxy.authentication_method or service.backend_version + + if authentication and authentication == 'oidc' then + local result = _M.discovery:call(proxy.oidc_issuer_endpoint) + + if result and service.id then + result.service_id = service.id + end + + return result end - return result + return nil end function _M.call(...) diff --git a/gateway/src/apicast/configuration_loader/remote_v2.lua b/gateway/src/apicast/configuration_loader/remote_v2.lua index 5ca66b087..e36d57581 100644 --- a/gateway/src/apicast/configuration_loader/remote_v2.lua +++ b/gateway/src/apicast/configuration_loader/remote_v2.lua @@ -103,7 +103,7 @@ local function service_config_endpoint(portal_endpoint, service_id, env, version end local function get_oidc_issuer_endpoint(proxy_content) - return proxy_content.proxy and proxy_content.proxy.oidc_issuer_endpoint + return proxy_content.proxy and (proxy_content.proxy.authentication_method == "oidc") and proxy_content.proxy.oidc_issuer_endpoint end local function parse_proxy_configs(self, proxy_configs) diff --git a/spec/configuration_loader/oidc_spec.lua b/spec/configuration_loader/oidc_spec.lua index 887424603..ccf11f8f5 100644 --- a/spec/configuration_loader/oidc_spec.lua +++ b/spec/configuration_loader/oidc_spec.lua @@ -24,6 +24,17 @@ describe('OIDC Configuration loader', function() assert(loader.call(config)) end) + it('ignores config with oidc_issuer_endpoint but not oidc authentication mode', function() + local config = cjson.encode{ + services = { + { id = 21, proxy = { oidc_issuer_endpoint = 'https://user:pass@example.com' } }, + { id = 42 }, + } + } + + assert(loader.call(config)) + end) + it('forwards all parameters', function() assert.same({'{"oidc":[]}', 'one', 'two'}, { loader.call('{}', 'one', 'two')}) end) @@ -31,7 +42,7 @@ describe('OIDC Configuration loader', function() it('gets openid configuration', function() local config = { services = { - { id = 21, proxy = { oidc_issuer_endpoint = 'https://user:pass@example.com' } }, + { id = 21, proxy = { oidc_issuer_endpoint = 'https://user:pass@example.com', authentication_method = 'oidc' }}, } } @@ -58,7 +69,8 @@ describe('OIDC Configuration loader', function() { "id": 21, "proxy": { - "oidc_issuer_endpoint": "https://user:pass@example.com" + "oidc_issuer_endpoint": "https://user:pass@example.com", + "authentication_method": "oidc" } } ], @@ -97,5 +109,50 @@ describe('OIDC Configuration loader', function() loader.call(cjson.encode(config)) end) + + it('ignore openid configuration if authentication_method is not oidc', function() + local config = { + services = { + { id = 21, proxy = { oidc_issuer_endpoint = 'https://user:pass@example.com', authentication_method = '1' }}, + } + } + + test_backend + .expect{ url = "https://example.com/.well-known/openid-configuration" } + .respond_with{ + status = 200, + headers = { content_type = 'application/json' }, + body = [[{"jwks_uri":"http://example.com/jwks","issuer":"https://example.com"}]], + } + + test_backend + .expect{ url = "http://example.com/jwks" } + .respond_with{ + status = 200, + headers = { content_type = 'application/json' }, + body = [[{"keys":[]}]], + } + + local oidc = loader.call(cjson.encode(config)) + local expected_oidc = cjson.decode([[ + { + "services": [ + { + "id": 21, + "proxy": { + "oidc_issuer_endpoint": "https://user:pass@example.com", + "authentication_method": "1" + } + } + ], + "oidc": [ + { + "service_id": 21 + } + ] + } + ]]) + assert.same(expected_oidc, cjson.decode(oidc)) + end) end) end) diff --git a/spec/configuration_loader/remote_v2_spec.lua b/spec/configuration_loader/remote_v2_spec.lua index 0eee62dd1..a4e9a5806 100644 --- a/spec/configuration_loader/remote_v2_spec.lua +++ b/spec/configuration_loader/remote_v2_spec.lua @@ -251,7 +251,10 @@ describe('Configuration Remote Loader V2', function() environment = 'sandbox', content = { id = 42, backend_version = 1, - proxy = { oidc_issuer_endpoint = 'http://user:pass@idp.example.com/auth/realms/foo/' } + proxy = { + authentication_method= 'oidc', + oidc_issuer_endpoint = 'http://user:pass@idp.example.com/auth/realms/foo/' + } } } } @@ -311,6 +314,28 @@ UwIDAQAB } }, }, config.oidc) end) + + it('ignore OIDC configuration when authentication_method is not oidc', function() + test_backend.expect{ url = 'http://example.com/admin/api/services/42/proxy/configs/staging/latest.json' }. + respond_with{ status = 200, body = cjson.encode( + { + proxy_config = { + version = 2, + environment = 'sandbox', + content = { + id = 42, backend_version = 1, + proxy = { + authentication_method= '1', + oidc_issuer_endpoint = 'http://user:pass@idp.example.com/auth/realms/foo/' + } + } + } + } + ) } + + local config = assert(loader:config({ id = 42 }, 'staging', 'latest')) + assert.is_nil(config.oidc) + end) end) describe(':index_per_service', function() @@ -580,7 +605,10 @@ UwIDAQAB { proxy_config = { content = { - proxy = { oidc_issuer_endpoint = 'http://user:pass@idp.example.com/auth/realms/foo/' } + proxy = { + authentication_method= 'oidc', + oidc_issuer_endpoint = 'http://user:pass@idp.example.com/auth/realms/foo/' + } } } } @@ -730,7 +758,10 @@ UwIDAQAB content = { id = 2, backend_version = 1, - proxy = { oidc_issuer_endpoint = 'http://user:pass@idp.example.com/auth/realms/foo/' } + proxy = { + authentication_method= 'oidc', + oidc_issuer_endpoint = 'http://user:pass@idp.example.com/auth/realms/foo/' + } } } } @@ -920,7 +951,10 @@ UwIDAQAB content = { id = 2, backend_version = 1, - proxy = { oidc_issuer_endpoint = 'http://user:pass@idp.example.com/auth/realms/foo/' } + proxy = { + authentication_method= 'oidc', + oidc_issuer_endpoint = 'http://user:pass@idp.example.com/auth/realms/foo/' + } } } }