apache/gravitino

[Improvement] TableHookDispatcher removes auth privileges when dropTable fails

Open

#10,131 opened on Mar 3, 2026

View on GitHub
 (5 comments) (0 reactions) (1 assignee)Java (887 forks)auto 404
good first issueimprovement

Repository metrics

Stars
 (3,058 stars)
PR merge metrics
 (PR metrics pending)

Description

What would you like to be improved?

TableHookDispatcher.dropTable(...) and purgeTable(...) call AuthorizationUtils.authorizationPluginRemovePrivileges(...) unconditionally. If the underlying dispatcher returns false (table not dropped/purged), privileges are still removed, causing authorization metadata to become inconsistent with the table's actual state.

How should we improve?

A possible solution is to guard privilege removal behind operation success:

  • In dropTable(...), call authorizationPluginRemovePrivileges(...) only when dropped == true.
  • In purgeTable(...), call authorizationPluginRemovePrivileges(...) only when purged == true.

Here's a unit test to help:

@Test
public void testDropTableShouldNotRemovePrivilegesWhenDropFails() {
  TableDispatcher dispatcher = Mockito.mock(TableDispatcher.class);
  TableHookDispatcher hookDispatcher = new TableHookDispatcher(dispatcher);
  NameIdentifier ident = NameIdentifier.of("metalake", "catalog", "schema", "table");

  Mockito.when(dispatcher.dropTable(ident)).thenReturn(false);

  try (MockedStatic<AuthorizationUtils> authz = Mockito.mockStatic(AuthorizationUtils.class)) {
    authz
        .when(() -> AuthorizationUtils.getMetadataObjectLocation(ident, Entity.EntityType.TABLE))
        .thenReturn(ImmutableList.of("/test"));

    boolean dropped = hookDispatcher.dropTable(ident);

    Assertions.assertFalse(dropped);
    authz.verify(
        () ->
            AuthorizationUtils.authorizationPluginRemovePrivileges(
                ident, Entity.EntityType.TABLE, ImmutableList.of("/test")),
        Mockito.never());
  }
}

Place in core/src/test/java/org/apache/gravitino/hook/TestTableHookDispatcher.java. Some imports will also need to be added.

Contributor guide