diff --git a/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/endpoint/BasicAuthenticatorResource.java b/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/endpoint/BasicAuthenticatorResource.java index 23d8316771c4..6798c9875182 100644 --- a/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/endpoint/BasicAuthenticatorResource.java +++ b/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/endpoint/BasicAuthenticatorResource.java @@ -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; @@ -30,7 +33,9 @@ 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; @@ -38,6 +43,7 @@ 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 @@ -45,15 +51,18 @@ 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; } /** @@ -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 @@ -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; } /** @@ -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 @@ -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; } /** @@ -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 @@ -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; } /** @@ -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() + ); + } } diff --git a/extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/authentication/CoordinatorBasicAuthenticatorResourceTest.java b/extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/authentication/CoordinatorBasicAuthenticatorResourceTest.java index b383a6d3d92f..a696286f5f83 100644 --- a/extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/authentication/CoordinatorBasicAuthenticatorResourceTest.java +++ b/extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/authentication/CoordinatorBasicAuthenticatorResourceTest.java @@ -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; @@ -68,6 +69,9 @@ public class CoordinatorBasicAuthenticatorResourceTest @Mock private AuthValidator authValidator; + + @Mock + private AuditManager auditManager; private BasicAuthenticatorResource resource; private CoordinatorBasicAuthenticatorMetadataStorageUpdater storageUpdater; private HttpServletRequest req; @@ -145,7 +149,8 @@ public void setUp() authenticatorMapper, objectMapper ), - authValidator + authValidator, + auditManager ); storageUpdater.start(); @@ -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 expectedUsers = ImmutableSet.of( BasicAuthUtils.ADMIN_NAME, @@ -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 expectedUsers = ImmutableSet.of( BasicAuthUtils.ADMIN_NAME, @@ -285,7 +290,7 @@ 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"); @@ -293,7 +298,7 @@ public void testCreateDeleteUser() 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); @@ -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()); @@ -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()); @@ -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"); @@ -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()); diff --git a/extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/basic/authentication/endpoint/BasicAuthenticatorResourceTest.java b/extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/basic/authentication/endpoint/BasicAuthenticatorResourceTest.java index fc403b08ca30..69ac7ef7ca1f 100644 --- a/extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/basic/authentication/endpoint/BasicAuthenticatorResourceTest.java +++ b/extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/basic/authentication/endpoint/BasicAuthenticatorResourceTest.java @@ -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 @@ -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 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 index 46c521200a83..5ebdaf0bf471 100644 --- a/server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java +++ b/server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java @@ -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) @@ -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) {