nic-6443 commented on code in PR #13649:
URL: https://github.com/apache/apisix/pull/13649#discussion_r3717586009
##########
apisix/plugins/openid-connect.lua:
##########
@@ -58,6 +58,25 @@ local function build_session_opts(session_conf)
end
+local function flatten_openidc_options(conf)
Review Comment:
The flat lua-resty-openidc option names need to be rejected here or in
`check_schema`. The root schema accepts unknown properties, so `use_dpop`,
`dpop_private_key`, and `dpop_public_jwk` currently pass validation. I
reproduced this through the Admin API: `dpop_private_key` was stored verbatim
in etcd, and because `conf.dpop` is absent this function leaves all three flat
fields intact, so 1.9.0 activates DPoP with the plaintext key. This also
bypasses the nested public-JWK checks; the equivalent flat PAR fields bypass
`par.endpoint` validation. Please make the flat PAR/DPoP names invalid input,
or otherwise guarantee they cannot bypass validation and encryption, and add an
Admin API regression.
##########
apisix/plugins/openid-connect.lua:
##########
@@ -304,6 +323,70 @@ local schema = {
type = "boolean",
default = false
},
+ par = {
+ description = "Pushed Authorization Requests (PAR) configuration.",
+ type = "object",
+ properties = {
+ enabled = {
+ description = "When true, use Pushed Authorization
Requests (PAR).",
+ type = "boolean",
+ default = false,
+ },
+ endpoint = {
+ description = "URL of the Pushed Authorization Requests
endpoint.",
+ type = "string",
+ },
+ endpoint_auth_method = {
+ description = "Authentication method for the PAR
endpoint.",
+ type = "string",
+ },
+ },
+ additionalProperties = false,
+ },
+ dpop = {
+ description = "Demonstrating Proof-of-Possession (DPoP)
configuration.",
+ type = "object",
+ properties = {
+ enabled = {
+ description = "When true, use DPoP proof JWTs.",
+ type = "boolean",
+ default = false,
+ },
+ signing_alg = {
+ description = "DPoP proof JWT signing algorithm.",
+ type = "string",
+ enum = {"ES256", "RS256", "PS256"},
+ default = "ES256",
+ },
+ private_key = {
+ description = "Private key used to sign DPoP proof JWTs.",
+ type = "string",
+ },
+ public_jwk = {
+ description = "Public JWK matching dpop.private_key.",
+ type = "object",
Review Comment:
This still accepts public JWKs that lua-resty-openidc cannot use. For
example, `dpop = {enabled = true, private_key = "...", public_jwk = {kty =
"RSA"}}` passes `check_schema`, but the first authorization request returns 500
with `opts.dpop_public_jwk with kty "RSA" must contain e and n`. Please
validate the `kty`-specific public members here (`e`/`n` for RSA, `crv`/`x`/`y`
for EC); the signing algorithm should also be compatible with that key type so
the default `ES256` cannot be paired with an RSA JWK.
##########
apisix/plugins/openid-connect.lua:
##########
@@ -383,6 +466,22 @@ local schema = {
type = "integer",
default = 60
},
+ -- resty.jwt signs the client assertion and raises an uncaught Lua
+ -- error for an algorithm it cannot handle, so an unconstrained value
+ -- would surface as a 500 per request instead of a rejected config.
+ -- lua-resty-openidc depends on lua-resty-jwt, which supports more
+ -- algorithms, but it installs the same resty/jwt.lua as the
+ -- api7-lua-resty-jwt this rockspec pins and luarocks leaves the
+ -- pinned one in place either way, so the effective set is this one.
+ client_jwt_assertion_alg = {
+ description = "Signing algorithm for the client assertion JWT.",
+ type = "string",
+ enum = {"HS256", "HS512", "RS256", "RS512", "ES256", "ES512"}
Review Comment:
The enum still admits combinations that lua-resty-openidc rejects before
making the endpoint request. Both `private_key_jwt` with `HS256` and
`client_secret_jwt` with `RS256` pass `check_schema`; the library then returns
the cannot-use-symmetric / cannot-use-asymmetric errors, and the authorization
flow surfaces that as a 500. Please validate the algorithm family against every
JWT-auth endpoint selection (token, introspection, and PAR). A config that
selects both JWT auth families for different endpoints also needs rejection
because this is one global algorithm.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
