Skip to content

Commit

Permalink
Audit some more endpoints
Browse files Browse the repository at this point in the history
  • Loading branch information
@kfaraz
kfaraz committed Dec 4, 2023
1 parent f418435 commit 5adc89a
Show file tree
Hide file tree
Showing 4 changed files with 113 additions and 40 deletions.
78 changes: 71 additions & 7 deletions ...a/org/apache/druid/security/basic/authentication/endpoint/BasicAuthenticatorResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@
import com.fasterxml.jackson.jaxrs.smile.SmileMediaTypes;
import com.google.inject.Inject;
import com.sun.jersey.spi.container.ResourceFilters;
import org.apache.druid.audit.AuditEvent;
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;
import org.apache.druid.security.basic.authentication.entity.BasicAuthenticatorCredentialUpdate;
Expand All @@ -30,30 +33,36 @@
import javax.servlet.http.HttpServletRequest;
import javax.ws.rs.Consumes;
import javax.ws.rs.DELETE;
import javax.ws.rs.DefaultValue;
import javax.ws.rs.GET;
import javax.ws.rs.HeaderParam;
import javax.ws.rs.POST;
import javax.ws.rs.Path;
import javax.ws.rs.PathParam;
import javax.ws.rs.Produces;
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
public class BasicAuthenticatorResource
{
private final BasicAuthenticatorResourceHandler handler;
private final AuthValidator authValidator;
private final AuditManager auditManager;

@Inject
public BasicAuthenticatorResource(
BasicAuthenticatorResourceHandler handler,
AuthValidator authValidator
AuthValidator authValidator,
AuditManager auditManager
)
{
this.handler = handler;
this.authValidator = authValidator;
this.auditManager = auditManager;
}

/**
Expand Down Expand Up @@ -137,6 +146,8 @@ public Response getUser(
* @param req HTTP request
* @param userName Name to assign the new user
*
* @param author
* @param comment
* @return OK response, or 400 error response if user already exists
*/
@POST
Expand All @@ -147,11 +158,21 @@ public Response getUser(
public Response createUser(
@Context HttpServletRequest req,
@PathParam("authenticatorName") final String authenticatorName,
@PathParam("userName") String userName
@PathParam("userName") String userName,
@HeaderParam(AuditManager.X_DRUID_AUTHOR) @DefaultValue("") final String author,
@HeaderParam(AuditManager.X_DRUID_COMMENT) @DefaultValue("") final String comment
)
{
authValidator.validateAuthenticatorName(authenticatorName);
return handler.createUser(authenticatorName, userName);

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

if (isSuccess(response)) {
final AuditInfo auditInfo = new AuditInfo(author, comment, req.getRemoteAddr());
performAudit(authenticatorName, "users.create", Collections.singletonMap("username", userName), auditInfo);
}

return response;
}

/**
Expand All @@ -160,6 +181,8 @@ public Response createUser(
* @param req HTTP request
* @param userName Name of user to delete
*
* @param author
* @param comment
* @return OK response, or 400 error response if user doesn't exist
*/
@DELETE
Expand All @@ -170,11 +193,20 @@ public Response createUser(
public Response deleteUser(
@Context HttpServletRequest req,
@PathParam("authenticatorName") final String authenticatorName,
@PathParam("userName") String userName
@PathParam("userName") String userName,
@HeaderParam(AuditManager.X_DRUID_AUTHOR) @DefaultValue("") final String author,
@HeaderParam(AuditManager.X_DRUID_COMMENT) @DefaultValue("") final String comment
)
{
authValidator.validateAuthenticatorName(authenticatorName);
return handler.deleteUser(authenticatorName, userName);
final Response response = handler.deleteUser(authenticatorName, userName);

if (isSuccess(response)) {
final AuditInfo auditInfo = new AuditInfo(author, comment, req.getRemoteAddr());
performAudit(authenticatorName, "users.delete", Collections.singletonMap("username", userName), auditInfo);
}

return response;
}

/**
Expand All @@ -183,6 +215,8 @@ public Response deleteUser(
* @param req HTTP request
* @param userName Name of user
*
* @param author
* @param comment
* @return OK response, 400 error if user doesn't exist
*/
@POST
Expand All @@ -194,11 +228,20 @@ public Response updateUserCredentials(
@Context HttpServletRequest req,
@PathParam("authenticatorName") final String authenticatorName,
@PathParam("userName") String userName,
BasicAuthenticatorCredentialUpdate update
BasicAuthenticatorCredentialUpdate update,
@HeaderParam(AuditManager.X_DRUID_AUTHOR) @DefaultValue("") final String author,
@HeaderParam(AuditManager.X_DRUID_COMMENT) @DefaultValue("") final String comment
)
{
authValidator.validateAuthenticatorName(authenticatorName);
return handler.updateUserCredentials(authenticatorName, userName, update);
final Response response = handler.updateUserCredentials(authenticatorName, userName, update);

if (isSuccess(response)) {
final AuditInfo auditInfo = new AuditInfo(author, comment, req.getRemoteAddr());
performAudit(authenticatorName, "users.update", Collections.singletonMap("username", userName), auditInfo);
}

return response;
}

/**
Expand Down Expand Up @@ -237,4 +280,25 @@ public Response authenticatorUpdateListener(
authValidator.validateAuthenticatorName(authenticatorName);
return handler.authenticatorUserUpdateListener(authenticatorName, serializedUserMap);
}

private boolean isSuccess(Response response) {
if (response == null) {
return false;
}

int responseCode = response.getStatus();
return responseCode >= 200 && responseCode < 300;
}

private void performAudit(String authenticatorName, String action, Object payload, AuditInfo auditInfo)
{
auditManager.doAudit(
AuditEvent.builder()
.key(authenticatorName)
.type(action)
.auditInfo(auditInfo)
.payload(payload)
.build()
);
}
}
41 changes: 24 additions & 17 deletions ...a/org/apache/druid/security/authentication/CoordinatorBasicAuthenticatorResourceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import com.fasterxml.jackson.dataformat.smile.SmileFactory;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import org.apache.druid.audit.AuditManager;
import org.apache.druid.metadata.DefaultPasswordProvider;
import org.apache.druid.metadata.MetadataStorageTablesConfig;
import org.apache.druid.metadata.TestDerbyConnector;
Expand Down Expand Up @@ -68,6 +69,9 @@ public class CoordinatorBasicAuthenticatorResourceTest

@Mock
private AuthValidator authValidator;

@Mock
private AuditManager auditManager;
private BasicAuthenticatorResource resource;
private CoordinatorBasicAuthenticatorMetadataStorageUpdater storageUpdater;
private HttpServletRequest req;
Expand Down Expand Up @@ -145,7 +149,8 @@ public void setUp()
authenticatorMapper,
objectMapper
),
authValidator
authValidator,
auditManager
);

storageUpdater.start();
Expand Down Expand Up @@ -175,9 +180,9 @@ public void testGetAllUsers()
Assert.assertEquals(200, response.getStatus());
Assert.assertEquals(ImmutableSet.of(BasicAuthUtils.ADMIN_NAME, BasicAuthUtils.INTERNAL_USER_NAME), response.getEntity());

resource.createUser(req, AUTHENTICATOR_NAME, "druid");
resource.createUser(req, AUTHENTICATOR_NAME, "druid2");
resource.createUser(req, AUTHENTICATOR_NAME, "druid3");
resource.createUser(req, AUTHENTICATOR_NAME, "druid", null, null);
resource.createUser(req, AUTHENTICATOR_NAME, "druid2", null, null);
resource.createUser(req, AUTHENTICATOR_NAME, "druid3", null, null);

Set<String> expectedUsers = ImmutableSet.of(
BasicAuthUtils.ADMIN_NAME,
Expand Down Expand Up @@ -215,13 +220,13 @@ public void testGetAllUsersSeparateDatabaseTables()
Assert.assertEquals(200, response.getStatus());
Assert.assertEquals(ImmutableSet.of(BasicAuthUtils.ADMIN_NAME, BasicAuthUtils.INTERNAL_USER_NAME), response.getEntity());

resource.createUser(req, AUTHENTICATOR_NAME, "druid");
resource.createUser(req, AUTHENTICATOR_NAME, "druid2");
resource.createUser(req, AUTHENTICATOR_NAME, "druid3");
resource.createUser(req, AUTHENTICATOR_NAME, "druid", null, null);
resource.createUser(req, AUTHENTICATOR_NAME, "druid2", null, null);
resource.createUser(req, AUTHENTICATOR_NAME, "druid3", null, null);

resource.createUser(req, AUTHENTICATOR_NAME2, "druid4");
resource.createUser(req, AUTHENTICATOR_NAME2, "druid5");
resource.createUser(req, AUTHENTICATOR_NAME2, "druid6");
resource.createUser(req, AUTHENTICATOR_NAME2, "druid4", null, null);
resource.createUser(req, AUTHENTICATOR_NAME2, "druid5", null, null);
resource.createUser(req, AUTHENTICATOR_NAME2, "druid6", null, null);

Set<String> expectedUsers = ImmutableSet.of(
BasicAuthUtils.ADMIN_NAME,
Expand Down Expand Up @@ -285,15 +290,15 @@ public void testGetAllUsersSeparateDatabaseTables()
@Test
public void testCreateDeleteUser()
{
Response response = resource.createUser(req, AUTHENTICATOR_NAME, "druid");
Response response = resource.createUser(req, AUTHENTICATOR_NAME, "druid", null, null);
Assert.assertEquals(200, response.getStatus());

response = resource.getUser(req, AUTHENTICATOR_NAME, "druid");
Assert.assertEquals(200, response.getStatus());
BasicAuthenticatorUser expectedUser = new BasicAuthenticatorUser("druid", null);
Assert.assertEquals(expectedUser, response.getEntity());

response = resource.deleteUser(req, AUTHENTICATOR_NAME, "druid");
response = resource.deleteUser(req, AUTHENTICATOR_NAME, "druid", null, null);
Assert.assertEquals(200, response.getStatus());

response = resource.getCachedSerializedUserMap(req, AUTHENTICATOR_NAME);
Expand All @@ -303,7 +308,7 @@ public void testCreateDeleteUser()
Assert.assertNotNull(cachedUserMap);
Assert.assertNull(cachedUserMap.get("druid"));

response = resource.deleteUser(req, AUTHENTICATOR_NAME, "druid");
response = resource.deleteUser(req, AUTHENTICATOR_NAME, "druid", null, null);
Assert.assertEquals(400, response.getStatus());
Assert.assertEquals(errorMapWithMsg("User [druid] does not exist."), response.getEntity());

Expand All @@ -315,14 +320,15 @@ public void testCreateDeleteUser()
@Test
public void testUserCredentials()
{
Response response = resource.createUser(req, AUTHENTICATOR_NAME, "druid");
Response response = resource.createUser(req, AUTHENTICATOR_NAME, "druid", null, null);
Assert.assertEquals(200, response.getStatus());

response = resource.updateUserCredentials(
req,
AUTHENTICATOR_NAME,
"druid",
new BasicAuthenticatorCredentialUpdate("helloworld", null)
new BasicAuthenticatorCredentialUpdate("helloworld", null),
null, null
);
Assert.assertEquals(200, response.getStatus());

Expand Down Expand Up @@ -369,7 +375,7 @@ public void testUserCredentials()
);
Assert.assertArrayEquals(recalculatedHash, hash);

response = resource.deleteUser(req, AUTHENTICATOR_NAME, "druid");
response = resource.deleteUser(req, AUTHENTICATOR_NAME, "druid", null, null);
Assert.assertEquals(200, response.getStatus());

response = resource.getUser(req, AUTHENTICATOR_NAME, "druid");
Expand All @@ -380,7 +386,8 @@ public void testUserCredentials()
req,
AUTHENTICATOR_NAME,
"druid",
new BasicAuthenticatorCredentialUpdate("helloworld", null)
new BasicAuthenticatorCredentialUpdate("helloworld", null),
null, null
);
Assert.assertEquals(400, response.getStatus());
Assert.assertEquals(errorMapWithMsg("User [druid] does not exist."), response.getEntity());
Expand Down
14 changes: 7 additions & 7 deletions ...g/apache/druid/security/basic/authentication/endpoint/BasicAuthenticatorResourceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public void setUp()
.when(authValidator)
.validateAuthenticatorName(INVALID_AUTHENTICATOR_NAME);

target = new BasicAuthenticatorResource(handler, authValidator);
target = new BasicAuthenticatorResource(handler, authValidator, null);
}

@Test
Expand Down Expand Up @@ -88,37 +88,37 @@ public void getCachedSerializedUserMapWithInvalidAuthenticatorNameShouldReturnEx
@Test
public void updateUserCredentialsShouldReturnExpectedResponse()
{
Assert.assertNotNull(target.updateUserCredentials(req, AUTHENTICATOR_NAME, USER_NAME, update));
Assert.assertNotNull(target.updateUserCredentials(req, AUTHENTICATOR_NAME, USER_NAME, update, null, null));
}

@Test(expected = IllegalArgumentException.class)
public void updateUserCredentialsWithInvalidAuthenticatorNameShouldReturnExpectedResponse()
{
target.updateUserCredentials(req, INVALID_AUTHENTICATOR_NAME, USER_NAME, update);
target.updateUserCredentials(req, INVALID_AUTHENTICATOR_NAME, USER_NAME, update, null, null);
}

@Test
public void deleteUserShouldReturnExpectedResponse()
{
Assert.assertNotNull(target.deleteUser(req, AUTHENTICATOR_NAME, USER_NAME));
Assert.assertNotNull(target.deleteUser(req, AUTHENTICATOR_NAME, USER_NAME, null, null));
}

@Test(expected = IllegalArgumentException.class)
public void deleteUserWithInvalidAuthenticatorNameShouldReturnExpectedResponse()
{
target.deleteUser(req, INVALID_AUTHENTICATOR_NAME, USER_NAME);
target.deleteUser(req, INVALID_AUTHENTICATOR_NAME, USER_NAME, null, null);
}

@Test
public void createUserShouldReturnExpectedResponse()
{
Assert.assertNotNull(target.createUser(req, AUTHENTICATOR_NAME, USER_NAME));
Assert.assertNotNull(target.createUser(req, AUTHENTICATOR_NAME, USER_NAME, null, null));
}

@Test(expected = IllegalArgumentException.class)
public void createUserWithInvalidAuthenticatorNameShouldReturnExpectedResponse()
{
target.createUser(req, INVALID_AUTHENTICATOR_NAME, USER_NAME);
target.createUser(req, INVALID_AUTHENTICATOR_NAME, USER_NAME, null, null);
}

@Test
Expand Down
20 changes: 11 additions & 9 deletions server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ public Response markSegmentsAsUnused(
);
auditPayload = Collections.singletonMap("segmentIds", segmentIds);
}
if (author != null && !author.isEmpty()) {
if (auditManager != null && author != null && !author.isEmpty()) {
auditManager.doAudit(
AuditEvent.builder()
.key(dataSourceName)
Expand Down Expand Up @@ -379,14 +379,16 @@ public Response killUnusedSegmentsInInterval(
overlordClient.runKillTask("api-issued", dataSourceName, theInterval, null),
true
);
auditManager.doAudit(
AuditEvent.builder()
.key(dataSourceName)
.type("segments.killTask")
.payload(ImmutableMap.of("killTaskId", killTaskId, "interval", theInterval))
.auditInfo(new AuditInfo(author, comment, req.getRemoteAddr()))
.build()
);
if (auditManager != null && author != null && !author.isEmpty()) {
auditManager.doAudit(
AuditEvent.builder()
.key(dataSourceName)
.type("segments.killTask")
.payload(ImmutableMap.of("killTaskId", killTaskId, "interval", theInterval))
.auditInfo(new AuditInfo(author, comment, req.getRemoteAddr()))
.build()
);
}
return Response.ok().build();
}
catch (Exception e) {
Expand Down

0 comments on commit 5adc89a

Please sign in to comment.