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]