You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ja...@apache.org on 2019/04/15 20:04:29 UTC

[lucene-solr] branch branch_7_7 updated: SOLR-12371: Editing authorization config via REST API now works in standalone mode

This is an automated email from the ASF dual-hosted git repository.

janhoy pushed a commit to branch branch_7_7
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git


The following commit(s) were added to refs/heads/branch_7_7 by this push:
     new 7c26fdd  SOLR-12371: Editing authorization config via REST API now works in standalone mode
7c26fdd is described below

commit 7c26fdd45a5d2752a3397458f87ecb5b519bec0a
Author: Jan Høydahl <ja...@apache.org>
AuthorDate: Mon Apr 15 21:09:30 2019 +0200

    SOLR-12371: Editing authorization config via REST API now works in standalone mode
    
    (cherry picked from commit 9707bb6fa91ea3062a404690aa7d041771f22746)
---
 solr/CHANGES.txt                                   |  2 +
 .../java/org/apache/solr/core/CoreContainer.java   | 73 ++++++++++++----------
 .../solr/security/BasicAuthStandaloneTest.java     | 61 +++++++++++-------
 3 files changed, 81 insertions(+), 55 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index a82325f..d8fd618 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -69,6 +69,8 @@ Bug fixes
 
 * SOLR-13285: Updates with enum fields and javabin cause ClassCastException (noble)
 
+* SOLR-12371: Editing authorization config via REST API now works in standalone mode (janhoy)
+
 ==================  7.7.1 ==================
 
 Consult the LUCENE_CHANGES.txt file for additional, low level, changes in this release.
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 be879b8..faa2f0e 100644
--- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java
+++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
@@ -325,6 +325,7 @@ public class CoreContainer {
 
   private synchronized void initializeAuthorizationPlugin(Map<String, Object> authorizationConf) {
     authorizationConf = Utils.getDeepCopy(authorizationConf, 4);
+    int newVersion = readVersion(authorizationConf);
     //Initialize the Authorization module
     SecurityPluginHolder<AuthorizationPlugin> old = authorizationPlugin;
     SecurityPluginHolder<AuthorizationPlugin> authorizationPlugin = null;
@@ -333,11 +334,12 @@ public class CoreContainer {
       if (klas == null) {
         throw new SolrException(ErrorCode.SERVER_ERROR, "class is required for authorization plugin");
       }
-      if (old != null && old.getZnodeVersion() == readVersion(authorizationConf)) {
+      if (old != null && old.getZnodeVersion() == newVersion && newVersion > 0) {
+        log.debug("Authorization config not modified");
         return;
       }
       log.info("Initializing authorization plugin: " + klas);
-      authorizationPlugin = new SecurityPluginHolder<>(readVersion(authorizationConf),
+      authorizationPlugin = new SecurityPluginHolder<>(newVersion,
           getResourceLoader().newInstance(klas, AuthorizationPlugin.class));
 
       // Read and pass the authorization context to the plugin
@@ -350,12 +352,14 @@ public class CoreContainer {
       try {
         old.plugin.close();
       } catch (Exception e) {
+        log.error("Exception while attempting to close old authorization plugin", e);
       }
     }
   }
 
   private synchronized void initializeAuthenticationPlugin(Map<String, Object> authenticationConfig) {
     authenticationConfig = Utils.getDeepCopy(authenticationConfig, 4);
+    int newVersion = readVersion(authenticationConfig);
     String pluginClassName = null;
     if (authenticationConfig != null) {
       if (authenticationConfig.containsKey("class")) {
@@ -377,10 +381,15 @@ public class CoreContainer {
     SecurityPluginHolder<AuthenticationPlugin> old = authenticationPlugin;
     SecurityPluginHolder<AuthenticationPlugin> authenticationPlugin = null;
 
+    if (old != null && old.getZnodeVersion() == newVersion && newVersion > 0) {
+      log.debug("Authentication config not modified");
+      return;
+    }
+
     // Initialize the plugin
     if (pluginClassName != null) {
       log.info("Initializing authentication plugin: " + pluginClassName);
-      authenticationPlugin = new SecurityPluginHolder<>(readVersion(authenticationConfig),
+      authenticationPlugin = new SecurityPluginHolder<>(newVersion,
           getResourceLoader().newInstance(pluginClassName,
               AuthenticationPlugin.class,
               null,
@@ -402,7 +411,7 @@ public class CoreContainer {
     if (authcPlugin instanceof HttpClientBuilderPlugin) {
       // Setup HttpClient for internode communication
       SolrHttpClientBuilder builder = ((HttpClientBuilderPlugin) authcPlugin).getHttpClientBuilder(HttpClientUtil.getHttpClientBuilder());
-      
+
       // 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.
@@ -411,7 +420,7 @@ public class CoreContainer {
       SolrHttpClientContextBuilder httpClientBuilder = new SolrHttpClientContextBuilder();
       if (builder.getCredentialsProviderProvider() != null) {
         httpClientBuilder.setDefaultCredentialsProvider(new CredentialsProviderProvider() {
-          
+
           @Override
           public CredentialsProvider getCredentialsProvider() {
             return builder.getCredentialsProviderProvider().getCredentialsProvider();
@@ -502,7 +511,7 @@ public class CoreContainer {
   public OrderedExecutor getReplayUpdatesExecutor() {
     return replayUpdatesExecutor;
   }
-  
+
   public MetricsHandler getMetricsHandler() {
     return metricsHandler;
   }
@@ -853,7 +862,7 @@ public class CoreContainer {
       }
 
       ExecutorUtil.shutdownAndAwaitTermination(coreContainerWorkExecutor);
-      
+
       // First wake up the closer thread, it'll terminate almost immediately since it checks isShutDown.
       synchronized (solrCores.getModifyLock()) {
         solrCores.getModifyLock().notifyAll(); // wake up anyone waiting
@@ -885,7 +894,7 @@ public class CoreContainer {
       synchronized (solrCores.getModifyLock()) {
         solrCores.getModifyLock().notifyAll(); // wake up the thread
       }
-      
+
       customThreadPool.submit(() -> {
         replayUpdatesExecutor.shutdownAndAwaitTermination();
       });
@@ -1093,8 +1102,8 @@ public class CoreContainer {
 
       return core;
     } catch (Exception ex) {
-      // First clean up any core descriptor, there should never be an existing core.properties file for any core that 
-      // failed to be created on-the-fly. 
+      // First clean up any core descriptor, there should never be an existing core.properties file for any core that
+      // failed to be created on-the-fly.
       coresLocator.delete(this, cd);
       if (isZooKeeperAware() && !preExisitingZkEntry) {
         try {
@@ -1226,7 +1235,7 @@ public class CoreContainer {
   private ConfigSet getConfigSet(CoreDescriptor cd) {
     return coreConfigService.getConfig(cd);
   }
-  
+
   /**
    * Take action when we failed to create a SolrCore. If error is due to corrupt index, try to recover. Various recovery
    * strategies can be specified via system properties "-DCoreInitFailedAction={fromleader, none}"
@@ -1252,13 +1261,13 @@ public class CoreContainer {
         break;
       }
     }
-    
+
     // If no CorruptIndexException, nothing we can try here
     if (cause == null) throw original;
-    
+
     CoreInitFailedAction action = CoreInitFailedAction.valueOf(System.getProperty(CoreInitFailedAction.class.getSimpleName(), "none"));
     log.debug("CorruptIndexException while creating core, will attempt to repair via {}", action);
-    
+
     switch (action) {
       case fromleader: // Recovery from leader on a CorruptedIndexException
         if (isZooKeeperAware()) {
@@ -1358,13 +1367,13 @@ public class CoreContainer {
   }
 
   /**
-   * Returns an immutable Map of Exceptions that occured when initializing 
-   * SolrCores (either at startup, or do to runtime requests to create cores) 
-   * keyed off of the name (String) of the SolrCore that had the Exception 
+   * Returns an immutable Map of Exceptions that occured when initializing
+   * SolrCores (either at startup, or do to runtime requests to create cores)
+   * keyed off of the name (String) of the SolrCore that had the Exception
    * during initialization.
    * <p>
-   * While the Map returned by this method is immutable and will not change 
-   * once returned to the client, the source data used to generate this Map 
+   * While the Map returned by this method is immutable and will not change
+   * once returned to the client, the source data used to generate this Map
    * can be changed as various SolrCore operations are performed:
    * </p>
    * <ul>
@@ -1415,7 +1424,7 @@ public class CoreContainer {
    * Recreates a SolrCore.
    * While the new core is loading, requests will continue to be dispatched to
    * and processed by the old core
-   * 
+   *
    * @param name the name of the SolrCore to reload
    */
   public void reload(String name) {
@@ -1515,7 +1524,7 @@ public class CoreContainer {
   public void unload(String name, boolean deleteIndexDir, boolean deleteDataDir, boolean deleteInstanceDir) {
 
     CoreDescriptor cd = solrCores.getCoreDescriptor(name);
-    
+
     if (name != null) {
       // check for core-init errors first
       CoreLoadFailure loadFailure = coreInitFailures.remove(name);
@@ -1532,7 +1541,7 @@ public class CoreContainer {
         return;
       }
     }
-      
+
     if (cd == null) {
       throw new SolrException(ErrorCode.BAD_REQUEST, "Cannot unload non-existent core [" + name + "]");
     }
@@ -1560,7 +1569,7 @@ public class CoreContainer {
         zkSys.getZkController().stopReplicationFromLeader(name);
       }
     }
-    
+
     core.unloadOnClose(cd, deleteIndexDir, deleteDataDir, deleteInstanceDir);
     if (close)
       core.closeAndWait();
@@ -1675,17 +1684,17 @@ public class CoreContainer {
   public BlobRepository getBlobRepository(){
     return blobRepository;
   }
-  
+
   /**
    * If using asyncSolrCoreLoad=true, calling this after {@link #load()} will
    * not return until all cores have finished loading.
-   * 
+   *
    * @param timeoutMs timeout, upon which method simply returns
    */
   public void waitForLoadingCoresToFinish(long timeoutMs) {
     solrCores.waitForLoadingCoresToFinish(timeoutMs);
   }
-  
+
   public void waitForLoadingCore(String name, long timeoutMs) {
     solrCores.waitForLoadingCoreToFinish(name, timeoutMs);
   }
@@ -1768,11 +1777,11 @@ public class CoreContainer {
   public boolean isZooKeeperAware() {
     return zkSys.getZkController() != null;
   }
-  
+
   public ZkController getZkController() {
     return zkSys.getZkController();
   }
-  
+
   public NodeConfig getConfig() {
     return cfg;
   }
@@ -1781,7 +1790,7 @@ public class CoreContainer {
   public ShardHandlerFactory getShardHandlerFactory() {
     return shardHandlerFactory;
   }
-  
+
   public UpdateShardHandler getUpdateShardHandler() {
     return updateShardHandler;
   }
@@ -1789,7 +1798,7 @@ public class CoreContainer {
   public SolrResourceLoader getResourceLoader() {
     return loader;
   }
-  
+
   public boolean isCoreLoading(String name) {
     return solrCores.isCoreLoading(name);
   }
@@ -1809,7 +1818,7 @@ public class CoreContainer {
   public long getStatus() {
     return status;
   }
-  
+
   // Occasaionally we need to access the transient cache handler in places other than coreContainer.
   public TransientSolrCoreCache getTransientCache() {
     return solrCores.getTransientCacheHandler();
@@ -1871,7 +1880,7 @@ public class CoreContainer {
         getZkController().giveupLeadership(solrCore.getCoreDescriptor(), tragicException);
       }
     }
-    
+
     return tragicException != null;
   }
 
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 da77b22..9ce7be8 100644
--- a/solr/core/src/test/org/apache/solr/security/BasicAuthStandaloneTest.java
+++ b/solr/core/src/test/org/apache/solr/security/BasicAuthStandaloneTest.java
@@ -16,11 +16,13 @@
  */
 package org.apache.solr.security;
 
+import java.io.IOException;
 import java.lang.invoke.MethodHandles;
 import java.nio.charset.Charset;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.util.Collections;
 import java.util.Properties;
 
 import org.apache.http.HttpResponse;
@@ -29,19 +31,15 @@ import org.apache.http.client.methods.HttpPost;
 import org.apache.http.entity.ByteArrayEntity;
 import org.apache.http.message.AbstractHttpMessage;
 import org.apache.http.message.BasicHeader;
-import org.apache.solr.client.solrj.SolrRequest;
+import org.apache.solr.SolrTestCaseJ4;
 import org.apache.solr.client.solrj.embedded.JettySolrRunner;
 import org.apache.solr.client.solrj.impl.HttpClientUtil;
 import org.apache.solr.client.solrj.impl.HttpSolrClient;
-import org.apache.solr.client.solrj.request.GenericSolrRequest;
-import org.apache.solr.client.solrj.request.RequestWriter.StringPayloadContentWriter;
-import org.apache.solr.common.params.CommonParams;
-import org.apache.solr.common.params.ModifiableSolrParams;
+import org.apache.solr.common.params.MapSolrParams;
 import org.apache.solr.common.util.Base64;
 import org.apache.solr.common.util.Utils;
 import org.apache.solr.handler.admin.SecurityConfHandler;
 import org.apache.solr.handler.admin.SecurityConfHandlerLocalForTesting;
-import org.apache.solr.SolrTestCaseJ4;
 import org.apache.solr.util.LogLevel;
 import org.junit.After;
 import org.junit.Before;
@@ -88,6 +86,7 @@ public class BasicAuthStandaloneTest extends SolrTestCaseJ4 {
   public void testBasicAuth() throws Exception {
 
     String authcPrefix = "/admin/authentication";
+    String authzPrefix = "/admin/authorization";
 
     HttpClient cl = null;
     HttpSolrClient httpSolrClient = null;
@@ -108,34 +107,35 @@ public class BasicAuthStandaloneTest extends SolrTestCaseJ4 {
           "'set-user': {'harry':'HarryIsCool'}\n" +
           "}";
 
-      GenericSolrRequest genericReq = new GenericSolrRequest(SolrRequest.METHOD.POST, authcPrefix, new ModifiableSolrParams());
-      genericReq.setContentWriter(new StringPayloadContentWriter(command, CommonParams.JSON_MIME));
-
-      HttpSolrClient finalHttpSolrClient = httpSolrClient;
-      HttpSolrClient.RemoteSolrException exp = expectThrows(HttpSolrClient.RemoteSolrException.class, () -> {
-        finalHttpSolrClient.request(genericReq);
-      });
-      assertEquals(401, exp.code());
+      doHttpPost(cl, baseUrl + authcPrefix, command, null, null, 401);
+      verifySecurityStatus(cl, baseUrl + authcPrefix, "authentication.enabled", "true", 20);
 
       command = "{\n" +
           "'set-user': {'harry':'HarryIsUberCool'}\n" +
           "}";
 
-      HttpPost httpPost = new HttpPost(baseUrl + authcPrefix);
-      setBasicAuthHeader(httpPost, "solr", "SolrRocks");
-      httpPost.setEntity(new ByteArrayEntity(command.getBytes(UTF_8)));
-      httpPost.addHeader("Content-Type", "application/json; charset=UTF-8");
-      verifySecurityStatus(cl, baseUrl + authcPrefix, "authentication.enabled", "true", 20);
-      HttpResponse r = cl.execute(httpPost);
-      int statusCode = r.getStatusLine().getStatusCode();
-      Utils.consumeFully(r.getEntity());
-      assertEquals("proper_cred sent, but access denied", 200, statusCode);
 
+      doHttpPost(cl, baseUrl + authcPrefix, command, "solr", "SolrRocks");
       verifySecurityStatus(cl, baseUrl + authcPrefix, "authentication/credentials/harry", NOT_NULL_PREDICATE, 20);
 
       // Read file from SOLR_HOME and verify that it contains our new user
       assertTrue(new String(Utils.toJSON(securityConfHandler.getSecurityConfig(false).getData()), 
           Charset.forName("UTF-8")).contains("harry"));
+
+      // Edit authorization
+      verifySecurityStatus(cl, baseUrl + authzPrefix, "authorization/permissions[1]/role", null, 20);
+      doHttpPost(cl, baseUrl + authzPrefix, "{'set-permission': {'name': 'update', 'role':'updaterole'}}", "solr", "SolrRocks");
+      command = "{\n" +
+          "'set-permission': {'name': 'read', 'role':'solr'}\n" +
+          "}";
+      doHttpPost(cl, baseUrl + authzPrefix, command, "solr", "SolrRocks");
+      try {
+        httpSolrClient.query("collection1", new MapSolrParams(Collections.singletonMap("q", "foo")));
+        fail("Should return a 401 response");
+      } catch (Exception e) {
+        // Test that the second doPost request to /security/authorization went through
+        verifySecurityStatus(cl, baseUrl + authzPrefix, "authorization/permissions[2]/role", "solr", 20);
+      }
     } finally {
       if (cl != null) {
         HttpClientUtil.close(cl);
@@ -144,6 +144,21 @@ public class BasicAuthStandaloneTest extends SolrTestCaseJ4 {
     }
   }
 
+  private void doHttpPost(HttpClient cl, String url, String jsonCommand, String basicUser, String basicPass) throws IOException {
+    doHttpPost(cl, url, jsonCommand, basicUser, basicPass, 200);
+  }
+
+  private void doHttpPost(HttpClient cl, String url, String jsonCommand, String basicUser, String basicPass, int expectStatusCode) throws IOException {
+    HttpPost httpPost = new HttpPost(url);
+    setBasicAuthHeader(httpPost, basicUser, basicPass);
+    httpPost.setEntity(new ByteArrayEntity(jsonCommand.replaceAll("'", "\"").getBytes(UTF_8)));
+    httpPost.addHeader("Content-Type", "application/json; charset=UTF-8");
+    HttpResponse r = cl.execute(httpPost);
+    int statusCode = r.getStatusLine().getStatusCode();
+    Utils.consumeFully(r.getEntity());
+    assertEquals("proper_cred sent, but access denied", expectStatusCode, statusCode);
+  }
+
   public static void setBasicAuthHeader(AbstractHttpMessage httpMsg, String user, String pwd) {
     String userPass = user + ":" + pwd;
     String encoded = Base64.byteArrayToBase64(userPass.getBytes(UTF_8));