gianm commented on code in PR #17564:
URL: https://github.com/apache/druid/pull/17564#discussion_r1909581703


##########
server/src/main/java/org/apache/druid/server/QueryResource.java:
##########
@@ -152,14 +152,14 @@ public Response cancelQuery(@PathParam("id") String 
queryId, @Context final Http
       datasources = new TreeSet<>();
     }
 
-    Access authResult = AuthorizationUtils.authorizeAllResourceActions(
+    AuthorizationResult authResult = 
AuthorizationUtils.authorizeAllResourceActions(
         req,
         Iterables.transform(datasources, 
AuthorizationUtils.DATASOURCE_WRITE_RA_GENERATOR),
         authorizerMapper
     );
 
-    if (!authResult.isAllowed()) {
-      throw new ForbiddenException(authResult.toString());
+    if (!authResult.isUserWithNoRestriction()) {
+      throw new ForbiddenException(authResult.getErrorMessage());

Review Comment:
   This is weird but it is difficult to do something better without deeper 
changes to cancellation authorization. Ideally the way it works is that people 
are only allowed to cancel their own queries, unless they have `STATE WRITE`, 
in which case they should be able to cancel any query. But that's not how 
cancellation works today, because we don't retain info about which user issued 
a query.
   
   IMO, it'd be good to adjust this in a follow up, so we can change things so 
it works like the above ☝️. For the time being I think it is OK to deny 
cancellation to people with policies.



-- 
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]

Reply via email to