dimas-b commented on code in PR #2784:
URL: https://github.com/apache/polaris/pull/2784#discussion_r2417680946


##########
integration-tests/src/main/java/org/apache/polaris/service/it/test/CatalogFederationIntegrationTest.java:
##########
@@ -340,4 +365,76 @@ void testFederatedCatalogWithTableRBAC() {
     managementApi.revokeGrant(federatedCatalogName, federatedCatalogRoleName, 
tableGrant);
     managementApi.addGrant(federatedCatalogName, federatedCatalogRoleName, 
defaultCatalogGrant);
   }
+
+  @Test
+  void testFederatedCatalogWithCredentialVending() {
+    managementApi.revokeGrant(federatedCatalogName, federatedCatalogRoleName, 
defaultCatalogGrant);
+
+    // Case 1: Only have TABLE_READ_PROPERTIES privilege, should not be able 
to read data
+    TableGrant tableGrant =
+        TableGrant.builder()
+            .setType(GrantResource.TypeEnum.TABLE)
+            .setPrivilege(TablePrivilege.TABLE_READ_PROPERTIES)
+            .setNamespace(List.of("ns1"))
+            .setTableName("test_table")
+            .build();
+    managementApi.addGrant(federatedCatalogName, federatedCatalogRoleName, 
tableGrant);
+    spark.sql("USE " + federatedCatalogName);
+
+    // Read table data should fail since TABLE_READ_PROPERTIES does not allow 
reading data
+    assertThatThrownBy(() -> spark.sql("SELECT * FROM ns1.test_table ORDER BY 
id"))
+        .isInstanceOf(ForbiddenException.class);
+
+    // Case 2: Only have TABLE_READ_DATA privilege, should be able to read 
data but not write
+    managementApi.revokeGrant(federatedCatalogName, federatedCatalogRoleName, 
tableGrant);
+    tableGrant =
+        TableGrant.builder()
+            .setType(GrantResource.TypeEnum.TABLE)
+            .setPrivilege(TablePrivilege.TABLE_READ_DATA)
+            .setNamespace(List.of("ns1"))
+            .setTableName("test_table")
+            .build();
+    managementApi.addGrant(federatedCatalogName, federatedCatalogRoleName, 
tableGrant);
+
+    // Verify that the vended credential allows reading the data
+    List<Row> ns1Data = spark.sql("SELECT * FROM ns1.test_table ORDER BY 
id").collectAsList();
+    assertThat(ns1Data).hasSize(2);
+    assertThat(ns1Data.get(0).getInt(0)).isEqualTo(1);
+    assertThat(ns1Data.get(0).getString(1)).isEqualTo("Alice");
+
+    // Verify that write is blocked since the vended credential should only 
have read permission
+    assertThatThrownBy(() -> spark.sql("INSERT INTO ns1.test_table VALUES (3, 
'Charlie')"))
+        .hasMessageContaining("Access Denied. (Service: S3, Status Code: 403");
+
+    // Case 3: TABLE_WRITE_DATA should
+    managementApi.revokeGrant(federatedCatalogName, federatedCatalogRoleName, 
tableGrant);
+    tableGrant =
+        TableGrant.builder()
+            .setType(GrantResource.TypeEnum.TABLE)
+            .setPrivilege(TablePrivilege.TABLE_WRITE_DATA)
+            .setNamespace(List.of("ns1"))
+            .setTableName("test_table")
+            .build();
+    managementApi.addGrant(federatedCatalogName, federatedCatalogRoleName, 
tableGrant);
+
+    spark.sql("INSERT INTO ns1.test_table VALUES (3, 'Charlie')");
+
+    // Verify the write was successful by reading back
+    List<Row> updatedData = spark.sql("SELECT * FROM ns1.test_table ORDER BY 
id").collectAsList();
+    assertThat(updatedData).hasSize(3);
+    assertThat(updatedData.get(2).getInt(0)).isEqualTo(3);
+    assertThat(updatedData.get(2).getString(1)).isEqualTo("Charlie");
+
+    // Verify the data is also visible from the local catalog (both point to 
same storage)
+    spark.sql(String.format("REFRESH TABLE %s.ns1.test_table", 
localCatalogName));
+    List<Row> localData =
+        spark
+            .sql(String.format("SELECT * FROM %s.ns1.test_table ORDER BY id", 
localCatalogName))
+            .collectAsList();
+    assertThat(localData).hasSize(3);
+    assertThat(localData.get(2).getInt(0)).isEqualTo(3);
+    assertThat(localData.get(2).getString(1)).isEqualTo("Charlie");
+    managementApi.revokeGrant(federatedCatalogName, federatedCatalogRoleName, 
tableGrant);
+    managementApi.addGrant(federatedCatalogName, federatedCatalogRoleName, 
defaultCatalogGrant);

Review Comment:
   The remove/re-add `defaultCatalogGrant` pattern looks risky. If the test 
fails this line will not execute... Will that affect other tests? 🤔 
   
   Also, `defaultCatalogGrant` seems to be added inside `@BeforeEach`... Do we 
have to re-add it here at all?
   
   



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