diff --git a/core/src/main/java/org/apache/gravitino/GravitinoEnv.java b/core/src/main/java/org/apache/gravitino/GravitinoEnv.java index 04de93186fd..a27df871040 100644 --- a/core/src/main/java/org/apache/gravitino/GravitinoEnv.java +++ b/core/src/main/java/org/apache/gravitino/GravitinoEnv.java @@ -426,7 +426,7 @@ private void initGravitinoServerComponents() { this.accessControlDispatcher = accessControlHookDispatcher; this.ownerManager = new OwnerManager(entityStore); - this.futureGrantManager = new FutureGrantManager(entityStore); + this.futureGrantManager = new FutureGrantManager(entityStore, ownerManager); } else { this.accessControlDispatcher = null; this.ownerManager = null; diff --git a/core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java b/core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java index 6dd42e6281c..c182980b8ec 100644 --- a/core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java +++ b/core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java @@ -140,45 +140,62 @@ public static void checkRoleNamespace(Namespace namespace) { // Every catalog has one authorization plugin, we should avoid calling // underlying authorization repeatedly. So we use a set to record which // catalog has been called the authorization plugin. - public static void callAuthorizationPlugin( + public static void callAuthorizationPluginForSecurableObjects( String metalake, List securableObjects, Set catalogsAlreadySet, Consumer consumer) { CatalogManager catalogManager = GravitinoEnv.getInstance().catalogManager(); for (SecurableObject securableObject : securableObjects) { - if (needApplyAllAuthorizationPlugin(securableObject)) { + if (needApplyAuthorizationPluginAllCatalogs(securableObject)) { Catalog[] catalogs = catalogManager.listCatalogsInfo(Namespace.of(metalake)); for (Catalog catalog : catalogs) { - callAuthorizationPluginImpl(catalogsAlreadySet, consumer, catalog); + callAuthorizationPluginImpl(consumer, catalog); } - } else if (supportsSingleAuthorizationPlugin(securableObject.type())) { + } else if (needApplyAuthorization(securableObject.type())) { NameIdentifier catalogIdent = NameIdentifierUtil.getCatalogIdentifier( MetadataObjectUtil.toEntityIdent(metalake, securableObject)); Catalog catalog = catalogManager.loadCatalog(catalogIdent); - callAuthorizationPluginImpl(catalogsAlreadySet, consumer, catalog); + if (!catalogsAlreadySet.contains(catalog.name())) { + catalogsAlreadySet.add(catalog.name()); + callAuthorizationPluginImpl(consumer, catalog); + } + } + } + } + + public static void callAuthorizationPluginForMetadataObject( + String metalake, MetadataObject metadataObject, Consumer consumer) { + CatalogManager catalogManager = GravitinoEnv.getInstance().catalogManager(); + if (needApplyAuthorizationPluginAllCatalogs(metadataObject.type())) { + Catalog[] catalogs = catalogManager.listCatalogsInfo(Namespace.of(metalake)); + for (Catalog catalog : catalogs) { + callAuthorizationPluginImpl(consumer, catalog); } + } else if (needApplyAuthorization(metadataObject.type())) { + NameIdentifier catalogIdent = + NameIdentifierUtil.getCatalogIdentifier( + MetadataObjectUtil.toEntityIdent(metalake, metadataObject)); + Catalog catalog = catalogManager.loadCatalog(catalogIdent); + callAuthorizationPluginImpl(consumer, catalog); } } private static void callAuthorizationPluginImpl( - Set catalogsAlreadySet, Consumer consumer, Catalog catalog) { - if (!catalogsAlreadySet.contains(catalog.name())) { - catalogsAlreadySet.add(catalog.name()); - - if (catalog instanceof BaseCatalog) { - BaseCatalog baseCatalog = (BaseCatalog) catalog; - if (baseCatalog.getAuthorizationPlugin() != null) { - consumer.accept(baseCatalog.getAuthorizationPlugin()); - } + Consumer consumer, Catalog catalog) { + + if (catalog instanceof BaseCatalog) { + BaseCatalog baseCatalog = (BaseCatalog) catalog; + if (baseCatalog.getAuthorizationPlugin() != null) { + consumer.accept(baseCatalog.getAuthorizationPlugin()); } } } - public static boolean needApplyAllAuthorizationPlugin(SecurableObject securableObject) { - // TODO: Add supportsSecurableObjects method for every privilege to simplify this code + public static boolean needApplyAuthorizationPluginAllCatalogs(SecurableObject securableObject) { + // TODO: Add `supportsSecurableObjects` method for every privilege to simplify this code if (securableObject.type() == MetadataObject.Type.METALAKE) { List privileges = securableObject.privileges(); for (Privilege privilege : privileges) { @@ -190,7 +207,11 @@ public static boolean needApplyAllAuthorizationPlugin(SecurableObject securableO return false; } - private static boolean supportsSingleAuthorizationPlugin(MetadataObject.Type type) { + private static boolean needApplyAuthorizationPluginAllCatalogs(MetadataObject.Type type) { + return type == MetadataObject.Type.METALAKE; + } + + private static boolean needApplyAuthorization(MetadataObject.Type type) { return type != MetadataObject.Type.ROLE && type != MetadataObject.Type.METALAKE; } } diff --git a/core/src/main/java/org/apache/gravitino/authorization/FutureGrantManager.java b/core/src/main/java/org/apache/gravitino/authorization/FutureGrantManager.java index 1745d53d901..c24817ea5eb 100644 --- a/core/src/main/java/org/apache/gravitino/authorization/FutureGrantManager.java +++ b/core/src/main/java/org/apache/gravitino/authorization/FutureGrantManager.java @@ -23,10 +23,13 @@ import java.io.IOException; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; import org.apache.gravitino.Entity; import org.apache.gravitino.EntityStore; +import org.apache.gravitino.MetadataObject; +import org.apache.gravitino.MetadataObjects; import org.apache.gravitino.NameIdentifier; import org.apache.gravitino.SupportsRelationOperations; import org.apache.gravitino.connector.BaseCatalog; @@ -45,13 +48,25 @@ */ public class FutureGrantManager { EntityStore entityStore; + OwnerManager ownerManager; - public FutureGrantManager(EntityStore entityStore) { + public FutureGrantManager(EntityStore entityStore, OwnerManager ownerManager) { this.entityStore = entityStore; + this.ownerManager = ownerManager; } public void grantNewlyCreatedCatalog(String metalake, BaseCatalog catalog) { try { + MetadataObject metalakeObject = + MetadataObjects.of(null, metalake, MetadataObject.Type.METALAKE); + Optional ownerOptional = ownerManager.getOwner(metalake, metalakeObject); + ownerOptional.ifPresent( + owner -> { + AuthorizationPlugin authorizationPlugin = catalog.getAuthorizationPlugin(); + if (authorizationPlugin != null) { + authorizationPlugin.onOwnerSet(metalakeObject, null, owner); + } + }); Map> userGrantRoles = Maps.newHashMap(); Map> groupGrantRoles = Maps.newHashMap(); @@ -69,7 +84,7 @@ public void grantNewlyCreatedCatalog(String metalake, BaseCatalog catalog) { boolean supportsFutureGrant = false; for (SecurableObject object : role.securableObjects()) { - if (AuthorizationUtils.needApplyAllAuthorizationPlugin(object)) { + if (AuthorizationUtils.needApplyAuthorizationPluginAllCatalogs(object)) { supportsFutureGrant = true; break; } diff --git a/core/src/main/java/org/apache/gravitino/authorization/OwnerManager.java b/core/src/main/java/org/apache/gravitino/authorization/OwnerManager.java index ec1b2643834..04285954e97 100644 --- a/core/src/main/java/org/apache/gravitino/authorization/OwnerManager.java +++ b/core/src/main/java/org/apache/gravitino/authorization/OwnerManager.java @@ -68,7 +68,11 @@ public OwnerManager(EntityStore store) { public void setOwner( String metalake, MetadataObject metadataObject, String ownerName, Owner.Type ownerType) { try { + Optional originOwner = getOwner(metalake, metadataObject); + NameIdentifier objectIdent = MetadataObjectUtil.toEntityIdent(metalake, metadataObject); + OwnerImpl newOwner = new OwnerImpl(); + if (ownerType == Owner.Type.USER) { NameIdentifier ownerIdent = AuthorizationUtils.ofUser(metalake, ownerName); TreeLockUtils.doWithTreeLock( @@ -86,6 +90,9 @@ public void setOwner( true); return null; }); + + newOwner.name = ownerName; + newOwner.type = Owner.Type.USER; } else if (ownerType == Owner.Type.GROUP) { NameIdentifier ownerIdent = AuthorizationUtils.ofGroup(metalake, ownerName); TreeLockUtils.doWithTreeLock( @@ -103,7 +110,16 @@ public void setOwner( true); return null; }); + + newOwner.name = ownerName; + newOwner.type = Owner.Type.GROUP; } + + AuthorizationUtils.callAuthorizationPluginForMetadataObject( + metalake, + metadataObject, + authorizationPlugin -> + authorizationPlugin.onOwnerSet(metadataObject, originOwner.orElse(null), newOwner)); } catch (NoSuchEntityException nse) { LOG.warn( "Metadata object {} or owner {} is not found", metadataObject.fullName(), ownerName, nse); @@ -124,16 +140,12 @@ public Optional getOwner(String metalake, MetadataObject metadataObject) OwnerImpl owner = new OwnerImpl(); NameIdentifier ident = MetadataObjectUtil.toEntityIdent(metalake, metadataObject); List entities = - TreeLockUtils.doWithTreeLock( - ident, - LockType.READ, - () -> - store - .relationOperations() - .listEntitiesByRelation( - SupportsRelationOperations.Type.OWNER_REL, - ident, - MetadataObjectUtil.toEntityType(metadataObject))); + store + .relationOperations() + .listEntitiesByRelation( + SupportsRelationOperations.Type.OWNER_REL, + ident, + MetadataObjectUtil.toEntityType(metadataObject)); if (entities.isEmpty()) { return Optional.empty(); diff --git a/core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java b/core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java index 01c87d03084..edb02cdcec5 100644 --- a/core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java +++ b/core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java @@ -112,7 +112,7 @@ User grantRolesToUser(String metalake, List roles, String user) { Set catalogs = Sets.newHashSet(); for (Role grantedRole : roleEntitiesToGrant) { - AuthorizationUtils.callAuthorizationPlugin( + AuthorizationUtils.callAuthorizationPluginForSecurableObjects( metalake, grantedRole.securableObjects(), catalogs, @@ -191,7 +191,7 @@ Group grantRolesToGroup(String metalake, List roles, String group) { Set catalogs = Sets.newHashSet(); for (Role grantedRole : roleEntitiesToGrant) { - AuthorizationUtils.callAuthorizationPlugin( + AuthorizationUtils.callAuthorizationPluginForSecurableObjects( metalake, grantedRole.securableObjects(), catalogs, @@ -269,7 +269,7 @@ Group revokeRolesFromGroup(String metalake, List roles, String group) { Set catalogs = Sets.newHashSet(); for (Role grantedRole : roleEntitiesToRevoke) { - AuthorizationUtils.callAuthorizationPlugin( + AuthorizationUtils.callAuthorizationPluginForSecurableObjects( metalake, grantedRole.securableObjects(), catalogs, @@ -349,7 +349,7 @@ User revokeRolesFromUser(String metalake, List roles, String user) { Set catalogs = Sets.newHashSet(); for (Role grantedRole : roleEntitiesToRevoke) { - AuthorizationUtils.callAuthorizationPlugin( + AuthorizationUtils.callAuthorizationPluginForSecurableObjects( metalake, grantedRole.securableObjects(), catalogs, diff --git a/core/src/main/java/org/apache/gravitino/authorization/RoleManager.java b/core/src/main/java/org/apache/gravitino/authorization/RoleManager.java index b1539d019da..3edcfaa573a 100644 --- a/core/src/main/java/org/apache/gravitino/authorization/RoleManager.java +++ b/core/src/main/java/org/apache/gravitino/authorization/RoleManager.java @@ -109,7 +109,7 @@ RoleEntity createRole( store.put(roleEntity, false /* overwritten */); cache.put(roleEntity.nameIdentifier(), roleEntity); - AuthorizationUtils.callAuthorizationPlugin( + AuthorizationUtils.callAuthorizationPluginForSecurableObjects( metalake, roleEntity.securableObjects(), Sets.newHashSet(), @@ -145,7 +145,7 @@ boolean deleteRole(String metalake, String role) { try { RoleEntity roleEntity = store.get(ident, Entity.EntityType.ROLE, RoleEntity.class); - AuthorizationUtils.callAuthorizationPlugin( + AuthorizationUtils.callAuthorizationPluginForSecurableObjects( metalake, roleEntity.securableObjects(), Sets.newHashSet(), diff --git a/core/src/main/java/org/apache/gravitino/connector/BaseCatalog.java b/core/src/main/java/org/apache/gravitino/connector/BaseCatalog.java index 56c8f7920ad..45a7b909850 100644 --- a/core/src/main/java/org/apache/gravitino/connector/BaseCatalog.java +++ b/core/src/main/java/org/apache/gravitino/connector/BaseCatalog.java @@ -196,7 +196,10 @@ public AuthorizationPlugin getAuthorizationPlugin() { private BaseAuthorization createAuthorizationPluginInstance() { String authorizationProvider = - (String) catalogPropertiesMetadata().getOrDefault(conf, AUTHORIZATION_PROVIDER); + catalogPropertiesMetadata().containsProperty(AUTHORIZATION_PROVIDER) + ? (String) catalogPropertiesMetadata().getOrDefault(conf, AUTHORIZATION_PROVIDER) + : null; + if (authorizationProvider == null) { LOG.info("Authorization provider is not set!"); return null; diff --git a/core/src/test/java/org/apache/gravitino/authorization/TestFutureGrantManager.java b/core/src/test/java/org/apache/gravitino/authorization/TestFutureGrantManager.java index 87acd022367..b07e42ac716 100644 --- a/core/src/test/java/org/apache/gravitino/authorization/TestFutureGrantManager.java +++ b/core/src/test/java/org/apache/gravitino/authorization/TestFutureGrantManager.java @@ -30,6 +30,7 @@ import java.io.IOException; import java.time.Instant; import java.util.Collections; +import java.util.Optional; import org.apache.gravitino.Entity; import org.apache.gravitino.EntityStore; import org.apache.gravitino.MetadataObject; @@ -49,6 +50,7 @@ public class TestFutureGrantManager { private static EntityStore entityStore = mock(EntityStore.class); + private static OwnerManager ownerManager = mock(OwnerManager.class); private static String METALAKE = "metalake"; private static AuthorizationPlugin authorizationPlugin; private static BaseMetalake metalakeEntity = @@ -72,10 +74,11 @@ public static void setUp() throws Exception { @Test void testGrantNormally() throws IOException { - FutureGrantManager manager = new FutureGrantManager(entityStore); + FutureGrantManager manager = new FutureGrantManager(entityStore, ownerManager); SupportsRelationOperations relationOperations = mock(SupportsRelationOperations.class); when(entityStore.relationOperations()).thenReturn(relationOperations); + when(ownerManager.getOwner(any(), any())).thenReturn(Optional.empty()); // test no securable objects RoleEntity roleEntity = mock(RoleEntity.class); @@ -100,9 +103,25 @@ void testGrantNormally() throws IOException { manager.grantNewlyCreatedCatalog(METALAKE, catalog); verify(authorizationPlugin, never()).onGrantedRolesToUser(any(), any()); verify(authorizationPlugin, never()).onGrantedRolesToGroup(any(), any()); + verify(authorizationPlugin, never()).onOwnerSet(any(), any(), any()); // test only grant users reset(authorizationPlugin); + when(ownerManager.getOwner(any(), any())) + .thenReturn( + Optional.of( + new Owner() { + @Override + public String name() { + return "test"; + } + + @Override + public Type type() { + return Type.USER; + } + })); + SecurableObject securableObject = mock(SecurableObject.class); when(securableObject.type()).thenReturn(MetadataObject.Type.METALAKE); when(securableObject.privileges()) @@ -111,6 +130,7 @@ void testGrantNormally() throws IOException { when(roleEntity.nameIdentifier()).thenReturn(AuthorizationUtils.ofRole(METALAKE, "role1")); manager.grantNewlyCreatedCatalog(METALAKE, catalog); + verify(authorizationPlugin).onOwnerSet(any(), any(), any()); verify(authorizationPlugin).onGrantedRolesToUser(any(), any()); verify(authorizationPlugin, never()).onGrantedRolesToGroup(any(), any()); @@ -128,6 +148,7 @@ void testGrantNormally() throws IOException { Entity.EntityType.ROLE)) .thenReturn(Lists.newArrayList(groupEntity)); manager.grantNewlyCreatedCatalog(METALAKE, catalog); + verify(authorizationPlugin).onOwnerSet(any(), any(), any()); verify(authorizationPlugin, never()).onGrantedRolesToUser(any(), any()); verify(authorizationPlugin).onGrantedRolesToGroup(any(), any()); @@ -144,6 +165,7 @@ void testGrantNormally() throws IOException { Entity.EntityType.ROLE)) .thenReturn(Lists.newArrayList(groupEntity)); manager.grantNewlyCreatedCatalog(METALAKE, catalog); + verify(authorizationPlugin).onOwnerSet(any(), any(), any()); verify(authorizationPlugin).onGrantedRolesToUser(any(), any()); verify(authorizationPlugin).onGrantedRolesToGroup(any(), any()); @@ -152,13 +174,14 @@ void testGrantNormally() throws IOException { when(securableObject.privileges()) .thenReturn(Lists.newArrayList(Privileges.CreateCatalog.allow())); manager.grantNewlyCreatedCatalog(METALAKE, catalog); + verify(authorizationPlugin).onOwnerSet(any(), any(), any()); verify(authorizationPlugin, never()).onGrantedRolesToUser(any(), any()); verify(authorizationPlugin, never()).onGrantedRolesToGroup(any(), any()); } @Test void testGrantWithException() throws IOException { - FutureGrantManager manager = new FutureGrantManager(entityStore); + FutureGrantManager manager = new FutureGrantManager(entityStore, ownerManager); SupportsRelationOperations relationOperations = mock(SupportsRelationOperations.class); when(entityStore.relationOperations()).thenReturn(relationOperations); doThrow(new IOException("mock error")) diff --git a/core/src/test/java/org/apache/gravitino/authorization/TestOwnerManager.java b/core/src/test/java/org/apache/gravitino/authorization/TestOwnerManager.java index bf8c57580cf..83a562f640d 100644 --- a/core/src/test/java/org/apache/gravitino/authorization/TestOwnerManager.java +++ b/core/src/test/java/org/apache/gravitino/authorization/TestOwnerManager.java @@ -40,12 +40,16 @@ import java.util.UUID; import org.apache.commons.io.FileUtils; import org.apache.commons.lang3.reflect.FieldUtils; +import org.apache.gravitino.Catalog; import org.apache.gravitino.Config; import org.apache.gravitino.EntityStore; import org.apache.gravitino.EntityStoreFactory; import org.apache.gravitino.GravitinoEnv; import org.apache.gravitino.MetadataObject; import org.apache.gravitino.MetadataObjects; +import org.apache.gravitino.catalog.CatalogManager; +import org.apache.gravitino.connector.BaseCatalog; +import org.apache.gravitino.connector.authorization.AuthorizationPlugin; import org.apache.gravitino.exceptions.NoSuchMetadataObjectException; import org.apache.gravitino.exceptions.NotFoundException; import org.apache.gravitino.lock.LockManager; @@ -75,6 +79,8 @@ public class TestOwnerManager { private static IdGenerator idGenerator; private static OwnerManager ownerManager; + private static CatalogManager catalogManager = Mockito.mock(CatalogManager.class); + private static AuthorizationPlugin authorizationPlugin = Mockito.mock(AuthorizationPlugin.class); @BeforeAll public static void setUp() throws IOException, IllegalAccessException { @@ -98,6 +104,7 @@ public static void setUp() throws IOException, IllegalAccessException { Mockito.doReturn(36000L).when(config).get(TREE_LOCK_CLEAN_INTERVAL); FieldUtils.writeField(GravitinoEnv.getInstance(), "lockManager", new LockManager(config), true); + FieldUtils.writeField(GravitinoEnv.getInstance(), "catalogManager", catalogManager, true); entityStore = EntityStoreFactory.createEntityStore(config); entityStore.initialize(config); @@ -137,6 +144,10 @@ public static void setUp() throws IOException, IllegalAccessException { entityStore.put(groupEntity, false /* overwritten*/); ownerManager = new OwnerManager(entityStore); + BaseCatalog catalog = Mockito.mock(BaseCatalog.class); + Mockito.when(catalogManager.listCatalogsInfo(Mockito.any())) + .thenReturn(new Catalog[] {catalog}); + Mockito.when(catalog.getAuthorizationPlugin()).thenReturn(authorizationPlugin); } @AfterAll @@ -155,6 +166,8 @@ public void testOwner() { MetadataObject metalakeObject = MetadataObjects.of(Lists.newArrayList(METALAKE), MetadataObject.Type.METALAKE); Assertions.assertFalse(ownerManager.getOwner(METALAKE, metalakeObject).isPresent()); + Mockito.verify(authorizationPlugin, Mockito.never()) + .onOwnerSet(Mockito.any(), Mockito.any(), Mockito.any()); // Test not-existed metadata object MetadataObject notExistObject = @@ -164,13 +177,16 @@ public void testOwner() { // Test to set the user as the owner ownerManager.setOwner(METALAKE, metalakeObject, USER, Owner.Type.USER); + Mockito.verify(authorizationPlugin).onOwnerSet(Mockito.any(), Mockito.any(), Mockito.any()); Owner owner = ownerManager.getOwner(METALAKE, metalakeObject).get(); Assertions.assertEquals(USER, owner.name()); Assertions.assertEquals(Owner.Type.USER, owner.type()); // Test to set the group as the owner + Mockito.reset(authorizationPlugin); ownerManager.setOwner(METALAKE, metalakeObject, GROUP, Owner.Type.GROUP); + Mockito.verify(authorizationPlugin).onOwnerSet(Mockito.any(), Mockito.any(), Mockito.any()); // Test not-existed metadata object Assertions.assertThrows( diff --git a/server/src/main/java/org/apache/gravitino/server/web/rest/OwnerOperations.java b/server/src/main/java/org/apache/gravitino/server/web/rest/OwnerOperations.java index fb5e69198cf..d4fe66b7f8c 100644 --- a/server/src/main/java/org/apache/gravitino/server/web/rest/OwnerOperations.java +++ b/server/src/main/java/org/apache/gravitino/server/web/rest/OwnerOperations.java @@ -78,7 +78,10 @@ public Response getOwnerForObject( return Utils.doAs( httpRequest, () -> { - Optional owner = ownerManager.getOwner(metalake, object); + NameIdentifier ident = MetadataObjectUtil.toEntityIdent(metalake, object); + Optional owner = + TreeLockUtils.doWithTreeLock( + ident, LockType.READ, () -> ownerManager.getOwner(metalake, object)); if (owner.isPresent()) { return Utils.ok(new OwnerResponse(DTOConverters.toDTO(owner.get()))); } else {