-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix(forward-auth): extra_headers not resolving variable on $post_arg. #12435
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?
Conversation
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.
Other doc changes LGTM
@@ -187,11 +187,12 @@ curl -X PUT 'http://127.0.0.1:9180/apisix/admin/routes/auth' \ | |||
"functions": [ | |||
"return function(conf, ctx) | |||
local core = require(\"apisix.core\") | |||
if core.request.header(ctx, \"tenant_id\") then | |||
local tenant_id = core.request.header(ctx, \"tenant_id\") | |||
if tenant_id == \"123\" then | |||
core.response.set_header(\"X-User-ID\", \"i-am-an-user\"); |
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.
Maybe remove this line to avoid any confusion? Since users will not actually observe this response header in this example.
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.
This is required to pass/deny the request. Users don't need to care about it. This is forward-auth server simulated.
Co-authored-by: Traky Deng <[email protected]>
The existing util.resolve_var doesn't handle all the variable cases properly. For eg: when passed with $post_arg.xyz, it fails to resolve the variable. So forward auth plugin is being modified to directly use the ctx.var
And no other plugin using util.resolve_var, tries to resolve $post_arg. so it is assumed that it's not part of the use case for util.resolve_var
This was not caught previously because the test cases didn't assert the values properly.
Checklist