Repository navigation
refactor(wolf-rbac): move header_prefix to route configuration - #13929
shreemaan-abhishek wants to merge 4 commits into
Conversation
Baoyuantop
left a comment
There was a problem hiding this comment.
Direction looks good (worth merging after the inline notes). Please address the Consumer schema fail-loud gap and the Attributes table split.
| }, | ||
| }, | ||
| required = {"appid"}, | ||
| } |
There was a problem hiding this comment.
After this move, leftover Consumer keys server / header_prefix / ssl_verify still validate (JSON Schema allows extra properties by default) but are ignored at runtime — traffic can silently hit the default http://127.0.0.1:12180.
Please fail loud on TYPE_CONSUMER: reject those three fields in check_schema (clearer than only additionalProperties = false, and matches an intentional migration), and add a negative Admin/schema test next to the existing “appid is required” case.
| | appid | string | True | N/A | Consumer application ID configured in Wolf. This field supports saving the value in Secret Manager using the [APISIX Secret](../terminology/secret.md) resource. | | ||
| | server | string | False | "http://127.0.0.1:12180" | Route or Service address of the Wolf server. | | ||
| | header_prefix | string | False | "X-" | Route or Service prefix for the identity headers added after successful authentication: `{prefix}UserId`, `{prefix}Username`, and `{prefix}Nickname`. | | ||
| | ssl_verify | boolean | False | false | Whether to verify the Wolf server's TLS certificate. Configured on a Route or Service. | |
There was a problem hiding this comment.
This table still mixes Consumer (appid) and Route/Service (server, header_prefix, ssl_verify) in one place. Please split into two tables (or add a Scope column), same as other dual-schema auth plugins — otherwise the breaking change is easy to misconfigure.
| | appid | string | 是 | 无 | Consumer 在 Wolf 中配置的应用 ID。该字段支持使用 [APISIX Secret](../terminology/secret.md) 资源,将值保存在 Secret Manager 中。 | | ||
| | server | string | 否 | "http://127.0.0.1:12180" | Route 或 Service 使用的 Wolf 服务地址。 | | ||
| | header_prefix | string | 否 | "X-" | Route 或 Service 的身份 HTTP 头前缀。鉴权成功后添加 `{prefix}UserId`、`{prefix}Username` 和 `{prefix}Nickname`。 | | ||
| | ssl_verify | boolean | 否 | false | 是否验证 Wolf 服务的 TLS 证书。在 Route 或 Service 上配置。 | |
There was a problem hiding this comment.
Same as the EN Attributes note: please split Consumer vs Route/Service fields into separate tables (or add a Scope column), and keep it consistent with the English docs.
| function _M.rewrite(conf, ctx) | ||
| if not auth_utils.is_running_under_multi_auth(ctx) then | ||
| clear_identity_headers(conf, ctx) | ||
| end |
There was a problem hiding this comment.
Now that header_prefix lives on the route, this can be unconditional: clear the route-prefix headers as the very first thing in rewrite, before any early return. Clearing is idempotent, so running it again after multi-auth's pre-clear is harmless, and that lets us drop the is_running_under_multi_auth branch, the clear_identity_headers wrapper (L84-94) and the auth_utils require. Something like:
local identity_headers = {"UserId", "Username", "Nickname"}
function _M.clear_auth_headers(conf, ctx)
for _, name in ipairs(identity_headers) do
core.request.set_header(ctx, conf.header_prefix .. name, nil)
end
end
function _M.rewrite(conf, ctx)
_M.clear_auth_headers(conf, ctx)
...The clear_auth_headers hook in multi-auth should stay though, since wolf-rbac never runs at all when an earlier auth plugin succeeds.
|
|
||
|
|
||
|
|
||
| === TEST 25b: malformed Wolf token does not preserve Route identity headers |
There was a problem hiding this comment.
Leftover 25b from the reindex, please renumber.
|
|
||
|
|
||
| === TEST 3: setup public API route | ||
| === TEST 3b: appid is required in Consumer configuration |
Only header_prefix moves to the route or service, so identity headers can be cleared before authentication. server and ssl_verify stay on the consumer, which lets the public API handlers keep resolving them from the consumer matched by appid.
|
@nic-6443 Thanks for the review. I'd like to go with a smaller schema change: move only All three public API handlers ( With
Keeping On the other points:
|
Description
wolf-rbacreadsheader_prefixfrom the matched Consumer, so the identity header namespace is unknown until a Wolf token has been parsed and a Consumer found. A Route therefore cannot clear or own its identity headers deterministically.This PR:
header_prefixto the Route/Service configuration (defaultX-)appid(now required),server, andssl_verifyon the Consumer, since a Wolfappidbelongs to one Wolf server; the public APIs (login,change_pwd,user_info) keep resolving them from the Consumer matched byappid{prefix}UserId,{prefix}Username, and{prefix}Nicknameat the start ofrewrite, and through aclear_auth_headershook thatmulti-authcalls before evaluating any authenticatorheader_prefixon a Consumer with a migration message; the old schema default"X-"is accepted and droppedThis is a breaking schema change limited to
header_prefix.Migration
Before upgrading, move a custom
header_prefixfrom each Consumer to every Route/Service usingwolf-rbac(includingwolf-rbacinsidemulti-auth), and remove it from the Consumer.appid,server, andssl_verifystay on the Consumer. Routes exposing the Wolf APIs need onlypublic-api, as before.Checklist
header_prefix)