[PR] [ISSUE #11362] fix(proxy): evaluate the admin ACL against the client address, not the transport peer [rocketmq]

["iloveeyjafjalla (via GitHub)" <gi...@apache.org>]

iloveeyjafjalla opened a new pull request, #11363:
URL: https://github.com/apache/rocketmq/pull/11363

   ### Which Issue(s) This PR Fixes
   
   - Fixes #11362
   
   ### Brief Description
   
   The admin ACL evaluated the source IP against the transport peer:
   
   ```java
   InetSocketAddress remoteAddress = (InetSocketAddress) call.getAttributes()
       .get(Grpc.TRANSPORT_ATTR_REMOTE_ADDR);
   ```
   
   The admin server shares the data plane's negotiator, so it accepts PROXY protocol connections, and its interceptor chain
   contains `HeaderInterceptor`, which resolves the real client address — the PROXY protocol address when present, the transport
   peer otherwise — into `GrpcConstants.REMOTE_ADDRESS`. That is exactly the value the data plane authorizes against
   (`DefaultAuthorizationContextBuilder` derives its source IP with
   `StringUtils.substringBeforeLast(metadata.get(GrpcConstants.REMOTE_ADDRESS), ":")`), so the two surfaces disagreed about the
   same client:
   
   - behind a proxy-protocol load balancer the transport peer is the balancer, so an `-i <client-range>` restriction was
     evaluated against the balancer — operators are denied when the balancer is outside the range, and when it is inside the
     range the restriction no longer restricts anything;
   - every `[PROXY-ADMIN-AUDIT]` line recorded the balancer as `sourceIp`.
   
   The change reads the normalized header (with the same `host:port` → host extraction the data plane uses) and keeps the
   transport peer as the fallback for the case where the interceptor runs without the standard pipeline.
   
   ### How Did You Test This Change?
   
   New tests in `ProxyAdminAuthInterceptorTest`:
   
   - `sourceIpUsesTheClientAddressHeaderInterceptorResolvedTest`: a call whose attributes carry the transport peer
     (`10.0.0.9:5000`, the balancer) and whose metadata carries the client address resolved by `HeaderInterceptor`
     (`203.0.113.7:4444`); the resolved ACL subject must be `203.0.113.7`. Fails before the change
     (`expected:<[203.0.113.7]> but was:<[10.0.0.9]>`), passes after it.
   - `sourceIpFallsBackToTheTransportPeerTest`: without the normalized header the transport peer is still used.
   
   Results on Temurin JDK 8 (same JDK as CI), base `9ea2ccdc2` plus this patch:
   
   - before the fix: `expected:<[203.0.113.7]> but was:<[10.0.0.9]>`
   - after the fix: `ProxyAdminAuthInterceptorTest — Tests run: 9, Failures: 0, Errors: 0, Skipped: 0`
   - `mvn -B -ntp -pl proxy -am package` (checkstyle + spotbugs enabled): `BUILD SUCCESS`, spotbugs `BugInstance size is 0`
   
   This change was prepared with AI assistance (analysis, patch and test drafting); the diff and every result above were
   reproduced locally by running the module's tests against the base commit and the patched tree.
   


-- 
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: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org