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