You are viewing a plain text version of this content. The canonical link for it is here.

Posted to issues@polaris.apache.org by "ArnavBalyan (via GitHub)" <gi...@apache.org> on 2026/06/14 17:35:50 UTC

[PR] fix(catalog): Fix authorize generic table drop against the table over namespace [polaris]

ArnavBalyan opened a new pull request, #4758:
URL: https://github.com/apache/polaris/pull/4758

   - dropGenericTable authorizes via the create under namespace, so the authz target is the parent namespace, not the table.
   - Table drop grant on generic table is ignored and the drop is wrongly denied.
   - Use the correct table level check instead of the namespace. 
   - Add ut
   
   ## Checklist
   - [X] 🛡️ Don't disclose security issues! (contact security@apache.org)
   - [X] 🔗 Clearly explained why the changes are needed, or linked related issues: Fixes #
   - [X] 🧪 Added/updated tests with good coverage, or manually tested (and explained how)
   - [X] 💡 Added comments for complex logic
   - [X] 🧾 Updated `CHANGELOG.md` (if needed)
   - [X] 📚 Updated documentation in `site/content/in-dev/unreleased` (if needed)
   


-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] fix(catalog): Fix authorize generic table drop against the table over namespace [polaris]

Posted by "ArnavBalyan (via GitHub)" <gi...@apache.org>.

ArnavBalyan commented on PR #4758:
URL: https://github.com/apache/polaris/pull/4758#issuecomment-4716947520

   > How is this different from #4779 ?
   
   
   Looks like they are fixing the same issue, I'm not sure which one was raised sooner
   
   


-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] fix(catalog): Fix authorize generic table drop against the table over namespace [polaris]

Posted by "ArnavBalyan (via GitHub)" <gi...@apache.org>.

ArnavBalyan closed pull request #4758: fix(catalog): Fix authorize generic table drop against the table over namespace
URL: https://github.com/apache/polaris/pull/4758


-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] fix(catalog): Fix authorize generic table drop against the table over namespace [polaris]

Posted by "ArnavBalyan (via GitHub)" <gi...@apache.org>.

ArnavBalyan commented on PR #4758:
URL: https://github.com/apache/polaris/pull/4758#issuecomment-4702521320

   cc @dimas-b @jbonofre thanks! :) 


-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] fix(catalog): Fix authorize generic table drop against the table over namespace [polaris]

Posted by "ArnavBalyan (via GitHub)" <gi...@apache.org>.

ArnavBalyan commented on PR #4758:
URL: https://github.com/apache/polaris/pull/4758#issuecomment-4716937670

   Looks like they are fixing the same issue, I'm not sure which one was raised sooner


-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] fix(catalog): Fix authorize generic table drop against the table over namespace [polaris]

Posted by "ArnavBalyan (via GitHub)" <gi...@apache.org>.

ArnavBalyan commented on PR #4758:
URL: https://github.com/apache/polaris/pull/4758#issuecomment-4720970259

   Thanks of course :) 


-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] fix(catalog): Fix authorize generic table drop against the table over namespace [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.

dimas-b commented on PR #4758:
URL: https://github.com/apache/polaris/pull/4758#issuecomment-4720899032

   Since #4779 already has some approvals, I propose to go with that PR.
   
   @ArnavBalyan : Appreciate your contribution, in any case!


-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] fix(catalog): Fix authorize generic table drop against the table over namespace [polaris]

Posted by "ArnavBalyan (via GitHub)" <gi...@apache.org>.

ArnavBalyan commented on PR #4758:
URL: https://github.com/apache/polaris/pull/4758#issuecomment-4716945943

   > Looks like they are fixing the same issue, I'm not sure which one was raised sooner
   
   
   Looks like they are fixing the same issue, I'm not sure which one was raised sooner


-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] fix(catalog): Fix authorize generic table drop against the table over namespace [polaris]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.

Copilot commented on code in PR #4758:
URL: https://github.com/apache/polaris/pull/4758#discussion_r3409950694


##########
runtime/service/src/test/java/org/apache/polaris/service/catalog/generic/PolarisGenericTableCatalogHandlerAuthzTest.java:
##########
@@ -126,4 +127,13 @@ Stream<DynamicNode> testDropGenericTablePrivileges() {
         .shouldPassWith(PolarisPrivilege.CATALOG_MANAGE_METADATA)
         .createTests();
   }
+
+  @Test
+  void dropGenericTableHonorsTableScopedGrant() {
+    assertSuccess(
+        adminService.grantPrivilegeOnTableToRole(
+            CATALOG_NAME, CATALOG_ROLE1, TABLE_NS1_1_GENERIC, PolarisPrivilege.TABLE_DROP));
+
+    newWrapper(Set.of(PRINCIPAL_ROLE1)).dropGenericTable(TABLE_NS1_1_GENERIC);
+  }

Review Comment:
   This test only exercises the success path after granting `TABLE_DROP`. To fully lock in the intended behavior (that table-scoped grants are required/recognized), add a negative assertion that the drop is denied before the grant (or for a principal without the table grant), then succeeds after the grant. Optionally also assert the returned boolean (if meaningful) to ensure the handler result is validated.



##########
runtime/service/src/test/java/org/apache/polaris/service/catalog/generic/PolarisGenericTableCatalogHandlerAuthzTest.java:
##########
@@ -126,4 +127,13 @@ Stream<DynamicNode> testDropGenericTablePrivileges() {
         .shouldPassWith(PolarisPrivilege.CATALOG_MANAGE_METADATA)
         .createTests();
   }
+
+  @Test
+  void dropGenericTableHonorsTableScopedGrant() {
+    assertSuccess(
+        adminService.grantPrivilegeOnTableToRole(
+            CATALOG_NAME, CATALOG_ROLE1, TABLE_NS1_1_GENERIC, PolarisPrivilege.TABLE_DROP));

Review Comment:
   The test grants a privilege but never revokes it. If the underlying authz state persists across tests within this class/test suite, this can create order-dependent failures. Consider revoking the privilege in teardown/`finally`, or using a per-test role/table fixture that is cleaned up automatically.



-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org