You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ho...@apache.org on 2019/08/18 19:21:04 UTC

[lucene-solr] branch master updated (54ab077 -> f5856ef)

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

hossman pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git.


    from 54ab077  Harden AliasIntegrationTest.testClusterStateProviderAPI
     new 251259d  SOLR-13700: Fixed a race condition when initializing metrics for new security plugins on security.json change
     new f5856ef  SOLR-13701: Fixed JWTAuthPlugin to update metrics prior to continuing w/other filters or returning error

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 solr/CHANGES.txt                                         |  4 ++++
 .../src/java/org/apache/solr/core/CoreContainer.java     | 13 ++++---------
 .../src/java/org/apache/solr/security/JWTAuthPlugin.java | 16 ++++++++--------
 .../org/apache/solr/cloud/SolrCloudAuthTestCase.java     | 15 ++++++---------
 4 files changed, 22 insertions(+), 26 deletions(-)


[lucene-solr] 02/02: SOLR-13701: Fixed JWTAuthPlugin to update metrics prior to continuing w/other filters or returning error

Posted by ho...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit f5856ef40479250abf496e684d88c54f27ee8e73
Author: Chris Hostetter <ho...@apache.org>
AuthorDate: Sun Aug 18 12:20:51 2019 -0700

    SOLR-13701: Fixed JWTAuthPlugin to update metrics prior to continuing w/other filters or returning error
---
 solr/CHANGES.txt                                         |  2 ++
 .../src/java/org/apache/solr/security/JWTAuthPlugin.java | 16 ++++++++--------
 .../org/apache/solr/cloud/SolrCloudAuthTestCase.java     | 15 ++++++---------
 3 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 3e8a747..1fa1857 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -153,6 +153,8 @@ Bug Fixes
 
 * SOLR-13700: Fixed a race condition when initializing metrics for new security plugins on security.json change (hossman)
 
+* SOLR-13701: Fixed JWTAuthPlugin to update metrics prior to continuing w/other filters or returning error (hossman)
+
 Other Changes
 ----------------------
 
diff --git a/solr/core/src/java/org/apache/solr/security/JWTAuthPlugin.java b/solr/core/src/java/org/apache/solr/security/JWTAuthPlugin.java
index 4c8c28a..c5ba67c 100644
--- a/solr/core/src/java/org/apache/solr/security/JWTAuthPlugin.java
+++ b/solr/core/src/java/org/apache/solr/security/JWTAuthPlugin.java
@@ -302,8 +302,8 @@ public class JWTAuthPlugin extends AuthenticationPlugin implements SpecProvider,
     if (jwtConsumer == null) {
       if (header == null && !blockUnknown) {
         log.info("JWTAuth not configured, but allowing anonymous access since {}==false", PARAM_BLOCK_UNKNOWN);
-        filterChain.doFilter(request, response);
         numPassThrough.inc();
+        filterChain.doFilter(request, response);
         return true;
       }
       // Retry config
@@ -342,22 +342,22 @@ public class JWTAuthPlugin extends AuthenticationPlugin implements SpecProvider,
         }
         if (log.isDebugEnabled())
           log.debug("Authentication SUCCESS");
-        filterChain.doFilter(wrapper, response);
         numAuthenticated.inc();
+        filterChain.doFilter(wrapper, response);
         return true;
 
       case PASS_THROUGH:
         if (log.isDebugEnabled())
           log.debug("Unknown user, but allow due to {}=false", PARAM_BLOCK_UNKNOWN);
-        filterChain.doFilter(request, response);
         numPassThrough.inc();
+        filterChain.doFilter(request, response);
         return true;
 
       case AUTZ_HEADER_PROBLEM:
       case JWT_PARSE_ERROR:
         log.warn("Authentication failed. {}, {}", authResponse.getAuthCode(), authResponse.getAuthCode().getMsg());
-        authenticationFailure(response, authResponse.getAuthCode().getMsg(), HttpServletResponse.SC_BAD_REQUEST, BearerWwwAuthErrorCode.invalid_request);
         numErrors.mark();
+        authenticationFailure(response, authResponse.getAuthCode().getMsg(), HttpServletResponse.SC_BAD_REQUEST, BearerWwwAuthErrorCode.invalid_request);
         return false;
 
       case CLAIM_MISMATCH:
@@ -365,25 +365,25 @@ public class JWTAuthPlugin extends AuthenticationPlugin implements SpecProvider,
       case JWT_VALIDATION_EXCEPTION:
       case PRINCIPAL_MISSING:
         log.warn("Authentication failed. {}, {}", authResponse.getAuthCode(), exceptionMessage);
-        authenticationFailure(response, authResponse.getAuthCode().getMsg(), HttpServletResponse.SC_UNAUTHORIZED, BearerWwwAuthErrorCode.invalid_token);
         numWrongCredentials.inc();
+        authenticationFailure(response, authResponse.getAuthCode().getMsg(), HttpServletResponse.SC_UNAUTHORIZED, BearerWwwAuthErrorCode.invalid_token);
         return false;
 
       case SIGNATURE_INVALID:
         log.warn("Signature validation failed: {}", exceptionMessage);
-        authenticationFailure(response, authResponse.getAuthCode().getMsg(), HttpServletResponse.SC_UNAUTHORIZED, BearerWwwAuthErrorCode.invalid_token);
         numWrongCredentials.inc();
+        authenticationFailure(response, authResponse.getAuthCode().getMsg(), HttpServletResponse.SC_UNAUTHORIZED, BearerWwwAuthErrorCode.invalid_token);
         return false;
 
       case SCOPE_MISSING:
-        authenticationFailure(response, authResponse.getAuthCode().getMsg(), HttpServletResponse.SC_UNAUTHORIZED, BearerWwwAuthErrorCode.insufficient_scope);
         numWrongCredentials.inc();
+        authenticationFailure(response, authResponse.getAuthCode().getMsg(), HttpServletResponse.SC_UNAUTHORIZED, BearerWwwAuthErrorCode.insufficient_scope);
         return false;
 
       case NO_AUTZ_HEADER:
       default:
-        authenticationFailure(response, authResponse.getAuthCode().getMsg(), HttpServletResponse.SC_UNAUTHORIZED, null);
         numMissingCredentials.inc();
+        authenticationFailure(response, authResponse.getAuthCode().getMsg(), HttpServletResponse.SC_UNAUTHORIZED, null);
         return false;
     }
   }
diff --git a/solr/test-framework/src/java/org/apache/solr/cloud/SolrCloudAuthTestCase.java b/solr/test-framework/src/java/org/apache/solr/cloud/SolrCloudAuthTestCase.java
index 1c4adbf..9485c80 100644
--- a/solr/test-framework/src/java/org/apache/solr/cloud/SolrCloudAuthTestCase.java
+++ b/solr/test-framework/src/java/org/apache/solr/cloud/SolrCloudAuthTestCase.java
@@ -119,16 +119,13 @@ public class SolrCloudAuthTestCase extends SolrCloudTestCase {
     expectedCounts.put("failMissingCredentials", (long) failMissingCredentials);
     expectedCounts.put("errors", (long) errors);
 
-    Map<String, Long> counts = countSecurityMetrics(cluster, prefix, AUTH_METRICS_KEYS);
-    boolean success = isMetricsEqualOrLarger(AUTH_METRICS_TO_COMPARE, expectedCounts, counts);
-    if (!success) {
-      log.info("First metrics count assert failed, pausing 2s before re-attempt");
-      Thread.sleep(2000);
-      counts = countSecurityMetrics(cluster, prefix, AUTH_METRICS_KEYS);
-      success = isMetricsEqualOrLarger(AUTH_METRICS_TO_COMPARE, expectedCounts, counts);
-    }
+    final Map<String, Long> counts = countSecurityMetrics(cluster, prefix, AUTH_METRICS_KEYS);
+    final boolean success = isMetricsEqualOrLarger(AUTH_METRICS_TO_COMPARE, expectedCounts, counts);
     
-    assertTrue("Expected metric minimums for prefix " + prefix + ": " + expectedCounts + ", but got: " + counts, success);
+    assertTrue("Expected metric minimums for prefix " + prefix + ": " + expectedCounts +
+               ", but got: " + counts + "(Possible cause is delay in loading modified " +
+               "security.json; see SOLR-13464 for test work around)",
+               success);
     
     if (counts.get("requests") > 0) {
       assertTrue("requestTimes count not > 1", counts.get("requestTimes") > 1);


[lucene-solr] 01/02: SOLR-13700: Fixed a race condition when initializing metrics for new security plugins on security.json change

Posted by ho...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 251259d5abf94578b51da3efd05327da72667e7e
Author: Chris Hostetter <ho...@apache.org>
AuthorDate: Sun Aug 18 12:18:33 2019 -0700

    SOLR-13700: Fixed a race condition when initializing metrics for new security plugins on security.json change
---
 solr/CHANGES.txt                                           |  2 ++
 solr/core/src/java/org/apache/solr/core/CoreContainer.java | 13 ++++---------
 2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index a5cfc9d..3e8a747 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -151,6 +151,8 @@ Bug Fixes
 
 * SOLR-13694: IndexSizeEstimator NullPointerException. (ab, hossman)
 
+* SOLR-13700: Fixed a race condition when initializing metrics for new security plugins on security.json change (hossman)
+
 Other Changes
 ----------------------
 
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 d6098fb..0098cc8 100644
--- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java
+++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
@@ -395,6 +395,7 @@ public class CoreContainer {
           getResourceLoader().newInstance(klas, AuditLoggerPlugin.class));
 
       newAuditloggerPlugin.plugin.init(auditConf);
+      newAuditloggerPlugin.plugin.initializeMetrics(metricManager, SolrInfoBean.Group.node.toString(), metricTag, "/auditlogging");
     } else {
       log.debug("Security conf doesn't exist. Skipping setup for audit logging module.");
     }
@@ -451,6 +452,8 @@ public class CoreContainer {
     if (authenticationPlugin != null) {
       authenticationPlugin.plugin.init(authenticationConfig);
       setupHttpClientForAuthPlugin(authenticationPlugin.plugin);
+      authenticationPlugin.plugin.initializeMetrics
+        (metricManager, SolrInfoBean.Group.node.toString(), metricTag, "/authentication");
     }
     this.authenticationPlugin = authenticationPlugin;
     try {
@@ -628,6 +631,7 @@ public class CoreContainer {
       getZkController().getZkStateReader().registerClusterPropertiesListener(clusterPropertiesListener);
       pkiAuthenticationPlugin = new PKIAuthenticationPlugin(this, zkSys.getZkController().getNodeName(),
           (PublicKeyHandler) containerHandlers.get(PublicKeyHandler.PATH));
+      pkiAuthenticationPlugin.initializeMetrics(metricManager, SolrInfoBean.Group.node.toString(), metricTag, "/authentication/pki");
       TracerConfigurator.loadTracer(loader, cfg.getTracerConfiguratorPluginInfo(), getZkController().getZkStateReader());
     }
 
@@ -871,16 +875,7 @@ public class CoreContainer {
     SecurityConfHandler.SecurityConfig securityConfig = securityConfHandler.getSecurityConfig(false);
     initializeAuthorizationPlugin((Map<String, Object>) securityConfig.getData().get("authorization"));
     initializeAuthenticationPlugin((Map<String, Object>) securityConfig.getData().get("authentication"));
-    if (authenticationPlugin != null) {
-      authenticationPlugin.plugin.initializeMetrics(metricManager, SolrInfoBean.Group.node.toString(), metricTag, "/authentication");
-    }
-    if (pkiAuthenticationPlugin != null && pkiAuthenticationPlugin.getMetricRegistry() == null) {
-      pkiAuthenticationPlugin.initializeMetrics(metricManager, SolrInfoBean.Group.node.toString(), metricTag, "/authentication/pki");
-    }
     initializeAuditloggerPlugin((Map<String, Object>) securityConfig.getData().get("auditlogging"));
-    if (auditloggerPlugin != null) {
-      auditloggerPlugin.plugin.initializeMetrics(metricManager, SolrInfoBean.Group.node.toString(), metricTag, "/auditlogging");
-    }
   }
 
   private static void checkForDuplicateCoreNames(List<CoreDescriptor> cds) {