justinmclean opened a new issue, #10131:
URL: https://github.com/apache/gravitino/issues/10131
### 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.
--
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]
