[fix][broker] Allow proxy to pass same role for authRole and originalRole

[michaeljmarshall]

Motivation

I broke 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 that incorrect assumption by changing the way verification is done for the roles. Specifically, when the two roles are the same and they are not a proxy role, we will consider it a valid combination.

Note that there is no inefficiency in this solution because When the authenticatedPrincipal is not a proxy role, that is the only role that is authenticated. Note also that we do not let the binary protocol authenticate this way, and that is consistent with the way the pulsar proxy forwards authentication data.

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.
   

   pulsar/pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/AdminProxyHandler.java

   Lines 370 to 373 in 3855585

   	String user = (String) clientRequest.getAttribute(AuthenticationFilter.AuthenticatedRoleAttributeName);
   	if (user != null) {
   	proxyRequest.header(ORIGINAL_PRINCIPAL_HEADER, user);
   	}

2. Copy the Authorization header into the broker's http request:
   

   pulsar/pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/AdminProxyHandler.java

   Lines 232 to 236 in 3855585

   	String authorization = oldRequest.getHeaders().get(HttpHeader.AUTHORIZATION);
   	Request newRequest = super.copyRequest(oldRequest, newURI);
   	if (authorization != null) {
   	newRequest.header(HttpHeader.AUTHORIZATION, authorization);
   	}

3. Configure the proxy's http client to use client TLS authentication (when configured):
   

   pulsar/pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/AdminProxyHandler.java

   Lines 269 to 277 in 3855585

   	AuthenticationDataProvider authData = auth.getAuthData();
   	if (authData.hasDataForTls()) {
   	sslCtx = SecurityUtility.createSslContext(
   	config.isTlsAllowInsecureConnection(),
   	trustCertificates,
   	authData.getTlsCertificates(),
   	authData.getTlsPrivateKey(),
   	config.getBrokerClientSslProvider()
   	);

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.

Modifications

• Consider the authenticatedPrincipal and the originalPrincipal a valid pair when they are equal and are not a proxyRole for http requests.

Alternative Solutions

I initially proposed that we only add the X-Original-Principal when we are using the proxy's authentication (see the first commit). I decided this solution is not ideal because it doesn't solve the problem, it doesn't make the brokers backwards compatible, and there isn't actually any inefficiency in passing the role as a header.

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 #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

• doc-required

Matching PR in forked repository

PR in forked repository: michaeljmarshall#31

[nicoloboschi]

LGTM

[michaeljmarshall]

/pulsarbot rerun-failure-checks

[mattisonchao]

LGTM

[michaeljmarshall]

The flaky proxy test is fixed by #19594

[momo-jun]

Hi @michaeljmarshall , I'm checking in to follow up with the doc updates. Did you have any plans to update the docs?

[michaeljmarshall]

After reviewing this, I realize now that the only place I needed to add docs was in the Javadocs. We don't document the HTTP proxy to broker interactions on our website, so nothing needs to be updated there.