feat(openid-connect): support PAR and DPoP client options#13649
feat(openid-connect): support PAR and DPoP client options#13649kevinlzw wants to merge 6 commits into
Conversation
|
This PR should also fix #13085. |
| }, | ||
| public_jwk = { | ||
| description = "Public JWK matching dpop.private_key.", | ||
| type = "object", |
There was a problem hiding this comment.
I verified a JWK carrying private key material passes check_schema here — {kty = "RSA", e = "AQAB", n = "...", d = "..."} is accepted. Since public_jwk is (rightly) not in encrypt_fields, a user who pastes their complete JWK ends up with the private exponent stored in plaintext in etcd, and the mistake only surfaces per request when lua-resty-openidc rejects the proof. Rejecting private JWK fields at config time keeps the key out of etcd and fails fast:
public_jwk = {
description = "Public JWK matching dpop.private_key.",
type = "object",
["not"] = {anyOf = {
{required = {"d"}}, {required = {"p"}}, {required = {"q"}},
{required = {"dp"}}, {required = {"dq"}}, {required = {"qi"}},
{required = {"oth"}}, {required = {"k"}},
}},
},Same field list as upstream's openidc_dpop_public_jwk_private_field; I checked the not/anyOf combo works with the jsonschema library APISIX uses.
There was a problem hiding this comment.
Thanks for the detailed catch. I added the schema-side rejection for private JWK members (d, p, q, dp, dq, qi, oth, and k) under dpop.public_jwk, so complete/private JWKs now fail check_schema before anything can be persisted in etcd. I also added a regression case that verifies a JWK containing d is rejected.
| end | ||
|
|
||
|
|
||
| local function flatten_openidc_options(conf) |
There was a problem hiding this comment.
The new tests are all schema-level, so nothing exercises this mapping at runtime — a typo in any of these assignments (or an option rename on the lua-resty-openidc side) would still pass CI. A single e2e block would cover it: serve a fake discovery doc plus a PAR mock from the test nginx (t/plugin/openid-connect-redis.t already does the fake-discovery part with a local authorization_endpoint), enable par with endpoint pointing at the mock, hit the route, and assert the 302 Location carries only client_id and request_uri and that scope/state stay out of the query. That one test proves the PAR wiring and the flattening end to end.
There was a problem hiding this comment.
Thanks, that makes sense. I added a Test::Nginx runtime block with a fake discovery document and a fake PAR endpoint, then configured the plugin through the Admin API and hit the route to exercise the actual flattening path. The test now parses the final authorize redirect and asserts the query shape exactly: only client_id and request_uri are present, while scope, state, response_type, and redirect_uri are absent.
| ngx.say(conf.client_rsa_private_key ~= "89ae4c8edadf1cd1c9f034335f136f87ad84b625c8f1") | ||
| ngx.say(conf.dpop.private_key ~= "dpop-private-key") |
There was a problem hiding this comment.
These ~= checks turn vacuously true when the field comes back nil, so a value dropped somewhere between admin API and etcd would read as "encrypted". The encryption in tests is deterministic (fixed key), so pinning the ciphertext like the old test did still works; if you'd rather avoid that brittleness, something like type(conf.dpop.private_key) == "string" and conf.dpop.private_key ~= "dpop-private-key" keeps the nil case failing.
There was a problem hiding this comment.
Good point. I updated the assertions to require the stored values to be strings before comparing them with the plaintext values, so a missing/nil value no longer passes as encrypted. I kept it value-agnostic rather than pinning the deterministic ciphertext, to avoid making the test depend on the exact encrypted blob.
Description
I maintain
lua-resty-openidc, and the latestlua-resty-openidc1.9.0 release added client-side PAR and DPoP support. This PR bumps APISIX to that release and exposes the new client-side OAuth/OIDC options through theopenid-connectPlugin.This PR adds nested APISIX Plugin configuration for:
The APISIX Plugin keeps user-facing PAR and DPoP options grouped under
paranddpop, then maps them to the flat option names expected bylua-resty-openidcbefore invoking the library. It also encryptsdpop.private_keyin etcd and documents the new options in English and Chinese.Related to #11219. This PR covers the
openid-connectPlugin acting as an OAuth/OIDC client/Relying Party. It does not implement APISIX resource-server-side DPoP proof validation.Backward compatibility
This change is backward compatible for existing
openid-connectPlugin configurations:par,dpop, or client assertion algorithm attributes.dpop.private_keyencryption only affects the new DPoP private-key field.Which issue(s) this PR fixes:
Fixes #13085
Related to #11219
Checklist
Tests
git -C apisix diff --checkapisix/plugins/openid-connect.luaprove -I../test-nginx/lib -I./ -r -s t/plugin/openid-connect.tFiles=1, Tests=175, Result: PASSmake lintwas run in a Docker temporary copy of the Windows checkout after normalizing CRLF line endings and installingluacheckin the temporary container:luacheck -q apisix t/lib:0 warnings / 0 errors in 376 fileslj-relengstill fails on existing repository-wide line-length/style findings, including pre-existingopenid-connect.lualong lines not introduced by this PR.