Skip to content

[fix][admin] PIP-478: PulsarAdminBuilderImpl.build() shares its configuration, so a second admin can inherit the first's OAuth2 IdP trust material #26398

Description

@lhotari

Motivation

PulsarAdminBuilderImpl.build() hands the builder's own ClientConfigurationData to the admin without
copying it:

// PulsarAdminBuilderImpl:49
public PulsarAdmin build() throws PulsarClientException {
    return new PulsarAdminImpl(conf.getServiceUrl(), conf,
            clientBuilderClassLoader, acceptGzipCompression, sharedResources);
}

and PulsarAdminImpl.foldOAuth2IdpPolicy writes into it during construction:

// PulsarAdminImpl:514
oauth2.idpTlsPolicy(clientDefault.jsseProvider(), clientDefault.jcaProvider()).ifPresent(policy -> {
    Map<TlsPurpose, TlsPolicy> policies = conf.getTlsPolicyMap();
    if (policies == null) {
        policies = new LinkedHashMap<>();
        conf.setTlsPolicyMap(policies);
    }
    policies.putIfAbsent(TlsPurpose.CLIENT_OAUTH2, policy);
});

The first build() installs the IdP policy into a map that stays reachable from the builder. On a second
build() the map is already present and already has a CLIENT_OAUTH2 entry, so putIfAbsent keeps the
first one:

PulsarAdmin a = builder.authentication(oauth2ForIdpA).build();
PulsarAdmin b = builder.authentication(oauth2ForIdpB).build();  // b resolves against IdP A's policy

Admin b then fetches its token against IdP B while trusting IdP A's material — the wrong private CA, or
the wrong mTLS client key — with no error and no log line.

Scope

Narrower than the client-side version of this bug. The admin never stores a composed PulsarTlsFactory
back onto conf, so there is no closed-factory failure and no cross-client teardown; a plain second
build() with unchanged authentication is idempotent. It needs two builds from one builder with
different OAuth2 credentials, which is unusual — but the outcome is silent wrong-trust rather than a
visible failure, which is why it is worth closing rather than documenting.

Suggested fix

The same one-line change the two client builders received in #26326 — hand the admin a copy:

return new PulsarAdminImpl(conf.getServiceUrl(), conf.clone(), ...);

ClientConfigurationData.clone() is a shallow copy, which is enough here: the defect is the shared
mutation target, and every caller that mutates the builder's configuration does so before build().
Note that a shallow copy still shares the tlsPolicyMap instance itself, so the copy should be paired
with either a defensive copy of that map or a put rather than putIfAbsent at the fold — worth
deciding when fixing, and worth a test that builds two admins with different OAuth2 IdP material from one
builder and asserts each gets its own CLIENT_OAUTH2 policy.

Context

Reported by @david-streamlio reviewing #26326, alongside the same defect on ClientBuilderImpl and
PulsarClientBuilderV5 — both of which are fixed in that PR. Split out here on his suggestion, since the
consequence and the reasoning differ enough to take on their own merits rather than for symmetry.

Verified against master before filing: both call sites are as quoted.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions