[#6011] fix(authz): MODIFY_TABLE should contain the select table pr…

…ivilege (#6620) ### What changes were proposed in this pull request? `MODIFY_TABLE` should contain the select table privilege ### Why are the changes needed? More proper semantics Fix: #6011 ### Does this PR introduce _any_ user-facing change? Correct the document. ### How was this patch tested? Modify the UT.
apache · Mar 7, 2025 · 61cfb52 · 61cfb52
1 parent 392cdd5
commit 61cfb52
Show file tree

Hide file tree

Showing 7 changed files with 57 additions and 56 deletions.
diff --git a/...org/apache/gravitino/authorization/chain/integration/test/TestChainedAuthorizationIT.java b/...org/apache/gravitino/authorization/chain/integration/test/TestChainedAuthorizationIT.java
@@ -320,22 +320,22 @@ void testCreateTable() throws InterruptedException {
   }
 
   @Test
-  void testReadWriteTableWithMetalakeLevelRole() throws InterruptedException {
+  void testSelectModifyTableWithMetalakeLevelRole() throws InterruptedException {
     // TODO
   }
 
   @Test
-  void testReadWriteTableWithTableLevelRole() throws InterruptedException {
+  void testSelectModifyTableWithTableLevelRole() throws InterruptedException {
     // TODO
   }
 
   @Test
-  void testReadOnlyTable() throws InterruptedException {
+  void testSelectOnlyTable() throws InterruptedException {
     // TODO
   }
 
   @Test
-  void testWriteOnlyTable() throws InterruptedException {
+  void testModifyOnlyTable() throws InterruptedException {
     // TODO
   }
 
@@ -385,32 +385,32 @@ protected void checkTableAllPrivilegesExceptForCreating() {
   }
 
   @Override
-  protected void checkUpdateSQLWithReadWritePrivileges() {
+  protected void checkUpdateSQLWithSelectModifyPrivileges() {
     // TODO
   }
 
   @Override
-  protected void checkUpdateSQLWithReadPrivileges() {
+  protected void checkUpdateSQLWithSelectPrivileges() {
     // TODO
   }
 
   @Override
-  protected void checkUpdateSQLWithWritePrivileges() {
+  protected void checkUpdateSQLWithModifyPrivileges() {
     // TODO
   }
 
   @Override
-  protected void checkDeleteSQLWithReadWritePrivileges() {
+  protected void checkDeleteSQLWithSelectModifyPrivileges() {
     // TODO
   }
 
   @Override
-  protected void checkDeleteSQLWithReadPrivileges() {
+  protected void checkDeleteSQLWithSelectPrivileges() {
     // TODO
   }
 
   @Override
-  protected void checkDeleteSQLWithWritePrivileges() {
+  protected void checkDeleteSQLWithModifyPrivileges() {
     // TODO
   }
 

diff --git a/...in/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationHadoopSQLPlugin.java b/...in/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationHadoopSQLPlugin.java
@@ -78,6 +78,8 @@ public Map<Privilege.Name, Set<AuthorizationPrivilege>> privilegesMappingRule()
         ImmutableSet.of(RangerHadoopSQLPrivilege.CREATE),
         Privilege.Name.MODIFY_TABLE,
         ImmutableSet.of(
+            RangerHadoopSQLPrivilege.READ,
+            RangerHadoopSQLPrivilege.SELECT,
             RangerHadoopSQLPrivilege.UPDATE,
             RangerHadoopSQLPrivilege.ALTER,
             RangerHadoopSQLPrivilege.WRITE),

diff --git a/...test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerBaseE2EIT.java b/...test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerBaseE2EIT.java
@@ -191,17 +191,17 @@ protected static void waitForUpdatingPolicies() {
 
   protected abstract void checkTableAllPrivilegesExceptForCreating();
 
-  protected abstract void checkUpdateSQLWithReadWritePrivileges();
+  protected abstract void checkUpdateSQLWithSelectModifyPrivileges();
 
-  protected abstract void checkUpdateSQLWithReadPrivileges();
+  protected abstract void checkUpdateSQLWithSelectPrivileges();
 
-  protected abstract void checkUpdateSQLWithWritePrivileges();
+  protected abstract void checkUpdateSQLWithModifyPrivileges();
 
-  protected abstract void checkDeleteSQLWithReadWritePrivileges();
+  protected abstract void checkDeleteSQLWithSelectModifyPrivileges();
 
-  protected abstract void checkDeleteSQLWithReadPrivileges();
+  protected abstract void checkDeleteSQLWithSelectPrivileges();
 
-  protected abstract void checkDeleteSQLWithWritePrivileges();
+  protected abstract void checkDeleteSQLWithModifyPrivileges();
 
   protected abstract void useCatalog();
 
@@ -313,7 +313,7 @@ void testCreateTable() throws InterruptedException {
   }
 
   @Test
-  void testReadWriteTableWithMetalakeLevelRole() throws InterruptedException {
+  void testSelectModifyTableWithMetalakeLevelRole() throws InterruptedException {
     // Choose a catalog
     useCatalog();
 
@@ -346,10 +346,10 @@ void testReadWriteTableWithMetalakeLevelRole() throws InterruptedException {
     sparkSession.sql(SQL_SELECT_TABLE).collectAsList();
 
     // case 3: Update data in the table
-    checkUpdateSQLWithReadWritePrivileges();
+    checkUpdateSQLWithSelectModifyPrivileges();
 
     // case 4:  Delete data from the table.
-    checkDeleteSQLWithReadWritePrivileges();
+    checkDeleteSQLWithSelectModifyPrivileges();
 
     // case 5: Succeed to alter the table
     testAlterTable();
@@ -368,7 +368,7 @@ void testReadWriteTableWithMetalakeLevelRole() throws InterruptedException {
   }
 
   @Test
-  void testReadWriteTableWithTableLevelRole() throws InterruptedException {
+  void testSelectModifyTableWithTableLevelRole() throws InterruptedException {
     // Choose a catalog
     useCatalog();
 
@@ -410,10 +410,10 @@ void testReadWriteTableWithTableLevelRole() throws InterruptedException {
     sparkSession.sql(SQL_SELECT_TABLE).collectAsList();
 
     // case 3: Update data in the table.
-    checkUpdateSQLWithReadWritePrivileges();
+    checkUpdateSQLWithSelectModifyPrivileges();
 
     // case 4: Delete data from the table.
-    checkDeleteSQLWithReadWritePrivileges();
+    checkDeleteSQLWithSelectModifyPrivileges();
 
     // case 5: Succeed to alter the table
     testAlterTable();
@@ -432,7 +432,7 @@ void testReadWriteTableWithTableLevelRole() throws InterruptedException {
   }
 
   @Test
-  void testReadOnlyTable() throws InterruptedException {
+  void testSelectOnlyTable() throws InterruptedException {
     // Choose a catalog
     useCatalog();
 
@@ -464,10 +464,10 @@ void testReadOnlyTable() throws InterruptedException {
     sparkSession.sql(SQL_SELECT_TABLE).collectAsList();
 
     // case 3: Update data in the table
-    checkUpdateSQLWithReadPrivileges();
+    checkUpdateSQLWithSelectPrivileges();
 
     // case 4: Delete data from the table
-    checkDeleteSQLWithReadPrivileges();
+    checkDeleteSQLWithSelectPrivileges();
 
     // case 5: Fail to alter the table
     Assertions.assertThrows(AccessControlException.class, () -> sparkSession.sql(SQL_ALTER_TABLE));
@@ -486,7 +486,7 @@ void testReadOnlyTable() throws InterruptedException {
   }
 
   @Test
-  void testWriteOnlyTable() throws InterruptedException {
+  void testModifyOnlyTable() throws InterruptedException {
     // Choose a catalog
     useCatalog();
 
@@ -514,15 +514,14 @@ void testWriteOnlyTable() throws InterruptedException {
     // case 1: Succeed to insert data into the table
     sparkSession.sql(SQL_INSERT_TABLE);
 
-    // case 2: Fail to select data from the table
-    Assertions.assertThrows(
-        AccessControlException.class, () -> sparkSession.sql(SQL_SELECT_TABLE).collectAsList());
+    // case 2: Succeed to select data from the table
+    sparkSession.sql(SQL_SELECT_TABLE).collectAsList();
 
     // case 3: Update data in the table
-    checkUpdateSQLWithWritePrivileges();
+    checkUpdateSQLWithModifyPrivileges();
 
     // case 4: Delete data from the table
-    checkDeleteSQLWithWritePrivileges();
+    checkDeleteSQLWithModifyPrivileges();
 
     // case 5: Succeed to alter the table
     testAlterTable();

diff --git a/...test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveE2EIT.java b/...test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveE2EIT.java
@@ -133,35 +133,35 @@ protected void checkWithoutPrivileges() {
   }
 
   @Override
-  protected void checkUpdateSQLWithReadWritePrivileges() {
+  protected void checkUpdateSQLWithSelectModifyPrivileges() {
     Assertions.assertThrows(
         SparkUnsupportedOperationException.class, () -> sparkSession.sql(SQL_UPDATE_TABLE));
   }
 
   @Override
-  protected void checkUpdateSQLWithReadPrivileges() {
+  protected void checkUpdateSQLWithSelectPrivileges() {
     Assertions.assertThrows(
         SparkUnsupportedOperationException.class, () -> sparkSession.sql(SQL_UPDATE_TABLE));
   }
 
   @Override
-  protected void checkUpdateSQLWithWritePrivileges() {
+  protected void checkUpdateSQLWithModifyPrivileges() {
     Assertions.assertThrows(
         SparkUnsupportedOperationException.class, () -> sparkSession.sql(SQL_UPDATE_TABLE));
   }
 
   @Override
-  protected void checkDeleteSQLWithReadWritePrivileges() {
+  protected void checkDeleteSQLWithSelectModifyPrivileges() {
     Assertions.assertThrows(AnalysisException.class, () -> sparkSession.sql(SQL_DELETE_TABLE));
   }
 
   @Override
-  protected void checkDeleteSQLWithReadPrivileges() {
+  protected void checkDeleteSQLWithSelectPrivileges() {
     Assertions.assertThrows(AnalysisException.class, () -> sparkSession.sql(SQL_DELETE_TABLE));
   }
 
   @Override
-  protected void checkDeleteSQLWithWritePrivileges() {
+  protected void checkDeleteSQLWithModifyPrivileges() {
     Assertions.assertThrows(AnalysisException.class, () -> sparkSession.sql(SQL_DELETE_TABLE));
   }
 

diff --git a/...t/java/org/apache/gravitino/authorization/ranger/integration/test/RangerIcebergE2EIT.java b/...t/java/org/apache/gravitino/authorization/ranger/integration/test/RangerIcebergE2EIT.java
@@ -111,33 +111,33 @@ protected String testUserName() {
     return System.getenv(HADOOP_USER_NAME);
   }
 
-  public void checkUpdateSQLWithReadWritePrivileges() {
+  public void checkUpdateSQLWithSelectModifyPrivileges() {
     sparkSession.sql(SQL_UPDATE_TABLE);
   }
 
   @Override
-  public void checkUpdateSQLWithReadPrivileges() {
+  public void checkUpdateSQLWithSelectPrivileges() {
     Assertions.assertThrows(AccessControlException.class, () -> sparkSession.sql(SQL_UPDATE_TABLE));
   }
 
   @Override
-  public void checkUpdateSQLWithWritePrivileges() {
-    Assertions.assertThrows(AccessControlException.class, () -> sparkSession.sql(SQL_UPDATE_TABLE));
+  public void checkUpdateSQLWithModifyPrivileges() {
+    sparkSession.sql(SQL_UPDATE_TABLE);
   }
 
   @Override
-  public void checkDeleteSQLWithReadWritePrivileges() {
+  public void checkDeleteSQLWithSelectModifyPrivileges() {
     sparkSession.sql(SQL_DELETE_TABLE);
   }
 
   @Override
-  public void checkDeleteSQLWithReadPrivileges() {
+  public void checkDeleteSQLWithSelectPrivileges() {
     Assertions.assertThrows(AccessControlException.class, () -> sparkSession.sql(SQL_DELETE_TABLE));
   }
 
   @Override
-  public void checkDeleteSQLWithWritePrivileges() {
-    Assertions.assertThrows(AccessControlException.class, () -> sparkSession.sql(SQL_DELETE_TABLE));
+  public void checkDeleteSQLWithModifyPrivileges() {
+    sparkSession.sql(SQL_DELETE_TABLE);
   }
 
   public void checkWithoutPrivileges() {

diff --git a/...st/java/org/apache/gravitino/authorization/ranger/integration/test/RangerPaimonE2EIT.java b/...st/java/org/apache/gravitino/authorization/ranger/integration/test/RangerPaimonE2EIT.java
@@ -130,32 +130,32 @@ protected void useCatalog() {
   }
 
   @Override
-  protected void checkUpdateSQLWithReadWritePrivileges() {
+  protected void checkUpdateSQLWithSelectModifyPrivileges() {
     // Kyuubi Paimon Ranger plugin doesn't support to update yet.
   }
 
   @Override
-  protected void checkUpdateSQLWithReadPrivileges() {
+  protected void checkUpdateSQLWithSelectPrivileges() {
     // Kyuubi Paimon Ranger plugin doesn't support to update yet.
   }
 
   @Override
-  protected void checkUpdateSQLWithWritePrivileges() {
+  protected void checkUpdateSQLWithModifyPrivileges() {
     // Kyuubi Paimon Ranger plugin doesn't support to update yet.
   }
 
   @Override
-  protected void checkDeleteSQLWithReadWritePrivileges() {
+  protected void checkDeleteSQLWithSelectModifyPrivileges() {
     // Kyuubi Paimon Ranger plugin doesn't support to delete yet.
   }
 
   @Override
-  protected void checkDeleteSQLWithReadPrivileges() {
+  protected void checkDeleteSQLWithSelectPrivileges() {
     // Kyuubi Paimon Ranger plugin doesn't support to delete yet.
   }
 
   @Override
-  protected void checkDeleteSQLWithWritePrivileges() {
+  protected void checkDeleteSQLWithModifyPrivileges() {
     // Kyuubi Paimon Ranger plugin doesn't support to delete yet.
   }
 

diff --git a/docs/security/access-control.md b/docs/security/access-control.md
@@ -202,11 +202,11 @@ and `USE_SCHEMA` privileges on its parent schema.
 
 ### Table privileges
 
-| Name         | Supports Securable Object         | Operation                                        |
-|--------------|-----------------------------------|--------------------------------------------------|
-| CREATE_TABLE | Metalake, Catalog, Schema         | Create a table                                   |
-| MODIFY_TABLE | Metalake, Catalog, Schema, Table  | Write data to a table or modify the table schema |
-| SELECT_TABLE | Metalake, Catalog, Schema, Table  | Select data from a table                         |
+| Name         | Supports Securable Object         | Operation                                                                 |
+|--------------|-----------------------------------|---------------------------------------------------------------------------|
+| CREATE_TABLE | Metalake, Catalog, Schema         | Create a table                                                            |
+| MODIFY_TABLE | Metalake, Catalog, Schema, Table  | Select data from a data, write data to a table or modify the table schema |
+| SELECT_TABLE | Metalake, Catalog, Schema, Table  | Select data from a table                                                  |
 
 ### Topic privileges