Fix failing test

kfaraz · Dec 19, 2023 · c87fb13 · c87fb13
1 parent 0b73b5f
commit c87fb13
Show file tree

Hide file tree

Showing 8 changed files with 33 additions and 25 deletions.
diff --git a/...a/org/apache/druid/security/basic/authentication/endpoint/BasicAuthenticatorResource.java b/...a/org/apache/druid/security/basic/authentication/endpoint/BasicAuthenticatorResource.java
@@ -25,6 +25,7 @@
 import org.apache.druid.audit.AuditEntry;
 import org.apache.druid.audit.AuditManager;
 import org.apache.druid.guice.LazySingleton;
+import org.apache.druid.java.util.common.StringUtils;
 import org.apache.druid.security.basic.BasicSecurityResourceFilter;
 import org.apache.druid.security.basic.authentication.entity.BasicAuthenticatorCredentialUpdate;
 import org.apache.druid.server.security.AuthValidator;
@@ -41,7 +42,6 @@
 import javax.ws.rs.core.Context;
 import javax.ws.rs.core.MediaType;
 import javax.ws.rs.core.Response;
-import java.util.Collections;
 
 @Path("/druid-ext/basic-security/authentication")
 @LazySingleton
@@ -160,7 +160,7 @@ public Response createUser(
     authValidator.validateAuthenticatorName(authenticatorName);
 
     final Response response = handler.createUser(authenticatorName, userName);
-    performAuditIfSuccess(authenticatorName, userName, req, response);
+    performAuditIfSuccess(authenticatorName, req, response, "Create user[%s]", userName);
 
     return response;
   }
@@ -186,7 +186,7 @@ public Response deleteUser(
   {
     authValidator.validateAuthenticatorName(authenticatorName);
     final Response response = handler.deleteUser(authenticatorName, userName);
-    performAuditIfSuccess(authenticatorName, userName, req, response);
+    performAuditIfSuccess(authenticatorName, req, response, "Delete user[%s]", userName);
 
     return response;
   }
@@ -213,7 +213,7 @@ public Response updateUserCredentials(
   {
     authValidator.validateAuthenticatorName(authenticatorName);
     final Response response = handler.updateUserCredentials(authenticatorName, userName, update);
-    performAuditIfSuccess(authenticatorName, userName, req, response);
+    performAuditIfSuccess(authenticatorName, req, response, "Update credentials for user[%s]", userName);
 
     return response;
   }
@@ -267,19 +267,20 @@ private boolean isSuccess(Response response)
 
   private void performAuditIfSuccess(
       String authenticatorName,
-      String updatedUser,
       HttpServletRequest request,
-      Response response
+      Response response,
+      String payloadFormat,
+      Object... payloadArgs
   )
   {
-    if (updatedUser != null && isSuccess(response)) {
+    if (isSuccess(response)) {
       auditManager.doAudit(
           AuditEntry.builder()
                     .key(authenticatorName)
-                    .type("basicAuthentication")
+                    .type("basic.authenticator")
                     .auditInfo(AuthorizationUtils.buildAuditInfo(request))
                     .request(AuthorizationUtils.buildRequestInfo("coordinator", request))
-                    .payload(Collections.singletonMap("username", updatedUser))
+                    .payload(StringUtils.format(payloadFormat, payloadArgs))
                     .build()
       );
     }

diff --git a/.../java/org/apache/druid/security/basic/authorization/endpoint/BasicAuthorizerResource.java b/.../java/org/apache/druid/security/basic/authorization/endpoint/BasicAuthorizerResource.java
@@ -690,7 +690,7 @@ private void performAuditIfSuccess(
       auditManager.doAudit(
           AuditEntry.builder()
                     .key(authorizerName)
-                    .type("basicAuthorization")
+                    .type("basic.authorizer")
                     .auditInfo(AuthorizationUtils.buildAuditInfo(request))
                     .request(AuthorizationUtils.buildRequestInfo("coordinator", request))
                     .payload(StringUtils.format(msgFormat, args))

diff --git a/...src/test/java/org/apache/druid/metadata/storage/mysql/MySQLMetadataStorageModuleTest.java b/...src/test/java/org/apache/druid/metadata/storage/mysql/MySQLMetadataStorageModuleTest.java
@@ -32,6 +32,7 @@
 import org.apache.druid.guice.LifecycleModule;
 import org.apache.druid.guice.MetadataConfigModule;
 import org.apache.druid.guice.annotations.Json;
+import org.apache.druid.guice.security.EscalatorModule;
 import org.apache.druid.java.util.emitter.core.NoopEmitter;
 import org.apache.druid.java.util.emitter.service.ServiceEmitter;
 import org.junit.Assert;
@@ -111,6 +112,7 @@ private Injector createInjector()
     MySQLMetadataStorageModule module = new MySQLMetadataStorageModule();
     Injector injector = GuiceInjectors.makeStartupInjectorWithModules(
         ImmutableList.of(
+            new EscalatorModule(),
             new MetadataConfigModule(),
             new LifecycleModule(),
             module,

diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/OverlordResource.java b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/OverlordResource.java
@@ -24,6 +24,7 @@
 import com.google.common.base.Optional;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 import com.google.inject.Inject;
@@ -138,6 +139,8 @@ public class OverlordResource
 
   private AtomicReference<WorkerBehaviorConfig> workerConfigRef = null;
   private static final List<String> API_TASK_STATES = ImmutableList.of("pending", "waiting", "running", "complete");
+  private static final Set<String> AUDITED_TASK_TYPES
+      = ImmutableSet.of("index", "index_parallel", "compact", "index_hadoop");
 
   private enum TaskStateLookup
   {
@@ -223,15 +226,17 @@ public Response taskPost(
           try {
             taskQueue.add(task);
 
-            auditManager.doAudit(
-                AuditEntry.builder()
-                          .key(task.getDataSource())
-                          .type("task")
-                          .request(AuthorizationUtils.buildRequestInfo("overlord", req))
-                          .payload(new TaskIdentifier(task.getId(), task.getGroupId(), task.getType()))
-                          .auditInfo(AuthorizationUtils.buildAuditInfo(req))
-                          .build()
-            );
+            if (AUDITED_TASK_TYPES.contains(task.getType())) {
+              auditManager.doAudit(
+                  AuditEntry.builder()
+                            .key(task.getDataSource())
+                            .type("task")
+                            .request(AuthorizationUtils.buildRequestInfo("overlord", req))
+                            .payload(new TaskIdentifier(task.getId(), task.getGroupId(), task.getType()))
+                            .auditInfo(AuthorizationUtils.buildAuditInfo(req))
+                            .build()
+              );
+            }
 
             return Response.ok(ImmutableMap.of("task", task.getId())).build();
           }

diff --git a/server/src/main/java/org/apache/druid/server/audit/LoggingAuditManagerConfig.java b/server/src/main/java/org/apache/druid/server/audit/LoggingAuditManagerConfig.java
@@ -52,7 +52,7 @@ public LoggingAuditManagerConfig(
   )
   {
     this.logLevel = Configs.valueOrDefault(logLevel, AuditLogger.Level.INFO);
-    this.auditSystemRequests = Configs.valueOrDefault(auditSystemRequests, false);
+    this.auditSystemRequests = Configs.valueOrDefault(auditSystemRequests, true);
     this.maxPayloadSizeBytes = Configs.valueOrDefault(maxPayloadSizeBytes, HumanReadableBytes.valueOf(-1));
     this.skipNullField = Configs.valueOrDefault(skipNullField, false);
   }

diff --git a/server/src/main/java/org/apache/druid/server/audit/SQLAuditManagerConfig.java b/server/src/main/java/org/apache/druid/server/audit/SQLAuditManagerConfig.java
@@ -57,7 +57,7 @@ public SQLAuditManagerConfig(
       @JsonProperty("includePayloadAsDimensionInMetric") Boolean includePayloadAsDimensionInMetric
   )
   {
-    this.auditSystemRequests = Configs.valueOrDefault(auditSystemRequests, false);
+    this.auditSystemRequests = Configs.valueOrDefault(auditSystemRequests, true);
     this.maxPayloadSizeBytes = Configs.valueOrDefault(maxPayloadSizeBytes, HumanReadableBytes.valueOf(-1));
     this.skipNullField = Configs.valueOrDefault(skipNullField, false);
     this.auditHistoryMillis = Configs.valueOrDefault(auditHistoryMillis, 7 * 24 * 60 * 60 * 1000L);

diff --git a/server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java b/server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java
@@ -256,7 +256,7 @@ public Response markSegmentsAsUnused(
       auditManager.doAudit(
           AuditEntry.builder()
                     .key(dataSourceName)
-                    .type("markSegmentsAsUnused")
+                    .type("segment.markUnused")
                     .payload(auditPayload)
                     .auditInfo(AuthorizationUtils.buildAuditInfo(req))
                     .request(AuthorizationUtils.buildRequestInfo("coordinator", req))
@@ -375,7 +375,7 @@ public Response killUnusedSegmentsInInterval(
       auditManager.doAudit(
           AuditEntry.builder()
                     .key(dataSourceName)
-                    .type("killUnusedSegmentsInInterval")
+                    .type("segment.kill")
                     .payload(ImmutableMap.of("killTaskId", killTaskId, "interval", theInterval))
                     .auditInfo(AuthorizationUtils.buildAuditInfo(req))
                     .request(AuthorizationUtils.buildRequestInfo("coordinator", req))

diff --git a/server/src/test/java/org/apache/druid/server/audit/AuditManagerConfigTest.java b/server/src/test/java/org/apache/druid/server/audit/AuditManagerConfigTest.java
@@ -47,7 +47,7 @@ public void testDefaultAuditConfig()
     Assert.assertTrue(config instanceof SQLAuditManagerConfig);
 
     final SQLAuditManagerConfig sqlAuditConfig = (SQLAuditManagerConfig) config;
-    Assert.assertFalse(sqlAuditConfig.isAuditSystemRequests());
+    Assert.assertTrue(sqlAuditConfig.isAuditSystemRequests());
     Assert.assertFalse(sqlAuditConfig.isSkipNullField());
     Assert.assertFalse(sqlAuditConfig.isIncludePayloadAsDimensionInMetric());
     Assert.assertEquals(-1, sqlAuditConfig.getMaxPayloadSizeBytes());
@@ -71,7 +71,7 @@ public void testLogAuditConfigWithDefaults()
     Assert.assertTrue(config instanceof LoggingAuditManagerConfig);
 
     final LoggingAuditManagerConfig logAuditConfig = (LoggingAuditManagerConfig) config;
-    Assert.assertFalse(logAuditConfig.isAuditSystemRequests());
+    Assert.assertTrue(logAuditConfig.isAuditSystemRequests());
     Assert.assertFalse(logAuditConfig.isSkipNullField());
     Assert.assertEquals(-1, logAuditConfig.getMaxPayloadSizeBytes());
     Assert.assertEquals(AuditLogger.Level.INFO, logAuditConfig.getLogLevel());