roryqi commented on code in PR #10868:
URL: https://github.com/apache/gravitino/pull/10868#discussion_r3166875651


##########
server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinAuthorizer.java:
##########
@@ -504,40 +516,106 @@ private void loadRolePrivilege(
                         Entity.EntityType.USER);
             List<CompletableFuture<Void>> loadRoleFutures = new ArrayList<>();
             for (RoleEntity role : entities) {
-              Long roleId = role.id();
-              allowEnforcer.addRoleForUser(String.valueOf(userId), 
String.valueOf(roleId));
-              denyEnforcer.addRoleForUser(String.valueOf(userId), 
String.valueOf(roleId));
-              if (loadedRoles.getIfPresent(roleId) != null) {
+              addRoleForUserAndLoadPolicies(
+                  userId, metalake, role.id(), role.name(), loadRoleFutures, 
entityStore);
+            }
+
+            // Load roles inherited from the user's groups.
+            for (GroupEntity groupEntity : resolveCurrentUserGroups(metalake, 
entityStore)) {
+              List<Long> roleIds = groupEntity.roleIds();
+              List<String> roleNames = groupEntity.roleNames();
+              if (roleIds == null || roleNames == null) {
+                continue;
+              }
+              if (roleIds.size() != roleNames.size()) {
+                LOG.warn(
+                    "Group {} has mismatched roleIds ({}) and roleNames ({}) 
-- skipping",
+                    groupEntity.name(),
+                    roleIds.size(),
+                    roleNames.size());
                 continue;
               }
-              CompletableFuture<Void> loadRoleFuture =
-                  CompletableFuture.supplyAsync(
-                          () -> {
-                            try {
-                              return entityStore.get(
-                                  NameIdentifierUtil.ofRole(metalake, 
role.name()),
-                                  Entity.EntityType.ROLE,
-                                  RoleEntity.class);
-                            } catch (Exception e) {
-                              throw new RuntimeException("Failed to load role: 
" + role.name(), e);
-                            }
-                          },
-                          executor)
-                      .thenAcceptAsync(
-                          roleEntity -> {
-                            loadPolicyByRoleEntity(roleEntity);
-                            loadedRoles.put(roleId, true);
-                          },
-                          executor);
-              loadRoleFutures.add(loadRoleFuture);
+              for (int i = 0; i < roleIds.size(); i++) {
+                addRoleForUserAndLoadPolicies(
+                    userId,
+                    metalake,
+                    roleIds.get(i),
+                    roleNames.get(i),
+                    loadRoleFutures,
+                    entityStore);
+              }
             }
+
             CompletableFuture.allOf(loadRoleFutures.toArray(new 
CompletableFuture[0])).join();
           } catch (IOException e) {
             throw new RuntimeException(e);
           }
         });
   }
 
+  /**
+   * Resolves GroupEntity objects for the current principal's groups, skipping 
any that are stale or
+   * not found in the store.
+   */
+  private List<GroupEntity> resolveCurrentUserGroups(String metalake, 
EntityStore entityStore) {
+    Principal principal = PrincipalUtils.getCurrentPrincipal();
+    List<GroupEntity> result = new ArrayList<>();
+    if (!(principal instanceof UserPrincipal)) {
+      return result;
+    }
+    for (UserGroup group : ((UserPrincipal) principal).getGroups()) {
+      try {
+        NameIdentifier groupIdent = NameIdentifierUtil.ofGroup(metalake, 
group.getGroupname());

Review Comment:
   Could u batch get the group entities?



-- 
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]

Reply via email to