SOLR-17644: Fix missing auth listener

[iamsanjay]

https://issues.apache.org/jira/browse/SOLR-17644

Checklist

Please review the following and check all that apply:

• I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
• I have created a Jira issue and added the issue ID to my pull request title.
• I have given Solr maintainers access to contribute to my PR branch. (optional but recommended, not available for branches on forks living under an organisation)
• I have developed this patch against the main branch.
• I have run ./gradlew check.
• [] I have added tests for my changes.
• I have added documentation for the Reference Guide

[iamsanjay]

This is not the final solution. I created this to initiate discussion on the best possible approach to solving this.

[iamsanjay]

Test case!

@Test
  public void testAuth() throws Exception {
    // This succeeds with auth enabled
    CollectionAdminRequest.createCollection("c123", "conf", 1, 1)
        .setBasicAuthCredentials(SecurityJson.USER, SecurityJson.PASS)
        .process(cluster.getSolrClient());
    cluster.waitForActiveCollection("c123", 1, 1);

    // Configure placement plugin
    PluginMeta plugin = new PluginMeta();
    plugin.name = PlacementPluginFactory.PLUGIN_NAME;
    plugin.klass = MinimizeCoresPlacementFactory.class.getName();
    V2Request v2Request =
        new V2Request.Builder("/cluster/plugin")
            .forceV2(true)
            .POST()
            .withPayload(singletonMap("add", plugin))
            .build();
    v2Request.setBasicAuthCredentials(SecurityJson.USER, SecurityJson.PASS);
    v2Request.process(cluster.getSolrClient());

    // Now this will fail with a 401 !!
    CollectionAdminRequest.createCollection("c456", "conf", 1, 1)
        .setBasicAuthCredentials(SecurityJson.USER, SecurityJson.PASS)
        .process(cluster.getSolrClient());
    cluster.waitForActiveCollection("c456", 1, 1);
    /// /////////
  }

[dsmiley]

Shouldn't this PR include a test like Colvin shared -- https://github.com/colvinco/solr/blob/f2e8825c5e46180eb121f20a69f246092ad60776/solr/core/src/test/org/apache/solr/security/BasicAuthIntegrationTest.java#L398-L403 so we can verify the fix?

(Doubtful but asking anyway) Is the bug a result of ZkController.getSolrCloudManager lazy creating the client instead of eager? I'm suspicious the lazy load saves anything of consequence.

Maybe CoreContainer.loadInternal needs to move these lines:

securityConfHandler =
        isZooKeeperAware() ? new SecurityConfHandlerZk(this) : new SecurityConfHandlerLocal(this);
    reloadSecurityProperties();

to immediately after zkSys.initZooKeeper....

[iamsanjay]

It's not about the lazy loading. SolrClientProvider object is being used inside the ZkController#getSolrCloudManager() but at that time auth listener is not available.

zkSys.initZooKeeper(this, cfg.getCloudConfig()); is what triggers somewhere ZkController#getSolrCloudManager()

But as you can see in the IF block we initialize the pkiAuthenticationSecurityBuilder after that.

solr/solr/core/src/java/org/apache/solr/core/CoreContainer.java

Lines 868 to 880 in 76c09a3

	zkSys.initZooKeeper(this, cfg.getCloudConfig());
	if (isZooKeeperAware()) {
	solrClientCache.setDefaultZKHost(getZkController().getZkServerAddress());
	// initialize ZkClient metrics
	zkSys.getZkMetricsProducer().initializeMetrics(solrMetricsContext, "zkClient");
	pkiAuthenticationSecurityBuilder =
	new PKIAuthenticationPlugin(
	this,
	zkSys.getZkController().getNodeName(),
	(PublicKeyHandler) containerHandlers.get(PublicKeyHandler.PATH));
	pkiAuthenticationSecurityBuilder.initializeMetrics(solrMetricsContext, "/authentication/pki");
	fileStore = new DistribFileStore(this);

[dsmiley]

I would prefer we initialize the httpClients with security as soon as possible, before they are obtained, even if the design seems to support dynamic changes after creation. It'd be better to guarantee the initialization has happened. But it seems to awkward to guarantee that right now. On my mind is dependency-injection frameworks which can manage complex object graphs with dependencies for startup initialization. Any way that's kind of a fantasy right now, and a big undertaking to embrace that.

I thought of a really simple solution here. I'm skeptical that ZkController.getSolrCloudManager truly needs to create yet another Http2SolrClient instance. It could use getCoreContainer().getDefaultHttpSolrClient(); (coming from HttpSolrClientProvider, which is instrumented with security, albeit later but that's okay). If we want to mess with timeout tuning then I think that should be HttpSolrClientProvider instance to consider that (which is configurable!) and have it be more global. getSolrCloudManager isn't special.