feat: add support for extra_headers in forward-auth plugin#12405
feat: add support for extra_headers in forward-auth plugin#12405Revolyssup merged 22 commits intoapache:masterfrom
Conversation
Co-authored-by: Traky Deng <[email protected]>
| When the decision is to be made on the basis of POST body, then it is recommended to use `$post_arg.xyz` with `extra_headers` field and make the decision on Authorization service on basis of headers rather than using POST `request_method` to pass the entire request body to Authorization service. | ||
| ::: | ||
|
|
||
| Create a serverless function on the `/auth` route that checks for the presence of the `tenant_id` header. If present, the route responds with HTTP 200 and sets the `X-User-ID` header to a fixed value `i-am-an-user`. If `tenant_id` is missing, it returns HTTP 400 with an error message. |
There was a problem hiding this comment.
Untranslated English content in chinese doc
Co-authored-by: Traky Deng <[email protected]>
Co-authored-by: Traky Deng <[email protected]>
…/apisix into revolyssup/forward-auth-fix-2
|
No additional comments for docs. |
apisix/plugins/forward-auth.lua
Outdated
| if not err and n_resolved > 0 then | ||
| auth_headers[header] = resolve_value | ||
| end |
There was a problem hiding this comment.
This basically says that the value itself is only rewritten when the variable substitution succeeds, which may not be necessary?
There was a problem hiding this comment.
I have modified to only skip in case of error.
When the value is not of type variable(doesn't contain $), it will still be added to auth headers.
Added one more test case to confirm it.
apisix/plugins/forward-auth.lua
Outdated
| auth_headers[header] = resolve_value | ||
| end | ||
| if err then | ||
| core.log.error("failed to resolve variable in extra header '", header, "': ", err) |
There was a problem hiding this comment.
another question: I think we should print the value, it is important information for user
bzp2010
left a comment
There was a problem hiding this comment.
Your checklist mentions that it is not compatible? Can you reconfirm this?
Oh this is compatible. Missed the last check. Fixed it. |
Description
A follow up for - #12404. The root cause is explained in the revert PR.
With the new solution:
Do not need to pass the whole request body to the auth service and extract data from body into headers and then forward to the auth service. The headers to be extracted will be specified with a new field: extra_headers
Fixes #11200
Checklist