diff --git a/extensions-contrib/grpc-query/src/main/java/org/apache/druid/grpc/server/QueryDriver.java b/extensions-contrib/grpc-query/src/main/java/org/apache/druid/grpc/server/QueryDriver.java index 442563d305be..10559026af54 100644 --- a/extensions-contrib/grpc-query/src/main/java/org/apache/druid/grpc/server/QueryDriver.java +++ b/extensions-contrib/grpc-query/src/main/java/org/apache/druid/grpc/server/QueryDriver.java @@ -148,7 +148,7 @@ private QueryResponse runNativeQuery(QueryRequest request, AuthenticationResult try { queryLifecycle.initialize(query); AuthorizationResult authorizationResult = queryLifecycle.authorize(authResult); - if (!authorizationResult.isAllowed()) { + if (!authorizationResult.getPermissionErrorMessage(true).isPresent()) { throw new ForbiddenException(Access.DEFAULT_ERROR_MESSAGE); } queryResponse = queryLifecycle.execute(); diff --git a/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/BasicSecurityResourceFilter.java b/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/BasicSecurityResourceFilter.java index 1c7d374da308..98046bc931e1 100644 --- a/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/BasicSecurityResourceFilter.java +++ b/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/BasicSecurityResourceFilter.java @@ -60,14 +60,14 @@ public ContainerRequest filter(ContainerRequest request) getAuthorizerMapper() ); - if (!authResult.isAllowed()) { + authResult.getPermissionErrorMessage(true).ifPresent(error -> { throw new WebApplicationException( Response.status(Response.Status.FORBIDDEN) .type(MediaType.TEXT_PLAIN) - .entity(StringUtils.format("Access-Check-Result: %s", authResult.getFailureMessage())) + .entity(StringUtils.format("Access-Check-Result: %s", error)) .build() ); - } + }); return request; } diff --git a/extensions-core/druid-catalog/src/main/java/org/apache/druid/catalog/http/CatalogResource.java b/extensions-core/druid-catalog/src/main/java/org/apache/druid/catalog/http/CatalogResource.java index 0eb9ebf58867..0435ef9e2f8d 100644 --- a/extensions-core/druid-catalog/src/main/java/org/apache/druid/catalog/http/CatalogResource.java +++ b/extensions-core/druid-catalog/src/main/java/org/apache/druid/catalog/http/CatalogResource.java @@ -59,7 +59,6 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; -import java.util.Objects; import java.util.stream.Collectors; /** @@ -582,9 +581,9 @@ private void authorizeTable( private void authorize(String resource, String key, Action action, HttpServletRequest request) { final AuthorizationResult authResult = authorizeAccess(resource, key, action, request); - if (!authResult.isAllowed()) { - throw new ForbiddenException(Objects.requireNonNull(authResult.getFailureMessage())); - } + authResult.getPermissionErrorMessage(true).ifPresent(error -> { + throw new ForbiddenException(error); + }); } private AuthorizationResult authorizeAccess(String resource, String key, Action action, HttpServletRequest request) diff --git a/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/dart/controller/http/DartSqlResource.java b/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/dart/controller/http/DartSqlResource.java index 97a1f32f66a9..3630e2c1d056 100644 --- a/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/dart/controller/http/DartSqlResource.java +++ b/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/dart/controller/http/DartSqlResource.java @@ -175,7 +175,8 @@ public GetQueriesResponse doGetRunningQueries( queries.sort(Comparator.comparing(DartQueryInfo::getStartTime).thenComparing(DartQueryInfo::getDartQueryId)); final GetQueriesResponse response; - if (stateReadAccess.isAllowed()) { + boolean hasFullPermission = !stateReadAccess.getPermissionErrorMessage(true).isPresent(); + if (hasFullPermission) { // User can READ STATE, so they can see all running queries, as well as authentication details. response = new GetQueriesResponse(queries); } else { @@ -245,9 +246,9 @@ public Response cancelQuery( return Response.status(Response.Status.ACCEPTED).build(); } - final AuthorizationResult access = authorizeCancellation(req, cancelables); + final AuthorizationResult authResult = authorizeCancellation(req, cancelables); - if (access.isAllowed()) { + if (!authResult.getPermissionErrorMessage(true).isPresent()) { sqlLifecycleManager.removeAll(sqlQueryId, cancelables); // Don't call cancel() on the cancelables. That just cancels native queries, which is useless here. Instead, diff --git a/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/dart/controller/sql/DartQueryMaker.java b/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/dart/controller/sql/DartQueryMaker.java index 66686f7640f9..ba0c503f1d76 100644 --- a/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/dart/controller/sql/DartQueryMaker.java +++ b/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/dart/controller/sql/DartQueryMaker.java @@ -52,6 +52,7 @@ import org.apache.druid.msq.sql.MSQTaskQueryMaker; import org.apache.druid.segment.column.ColumnType; import org.apache.druid.server.QueryResponse; +import org.apache.druid.server.security.ForbiddenException; import org.apache.druid.sql.calcite.planner.PlannerContext; import org.apache.druid.sql.calcite.rel.DruidQuery; import org.apache.druid.sql.calcite.run.QueryMaker; @@ -127,6 +128,9 @@ public DartQueryMaker( @Override public QueryResponse runQuery(DruidQuery druidQuery) { + plannerContext.getAuthorizationResult().getPermissionErrorMessage(true).ifPresent(error -> { + throw new ForbiddenException(error); + }); final MSQSpec querySpec = MSQTaskQueryMaker.makeQuerySpec( null, druidQuery, diff --git a/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/rpc/MSQResourceUtils.java b/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/rpc/MSQResourceUtils.java index 7d9779b723f5..795f0fbdaa85 100644 --- a/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/rpc/MSQResourceUtils.java +++ b/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/rpc/MSQResourceUtils.java @@ -27,7 +27,6 @@ import javax.servlet.http.HttpServletRequest; import java.util.List; -import java.util.Objects; /** * Utility methods for MSQ resources such as {@link ControllerResource}. @@ -42,15 +41,15 @@ public static void authorizeAdminRequest( { final List resourceActions = permissionMapper.getAdminPermissions(); - AuthorizationResult access = AuthorizationUtils.authorizeAllResourceActions( + AuthorizationResult authResult = AuthorizationUtils.authorizeAllResourceActions( request, resourceActions, authorizerMapper ); - if (!access.isAllowed()) { - throw new ForbiddenException(Objects.requireNonNull(access.getFailureMessage())); - } + authResult.getPermissionErrorMessage(true).ifPresent(error -> { + throw new ForbiddenException(error); + }); } public static void authorizeQueryRequest( @@ -62,14 +61,14 @@ public static void authorizeQueryRequest( { final List resourceActions = permissionMapper.getQueryPermissions(queryId); - AuthorizationResult access = AuthorizationUtils.authorizeAllResourceActions( + AuthorizationResult authResult = AuthorizationUtils.authorizeAllResourceActions( request, resourceActions, authorizerMapper ); - if (!access.isAllowed()) { - throw new ForbiddenException(Objects.requireNonNull(access.getFailureMessage())); - } + authResult.getPermissionErrorMessage(true).ifPresent(error -> { + throw new ForbiddenException(error); + }); } } diff --git a/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/sql/MSQTaskQueryMaker.java b/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/sql/MSQTaskQueryMaker.java index 5462b9917376..e6ecd5a5785f 100644 --- a/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/sql/MSQTaskQueryMaker.java +++ b/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/sql/MSQTaskQueryMaker.java @@ -54,6 +54,7 @@ import org.apache.druid.segment.column.ColumnType; import org.apache.druid.server.QueryResponse; import org.apache.druid.server.lookup.cache.LookupLoadingSpec; +import org.apache.druid.server.security.ForbiddenException; import org.apache.druid.sql.calcite.parser.DruidSqlIngest; import org.apache.druid.sql.calcite.parser.DruidSqlInsert; import org.apache.druid.sql.calcite.parser.DruidSqlReplace; @@ -116,6 +117,9 @@ public class MSQTaskQueryMaker implements QueryMaker @Override public QueryResponse runQuery(final DruidQuery druidQuery) { + plannerContext.getAuthorizationResult().getPermissionErrorMessage(true).ifPresent(error -> { + throw new ForbiddenException(error); + }); Hook.QUERY_PLAN.run(druidQuery.getQuery()); plannerContext.dispatchHook(DruidHook.NATIVE_PLAN, druidQuery.getQuery()); diff --git a/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/sql/resources/SqlStatementResource.java b/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/sql/resources/SqlStatementResource.java index 33527e6eb903..0bf82ea97cb9 100644 --- a/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/sql/resources/SqlStatementResource.java +++ b/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/sql/resources/SqlStatementResource.java @@ -675,21 +675,21 @@ private MSQControllerTask getMSQControllerTaskAndCheckPermission( return msqControllerTask; } - AuthorizationResult access = AuthorizationUtils.authorizeAllResourceActions( + AuthorizationResult authResult = AuthorizationUtils.authorizeAllResourceActions( authenticationResult, Collections.singletonList(new ResourceAction(Resource.STATE_RESOURCE, forAction)), authorizerMapper ); - if (access.isAllowed()) { - return msqControllerTask; - } + authResult.getPermissionErrorMessage(true).ifPresent(error -> { + throw new ForbiddenException(StringUtils.format( + "The current user[%s] cannot view query id[%s] since the query is owned by another user", + currentUser, + queryId + )); + }); - throw new ForbiddenException(StringUtils.format( - "The current user[%s] cannot view query id[%s] since the query is owned by another user", - currentUser, - queryId - )); + return msqControllerTask; } /** diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/common/task/IndexTaskUtils.java b/indexing-service/src/main/java/org/apache/druid/indexing/common/task/IndexTaskUtils.java index 70b1b50e2daf..b1bd4af65b33 100644 --- a/indexing-service/src/main/java/org/apache/druid/indexing/common/task/IndexTaskUtils.java +++ b/indexing-service/src/main/java/org/apache/druid/indexing/common/task/IndexTaskUtils.java @@ -43,7 +43,6 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; -import java.util.Objects; public class IndexTaskUtils { @@ -80,12 +79,11 @@ public static AuthorizationResult datasourceAuthorizationCheck( action ); - AuthorizationResult access = AuthorizationUtils.authorizeResourceAction(req, resourceAction, authorizerMapper); - if (!access.isAllowed()) { - throw new ForbiddenException(Objects.requireNonNull(access.getFailureMessage())); - } - - return access; + AuthorizationResult authResult = AuthorizationUtils.authorizeResourceAction(req, resourceAction, authorizerMapper); + authResult.getPermissionErrorMessage(true).ifPresent(error -> { + throw new ForbiddenException(error); + }); + return authResult; } public static void setTaskDimensions(final ServiceMetricEvent.Builder metricBuilder, final Task task) diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/OverlordResource.java b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/OverlordResource.java index b432132e0f27..87e7e581b15a 100644 --- a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/OverlordResource.java +++ b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/OverlordResource.java @@ -101,7 +101,6 @@ import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.Set; /** @@ -183,10 +182,9 @@ public Response taskPost( resourceActions, authorizerMapper ); - - if (!authResult.isAllowed()) { - throw new ForbiddenException(Objects.requireNonNull(authResult.getFailureMessage())); - } + authResult.getPermissionErrorMessage(true).ifPresent(error -> { + throw new ForbiddenException(error); + }); return asLeaderWith( taskMaster.getTaskQueue(), @@ -615,17 +613,18 @@ public Response getTasks( resourceAction, authorizerMapper ); - if (!authResult.isAllowed()) { + + authResult.getPermissionErrorMessage(true).ifPresent(error -> { throw new WebApplicationException( Response.status(Response.Status.FORBIDDEN) .type(MediaType.TEXT_PLAIN) .entity(StringUtils.format( "Access-Check-Result: %s", - Objects.requireNonNull(authResult.getFailureMessage()) + error )) .build() ); - } + }); } return asLeaderWith( @@ -667,9 +666,9 @@ public Response killPendingSegments( authorizerMapper ); - if (!authResult.isAllowed()) { - throw new ForbiddenException(Objects.requireNonNull(authResult.getFailureMessage())); - } + authResult.getPermissionErrorMessage(true).ifPresent(error -> { + throw new ForbiddenException(error); + }); if (overlord.isLeader()) { try { diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/security/SupervisorResourceFilter.java b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/security/SupervisorResourceFilter.java index b0647623c509..ddca13c24222 100644 --- a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/security/SupervisorResourceFilter.java +++ b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/security/SupervisorResourceFilter.java @@ -41,7 +41,6 @@ import javax.ws.rs.core.MediaType; import javax.ws.rs.core.PathSegment; import javax.ws.rs.core.Response; -import java.util.Objects; public class SupervisorResourceFilter extends AbstractResourceFilter { @@ -104,9 +103,9 @@ public boolean apply(PathSegment input) getAuthorizerMapper() ); - if (!authResult.isAllowed()) { - throw new ForbiddenException(Objects.requireNonNull(authResult.getFailureMessage())); - } + authResult.getPermissionErrorMessage(true).ifPresent(error -> { + throw new ForbiddenException(error); + }); return request; } 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 bd63c1197814..7e431dca8797 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 @@ -40,7 +40,6 @@ import javax.ws.rs.WebApplicationException; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; -import java.util.Objects; /** * Use this ResourceFilter when the datasource information is present after "task" segment in the request Path @@ -99,9 +98,9 @@ public ContainerRequest filter(ContainerRequest request) getAuthorizerMapper() ); - if (!authResult.isAllowed()) { - throw new ForbiddenException(Objects.requireNonNull(authResult.getFailureMessage())); - } + authResult.getPermissionErrorMessage(true).ifPresent(error -> { + throw new ForbiddenException(error); + }); return request; } diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/sampler/SamplerResource.java b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/sampler/SamplerResource.java index 2b4fa1565328..de2f26c3923c 100644 --- a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/sampler/SamplerResource.java +++ b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/sampler/SamplerResource.java @@ -40,7 +40,6 @@ import javax.ws.rs.core.Context; import javax.ws.rs.core.MediaType; import java.util.HashSet; -import java.util.Objects; import java.util.Set; @Path("/druid/indexer/v1/sampler") @@ -79,9 +78,9 @@ public SamplerResponse post(final SamplerSpec sampler, @Context final HttpServle authorizerMapper ); - if (!authResult.isAllowed()) { - throw new ForbiddenException(Objects.requireNonNull(authResult.getFailureMessage())); - } + authResult.getPermissionErrorMessage(true).ifPresent(error -> { + throw new ForbiddenException(error); + }); return sampler.sample(); } } diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/supervisor/SupervisorResource.java b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/supervisor/SupervisorResource.java index dc1bc41bbebd..487dbd4eeaa3 100644 --- a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/supervisor/SupervisorResource.java +++ b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/supervisor/SupervisorResource.java @@ -148,9 +148,9 @@ public Response specPost(final SupervisorSpec spec, @Context final HttpServletRe authorizerMapper ); - if (!authResult.isAllowed()) { - throw new ForbiddenException(Objects.requireNonNull(authResult.getFailureMessage())); - } + authResult.getPermissionErrorMessage(true).ifPresent(error -> { + throw new ForbiddenException(error); + }); manager.createOrUpdateAndStartSupervisor(spec); diff --git a/server/src/main/java/org/apache/druid/segment/realtime/ChatHandlers.java b/server/src/main/java/org/apache/druid/segment/realtime/ChatHandlers.java index bcfff65b1852..28074d037b4b 100644 --- a/server/src/main/java/org/apache/druid/segment/realtime/ChatHandlers.java +++ b/server/src/main/java/org/apache/druid/segment/realtime/ChatHandlers.java @@ -29,7 +29,6 @@ import org.apache.druid.server.security.ResourceType; import javax.servlet.http.HttpServletRequest; -import java.util.Objects; public class ChatHandlers { @@ -50,11 +49,11 @@ public static AuthorizationResult authorizationCheck( action ); - AuthorizationResult access = AuthorizationUtils.authorizeResourceAction(req, resourceAction, authorizerMapper); - if (!access.isAllowed()) { - throw new ForbiddenException(Objects.requireNonNull(access.getFailureMessage())); - } + AuthorizationResult authResult = AuthorizationUtils.authorizeResourceAction(req, resourceAction, authorizerMapper); + authResult.getPermissionErrorMessage(true).ifPresent(error -> { + throw new ForbiddenException(error); + }); - return access; + return authResult; } } diff --git a/server/src/main/java/org/apache/druid/server/QueryLifecycle.java b/server/src/main/java/org/apache/druid/server/QueryLifecycle.java index c314a47554f6..a0e1f0eded14 100644 --- a/server/src/main/java/org/apache/druid/server/QueryLifecycle.java +++ b/server/src/main/java/org/apache/druid/server/QueryLifecycle.java @@ -66,7 +66,6 @@ import java.util.HashSet; import java.util.LinkedHashMap; import java.util.Map; -import java.util.Objects; import java.util.Set; import java.util.UUID; import java.util.concurrent.TimeUnit; @@ -156,10 +155,6 @@ public QueryResponse runSimple( final QueryResponse queryResponse; try { preAuthorized(authenticationResult, authorizationResult); - if (!authorizationResult.isAllowed()) { - throw new ISE(Objects.requireNonNull(authorizationResult.getFailureMessage())); - } - queryResponse = execute(); results = queryResponse.getResults(); } @@ -280,9 +275,13 @@ private void preAuthorized( final AuthorizationResult authorizationResult ) { - // gotta transition those states, even if we are already authorized + // The authorization have already been checked previously (or skipped). This just follows the state transition + // process, should not throw unauthorized error. transition(State.INITIALIZED, State.AUTHORIZING); doAuthorize(authenticationResult, authorizationResult); + if (!state.equals(State.AUTHORIZED)) { + throw DruidException.defensive("Unexpected state [%s], expecting [%s].", state, State.AUTHORIZED); + } } private AuthorizationResult doAuthorize( @@ -293,7 +292,7 @@ private AuthorizationResult doAuthorize( Preconditions.checkNotNull(authenticationResult, "authenticationResult"); Preconditions.checkNotNull(authorizationResult, "authorizationResult"); - if (!authorizationResult.isAllowed()) { + if (authorizationResult.getPermissionErrorMessage(false).isPresent()) { // Not authorized; go straight to Jail, do not pass Go. transition(State.AUTHORIZING, State.UNAUTHORIZED); } else { diff --git a/server/src/main/java/org/apache/druid/server/QueryResource.java b/server/src/main/java/org/apache/druid/server/QueryResource.java index 94f2d1f1f57e..c7c0a6e1ae0b 100644 --- a/server/src/main/java/org/apache/druid/server/QueryResource.java +++ b/server/src/main/java/org/apache/druid/server/QueryResource.java @@ -74,7 +74,6 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.util.Objects; import java.util.Set; import java.util.TreeSet; import java.util.concurrent.atomic.AtomicLong; @@ -159,9 +158,9 @@ public Response cancelQuery(@PathParam("id") String queryId, @Context final Http authorizerMapper ); - if (!authResult.isAllowed()) { - throw new ForbiddenException(Objects.requireNonNull(authResult.getFailureMessage())); - } + authResult.getPermissionErrorMessage(true).ifPresent(error -> { + throw new ForbiddenException(error); + }); queryScheduler.cancelQuery(queryId); return Response.status(Response.Status.ACCEPTED).build(); @@ -215,9 +214,9 @@ public Response doPost( return io.getResponseWriter().buildNonOkResponse(qe.getFailType().getExpectedStatus(), qe); } - if (!authResult.isAllowed()) { - throw new ForbiddenException(Objects.requireNonNull(authResult.getFailureMessage())); - } + authResult.getPermissionErrorMessage(true).ifPresent(error -> { + throw new ForbiddenException(error); + }); final QueryResourceQueryResultPusher pusher = new QueryResourceQueryResultPusher(req, queryLifecycle, io); return pusher.push(); diff --git a/server/src/main/java/org/apache/druid/server/http/security/ConfigResourceFilter.java b/server/src/main/java/org/apache/druid/server/http/security/ConfigResourceFilter.java index 8184b165df4f..6c357900da2a 100644 --- a/server/src/main/java/org/apache/druid/server/http/security/ConfigResourceFilter.java +++ b/server/src/main/java/org/apache/druid/server/http/security/ConfigResourceFilter.java @@ -29,8 +29,6 @@ import org.apache.druid.server.security.ResourceAction; import org.apache.druid.server.security.ResourceType; -import java.util.Objects; - /** * Use this ResourceFilter at end points where Druid Cluster configuration is read or written * Here are some example paths where this filter is used - @@ -64,9 +62,9 @@ public ContainerRequest filter(ContainerRequest request) getAuthorizerMapper() ); - if (!authResult.isAllowed()) { - throw new ForbiddenException(Objects.requireNonNull(authResult.getFailureMessage())); - } + authResult.getPermissionErrorMessage(true).ifPresent(error -> { + throw new ForbiddenException(error); + }); return request; } diff --git a/server/src/main/java/org/apache/druid/server/http/security/DatasourceResourceFilter.java b/server/src/main/java/org/apache/druid/server/http/security/DatasourceResourceFilter.java index 970ba60223d8..4c239a902486 100644 --- a/server/src/main/java/org/apache/druid/server/http/security/DatasourceResourceFilter.java +++ b/server/src/main/java/org/apache/druid/server/http/security/DatasourceResourceFilter.java @@ -33,7 +33,6 @@ import javax.ws.rs.core.PathSegment; import java.util.List; -import java.util.Objects; /** * Use this resource filter for API endpoints that contain {@link #DATASOURCES_PATH_SEGMENT} in their request path. @@ -64,9 +63,9 @@ public ContainerRequest filter(ContainerRequest request) getAuthorizerMapper() ); - if (!authResult.isAllowed()) { - throw new ForbiddenException(Objects.requireNonNull(authResult.getFailureMessage())); - } + authResult.getPermissionErrorMessage(true).ifPresent(error -> { + throw new ForbiddenException(error); + }); return request; } diff --git a/server/src/main/java/org/apache/druid/server/http/security/RulesResourceFilter.java b/server/src/main/java/org/apache/druid/server/http/security/RulesResourceFilter.java index b90d967a91d7..04478d4cdac8 100644 --- a/server/src/main/java/org/apache/druid/server/http/security/RulesResourceFilter.java +++ b/server/src/main/java/org/apache/druid/server/http/security/RulesResourceFilter.java @@ -33,7 +33,6 @@ import org.apache.druid.server.security.ResourceType; import javax.ws.rs.core.PathSegment; -import java.util.Objects; /** @@ -82,9 +81,9 @@ public boolean apply(PathSegment input) getAuthorizerMapper() ); - if (!authResult.isAllowed()) { - throw new ForbiddenException(Objects.requireNonNull(authResult.getFailureMessage())); - } + authResult.getPermissionErrorMessage(true).ifPresent(error -> { + throw new ForbiddenException(error); + }); return request; } diff --git a/server/src/main/java/org/apache/druid/server/http/security/StateResourceFilter.java b/server/src/main/java/org/apache/druid/server/http/security/StateResourceFilter.java index 890e37452386..600fd4f4ebe9 100644 --- a/server/src/main/java/org/apache/druid/server/http/security/StateResourceFilter.java +++ b/server/src/main/java/org/apache/druid/server/http/security/StateResourceFilter.java @@ -28,7 +28,6 @@ import org.apache.druid.server.security.Resource; import org.apache.druid.server.security.ResourceAction; -import java.util.Objects; /** * Use this ResourceFilter at end points where Druid Cluster State is read or written @@ -67,9 +66,9 @@ public ContainerRequest filter(ContainerRequest request) getAuthorizerMapper() ); - if (!authResult.isAllowed()) { - throw new ForbiddenException(Objects.requireNonNull(authResult.getFailureMessage())); - } + authResult.getPermissionErrorMessage(true).ifPresent(error -> { + throw new ForbiddenException(error); + }); return request; } diff --git a/server/src/main/java/org/apache/druid/server/security/AuthorizationResult.java b/server/src/main/java/org/apache/druid/server/security/AuthorizationResult.java index d4c16615daeb..ed95dfbf9618 100644 --- a/server/src/main/java/org/apache/druid/server/security/AuthorizationResult.java +++ b/server/src/main/java/org/apache/druid/server/security/AuthorizationResult.java @@ -21,11 +21,13 @@ import com.google.common.collect.ImmutableMap; import org.apache.druid.query.filter.DimFilter; +import org.apache.druid.query.filter.TrueDimFilter; import javax.annotation.Nonnull; import javax.annotation.Nullable; import java.util.Collections; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -98,23 +100,28 @@ public AuthorizationResult withResourceActions( ) { return new AuthorizationResult( - isAllowed(), - getFailureMessage(), + isAllowed, + failureMessage, ImmutableMap.copyOf(getPolicyFilters()), sqlResourceActions, allResourceActions ); } - public boolean isAllowed() + public Optional getPermissionErrorMessage(boolean policyFilterNotPermitted) { - return isAllowed; - } - - @Nullable - public String getFailureMessage() - { - return failureMessage; + if (!isAllowed) { + return Optional.of(Objects.requireNonNull(failureMessage)); + } + + if (policyFilterNotPermitted && policyFilters.values() + .stream() + .flatMap(Optional::stream) + .anyMatch(filter -> !(filter instanceof TrueDimFilter))) { + return Optional.of(Access.DEFAULT_ERROR_MESSAGE); + } + + return Optional.empty(); } public Map> getPolicyFilters() diff --git a/server/src/main/java/org/apache/druid/server/security/AuthorizationUtils.java b/server/src/main/java/org/apache/druid/server/security/AuthorizationUtils.java index 5fbac8a5b668..47b7d0a84959 100644 --- a/server/src/main/java/org/apache/druid/server/security/AuthorizationUtils.java +++ b/server/src/main/java/org/apache/druid/server/security/AuthorizationUtils.java @@ -227,14 +227,17 @@ public static AuthorizationResult authorizeAllResourceActions( throw new ISE("Request already had authorization check."); } - AuthorizationResult access = authorizeAllResourceActions( + AuthorizationResult authResult = authorizeAllResourceActions( authenticationResultFromRequest(request), resourceActions, authorizerMapper ); - request.setAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED, access.isAllowed()); - return access; + request.setAttribute( + AuthConfig.DRUID_AUTHORIZATION_CHECKED, + !authResult.getPermissionErrorMessage(false).isPresent() + ); + return authResult; } /** diff --git a/server/src/test/java/org/apache/druid/server/QueryLifecycleTest.java b/server/src/test/java/org/apache/druid/server/QueryLifecycleTest.java index e745def91bd6..a816345fed7b 100644 --- a/server/src/test/java/org/apache/druid/server/QueryLifecycleTest.java +++ b/server/src/test/java/org/apache/druid/server/QueryLifecycleTest.java @@ -224,7 +224,7 @@ public void testAuthorizedWithNoPolicyRestriction() .build(); QueryLifecycle lifecycle = createLifecycle(authConfig); lifecycle.initialize(query); - Assert.assertTrue(lifecycle.authorize(authenticationResult).isAllowed()); + Assert.assertTrue(lifecycle.authorize(authenticationResult).getPermissionErrorMessage(false).isEmpty()); lifecycle.execute(); lifecycle = createLifecycle(authConfig); @@ -265,7 +265,7 @@ public void testAuthorizedWithAlwaysTruePolicyRestriction() .build(); QueryLifecycle lifecycle = createLifecycle(authConfig); lifecycle.initialize(query); - Assert.assertTrue(lifecycle.authorize(authenticationResult).isAllowed()); + Assert.assertTrue(lifecycle.authorize(authenticationResult).getPermissionErrorMessage(false).isEmpty()); lifecycle.execute(); lifecycle = createLifecycle(authConfig); @@ -310,7 +310,7 @@ public void testAuthorizedWithOnePolicyRestriction() .build(); QueryLifecycle lifecycle = createLifecycle(authConfig); lifecycle.initialize(query); - Assert.assertTrue(lifecycle.authorize(authenticationResult).isAllowed()); + Assert.assertTrue(lifecycle.authorize(authenticationResult).getPermissionErrorMessage(false).isEmpty()); lifecycle.execute(); lifecycle = createLifecycle(authConfig); @@ -475,11 +475,11 @@ public void testAuthorizeQueryContext_authorized() revisedContext ); - Assert.assertTrue(lifecycle.authorize(mockRequest()).isAllowed()); + Assert.assertTrue(lifecycle.authorize(mockRequest()).getPermissionErrorMessage(false).isEmpty()); lifecycle = createLifecycle(authConfig); lifecycle.initialize(query); - Assert.assertTrue(lifecycle.authorize(authenticationResult).isAllowed()); + Assert.assertTrue(lifecycle.authorize(authenticationResult).getPermissionErrorMessage(false).isEmpty()); } @Test @@ -521,11 +521,11 @@ public void testAuthorizeQueryContext_notAuthorized() .build(); QueryLifecycle lifecycle = createLifecycle(authConfig); lifecycle.initialize(query); - Assert.assertFalse(lifecycle.authorize(mockRequest()).isAllowed()); + Assert.assertFalse(lifecycle.authorize(mockRequest()).getPermissionErrorMessage(false).isEmpty()); lifecycle = createLifecycle(authConfig); lifecycle.initialize(query); - Assert.assertFalse(lifecycle.authorize(authenticationResult).isAllowed()); + Assert.assertFalse(lifecycle.authorize(authenticationResult).getPermissionErrorMessage(false).isEmpty()); } @Test @@ -571,11 +571,11 @@ public void testAuthorizeQueryContext_unsecuredKeys() revisedContext ); - Assert.assertTrue(lifecycle.authorize(mockRequest()).isAllowed()); + Assert.assertTrue(lifecycle.authorize(mockRequest()).getPermissionErrorMessage(false).isEmpty()); lifecycle = createLifecycle(authConfig); lifecycle.initialize(query); - Assert.assertTrue(lifecycle.authorize(authenticationResult).isAllowed()); + Assert.assertTrue(lifecycle.authorize(authenticationResult).getPermissionErrorMessage(false).isEmpty()); } @Test @@ -622,11 +622,11 @@ public void testAuthorizeQueryContext_securedKeys() revisedContext ); - Assert.assertTrue(lifecycle.authorize(mockRequest()).isAllowed()); + Assert.assertTrue(lifecycle.authorize(mockRequest()).getPermissionErrorMessage(false).isEmpty()); lifecycle = createLifecycle(authConfig); lifecycle.initialize(query); - Assert.assertTrue(lifecycle.authorize(authenticationResult).isAllowed()); + Assert.assertTrue(lifecycle.authorize(authenticationResult).getPermissionErrorMessage(false).isEmpty()); } @Test @@ -671,11 +671,11 @@ public void testAuthorizeQueryContext_securedKeysNotAuthorized() .build(); QueryLifecycle lifecycle = createLifecycle(authConfig); lifecycle.initialize(query); - Assert.assertFalse(lifecycle.authorize(mockRequest()).isAllowed()); + Assert.assertFalse(lifecycle.authorize(mockRequest()).getPermissionErrorMessage(false).isEmpty()); lifecycle = createLifecycle(authConfig); lifecycle.initialize(query); - Assert.assertFalse(lifecycle.authorize(authenticationResult).isAllowed()); + Assert.assertFalse(lifecycle.authorize(authenticationResult).getPermissionErrorMessage(false).isEmpty()); } @Test @@ -731,11 +731,11 @@ public void testAuthorizeLegacyQueryContext_authorized() Assert.assertTrue(revisedContext.containsKey("baz")); Assert.assertTrue(revisedContext.containsKey("queryId")); - Assert.assertTrue(lifecycle.authorize(mockRequest()).isAllowed()); + Assert.assertTrue(lifecycle.authorize(mockRequest()).getPermissionErrorMessage(false).isEmpty()); lifecycle = createLifecycle(authConfig); lifecycle.initialize(query); - Assert.assertTrue(lifecycle.authorize(mockRequest()).isAllowed()); + Assert.assertTrue(lifecycle.authorize(mockRequest()).getPermissionErrorMessage(false).isEmpty()); } public static Query queryMatchDataSource(DataSource dataSource) diff --git a/sql/src/main/java/org/apache/druid/sql/AbstractStatement.java b/sql/src/main/java/org/apache/druid/sql/AbstractStatement.java index b3f04bf0fdda..90962fa05716 100644 --- a/sql/src/main/java/org/apache/druid/sql/AbstractStatement.java +++ b/sql/src/main/java/org/apache/druid/sql/AbstractStatement.java @@ -151,9 +151,9 @@ protected void authorize( // Authentication is done by the planner using the function provided // here. The planner ensures that this step is done before planning. authResult = planner.authorize(authorizer, contextResources); - if (!authResult.isAllowed()) { - throw new ForbiddenException(Objects.requireNonNull(authResult.getFailureMessage())); - } + authResult.getPermissionErrorMessage(false).ifPresent(error -> { + throw new ForbiddenException(error); + }); } /** diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/schema/SystemSchema.java b/sql/src/main/java/org/apache/druid/sql/calcite/schema/SystemSchema.java index fe2e54fef83f..ac4c645560bc 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/schema/SystemSchema.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/schema/SystemSchema.java @@ -1147,15 +1147,15 @@ private static void checkStateReadAccessForServers( AuthorizerMapper authorizerMapper ) { - final AuthorizationResult stateAccess = AuthorizationUtils.authorizeAllResourceActions( + final AuthorizationResult authResult = AuthorizationUtils.authorizeAllResourceActions( authenticationResult, Collections.singletonList(new ResourceAction(Resource.STATE_RESOURCE, Action.READ)), authorizerMapper ); - if (!stateAccess.isAllowed()) { - throw new ForbiddenException("Insufficient permission to view servers: " - + Objects.requireNonNull(stateAccess.getFailureMessage())); - } + + authResult.getPermissionErrorMessage(true).ifPresent(error -> { + throw new ForbiddenException("Insufficient permission to view servers: " + error); + }); } /** diff --git a/sql/src/main/java/org/apache/druid/sql/http/SqlResource.java b/sql/src/main/java/org/apache/druid/sql/http/SqlResource.java index 9be9c3077129..b93de5ce9653 100644 --- a/sql/src/main/java/org/apache/druid/sql/http/SqlResource.java +++ b/sql/src/main/java/org/apache/druid/sql/http/SqlResource.java @@ -140,9 +140,9 @@ public Response cancelQuery( return Response.status(Status.NOT_FOUND).build(); } - final AuthorizationResult access = authorizeCancellation(req, lifecycles); + final AuthorizationResult authResult = authorizeCancellation(req, lifecycles); - if (access.isAllowed()) { + if (!authResult.getPermissionErrorMessage(true).isPresent()) { // should remove only the lifecycles in the snapshot. sqlLifecycleManager.removeAll(sqlQueryId, lifecycles); lifecycles.forEach(Cancelable::cancel);