AlinsRan commented on code in PR #13649:
URL: https://github.com/apache/apisix/pull/13649#discussion_r3733738639
##########
apisix/plugins/openid-connect.lua:
##########
@@ -488,8 +635,168 @@ local _M = {
name = plugin_name,
schema = schema,
_build_session_opts = build_session_opts,
+ _flatten_openidc_options = flatten_openidc_options,
}
+-- lua-resty-openidc rejects a JWK that lacks the members its key type needs,
+-- but only once the first authorization request builds the thumbprint, which
+-- surfaces as a 500 per request instead of a rejected configuration.
+local function check_dpop_key(dpop)
+ -- lua-resty-openidc reads none of this while use_dpop is false, so a
+ -- configuration that stages the key material before turning DPoP on is
+ -- valid and must not be rejected
+ if not (dpop and dpop.enabled) then
+ return true
+ end
+
+ local jwk = dpop.public_jwk
+ if not jwk then
+ return true
+ end
+
+ local required = dpop_jwk_required_members[jwk.kty]
+ if not required then
+ return false, "property \"dpop.public_jwk\" validation failed: kty \""
+ .. tostring(jwk.kty) .. "\" is not supported"
+ end
+
+ for _, member in ipairs(required) do
+ if jwk[member] == nil then
+ return false, "property \"dpop.public_jwk\" validation failed: kty
\""
+ .. jwk.kty .. "\" requires " .. concat(required, ",
")
+ end
+ -- the members go into the RFC 7638 thumbprint verbatim, so a
non-string
+ -- would be encoded as itself and produce a thumbprint no OP can match
+ if type(jwk[member]) ~= "string" or jwk[member] == "" then
+ return false, "property \"dpop.public_jwk\" validation failed: \""
+ .. member .. "\" must be a non-empty string"
+ end
+ end
+
+ local expected = dpop_alg_key_type[dpop.signing_alg]
+ if expected then
+ if expected.kty ~= jwk.kty then
+ return false, "property \"dpop.signing_alg\" \"" ..
dpop.signing_alg
+ .. "\" requires an " .. expected.kty .. "
\"dpop.public_jwk\""
+ end
+ if expected.crv and jwk.crv ~= expected.crv then
+ return false, "property \"dpop.signing_alg\" \"" ..
dpop.signing_alg
+ .. "\" requires \"dpop.public_jwk\" crv \"" ..
expected.crv
+ .. "\", got \"" .. tostring(jwk.crv) .. "\""
+ end
+
+ -- the proof is signed with the private key, not the JWK, so a matching
+ -- JWK says nothing about it; openidc_dpop_sign fails per request on a
+ -- key it cannot load or cannot use with the algorithm's padding
+ local private_key = dpop.private_key
+ if private_key and not secret.is_secret_ref(private_key) then
+ local key, err = pkey.new(private_key)
+ if not key then
+ return false, "property \"dpop.private_key\" is not a valid
key: "
+ .. tostring(err)
+ end
+ local key_type = key:get_key_type()
+ key_type = type(key_type) == "table" and key_type.sn or key_type
+ if key_type ~= dpop_openssl_key_type[expected.kty] then
+ return false, "property \"dpop.private_key\" is not an "
+ .. expected.kty .. " key, which
\"dpop.signing_alg\" \""
+ .. dpop.signing_alg .. "\" requires"
+ end
+ end
+ end
+
+ return true
+end
+
+
+-- The token endpoint only logs and falls back when its auth method cannot be
+-- used, but the PAR request fails outright (openidc.lua:547), which surfaces
+-- as a 500. PAR uses its own method when set and token_endpoint_auth_method
+-- otherwise, so validate whichever one it will actually use.
+local function check_par_auth_method(conf)
+ if not (conf.par and conf.par.enabled) then
+ return true
+ end
+
+ local method = conf.par.endpoint_auth_method
+ local source = "par.endpoint_auth_method"
+ if not method then
+ method = conf.token_endpoint_auth_method
+ source = "token_endpoint_auth_method"
+ end
+ if not method then
+ return true
+ end
+
+ local credential = token_auth_method_credential[method]
+ if credential == nil then
+ return false, "property \"" .. source .. "\" \"" .. method
+ .. "\" is not supported when \"par.enabled\" is true"
+ end
+ if credential and not conf[credential] then
+ return false, "property \"" .. source .. "\" \"" .. method
+ .. "\" requires \"" .. credential .. "\" when
\"par.enabled\" is true"
+ end
+
+ return true
+end
+
+
+-- The client assertion is signed with a single algorithm, but each endpoint
+-- picks its own auth method. lua-resty-openidc rejects a symmetric algorithm
+-- with private_key_jwt and an asymmetric one with client_secret_jwt when the
+-- endpoint is called, which surfaces as a 500. With no algorithm configured
+-- the library defaults per auth method, so the families cannot conflict.
+local function check_client_jwt_assertion_alg(conf)
+ local alg = conf.client_jwt_assertion_alg
+ if not alg then
+ return true
+ end
+
+ -- bearer_only never runs the authorization code flow, so only the
+ -- introspection endpoint is ever called
+ local selections = {
+ {name = "introspection_endpoint_auth_method",
+ method = conf.introspection_endpoint_auth_method},
+ }
+ if not conf.bearer_only then
+ core.table.insert(selections, {name = "token_endpoint_auth_method",
+ method =
conf.token_endpoint_auth_method})
+ core.table.insert(selections, {name = "par.endpoint_auth_method",
+ method = conf.par and
conf.par.endpoint_auth_method})
Review Comment:
Correct, fixed in e2ed91e. `par.endpoint_auth_method` now only counts toward
a family conflict when `par.enabled` is true — and, since PAR without a method
of its own uses the resolved `token_endpoint_auth_method` that is already
counted, only when it is set explicitly. TEST 81 covers the disabled case.
##########
docs/en/latest/plugins/openid-connect.md:
##########
@@ -60,6 +60,15 @@ The `openid-connect` Plugin supports the integration with
[OpenID Connect (OIDC)
| public_key | string | False | | | Public key used to verify JWT signature if
asymmetric algorithm is used. Providing this value to perform token
verification will skip token introspection in client credentials flow. You can
pass the public key in `-----BEGIN PUBLIC KEY-----\n……\n-----END PUBLIC
KEY-----` format. |
| use_jwks | boolean | False | false | | If true and if `public_key` is not
set, use the JWKS to verify JWT signature and skip token introspection in
client credentials flow. The JWKS endpoint is parsed from the discovery
document. |
| use_pkce | boolean | False | false | | If true, use the Proof Key for Code
Exchange (PKCE) for Authorization Code Flow as defined in [RFC
7636](https://datatracker.ietf.org/doc/html/rfc7636). |
+| par | object | False | | | Pushed Authorization Requests (PAR)
configuration. |
+| par.enabled | boolean | False | false | | If true, use OAuth 2.0 Pushed
Authorization Requests (PAR) as defined in [RFC
9126](https://datatracker.ietf.org/doc/html/rfc9126). Authorization request
parameters are sent to the PAR endpoint and the browser is redirected with the
returned `request_uri`. |
+| par.endpoint | string | False | | | URL of the PAR endpoint. If unset, the
endpoint from the well-known discovery document is used. |
+| par.endpoint_auth_method | string | False | | | Authentication method for
the PAR endpoint. If unset, `token_endpoint_auth_method` is used. |
Review Comment:
Fixed in e2ed91e, in both documents.
`par.endpoint_auth_method` now lists the four values and states the
credential requirement, including that the PAR request fails outright when its
method cannot be used, unlike the token endpoint which falls back.
The shared rows are corrected too: `client_rsa_private_key` and
`client_rsa_private_key_id` now say "whenever `private_key_jwt` is selected, by
`token_endpoint_auth_method`, `introspection_endpoint_auth_method` or
`par.endpoint_auth_method`", and `client_jwt_assertion_expires_in` likewise
covers all three endpoints. `client_rsa_private_key` also documents that its
key type has to match `client_jwt_assertion_alg` — RSA for `RS*`, an EC key on
the matching curve for `ES*` — and the `client_jwt_assertion_alg` row now
states that it is one algorithm for every endpoint, so a configuration cannot
select `private_key_jwt` for one and `client_secret_jwt` for another while
setting it.
--
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]
