nic-6443 commented on code in PR #13649:
URL: https://github.com/apache/apisix/pull/13649#discussion_r3520524183
##########
apisix/plugins/openid-connect.lua:
##########
@@ -52,6 +52,25 @@ local function build_session_opts(session_conf)
end
+local function flatten_openidc_options(conf)
Review 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.
##########
apisix/plugins/openid-connect.lua:
##########
@@ -298,6 +317,60 @@ 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:
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:
```lua
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.
##########
t/plugin/openid-connect.t:
##########
@@ -152,23 +155,22 @@ passed
-=== TEST 5: verify encrypted field
+=== TEST 5: verify encrypted fields
--- config
location /t {
content_by_lua_block {
- local json = require("toolkit.json")
- local t = require("lib.test_admin").test
-
-
- -- get plugin conf from etcd, client_rsa_private_key is encrypted
+ -- get plugin conf from etcd, private key fields are encrypted
local etcd = require("apisix.core.etcd")
local res = assert(etcd.get('/routes/1'))
-
ngx.say(res.body.node.value.plugins["openid-connect"].client_rsa_private_key)
+ local conf = res.body.node.value.plugins["openid-connect"]
+ ngx.say(conf.client_rsa_private_key ~=
"89ae4c8edadf1cd1c9f034335f136f87ad84b625c8f1")
+ ngx.say(conf.dpop.private_key ~= "dpop-private-key")
Review 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.
--
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]
