Copilot commented on code in PR #890:
URL: https://github.com/apache/ranger/pull/890#discussion_r2991119282
##########
authz-embedded/src/main/java/org/apache/ranger/authz/embedded/RangerAuthzPlugin.java:
##########
@@ -70,7 +70,7 @@ public class RangerAuthzPlugin {
private final RangerBasePlugin plugin;
private final Map<String, RangerResourceNameParser> rrnTemplates = new
HashMap<>();
- public RangerAuthzPlugin(String serviceType, String serviceName,
Properties properties) {
+ RangerAuthzPlugin(String serviceType, String serviceName, Properties
properties) {
plugin = new RangerBasePlugin(getPluginConfig(serviceType,
serviceName, properties)) {
Review Comment:
The PR description says `RangerAuthzPlugin` visibility is being restricted
to package level, but this change only makes the constructor package-private;
the class itself is still `public` (so it remains part of the public API
surface, even if it’s not instantiable externally). If the intent is truly
package-only visibility, consider removing the `public` modifier from the class
declaration as well (or update the PR description to match the actual
restriction).
##########
authz-embedded/src/main/java/org/apache/ranger/authz/embedded/RangerEmbeddedAuthorizer.java:
##########
@@ -106,29 +106,29 @@ public RangerMultiAuthzResult
authorize(RangerMultiAuthzRequest request) throws
}
}
- public RangerAuthzResult authorize(RangerAuthzRequest request,
RangerAuthzAuditHandler auditHandler) throws RangerAuthzException {
- validateRequest(request);
+ @Override
+ public RangerResourcePermissions
getResourcePermissions(RangerResourcePermissionsRequest request) throws
RangerAuthzException {
+ validateAccessContext(request.getContext());
RangerAuthzPlugin plugin =
getOrCreatePlugin(request.getContext().getServiceName(),
request.getContext().getServiceType());
- return authorize(request, plugin, auditHandler);
+ return plugin.getResourcePermissions(request);
Review Comment:
`getResourcePermissions()` validates only the access context. Since the
method immediately dereferences `request.getResource()`/`request.getContext()`,
a null `request` or `resource` will currently result in a
`NullPointerException` instead of a consistent `RangerAuthzException` (and it
also bypasses existing `validateResourceInfo()` checks). Consider validating
the request object and calling `validateResourceInfo(request.getResource())` in
addition to `validateAccessContext(request.getContext())` (or adding a
dedicated `validateRequest(RangerResourcePermissionsRequest)` helper similar to
the other request types).
--
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]
