apache / apache/gravitino

[Improvement] TableHookDispatcher removes auth privileges when dropTable fails

Open
#10,131 5 comments 0 reactions 1 assignee Claimed by @arjnklc View on GitHub
good first issue improvement
Dominant language
Java
Stars
3.2k
Forks
935
Avg merge
1d 16h
Merged PRs (30d)
298

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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.