Skip to content

Commit

Permalink
[#4620] improvement(authz): Throw the necessary exception when Ranger…
Browse files Browse the repository at this point in the history 
… plugin the exception
  • Loading branch information
@jerqi
jerqi committed Feb 25, 2025
1 parent e907219 commit 3ed323b
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 49 deletions.
5 changes: 5 additions & 0 deletions ...in/src/main/java/org/apache/gravitino/authorization/chain/ChainedAuthorizationPlugin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,11 @@ public Boolean onRoleAcquired(Role role) throws AuthorizationPluginException {

@Override
public Boolean onRoleDeleted(Role role) throws AuthorizationPluginException {
onRoleUpdated(
role,
role.securableObjects().stream()
.map(securableObject -> RoleChange.removeSecurableObject(role.name(), securableObject))
.toArray(RoleChange[]::new));
return chainedAction(plugin -> plugin.onRoleDeleted(role));
}

Expand Down
94 changes: 49 additions & 45 deletions ...er/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationPlugin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -273,8 +273,13 @@ public Boolean onRoleDeleted(Role role) throws AuthorizationPluginException {
rangerClient.deleteRole(
rangerHelper.generateGravitinoRoleName(role.name()), rangerAdminName, rangerServiceName);
} catch (RangerServiceException e) {
// Ignore exception to support idempotent operation
LOG.warn("Ranger delete role: {} failed!", role, e);
if (e.getMessage().contains("No RangerRole found for name")) {
// Ignore exception to support idempotent operation
LOG.warn("Ranger delete role: {} failed!", role, e);
} else {
throw new AuthorizationPluginException(
"Fail to delete role %s exception: %s", role, e.getMessage());
}
}
return Boolean.TRUE;
}
Expand All @@ -292,14 +297,13 @@ public Boolean onRoleUpdated(Role role, RoleChange... changes)

List<AuthorizationSecurableObject> authzSecurableObjects =
translatePrivilege(securableObject);
authzSecurableObjects.stream()
.forEach(
authzSecurableObject -> {
if (!doAddSecurableObject(role.name(), authzSecurableObject)) {
throw new AuthorizationPluginException(
"Failed to add the securable object to the Ranger policy!");
}
});
authzSecurableObjects.forEach(
authzSecurableObject -> {
if (!doAddSecurableObject(role.name(), authzSecurableObject)) {
throw new AuthorizationPluginException(
"Failed to add the securable object to the Ranger policy!");
}
});
} else if (change instanceof RoleChange.RemoveSecurableObject) {
SecurableObject securableObject =
((RoleChange.RemoveSecurableObject) change).getSecurableObject();
Expand Down Expand Up @@ -337,16 +341,14 @@ public Boolean onRoleUpdated(Role role, RoleChange... changes)
translatePrivilege(oldSecurableObject);
List<AuthorizationSecurableObject> rangerNewSecurableObjects =
translatePrivilege(newSecurableObject);
rangerOldSecurableObjects.stream()
.forEach(
AuthorizationSecurableObject -> {
removeSecurableObject(role.name(), AuthorizationSecurableObject);
});
rangerNewSecurableObjects.stream()
.forEach(
AuthorizationSecurableObject -> {
doAddSecurableObject(role.name(), AuthorizationSecurableObject);
});
rangerOldSecurableObjects.forEach(
AuthorizationSecurableObject -> {
removeSecurableObject(role.name(), AuthorizationSecurableObject);
});
rangerNewSecurableObjects.forEach(
AuthorizationSecurableObject -> {
doAddSecurableObject(role.name(), AuthorizationSecurableObject);
});
} else {
throw new IllegalArgumentException(
"Unsupported role change type: "
Expand Down Expand Up @@ -499,23 +501,21 @@ public Boolean onOwnerSet(MetadataObject metadataObject, Owner preOwner, Owner n
LOG.warn("Grant owner role: {} failed!", ownerRoleName, e);
}

rangerSecurableObjects.stream()
.forEach(
rangerSecurableObject -> {
RangerPolicy policy = findManagedPolicy(rangerSecurableObject);
try {
if (policy == null) {
policy = addOwnerRoleToNewPolicy(rangerSecurableObject, ownerRoleName);
rangerClient.createPolicy(policy);
} else {
rangerHelper.updatePolicyOwnerRole(policy, ownerRoleName);
rangerClient.updatePolicy(policy.getId(), policy);
}
} catch (RangerServiceException e) {
throw new AuthorizationPluginException(
e, "Failed to add the owner to the Ranger!");
}
});
rangerSecurableObjects.forEach(
rangerSecurableObject -> {
RangerPolicy policy = findManagedPolicy(rangerSecurableObject);
try {
if (policy == null) {
policy = addOwnerRoleToNewPolicy(rangerSecurableObject, ownerRoleName);
rangerClient.createPolicy(policy);
} else {
rangerHelper.updatePolicyOwnerRole(policy, ownerRoleName);
rangerClient.updatePolicy(policy.getId(), policy);
}
} catch (RangerServiceException e) {
throw new AuthorizationPluginException(e, "Failed to add the owner to the Ranger!");
}
});
break;
case SCHEMA:
case TABLE:
Expand Down Expand Up @@ -576,8 +576,9 @@ public Boolean onGrantedRolesToUser(List<Role> roles, User user)
try {
rangerClient.grantRole(rangerServiceName, grantRevokeRoleRequest);
} catch (RangerServiceException e) {
// Ignore exception, support idempotent operation
LOG.warn("Grant role: {} to user: {} failed!", role, user, e);
throw new AuthorizationPluginException(
"Fail to grant role %s to user %s, exception: %s",
role.name(), user.name(), e.getMessage());
}
});

Expand Down Expand Up @@ -611,8 +612,9 @@ public Boolean onRevokedRolesFromUser(List<Role> roles, User user)
try {
rangerClient.revokeRole(rangerServiceName, grantRevokeRoleRequest);
} catch (RangerServiceException e) {
// Ignore exception to support idempotent operation
LOG.warn("Revoke role: {} from user: {} failed!", role, user, e);
throw new AuthorizationPluginException(
"Fail to revoke role %s from user %s, exception: %s",
role.name(), user.name(), e.getMessage());
}
});

Expand Down Expand Up @@ -646,8 +648,9 @@ public Boolean onGrantedRolesToGroup(List<Role> roles, Group group)
try {
rangerClient.grantRole(rangerServiceName, grantRevokeRoleRequest);
} catch (RangerServiceException e) {
// Ignore exception to support idempotent operation
LOG.warn("Grant role: {} to group: {} failed!", role, group, e);
throw new AuthorizationPluginException(
"Fail to grant role: %s to group %s, exception: %s.",
role, group, e.getMessage());
}
});
return Boolean.TRUE;
Expand Down Expand Up @@ -678,8 +681,9 @@ public Boolean onRevokedRolesFromGroup(List<Role> roles, Group group)
try {
rangerClient.revokeRole(rangerServiceName, grantRevokeRoleRequest);
} catch (RangerServiceException e) {
// Ignore exception to support idempotent operation
LOG.warn("Revoke role: {} from group: {} failed!", role, group, e);
throw new AuthorizationPluginException(
"Fail to revoke role %s from group %s, exception: %s",
role.name(), group.name(), e.getMessage());
}
});

Expand Down
15 changes: 12 additions & 3 deletions ...rization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ protected GrantRevokeRoleRequest createGrantRevokeRoleRequest(
Set<String> groups =
StringUtils.isEmpty(groupName) ? Sets.newHashSet() : Sets.newHashSet(groupName);

if (users.size() == 0 && groups.size() == 0) {
if (users.isEmpty() && groups.isEmpty()) {
throw new AuthorizationPluginException("The user and group cannot be empty!");
}

Expand Down Expand Up @@ -278,9 +278,18 @@ protected RangerRole createRangerRoleIfNotExists(String roleName, boolean isOwne
try {
rangerRole = rangerClient.getRole(roleName, rangerAdminName, rangerServiceName);
} catch (RangerServiceException e) {
// ignore exception, If the role does not exist, then create it.
LOG.warn("The role({}) does not exist in the Ranger!", roleName);

// The client will return a error message contains `doesn't have permission` if the role does
// not exist, then create it.
if (e.getMessage().contains("User doesn't have permissions to get details")) {
LOG.warn("The role({}) does not exist in the Ranger!, e: {}", roleName, e);
} else {
throw new AuthorizationPluginException(
"Failed to check role(%s) whether exists in the Ranger! e: %s",
roleName, e.getMessage());
}
}

try {
if (rangerRole == null) {
rangerRole = new RangerRole(roleName, RangerHelper.MANAGED_BY_GRAVITINO, null, null, null);
Expand Down
8 changes: 7 additions & 1 deletion ...rc/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,9 @@ public void testOnRoleCreated() {
RoleEntity role = mock3TableRole(currentFunName());
Assertions.assertTrue(rangerAuthHivePlugin.onRoleCreated(role));
verifyRoleInRanger(rangerAuthHivePlugin, role);

// Repeat to create the same to verify the idempotent operation
Assertions.assertTrue(rangerAuthHivePlugin.onRoleCreated(role));
}

@Test
Expand Down Expand Up @@ -266,6 +269,9 @@ public void testOnRoleDeleted() {
Assertions.assertTrue(rangerAuthHivePlugin.onRoleDeleted(role));
// Check if the policy is deleted
assertFindManagedPolicyItems(role, false);

// Repeat to delete the same role to verify the idempotent operation
Assertions.assertTrue(rangerAuthHivePlugin.onRoleDeleted(role));
}

@Test
Expand Down Expand Up @@ -294,7 +300,7 @@ public void testOnRoleDeleted2() {

// delete this role
Assertions.assertTrue(rangerAuthHivePlugin.onRoleDeleted(role));
// Because this metaobject has owner, so the policy should not be deleted
// Because this metadata object has owner, so the policy should not be deleted
assertFindManagedPolicyItems(role, false);
}

Expand Down

0 comments on commit 3ed323b

Please sign in to comment.