[#6306] fix(authz): Fix the OOM of JdbcAuthorizationPlugin

apache · Jan 17, 2025 · d8d913f · d8d913f
1 parent e6225a0
commit d8d913f
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 11 deletions.
diff --git a/...common/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationPlugin.java b/...common/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationPlugin.java
@@ -140,7 +140,6 @@ public Boolean onRoleDeleted(Role role) throws AuthorizationPluginException {
   @Override
   public Boolean onRoleUpdated(Role role, RoleChange... changes)
       throws AuthorizationPluginException {
-    onRoleCreated(role);
     for (RoleChange change : changes) {
       if (change instanceof RoleChange.AddSecurableObject) {
         SecurableObject object = ((RoleChange.AddSecurableObject) change).getSecurableObject();

diff --git a/...on/src/test/java/org/apache/gravitino/authorization/jdbc/TestJdbcAuthorizationPlugin.java b/...on/src/test/java/org/apache/gravitino/authorization/jdbc/TestJdbcAuthorizationPlugin.java
@@ -148,23 +148,21 @@ public void testPermissionManagement() {
 
     // Test metalake object and different role change
     resetSQLIndex();
-    expectSQLs = Lists.newArrayList("CREATE ROLE tmp", "GRANT SELECT ON TABLE *.* TO ROLE tmp");
+    expectSQLs = Lists.newArrayList("GRANT SELECT ON TABLE *.* TO ROLE tmp");
     SecurableObject metalakeObject =
         SecurableObjects.ofMetalake("metalake", Lists.newArrayList(Privileges.SelectTable.allow()));
     RoleChange roleChange = RoleChange.addSecurableObject("tmp", metalakeObject);
     plugin.onRoleUpdated(role, roleChange);
 
     resetSQLIndex();
-    expectSQLs = Lists.newArrayList("CREATE ROLE tmp", "REVOKE SELECT ON TABLE *.* FROM ROLE tmp");
+    expectSQLs = Lists.newArrayList("REVOKE SELECT ON TABLE *.* FROM ROLE tmp");
     roleChange = RoleChange.removeSecurableObject("tmp", metalakeObject);
     plugin.onRoleUpdated(role, roleChange);
 
     resetSQLIndex();
     expectSQLs =
         Lists.newArrayList(
-            "CREATE ROLE tmp",
-            "REVOKE SELECT ON TABLE *.* FROM ROLE tmp",
-            "GRANT CREATE ON TABLE *.* TO ROLE tmp");
+            "REVOKE SELECT ON TABLE *.* FROM ROLE tmp", "GRANT CREATE ON TABLE *.* TO ROLE tmp");
     SecurableObject newMetalakeObject =
         SecurableObjects.ofMetalake("metalake", Lists.newArrayList(Privileges.CreateTable.allow()));
     roleChange = RoleChange.updateSecurableObject("tmp", metalakeObject, newMetalakeObject);
@@ -175,7 +173,7 @@ public void testPermissionManagement() {
     SecurableObject catalogObject =
         SecurableObjects.ofCatalog("catalog", Lists.newArrayList(Privileges.SelectTable.allow()));
     roleChange = RoleChange.addSecurableObject("tmp", catalogObject);
-    expectSQLs = Lists.newArrayList("CREATE ROLE tmp", "GRANT SELECT ON TABLE *.* TO ROLE tmp");
+    expectSQLs = Lists.newArrayList("GRANT SELECT ON TABLE *.* TO ROLE tmp");
     plugin.onRoleUpdated(role, roleChange);
 
     // Test schema object
@@ -184,8 +182,7 @@ public void testPermissionManagement() {
         SecurableObjects.ofSchema(
             catalogObject, "schema", Lists.newArrayList(Privileges.SelectTable.allow()));
     roleChange = RoleChange.addSecurableObject("tmp", schemaObject);
-    expectSQLs =
-        Lists.newArrayList("CREATE ROLE tmp", "GRANT SELECT ON TABLE schema.* TO ROLE tmp");
+    expectSQLs = Lists.newArrayList("GRANT SELECT ON TABLE schema.* TO ROLE tmp");
     plugin.onRoleUpdated(role, roleChange);
 
     // Test table object
@@ -194,8 +191,18 @@ public void testPermissionManagement() {
         SecurableObjects.ofTable(
             schemaObject, "table", Lists.newArrayList(Privileges.SelectTable.allow()));
     roleChange = RoleChange.addSecurableObject("tmp", tableObject);
-    expectSQLs =
-        Lists.newArrayList("CREATE ROLE tmp", "GRANT SELECT ON TABLE schema.table TO ROLE tmp");
+    expectSQLs = Lists.newArrayList("GRANT SELECT ON TABLE schema.table TO ROLE tmp");
+    plugin.onRoleUpdated(role, roleChange);
+
+    // Test the role with objects
+    resetSQLIndex();
+    role =
+        RoleEntity.builder()
+            .withId(-1L)
+            .withName("tmp")
+            .withSecurableObjects(Lists.newArrayList(tableObject))
+            .withAuditInfo(AuditInfo.EMPTY)
+            .build();
     plugin.onRoleUpdated(role, roleChange);
   }