Copilot commented on code in PR #766:
URL: https://github.com/apache/ranger/pull/766#discussion_r2615595600
##########
plugin-ozone/src/main/java/org/apache/ranger/authorization/ozone/authorizer/RangerOzoneAuthorizer.java:
##########
@@ -188,6 +213,61 @@ public boolean checkAccess(IOzoneObj ozoneObject,
RequestContext context) {
return returnValue;
}
+ @Override
+ public String generateAssumeRoleSessionPolicy(AssumeRoleRequest
assumeRoleRequest) throws OMException {
+ LOG.debug("==>
RangerOzoneAuthorizer.generateAssumeRoleSessionPolicy(assumeRoleRequest={})",
assumeRoleRequest);
+
+ if (assumeRoleRequest == null) {
+ throw new OMException("invalid request: null",
OMException.ResultCodes.INVALID_REQUEST);
+ } else if (assumeRoleRequest.getClientUgi() == null) {
+ throw new OMException("invalid request: request.clientUgi null",
OMException.ResultCodes.INVALID_REQUEST);
+ } else if (assumeRoleRequest.getTargetRoleName() == null) {
+ throw new OMException("invalid request: request.targetRoleName
null", OMException.ResultCodes.INVALID_REQUEST);
+ }
+
+ RangerBasePlugin plugin = rangerPlugin;
+
+ if (plugin == null) {
+ throw new OMException("Ranger authorizer not initialized",
OMException.ResultCodes.INTERNAL_ERROR);
+ }
+
+ UserGroupInformation ugi = assumeRoleRequest.getClientUgi();
+ RangerAccessResourceImpl resource = new
RangerAccessResourceImpl(Collections.singletonMap(KEY_RESOURCE_ROLE,
assumeRoleRequest.getTargetRoleName()));
+ RangerAccessRequestImpl request = new
RangerAccessRequestImpl(resource, ACCESS_TYPE_ASSUME_ROLE,
ugi.getShortUserName(), Sets.newHashSet(ugi.getGroupNames()), null);
+
+ try {
+ RangerAccessResult result = plugin.isAccessAllowed(request);
+
+ if (result != null && result.getIsAccessDetermined() &&
result.getIsAllowed()) {
+ final Set<OzoneGrant> ozoneGrants =
assumeRoleRequest.getGrants();
+ final List<RangerInlinePolicy.Grant> inlineGrants;
+
+ if (ozoneGrants == null) { // allow all permissions
+ inlineGrants = null;
+ } else if (ozoneGrants.isEmpty()) { // don't allow any
permission
+ inlineGrants = Collections.singletonList(new
RangerInlinePolicy.Grant());
Review Comment:
Creating an empty grant to represent 'no permissions' is non-intuitive.
Consider using Collections.emptyList() with a comment explaining that empty
list means no permissions, or defining a constant DENY_ALL_GRANT to make the
intent clearer.
```suggestion
// Use emptyList to represent "no permissions"
inlineGrants = Collections.emptyList();
```
##########
plugin-ozone/src/main/java/org/apache/ranger/authorization/ozone/authorizer/RangerOzoneAuthorizer.java:
##########
@@ -77,6 +98,11 @@ public RangerOzoneAuthorizer() {
}
}
+ // for testing only
+ RangerOzoneAuthorizer(RangerBasePlugin plugin) {
Review Comment:
The test-only constructor has package-private visibility which may not be
restrictive enough. Consider making it package-private with an explicit comment
or adding @VisibleForTesting annotation to clearly document its intended usage
scope.
--
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]
