From 283621f2e70f85a113e0c60bd005e22bd4e586c4 Mon Sep 17 00:00:00 2001
From: Eloy Coto <eloy.coto@acalustra.com>
Date: Tue, 20 Apr 2021 12:43:32 +0200
Subject: [PATCH] OIDC: Fix discovery content-type issues

When JWK response was not application/json content_type, the keys cannot
be parsed and OIDC flow failed.

With this fix, if the application is returning `jwk-set+json`, something
valid that was not parsed as we should.[0]

[0] https://tools.ietf.org/html/rfc7517#section-8.5.1

Fix: THREESCALE-6913

Signed-off-by: Eloy Coto <eloy.coto@acalustra.com>
---
 CHANGELOG.md                         |  1 +
 gateway/src/resty/oidc/discovery.lua | 16 +++++++++++---
 spec/resty/oidc/discovery_spec.lua   | 31 +++++++++++++++++++++++++++-
 3 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 34131031e..be836a765 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Fixed hostname_rewrite incompatibility with Routing Policy [PR #1263](https://github.com/3scale/APIcast/pull/1263) [THREESCALE-6723](https://issues.redhat.com/browse/THREESCALE-6723)
 - Fixed issues with URI when using Routing Policy [PR #1245](https://github.com/3scale/APIcast/pull/1245) [THREESCALE-6410](https://issues.redhat.com/browse/THREESCALE-6410)
 - Fixed typo on TLS jsonschema [PR #1260](https://github.com/3scale/APIcast/pull/1260) [THREESCALE-6390](https://issues.redhat.com/browse/THREESCALE-6390)
+- Fixed issues on OIDC jwk discovery [PR #1268](https://github.com/3scale/APIcast/pull/1268) [THREESCALE-6913](https://issues.redhat.com/browse/THREESCALE-6913)
 
 
 ### Added
diff --git a/gateway/src/resty/oidc/discovery.lua b/gateway/src/resty/oidc/discovery.lua
index 8dc521449..a90fb4a7d 100644
--- a/gateway/src/resty/oidc/discovery.lua
+++ b/gateway/src/resty/oidc/discovery.lua
@@ -15,6 +15,11 @@ local oidc_log_level = ngx[string.upper(resty_env.value('APICAST_OIDC_LOG_LEVEL'
 
 local cache_max_size = 100
 
+local json_content_types = {
+  ["application/json"] = true,
+  ["application/jwk-set+json"] = true
+}
+
 local _M = {
   _TYPE = "oidc_discovery"
 }
@@ -32,7 +37,7 @@ local function mime_type(content_type)
 end
 
 local function decode_json(response)
-    if mime_type(response.headers.content_type) == 'application/json' then
+    if json_content_types[mime_type(response.headers.content_type)] then
         return cjson.decode(response.body)
     else
         return nil, 'not json'
@@ -111,7 +116,7 @@ function _M:openid_configuration(issuer)
 
     local config = decode_json(res)
     if not config then
-        ngx.log(oidc_log_level, 'invalid OIDC Provider, expected application/json got:  ', res.headers.content_type, ' body: ', res.body)
+        ngx.log(oidc_log_level, 'invalid OIDC Provider, cannot decode the message::  ', res.headers.content_type, ' body: ', res.body)
         return nil, 'invalid JSON'
     end
     return config
@@ -140,7 +145,12 @@ function _M:jwks(configuration)
     local res = http_client.get(jwks_uri)
 
     if res.status == 200 then
-        return jwk.convert_keys(decode_json(res))
+        local val, err = jwk.convert_keys(decode_json(res))
+        if err then
+          ngx.log(oidc_log_level, 'invalid JWKS config, cannot decode the message: ', res.headers.content_type, ', error:', err ,', body: ', res.body)
+          return nil, "Cannot convert keys"
+        end
+        return val
     else
         return nil, 'invalid response'
     end
diff --git a/spec/resty/oidc/discovery_spec.lua b/spec/resty/oidc/discovery_spec.lua
index 86760cf8c..a9e9a2913 100644
--- a/spec/resty/oidc/discovery_spec.lua
+++ b/spec/resty/oidc/discovery_spec.lua
@@ -140,6 +140,35 @@ UwIDAQAB
             } }, keys)
         end)
 
+        it('can process jwk-set+json content type ', function()
+            local config = { jwks_uri = 'https://idp.example.com/auth/realms/foo/jwks' }
+            test_backend
+                .expect{ url = config.jwks_uri }
+                .respond_with{
+                    status = 200,
+                    headers = { content_type = 'application/jwk-set+json;charset=UTF-8' },
+                    body = fixture('oidc', 'jwk', 'forgerock.json')
+                }
+
+            local keys = assert(discovery:jwks(config))
+
+            assert.same(cjson.decode(fixture('oidc', 'jwk', 'forgerock.apicast.json')),
+                    keys)
+        end)
+
+        it('cannot process text content type ', function()
+            local config = { jwks_uri = 'https://idp.example.com/auth/realms/foo/jwks' }
+            test_backend
+                .expect{ url = config.jwks_uri }
+                .respond_with{
+                    status = 200,
+                    headers = { content_type = 'application/text' },
+                    body = fixture('oidc', 'jwk', 'forgerock.json')
+                }
+
+            assert.returns_error('Cannot convert keys', discovery:jwks(config))
+        end)
+
         it('can process ForgeRock response', function()
             local config = { jwks_uri = 'https://idp.example.com/auth/realms/foo/jwks' }
             test_backend
@@ -180,7 +209,7 @@ UwIDAQAB
                     .expect{ url = config.jwks_uri }
                     .respond_with{ status = 200, body = '' }
 
-            assert.returns_error('not json', discovery:jwks(config))
+            assert.returns_error('Cannot convert keys', discovery:jwks(config))
         end)
     end)
 end)