collado-mike commented on code in PR #3852:
URL: https://github.com/apache/polaris/pull/3852#discussion_r3017562874
##########
runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java:
##########
@@ -2331,4 +2400,336 @@ private static PolarisEntitySubType
selectEntitySubType(List<PolarisEntitySubTyp
subTypes));
}
}
+
+ /**
+ * Grants the principal_role_viewer principal role to all principals
assigned to the specified
+ * principal role. This allows catalog admins to list principal roles.
+ */
+ private void grantCatalogRoleManagerIfNeeded(PrincipalRoleEntity
principalRoleEntity) {
+ // Load principal_role_viewer directly from metastore
+ EntityResult catalogRoleManagerResult =
+ metaStoreManager.readEntityByName(
+ getCurrentPolarisContext(),
+ null,
+ PolarisEntityType.PRINCIPAL_ROLE,
+ PolarisEntitySubType.NULL_SUBTYPE,
+ PolarisEntityConstants.getNameOfPrincipalRoleViewerRole());
+
+ if (!catalogRoleManagerResult.isSuccess() ||
catalogRoleManagerResult.getEntity() == null) {
+ LOGGER.warn(
+ "principal_role_viewer role not found. This role should be created
during bootstrap. "
+ + "Existing deployments may need to re-bootstrap to enable this
feature.");
+ return;
+ }
+
+ PrincipalRoleEntity catalogRoleManagerEntity =
+ PrincipalRoleEntity.of(catalogRoleManagerResult.getEntity());
+
+ // Find all principals that have this principal role and grant
principal_role_viewer to them
+ LoadGrantsResult grantsResult =
+ metaStoreManager.loadGrantsOnSecurable(getCurrentPolarisContext(),
principalRoleEntity);
+
+ if (grantsResult.isSuccess()) {
+ for (PolarisGrantRecord grant : grantsResult.getGrantRecords()) {
+ // Check if this is a PRINCIPAL_ROLE_USAGE grant (principal using this
role)
+ if (grant.getPrivilegeCode() ==
PolarisPrivilege.PRINCIPAL_ROLE_USAGE.getCode()) {
+ // Load the principal (grantee)
+ EntityResult principalResult =
+ metaStoreManager.loadEntity(
+ getCurrentPolarisContext(),
+ grant.getGranteeCatalogId(),
+ grant.getGranteeId(),
+ PolarisEntityType.PRINCIPAL);
+
+ if (principalResult.isSuccess() && principalResult.getEntity() !=
null) {
+ PrincipalEntity principal =
PrincipalEntity.of(principalResult.getEntity());
+
+ // Check if the principal already has principal_role_viewer
+ LoadGrantsResult principalGrantsResult =
+
metaStoreManager.loadGrantsToGrantee(getCurrentPolarisContext(), principal);
+
+ boolean alreadyHasCatalogRoleManager = false;
+ if (principalGrantsResult.isSuccess()) {
+ for (PolarisGrantRecord existingGrant :
principalGrantsResult.getGrantRecords()) {
+ if (existingGrant.getSecurableId() ==
catalogRoleManagerEntity.getId()
+ && existingGrant.getPrivilegeCode()
+ == PolarisPrivilege.PRINCIPAL_ROLE_USAGE.getCode()) {
+ alreadyHasCatalogRoleManager = true;
+ break;
+ }
+ }
+ }
+
+ // Only grant if not already granted
+ if (!alreadyHasCatalogRoleManager) {
+ metaStoreManager.grantUsageOnRoleToGrantee(
+ getCurrentPolarisContext(), null, catalogRoleManagerEntity,
principal);
+ }
+ }
+ }
+ }
+ }
+ }
+
+ /**
+ * Revokes the principal_role_viewer principal role from all principals
assigned to the specified
+ * principal role if they no longer have any catalog_admin grants.
+ */
+ private void revokeCatalogRoleManagerIfNeeded(PrincipalRoleEntity
principalRoleEntity) {
Review Comment:
This is an n^2 loop that is very likely to leave the catalog privileges in a
weird state. For every principal granted this principal role, you find all
principal roles that have been granted that principal and then check the
catalog roles of each. If 1000 principals have 1000 roles, you'll make
1,000,000 calls. If the request times out halfway through (likely), you may
only remove the list role from half of the principals, leaving the other half
with a role they shouldn't have access to. I think something atomic and O(1)
would be a better approach to this problem
--
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]
