Jackie-Jiang commented on code in PR #11016:
URL: https://github.com/apache/pinot/pull/11016#discussion_r1271579047


##########
pinot-broker/src/main/java/org/apache/pinot/broker/broker/AuthenticationFilter.java:
##########
@@ -81,10 +85,71 @@ public void filter(ContainerRequestContext requestContext)
 
     HttpRequesterIdentity httpRequestIdentity = 
HttpRequesterIdentity.fromRequest(request);
 
+    // default authorization handling
     if (!accessControl.hasAccess(httpRequestIdentity)) {
       throw new WebApplicationException("Failed access check for " + 
httpRequestIdentity.getEndpointUrl(),
           Response.Status.FORBIDDEN);
     }
+
+    handleFinerGrainAuth(endpointMethod, uriInfo, accessControl, 
httpRequestIdentity);
+  }
+
+  /**
+   * Check for finer grain authorization of APIs.
+   * There are 2 possible cases:
+   * 1. {@link Authorize} annotation is present on the method. In this case, 
do the finer grain authorization using the
+   *    fields of the annotation. There are 2 possibilities depending on the 
targetType ({@link TargetType}):
+   *    a. The targetType is {@link TargetType#CLUSTER}. In this case, the 
paramName field
+   *       ({@link Authorize#paramName()}) is not used, since the target is 
the Pinot cluster.
+   *    b. The targetType is {@link TargetType#TABLE}. In this case, the 
paramName field
+   *       ({@link Authorize#paramName()}) is mandatory, and it must be found 
in either the path parameters or the
+   *       query parameters.
+   * 2. {@link Authorize} annotation is not present on the method. In this use 
the default authorization.
+   *
+   * @param endpointMethod of the API
+   * @param uriInfo of the API
+   * @param accessControl to check the access
+   * @param httpRequestIdentity of the requester
+   */
+  private void handleFinerGrainAuth(Method endpointMethod, UriInfo uriInfo, 
AccessControl accessControl,
+      HttpRequesterIdentity httpRequestIdentity) {
+    if (endpointMethod.isAnnotationPresent(Authorize.class)) {
+      final Authorize auth = endpointMethod.getAnnotation(Authorize.class);
+      String targetId = null;
+      // Message to use in the access denied exception
+      String accessDeniedMsg;
+      if (auth.targetType() == TargetType.TABLE) {
+        // paramName is mandatory for table level authorization
+        if (StringUtils.isEmpty(auth.paramName())) {
+          throw new WebApplicationException(
+              "paramName not found for table level authorization in API: " + 
uriInfo.getRequestUri(),
+              Response.Status.INTERNAL_SERVER_ERROR);
+        }
+
+        // find the paramName in the path or query params
+        targetId = RBACAuthUtils.findParam(auth.paramName(), 
uriInfo.getPathParameters(), uriInfo.getQueryParameters());

Review Comment:
   I'd suggest just handling it here with 
`TableNameBuilder.extractRawTableName()` when the target type is table. 
Currently there is no way for us to differentiate whether the param is raw 
table name or table name with type 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org

Reply via email to