apache / apache/gravitino

[Improvement] Prevent fileset privilege removal when drop operation returns false

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

Description

### What would you like to be improved?

FilesetHookDispatcher.dropFileset always calls AuthorizationUtils.authorizationPluginRemovePrivileges(...) after delegating the drop.
But the delegate can validly return false (fileset not dropped / not found). In that path, metadata still exists (or at least was not deleted by this operation), yet authorization privileges are removed anyway. This can desynchronize authorization state from metadata state and cause incorrect access behavior.

### How should we improve?

Guard privilege removal by the drop result:
- Fetch locations as today.
- Call dispatcher.dropFileset(ident).
- Only call authorizationPluginRemovePrivileges(...) when dropped == true.
- Return dropped unchanged.

Here's a test to help:
```

@Test
public void testDropFilesetShouldNotRemovePrivilegesWhenDropReturnsFalse() {
NameIdentifier ident = NameIdentifier.of("metalake", "catalog", "schema", "fileset");
FilesetDispatcher delegate = Mockito.mock(FilesetDispatcher.class);
FilesetHookDispatcher hookDispatcher = new FilesetHookDispatcher(delegate);
List locations = ImmutableList.of("/tmp/fileset");

Mockito.when(delegate.dropFileset(ident)).thenReturn(false);

try (MockedStatic mockedAuthz =
Mockito.mockStatic(AuthorizationUtils.class)) {
mockedAuthz
.when(
() -> AuthorizationUtils.getMetadataObjectLocation(ident, Entity.EntityType.FILESET))
.thenReturn(locations);

hookDispatcher.dropFileset(ident);

mockedAuthz.verify(
() ->
AuthorizationUtils.authorizationPluginRemovePrivileges(
ident, Entity.EntityType.FILESET, locations),
Mockito.never());
}
}
```

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.