mneethiraj commented on code in PR #703:
URL: https://github.com/apache/ranger/pull/703#discussion_r2440462307
##########
security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java:
##########
@@ -1742,7 +1742,7 @@ public RangerPolicy updatePolicy(RangerPolicy policy,
@PathParam("id") Long id)
validator.validate(policy, Action.UPDATE, bizUtil.isAdmin() ||
isServiceAdmin(policy.getService()) || isZoneAdmin(policy.getZoneName()));
- ensureAdminAccess(policy);
+ ensureAdminAccess(policy, null);
Review Comment:
To avoid updates to existing use of `ensureAdminAccess(policy)`, I suggest
adding following method:
```
void ensureAdminAccess(RangerPolicy policy) {
ensureAdminAccess(policy, null);
}
```
##########
security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java:
##########
@@ -3204,12 +3204,12 @@ public RangerPolicy getPolicyByName(String serviceName,
String policyName, Strin
return ret;
}
- void ensureAdminAccess(RangerPolicy policy) {
+ void ensureAdminAccess(RangerPolicy policy, String grantor) {
blockIfGdsService(policy.getService());
boolean isAdmin = bizUtil.isAdmin();
boolean isKeyAdmin = bizUtil.isKeyAdmin();
- String userName = bizUtil.getCurrentUserLoginId();
+ String userName = bizUtil.getCurrentUserLoginId() != null ?
bizUtil.getCurrentUserLoginId() : grantor;
Review Comment:
When `grantor` is provided, that value should be used as `userName`, instead
of `bizUtil.getCurrentUserLoginId()`. Also, shouldn't `isAdmin` and
`isKeyAdmin` also be derived from `grantor`?
```
final String userName;
final boolean isAdmin;
final boolean isKeyAdmin;
if (StringUtils.isEmpty(grantor)) { // use currently logged-in user
userName = bizUtil.getCurrentUserLoginId();
isAdmin = bizUtil.isAdmin();
isKeyAdmin = bizUtil.isKeyAdmin();
} else {
// find role of the given grantor; logic from
SessionMgr.setUserRoles(userSession)
Collection<String> userRoles = userMgr.getRolesByLoginId(grantor); // add
@Autowired UserMgr userMgr;
userName = grantor;
isAdmin = userRoles.contains(RangerConstants.ROLE_SYS_ADMIN);
isKeyAdmin = userRoles.contains(RangerConstants.ROLE_KEY_ADMIN);
}
```
--
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]
