Skip to content

Commit

Permalink
[apache#3963][followup] feat(core): Call the method ownerset of autho…
Browse files Browse the repository at this point in the history 
…rization plugin (apache#4623)

### What changes were proposed in this pull request?

This is the follow-up pull request of apache#3963.
apache#3963 add authorization plugin to set owner. This pull request is to
call them.

### Why are the changes needed?

Fix: apache#3963

### Does this PR introduce _any_ user-facing change?
No.

### How was this patch tested?
Add new UT.
  • Loading branch information
@jerqi
jerqi committed Aug 23, 2024
1 parent f775696 commit 0af21a0
Show file tree
Hide file tree
Showing 10 changed files with 133 additions and 40 deletions.
2 changes: 1 addition & 1 deletion core/src/main/java/org/apache/gravitino/GravitinoEnv.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
55 changes: 38 additions & 17 deletions core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<SecurableObject> securableObjects,
Set<String> catalogsAlreadySet,
Consumer<AuthorizationPlugin> 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<AuthorizationPlugin> 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<String> catalogsAlreadySet, Consumer<AuthorizationPlugin> 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<AuthorizationPlugin> 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<Privilege> privileges = securableObject.privileges();
for (Privilege privilege : privileges) {
Expand All @@ -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;
}
}
19 changes: 17 additions & 2 deletions core/src/main/java/org/apache/gravitino/authorization/FutureGrantManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<Owner> ownerOptional = ownerManager.getOwner(metalake, metalakeObject);
ownerOptional.ifPresent(
owner -> {
AuthorizationPlugin authorizationPlugin = catalog.getAuthorizationPlugin();
if (authorizationPlugin != null) {
authorizationPlugin.onOwnerSet(metalakeObject, null, owner);
}
});

Map<UserEntity, Set<RoleEntity>> userGrantRoles = Maps.newHashMap();
Map<GroupEntity, Set<RoleEntity>> groupGrantRoles = Maps.newHashMap();
Expand All @@ -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;
}
Expand Down
32 changes: 22 additions & 10 deletions core/src/main/java/org/apache/gravitino/authorization/OwnerManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,11 @@ public OwnerManager(EntityStore store) {
public void setOwner(
String metalake, MetadataObject metadataObject, String ownerName, Owner.Type ownerType) {
try {
Optional<Owner> 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(
Expand All @@ -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(
Expand All @@ -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);
Expand All @@ -124,16 +140,12 @@ public Optional<Owner> getOwner(String metalake, MetadataObject metadataObject)
OwnerImpl owner = new OwnerImpl();
NameIdentifier ident = MetadataObjectUtil.toEntityIdent(metalake, metadataObject);
List<? extends Entity> 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();
Expand Down
8 changes: 4 additions & 4 deletions core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ User grantRolesToUser(String metalake, List<String> roles, String user) {

Set<String> catalogs = Sets.newHashSet();
for (Role grantedRole : roleEntitiesToGrant) {
AuthorizationUtils.callAuthorizationPlugin(
AuthorizationUtils.callAuthorizationPluginForSecurableObjects(
metalake,
grantedRole.securableObjects(),
catalogs,
Expand Down Expand Up @@ -191,7 +191,7 @@ Group grantRolesToGroup(String metalake, List<String> roles, String group) {

Set<String> catalogs = Sets.newHashSet();
for (Role grantedRole : roleEntitiesToGrant) {
AuthorizationUtils.callAuthorizationPlugin(
AuthorizationUtils.callAuthorizationPluginForSecurableObjects(
metalake,
grantedRole.securableObjects(),
catalogs,
Expand Down Expand Up @@ -269,7 +269,7 @@ Group revokeRolesFromGroup(String metalake, List<String> roles, String group) {

Set<String> catalogs = Sets.newHashSet();
for (Role grantedRole : roleEntitiesToRevoke) {
AuthorizationUtils.callAuthorizationPlugin(
AuthorizationUtils.callAuthorizationPluginForSecurableObjects(
metalake,
grantedRole.securableObjects(),
catalogs,
Expand Down Expand Up @@ -349,7 +349,7 @@ User revokeRolesFromUser(String metalake, List<String> roles, String user) {

Set<String> catalogs = Sets.newHashSet();
for (Role grantedRole : roleEntitiesToRevoke) {
AuthorizationUtils.callAuthorizationPlugin(
AuthorizationUtils.callAuthorizationPluginForSecurableObjects(
metalake,
grantedRole.securableObjects(),
catalogs,
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/org/apache/gravitino/authorization/RoleManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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(),
Expand Down
5 changes: 4 additions & 1 deletion core/src/main/java/org/apache/gravitino/connector/BaseCatalog.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
27 changes: 25 additions & 2 deletions core/src/test/java/org/apache/gravitino/authorization/TestFutureGrantManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 =
Expand All @@ -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);
Expand All @@ -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())
Expand All @@ -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());

Expand All @@ -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());

Expand All @@ -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());

Expand All @@ -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"))
Expand Down
Loading

0 comments on commit 0af21a0

Please sign in to comment.