fix(authz-keycloak): strip query string when resolving resources with lazy_load_paths#12914
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes the authz-keycloak plugin’s lazy resource discovery so Keycloak resource matching isn’t impacted by query parameters.
Changes:
- Update lazy-load resource resolution to use a URI value without the query string.
- Add an integration test that exercises
lazy_load_paths=truewith a request containing query parameters.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| apisix/plugins/authz-keycloak.lua | Switches the URI used for UMA resource discovery to avoid including query parameters. |
| t/plugin/authz-keycloak2.t | Adds coverage to ensure requests with query strings still authorize successfully under lazy resource discovery. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| -- Resolve URI to resource(s). | ||
| permission, err = authz_keycloak_resolve_resource(conf, ctx.var.request_uri, | ||
| permission, err = authz_keycloak_resolve_resource(conf, ctx.var.uri, | ||
| sa_access_token) |
There was a problem hiding this comment.
Using ctx.var.uri does strip the query string, but it also changes semantics vs ctx.var.request_uri: $uri is normalized/decoded and can be modified by internal rewrites (e.g., proxy-rewrite), while $request_uri is the original raw request (path+args). If the intent is only to drop the query string while preserving the original request path used for Keycloak resource matching, consider extracting the path component from ctx.var.request_uri (split on ?) instead of switching variables, or update the PR description/tests to cover the rewrite/normalization behavior change.
Description
Summary
ctx.var.uriinstead ofctx.var.request_uriwhen resolving resources withlazy_load_paths=true, ensuring query parameters are stripped before sending to Keycloak's resource registration endpointWhat's the problem?
When
lazy_load_paths=trueis enabled, the plugin incorrectly includes query parameters when calling Keycloak's UMA resource registration endpoint (resource_set?matchingUri=true). This causes Keycloak to fail resource matching with "invalid_resource" errors.For example, a request to
/api/items?country=eswould send the full URI including query string to Keycloak, which cannot match it against a resource configured with URI/api/items.refs:
What's the solution?
Replace
ctx.var.request_uri(path + query string) withctx.var.uri(path only) when resolving resources. This aligns with Keycloak's official Policy Enforcer behavior which usesrequest.getRelativePath().Which issue(s) this PR fixes:
Fixes #12785
Checklist