AlinsRan commented on code in PR #13905:
URL: https://github.com/apache/apisix/pull/13905#discussion_r3921524689
##########
apisix/plugins/wolf-rbac.lua:
##########
@@ -44,26 +52,54 @@ local schema = {
type = "string",
default = "http://127.0.0.1:12180";
},
- header_prefix = {
- type = "string",
- default = "X-"
- },
+ header_prefix = header_prefix,
ssl_verify = {
type = "boolean",
},
}
-}
+
+ if include_output_header_prefix then
+ properties.output_header_prefix = {
+ type = "string",
+ }
+ end
+
+ return {
+ type = "object",
+ properties = properties,
+ }
+end
+
+
+-- header_prefix remains accepted by the Route schema for compatibility with
+-- stored configurations, but only output_header_prefix opts a Route into
+-- controlling the identity-header namespace.
+local schema = create_schema(nil, true)
+local consumer_schema = create_schema(default_header_prefix)
local _M = {
version = 0.1,
priority = 2555,
type = 'auth',
name = plugin_name,
schema = schema,
+ consumer_schema = consumer_schema,
}
local token_version = 'V1'
+local function clear_identity_headers(ctx, prefix)
+ core.request.set_header(ctx, prefix .. "UserId", nil)
+ core.request.set_header(ctx, prefix .. "Username", nil)
+ core.request.set_header(ctx, prefix .. "Nickname", nil)
+end
+
+
+function _M.clear_auth_headers(conf, ctx)
+ clear_identity_headers(ctx, conf.output_header_prefix or
default_header_prefix)
Review Comment:
`clear_auth_headers` only knows the Route-level `output_header_prefix`, so
the pre-clear pass in `multi-auth` can only scrub the `X-` namespace. In the
configuration this PR documents as the supported compatibility path — no
`output_header_prefix` on the Route, a custom `header_prefix` on the Consumer —
the spoofing hole stays open whenever another authenticator wins first.
The Consumer-fallback clear added further down (`if not
conf.output_header_prefix and prefix ~= default_header_prefix`) does not cover
it: it sits inside `rewrite()` after `cur_consumer` has been resolved, and
`rewrite()` never runs at all once an earlier authenticator returns success.
Repro on `ef63c5ef` — `multi-auth: [key-auth, wolf-rbac{}]`, Consumer
`header_prefix: "X-Wolf-"`, request carries a valid `apikey` plus spoofed
`X-Wolf-*` headers:
```
not ok - response_body
got: 'nil,nil,nil,spoofed-consumer,spoofed-consumer,spoofed-consumer'
expected: 'nil,nil,nil,nil,nil,nil'
not ok - pattern "hit wolf-rbac rewrite" should match a line in error.log
```
The second assertion is the `core.log.info("hit wolf-rbac rewrite")` already
present in `rewrite()`; it never fires, which confirms the fallback clear is
unreachable on this path.
The added coverage does not exercise this: TEST 25 sets
`output_header_prefix` on the Route, and TEST 24 injects only `X-*` headers
even though TEST 23 creates the `wolf-prefixed` Consumer.
This needs a decision rather than one more test case. Either the pre-clear
pass resolves the Consumer prefixes (which the PR description rules out — "does
not enumerate or cache Consumer configurations"), or Consumer-level
`header_prefix` is treated as unsafe and removed outright instead of kept as a
working deprecated fallback. As written, neither holds.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
