From 337ccdd2592a9c97f1a03a4ac5efcec203709edf Mon Sep 17 00:00:00 2001 From: Amrit Sarkar Date: Thu, 8 Oct 2026 21:47:01 -0700 Subject: [PATCH] SOLR-18010: Fix security.json corruption from parallel edits In standalone mode, two security edits running at the same time could undo each other or leave security.json as invalid JSON, and Solr then fails to start. The Admin UI does this when creating a role with permissions, since it sends one request per permission all at once. Security edits in standalone mode now run one at a time. --- .../SOLR-18010-security-json-corruption.yml | 7 +++ .../admin/SecurityConfHandlerLocal.java | 13 +++++ .../security/BasicAuthStandaloneTest.java | 55 +++++++++++++++++++ 3 files changed, 75 insertions(+) create mode 100644 changelog/unreleased/SOLR-18010-security-json-corruption.yml diff --git a/changelog/unreleased/SOLR-18010-security-json-corruption.yml b/changelog/unreleased/SOLR-18010-security-json-corruption.yml new file mode 100644 index 000000000000..251218632156 --- /dev/null +++ b/changelog/unreleased/SOLR-18010-security-json-corruption.yml @@ -0,0 +1,7 @@ +title: Concurrent security edits in standalone mode, such as creating a role with permissions in the Admin UI, no longer corrupt security.json. +type: fixed +authors: + - name: Amrit Sarkar +links: + - name: SOLR-18010 + url: https://issues.apache.org/jira/browse/SOLR-18010 diff --git a/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandlerLocal.java b/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandlerLocal.java index 79d86c936f9a..a5d951968001 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandlerLocal.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandlerLocal.java @@ -28,6 +28,7 @@ import org.apache.solr.common.util.CommandOperation; import org.apache.solr.common.util.Utils; import org.apache.solr.core.CoreContainer; +import org.apache.solr.request.SolrQueryRequest; import org.apache.solr.response.SolrQueryResponse; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -36,12 +37,24 @@ public class SecurityConfHandlerLocal extends SecurityConfHandler { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); protected Path securityJsonPath; + private final Object securityJsonLock = new Object(); public SecurityConfHandlerLocal(CoreContainer coreContainer) { super(coreContainer); securityJsonPath = coreContainer.getSolrHome().resolve("security.json"); } + /** + * Serializes requests, since a local file has no versioned write to detect concurrent edits the + * way ZooKeeper does. Reads are included so they never see a partially written file. + */ + @Override + public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception { + synchronized (securityJsonLock) { + super.handleRequestBody(req, rsp); + } + } + /** * Fetches security props from SOLR_HOME * diff --git a/solr/core/src/test/org/apache/solr/security/BasicAuthStandaloneTest.java b/solr/core/src/test/org/apache/solr/security/BasicAuthStandaloneTest.java index bdd263211837..35ff50dd0507 100644 --- a/solr/core/src/test/org/apache/solr/security/BasicAuthStandaloneTest.java +++ b/solr/core/src/test/org/apache/solr/security/BasicAuthStandaloneTest.java @@ -23,10 +23,18 @@ import java.io.IOException; import java.nio.file.Path; +import java.util.ArrayList; import java.util.Base64; +import java.util.List; import java.util.Map; +import java.util.concurrent.CyclicBarrier; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; import org.apache.solr.SolrTestCaseJ4; import org.apache.solr.common.params.MapSolrParams; +import org.apache.solr.common.util.ExecutorUtil; +import org.apache.solr.common.util.SolrNamedThreadFactory; import org.apache.solr.common.util.Utils; import org.apache.solr.embedded.JettySolrRunner; import org.apache.solr.handler.admin.SecurityConfHandler; @@ -124,6 +132,53 @@ public void testBasicAuth() throws Exception { } } + @Test + public void testConcurrentEdits() throws Exception { + String authzPrefix = "/admin/authorization"; + HttpClient httpClient = jetty.getSolrClient().getHttpClient(); + String url = buildUrl(jetty.getLocalPort()) + authzPrefix; + + securityConfHandler.persistConf( + new SecurityConfHandler.SecurityConfig() + .setData(Utils.fromJSONString(STD_CONF.replace("'", "\"")))); + securityConfHandler.securityConfEdited(); + verifySecurityStatus(httpClient, url, "authorization.enabled", "true", 20); + + int numEdits = 20; + // Release all edits at once so they overlap, like the Admin UI's per-permission POSTs. The + // await timeout fails the test instead of hanging if the pool ever has fewer threads than + // numEdits. + CyclicBarrier barrier = new CyclicBarrier(numEdits); + ExecutorService executor = + ExecutorUtil.newMDCAwareFixedThreadPool( + numEdits, new SolrNamedThreadFactory("securityEdit")); + try { + List> futures = new ArrayList<>(); + for (int i = 0; i < numEdits; i++) { + String command = "{'set-user-role': {'user" + i + "': ['dev']}}"; + futures.add( + executor.submit( + () -> { + barrier.await(30, TimeUnit.SECONDS); + doHttpPost(httpClient, url, command, "solr", "SolrRocks"); + return null; + })); + } + for (Future future : futures) { + future.get(); + } + } finally { + ExecutorUtil.shutdownAndAwaitTermination(executor); + } + + Map authz = + (Map) securityConfHandler.getSecurityConfig(false).getData().get("authorization"); + Map userRoles = (Map) authz.get("user-role"); + for (int i = 0; i < numEdits; i++) { + assertTrue("missing user" + i + " in " + userRoles, userRoles.containsKey("user" + i)); + } + } + static void doHttpPost( HttpClient cl, String url, String jsonCommand, String basicUser, String basicPass) throws IOException {