soumitra-st commented on code in PR #11016:
URL: https://github.com/apache/pinot/pull/11016#discussion_r1271545265
##########
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:
Got it. Shall we leave that to the implementation of
[AccessControl.hasAccess](https://github.com/apache/pinot/pull/11016/files#diff-5d02b4239775fe072ec911349f56c32e7ca2d6cd77226d6239b1511096d57e2cR72)
to remove the trailing _OFFLINE/_REALTIME and validate the authorization on
the raw table name if that is what someone wants to do? As such from the
framework perspective Pinot should send the actual table name used in the API
call. What do you think?
--
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
