Skip to content

Commit

Permalink
Fix tests
Browse files Browse the repository at this point in the history
  • Loading branch information
kfaraz committed Dec 8, 2023
1 parent 6144091 commit 5e5a2fc
Show file tree
Hide file tree
Showing 8 changed files with 149 additions and 136 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import com.google.inject.Inject;
import com.sun.jersey.spi.container.ResourceFilters;
import org.apache.druid.audit.AuditEntry;
import org.apache.druid.audit.AuditInfo;
import org.apache.druid.audit.AuditManager;
import org.apache.druid.guice.LazySingleton;
import org.apache.druid.security.basic.BasicSecurityResourceFilter;
Expand Down Expand Up @@ -161,11 +160,7 @@ public Response createUser(
authValidator.validateAuthenticatorName(authenticatorName);

final Response response = handler.createUser(authenticatorName, userName);

if (isSuccess(response)) {
final AuditInfo auditInfo = AuthorizationUtils.buildAuditInfo(req);
performAudit(authenticatorName, "users.create", Collections.singletonMap("username", userName), auditInfo);
}
performAudit(authenticatorName, "basicAuth.createUser", userName, req, response);

return response;
}
Expand All @@ -191,11 +186,7 @@ public Response deleteUser(
{
authValidator.validateAuthenticatorName(authenticatorName);
final Response response = handler.deleteUser(authenticatorName, userName);

if (isSuccess(response)) {
final AuditInfo auditInfo = AuthorizationUtils.buildAuditInfo(req);
performAudit(authenticatorName, "users.delete", Collections.singletonMap("username", userName), auditInfo);
}
performAudit(authenticatorName, "basicAuth.deleteUser", userName, req, response);

return response;
}
Expand All @@ -222,11 +213,7 @@ public Response updateUserCredentials(
{
authValidator.validateAuthenticatorName(authenticatorName);
final Response response = handler.updateUserCredentials(authenticatorName, userName, update);

if (isSuccess(response)) {
final AuditInfo auditInfo = AuthorizationUtils.buildAuditInfo(req);
performAudit(authenticatorName, "users.update", Collections.singletonMap("username", userName), auditInfo);
}
performAudit(authenticatorName, "basicAuth.updateUserCreds", userName, req, response);

return response;
}
Expand Down Expand Up @@ -278,15 +265,24 @@ private boolean isSuccess(Response response)
return responseCode >= 200 && responseCode < 300;
}

private void performAudit(String authenticatorName, String action, Object payload, AuditInfo auditInfo)
private void performAudit(
String authenticatorName,
String action,
String updatedUser,
HttpServletRequest request,
Response response
)
{
auditManager.doAudit(
AuditEntry.builder()
.key(authenticatorName)
.type(action)
.auditInfo(auditInfo)
.payload(payload)
.build()
);
if (isSuccess(response)) {
auditManager.doAudit(
AuditEntry.builder()
.key(authenticatorName)
.type(action)
.auditInfo(AuthorizationUtils.buildAuditInfo(request))
.request(AuthorizationUtils.buildRequestInfo("coordinator", request))
.payload(Collections.singletonMap("username", updatedUser))
.build()
);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,7 @@ public Response taskPost(
AuditEntry.builder()
.key(task.getDataSource())
.type("ingest.batch")
.request(AuthorizationUtils.buildRequestInfo("overlord", req))
.payload(new TaskIdentifier(task.getId(), task.getGroupId(), task.getType()))
.auditInfo(auditInfo)
.build()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,10 @@ public void setUp() throws Exception
EasyMock.expect(req.getAttribute(AuthConfig.DRUID_AUTHENTICATION_RESULT)).andReturn(
new AuthenticationResult("druid", "druid", null, null)
).anyTimes();
EasyMock.expect(req.getMethod()).andReturn("GET").anyTimes();
EasyMock.expect(req.getRequestURI()).andReturn("/request/uri").anyTimes();
EasyMock.expect(req.getQueryString()).andReturn("query=string").anyTimes();

EasyMock.expect(req.getAttribute(AuthConfig.DRUID_ALLOW_UNSECURED_PATH)).andReturn(null).anyTimes();
EasyMock.expect(req.getAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED)).andReturn(null).anyTimes();
req.setAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED, true);
Expand Down Expand Up @@ -264,13 +268,14 @@ public void testOverlordRun() throws Exception
final TaskStorageQueryAdapter taskStorageQueryAdapter = new TaskStorageQueryAdapter(taskStorage, taskLockbox, taskMaster);
final WorkerTaskRunnerQueryAdapter workerTaskRunnerQueryAdapter = new WorkerTaskRunnerQueryAdapter(taskMaster, null);
// Test Overlord resource stuff
AuditManager auditManager = EasyMock.createNiceMock(AuditManager.class);
overlordResource = new OverlordResource(
taskMaster,
taskStorageQueryAdapter,
new IndexerMetadataStorageAdapter(taskStorageQueryAdapter, null),
null,
null,
null,
auditManager,
AuthTestUtils.TEST_AUTHORIZER_MAPPER,
workerTaskRunnerQueryAdapter,
null,
Expand Down
16 changes: 8 additions & 8 deletions processing/src/main/java/org/apache/druid/audit/RequestInfo.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,20 +31,20 @@ public class RequestInfo
{
private final String service;
private final String method;
private final String path;
private final String uri;
private final String queryParams;

@JsonCreator
public RequestInfo(
@JsonProperty("service") String service,
@JsonProperty("method") String method,
@JsonProperty("path") String path,
@JsonProperty("uri") String uri,
@JsonProperty("queryParams") String queryParams
)
{
this.service = service;
this.method = method;
this.path = path;
this.uri = uri;
this.queryParams = queryParams;
}

Expand All @@ -61,9 +61,9 @@ public String getMethod()
}

@JsonProperty
public String getPath()
public String getUri()
{
return path;
return uri;
}

@JsonProperty
Expand All @@ -84,14 +84,14 @@ public boolean equals(Object o)
RequestInfo that = (RequestInfo) o;
return Objects.equals(this.service, that.service)
&& Objects.equals(this.method, that.method)
&& Objects.equals(this.path, that.path)
&& Objects.equals(this.uri, that.uri)
&& Objects.equals(this.queryParams, that.queryParams);
}

@Override
public int hashCode()
{
return Objects.hash(service, method, path, queryParams);
return Objects.hash(service, method, uri, queryParams);
}

@Override
Expand All @@ -100,7 +100,7 @@ public String toString()
return "RequestInfo{" +
"service='" + service + '\'' +
", method='" + method + '\'' +
", path='" + path + '\'' +
", path='" + uri + '\'' +
", queryParams='" + queryParams + '\'' +
'}';
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public enum Level
}

private static final String MSG_FORMAT
= "User[%s], identity[%s], IP[%s] performed action[%s] on key[%s] with comment[%s]. Payload[%s].";
= "User[%s], identity[%s], IP[%s] performed action[%s] on key[%s] with comment[%s]. Request[%s], payload[%s].";

private final Level level;
private final Logger logger = new Logger(AuditLogger.class);
Expand All @@ -49,6 +49,7 @@ public void log(AuditEntry entry)
entry.getType(),
entry.getKey(),
entry.getAuditInfo().getComment(),
entry.getRequest(),
entry.getPayload()
};
switch (level) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -253,16 +253,15 @@ public Response markSegmentsAsUnused(
);
auditPayload = Collections.singletonMap("segmentIds", segmentIds);
}
if (auditManager != null) {
auditManager.doAudit(
AuditEntry.builder()
.key(dataSourceName)
.type("segments.markUnused")
.payload(auditPayload)
.auditInfo(AuthorizationUtils.buildAuditInfo(req))
.build()
);
}
auditManager.doAudit(
AuditEntry.builder()
.key(dataSourceName)
.type("markSegmentsAsUnused")
.payload(auditPayload)
.auditInfo(AuthorizationUtils.buildAuditInfo(req))
.request(AuthorizationUtils.buildRequestInfo("coordinator", req))
.build()
);
return numUpdatedSegments;
};
return doMarkSegmentsWithPayload("markSegmentsAsUnused", dataSourceName, payload, markSegments);
Expand Down Expand Up @@ -373,16 +372,15 @@ public Response killUnusedSegmentsInInterval(
overlordClient.runKillTask("api-issued", dataSourceName, theInterval, null),
true
);
if (auditManager != null) {
auditManager.doAudit(
AuditEntry.builder()
.key(dataSourceName)
.type("segments.killTask")
.payload(ImmutableMap.of("killTaskId", killTaskId, "interval", theInterval))
.auditInfo(AuthorizationUtils.buildAuditInfo(req))
.build()
);
}
auditManager.doAudit(
AuditEntry.builder()
.key(dataSourceName)
.type("killUnusedSegmentsInInterval")
.payload(ImmutableMap.of("killTaskId", killTaskId, "interval", theInterval))
.auditInfo(AuthorizationUtils.buildAuditInfo(req))
.request(AuthorizationUtils.buildRequestInfo("coordinator", req))
.build()
);
return Response.ok().build();
}
catch (Exception e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.google.common.collect.Lists;
import org.apache.druid.audit.AuditInfo;
import org.apache.druid.audit.AuditManager;
import org.apache.druid.audit.RequestInfo;
import org.apache.druid.error.DruidException;
import org.apache.druid.java.util.common.ISE;

Expand Down Expand Up @@ -130,6 +131,16 @@ public static AuditInfo buildAuditInfo(HttpServletRequest request)
);
}

public static RequestInfo buildRequestInfo(String service, HttpServletRequest request)
{
return new RequestInfo(
service,
request.getMethod(),
request.getRequestURI(),
request.getQueryString()
);
}

/**
* Check a list of resource-actions to be performed by the identity represented by authenticationResult.
*
Expand Down
Loading

0 comments on commit 5e5a2fc

Please sign in to comment.