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/11/26 20:04:24 UTC
[lucene-solr] 03/04: SOLR-13905 Make findRequestType in AuditEvent
more robust (#1014)
This is an automated email from the ASF dual-hosted git repository.
janhoy pushed a commit to branch branch_8_3
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
commit e6d46a758dbf33c03e49e927cce773868546c2f5
Author: Jan Høydahl <ja...@apache.org>
AuthorDate: Fri Nov 22 23:34:20 2019 +0100
SOLR-13905 Make findRequestType in AuditEvent more robust (#1014)
(cherry picked from commit e45c5ce9b9e70650f119976b8b2d91b3c760cb48)
---
solr/CHANGES.txt | 2 +
.../java/org/apache/solr/security/AuditEvent.java | 47 ++++++++++++----------
.../solr/security/AuditLoggerIntegrationTest.java | 14 +++++++
.../solr/security/AuditLoggerPluginTest.java | 22 +++++++++-
4 files changed, 63 insertions(+), 22 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index fb5b15c..66616fe 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -37,6 +37,8 @@ Bug Fixes
* SOLR-13941: Tests configure Jetty differently than when running via start.jar (janhoy, Uwe Schindler)
+* SOLR-13905: Make findRequestType in AuditEvent more robust (janhoy)
+
================== 8.3.0 ==================
Consult the LUCENE_CHANGES.txt file for additional, low level, changes in this release.
diff --git a/solr/core/src/java/org/apache/solr/security/AuditEvent.java b/solr/core/src/java/org/apache/solr/security/AuditEvent.java
index 4fee14b..17db340 100644
--- a/solr/core/src/java/org/apache/solr/security/AuditEvent.java
+++ b/solr/core/src/java/org/apache/solr/security/AuditEvent.java
@@ -131,7 +131,6 @@ public class AuditEvent {
this.solrPort = httpRequest.getLocalPort();
this.solrIp = httpRequest.getLocalAddr();
this.clientIp = httpRequest.getRemoteAddr();
- this.resource = ServletUtils.getPathAfterContext(httpRequest);
this.httpMethod = httpRequest.getMethod();
this.httpQueryString = httpRequest.getQueryString();
this.headers = getHeadersFromRequest(httpRequest);
@@ -141,6 +140,7 @@ public class AuditEvent {
this.solrParams.put(sp.getKey(), Arrays.asList(sp.getValue()));
});
+ setResource(ServletUtils.getPathAfterContext(httpRequest));
setRequestType(findRequestType());
if (exception != null) setException(exception);
@@ -167,7 +167,7 @@ public class AuditEvent {
this(eventType, httpRequest);
this.collections = authorizationContext.getCollectionRequests()
.stream().map(r -> r.collectionName).collect(Collectors.toList());
- this.resource = authorizationContext.getResource();
+ setResource(authorizationContext.getResource());
this.requestType = RequestType.convertType(authorizationContext.getRequestType());
authorizationContext.getParams().forEach(p -> {
this.solrParams.put(p.getKey(), Arrays.asList(p.getValue()));
@@ -380,7 +380,7 @@ public class AuditEvent {
}
public AuditEvent setResource(String resource) {
- this.resource = resource;
+ this.resource = normalizeResourcePath(resource);
return this;
}
@@ -452,30 +452,35 @@ public class AuditEvent {
}
private RequestType findRequestType() {
- if (ADMIN_PATH_REGEXES.stream().map(Pattern::compile)
- .anyMatch(p -> p.matcher(resource).matches())) return RequestType.ADMIN;
- if (SEARCH_PATH_REGEXES.stream().map(Pattern::compile)
- .anyMatch(p -> p.matcher(resource).matches())) return RequestType.SEARCH;
- if (INDEXING_PATH_REGEXES.stream().map(Pattern::compile)
- .anyMatch(p -> p.matcher(resource).matches())) return RequestType.UPDATE;
- if (STREAMING_PATH_REGEXES.stream().map(Pattern::compile)
- .anyMatch(p -> p.matcher(resource).matches())) return RequestType.STREAMING;
+ if (resource == null) return RequestType.UNKNOWN;
+ if (SEARCH_PATH_PATTERNS.stream().anyMatch(p -> p.matcher(resource).matches())) return RequestType.SEARCH;
+ if (INDEXING_PATH_PATTERNS.stream().anyMatch(p -> p.matcher(resource).matches())) return RequestType.UPDATE;
+ if (STREAMING_PATH_PATTERNS.stream().anyMatch(p -> p.matcher(resource).matches())) return RequestType.STREAMING;
+ if (ADMIN_PATH_PATTERNS.stream().anyMatch(p -> p.matcher(resource).matches())) return RequestType.ADMIN;
return RequestType.UNKNOWN;
}
-
+
+ protected String normalizeResourcePath(String resourcePath) {
+ if (resourcePath == null) return "";
+ return resourcePath.replaceFirst("^/____v2", "/api");
+ }
+
private static final List<String> ADMIN_PATH_REGEXES = Arrays.asList(
"^/admin/.*",
- "^/(____v2|api)/(c|collections)$",
- "^/(____v2|api)/(c|collections)/[^/]+/config$",
- "^/(____v2|api)/(c|collections)/[^/]+/schema$",
- "^/(____v2|api)/(c|collections)/[^/]+/shards.*",
- "^/(____v2|api)/cores.*$",
- "^/(____v2|api)/node$",
- "^/(____v2|api)/cluster$");
+ "^/api/(c|collections)$",
+ "^/api/(c|collections)/[^/]+/config$",
+ "^/api/(c|collections)/[^/]+/schema$",
+ "^/api/(c|collections)/[^/]+/shards.*",
+ "^/api/cores.*$",
+ "^/api/node.*$",
+ "^/api/cluster.*$");
private static final List<String> STREAMING_PATH_REGEXES = Collections.singletonList(".*/stream.*");
-
private static final List<String> INDEXING_PATH_REGEXES = Collections.singletonList(".*/update.*");
-
private static final List<String> SEARCH_PATH_REGEXES = Arrays.asList(".*/select.*", ".*/query.*");
+
+ private static final List<Pattern> ADMIN_PATH_PATTERNS = ADMIN_PATH_REGEXES.stream().map(Pattern::compile).collect(Collectors.toList());
+ private static final List<Pattern> STREAMING_PATH_PATTERNS = STREAMING_PATH_REGEXES.stream().map(Pattern::compile).collect(Collectors.toList());
+ private static final List<Pattern> INDEXING_PATH_PATTERNS = INDEXING_PATH_REGEXES.stream().map(Pattern::compile).collect(Collectors.toList());
+ private static final List<Pattern> SEARCH_PATH_PATTERNS = SEARCH_PATH_REGEXES.stream().map(Pattern::compile).collect(Collectors.toList());
}
diff --git a/solr/core/src/test/org/apache/solr/security/AuditLoggerIntegrationTest.java b/solr/core/src/test/org/apache/solr/security/AuditLoggerIntegrationTest.java
index 9df09c5..6e5664f 100644
--- a/solr/core/src/test/org/apache/solr/security/AuditLoggerIntegrationTest.java
+++ b/solr/core/src/test/org/apache/solr/security/AuditLoggerIntegrationTest.java
@@ -17,11 +17,13 @@
package org.apache.solr.security;
import java.io.BufferedReader;
+import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStreamReader;
import java.lang.invoke.MethodHandles;
import java.net.ServerSocket;
import java.net.Socket;
+import java.net.URL;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Arrays;
@@ -38,6 +40,7 @@ import com.codahale.metrics.Timer;
import com.fasterxml.jackson.databind.DeserializationFeature;
import com.fasterxml.jackson.databind.ObjectMapper;
import org.apache.commons.io.FileUtils;
+import org.apache.commons.io.IOUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.lucene.util.TestUtil;
import org.apache.solr.SolrTestCaseJ4;
@@ -217,6 +220,17 @@ public class AuditLoggerIntegrationTest extends SolrCloudAuthTestCase {
}
@Test
+ public void illegalAdminPathError() throws Exception {
+ setupCluster(false, null, false);
+ String baseUrl = testHarness.get().cluster.getJettySolrRunner(0).getBaseUrl().toString();
+ expectThrows(FileNotFoundException.class, () -> {
+ IOUtils.toString(new URL(baseUrl.replace("/solr", "") + "/api/node/foo"), StandardCharsets.UTF_8);
+ });
+ final List<AuditEvent> events = testHarness.get().receiver.waitForAuditEvents(1);
+ assertAuditEvent(events.get(0), ERROR, "/api/node/foo", ADMIN, null, 404);
+ }
+
+ @Test
public void authValid() throws Exception {
setupCluster(false, null, true);
final CloudSolrClient client = testHarness.get().cluster.getSolrClient();
diff --git a/solr/core/src/test/org/apache/solr/security/AuditLoggerPluginTest.java b/solr/core/src/test/org/apache/solr/security/AuditLoggerPluginTest.java
index 738a24d..4606482 100644
--- a/solr/core/src/test/org/apache/solr/security/AuditLoggerPluginTest.java
+++ b/solr/core/src/test/org/apache/solr/security/AuditLoggerPluginTest.java
@@ -91,6 +91,18 @@ public class AuditLoggerPluginTest extends SolrTestCaseJ4 {
.setDate(SAMPLE_DATE)
.setCollections(Collections.singletonList("streamcoll"))
.setResource("/stream");
+ protected static final AuditEvent EVENT_HEALTH_API = new AuditEvent(AuditEvent.EventType.COMPLETED)
+ .setUsername("Jan")
+ .setHttpMethod("GET")
+ .setMessage("Healthy")
+ .setDate(SAMPLE_DATE)
+ .setResource("/api/node/health");
+ protected static final AuditEvent EVENT_HEALTH_V2 = new AuditEvent(AuditEvent.EventType.COMPLETED)
+ .setUsername("Jan")
+ .setHttpMethod("GET")
+ .setMessage("Healthy")
+ .setDate(SAMPLE_DATE)
+ .setResource("/____v2/node/health");
private MockAuditLoggerPlugin plugin;
private HashMap<String, Object> config;
@@ -134,6 +146,7 @@ public class AuditLoggerPluginTest extends SolrTestCaseJ4 {
assertFalse(plugin.shouldLog(EVENT_ANONYMOUS.getEventType()));
assertFalse(plugin.shouldLog(EVENT_AUTHENTICATED.getEventType()));
assertFalse(plugin.shouldLog(EVENT_AUTHORIZED.getEventType()));
+ assertFalse(plugin.shouldLog(EVENT_AUTHORIZED.getEventType()));
}
@Test(expected = SolrException.class)
@@ -167,7 +180,14 @@ public class AuditLoggerPluginTest extends SolrTestCaseJ4 {
assertEquals(1, plugin.typeCounts.getOrDefault("REJECTED", new AtomicInteger()).get());
assertEquals(2, plugin.events.size());
}
-
+
+ @Test
+ public void v2ApiPath() {
+ assertEquals("/api/node/health", EVENT_HEALTH_API.getResource());
+ // /____v2/ is mapped to /api/
+ assertEquals("/api/node/health", EVENT_HEALTH_V2.getResource());
+ }
+
@Test
public void jsonEventFormatter() {
assertEquals("{\"message\":\"Anonymous\",\"level\":\"INFO\",\"date\":" + SAMPLE_DATE.getTime() + ",\"solrParams\":{},\"solrPort\":0,\"resource\":\"/collection1\",\"httpMethod\":\"GET\",\"eventType\":\"ANONYMOUS\",\"status\":-1,\"qtime\":-1.0}",