apache/gravitino

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

Open

#10.269 geöffnet am 6. März 2026

Auf GitHub ansehen
 (3 Kommentare) (0 Reaktionen) (1 zugewiesene Person)Java (887 Forks)auto 404
good first issueimprovement

Repository-Metriken

Stars
 (3.058 Stars)
PR-Merge-Metriken
 (PR-Metriken ausstehend)

Beschreibung

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<String> locations = ImmutableList.of("/tmp/fileset");

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

    try (MockedStatic<AuthorizationUtils> 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