This is an automated email from the ASF dual-hosted git repository.

apucher pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git


The following commit(s) were added to refs/heads/master by this push:
     new f6e26c2350 add @ManualAuthorization annotation for non-standard 
endpoints (#9252)
f6e26c2350 is described below

commit f6e26c2350d7b997fa6d4bd690bc9c9058875309
Author: Alexander Pucher <[email protected]>
AuthorDate: Wed Aug 24 21:56:13 2022 -0700

    add @ManualAuthorization annotation for non-standard endpoints (#9252)
    
    This PR adds a new annotation @ManualAuthorization for REST endpoints, 
which allows developers to skip the default authorization and deserialize 
payloads before manually invoking authorization, e.g. via 
AccessControlUtils.validatePermissions(). This annotation comes with obvious 
risks and should be used sparingly, as it enables requests to bypass most of 
the AuthFilter.
---
 .../pinot/controller/api/access/AccessControl.java | 16 +++---
 .../controller/api/access/AccessControlUtils.java  | 56 ++++++--------------
 .../api/access/AuthenticationFilter.java           |  8 ++-
 .../controller/api/access/ManualAuthorization.java | 36 +++++++++++++
 .../access/ZkBasicAuthAccessControlFactory.java    |  8 ---
 .../PinotAccessControlUserRestletResource.java     | 33 ++----------
 .../api/resources/PinotControllerAuthResource.java |  6 ---
 .../api/resources/PinotQueryResource.java          | 16 +++---
 .../api/resources/PinotSchemaRestletResource.java  | 18 +++++--
 .../PinotSegmentUploadDownloadRestletResource.java |  2 +-
 .../api/resources/PinotTableRestletResource.java   | 25 ++++++++-
 .../api/resources/TableConfigsRestletResource.java | 31 ++++++++++-
 .../pinot/controller/api/AccessControlTest.java    | 60 ----------------------
 .../pinot/controller/helix/ControllerTest.java     |  5 +-
 .../pinot/spi/utils/builder/TableNameBuilder.java  |  3 ++
 15 files changed, 156 insertions(+), 167 deletions(-)

diff --git 
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/access/AccessControl.java
 
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/access/AccessControl.java
index 3e19a9a45c..1b7c61993a 100644
--- 
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/access/AccessControl.java
+++ 
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/access/AccessControl.java
@@ -18,6 +18,7 @@
  */
 package org.apache.pinot.controller.api.access;
 
+import javax.annotation.Nullable;
 import javax.ws.rs.core.HttpHeaders;
 import org.apache.pinot.spi.annotations.InterfaceAudience;
 import org.apache.pinot.spi.annotations.InterfaceStability;
@@ -40,7 +41,9 @@ public interface AccessControl {
    * @return Whether the client has data access to the table
    */
   @Deprecated
-  boolean hasDataAccess(HttpHeaders httpHeaders, String tableName);
+  default boolean hasDataAccess(HttpHeaders httpHeaders, String tableName) {
+    return hasAccess(tableName, AccessType.READ, httpHeaders, null);
+  }
 
   /**
    * Return whether the client has permission to the given table
@@ -51,7 +54,8 @@ public interface AccessControl {
    * @param endpointUrl the request url for which this access control is called
    * @return whether the client has permission
    */
-  default boolean hasAccess(String tableName, AccessType accessType, 
HttpHeaders httpHeaders, String endpointUrl) {
+  default boolean hasAccess(@Nullable String tableName, AccessType accessType, 
HttpHeaders httpHeaders,
+      @Nullable String endpointUrl) {
     return true;
   }
 
@@ -63,12 +67,8 @@ public interface AccessControl {
    * @param endpointUrl the request url for which this access control is called
    * @return whether the client has permission
    */
-  default boolean hasAccess(AccessType accessType, HttpHeaders httpHeaders, 
String endpointUrl) {
-    return true;
-  }
-
-  default boolean hasAccess(HttpHeaders httpHeaders) {
-    return true;
+  default boolean hasAccess(AccessType accessType, HttpHeaders httpHeaders, 
@Nullable String endpointUrl) {
+    return hasAccess(null, accessType, httpHeaders, endpointUrl);
   }
 
   /**
diff --git 
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/access/AccessControlUtils.java
 
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/access/AccessControlUtils.java
index a19cad5054..59c0b3a9d4 100644
--- 
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/access/AccessControlUtils.java
+++ 
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/access/AccessControlUtils.java
@@ -50,56 +50,32 @@ public final class AccessControlUtils {
    */
   public static void validatePermission(@Nullable String tableName, AccessType 
accessType,
       @Nullable HttpHeaders httpHeaders, @Nullable String endpointUrl, 
AccessControl accessControl) {
-    String message = null;
+    String userMessage = getUserMessage(tableName, accessType, endpointUrl);
+    String rawTableName = TableNameBuilder.extractRawTableName(tableName);
+
     try {
-      if (StringUtils.isBlank(tableName)) {
-        message = String.format("%s '%s'", accessType, endpointUrl);
-        if (!accessControl.hasAccess(accessType, httpHeaders, endpointUrl)) {
-          accessDenied(message);
+      if (rawTableName == null) {
+        if (accessControl.hasAccess(accessType, httpHeaders, endpointUrl)) {
+          return;
         }
       } else {
-        message = String.format("%s '%s' for table '%s'", accessType, 
endpointUrl, tableName);
-        String rawTableName = TableNameBuilder.extractRawTableName(tableName);
-        if (!accessControl.hasAccess(rawTableName, accessType, httpHeaders, 
endpointUrl)) {
-          accessDenied(message);
+        if (accessControl.hasAccess(rawTableName, accessType, httpHeaders, 
endpointUrl)) {
+          return;
         }
       }
-    } catch (ControllerApplicationException e) {
-      throw e;
     } catch (Exception e) {
-      throw new ControllerApplicationException(LOGGER, "Caught exception while 
validating permission for " + message,
-          Response.Status.INTERNAL_SERVER_ERROR, e);
+      throw new ControllerApplicationException(LOGGER, "Caught exception while 
validating permission for "
+          + userMessage, Response.Status.INTERNAL_SERVER_ERROR, e);
     }
-  }
 
-  /**
-   * Validate permission for the given access type for a non-table level 
endpoint
-   *
-   * @param accessType type of the access
-   * @param httpHeaders HTTP headers containing requester identity required by 
access control object
-   * @param endpointUrl the request url for which this access control is called
-   * @param accessControl AccessControl object which does the actual validation
-   */
-  public static void validatePermission(AccessType accessType, @Nullable 
HttpHeaders httpHeaders,
-      @Nullable String endpointUrl, AccessControl accessControl) {
-    validatePermission(null, accessType, httpHeaders, endpointUrl, 
accessControl);
+    throw new ControllerApplicationException(LOGGER, "Permission is denied for 
" + userMessage,
+        Response.Status.FORBIDDEN);
   }
 
-  /**
-   * Validate permission for the given access type and endpointUrl
-   *
-   * @param httpHeaders HTTP headers containing requester identity required by 
access control object
-   * @param endpointUrl the request url for which this access control is called
-   */
-  public static void validatePermission(@Nullable HttpHeaders httpHeaders, 
@Nullable String endpointUrl,
-      AccessControl accessControl) {
-    if (!accessControl.hasAccess(httpHeaders)) {
-      accessDenied(endpointUrl);
+  private static String getUserMessage(String tableName, AccessType 
accessType, String endpointUrl) {
+    if (StringUtils.isBlank(tableName)) {
+      return String.format("%s '%s'", accessType, endpointUrl);
     }
-  }
-
-  private static void accessDenied(String resource) {
-    throw new ControllerApplicationException(LOGGER, "Permission is denied for 
" + resource,
-        Response.Status.FORBIDDEN);
+    return String.format("%s '%s' for table '%s'", accessType, endpointUrl, 
tableName);
   }
 }
diff --git 
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/access/AuthenticationFilter.java
 
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/access/AuthenticationFilter.java
index 8ebd1a2883..f0a804cefe 100644
--- 
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/access/AuthenticationFilter.java
+++ 
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/access/AuthenticationFilter.java
@@ -67,9 +67,10 @@ public class AuthenticationFilter implements 
ContainerRequestFilter {
   @Override
   public void filter(ContainerRequestContext requestContext)
       throws IOException {
+    Request request = _requestProvider.get();
     Method endpointMethod = _resourceInfo.getResourceMethod();
     AccessControl accessControl = _accessControlFactory.create();
-    String endpointUrl = _requestProvider.get().getRequestURL().toString();
+    String endpointUrl = 
request.getRequestURI().substring(request.getContextPath().length()); // 
extract path only
     UriInfo uriInfo = requestContext.getUriInfo();
 
     // exclude public/unprotected paths
@@ -82,6 +83,11 @@ public class AuthenticationFilter implements 
ContainerRequestFilter {
       return;
     }
 
+    // check if the method's authorization is disabled (i.e. performed 
manually within method)
+    if (endpointMethod.isAnnotationPresent(ManualAuthorization.class)) {
+      return;
+    }
+
     // Note that table name is extracted from "path parameters" or "query 
parameters" if it's defined as one of the
     // followings:
     //     - "tableName",
diff --git 
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/access/ManualAuthorization.java
 
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/access/ManualAuthorization.java
new file mode 100644
index 0000000000..e445d902a3
--- /dev/null
+++ 
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/access/ManualAuthorization.java
@@ -0,0 +1,36 @@
+/**
+ * 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.pinot.controller.api.access;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+
+/**
+ * Annotation to be used on top of REST endpoints. Methods annotated with this 
annotation don't perform default
+ * authorization via AuthenticationFilter. This is useful when performing 
authorization manually via calls to
+ * {@code AuthenticationFiler.validatePermissions()}
+ */
+@Retention(RetentionPolicy.RUNTIME)
+@Target(ElementType.METHOD)
+public @interface ManualAuthorization {
+}
diff --git 
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/access/ZkBasicAuthAccessControlFactory.java
 
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/access/ZkBasicAuthAccessControlFactory.java
index d032b98681..98c63623ab 100644
--- 
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/access/ZkBasicAuthAccessControlFactory.java
+++ 
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/access/ZkBasicAuthAccessControlFactory.java
@@ -31,8 +31,6 @@ import org.apache.pinot.controller.ControllerConf;
 import org.apache.pinot.controller.helix.core.PinotHelixResourceManager;
 import org.apache.pinot.core.auth.BasicAuthUtils;
 import org.apache.pinot.core.auth.ZkBasicAuthPrincipal;
-import org.apache.pinot.spi.config.user.ComponentType;
-import org.apache.pinot.spi.config.user.RoleType;
 import org.apache.pinot.spi.env.PinotConfiguration;
 
 
@@ -95,12 +93,6 @@ public class ZkBasicAuthAccessControlFactory implements 
AccessControlFactory {
             return getPrincipal(httpHeaders).isPresent();
         }
 
-        @Override
-        public boolean hasAccess(HttpHeaders httpHeaders) {
-            return getPrincipal(httpHeaders)
-                .filter(p -> p.hasPermission(RoleType.ADMIN, 
ComponentType.CONTROLLER)).isPresent();
-        }
-
         private Optional<ZkBasicAuthPrincipal> getPrincipal(HttpHeaders 
headers) {
             if (headers == null) {
                 return Optional.empty();
diff --git 
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotAccessControlUserRestletResource.java
 
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotAccessControlUserRestletResource.java
index b1d3b9a437..eb7fa763fd 100644
--- 
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotAccessControlUserRestletResource.java
+++ 
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotAccessControlUserRestletResource.java
@@ -36,7 +36,6 @@ import javax.ws.rs.Path;
 import javax.ws.rs.PathParam;
 import javax.ws.rs.Produces;
 import javax.ws.rs.QueryParam;
-import javax.ws.rs.core.Context;
 import javax.ws.rs.core.HttpHeaders;
 import javax.ws.rs.core.MediaType;
 import javax.ws.rs.core.Response;
@@ -44,8 +43,6 @@ import org.apache.helix.store.zk.ZkHelixPropertyStore;
 import org.apache.helix.zookeeper.datamodel.ZNRecord;
 import org.apache.pinot.common.metadata.ZKMetadataProvider;
 import org.apache.pinot.common.utils.BcryptUtils;
-import org.apache.pinot.controller.api.access.AccessControlFactory;
-import org.apache.pinot.controller.api.access.AccessControlUtils;
 import org.apache.pinot.controller.api.access.AccessType;
 import org.apache.pinot.controller.api.access.Authenticate;
 import 
org.apache.pinot.controller.api.exception.ControllerApplicationException;
@@ -54,7 +51,6 @@ import 
org.apache.pinot.controller.helix.core.PinotHelixResourceManager;
 import org.apache.pinot.spi.config.user.ComponentType;
 import org.apache.pinot.spi.config.user.UserConfig;
 import org.apache.pinot.spi.utils.JsonUtils;
-import org.glassfish.grizzly.http.server.Request;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -93,17 +89,12 @@ public class PinotAccessControlUserRestletResource {
     @Inject
     PinotHelixResourceManager _pinotHelixResourceManager;
 
-    @Inject
-    AccessControlFactory _accessControlFactory;
-
     @GET
     @Produces(MediaType.APPLICATION_JSON)
     @Path("/users")
     @ApiOperation(value = "List all uses in cluster", notes = "List all users 
in cluster")
-    public String listUers(@Context HttpHeaders httpHeaders, @Context Request 
request) {
+    public String listUers() {
         try {
-            String endpointUrl = request.getRequestURL().toString();
-            AccessControlUtils.validatePermission(httpHeaders, endpointUrl, 
_accessControlFactory.create());
             ZkHelixPropertyStore<ZNRecord> propertyStore = 
_pinotHelixResourceManager.getPropertyStore();
             Map<String, UserConfig> allUserInfo = 
ZKMetadataProvider.getAllUserInfo(propertyStore);
             return JsonUtils.newObjectNode().set("users", 
JsonUtils.objectToJsonNode(allUserInfo)).toString();
@@ -116,11 +107,8 @@ public class PinotAccessControlUserRestletResource {
     @Produces(MediaType.APPLICATION_JSON)
     @Path("/users/{username}")
     @ApiOperation(value = "Get an user in cluster", notes = "Get an user in 
cluster")
-    public String getUser(@PathParam("username") String username, 
@QueryParam("component") String componentTypeStr,
-        @Context HttpHeaders httpHeaders, @Context Request request) {
+    public String getUser(@PathParam("username") String username, 
@QueryParam("component") String componentTypeStr) {
         try {
-            String endpointUrl = request.getRequestURL().toString();
-            AccessControlUtils.validatePermission(httpHeaders, endpointUrl, 
_accessControlFactory.create());
             ZkHelixPropertyStore<ZNRecord> propertyStore = 
_pinotHelixResourceManager.getPropertyStore();
             ComponentType componentType = 
Constants.validateComponentType(componentTypeStr);
             String usernameWithType = username + "_" + componentType.name();
@@ -136,7 +124,7 @@ public class PinotAccessControlUserRestletResource {
     @Produces(MediaType.APPLICATION_JSON)
     @Path("/users")
     @ApiOperation(value = "Add a user", notes = "Add a user")
-    public SuccessResponse addUser(String userConfigStr, @Context HttpHeaders 
httpHeaders, @Context Request request) {
+    public SuccessResponse addUser(String userConfigStr) {
         // TODO introduce a table config ctor with json string.
 
         UserConfig userConfig;
@@ -144,8 +132,6 @@ public class PinotAccessControlUserRestletResource {
         try {
             userConfig = JsonUtils.stringToObject(userConfigStr, 
UserConfig.class);
             username = userConfig.getUserName();
-            String endpointUrl = request.getRequestURL().toString();
-            AccessControlUtils.validatePermission(httpHeaders, endpointUrl, 
_accessControlFactory.create());
             if (username.contains(".") || username.contains(" ")) {
                 throw new IllegalStateException("Username: " + username + " 
containing '.' or space is not allowed");
             }
@@ -171,8 +157,7 @@ public class PinotAccessControlUserRestletResource {
     @Produces(MediaType.APPLICATION_JSON)
     @ApiOperation(value = "Delete a user", notes = "Delete a user")
     public SuccessResponse deleteUser(@PathParam("username") String username,
-        @QueryParam("component") String componentTypeStr,
-        @Context HttpHeaders httpHeaders, @Context Request request) {
+        @QueryParam("component") String componentTypeStr) {
 
         List<String> usersDeleted = new LinkedList<>();
         String usernameWithComponentType = username + "_" + componentTypeStr;
@@ -182,9 +167,6 @@ public class PinotAccessControlUserRestletResource {
             boolean userExist = false;
             userExist = _pinotHelixResourceManager.hasUser(username, 
componentTypeStr);
 
-            String endpointUrl = request.getRequestURL().toString();
-            AccessControlUtils.validatePermission(httpHeaders, endpointUrl, 
_accessControlFactory.create());
-
             _pinotHelixResourceManager.deleteUser(usernameWithComponentType);
             if (userExist) {
                 usersDeleted.add(username);
@@ -210,16 +192,11 @@ public class PinotAccessControlUserRestletResource {
         @PathParam("username") String username,
         @QueryParam("component") String componentTypeStr,
         @QueryParam("passwordChanged") boolean passwordChanged,
-        String userConfigString,
-        @Context HttpHeaders httpHeaders,
-        @Context Request request) {
+        String userConfigString) {
 
         UserConfig userConfig;
         String usernameWithComponentType = username + "_" + componentTypeStr;
         try {
-            String endpointUrl = request.getRequestURL().toString();
-            AccessControlUtils.validatePermission(httpHeaders, endpointUrl, 
_accessControlFactory.create());
-
             userConfig = JsonUtils.stringToObject(userConfigString, 
UserConfig.class);
             if (passwordChanged) {
                 
userConfig.setPassword(BcryptUtils.encrypt(userConfig.getPassword()));
diff --git 
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotControllerAuthResource.java
 
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotControllerAuthResource.java
index 0dbab0fccf..77b77479ca 100644
--- 
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotControllerAuthResource.java
+++ 
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotControllerAuthResource.java
@@ -35,7 +35,6 @@ import javax.ws.rs.QueryParam;
 import javax.ws.rs.core.Context;
 import javax.ws.rs.core.HttpHeaders;
 import javax.ws.rs.core.MediaType;
-import org.apache.commons.lang3.StringUtils;
 import org.apache.pinot.controller.api.access.AccessControl;
 import org.apache.pinot.controller.api.access.AccessControlFactory;
 import org.apache.pinot.controller.api.access.AccessType;
@@ -76,11 +75,6 @@ public class PinotControllerAuthResource {
       @ApiParam(value = "API access type") @QueryParam("accessType") 
AccessType accessType,
       @ApiParam(value = "Endpoint URL") @QueryParam("endpointUrl") String 
endpointUrl) {
     AccessControl accessControl = _accessControlFactory.create();
-
-    if (StringUtils.isBlank(tableName)) {
-      return accessControl.hasAccess(accessType, _httpHeaders, endpointUrl);
-    }
-
     return accessControl.hasAccess(tableName, accessType, _httpHeaders, 
endpointUrl);
   }
 
diff --git 
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotQueryResource.java
 
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotQueryResource.java
index b8e043319c..83097067b8 100644
--- 
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotQueryResource.java
+++ 
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotQueryResource.java
@@ -54,6 +54,8 @@ import org.apache.pinot.common.utils.request.RequestUtils;
 import org.apache.pinot.controller.ControllerConf;
 import org.apache.pinot.controller.api.access.AccessControl;
 import org.apache.pinot.controller.api.access.AccessControlFactory;
+import org.apache.pinot.controller.api.access.AccessType;
+import org.apache.pinot.controller.api.access.ManualAuthorization;
 import org.apache.pinot.controller.helix.core.PinotHelixResourceManager;
 import org.apache.pinot.core.query.executor.sql.SqlQueryExecutor;
 import org.apache.pinot.spi.utils.CommonConstants;
@@ -87,6 +89,7 @@ public class PinotQueryResource {
 
   @POST
   @Path("sql")
+  @ManualAuthorization // performed by broker
   public String handlePostSql(String requestJsonStr, @Context HttpHeaders 
httpHeaders) {
     try {
       JsonNode requestJson = JsonUtils.stringToJsonNode(requestJsonStr);
@@ -100,7 +103,7 @@ public class PinotQueryResource {
         queryOptions = requestJson.get("queryOptions").asText();
       }
       LOGGER.debug("Trace: {}, Running query: {}", traceEnabled, sqlQuery);
-      return executeSqlQuery(httpHeaders, sqlQuery, traceEnabled, 
queryOptions);
+      return executeSqlQuery(httpHeaders, sqlQuery, traceEnabled, 
queryOptions, "/sql");
     } catch (Exception e) {
       LOGGER.error("Caught exception while processing post request", e);
       return QueryException.getException(QueryException.INTERNAL_ERROR, 
e).toString();
@@ -113,7 +116,7 @@ public class PinotQueryResource {
       @QueryParam("queryOptions") String queryOptions, @Context HttpHeaders 
httpHeaders) {
     try {
       LOGGER.debug("Trace: {}, Running query: {}", traceEnabled, sqlQuery);
-      return executeSqlQuery(httpHeaders, sqlQuery, traceEnabled, 
queryOptions);
+      return executeSqlQuery(httpHeaders, sqlQuery, traceEnabled, 
queryOptions, "/sql");
     } catch (Exception e) {
       LOGGER.error("Caught exception while processing get request", e);
       return QueryException.getException(QueryException.INTERNAL_ERROR, 
e).toString();
@@ -121,7 +124,7 @@ public class PinotQueryResource {
   }
 
   private String executeSqlQuery(@Context HttpHeaders httpHeaders, String 
sqlQuery, String traceEnabled,
-      @Nullable String queryOptions)
+      @Nullable String queryOptions, String endpointUrl)
       throws Exception {
     SqlNodeAndOptions sqlNodeAndOptions = 
CalciteSqlParser.compileToSqlNodeAndOptions(sqlQuery);
     Map<String, String> options = sqlNodeAndOptions.getOptions();
@@ -134,7 +137,7 @@ public class PinotQueryResource {
     if 
(Boolean.parseBoolean(options.get(QueryOptionKey.USE_MULTISTAGE_ENGINE))) {
       if 
(_controllerConf.getProperty(CommonConstants.Helix.CONFIG_OF_MULTI_STAGE_ENGINE_ENABLED,
           CommonConstants.Helix.DEFAULT_MULTI_STAGE_ENGINE_ENABLED)) {
-        return getMultiStageQueryResponse(sqlQuery, queryOptions, httpHeaders);
+        return getMultiStageQueryResponse(sqlQuery, queryOptions, httpHeaders, 
endpointUrl);
       } else {
         throw new UnsupportedOperationException("V2 Multi-Stage query engine 
not enabled. "
             + "Please see https://docs.pinot.apache.org/ for instruction to 
enable V2 engine.");
@@ -156,12 +159,13 @@ public class PinotQueryResource {
     }
   }
 
-  private String getMultiStageQueryResponse(String query, String queryOptions, 
HttpHeaders httpHeaders) {
+  private String getMultiStageQueryResponse(String query, String queryOptions, 
HttpHeaders httpHeaders,
+      String endpointUrl) {
 
     // Validate data access
     // we don't have a cross table access control rule so only ADMIN can make 
request to multi-stage engine.
     AccessControl accessControl = _accessControlFactory.create();
-    if (!accessControl.hasAccess(httpHeaders)) {
+    if (!accessControl.hasAccess(null, AccessType.READ, httpHeaders, 
endpointUrl)) {
       return QueryException.ACCESS_DENIED_ERROR.toString();
     }
 
diff --git 
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSchemaRestletResource.java
 
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSchemaRestletResource.java
index 2bb05d0553..176d796660 100644
--- 
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSchemaRestletResource.java
+++ 
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSchemaRestletResource.java
@@ -61,6 +61,7 @@ import 
org.apache.pinot.controller.api.access.AccessControlFactory;
 import org.apache.pinot.controller.api.access.AccessControlUtils;
 import org.apache.pinot.controller.api.access.AccessType;
 import org.apache.pinot.controller.api.access.Authenticate;
+import org.apache.pinot.controller.api.access.ManualAuthorization;
 import org.apache.pinot.controller.api.events.MetadataEventNotifierFactory;
 import org.apache.pinot.controller.api.events.SchemaEventType;
 import 
org.apache.pinot.controller.api.exception.ControllerApplicationException;
@@ -209,6 +210,7 @@ public class PinotSchemaRestletResource {
       @ApiResponse(code = 400, message = "Missing or invalid request body"),
       @ApiResponse(code = 500, message = "Internal error")
   })
+  @ManualAuthorization // performed after parsing schema
   public ConfigSuccessResponse addSchema(
       @ApiParam(value = "Whether to override the schema if the schema exists") 
@DefaultValue("true")
       @QueryParam("override") boolean override, FormDataMultiPart multiPart, 
@Context HttpHeaders httpHeaders,
@@ -235,6 +237,7 @@ public class PinotSchemaRestletResource {
       @ApiResponse(code = 400, message = "Missing or invalid request body"),
       @ApiResponse(code = 500, message = "Internal error")
   })
+  @ManualAuthorization // performed after parsing schema
   public ConfigSuccessResponse addSchema(
       @ApiParam(value = "Whether to override the schema if the schema exists") 
@DefaultValue("true")
       @QueryParam("override") boolean override, String schemaJsonString, 
@Context HttpHeaders httpHeaders,
@@ -250,7 +253,7 @@ public class PinotSchemaRestletResource {
     Schema schema = schemaAndUnrecognizedProperties.getLeft();
     String endpointUrl = request.getRequestURL().toString();
     validateSchemaName(schema.getSchemaName());
-    AccessControlUtils.validatePermission(schema.getSchemaName(), 
AccessType.CREATE, httpHeaders, endpointUrl,
+    AccessControlUtils.validatePermission(schema.getSchemaName(), 
AccessType.READ, httpHeaders, endpointUrl,
         _accessControlFactory.create());
     SuccessResponse successResponse = addSchema(schema, override);
     return new ConfigSuccessResponse(successResponse.getStatus(), 
schemaAndUnrecognizedProperties.getRight());
@@ -266,11 +269,16 @@ public class PinotSchemaRestletResource {
       @ApiResponse(code = 400, message = "Missing or invalid request body"),
       @ApiResponse(code = 500, message = "Internal error")
   })
-  public String validateSchema(FormDataMultiPart multiPart) {
+  @ManualAuthorization // performed after parsing schema
+  public String validateSchema(FormDataMultiPart multiPart, @Context 
HttpHeaders httpHeaders,
+      @Context Request request) {
     Pair<Schema, Map<String, Object>> schemaAndUnrecognizedProps =
         getSchemaAndUnrecognizedPropertiesFromMultiPart(multiPart);
     Schema schema = schemaAndUnrecognizedProps.getLeft();
+    String endpointUrl = request.getRequestURL().toString();
     validateSchemaInternal(schema);
+    AccessControlUtils.validatePermission(schema.getSchemaName(), 
AccessType.READ, httpHeaders, endpointUrl,
+        _accessControlFactory.create());
     ObjectNode response = schema.toJsonObject();
     response.set("unrecognizedProperties", 
JsonUtils.objectToJsonNode(schemaAndUnrecognizedProps.getRight()));
     try {
@@ -291,7 +299,8 @@ public class PinotSchemaRestletResource {
       @ApiResponse(code = 400, message = "Missing or invalid request body"),
       @ApiResponse(code = 500, message = "Internal error")
   })
-  public String validateSchema(String schemaJsonString) {
+  @ManualAuthorization // performed after parsing schema
+  public String validateSchema(String schemaJsonString, @Context HttpHeaders 
httpHeaders, @Context Request request) {
     Pair<Schema, Map<String, Object>> schemaAndUnrecognizedProps = null;
     try {
       schemaAndUnrecognizedProps = 
JsonUtils.stringToObjectAndUnrecognizedProperties(schemaJsonString, 
Schema.class);
@@ -300,7 +309,10 @@ public class PinotSchemaRestletResource {
       throw new ControllerApplicationException(LOGGER, msg, 
Response.Status.BAD_REQUEST, e);
     }
     Schema schema = schemaAndUnrecognizedProps.getLeft();
+    String endpointUrl = request.getRequestURL().toString();
     validateSchemaInternal(schema);
+    AccessControlUtils.validatePermission(schema.getSchemaName(), 
AccessType.CREATE, httpHeaders, endpointUrl,
+        _accessControlFactory.create());
     ObjectNode response = schema.toJsonObject();
     response.set("unrecognizedProperties", 
JsonUtils.objectToJsonNode(schemaAndUnrecognizedProps.getRight()));
     try {
diff --git 
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentUploadDownloadRestletResource.java
 
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentUploadDownloadRestletResource.java
index 6b2b24c286..4fb4e298bf 100644
--- 
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentUploadDownloadRestletResource.java
+++ 
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentUploadDownloadRestletResource.java
@@ -142,6 +142,7 @@ public class PinotSegmentUploadDownloadRestletResource {
   @ApiOperation(value = "Download a segment", notes = "Download a segment")
   @TrackInflightRequestMetrics
   @TrackedByGauge(gauge = ControllerGauge.SEGMENT_DOWNLOADS_IN_PROGRESS)
+  @Authenticate(AccessType.READ)
   public Response downloadSegment(
       @ApiParam(value = "Name of the table", required = true) 
@PathParam("tableName") String tableName,
       @ApiParam(value = "Name of the segment", required = true) 
@PathParam("segmentName") @Encoded String segmentName,
@@ -160,7 +161,6 @@ public class PinotSegmentUploadDownloadRestletResource {
       throw new ControllerApplicationException(LOGGER, "No data access to 
table: " + tableName,
           Response.Status.FORBIDDEN);
     }
-
     segmentName = URIUtils.decode(segmentName);
     URI dataDirURI = ControllerFilePathProvider.getInstance().getDataDirURI();
     Response.ResponseBuilder builder = Response.ok();
diff --git 
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
 
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
index 5d5030a4e6..2283d5bfa3 100644
--- 
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
+++ 
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
@@ -80,6 +80,7 @@ import 
org.apache.pinot.controller.api.access.AccessControlFactory;
 import org.apache.pinot.controller.api.access.AccessControlUtils;
 import org.apache.pinot.controller.api.access.AccessType;
 import org.apache.pinot.controller.api.access.Authenticate;
+import org.apache.pinot.controller.api.access.ManualAuthorization;
 import 
org.apache.pinot.controller.api.exception.ControllerApplicationException;
 import org.apache.pinot.controller.api.exception.InvalidTableConfigException;
 import org.apache.pinot.controller.api.exception.TableAlreadyExistsException;
@@ -168,6 +169,7 @@ public class PinotTableRestletResource {
   @Produces(MediaType.APPLICATION_JSON)
   @Path("/tables")
   @ApiOperation(value = "Adds a table", notes = "Adds a table")
+  @ManualAuthorization // performed after parsing table configs
   public ConfigSuccessResponse addTable(
       String tableConfigStr,
       @ApiParam(value = "comma separated list of validation type(s) to skip. 
supported types: (ALL|TASK|UPSERT)")
@@ -531,10 +533,12 @@ public class PinotTableRestletResource {
   @ApiOperation(value = "Validate table config for a table",
       notes = "This API returns the table config that matches the one you get 
from 'GET /tables/{tableName}'."
           + " This allows us to validate table config before apply.")
+  @ManualAuthorization // performed after parsing TableConfig
   public ObjectNode checkTableConfig(
       String tableConfigStr,
       @ApiParam(value = "comma separated list of validation type(s) to skip. 
supported types: (ALL|TASK|UPSERT)")
-      @QueryParam("validationTypesToSkip") @Nullable String typesToSkip) {
+      @QueryParam("validationTypesToSkip") @Nullable String typesToSkip, 
@Context HttpHeaders httpHeaders,
+      @Context Request request) {
     Pair<TableConfig, Map<String, Object>> tableConfig;
     try {
       tableConfig = 
JsonUtils.stringToObjectAndUnrecognizedProperties(tableConfigStr, 
TableConfig.class);
@@ -542,6 +546,13 @@ public class PinotTableRestletResource {
       String msg = String.format("Invalid table config json string: %s", 
tableConfigStr);
       throw new ControllerApplicationException(LOGGER, msg, 
Response.Status.BAD_REQUEST, e);
     }
+
+    // validate permission
+    String tableName = tableConfig.getLeft().getTableName();
+    String endpointUrl = request.getRequestURL().toString();
+    AccessControlUtils.validatePermission(tableName, AccessType.READ, 
httpHeaders, endpointUrl,
+        _accessControlFactory.create());
+
     ObjectNode validationResponse =
         validateConfig(tableConfig.getLeft(), 
_pinotHelixResourceManager.getSchemaForTableConfig(tableConfig.getLeft()),
             typesToSkip);
@@ -559,15 +570,25 @@ public class PinotTableRestletResource {
           + "Validate given table config and schema. If specified schema is 
null, attempt to retrieve schema using the "
           + "table name. This API returns the table config that matches the 
one you get from 'GET /tables/{tableName}'."
           + " This allows us to validate table config before apply.")
+  @ManualAuthorization // performed after parsing TableAndSchemaConfig
   public String validateTableAndSchema(
       TableAndSchemaConfig tableSchemaConfig,
       @ApiParam(value = "comma separated list of validation type(s) to skip. 
supported types: (ALL|TASK|UPSERT)")
-      @QueryParam("validationTypesToSkip") @Nullable String typesToSkip) {
+      @QueryParam("validationTypesToSkip") @Nullable String typesToSkip, 
@Context HttpHeaders httpHeaders,
+      @Context Request request) {
     TableConfig tableConfig = tableSchemaConfig.getTableConfig();
     Schema schema = tableSchemaConfig.getSchema();
+
     if (schema == null) {
       schema = _pinotHelixResourceManager.getSchemaForTableConfig(tableConfig);
     }
+
+    // validate permission
+    String schemaName = schema != null ? schema.getSchemaName() : null;
+    String endpointUrl = request.getRequestURL().toString();
+    AccessControlUtils.validatePermission(schemaName, AccessType.READ, 
httpHeaders, endpointUrl,
+        _accessControlFactory.create());
+
     return validateConfig(tableSchemaConfig.getTableConfig(), schema, 
typesToSkip).toString();
   }
 
diff --git 
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableConfigsRestletResource.java
 
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableConfigsRestletResource.java
index d238756783..5fe6e325b2 100644
--- 
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableConfigsRestletResource.java
+++ 
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableConfigsRestletResource.java
@@ -56,6 +56,7 @@ import 
org.apache.pinot.controller.api.access.AccessControlFactory;
 import org.apache.pinot.controller.api.access.AccessControlUtils;
 import org.apache.pinot.controller.api.access.AccessType;
 import org.apache.pinot.controller.api.access.Authenticate;
+import org.apache.pinot.controller.api.access.ManualAuthorization;
 import 
org.apache.pinot.controller.api.exception.ControllerApplicationException;
 import org.apache.pinot.controller.api.exception.InvalidTableConfigException;
 import org.apache.pinot.controller.api.exception.TableAlreadyExistsException;
@@ -160,6 +161,7 @@ public class TableConfigsRestletResource {
   @Path("/tableConfigs")
   @ApiOperation(value = "Add the TableConfigs using the tableConfigsStr json",
       notes = "Add the TableConfigs using the tableConfigsStr json")
+  @ManualAuthorization // performed after parsing table configs
   public ConfigSuccessResponse addConfig(
       String tableConfigsStr,
       @ApiParam(value = "comma separated list of validation type(s) to skip. 
supported types: (ALL|TASK|UPSERT)")
@@ -185,6 +187,7 @@ public class TableConfigsRestletResource {
           Response.Status.BAD_REQUEST);
     }
 
+    // validate permission
     TableConfig offlineTableConfig = tableConfigs.getOffline();
     TableConfig realtimeTableConfig = tableConfigs.getRealtime();
     Schema schema = tableConfigs.getSchema();
@@ -376,9 +379,11 @@ public class TableConfigsRestletResource {
   @Path("/tableConfigs/validate")
   @Produces(MediaType.APPLICATION_JSON)
   @ApiOperation(value = "Validate the TableConfigs", notes = "Validate the 
TableConfigs")
+  @ManualAuthorization // performed after parsing TableConfigs
   public String validateConfig(String tableConfigsStr,
       @ApiParam(value = "comma separated list of validation type(s) to skip. 
supported types: (ALL|TASK|UPSERT)")
-      @QueryParam("validationTypesToSkip") @Nullable String typesToSkip) {
+      @QueryParam("validationTypesToSkip") @Nullable String typesToSkip, 
@Context HttpHeaders httpHeaders,
+      @Context Request request) {
     Pair<TableConfigs, Map<String, Object>> tableConfigsAndUnrecognizedProps;
     TableConfigs tableConfigs;
     try {
@@ -389,6 +394,30 @@ public class TableConfigsRestletResource {
       throw new ControllerApplicationException(LOGGER,
           String.format("Invalid TableConfigs json string: %s", 
tableConfigsStr), Response.Status.BAD_REQUEST, e);
     }
+
+    // validate permission
+    String endpointUrl = request.getRequestURL().toString();
+    AccessControl accessControl = _accessControlFactory.create();
+    Schema schema = tableConfigs.getSchema();
+    TableConfig offlineTableConfig = tableConfigs.getOffline();
+    TableConfig realtimeTableConfig = tableConfigs.getRealtime();
+
+    AccessControlUtils
+        .validatePermission(schema.getSchemaName(), AccessType.READ, 
httpHeaders, endpointUrl, accessControl);
+
+    if (offlineTableConfig != null) {
+      tuneConfig(offlineTableConfig, schema);
+      AccessControlUtils
+          .validatePermission(offlineTableConfig.getTableName(), 
AccessType.READ, httpHeaders, endpointUrl,
+              accessControl);
+    }
+    if (realtimeTableConfig != null) {
+      tuneConfig(realtimeTableConfig, schema);
+      AccessControlUtils
+          .validatePermission(realtimeTableConfig.getTableName(), 
AccessType.READ, httpHeaders, endpointUrl,
+              accessControl);
+    }
+
     TableConfigs validatedTableConfigs = validateConfig(tableConfigs, 
typesToSkip);
     ObjectNode response = 
JsonUtils.objectToJsonNode(validatedTableConfigs).deepCopy();
     response.set("unrecognizedProperties", 
JsonUtils.objectToJsonNode(tableConfigsAndUnrecognizedProps.getRight()));
diff --git 
a/pinot-controller/src/test/java/org/apache/pinot/controller/api/AccessControlTest.java
 
b/pinot-controller/src/test/java/org/apache/pinot/controller/api/AccessControlTest.java
deleted file mode 100644
index 52ffd7e24c..0000000000
--- 
a/pinot-controller/src/test/java/org/apache/pinot/controller/api/AccessControlTest.java
+++ /dev/null
@@ -1,60 +0,0 @@
-/**
- * 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.pinot.controller.api;
-
-import java.io.IOException;
-import org.apache.pinot.controller.api.access.AccessControl;
-import org.apache.pinot.controller.api.access.AccessControlFactory;
-import org.apache.pinot.controller.helix.ControllerTest;
-import org.testng.Assert;
-import org.testng.annotations.BeforeClass;
-import org.testng.annotations.Test;
-
-
-public class AccessControlTest {
-  private static final ControllerTest TEST_INSTANCE = 
ControllerTest.getInstance();
-
-  private static final String TABLE_NAME = "accessTestTable";
-
-  @BeforeClass
-  public void setUp()
-      throws Exception {
-    TEST_INSTANCE.setupSharedStateAndValidate();
-  }
-
-  @Test
-  public void testAccessDenied() {
-    try {
-      ControllerTest.sendGetRequest(
-          
TEST_INSTANCE.getControllerRequestURLBuilder().forSegmentDownload(TABLE_NAME, 
"testSegment"));
-      Assert.fail("Access not denied");
-    } catch (IOException e) {
-      Assert.assertTrue(e.getMessage().contains("Got error status code: 403 
(Forbidden)"));
-    }
-  }
-
-  public static class DenyAllAccessFactory implements AccessControlFactory {
-    private static final AccessControl DENY_ALL_ACCESS = (httpHeaders, 
tableName) -> !tableName.equals(TABLE_NAME);
-
-    @Override
-    public AccessControl create() {
-      return DENY_ALL_ACCESS;
-    }
-  }
-}
diff --git 
a/pinot-controller/src/test/java/org/apache/pinot/controller/helix/ControllerTest.java
 
b/pinot-controller/src/test/java/org/apache/pinot/controller/helix/ControllerTest.java
index 7485957650..cadead8f70 100644
--- 
a/pinot-controller/src/test/java/org/apache/pinot/controller/helix/ControllerTest.java
+++ 
b/pinot-controller/src/test/java/org/apache/pinot/controller/helix/ControllerTest.java
@@ -60,7 +60,7 @@ import org.apache.pinot.common.utils.http.HttpClient;
 import org.apache.pinot.controller.BaseControllerStarter;
 import org.apache.pinot.controller.ControllerConf;
 import org.apache.pinot.controller.ControllerStarter;
-import org.apache.pinot.controller.api.AccessControlTest;
+import org.apache.pinot.controller.api.access.AllowAllAccessFactory;
 import org.apache.pinot.controller.helix.core.PinotHelixResourceManager;
 import org.apache.pinot.spi.config.table.TableConfig;
 import org.apache.pinot.spi.config.table.TableType;
@@ -895,8 +895,7 @@ public class ControllerTest {
     Map<String, Object> properties = getDefaultControllerConfiguration();
 
     // TODO: move these test specific configs into respective test classes.
-    // Used in AccessControlTest
-    properties.put(ControllerConf.ACCESS_CONTROL_FACTORY_CLASS, 
AccessControlTest.DenyAllAccessFactory.class.getName());
+    properties.put(ControllerConf.ACCESS_CONTROL_FACTORY_CLASS, 
AllowAllAccessFactory.class.getName());
 
     // Used in PinotTableRestletResourceTest
     properties.put(ControllerConf.TABLE_MIN_REPLICAS, MIN_NUM_REPLICAS);
diff --git 
a/pinot-spi/src/main/java/org/apache/pinot/spi/utils/builder/TableNameBuilder.java
 
b/pinot-spi/src/main/java/org/apache/pinot/spi/utils/builder/TableNameBuilder.java
index 5c54ba4efe..6c8b233f8f 100644
--- 
a/pinot-spi/src/main/java/org/apache/pinot/spi/utils/builder/TableNameBuilder.java
+++ 
b/pinot-spi/src/main/java/org/apache/pinot/spi/utils/builder/TableNameBuilder.java
@@ -97,6 +97,9 @@ public class TableNameBuilder {
    * @return Table name without type suffix
    */
   public static String extractRawTableName(String tableName) {
+    if (tableName == null) {
+      return null;
+    }
     if (OFFLINE.tableHasTypeSuffix(tableName)) {
       return tableName.substring(0, tableName.length() - 
OFFLINE._typeSuffix.length());
     }


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to