This is an automated email from the ASF dual-hosted git repository. epugh pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/solr.git
commit 739fe3b0a54c88e6eaeb094eb1451b2f0d8e8700 Author: Mark Miller <[email protected]> AuthorDate: Tue Feb 8 13:51:34 2022 -0600 SOLR-14569: Configuring a shardHandlerFactory on the /select requestHandler results in HTTP 401 when searching on alias in secured Solr. --- .gitignore | 3 + .stignore | 1 + solr/CHANGES.txt | 3 + .../java/org/apache/solr/core/CoreContainer.java | 39 ++++--- .../solr/handler/component/SearchHandler.java | 1 + .../java/org/apache/solr/servlet/HttpSolrCall.java | 4 +- .../apache/solr/servlet/SolrDispatchFilter.java | 4 +- .../conf/schema.xml | 29 ++++++ .../conf/solrconfig.xml | 56 ++++++++++ .../AuthWithShardHandlerFactoryOverrideTest.java | 116 +++++++++++++++++++++ 10 files changed, 237 insertions(+), 19 deletions(-) diff --git a/.gitignore b/.gitignore index f54fadc..2130f3a 100644 --- a/.gitignore +++ b/.gitignore @@ -23,6 +23,9 @@ out/ .editorconfig +# syncthing +.stignore + # macOS .DS_Store diff --git a/.stignore b/.stignore new file mode 100644 index 0000000..378eac2 --- /dev/null +++ b/.stignore @@ -0,0 +1 @@ +build diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index dd99c8e..b9f96dc 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -631,6 +631,9 @@ Bug Fixes * SOLR-15587: Don't use the UrlScheme singleton on the client-side to determine cluster's scheme, only get from ClusterState provider impl (Timothy Potter) +* SOLR-14569: Configuring a shardHandlerFactory on the /select requestHandler results in HTTP 401 when searching on alias + in secured Solr. (Isabelle Giguere, Jason Gerlowski, Anshum Gupta, Mark Mark Miller) + ================== 8.11.1 ================== Bug Fixes diff --git a/solr/core/src/java/org/apache/solr/core/CoreContainer.java b/solr/core/src/java/org/apache/solr/core/CoreContainer.java index 456fec7..99abee8 100644 --- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java +++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java @@ -189,7 +189,7 @@ public class CoreContainer { private volatile InfoHandler infoHandler; protected volatile ConfigSetsHandler configSetsHandler = null; - private volatile PKIAuthenticationPlugin pkiAuthenticationPlugin; + private volatile PKIAuthenticationPlugin pkiAuthenticationSecurityBuilder; protected volatile Properties containerProperties; @@ -526,13 +526,19 @@ public class CoreContainer { // Setup HttpClient for internode communication HttpClientBuilderPlugin builderPlugin = ((HttpClientBuilderPlugin) authcPlugin); SolrHttpClientBuilder builder = builderPlugin.getHttpClientBuilder(HttpClientUtil.getHttpClientBuilder()); - shardHandlerFactory.setSecurityBuilder(builderPlugin); - updateShardHandler.setSecurityBuilder(builderPlugin); - // The default http client of the core container's shardHandlerFactory has already been created and - // configured using the default httpclient configurer. We need to reconfigure it using the plugin's - // http client configurer to set it up for internode communication. - log.debug("Reconfiguring HttpClient settings."); + + // this caused plugins like KerberosPlugin to register it's intercepts, but this intercept logic is also + // handled by the pki authentication code when it decideds to let the plugin handle auth via it's intercept + // - so you would end up with two intercepts + // --> + // shardHandlerFactory.setSecurityBuilder(builderPlugin); // calls setup for the authcPlugin + // updateShardHandler.setSecurityBuilder(builderPlugin); + // <-- + + // This should not happen here at all - it's only currently required due to its affect on http1 clients + // in a test or two incorrectly counting on it for their configuration. + // --> SolrHttpClientContextBuilder httpClientBuilder = new SolrHttpClientContextBuilder(); if (builder.getCredentialsProviderProvider() != null) { @@ -555,13 +561,16 @@ public class CoreContainer { } HttpClientUtil.setHttpClientRequestContextBuilder(httpClientBuilder); + + // <-- } + // Always register PKI auth interceptor, which will then delegate the decision of who should secure // each request to the configured authentication plugin. - if (pkiAuthenticationPlugin != null && !pkiAuthenticationPlugin.isInterceptorRegistered()) { - pkiAuthenticationPlugin.getHttpClientBuilder(HttpClientUtil.getHttpClientBuilder()); - shardHandlerFactory.setSecurityBuilder(pkiAuthenticationPlugin); - updateShardHandler.setSecurityBuilder(pkiAuthenticationPlugin); + if (pkiAuthenticationSecurityBuilder != null && !pkiAuthenticationSecurityBuilder.isInterceptorRegistered()) { + pkiAuthenticationSecurityBuilder.getHttpClientBuilder(HttpClientUtil.getHttpClientBuilder()); + shardHandlerFactory.setSecurityBuilder(pkiAuthenticationSecurityBuilder); + updateShardHandler.setSecurityBuilder(pkiAuthenticationSecurityBuilder); } } @@ -618,8 +627,8 @@ public class CoreContainer { return containerProperties; } - public PKIAuthenticationPlugin getPkiAuthenticationPlugin() { - return pkiAuthenticationPlugin; + public PKIAuthenticationPlugin getPkiAuthenticationSecurityBuilder() { + return pkiAuthenticationSecurityBuilder; } public SolrMetricManager getMetricManager() { @@ -709,10 +718,10 @@ public class CoreContainer { zkSys.initZooKeeper(this, cfg.getCloudConfig()); if (isZooKeeperAware()) { - pkiAuthenticationPlugin = new PKIAuthenticationPlugin(this, zkSys.getZkController().getNodeName(), + pkiAuthenticationSecurityBuilder = new PKIAuthenticationPlugin(this, zkSys.getZkController().getNodeName(), (PublicKeyHandler) containerHandlers.get(PublicKeyHandler.PATH)); // use deprecated API for back-compat, remove in 9.0 - pkiAuthenticationPlugin.initializeMetrics(solrMetricsContext, "/authentication/pki"); + pkiAuthenticationSecurityBuilder.initializeMetrics(solrMetricsContext, "/authentication/pki"); packageStoreAPI = new PackageStoreAPI(this); containerHandlers.getApiBag().registerObject(packageStoreAPI.readAPI); diff --git a/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java b/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java index 9cf5707..3ee8575 100644 --- a/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java @@ -145,6 +145,7 @@ public class SearchHandler extends RequestHandlerBase implements SolrCoreAware, shardHandlerFactory.close(); } }); + shardHandlerFactory.setSecurityBuilder(core.getCoreContainer().getPkiAuthenticationSecurityBuilder()); } if (core.getCoreContainer().isZooKeeperAware()) { diff --git a/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java b/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java index 8536207..43a0e95 100644 --- a/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java +++ b/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java @@ -649,8 +649,8 @@ public class HttpSolrCall { if(PublicKeyHandler.PATH.equals(path)) return false; //admin/info/key is the path where public key is exposed . it is always unsecured if ("/".equals(path) || "/solr/".equals(path)) return false; // Static Admin UI files must always be served - if (cores.getPkiAuthenticationPlugin() != null && req.getUserPrincipal() != null) { - boolean b = cores.getPkiAuthenticationPlugin().needsAuthorization(req); + if (cores.getPkiAuthenticationSecurityBuilder() != null && req.getUserPrincipal() != null) { + boolean b = cores.getPkiAuthenticationSecurityBuilder().needsAuthorization(req); log.debug("PkiAuthenticationPlugin says authorization required : {} ", b); return b; } diff --git a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java index 130ca18..7f6ac16 100644 --- a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java +++ b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java @@ -290,8 +290,8 @@ public class SolrDispatchFilter extends BaseSolrFilter implements PathExcluder { return; } String header = request.getHeader(PKIAuthenticationPlugin.HEADER); - if (header != null && cores.getPkiAuthenticationPlugin() != null) - authenticationPlugin = cores.getPkiAuthenticationPlugin(); + if (header != null && cores.getPkiAuthenticationSecurityBuilder() != null) + authenticationPlugin = cores.getPkiAuthenticationSecurityBuilder(); try { if (log.isDebugEnabled()) { log.debug("Request to authenticate: {}, domain: {}, port: {}", request, request.getLocalName(), request.getLocalPort()); diff --git a/solr/core/src/test-files/solr/configsets/cloud-minimal_override-shardHandler/conf/schema.xml b/solr/core/src/test-files/solr/configsets/cloud-minimal_override-shardHandler/conf/schema.xml new file mode 100644 index 0000000..4124fea --- /dev/null +++ b/solr/core/src/test-files/solr/configsets/cloud-minimal_override-shardHandler/conf/schema.xml @@ -0,0 +1,29 @@ +<?xml version="1.0" encoding="UTF-8" ?> +<!-- + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +--> +<schema name="minimal" version="1.1"> + <fieldType name="string" class="solr.StrField"/> + <fieldType name="int" class="${solr.tests.IntegerFieldType}" docValues="${solr.tests.numeric.dv}" precisionStep="0" omitNorms="true" positionIncrementGap="0"/> + <fieldType name="long" class="${solr.tests.LongFieldType}" docValues="${solr.tests.numeric.dv}" precisionStep="0" omitNorms="true" positionIncrementGap="0"/> + <dynamicField name="*" type="string" indexed="true" stored="true"/> + <!-- for versioning --> + <field name="_version_" type="long" indexed="true" stored="true"/> + <field name="_root_" type="string" indexed="true" stored="true" multiValued="false" required="false"/> + <field name="id" type="string" indexed="true" stored="true"/> + <dynamicField name="*_s" type="string" indexed="true" stored="true" /> + <uniqueKey>id</uniqueKey> +</schema> diff --git a/solr/core/src/test-files/solr/configsets/cloud-minimal_override-shardHandler/conf/solrconfig.xml b/solr/core/src/test-files/solr/configsets/cloud-minimal_override-shardHandler/conf/solrconfig.xml new file mode 100644 index 0000000..380da3e --- /dev/null +++ b/solr/core/src/test-files/solr/configsets/cloud-minimal_override-shardHandler/conf/solrconfig.xml @@ -0,0 +1,56 @@ +<?xml version="1.0" ?> + +<!-- + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +--> + +<!-- Minimal solrconfig.xml with /select, /admin and /update only --> + +<config> + + <dataDir>${solr.data.dir:}</dataDir> + + <directoryFactory name="DirectoryFactory" + class="${solr.directoryFactory:solr.NRTCachingDirectoryFactory}"/> + <schemaFactory class="ClassicIndexSchemaFactory"/> + + <luceneMatchVersion>${tests.luceneMatchVersion:LATEST}</luceneMatchVersion> + + <updateHandler class="solr.DirectUpdateHandler2"> + <commitWithin> + <softCommit>${solr.commitwithin.softcommit:true}</softCommit> + </commitWithin> + <updateLog class="${solr.ulog:solr.UpdateLog}"></updateLog> + </updateHandler> + + <requestHandler name="/select" class="solr.SearchHandler"> + <shardHandlerFactory class="HttpShardHandlerFactory"> + <int name="socketTimeOut">1000</int> + <int name="connTimeOut">5000</int> + <int name="corePoolSize">5</int> + </shardHandlerFactory> + <lst name="defaults"> + <str name="echoParams">explicit</str> + <str name="indent">true</str> + <str name="df">text</str> + </lst> + </requestHandler> + + <indexConfig> + <mergeScheduler class="${solr.mscheduler:org.apache.lucene.index.ConcurrentMergeScheduler}"/> +: </indexConfig> +</config> + diff --git a/solr/core/src/test/org/apache/solr/security/AuthWithShardHandlerFactoryOverrideTest.java b/solr/core/src/test/org/apache/solr/security/AuthWithShardHandlerFactoryOverrideTest.java new file mode 100644 index 0000000..6f7eba6 --- /dev/null +++ b/solr/core/src/test/org/apache/solr/security/AuthWithShardHandlerFactoryOverrideTest.java @@ -0,0 +1,116 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.solr.security; + +import org.apache.solr.client.solrj.SolrResponse; +import org.apache.solr.client.solrj.embedded.JettySolrRunner; +import org.apache.solr.client.solrj.impl.Http2SolrClient; +import org.apache.solr.client.solrj.request.CollectionAdminRequest; +import org.apache.solr.client.solrj.request.QueryRequest; +import org.apache.solr.cloud.SolrCloudAuthTestCase; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +/** + * Similar to BasicAuthOnSingleNodeTest, but using a different configset, to test SOLR-14569 + */ +public class AuthWithShardHandlerFactoryOverrideTest extends SolrCloudAuthTestCase { + + private static final String COLLECTION = "authCollection"; + private static final String ALIAS = "alias"; + + private static final String SECURITY_CONF = "{\n" + + " \"authentication\":{\n" + + " \"blockUnknown\": true,\n" + + " \"class\":\"solr.BasicAuthPlugin\",\n" + + " \"credentials\":{\"solr\":\"EEKn7ywYk5jY8vG9TyqlG2jvYuvh1Q7kCCor6Hqm320= 6zkmjMjkMKyJX6/f0VarEWQujju5BzxZXub6WOrEKCw=\"}\n" + + " },\n" + + " \"authorization\":{\n" + + " \"class\":\"solr.RuleBasedAuthorizationPlugin\",\n" + + " \"permissions\":[\n" + + " {\"name\":\"all\", \"role\":\"admin\"}\n" + + " ],\n" + + " \"user-role\":{\"solr\":\"admin\"}\n" + + " }\n" + + "}"; + + @Before + public void setupCluster() throws Exception { + configureCluster(1) + .addConfig("conf", configset("cloud-minimal_override-shardHandler")) + .withSecurityJson(SECURITY_CONF) + .configure(); + CollectionAdminRequest.createCollection(COLLECTION, "conf", 4, 1) + .setBasicAuthCredentials("solr", "solr") + .process(cluster.getSolrClient()); + + CollectionAdminRequest.createAlias(ALIAS, COLLECTION) + .setBasicAuthCredentials("solr", "solr") + .process(cluster.getSolrClient()); + + cluster.waitForActiveCollection(COLLECTION, 4, 4); + + JettySolrRunner jetty = cluster.getJettySolrRunner(0); + jetty.stop(); + cluster.waitForJettyToStop(jetty); + jetty.start(); + cluster.waitForAllNodes(30); + cluster.waitForActiveCollection(COLLECTION, 4, 4); + } + + @Override + @After + public void tearDown() throws Exception { + cluster.shutdown(); + super.tearDown(); + } + + @Test + public void collectionTest() throws Exception { + try (Http2SolrClient client = new Http2SolrClient.Builder(cluster.getJettySolrRunner(0).getBaseUrl().toString()) + .build()){ + + for (int i = 0; i < 30; i++) { + SolrResponse response = new QueryRequest(params("q", "*:*")) + .setBasicAuthCredentials("solr", "solr").process(client, COLLECTION); + // likely to be non-null, even if an error occurred + assertNotNull(response); + assertNotNull(response.getResponse()); + // now we know we have results + assertNotNull(response.getResponse().get("response")); + } + } + } + + @Test + public void aliasTest() throws Exception { + try (Http2SolrClient client = new Http2SolrClient.Builder(cluster.getJettySolrRunner(0).getBaseUrl().toString()) + .build()){ + + for (int i = 0; i < 30; i++) { + SolrResponse response = new QueryRequest(params("q", "*:*")) + .setBasicAuthCredentials("solr", "solr").process(client, ALIAS); + // likely to be non-null, even if an error occurred + assertNotNull(response); + assertNotNull(response.getResponse()); + // now we know we have results + assertNotNull(response.getResponse().get("response")); + } + } + } +}
