Jackie-Jiang commented on code in PR #11016:
URL: https://github.com/apache/pinot/pull/11016#discussion_r1271269196
##########
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:
What I mean is that should we always use raw table name to authorize?
The `tableName` param usually take both raw table name and table name with
type, and I don't see a scenario where a user is authorized to only the OFFLINE
table but not the REALTIME table. If we want to support that, we need to define
the access for raw table name and table name with type separately.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
