membphis commented on code in PR #13905:
URL: https://github.com/apache/apisix/pull/13905#discussion_r3920320378
##########
apisix/plugins/wolf-rbac.lua:
##########
@@ -276,6 +313,14 @@ function _M.rewrite(conf, ctx)
return 401, fail_response("Invalid appid in rbac token")
end
core.log.info("consumer appid: ", appid)
+ local prefix = conf.header_prefix
Review Comment:
[P1] Preserve the identity-header contract of existing Route configurations
This is a breaking change for existing deployments, not only a precedence
change for newly configured Routes. The old schema inserts header_prefix: "X-"
when the Route field is omitted, and the Admin API persists the validated
configuration. After upgrade, this branch treats that stored default as an
explicit Route choice and overrides a Consumer-level custom prefix.
For an existing Route containing the historical "X-" default and a Consumer
using "X-Wolf-", a focused test of the actual old and new rewrite functions
shows that the old code replaces a client-supplied X-Wolf-UserId with the
authenticated user ID. The new code leaves the client-supplied X-Wolf-UserId
intact and writes the authenticated ID to X-UserId instead. An upstream that
still relies on X-Wolf-* can therefore lose the trusted identity or consume a
spoofed value after upgrade. Default-prefix Consumers and newly created Routes
that truly omit the field do not trigger this case.
Please fix the compatibility regression by preserving the historical
Consumer-prefix contract until an explicit migration or a distinguishable
opt-in selects Route precedence. Removing the default from the new schema does
not remove defaults already stored in old configurations.
Please also explicitly declare this as a BREAKING CHANGE in the PR
description, including affected historical configurations, required migration
steps, rollout order, and recovery guidance. Add an upgrade regression starting
from an old-schema-defaulted Route and assert both authenticated identity
output and removal or replacement of spoofed headers in the trusted namespace.
--
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]
