From e6c9142129f1462feabefb4b13aa8a037fbe793f Mon Sep 17 00:00:00 2001 From: Suneet Saldanha Date: Mon, 13 Jul 2020 21:15:54 -0700 Subject: [PATCH] Add validation for authenticator and authorizer name (#10106) * Add validation for authorizer name * fix deps * add javadocs * Do not use resource filters * Fix BasicAuthenticatorResource as well * Add integration tests * fix test * fix --- .../utils/IdUtils.java} | 6 +- .../utils/IdUtilsTest.java} | 24 +- extensions-core/druid-basic-security/pom.xml | 5 + .../endpoint/BasicAuthenticatorResource.java | 13 +- .../endpoint/BasicAuthorizerResource.java | 29 +- ...dinatorBasicAuthenticatorResourceTest.java | 10 +- ...oordinatorBasicAuthorizerResourceTest.java | 16 +- .../BasicAuthenticatorResourceTest.java | 147 ++++++++ .../endpoint/BasicAuthorizerResourceTest.java | 337 ++++++++++++++++++ .../indexing/kafka/KafkaConsumerConfigs.java | 4 +- .../kafka/supervisor/KafkaSupervisor.java | 4 +- .../kinesis/supervisor/KinesisSupervisor.java | 4 +- .../indexing/common/task/AbstractTask.java | 4 +- .../http/security/TaskResourceFilter.java | 4 +- .../worker/IntermediaryDataManager.java | 6 +- .../indexing/worker/http/WorkerResource.java | 4 +- .../druid/testing/utils/KafkaEventWriter.java | 4 +- .../ITBasicAuthConfigurationTest.java | 47 +++ .../druid/segment/indexing/DataSchema.java | 4 +- .../AuthorizerMapperModule.java | 9 +- .../druid/server/security/AuthValidator.java | 53 +++ .../segment/indexing/DataSchemaTest.java | 22 +- .../AuthorizerMapperModuleTest.java | 61 ++++ .../server/security/AuthValidatorTest.java | 99 +++++ 24 files changed, 859 insertions(+), 57 deletions(-) rename core/src/main/java/org/apache/druid/{indexer/TaskIdUtils.java => common/utils/IdUtils.java} (95%) rename core/src/test/java/org/apache/druid/{indexer/TaskIdUtilsTest.java => common/utils/IdUtilsTest.java} (83%) create mode 100644 extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/basic/authentication/endpoint/BasicAuthenticatorResourceTest.java create mode 100644 extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/basic/authorization/endpoint/BasicAuthorizerResourceTest.java create mode 100644 server/src/main/java/org/apache/druid/server/security/AuthValidator.java create mode 100644 server/src/test/java/org/apache/druid/server/initialization/AuthorizerMapperModuleTest.java create mode 100644 server/src/test/java/org/apache/druid/server/security/AuthValidatorTest.java diff --git a/core/src/main/java/org/apache/druid/indexer/TaskIdUtils.java b/core/src/main/java/org/apache/druid/common/utils/IdUtils.java similarity index 95% rename from core/src/main/java/org/apache/druid/indexer/TaskIdUtils.java rename to core/src/main/java/org/apache/druid/common/utils/IdUtils.java index 76317d9ca5a..f0f4ef90a9a 100644 --- a/core/src/main/java/org/apache/druid/indexer/TaskIdUtils.java +++ b/core/src/main/java/org/apache/druid/common/utils/IdUtils.java @@ -17,7 +17,7 @@ * under the License. */ -package org.apache.druid.indexer; +package org.apache.druid.common.utils; import com.google.common.base.Joiner; import com.google.common.base.Preconditions; @@ -28,7 +28,7 @@ import java.util.concurrent.ThreadLocalRandom; import java.util.regex.Matcher; import java.util.regex.Pattern; -public class TaskIdUtils +public class IdUtils { private static final Pattern INVALIDCHARS = Pattern.compile("(?s).*[^\\S ].*"); @@ -66,6 +66,6 @@ public class TaskIdUtils public static String getRandomIdWithPrefix(String prefix) { - return UNDERSCORE_JOINER.join(prefix, TaskIdUtils.getRandomId()); + return UNDERSCORE_JOINER.join(prefix, IdUtils.getRandomId()); } } diff --git a/core/src/test/java/org/apache/druid/indexer/TaskIdUtilsTest.java b/core/src/test/java/org/apache/druid/common/utils/IdUtilsTest.java similarity index 83% rename from core/src/test/java/org/apache/druid/indexer/TaskIdUtilsTest.java rename to core/src/test/java/org/apache/druid/common/utils/IdUtilsTest.java index 5fed8fb34fb..2adbd495c30 100644 --- a/core/src/test/java/org/apache/druid/indexer/TaskIdUtilsTest.java +++ b/core/src/test/java/org/apache/druid/common/utils/IdUtilsTest.java @@ -17,13 +17,13 @@ * under the License. */ -package org.apache.druid.indexer; +package org.apache.druid.common.utils; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; -public class TaskIdUtilsTest +public class IdUtilsTest { private static final String THINGO = "thingToValidate"; public static final String VALID_ID_CHARS = "alpha123..*~!@#&%^&*()-+ Россия\\ 한국 中国!"; @@ -34,7 +34,7 @@ public class TaskIdUtilsTest @Test public void testValidIdName() { - TaskIdUtils.validateId(THINGO, VALID_ID_CHARS); + IdUtils.validateId(THINGO, VALID_ID_CHARS); } @Test @@ -42,7 +42,7 @@ public class TaskIdUtilsTest { expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("thingToValidate cannot be null or empty. Please provide a thingToValidate."); - TaskIdUtils.validateId(THINGO, null); + IdUtils.validateId(THINGO, null); } @Test @@ -50,7 +50,7 @@ public class TaskIdUtilsTest { expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("thingToValidate cannot be null or empty. Please provide a thingToValidate."); - TaskIdUtils.validateId(THINGO, ""); + IdUtils.validateId(THINGO, ""); } @Test @@ -58,7 +58,7 @@ public class TaskIdUtilsTest { expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("thingToValidate cannot contain the '/' character."); - TaskIdUtils.validateId(THINGO, "/paths/are/bad/since/we/make/files/from/stuff"); + IdUtils.validateId(THINGO, "/paths/are/bad/since/we/make/files/from/stuff"); } @Test @@ -66,7 +66,7 @@ public class TaskIdUtilsTest { expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("thingToValidate cannot start with the '.' character."); - TaskIdUtils.validateId(THINGO, "./nice/try"); + IdUtils.validateId(THINGO, "./nice/try"); } @Test @@ -74,7 +74,7 @@ public class TaskIdUtilsTest { expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("thingToValidate cannot contain whitespace character except space."); - TaskIdUtils.validateId(THINGO, "spaces\tare\tbetter\tthan\ttabs\twhich\tare\tillegal"); + IdUtils.validateId(THINGO, "spaces\tare\tbetter\tthan\ttabs\twhich\tare\tillegal"); } @Test @@ -82,7 +82,7 @@ public class TaskIdUtilsTest { expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("thingToValidate cannot contain whitespace character except space."); - TaskIdUtils.validateId(THINGO, "new\nline"); + IdUtils.validateId(THINGO, "new\nline"); } @Test @@ -90,7 +90,7 @@ public class TaskIdUtilsTest { expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("thingToValidate cannot contain whitespace character except space."); - TaskIdUtils.validateId(THINGO, "does\rexist\rby\ritself"); + IdUtils.validateId(THINGO, "does\rexist\rby\ritself"); } @Test @@ -98,7 +98,7 @@ public class TaskIdUtilsTest { expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("thingToValidate cannot contain whitespace character except space."); - TaskIdUtils.validateId(THINGO, "wtf\u000Bis line tabulation"); + IdUtils.validateId(THINGO, "wtf\u000Bis line tabulation"); } @Test @@ -106,6 +106,6 @@ public class TaskIdUtilsTest { expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("thingToValidate cannot contain whitespace character except space."); - TaskIdUtils.validateId(THINGO, "form\u000cfeed?"); + IdUtils.validateId(THINGO, "form\u000cfeed?"); } } diff --git a/extensions-core/druid-basic-security/pom.xml b/extensions-core/druid-basic-security/pom.xml index 16f6d5d7bd9..0ac8820b14c 100644 --- a/extensions-core/druid-basic-security/pom.xml +++ b/extensions-core/druid-basic-security/pom.xml @@ -132,6 +132,11 @@ junit test + + org.mockito + mockito-core + test + org.easymock easymock 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 b14b4fab0ca..23d8316771c 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 @@ -25,6 +25,7 @@ import com.sun.jersey.spi.container.ResourceFilters; import org.apache.druid.guice.LazySingleton; import org.apache.druid.security.basic.BasicSecurityResourceFilter; import org.apache.druid.security.basic.authentication.entity.BasicAuthenticatorCredentialUpdate; +import org.apache.druid.server.security.AuthValidator; import javax.servlet.http.HttpServletRequest; import javax.ws.rs.Consumes; @@ -43,13 +44,16 @@ import javax.ws.rs.core.Response; public class BasicAuthenticatorResource { private final BasicAuthenticatorResourceHandler handler; + private final AuthValidator authValidator; @Inject public BasicAuthenticatorResource( - BasicAuthenticatorResourceHandler handler + BasicAuthenticatorResourceHandler handler, + AuthValidator authValidator ) { this.handler = handler; + this.authValidator = authValidator; } /** @@ -102,6 +106,7 @@ public class BasicAuthenticatorResource @PathParam("authenticatorName") final String authenticatorName ) { + authValidator.validateAuthenticatorName(authenticatorName); return handler.getAllUsers(authenticatorName); } @@ -122,6 +127,7 @@ public class BasicAuthenticatorResource @PathParam("userName") final String userName ) { + authValidator.validateAuthenticatorName(authenticatorName); return handler.getUser(authenticatorName, userName); } @@ -144,6 +150,7 @@ public class BasicAuthenticatorResource @PathParam("userName") String userName ) { + authValidator.validateAuthenticatorName(authenticatorName); return handler.createUser(authenticatorName, userName); } @@ -166,6 +173,7 @@ public class BasicAuthenticatorResource @PathParam("userName") String userName ) { + authValidator.validateAuthenticatorName(authenticatorName); return handler.deleteUser(authenticatorName, userName); } @@ -189,6 +197,7 @@ public class BasicAuthenticatorResource BasicAuthenticatorCredentialUpdate update ) { + authValidator.validateAuthenticatorName(authenticatorName); return handler.updateUserCredentials(authenticatorName, userName, update); } @@ -207,6 +216,7 @@ public class BasicAuthenticatorResource @PathParam("authenticatorName") final String authenticatorName ) { + authValidator.validateAuthenticatorName(authenticatorName); return handler.getCachedSerializedUserMap(authenticatorName); } @@ -224,6 +234,7 @@ public class BasicAuthenticatorResource byte[] serializedUserMap ) { + authValidator.validateAuthenticatorName(authenticatorName); return handler.authenticatorUserUpdateListener(authenticatorName, serializedUserMap); } } diff --git a/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authorization/endpoint/BasicAuthorizerResource.java b/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authorization/endpoint/BasicAuthorizerResource.java index 0e70e5a3dcd..cb8a9fa2a24 100644 --- a/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authorization/endpoint/BasicAuthorizerResource.java +++ b/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authorization/endpoint/BasicAuthorizerResource.java @@ -25,6 +25,7 @@ import com.sun.jersey.spi.container.ResourceFilters; import org.apache.druid.guice.LazySingleton; import org.apache.druid.security.basic.BasicSecurityResourceFilter; import org.apache.druid.security.basic.authorization.entity.BasicAuthorizerGroupMapping; +import org.apache.druid.server.security.AuthValidator; import org.apache.druid.server.security.ResourceAction; import javax.servlet.http.HttpServletRequest; @@ -46,13 +47,16 @@ import java.util.List; public class BasicAuthorizerResource { private final BasicAuthorizerResourceHandler resourceHandler; + private final AuthValidator authValidator; @Inject public BasicAuthorizerResource( - BasicAuthorizerResourceHandler resourceHandler + BasicAuthorizerResourceHandler resourceHandler, + AuthValidator authValidator ) { this.resourceHandler = resourceHandler; + this.authValidator = authValidator; } /** @@ -106,6 +110,7 @@ public class BasicAuthorizerResource @PathParam("authorizerName") final String authorizerName ) { + authValidator.validateAuthorizerName(authorizerName); return resourceHandler.getAllUsers(authorizerName); } @@ -124,6 +129,7 @@ public class BasicAuthorizerResource @PathParam("authorizerName") final String authorizerName ) { + authValidator.validateAuthorizerName(authorizerName); return resourceHandler.getAllGroupMappings(authorizerName); } @@ -146,6 +152,7 @@ public class BasicAuthorizerResource @QueryParam("simplifyPermissions") String simplifyPermissions ) { + authValidator.validateAuthorizerName(authorizerName); return resourceHandler.getUser(authorizerName, userName, full != null, simplifyPermissions != null); } @@ -167,6 +174,7 @@ public class BasicAuthorizerResource @QueryParam("full") String full ) { + authValidator.validateAuthorizerName(authorizerName); return resourceHandler.getGroupMapping(authorizerName, groupMappingName, full != null); } @@ -189,6 +197,7 @@ public class BasicAuthorizerResource @PathParam("userName") String userName ) { + authValidator.validateAuthorizerName(authorizerName); return resourceHandler.createUser(authorizerName, userName); } @@ -211,6 +220,7 @@ public class BasicAuthorizerResource @PathParam("userName") String userName ) { + authValidator.validateAuthorizerName(authorizerName); return resourceHandler.deleteUser(authorizerName, userName); } @@ -234,6 +244,7 @@ public class BasicAuthorizerResource BasicAuthorizerGroupMapping groupMapping ) { + authValidator.validateAuthorizerName(authorizerName); return resourceHandler.createGroupMapping( authorizerName, new BasicAuthorizerGroupMapping(groupMappingName, groupMapping.getGroupPattern(), groupMapping.getRoles()) @@ -259,6 +270,7 @@ public class BasicAuthorizerResource @PathParam("groupMappingName") String groupMappingName ) { + authValidator.validateAuthorizerName(authorizerName); return resourceHandler.deleteGroupMapping(authorizerName, groupMappingName); } @@ -277,6 +289,7 @@ public class BasicAuthorizerResource @PathParam("authorizerName") final String authorizerName ) { + authValidator.validateAuthorizerName(authorizerName); return resourceHandler.getAllRoles(authorizerName); } @@ -301,6 +314,7 @@ public class BasicAuthorizerResource @QueryParam("simplifyPermissions") String simplifyPermissions ) { + authValidator.validateAuthorizerName(authorizerName); return resourceHandler.getRole(authorizerName, roleName, full != null, simplifyPermissions != null); } @@ -323,6 +337,7 @@ public class BasicAuthorizerResource @PathParam("roleName") final String roleName ) { + authValidator.validateAuthorizerName(authorizerName); return resourceHandler.createRole(authorizerName, roleName); } @@ -345,6 +360,7 @@ public class BasicAuthorizerResource @PathParam("roleName") String roleName ) { + authValidator.validateAuthorizerName(authorizerName); return resourceHandler.deleteRole(authorizerName, roleName); } @@ -369,6 +385,7 @@ public class BasicAuthorizerResource @PathParam("roleName") String roleName ) { + authValidator.validateAuthorizerName(authorizerName); return resourceHandler.assignRoleToUser(authorizerName, userName, roleName); } @@ -393,6 +410,7 @@ public class BasicAuthorizerResource @PathParam("roleName") String roleName ) { + authValidator.validateAuthorizerName(authorizerName); return resourceHandler.unassignRoleFromUser(authorizerName, userName, roleName); } @@ -417,6 +435,7 @@ public class BasicAuthorizerResource @PathParam("roleName") String roleName ) { + authValidator.validateAuthorizerName(authorizerName); return resourceHandler.assignRoleToGroupMapping(authorizerName, groupMappingName, roleName); } @@ -441,6 +460,7 @@ public class BasicAuthorizerResource @PathParam("roleName") String roleName ) { + authValidator.validateAuthorizerName(authorizerName); return resourceHandler.unassignRoleFromGroupMapping(authorizerName, groupMappingName, roleName); } @@ -465,6 +485,7 @@ public class BasicAuthorizerResource List permissions ) { + authValidator.validateAuthorizerName(authorizerName); return resourceHandler.setRolePermissions(authorizerName, roleName, permissions); } @@ -487,6 +508,7 @@ public class BasicAuthorizerResource @PathParam("roleName") String roleName ) { + authValidator.validateAuthorizerName(authorizerName); return resourceHandler.getRolePermissions(authorizerName, roleName); } @@ -505,6 +527,7 @@ public class BasicAuthorizerResource @PathParam("authorizerName") final String authorizerName ) { + authValidator.validateAuthorizerName(authorizerName); return resourceHandler.getCachedUserMaps(authorizerName); } @@ -523,6 +546,7 @@ public class BasicAuthorizerResource @PathParam("authorizerName") final String authorizerName ) { + authValidator.validateAuthorizerName(authorizerName); return resourceHandler.getCachedGroupMappingMaps(authorizerName); } @@ -544,6 +568,7 @@ public class BasicAuthorizerResource byte[] serializedUserAndRoleMap ) { + authValidator.validateAuthorizerName(authorizerName); return resourceHandler.authorizerUserUpdateListener(authorizerName, serializedUserAndRoleMap); } @@ -561,6 +586,7 @@ public class BasicAuthorizerResource byte[] serializedUserAndRoleMap ) { + authValidator.validateAuthorizerName(authorizerName); return resourceHandler.authorizerUserUpdateListener(authorizerName, serializedUserAndRoleMap); } @@ -578,6 +604,7 @@ public class BasicAuthorizerResource byte[] serializedGroupMappingAndRoleMap ) { + authValidator.validateAuthorizerName(authorizerName); return resourceHandler.authorizerGroupMappingUpdateListener(authorizerName, serializedGroupMappingAndRoleMap); } } 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 cce1b2402c7..b383a6d3d92 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 @@ -35,6 +35,7 @@ import org.apache.druid.security.basic.authentication.endpoint.CoordinatorBasicA import org.apache.druid.security.basic.authentication.entity.BasicAuthenticatorCredentialUpdate; import org.apache.druid.security.basic.authentication.entity.BasicAuthenticatorCredentials; import org.apache.druid.security.basic.authentication.entity.BasicAuthenticatorUser; +import org.apache.druid.server.security.AuthValidator; import org.apache.druid.server.security.AuthenticatorMapper; import org.easymock.EasyMock; import org.junit.After; @@ -43,12 +44,16 @@ import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; import javax.servlet.http.HttpServletRequest; import javax.ws.rs.core.Response; import java.util.Map; import java.util.Set; +@RunWith(MockitoJUnitRunner.class) public class CoordinatorBasicAuthenticatorResourceTest { private static final String AUTHENTICATOR_NAME = "test"; @@ -61,6 +66,8 @@ public class CoordinatorBasicAuthenticatorResourceTest @Rule public final TestDerbyConnector.DerbyConnectorRule derbyConnectorRule = new TestDerbyConnector.DerbyConnectorRule(); + @Mock + private AuthValidator authValidator; private BasicAuthenticatorResource resource; private CoordinatorBasicAuthenticatorMetadataStorageUpdater storageUpdater; private HttpServletRequest req; @@ -137,7 +144,8 @@ public class CoordinatorBasicAuthenticatorResourceTest storageUpdater, authenticatorMapper, objectMapper - ) + ), + authValidator ); storageUpdater.start(); diff --git a/extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/authorization/CoordinatorBasicAuthorizerResourceTest.java b/extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/authorization/CoordinatorBasicAuthorizerResourceTest.java index bdaf8e6a917..746d3339f97 100644 --- a/extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/authorization/CoordinatorBasicAuthorizerResourceTest.java +++ b/extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/authorization/CoordinatorBasicAuthorizerResourceTest.java @@ -45,17 +45,20 @@ import org.apache.druid.security.basic.authorization.entity.BasicAuthorizerUser; import org.apache.druid.security.basic.authorization.entity.BasicAuthorizerUserFull; import org.apache.druid.security.basic.authorization.entity.BasicAuthorizerUserFullSimplifiedPermissions; import org.apache.druid.server.security.Action; +import org.apache.druid.server.security.AuthValidator; import org.apache.druid.server.security.AuthorizerMapper; import org.apache.druid.server.security.Resource; import org.apache.druid.server.security.ResourceAction; import org.apache.druid.server.security.ResourceType; -import org.easymock.EasyMock; import org.junit.After; import org.junit.Assert; import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; import javax.servlet.http.HttpServletRequest; import javax.ws.rs.core.Response; @@ -68,6 +71,7 @@ import java.util.concurrent.Callable; import java.util.concurrent.ExecutorService; import java.util.concurrent.Future; +@RunWith(MockitoJUnitRunner.class) public class CoordinatorBasicAuthorizerResourceTest { private static final String AUTHORIZER_NAME = "test"; @@ -79,18 +83,19 @@ public class CoordinatorBasicAuthorizerResourceTest @Rule public final TestDerbyConnector.DerbyConnectorRule derbyConnectorRule = new TestDerbyConnector.DerbyConnectorRule(); + @Mock + private AuthValidator authValidator; + @Mock + private HttpServletRequest req; private TestDerbyConnector connector; private MetadataStorageTablesConfig tablesConfig; private BasicAuthorizerResource resource; private CoordinatorBasicAuthorizerMetadataStorageUpdater storageUpdater; - private HttpServletRequest req; @Before public void setUp() { - req = EasyMock.createStrictMock(HttpServletRequest.class); - connector = derbyConnectorRule.getConnector(); tablesConfig = derbyConnectorRule.metadataTablesConfigSupplier().get(); connector.createConfigTable(); @@ -148,7 +153,8 @@ public class CoordinatorBasicAuthorizerResourceTest storageUpdater, authorizerMapper, new ObjectMapper(new SmileFactory()) - ) + ), + authValidator ); storageUpdater.start(); 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 new file mode 100644 index 00000000000..fc403b08ca3 --- /dev/null +++ b/extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/basic/authentication/endpoint/BasicAuthenticatorResourceTest.java @@ -0,0 +1,147 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.security.basic.authentication.endpoint; + +import org.apache.druid.security.basic.authentication.entity.BasicAuthenticatorCredentialUpdate; +import org.apache.druid.server.security.AuthValidator; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Answers; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.MockitoJUnitRunner; + +import javax.servlet.http.HttpServletRequest; +import java.nio.charset.StandardCharsets; + +@RunWith(MockitoJUnitRunner.class) +public class BasicAuthenticatorResourceTest +{ + private static final String AUTHENTICATOR_NAME = "AUTHENTICATOR_NAME"; + private static final String INVALID_AUTHENTICATOR_NAME = "INVALID_AUTHENTICATOR_NAME"; + private static final String USER_NAME = "USER_NAME"; + private static final byte[] SERIALIZED_USER_MAP = "SERIALIZED_USER_MAP".getBytes(StandardCharsets.UTF_8); + @Mock(answer = Answers.RETURNS_MOCKS) + private BasicAuthenticatorResourceHandler handler; + @Mock + private AuthValidator authValidator; + @Mock + private HttpServletRequest req; + @Mock + private BasicAuthenticatorCredentialUpdate update; + + private BasicAuthenticatorResource target; + + @Before + public void setUp() + { + Mockito.doThrow(IllegalArgumentException.class) + .when(authValidator) + .validateAuthenticatorName(INVALID_AUTHENTICATOR_NAME); + + target = new BasicAuthenticatorResource(handler, authValidator); + } + + @Test + public void authenticatorUpdateListenerShouldReturnExpectedResponse() + { + Assert.assertNotNull(target.authenticatorUpdateListener(req, AUTHENTICATOR_NAME, SERIALIZED_USER_MAP)); + } + + @Test(expected = IllegalArgumentException.class) + public void authenticatorUpdateListenerWithInvalidAuthenticatorNameShouldReturnExpectedResponse() + { + target.authenticatorUpdateListener(req, INVALID_AUTHENTICATOR_NAME, SERIALIZED_USER_MAP); + } + + @Test + public void getCachedSerializedUserMapShouldReturnExpectedResponse() + { + Assert.assertNotNull(target.getCachedSerializedUserMap(req, AUTHENTICATOR_NAME)); + } + + @Test(expected = IllegalArgumentException.class) + public void getCachedSerializedUserMapWithInvalidAuthenticatorNameShouldReturnExpectedResponse() + { + target.getCachedSerializedUserMap(req, INVALID_AUTHENTICATOR_NAME); + } + + @Test + public void updateUserCredentialsShouldReturnExpectedResponse() + { + Assert.assertNotNull(target.updateUserCredentials(req, AUTHENTICATOR_NAME, USER_NAME, update)); + } + + @Test(expected = IllegalArgumentException.class) + public void updateUserCredentialsWithInvalidAuthenticatorNameShouldReturnExpectedResponse() + { + target.updateUserCredentials(req, INVALID_AUTHENTICATOR_NAME, USER_NAME, update); + } + + @Test + public void deleteUserShouldReturnExpectedResponse() + { + Assert.assertNotNull(target.deleteUser(req, AUTHENTICATOR_NAME, USER_NAME)); + } + + @Test(expected = IllegalArgumentException.class) + public void deleteUserWithInvalidAuthenticatorNameShouldReturnExpectedResponse() + { + target.deleteUser(req, INVALID_AUTHENTICATOR_NAME, USER_NAME); + } + + @Test + public void createUserShouldReturnExpectedResponse() + { + Assert.assertNotNull(target.createUser(req, AUTHENTICATOR_NAME, USER_NAME)); + } + + @Test(expected = IllegalArgumentException.class) + public void createUserWithInvalidAuthenticatorNameShouldReturnExpectedResponse() + { + target.createUser(req, INVALID_AUTHENTICATOR_NAME, USER_NAME); + } + + @Test + public void getUserShouldReturnExpectedResponse() + { + Assert.assertNotNull(target.getUser(req, AUTHENTICATOR_NAME, USER_NAME)); + } + + @Test(expected = IllegalArgumentException.class) + public void getUserWithInvalidAuthenticatorNameShouldReturnExpectedResponse() + { + target.getUser(req, INVALID_AUTHENTICATOR_NAME, USER_NAME); + } + + @Test + public void getAllUsersShouldReturnExpectedResponse() + { + Assert.assertNotNull(target.getAllUsers(req, AUTHENTICATOR_NAME)); + } + + @Test(expected = IllegalArgumentException.class) + public void getAllUsersWithInvalidAuthenticatorNameShouldReturnExpectedResponse() + { + target.getAllUsers(req, INVALID_AUTHENTICATOR_NAME); + } +} diff --git a/extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/basic/authorization/endpoint/BasicAuthorizerResourceTest.java b/extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/basic/authorization/endpoint/BasicAuthorizerResourceTest.java new file mode 100644 index 00000000000..de0bf1db4e3 --- /dev/null +++ b/extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/basic/authorization/endpoint/BasicAuthorizerResourceTest.java @@ -0,0 +1,337 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.security.basic.authorization.endpoint; + +import com.google.common.collect.ImmutableList; +import org.apache.druid.security.basic.authorization.entity.BasicAuthorizerGroupMapping; +import org.apache.druid.server.security.AuthValidator; +import org.apache.druid.server.security.ResourceAction; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Answers; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.MockitoJUnitRunner; + +import javax.servlet.http.HttpServletRequest; +import java.nio.charset.StandardCharsets; +import java.util.List; + +@RunWith(MockitoJUnitRunner.class) +public class BasicAuthorizerResourceTest +{ + private static final String AUTHORIZER_NAME = "AUTHORIZER_NAME"; + private static final String INVALID_AUTHORIZER_NAME = "INVALID_AUTHORIZER_NAME"; + private static final String USER_NAME = "USER_NAME"; + private static final String GROUP_MAPPING_NAME = "GROUP_MAPPING_NAME"; + private static final String ROLE_NAME = "ROLE_NAME"; + private static final byte[] SERIALIZED_ROLE_MAP = "SERIALIZED_ROLE_MAP".getBytes(StandardCharsets.UTF_8); + + @Mock(answer = Answers.RETURNS_MOCKS) + private BasicAuthorizerResourceHandler resourceHandler; + @Mock + private AuthValidator authValidator; + @Mock(answer = Answers.RETURNS_MOCKS) + private BasicAuthorizerGroupMapping groupMapping; + @Mock + private ResourceAction resourceAction; + private List resourceActions; + @Mock + private HttpServletRequest req; + + private BasicAuthorizerResource target; + + @Before + public void setUp() + { + resourceActions = ImmutableList.of(resourceAction); + Mockito.doThrow(IllegalArgumentException.class) + .when(authValidator) + .validateAuthorizerName(INVALID_AUTHORIZER_NAME); + + target = new BasicAuthorizerResource(resourceHandler, authValidator); + } + + @Test + public void getAllUsersShouldReturnExpectedUsers() + { + Assert.assertNotNull(target.getAllUsers(req, AUTHORIZER_NAME)); + } + + @Test(expected = IllegalArgumentException.class) + public void getAllUsersWithInvalidAuthorizerNameShouldThrowException() + { + target.getAllUsers(req, INVALID_AUTHORIZER_NAME); + } + + @Test + public void getAllGroupMappingsShouldReturnExpectedGroupMappings() + { + Assert.assertNotNull(target.getAllGroupMappings(req, AUTHORIZER_NAME)); + } + + @Test(expected = IllegalArgumentException.class) + public void getAllGroupMappingsWithInvalidAuthorizerNameShouldThrowException() + { + target.getAllGroupMappings(req, INVALID_AUTHORIZER_NAME); + } + + @Test + public void getUserShouldReturnExpectedUser() + { + Assert.assertNotNull(target.getUser(req, AUTHORIZER_NAME, USER_NAME, null, null)); + } + + @Test(expected = IllegalArgumentException.class) + public void getUserWithInvalidAuthorizerNameShouldThrowException() + { + target.getUser(req, INVALID_AUTHORIZER_NAME, USER_NAME, null, null); + } + + @Test + public void getGroupMappingShouldReturnExpectedGroupMapping() + { + Assert.assertNotNull(target.getGroupMapping(req, AUTHORIZER_NAME, GROUP_MAPPING_NAME, null)); + } + + @Test(expected = IllegalArgumentException.class) + public void getGroupMappingWithInvalidAuthorizerNameShouldThrowException() + { + target.getGroupMapping(req, INVALID_AUTHORIZER_NAME, GROUP_MAPPING_NAME, null); + } + + @Test + public void createUserShouldReturnExpectedResponse() + { + Assert.assertNotNull(target.createUser(req, AUTHORIZER_NAME, USER_NAME)); + } + + @Test(expected = IllegalArgumentException.class) + public void createUserWithInvalidAuthorizerNameShouldThrowException() + { + target.createUser(req, INVALID_AUTHORIZER_NAME, USER_NAME); + } + + @Test + public void deleteUserShouldReturnExpectedResponse() + { + Assert.assertNotNull(target.deleteUser(req, AUTHORIZER_NAME, USER_NAME)); + } + + @Test(expected = IllegalArgumentException.class) + public void deleteUserWithInvalidAuthorizerNameShouldThrowException() + { + target.deleteUser(req, INVALID_AUTHORIZER_NAME, USER_NAME); + } + + @Test + public void createGroupMappingShouldReturnExpectedResponse() + { + Assert.assertNotNull(target.createGroupMapping(req, AUTHORIZER_NAME, GROUP_MAPPING_NAME, groupMapping)); + } + + @Test(expected = IllegalArgumentException.class) + public void createGroupMappingWithInvalidAuthorizerNameShouldThrowException() + { + target.createGroupMapping(req, INVALID_AUTHORIZER_NAME, GROUP_MAPPING_NAME, groupMapping); + } + + @Test + public void deleteGroupMappingShouldReturnExpectedResponse() + { + Assert.assertNotNull(target.deleteGroupMapping(req, AUTHORIZER_NAME, GROUP_MAPPING_NAME)); + } + + @Test(expected = IllegalArgumentException.class) + public void deleteGroupMappingWithInvalidAuthorizerNameShouldThrowException() + { + target.deleteGroupMapping(req, INVALID_AUTHORIZER_NAME, GROUP_MAPPING_NAME); + } + + @Test + public void getRoleShouldReturnExpectedResult() + { + Assert.assertNotNull(target.getRole(req, AUTHORIZER_NAME, ROLE_NAME, null, null)); + } + + @Test(expected = IllegalArgumentException.class) + public void getRoleWithInvalidAuthorizerNameShouldThrowException() + { + target.getRole(req, INVALID_AUTHORIZER_NAME, ROLE_NAME, null, null); + } + + @Test + public void createRoleShouldReturnExpectedResult() + { + Assert.assertNotNull(target.createRole(req, AUTHORIZER_NAME, ROLE_NAME)); + } + + @Test(expected = IllegalArgumentException.class) + public void createRoleWithInvalidAuthorizerNameShouldThrowException() + { + target.createRole(req, INVALID_AUTHORIZER_NAME, ROLE_NAME); + } + + @Test + public void deleteRoleShouldReturnExpectedResult() + { + Assert.assertNotNull(target.deleteRole(req, AUTHORIZER_NAME, ROLE_NAME)); + } + + @Test(expected = IllegalArgumentException.class) + public void deleteRoleWithInvalidAuthorizerNameShouldThrowException() + { + target.deleteRole(req, INVALID_AUTHORIZER_NAME, ROLE_NAME); + } + + @Test + public void assignRoleToUserShouldReturnExpectedResult() + { + Assert.assertNotNull(target.assignRoleToUser(req, AUTHORIZER_NAME, USER_NAME, ROLE_NAME)); + } + + @Test(expected = IllegalArgumentException.class) + public void assignRoleToUserWithInvalidAuthorizerNameShouldThrowException() + { + target.assignRoleToUser(req, INVALID_AUTHORIZER_NAME, USER_NAME, ROLE_NAME); + } + + @Test + public void unassignRoleFromUserShouldReturnExpectedResult() + { + Assert.assertNotNull(target.unassignRoleFromUser(req, AUTHORIZER_NAME, USER_NAME, ROLE_NAME)); + } + + @Test(expected = IllegalArgumentException.class) + public void unassignRoleFromUserWithInvalidAuthorizerNameShouldThrowException() + { + target.unassignRoleFromUser(req, INVALID_AUTHORIZER_NAME, USER_NAME, ROLE_NAME); + } + + @Test + public void assignRoleToGroupMappingShouldReturnExpectedResult() + { + Assert.assertNotNull(target.assignRoleToGroupMapping(req, AUTHORIZER_NAME, GROUP_MAPPING_NAME, ROLE_NAME)); + } + + @Test(expected = IllegalArgumentException.class) + public void assignRoleToGroupMappingWithInvalidAuthorizerNameShouldThrowException() + { + target.assignRoleToGroupMapping(req, INVALID_AUTHORIZER_NAME, GROUP_MAPPING_NAME, ROLE_NAME); + } + + @Test + public void unassignRoleFromGroupMappingShouldReturnExpectedResult() + { + Assert.assertNotNull(target.unassignRoleFromGroupMapping(req, AUTHORIZER_NAME, GROUP_MAPPING_NAME, ROLE_NAME)); + } + + @Test(expected = IllegalArgumentException.class) + public void unassignRoleFromGroupMappingWithInvalidAuthorizerNameShouldThrowException() + { + target.unassignRoleFromGroupMapping(req, INVALID_AUTHORIZER_NAME, GROUP_MAPPING_NAME, ROLE_NAME); + } + + @Test + public void setRolePermissionsShouldReturnExpectedResult() + { + Assert.assertNotNull(target.setRolePermissions(req, AUTHORIZER_NAME, ROLE_NAME, resourceActions)); + } + + @Test(expected = IllegalArgumentException.class) + public void setRolePermissionsWithInvalidAuthorizerNameShouldThrowException() + { + target.setRolePermissions(req, INVALID_AUTHORIZER_NAME, ROLE_NAME, resourceActions); + } + + @Test + public void getRolePermissionsShouldReturnExpectedResult() + { + Assert.assertNotNull(target.getRolePermissions(req, AUTHORIZER_NAME, ROLE_NAME)); + } + + @Test(expected = IllegalArgumentException.class) + public void getRolePermissionsWithInvalidAuthorizerNameShouldThrowException() + { + target.getRolePermissions(req, INVALID_AUTHORIZER_NAME, ROLE_NAME); + } + + @Test + public void getCachedSerializedUserMapShouldReturnExpectedResult() + { + Assert.assertNotNull(target.getCachedSerializedUserMap(req, AUTHORIZER_NAME)); + } + + @Test(expected = IllegalArgumentException.class) + public void getCachedSerializedUserMapWithInvalidAuthorizerNameShouldThrowException() + { + target.getCachedSerializedUserMap(req, INVALID_AUTHORIZER_NAME); + } + + @Test + public void getCachedSerializedGroupMapShouldReturnExpectedResult() + { + Assert.assertNotNull(target.getCachedSerializedGroupMap(req, AUTHORIZER_NAME)); + } + + @Test(expected = IllegalArgumentException.class) + public void getCachedSerializedGroupMapWithInvalidAuthorizerNameShouldThrowException() + { + target.getCachedSerializedGroupMap(req, INVALID_AUTHORIZER_NAME); + } + + @Test + public void authorizerUpdateListenerShouldReturnExpectedResult() + { + Assert.assertNotNull(target.authorizerUpdateListener(req, AUTHORIZER_NAME, SERIALIZED_ROLE_MAP)); + } + + @Test(expected = IllegalArgumentException.class) + public void authorizerUpdateListenerWithInvalidAuthorizerNameShouldThrowException() + { + target.authorizerUpdateListener(req, INVALID_AUTHORIZER_NAME, SERIALIZED_ROLE_MAP); + } + + @Test + public void authorizerUserUpdateListenerShouldReturnExpectedResult() + { + Assert.assertNotNull(target.authorizerUserUpdateListener(req, AUTHORIZER_NAME, SERIALIZED_ROLE_MAP)); + } + + @Test(expected = IllegalArgumentException.class) + public void authorizerUserUpdateListenerWithInvalidAuthorizerNameShouldThrowException() + { + target.authorizerUserUpdateListener(req, INVALID_AUTHORIZER_NAME, SERIALIZED_ROLE_MAP); + } + + @Test + public void authorizerGroupMappingUpdateListenerShouldReturnExpectedResult() + { + Assert.assertNotNull(target.authorizerGroupMappingUpdateListener(req, AUTHORIZER_NAME, SERIALIZED_ROLE_MAP)); + } + + @Test(expected = IllegalArgumentException.class) + public void authorizerGroupMappingUpdateListenerWithInvalidAuthorizerNameShouldThrowException() + { + target.authorizerGroupMappingUpdateListener(req, INVALID_AUTHORIZER_NAME, SERIALIZED_ROLE_MAP); + } +} diff --git a/extensions-core/kafka-indexing-service/src/main/java/org/apache/druid/indexing/kafka/KafkaConsumerConfigs.java b/extensions-core/kafka-indexing-service/src/main/java/org/apache/druid/indexing/kafka/KafkaConsumerConfigs.java index 7339d26f7e6..e9c6e79358a 100644 --- a/extensions-core/kafka-indexing-service/src/main/java/org/apache/druid/indexing/kafka/KafkaConsumerConfigs.java +++ b/extensions-core/kafka-indexing-service/src/main/java/org/apache/druid/indexing/kafka/KafkaConsumerConfigs.java @@ -19,7 +19,7 @@ package org.apache.druid.indexing.kafka; -import org.apache.druid.indexer.TaskIdUtils; +import org.apache.druid.common.utils.IdUtils; import org.apache.druid.java.util.common.StringUtils; import java.util.HashMap; @@ -35,7 +35,7 @@ public class KafkaConsumerConfigs { final Map props = new HashMap<>(); props.put("metadata.max.age.ms", "10000"); - props.put("group.id", StringUtils.format("kafka-supervisor-%s", TaskIdUtils.getRandomId())); + props.put("group.id", StringUtils.format("kafka-supervisor-%s", IdUtils.getRandomId())); props.put("auto.offset.reset", "none"); props.put("enable.auto.commit", "false"); props.put("isolation.level", "read_committed"); diff --git a/extensions-core/kafka-indexing-service/src/main/java/org/apache/druid/indexing/kafka/supervisor/KafkaSupervisor.java b/extensions-core/kafka-indexing-service/src/main/java/org/apache/druid/indexing/kafka/supervisor/KafkaSupervisor.java index b2352fe40c4..0eb90f99012 100644 --- a/extensions-core/kafka-indexing-service/src/main/java/org/apache/druid/indexing/kafka/supervisor/KafkaSupervisor.java +++ b/extensions-core/kafka-indexing-service/src/main/java/org/apache/druid/indexing/kafka/supervisor/KafkaSupervisor.java @@ -23,7 +23,7 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.annotations.VisibleForTesting; -import org.apache.druid.indexer.TaskIdUtils; +import org.apache.druid.common.utils.IdUtils; import org.apache.druid.indexing.common.stats.RowIngestionMetersFactory; import org.apache.druid.indexing.common.task.Task; import org.apache.druid.indexing.common.task.TaskResource; @@ -219,7 +219,7 @@ public class KafkaSupervisor extends SeekableStreamSupervisor List> taskList = new ArrayList<>(); for (int i = 0; i < replicas; i++) { - String taskId = TaskIdUtils.getRandomIdWithPrefix(baseSequenceName); + String taskId = IdUtils.getRandomIdWithPrefix(baseSequenceName); taskList.add(new KafkaIndexTask( taskId, new TaskResource(baseSequenceName, 1), diff --git a/extensions-core/kinesis-indexing-service/src/main/java/org/apache/druid/indexing/kinesis/supervisor/KinesisSupervisor.java b/extensions-core/kinesis-indexing-service/src/main/java/org/apache/druid/indexing/kinesis/supervisor/KinesisSupervisor.java index 494e901dad7..8128796eb57 100644 --- a/extensions-core/kinesis-indexing-service/src/main/java/org/apache/druid/indexing/kinesis/supervisor/KinesisSupervisor.java +++ b/extensions-core/kinesis-indexing-service/src/main/java/org/apache/druid/indexing/kinesis/supervisor/KinesisSupervisor.java @@ -24,7 +24,7 @@ import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.collect.ImmutableMap; import org.apache.druid.common.aws.AWSCredentialsConfig; -import org.apache.druid.indexer.TaskIdUtils; +import org.apache.druid.common.utils.IdUtils; import org.apache.druid.indexing.common.stats.RowIngestionMetersFactory; import org.apache.druid.indexing.common.task.Task; import org.apache.druid.indexing.common.task.TaskResource; @@ -166,7 +166,7 @@ public class KinesisSupervisor extends SeekableStreamSupervisor List> taskList = new ArrayList<>(); for (int i = 0; i < replicas; i++) { - String taskId = TaskIdUtils.getRandomIdWithPrefix(baseSequenceName); + String taskId = IdUtils.getRandomIdWithPrefix(baseSequenceName); taskList.add(new KinesisIndexTask( taskId, new TaskResource(baseSequenceName, 1), diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/common/task/AbstractTask.java b/indexing-service/src/main/java/org/apache/druid/indexing/common/task/AbstractTask.java index 728f3dedfd0..dbc9119736f 100644 --- a/indexing-service/src/main/java/org/apache/druid/indexing/common/task/AbstractTask.java +++ b/indexing-service/src/main/java/org/apache/druid/indexing/common/task/AbstractTask.java @@ -24,7 +24,7 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.base.Joiner; import com.google.common.base.Objects; import com.google.common.base.Preconditions; -import org.apache.druid.indexer.TaskIdUtils; +import org.apache.druid.common.utils.IdUtils; import org.apache.druid.indexer.TaskStatus; import org.apache.druid.indexing.common.TaskLock; import org.apache.druid.indexing.common.actions.LockListAction; @@ -92,7 +92,7 @@ public abstract class AbstractTask implements Task } final List objects = new ArrayList<>(); - final String suffix = TaskIdUtils.getRandomId(); + final String suffix = IdUtils.getRandomId(); objects.add(typeName); objects.add(dataSource); objects.add(suffix); diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/security/TaskResourceFilter.java b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/security/TaskResourceFilter.java index 042c2a2b6f5..51b17f81def 100644 --- a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/security/TaskResourceFilter.java +++ b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/security/TaskResourceFilter.java @@ -25,7 +25,7 @@ import com.google.common.base.Predicate; import com.google.common.collect.Iterables; import com.google.inject.Inject; import com.sun.jersey.spi.container.ContainerRequest; -import org.apache.druid.indexer.TaskIdUtils; +import org.apache.druid.common.utils.IdUtils; import org.apache.druid.indexing.common.task.Task; import org.apache.druid.indexing.overlord.TaskStorageQueryAdapter; import org.apache.druid.java.util.common.StringUtils; @@ -82,7 +82,7 @@ public class TaskResourceFilter extends AbstractResourceFilter ).getPath() ); taskId = StringUtils.urlDecode(taskId); - TaskIdUtils.validateId("taskId", taskId); + IdUtils.validateId("taskId", taskId); Optional taskOptional = taskStorageQueryAdapter.getTask(taskId); if (!taskOptional.isPresent()) { diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/worker/IntermediaryDataManager.java b/indexing-service/src/main/java/org/apache/druid/indexing/worker/IntermediaryDataManager.java index 6df598c2403..408fba5fa6e 100644 --- a/indexing-service/src/main/java/org/apache/druid/indexing/worker/IntermediaryDataManager.java +++ b/indexing-service/src/main/java/org/apache/druid/indexing/worker/IntermediaryDataManager.java @@ -26,8 +26,8 @@ import org.apache.commons.io.FileUtils; import org.apache.commons.lang3.mutable.MutableInt; import org.apache.druid.client.indexing.IndexingServiceClient; import org.apache.druid.client.indexing.TaskStatus; +import org.apache.druid.common.utils.IdUtils; import org.apache.druid.guice.ManageLifecycle; -import org.apache.druid.indexer.TaskIdUtils; import org.apache.druid.indexing.common.config.TaskConfig; import org.apache.druid.indexing.worker.config.WorkerConfig; import org.apache.druid.java.util.common.DateTimes; @@ -337,7 +337,7 @@ public class IntermediaryDataManager @Nullable public File findPartitionFile(String supervisorTaskId, String subTaskId, Interval interval, int bucketId) { - TaskIdUtils.validateId("supervisorTaskId", supervisorTaskId); + IdUtils.validateId("supervisorTaskId", supervisorTaskId); for (StorageLocation location : shuffleDataLocations) { final File partitionDir = new File(location.getPath(), getPartitionDir(supervisorTaskId, interval, bucketId)); if (partitionDir.exists()) { @@ -366,7 +366,7 @@ public class IntermediaryDataManager public void deletePartitions(String supervisorTaskId) throws IOException { - TaskIdUtils.validateId("supervisorTaskId", supervisorTaskId); + IdUtils.validateId("supervisorTaskId", supervisorTaskId); for (StorageLocation location : shuffleDataLocations) { final File supervisorTaskPath = new File(location.getPath(), supervisorTaskId); if (supervisorTaskPath.exists()) { diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/worker/http/WorkerResource.java b/indexing-service/src/main/java/org/apache/druid/indexing/worker/http/WorkerResource.java index dd108f3f188..a66a4e7f896 100644 --- a/indexing-service/src/main/java/org/apache/druid/indexing/worker/http/WorkerResource.java +++ b/indexing-service/src/main/java/org/apache/druid/indexing/worker/http/WorkerResource.java @@ -27,7 +27,7 @@ import com.google.common.collect.Lists; import com.google.common.io.ByteSource; import com.google.inject.Inject; import com.sun.jersey.spi.container.ResourceFilters; -import org.apache.druid.indexer.TaskIdUtils; +import org.apache.druid.common.utils.IdUtils; import org.apache.druid.indexing.overlord.TaskRunner; import org.apache.druid.indexing.overlord.TaskRunnerWorkItem; import org.apache.druid.indexing.worker.Worker; @@ -189,7 +189,7 @@ public class WorkerResource @QueryParam("offset") @DefaultValue("0") long offset ) { - TaskIdUtils.validateId("taskId", taskId); + IdUtils.validateId("taskId", taskId); if (!(taskRunner instanceof TaskLogStreamer)) { return Response.status(501) .entity(StringUtils.format( diff --git a/integration-tests/src/main/java/org/apache/druid/testing/utils/KafkaEventWriter.java b/integration-tests/src/main/java/org/apache/druid/testing/utils/KafkaEventWriter.java index a505285d6de..1d5973c4782 100644 --- a/integration-tests/src/main/java/org/apache/druid/testing/utils/KafkaEventWriter.java +++ b/integration-tests/src/main/java/org/apache/druid/testing/utils/KafkaEventWriter.java @@ -19,7 +19,7 @@ package org.apache.druid.testing.utils; -import org.apache.druid.indexer.TaskIdUtils; +import org.apache.druid.common.utils.IdUtils; import org.apache.druid.testing.IntegrationTestingConfig; import org.apache.kafka.clients.producer.KafkaProducer; import org.apache.kafka.clients.producer.ProducerRecord; @@ -51,7 +51,7 @@ public class KafkaEventWriter implements StreamEventWriter this.txnEnabled = txnEnabled; if (txnEnabled) { properties.setProperty("enable.idempotence", "true"); - properties.setProperty("transactional.id", TaskIdUtils.getRandomId()); + properties.setProperty("transactional.id", IdUtils.getRandomId()); } this.producer = new KafkaProducer<>( properties, diff --git a/integration-tests/src/test/java/org/apache/druid/tests/security/ITBasicAuthConfigurationTest.java b/integration-tests/src/test/java/org/apache/druid/tests/security/ITBasicAuthConfigurationTest.java index 2a8506f4651..9f127a5890d 100644 --- a/integration-tests/src/test/java/org/apache/druid/tests/security/ITBasicAuthConfigurationTest.java +++ b/integration-tests/src/test/java/org/apache/druid/tests/security/ITBasicAuthConfigurationTest.java @@ -54,6 +54,7 @@ import org.testng.annotations.BeforeMethod; import org.testng.annotations.Guice; import org.testng.annotations.Test; +import java.nio.charset.StandardCharsets; import java.sql.Connection; import java.sql.DriverManager; import java.sql.ResultSet; @@ -472,6 +473,52 @@ public class ITBasicAuthConfigurationTest testOptionsRequests(adminClient); } + @Test + public void testInvalidAuthNames() + { + String invalidName = "invalid%2Fname"; + HttpClient adminClient = new CredentialedHttpClient( + new BasicCredentials("admin", "priest"), + httpClient + ); + + HttpUtil.makeRequestWithExpectedStatus( + adminClient, + HttpMethod.POST, + StringUtils.format( + "%s/druid-ext/basic-security/authentication/listen/%s", + config.getCoordinatorUrl(), + invalidName + ), + "SERIALIZED_DATA".getBytes(StandardCharsets.UTF_8), + HttpResponseStatus.INTERNAL_SERVER_ERROR + ); + + HttpUtil.makeRequestWithExpectedStatus( + adminClient, + HttpMethod.POST, + StringUtils.format( + "%s/druid-ext/basic-security/authorization/listen/users/%s", + config.getCoordinatorUrl(), + invalidName + ), + "SERIALIZED_DATA".getBytes(StandardCharsets.UTF_8), + HttpResponseStatus.INTERNAL_SERVER_ERROR + ); + + HttpUtil.makeRequestWithExpectedStatus( + adminClient, + HttpMethod.POST, + StringUtils.format( + "%s/druid-ext/basic-security/authorization/listen/groupMappings/%s", + config.getCoordinatorUrl(), + invalidName + ), + "SERIALIZED_DATA".getBytes(StandardCharsets.UTF_8), + HttpResponseStatus.INTERNAL_SERVER_ERROR + ); + } + @Test public void testMaliciousUser() { diff --git a/server/src/main/java/org/apache/druid/segment/indexing/DataSchema.java b/server/src/main/java/org/apache/druid/segment/indexing/DataSchema.java index 6be9a7141bf..3fc0fa26b27 100644 --- a/server/src/main/java/org/apache/druid/segment/indexing/DataSchema.java +++ b/server/src/main/java/org/apache/druid/segment/indexing/DataSchema.java @@ -28,11 +28,11 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.collect.Sets; +import org.apache.druid.common.utils.IdUtils; import org.apache.druid.data.input.impl.DimensionsSpec; import org.apache.druid.data.input.impl.InputRowParser; import org.apache.druid.data.input.impl.ParseSpec; import org.apache.druid.data.input.impl.TimestampSpec; -import org.apache.druid.indexer.TaskIdUtils; import org.apache.druid.java.util.common.IAE; import org.apache.druid.java.util.common.logger.Logger; import org.apache.druid.query.aggregation.AggregatorFactory; @@ -147,7 +147,7 @@ public class DataSchema private static void validateDatasourceName(String dataSource) { - TaskIdUtils.validateId("dataSource", dataSource); + IdUtils.validateId("dataSource", dataSource); } private static DimensionsSpec computeDimensionsSpec( diff --git a/server/src/main/java/org/apache/druid/server/initialization/AuthorizerMapperModule.java b/server/src/main/java/org/apache/druid/server/initialization/AuthorizerMapperModule.java index c8d9482b045..f547d14c5f3 100644 --- a/server/src/main/java/org/apache/druid/server/initialization/AuthorizerMapperModule.java +++ b/server/src/main/java/org/apache/druid/server/initialization/AuthorizerMapperModule.java @@ -33,9 +33,9 @@ import org.apache.druid.initialization.DruidModule; import org.apache.druid.java.util.common.IAE; import org.apache.druid.java.util.common.ISE; import org.apache.druid.java.util.common.StringUtils; -import org.apache.druid.java.util.common.logger.Logger; import org.apache.druid.server.security.AllowAllAuthorizer; import org.apache.druid.server.security.AuthConfig; +import org.apache.druid.server.security.AuthValidator; import org.apache.druid.server.security.Authorizer; import org.apache.druid.server.security.AuthorizerMapper; @@ -50,7 +50,6 @@ import java.util.Set; public class AuthorizerMapperModule implements DruidModule { private static final String AUTHORIZER_PROPERTIES_FORMAT_STRING = "druid.auth.authorizer.%s"; - private static Logger log = new Logger(AuthorizerMapperModule.class); @Override public void configure(Binder binder) @@ -58,7 +57,8 @@ public class AuthorizerMapperModule implements DruidModule binder.bind(AuthorizerMapper.class) .toProvider(new AuthorizerMapperProvider()) .in(LazySingleton.class); - + binder.bind(AuthValidator.class) + .in(LazySingleton.class); LifecycleModule.register(binder, AuthorizerMapper.class); } @@ -97,7 +97,8 @@ public class AuthorizerMapperModule implements DruidModule AllowAllAuthorizer allowAllAuthorizer = new AllowAllAuthorizer(); authorizerMap.put(AuthConfig.ALLOW_ALL_NAME, allowAllAuthorizer); - return new AuthorizerMapper(null) { + return new AuthorizerMapper(null) + { @Override public Authorizer getAuthorizer(String name) { diff --git a/server/src/main/java/org/apache/druid/server/security/AuthValidator.java b/server/src/main/java/org/apache/druid/server/security/AuthValidator.java new file mode 100644 index 00000000000..b0ad386c1f5 --- /dev/null +++ b/server/src/main/java/org/apache/druid/server/security/AuthValidator.java @@ -0,0 +1,53 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.server.security; + +import org.apache.druid.common.utils.IdUtils; + +/** + * Utility functions to validate the an authorizer. + */ +public class AuthValidator +{ + private static final String AUTHORIZER_NAME = "authorizerName"; + private static final String AUTHENTICATOR_NAME = "authenticatorName"; + + /** + * Validates the provided authorizerName. + * + * @param authorizerName the name of the authorizer. + * @throws IllegalArgumentException on invalid authorizer names. + */ + public void validateAuthorizerName(String authorizerName) + { + IdUtils.validateId(AUTHORIZER_NAME, authorizerName); + } + + /** + * Validates the provided authenticatorName. + * + * @param authenticatorName the name of the authenticator. + * @throws IllegalArgumentException on invalid authenticator names. + */ + public void validateAuthenticatorName(String authenticatorName) + { + IdUtils.validateId(AUTHENTICATOR_NAME, authenticatorName); + } +} diff --git a/server/src/test/java/org/apache/druid/segment/indexing/DataSchemaTest.java b/server/src/test/java/org/apache/druid/segment/indexing/DataSchemaTest.java index 5f116b4fe74..13bf27d2c17 100644 --- a/server/src/test/java/org/apache/druid/segment/indexing/DataSchemaTest.java +++ b/server/src/test/java/org/apache/druid/segment/indexing/DataSchemaTest.java @@ -26,12 +26,12 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import org.apache.commons.text.StringEscapeUtils; +import org.apache.druid.common.utils.IdUtilsTest; import org.apache.druid.data.input.InputRow; import org.apache.druid.data.input.impl.DimensionsSpec; import org.apache.druid.data.input.impl.JSONParseSpec; import org.apache.druid.data.input.impl.StringInputRowParser; import org.apache.druid.data.input.impl.TimestampSpec; -import org.apache.druid.indexer.TaskIdUtilsTest; import org.apache.druid.java.util.common.DateTimes; import org.apache.druid.java.util.common.IAE; import org.apache.druid.java.util.common.Intervals; @@ -85,7 +85,7 @@ public class DataSchemaTest extends InitializedNullHandlingTest ); DataSchema schema = new DataSchema( - TaskIdUtilsTest.VALID_ID_CHARS, + IdUtilsTest.VALID_ID_CHARS, parser, new AggregatorFactory[]{ new DoubleSumAggregatorFactory("metric1", "col1"), @@ -123,7 +123,7 @@ public class DataSchemaTest extends InitializedNullHandlingTest ); DataSchema schema = new DataSchema( - TaskIdUtilsTest.VALID_ID_CHARS, + IdUtilsTest.VALID_ID_CHARS, parser, new AggregatorFactory[]{ new DoubleSumAggregatorFactory("metric1", "col1"), @@ -161,7 +161,7 @@ public class DataSchemaTest extends InitializedNullHandlingTest ); DataSchema schema = new DataSchema( - TaskIdUtilsTest.VALID_ID_CHARS, + IdUtilsTest.VALID_ID_CHARS, parserMap, new AggregatorFactory[]{ new DoubleSumAggregatorFactory("metric1", "col1"), @@ -220,7 +220,7 @@ public class DataSchemaTest extends InitializedNullHandlingTest ); DataSchema schema = new DataSchema( - TaskIdUtilsTest.VALID_ID_CHARS, + IdUtilsTest.VALID_ID_CHARS, parser, new AggregatorFactory[]{ new DoubleSumAggregatorFactory("metric1", "col1"), @@ -254,7 +254,7 @@ public class DataSchemaTest extends InitializedNullHandlingTest ); DataSchema schema = new DataSchema( - TaskIdUtilsTest.VALID_ID_CHARS, + IdUtilsTest.VALID_ID_CHARS, parser, new AggregatorFactory[]{ new DoubleSumAggregatorFactory("metric1", "col1"), @@ -272,7 +272,7 @@ public class DataSchemaTest extends InitializedNullHandlingTest public void testSerdeWithInvalidParserMap() throws Exception { String jsonStr = "{" - + "\"dataSource\":\"" + StringEscapeUtils.escapeJson(TaskIdUtilsTest.VALID_ID_CHARS) + "\"," + + "\"dataSource\":\"" + StringEscapeUtils.escapeJson(IdUtilsTest.VALID_ID_CHARS) + "\"," + "\"parser\":{\"type\":\"invalid\"}," + "\"metricsSpec\":[{\"type\":\"doubleSum\",\"name\":\"metric1\",\"fieldName\":\"col1\"}]," + "\"granularitySpec\":{" @@ -376,7 +376,7 @@ public class DataSchemaTest extends InitializedNullHandlingTest public void testSerde() throws Exception { String jsonStr = "{" - + "\"dataSource\":\"" + StringEscapeUtils.escapeJson(TaskIdUtilsTest.VALID_ID_CHARS) + "\"," + + "\"dataSource\":\"" + StringEscapeUtils.escapeJson(IdUtilsTest.VALID_ID_CHARS) + "\"," + "\"parser\":{" + "\"type\":\"string\"," + "\"parseSpec\":{" @@ -400,7 +400,7 @@ public class DataSchemaTest extends InitializedNullHandlingTest DataSchema.class ); - Assert.assertEquals(actual.getDataSource(), TaskIdUtilsTest.VALID_ID_CHARS); + Assert.assertEquals(actual.getDataSource(), IdUtilsTest.VALID_ID_CHARS); Assert.assertEquals( actual.getParser().getParseSpec(), new JSONParseSpec( @@ -482,7 +482,7 @@ public class DataSchemaTest extends InitializedNullHandlingTest ); DataSchema originalSchema = new DataSchema( - TaskIdUtilsTest.VALID_ID_CHARS, + IdUtilsTest.VALID_ID_CHARS, parser, new AggregatorFactory[]{ new DoubleSumAggregatorFactory("metric1", "col1"), @@ -522,7 +522,7 @@ public class DataSchemaTest extends InitializedNullHandlingTest ); TestModifiedDataSchema originalSchema = new TestModifiedDataSchema( - TaskIdUtilsTest.VALID_ID_CHARS, + IdUtilsTest.VALID_ID_CHARS, null, null, new AggregatorFactory[]{ diff --git a/server/src/test/java/org/apache/druid/server/initialization/AuthorizerMapperModuleTest.java b/server/src/test/java/org/apache/druid/server/initialization/AuthorizerMapperModuleTest.java new file mode 100644 index 00000000000..5bc2231063d --- /dev/null +++ b/server/src/test/java/org/apache/druid/server/initialization/AuthorizerMapperModuleTest.java @@ -0,0 +1,61 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.server.initialization; + +import com.google.inject.Guice; +import com.google.inject.Injector; +import com.google.inject.Scopes; +import org.apache.druid.guice.LazySingleton; +import org.apache.druid.server.security.AuthValidator; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +import javax.validation.Validation; +import javax.validation.Validator; + +public class AuthorizerMapperModuleTest +{ + private AuthorizerMapperModule target; + private Injector injector; + + @Before + public void setUp() + { + target = new AuthorizerMapperModule(); + injector = Guice.createInjector( + (binder) -> { + binder.bind(Validator.class).toInstance(Validation.buildDefaultValidatorFactory().getValidator()); + binder.bindScope(LazySingleton.class, Scopes.SINGLETON); + }, + target + ); + } + + @Test + public void testAuthorizerNameValidatorIsInjectedAsSingleton() + { + AuthValidator authValidator = + injector.getInstance(AuthValidator.class); + AuthValidator other = + injector.getInstance(AuthValidator.class); + Assert.assertSame(authValidator, other); + } +} diff --git a/server/src/test/java/org/apache/druid/server/security/AuthValidatorTest.java b/server/src/test/java/org/apache/druid/server/security/AuthValidatorTest.java new file mode 100644 index 00000000000..3edfec1b5d0 --- /dev/null +++ b/server/src/test/java/org/apache/druid/server/security/AuthValidatorTest.java @@ -0,0 +1,99 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.server.security; + +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; + +public class AuthValidatorTest +{ + @Rule + public ExpectedException expectedException = ExpectedException.none(); + + public AuthValidator target; + + @Before + public void setUp() + { + target = new AuthValidator(); + } + + @Test + public void testAuthorizerNameWithEmptyIsInvalid() + { + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("authorizerName cannot be null or empty."); + target.validateAuthorizerName(""); + } + + @Test + public void testAuthorizerNameWithNullIsInvalid() + { + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("authorizerName cannot be null or empty."); + target.validateAuthorizerName(null); + } + + @Test + public void testAuthorizerNameStartsWithDotIsInValid() + { + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("authorizerName cannot start with the '.' character."); + target.validateAuthorizerName(".test"); + } + + @Test + public void testAuthorizerNameWithSlashIsInvalid() + { + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("authorizerName cannot contain the '/' character."); + target.validateAuthorizerName("tes/t"); + } + + @Test + public void testAuthorizerNameWithWhitespaceIsInvalid() + { + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("authorizerName cannot contain whitespace character except space."); + target.validateAuthorizerName("tes\tt"); + } + + @Test + public void testAuthorizerNameWithAllowedCharactersIsValid() + { + target.validateAuthorizerName("t.e.$\\, Россия 한국中国!?"); + } + + @Test + public void testAuthenticatorNameWithAllowedCharactersIsValid() + { + target.validateAuthenticatorName("t.e.$\\, Россия 한국中国!?"); + } + + @Test + public void testAuthenticatorNameWithWhitespaceIsInvalid() + { + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("authenticatorName cannot contain whitespace character except space."); + target.validateAuthenticatorName("tes\tt"); + } +}