Re: [PR] SOLR-17644: Fix missing auth listener [solr]
iamsanjay merged PR #3208: URL: https://github.com/apache/solr/pull/3208 -- 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: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17644: Fix missing auth listener [solr]
dsmiley commented on PR #3208: URL: https://github.com/apache/solr/pull/3208#issuecomment-2688573136 tests pass! -- 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: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17644: Fix missing auth listener [solr]
dsmiley commented on code in PR #3208:
URL: https://github.com/apache/solr/pull/3208#discussion_r1973633118
##
solr/core/src/test/org/apache/solr/security/BasicAuthIntegrationTest.java:
##
@@ -373,6 +376,32 @@ public void testBasicAuth() throws Exception {
assertAuthMetricsMinimums(32, 20, 9, 1, 2, 0);
assertPkiAuthMetricsMinimums(19, 19, 0, 0, 0, 0);
+ // This succeeds with auth enabled
+ CollectionAdminRequest.createCollection("c123", "conf", 3, 1)
+ .setBasicAuthCredentials("harry", "HarryIsUberCool")
+ .process(cluster.getSolrClient());
+ cluster.waitForActiveCollection("c123", 3, 3);
+
+ // 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("harry", "HarryIsUberCool");
+ v2Request.process(cluster.getSolrClient());
+
+ // Now this will fail with a 401 !!
Review Comment:
Well it won't any longer :-), once we merge a fix. You can mention a JIRA
issue where it did.
--
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: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17644: Fix missing auth listener [solr]
iamsanjay commented on PR #3208: URL: https://github.com/apache/solr/pull/3208#issuecomment-2687064213 If I look the default connection and socket timeouts. ``` HttpClientUtil.DEFAULT_SO_TIMEOUT = 60,000 HttpClientUtil.DEFAULT_CONNECT_TIMEOUT = 60,000 ``` But I will make the change to ZkController to use the HttpSolrClientProvider instead of creating one. I also like this solution better. I only wonder if reducing the default timeouts was the reason in the first-place it was re-created here in ZkController. -- 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: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17644: Fix missing auth listener [solr]
iamsanjay commented on PR #3208: URL: https://github.com/apache/solr/pull/3208#issuecomment-2687044559 We are creating client inside ZkController with custom timeouts ``` http2SolrClient = new Http2SolrClient.Builder() .withHttpClient(cc.getDefaultHttpSolrClient()) .withIdleTimeout(3, TimeUnit.MILLISECONDS) .withConnectionTimeout(15000, TimeUnit.MILLISECONDS) .build(); ``` And HttpSolrClientProvider timeouts controlled via UpdateShardHandlerConfig ``` httpClientBuilder .withConnectionTimeout(cfg.getDistributedConnectionTimeout(), TimeUnit.MILLISECONDS) .withIdleTimeout(cfg.getDistributedSocketTimeout(), TimeUnit.MILLISECONDS) .withMaxConnectionsPerHost(cfg.getMaxUpdateConnectionsPerHost()); ``` -- 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: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17644: Fix missing auth listener [solr]
dsmiley commented on PR #3208: URL: https://github.com/apache/solr/pull/3208#issuecomment-2684074527 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. -- 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: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17644: Fix missing auth listener [solr]
iamsanjay commented on PR #3208: URL: https://github.com/apache/solr/pull/3208#issuecomment-2677628347 https://github.com/user-attachments/assets/2c5da665-2087-464d-b7b4-ae95ede9ce56"; /> 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. https://github.com/apache/solr/blob/76c09a35dba42913a6bcb281b52b00f87564624a/solr/core/src/java/org/apache/solr/core/CoreContainer.java#L868-L880 -- 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: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17644: Fix missing auth listener [solr]
iamsanjay commented on PR #3208:
URL: https://github.com/apache/solr/pull/3208#issuecomment-2677547561
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);
/// /
}
```
--
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: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17644: Fix missing auth listener [solr]
iamsanjay commented on PR #3208: URL: https://github.com/apache/solr/pull/3208#issuecomment-2677546452 This is not the final solution. I created this to initiate discussion on the best possible approach to solving this. -- 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: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
