[OIDC] support JWK discovery through OIDC Discovery process #850
Conversation
mikz
changed the title
mikz
force-pushed
the
oidc-jwk
branch
3 times, most recently
from
0dfafc4 to
46eb8f4
Compare
![codeclimate[bot]](https://avatars.githubusercontent.com/in/9403?s=60&v=4){kind=small}
| formatted_key = formatted_key..string.sub(key, i, i+63).."\n" | ||
| end | ||
| formatted_key = formatted_key.."-----END PUBLIC KEY-----" | ||
| return formatted_key |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
gateway/src/resty/oidc/jwk.lua
Outdated
| if not res then return end | ||
| local keys = tab_new(0, #res.keys) | ||
|
|
||
| for i,jwk in ipairs(res.keys) do |
| .respond_with{ | ||
| status = 200, | ||
| headers = { content_type = 'application/json' }, | ||
| body = fixture('oidc', 'jwk', 'forgerock.json') |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
|
||
| local keys = assert(discovery:jwks(config)) | ||
|
|
||
| assert.same(cjson.decode(fixture('oidc', 'jwk', 'forgerock.apicast.json')), |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
mikz
force-pushed
the
oidc-jwk
branch
4 times, most recently
from
fd95997 to
21bfa66
Compare
mikz
changed the title
|
@3scale/product this implements JWK originally planned for 3.4 release. Would be good to know to which JIRA to link it in the Changelog. |
| end) | ||
|
|
||
| describe(':openid_configuration(endpoint)', function() | ||
| it('loads configuration from the discovery endpoint', function() |
There was a problem hiding this comment.
We should also test the error cases
| } }, keys) | ||
| end) | ||
|
|
||
| it('can process ForgeRock response', function() |
There was a problem hiding this comment.
Is Forgerock different somehow or just an example?
There was a problem hiding this comment.
It has some keys with ciphers we don't support and possibly some properties can be different.
I wanted to keep it "ForgeRock" so it is obvious we support ForgeRock and it was copied from ForgeRock.
Over time we can add tests for edge cases and describe them like "does not crash on unknown ciphers". But for now I wanted to say this is ForgeRock and it works.
gateway/src/resty/oidc/discovery.lua
Outdated
| return config | ||
| end | ||
|
|
||
| function _M:jwks(configuration) |
There was a problem hiding this comment.
Maybe add a comment to make it clear that apart from extracting keys this method converts them to the PEM format?
mikz
force-pushed
the
oidc-jwk
branch
2 times, most recently
from
48bbee2 to
a37ba95
Compare
| before_each(function() test_backend = test_backend_client.new() end) | ||
| before_each(function() loader.discovery.http_client.backend = test_backend end) | ||
|
|
||
| it('ignores config', function() |
Uses OpenSSL via FFI to convert JWK to PEM formatted certificate. Currently only supports RSA.
Load configuration via OIDC Discovery endpoint.
This loader can be used to decorate existing loaders to add OIDC information.
* Use JWK instead of "public_key" property * All configuration loaders get OIDC configuration (including config file, fixes #654)
Load all JWK serialized keys offered by the OIDC Discovery endpoint.
Only RSA is supported at the moment.
This is internal change - how we load signing keys from the IDP. Previously we used Keycloak/RHSSO specific method: loading "public_key" from the issuer endpoint. That is not compatible with OIDC spec, so this PR introduces loading JWK keys from the "jwks_uri" offered by the OIDC Discovery.
It uses OpenSSL via FFI, so there can be some potential memory access issues leading to segfaults/memory leaks. However, I run out unit test suite 10000 times just to verify it does not crash and the memory usage does not increase.
This also unifies how we load OIDC configuration and improves its internal format. OIDC configuration is loaded even when using JSON configuration file (fixes #654), but only when not already provided.
JSON configurations with "oidc" configuration are no longer valid, as the internal structure changed.
TODO