michaeljmarshall opened a new pull request, #19557: URL: https://github.com/apache/pulsar/pull/19557
### Motivation I broker the Pulsar Proxy with #19455 because that PR requires that when `X-Original-Principal` is supplied, the auth role must be a proxy role. This is not always the case for proxied admin requests. This PR seeks to fix the proxy by changing the way that the proxy uses the `X-Original-Principal` header. The primary benefit will be a reduction in unnecessary authorization checks. Currently, we do the following when authentication is enabled in the proxy: 1. Authenticate the client's http request and put the resulting role in the `X-Original-Principal` header for the call to the broker. https://github.com/apache/pulsar/blob/38555851359f9cfc172650c387a58c5a03809e97/pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/AdminProxyHandler.java#L370-L373 2. Copy the `Authorization` header into the broker's http request: https://github.com/apache/pulsar/blob/38555851359f9cfc172650c387a58c5a03809e97/pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/AdminProxyHandler.java#L232-L236 3. Configure the proxy's http client to use client TLS authentication (when configured): https://github.com/apache/pulsar/blob/38555851359f9cfc172650c387a58c5a03809e97/pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/AdminProxyHandler.java#L269-L277 The problem with #19455 is that it assumes the proxy supplies its own authentication data. However, that only happens when using TLS authentication. Otherwise, the proxy forwards the client's authentication data in the `Authorization` header. As such, calls will fail because the `X-Original-Principal` header supplied without using a proxy role. In this PR, I propose that we only add the `X-Original-Principal` when we are using the proxy's authentication. ### Modifications * Only add the `X-Original-Principal` header when the proxy is authenticating with the broker via TLS. ### Verifying this change When cherry-picking #19455 to branch-2.9, I discovered that `PackagesOpsWithAuthTest#testPackagesOps` was consistently failing because of the way the proxy supplies authentication data when proxying http requests. That test was removed by https://github.com/apache/pulsar/pull/12771, which explains why I didn't catch the error sooner. This PR includes a test that fails without this change. Note that the primary issue must be that we didn't have any tests doing authentication forwarding through the proxy. Now we will have both relevant tests where the proxy is and is not authenticating. ### Does this pull request potentially affect one of the following parts: This is not a breaking change. ### Documentation - [x] `doc-required` ### Matching PR in forked repository PR in forked repository: https://github.com/michaeljmarshall/pulsar/pull/31 -- 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]
