LakshSingla commented on code in PR #12959:
URL: https://github.com/apache/druid/pull/12959#discussion_r954104824
##########
extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authorization/BasicRoleBasedAuthorizer.java:
##########
@@ -90,7 +94,11 @@ public Access authorize(AuthenticationResult
authenticationResult, Resource reso
Map<String, BasicAuthorizerRole> roleMap = roleProvider.getRoleMap(name);
if (roleNames.isEmpty()) {
- return new Access(false);
+ LOG.info("No roles found for authorizer [%s] and user [%s]", name,
authenticationResult.getIdentity());
+ return new Access(false, StringUtils.format("You don't have '%s'
permission for Resource [%s:%s]",
+ action.name(),
+ resource.getType(),
+ resource.getName()));
Review Comment:
Is it safe to expose the name of the resource to the end user?
If the error message when a resource doesn't exist versus when a resource
exists but the user is not authorized to access it is not the same, might lead
to the user knowing which resources are present in the cluster.
Edit: From the test case, and it seems like we are returning the same error
message for both the scenarios, so I think it should be cool to expose the
resource name to the user.
--
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]
