Skip to content

fix(authz-keycloak): strip query string when resolving resources with lazy_load_paths#12914

Merged
Baoyuantop merged 2 commits into
apache:masterfrom
SkyeYoung:young/fix/authz-keycloak-without-querystring
Jan 29, 2026
Merged

fix(authz-keycloak): strip query string when resolving resources with lazy_load_paths#12914
Baoyuantop merged 2 commits into
apache:masterfrom
SkyeYoung:young/fix/authz-keycloak-without-querystring

Conversation

@SkyeYoung

@SkyeYoung SkyeYoung commented Jan 15, 2026

Copy link
Copy Markdown
Member

Description

Summary

  • Fix authz-keycloak plugin to use ctx.var.uri instead of ctx.var.request_uri when resolving resources with lazy_load_paths=true, ensuring query parameters are stripped before sending to Keycloak's resource registration endpoint

What's the problem?

When lazy_load_paths=true is 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=es would 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) with ctx.var.uri (path only) when resolving resources. This aligns with Keycloak's official Policy Enforcer behavior which uses request.getRelativePath().

Which issue(s) this PR fixes:

Fixes #12785

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

@SkyeYoung
SkyeYoung marked this pull request as ready for review January 28, 2026 06:07
@dosubot dosubot Bot added size:M This PR changes 30-99 lines, ignoring generated files. bug Something isn't working labels Jan 28, 2026
@moonming
moonming requested a review from Copilot January 28, 2026 06:35

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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=true with 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.

Comment on lines 569 to 571
-- 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)

Copilot AI Jan 28, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
@Baoyuantop
Baoyuantop merged commit 3546054 into apache:master Jan 29, 2026
38 of 40 checks passed
@SkyeYoung
SkyeYoung deleted the young/fix/authz-keycloak-without-querystring branch January 29, 2026 06:05
wistefan pushed a commit to wistefan/apisix that referenced this pull request Jun 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: authz-keycloak should omit querystring, so that keycloak authorization works

5 participants