feat(openid-connect): add jwt audience validator #11987
Conversation
dosubot
bot
added
size:XL
| }, | ||
| match_with_client_id = { | ||
| type = "boolean", | ||
| description = "audience must euqal to or includes client_id", |
There was a problem hiding this comment.
From the code, it can only be equals?
There was a problem hiding this comment.
Yes, if the match is true but not required, the correlation check with client_id is only performed if the aud value is not empty.
| return 403, core.json.encode(error_response) | ||
| end | ||
| elseif conf.client_id ~= audience_value then | ||
| core.log.error("OIDC introspection failed: ", |
There was a problem hiding this comment.
Compared to error, warn level seems more appropriate for these logs.
There was a problem hiding this comment.
Makes sense, but the rest of the code uses error, so I'm choosing to follow the precedent, should we break that "convention"?
We should consider the design of the configuration items more carefully, as it may not align with ergonomic best practices. I'd appreciate your feedback. From all the people who are concerned about this, not just the maintainers themselves. |
|
No feedback so far, should go ahead, review and merge it. |
| end | ||
| if core.table.try_read_attr(conf, "claim_validator", "audience", "match_with_client_id") | ||
| and audience_value ~= nil then | ||
| local error_response = { error = "mismatched audience" } |
There was a problem hiding this comment.
Can we directly use a json string and return it instead of using a lua table and calling json.encode?
There was a problem hiding this comment.
If so, should I need to set the Content-Type: application/json header manually? 🤔
apisix/plugins/openid-connect.lua
Outdated
| }, | ||
| }, | ||
| }, | ||
| default = {}, |
There was a problem hiding this comment.
can we remove this default value?
empty table {}, it seems a little weird
There was a problem hiding this comment.
This can be remove, but claim_validator will not appear in configurations that have populated the defaults, nor will configuration items in audience, and the populating of the defaults is not recursive.
However, after checking, the code logic correctly handles the case where these config items don't exist.
The line will be removed.
Description
Add JWT audience authentication to the OpenID Connect plugin, which allows:
Fixes #11968 #11059
One of the developers in #11059 mentioned that it is possible to use some of the APIs in
jwt-validatorsto implement JWT validation inlua-resty-openidc, but it doesn't work, that library only works with local verification that uses a public key, and not with the Introspection API. We have to implement the functionality directly in the plugin code to support it in both scenarios.To keep compatibility, these features are not turned on by default and it is up to you to decide if you want to turn them on. Although the OIDC spec requires this to be the default behavior.
Checklist