-
Notifications
You must be signed in to change notification settings - Fork 2.9k
feat(openid-connect): support PAR and DPoP client options #13649
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
a9fd7f3
6a6f8e4
594f6a5
b8be0d3
14a8366
d64a6f1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -52,6 +52,25 @@ local function build_session_opts(session_conf) | |
| end | ||
|
|
||
|
|
||
| local function flatten_openidc_options(conf) | ||
| if conf.par then | ||
| conf.use_par = conf.par.enabled | ||
| conf.pushed_authorization_request_endpoint = conf.par.endpoint | ||
| conf.pushed_authorization_request_endpoint_auth_method = | ||
| conf.par.endpoint_auth_method | ||
| conf.par = nil | ||
| end | ||
|
|
||
| if conf.dpop then | ||
| conf.use_dpop = conf.dpop.enabled | ||
| conf.dpop_signing_alg = conf.dpop.signing_alg | ||
| conf.dpop_private_key = conf.dpop.private_key | ||
| conf.dpop_public_jwk = conf.dpop.public_jwk | ||
| conf.dpop = nil | ||
| end | ||
| end | ||
|
|
||
|
|
||
| local schema = { | ||
| type = "object", | ||
| properties = { | ||
|
|
@@ -298,6 +317,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", | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I verified a JWK carrying private key material passes 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
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the detailed catch. I added the schema-side rejection for private JWK members ( |
||
| ["not"] = {anyOf = { | ||
| {required = {"d"}}, | ||
| {required = {"p"}}, | ||
| {required = {"q"}}, | ||
| {required = {"dp"}}, | ||
| {required = {"dq"}}, | ||
| {required = {"qi"}}, | ||
| {required = {"oth"}}, | ||
| {required = {"k"}}, | ||
| }}, | ||
| }, | ||
| }, | ||
| ["if"] = { | ||
| properties = { | ||
| enabled = { const = true }, | ||
| }, | ||
| }, | ||
| ["then"] = { | ||
| required = {"private_key", "public_jwk"}, | ||
| }, | ||
| additionalProperties = false, | ||
| }, | ||
| set_access_token_header = { | ||
| description = "Whether the access token should be added as a header to the request " .. | ||
| "for downstream", | ||
|
|
@@ -371,6 +454,14 @@ local schema = { | |
| type = "integer", | ||
| default = 60 | ||
| }, | ||
| client_jwt_assertion_alg = { | ||
| description = "Signing algorithm for the client assertion JWT.", | ||
| type = "string" | ||
| }, | ||
| client_jwt_assertion_audience = { | ||
| description = "Audience for the client assertion JWT.", | ||
| type = "string" | ||
| }, | ||
| renew_access_token_on_expiry = { | ||
| description = "Whether to attempt silently renewing the access token.", | ||
| type = "boolean", | ||
|
|
@@ -464,7 +555,7 @@ local schema = { | |
| default = nil, | ||
| } | ||
| }, | ||
| encrypt_fields = {"client_secret", "client_rsa_private_key", | ||
| encrypt_fields = {"client_secret", "client_rsa_private_key", "dpop.private_key", | ||
| "session.secret", "session.redis.password"}, | ||
| required = {"client_id", "discovery"} | ||
| } | ||
|
|
@@ -509,7 +600,8 @@ function _M.check_schema(conf) | |
| end | ||
|
|
||
| local check = {"discovery", "introspection_endpoint", "redirect_uri", | ||
| "post_logout_redirect_uri", "proxy_opts.http_proxy", "proxy_opts.https_proxy"} | ||
| "post_logout_redirect_uri", "par.endpoint", "proxy_opts.http_proxy", | ||
| "proxy_opts.https_proxy"} | ||
| core.utils.check_https(check, conf, plugin_name) | ||
| core.utils.check_tls_bool({"ssl_verify"}, conf, plugin_name) | ||
|
|
||
|
|
@@ -701,6 +793,7 @@ end | |
|
|
||
| function _M.rewrite(plugin_conf, ctx) | ||
| local conf = core.table.clone(plugin_conf) | ||
| flatten_openidc_options(conf) | ||
|
|
||
| -- Snapshot the client-supplied X-Access-Token (it doubles as a bearer | ||
| -- input via get_bearer_access_token) and clear the four headers this | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
parwithendpointpointing at the mock, hit the route, and assert the 302 Location carries onlyclient_idandrequest_uriand 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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_idandrequest_uriare present, whilescope,state,response_type, andredirect_uriare absent.