feat: add "valid_issuers" field in openidc plugin #12002
Conversation
dosubot
bot
added
the
size:L
Added. Please review again. |
|
@nic-chen Okay I think the last test can be done without the mock server. I have removed that and now the token verification happens as well. You can review |
apisix/plugins/openid-connect.lua
Outdated
| if valid_issuers then | ||
| opts.valid_issuers = valid_issuers | ||
| end | ||
| end | ||
| if conf.public_key or conf.use_jwks then |
There was a problem hiding this comment.
If public_key is specified, it will not verify issuers. Are you sure this is expected?
There was a problem hiding this comment.
you are right, we should verify issuer when use public_key too.
| valid_issuers = { | ||
| description = [[Whitelist the vetted issuers of the jwt. | ||
| When not passed by the user, the issuer returned by discovery endpoint will be used. | ||
| In case both are missing, the issuer will not be validated.]], | ||
| type = "array", | ||
| items = { | ||
| type = "string" | ||
| } | ||
| }, |
There was a problem hiding this comment.
This has already had another implementation that was approved and we have to decide which one is better.
There was a problem hiding this comment.
Your PR adds audience validation and my PR adds issuer validation.
Audience validation matches "aud" in the JWT with "client_id"
Issuer validation matches "issuer" in the JWT with discovery.issuer/or passed by user.
If the problem that I am trying to solve is this "I want to reject a request because customer passed a JWT with unrecognised issuer."
There was a problem hiding this comment.
I certainly know what I did, and know what you did, and I mentioned that we have to choose one of the better configuration schema designs, while accommodating design that are similar in use but so different is unacceptable.
Specifically, if they are tiled, then this and all subsequent validator configuration items need to be tiled, otherwise it's going to be structured like that PR #11987.
That's exactly why that PR hasn't merged yet, it's waiting for some feedback.
Description
Adds a field
valid_issuerwhen jwks is used to verify the issuer of jwt.Whitelist the vetted issuers of the jwt.
When not passed by the user, the issuer returned by discovery endpoint will be used.
In case both are missing, the issuer will not be validated.
Checklist