Skip to content

Commit

Permalink
[#5934] fix(auth): Avoid other catalogs' privileges are pushed down (#…
Browse files Browse the repository at this point in the history 
…5935)

### What changes were proposed in this pull request?
Avoid other catalogs' privileges are pushed down.
For example, if a role has two catalogs. One catalog has select table,
the other catalog has create table.
The plugin will make the role can create and select table at the same
time.

### Why are the changes needed?

Fix: #5934

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

No.

### How was this patch tested?

Add a UT
  • Loading branch information
@jerqi
jerqi authored Dec 20, 2024
1 parent 908e994 commit 2a5838f
Show file tree
Hide file tree
Showing 4 changed files with 200 additions and 73 deletions.
118 changes: 82 additions & 36 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 @@ -18,10 +18,12 @@
*/
package org.apache.gravitino.authorization;

import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
import java.util.Collection;
import java.util.List;
import java.util.Set;
import java.util.function.BiConsumer;
import java.util.function.Consumer;
import org.apache.gravitino.Catalog;
import org.apache.gravitino.Entity;
Expand All @@ -39,6 +41,7 @@
import org.apache.gravitino.exceptions.NoSuchCatalogException;
import org.apache.gravitino.exceptions.NoSuchMetadataObjectException;
import org.apache.gravitino.exceptions.NoSuchUserException;
import org.apache.gravitino.meta.RoleEntity;
import org.apache.gravitino.utils.MetadataObjectUtil;
import org.apache.gravitino.utils.NameIdentifierUtil;

Expand Down Expand Up @@ -144,8 +147,8 @@ public static void checkRoleNamespace(Namespace namespace) {
public static void callAuthorizationPluginForSecurableObjects(
String metalake,
List<SecurableObject> securableObjects,
Set<String> catalogsAlreadySet,
Consumer<AuthorizationPlugin> consumer) {
BiConsumer<AuthorizationPlugin, String> consumer) {
Set<String> catalogsAlreadySet = Sets.newHashSet();
CatalogManager catalogManager = GravitinoEnv.getInstance().catalogManager();
for (SecurableObject securableObject : securableObjects) {
if (needApplyAuthorizationPluginAllCatalogs(securableObject)) {
Expand Down Expand Up @@ -245,40 +248,6 @@ public static void checkPrivilege(
}
}

private static void checkCatalogType(
NameIdentifier catalogIdent, Catalog.Type type, Privilege privilege) {
Catalog catalog = GravitinoEnv.getInstance().catalogDispatcher().loadCatalog(catalogIdent);
if (catalog.type() != type) {
throw new IllegalPrivilegeException(
"Catalog %s type %s doesn't support privilege %s",
catalogIdent, catalog.type(), privilege);
}
}

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;
}

private static void callAuthorizationPluginImpl(
Consumer<AuthorizationPlugin> consumer, Catalog catalog) {

if (catalog instanceof BaseCatalog) {
BaseCatalog baseCatalog = (BaseCatalog) catalog;
if (baseCatalog.getAuthorizationPlugin() != null) {
consumer.accept(baseCatalog.getAuthorizationPlugin());
}
} else {
throw new IllegalArgumentException(
String.format(
"Catalog %s is not a BaseCatalog, we don't support authorization plugin for it",
catalog.type()));
}
}

public static void authorizationPluginRemovePrivileges(
NameIdentifier ident, Entity.EntityType type) {
// If we enable authorization, we should remove the privileges about the entity in the
Expand Down Expand Up @@ -313,4 +282,81 @@ public static void authorizationPluginRenamePrivileges(
});
}
}

public static Role filterSecurableObjects(
RoleEntity role, String metalakeName, String catalogName) {
List<SecurableObject> securableObjects = role.securableObjects();
List<SecurableObject> filteredSecurableObjects = Lists.newArrayList();
for (SecurableObject securableObject : securableObjects) {
NameIdentifier identifier = MetadataObjectUtil.toEntityIdent(metalakeName, securableObject);
if (securableObject.type() == MetadataObject.Type.METALAKE) {
filteredSecurableObjects.add(securableObject);
} else {
NameIdentifier catalogIdent = NameIdentifierUtil.getCatalogIdentifier(identifier);

if (catalogIdent.name().equals(catalogName)) {
filteredSecurableObjects.add(securableObject);
}
}
}

return RoleEntity.builder()
.withId(role.id())
.withName(role.name())
.withAuditInfo(role.auditInfo())
.withNamespace(role.namespace())
.withSecurableObjects(filteredSecurableObjects)
.withProperties(role.properties())
.build();
}

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;
}

private static void callAuthorizationPluginImpl(
BiConsumer<AuthorizationPlugin, String> consumer, Catalog catalog) {

if (catalog instanceof BaseCatalog) {
BaseCatalog baseCatalog = (BaseCatalog) catalog;
if (baseCatalog.getAuthorizationPlugin() != null) {
consumer.accept(baseCatalog.getAuthorizationPlugin(), catalog.name());
}
} else {
throw new IllegalArgumentException(
String.format(
"Catalog %s is not a BaseCatalog, we don't support authorization plugin for it",
catalog.type()));
}
}

private static void callAuthorizationPluginImpl(
Consumer<AuthorizationPlugin> consumer, Catalog catalog) {

if (catalog instanceof BaseCatalog) {
BaseCatalog baseCatalog = (BaseCatalog) catalog;
if (baseCatalog.getAuthorizationPlugin() != null) {
consumer.accept(baseCatalog.getAuthorizationPlugin());
}
} else {
throw new IllegalArgumentException(
String.format(
"Catalog %s is not a BaseCatalog, we don't support authorization plugin for it",
catalog.type()));
}
}

private static void checkCatalogType(
NameIdentifier catalogIdent, Catalog.Type type, Privilege privilege) {
Catalog catalog = GravitinoEnv.getInstance().catalogDispatcher().loadCatalog(catalogIdent);
if (catalog.type() != type) {
throw new IllegalPrivilegeException(
"Catalog %s type %s doesn't support privilege %s",
catalogIdent, catalog.type(), privilege);
}
}
}
83 changes: 51 additions & 32 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 @@ -21,6 +21,7 @@
import static org.apache.gravitino.authorization.AuthorizationUtils.GROUP_DOES_NOT_EXIST_MSG;
import static org.apache.gravitino.authorization.AuthorizationUtils.ROLE_DOES_NOT_EXIST_MSG;
import static org.apache.gravitino.authorization.AuthorizationUtils.USER_DOES_NOT_EXIST_MSG;
import static org.apache.gravitino.authorization.AuthorizationUtils.filterSecurableObjects;

import com.google.common.collect.Lists;
import java.io.IOException;
Expand Down Expand Up @@ -115,17 +116,22 @@ User grantRolesToUser(String metalake, List<String> roles, String user) {
.build();
});

Set<String> catalogs = Sets.newHashSet();
List<SecurableObject> securableObjects = Lists.newArrayList();

for (Role grantedRole : roleEntitiesToGrant) {
AuthorizationUtils.callAuthorizationPluginForSecurableObjects(
metalake,
grantedRole.securableObjects(),
catalogs,
authorizationPlugin ->
authorizationPlugin.onGrantedRolesToUser(
Lists.newArrayList(roleEntitiesToGrant), updatedUser));
securableObjects.addAll(grantedRole.securableObjects());
}

AuthorizationUtils.callAuthorizationPluginForSecurableObjects(
metalake,
securableObjects,
(authorizationPlugin, catalogName) ->
authorizationPlugin.onGrantedRolesToUser(
roleEntitiesToGrant.stream()
.map(roleEntity -> filterSecurableObjects(roleEntity, metalake, catalogName))
.collect(Collectors.toList()),
updatedUser));

return updatedUser;
} catch (NoSuchEntityException nse) {
LOG.warn("Failed to grant, user {} does not exist in the metalake {}", user, metalake, nse);
Expand Down Expand Up @@ -196,17 +202,22 @@ Group grantRolesToGroup(String metalake, List<String> roles, String group) {
.build();
});

Set<String> catalogs = Sets.newHashSet();
List<SecurableObject> securableObjects = Lists.newArrayList();

for (Role grantedRole : roleEntitiesToGrant) {
AuthorizationUtils.callAuthorizationPluginForSecurableObjects(
metalake,
grantedRole.securableObjects(),
catalogs,
authorizationPlugin ->
authorizationPlugin.onGrantedRolesToGroup(
Lists.newArrayList(roleEntitiesToGrant), updatedGroup));
securableObjects.addAll(grantedRole.securableObjects());
}

AuthorizationUtils.callAuthorizationPluginForSecurableObjects(
metalake,
securableObjects,
(authorizationPlugin, catalogName) ->
authorizationPlugin.onGrantedRolesToGroup(
roleEntitiesToGrant.stream()
.map(roleEntity -> filterSecurableObjects(roleEntity, metalake, catalogName))
.collect(Collectors.toList()),
updatedGroup));

return updatedGroup;
} catch (NoSuchEntityException nse) {
LOG.warn("Failed to grant, group {} does not exist in the metalake {}", group, metalake, nse);
Expand Down Expand Up @@ -276,17 +287,21 @@ Group revokeRolesFromGroup(String metalake, List<String> roles, String group) {
.build();
});

Set<String> catalogs = Sets.newHashSet();
List<SecurableObject> securableObjects = Lists.newArrayList();
for (Role grantedRole : roleEntitiesToRevoke) {
AuthorizationUtils.callAuthorizationPluginForSecurableObjects(
metalake,
grantedRole.securableObjects(),
catalogs,
authorizationPlugin ->
authorizationPlugin.onRevokedRolesFromGroup(
Lists.newArrayList(roleEntitiesToRevoke), updatedGroup));
securableObjects.addAll(grantedRole.securableObjects());
}

AuthorizationUtils.callAuthorizationPluginForSecurableObjects(
metalake,
securableObjects,
(authorizationPlugin, catalogName) ->
authorizationPlugin.onRevokedRolesFromGroup(
roleEntitiesToRevoke.stream()
.map(roleEntity -> filterSecurableObjects(roleEntity, metalake, catalogName))
.collect(Collectors.toList()),
updatedGroup));

return updatedGroup;

} catch (NoSuchEntityException nse) {
Expand Down Expand Up @@ -358,17 +373,21 @@ User revokeRolesFromUser(String metalake, List<String> roles, String user) {
.build();
});

Set<String> catalogs = Sets.newHashSet();
List<SecurableObject> securableObjects = Lists.newArrayList();
for (Role grantedRole : roleEntitiesToRevoke) {
AuthorizationUtils.callAuthorizationPluginForSecurableObjects(
metalake,
grantedRole.securableObjects(),
catalogs,
authorizationPlugin ->
authorizationPlugin.onRevokedRolesFromUser(
Lists.newArrayList(roleEntitiesToRevoke), updatedUser));
securableObjects.addAll(grantedRole.securableObjects());
}

AuthorizationUtils.callAuthorizationPluginForSecurableObjects(
metalake,
securableObjects,
(authorizationPlugin, catalogName) ->
authorizationPlugin.onRevokedRolesFromUser(
roleEntitiesToRevoke.stream()
.map(roleEntity -> filterSecurableObjects(roleEntity, metalake, catalogName))
.collect(Collectors.toList()),
updatedUser));

return updatedUser;
} catch (NoSuchEntityException nse) {
LOG.warn("Failed to revoke, user {} does not exist in the metalake {}", user, metalake, nse);
Expand Down
11 changes: 6 additions & 5 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 @@ -21,7 +21,6 @@

import static org.apache.gravitino.metalake.MetalakeManager.checkMetalake;

import com.google.common.collect.Sets;
import java.io.IOException;
import java.time.Instant;
import java.util.List;
Expand Down Expand Up @@ -87,8 +86,9 @@ RoleEntity createRole(
AuthorizationUtils.callAuthorizationPluginForSecurableObjects(
metalake,
roleEntity.securableObjects(),
Sets.newHashSet(),
authorizationPlugin -> authorizationPlugin.onRoleCreated(roleEntity));
(authorizationPlugin, catalogName) ->
authorizationPlugin.onRoleCreated(
AuthorizationUtils.filterSecurableObjects(roleEntity, metalake, catalogName)));

return roleEntity;
} catch (EntityAlreadyExistsException e) {
Expand Down Expand Up @@ -122,8 +122,9 @@ boolean deleteRole(String metalake, String role) {
AuthorizationUtils.callAuthorizationPluginForSecurableObjects(
metalake,
roleEntity.securableObjects(),
Sets.newHashSet(),
authorizationPlugin -> authorizationPlugin.onRoleDeleted(roleEntity));
(authorizationPlugin, catalogName) ->
authorizationPlugin.onRoleDeleted(
AuthorizationUtils.filterSecurableObjects(roleEntity, metalake, catalogName)));
} catch (NoSuchEntityException nse) {
// ignore, because the role may have been deleted.
}
Expand Down
61 changes: 61 additions & 0 deletions core/src/test/java/org/apache/gravitino/authorization/TestAuthorizationUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,14 @@
*/
package org.apache.gravitino.authorization;

import com.google.common.collect.Lists;
import java.util.List;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.Namespace;
import org.apache.gravitino.exceptions.IllegalNameIdentifierException;
import org.apache.gravitino.exceptions.IllegalNamespaceException;
import org.apache.gravitino.meta.AuditInfo;
import org.apache.gravitino.meta.RoleEntity;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

Expand Down Expand Up @@ -149,4 +153,61 @@ void testCheckNamespace() {
IllegalNamespaceException.class,
() -> AuthorizationUtils.checkRoleNamespace(Namespace.of("a", "b", "c", "d")));
}

@Test
void testFilteredSecurableObjects() {

List<SecurableObject> securableObjects = Lists.newArrayList();

SecurableObject metalakeObject =
SecurableObjects.ofMetalake("metalake", Lists.newArrayList(Privileges.SelectTable.allow()));
securableObjects.add(metalakeObject);

SecurableObject catalog1Object =
SecurableObjects.ofCatalog("catalog1", Lists.newArrayList(Privileges.SelectTable.allow()));
securableObjects.add(catalog1Object);

SecurableObject catalog2Object =
SecurableObjects.ofCatalog("catalog2", Lists.newArrayList(Privileges.SelectTable.allow()));
securableObjects.add(catalog2Object);

SecurableObject schema1Object =
SecurableObjects.ofSchema(
catalog1Object, "schema1", Lists.newArrayList(Privileges.SelectTable.allow()));
SecurableObject table1Object =
SecurableObjects.ofTable(
schema1Object, "table1", Lists.newArrayList(Privileges.SelectTable.allow()));
securableObjects.add(table1Object);
securableObjects.add(schema1Object);

SecurableObject schema2Object =
SecurableObjects.ofSchema(
catalog2Object, "schema2", Lists.newArrayList(Privileges.SelectTable.allow()));
SecurableObject table2Object =
SecurableObjects.ofTable(
schema2Object, "table2", Lists.newArrayList(Privileges.SelectTable.allow()));
securableObjects.add(table2Object);
securableObjects.add(schema2Object);

RoleEntity role =
RoleEntity.builder()
.withId(1L)
.withName("role")
.withSecurableObjects(securableObjects)
.withAuditInfo(AuditInfo.EMPTY)
.build();
Role filteredRole = AuthorizationUtils.filterSecurableObjects(role, "metalake", "catalog1");
Assertions.assertEquals(4, filteredRole.securableObjects().size());
Assertions.assertTrue(filteredRole.securableObjects().contains(metalakeObject));
Assertions.assertTrue(filteredRole.securableObjects().contains(catalog1Object));
Assertions.assertTrue(filteredRole.securableObjects().contains(schema1Object));
Assertions.assertTrue(filteredRole.securableObjects().contains(table1Object));

filteredRole = AuthorizationUtils.filterSecurableObjects(role, "metalake", "catalog2");
Assertions.assertEquals(4, filteredRole.securableObjects().size());
Assertions.assertTrue(filteredRole.securableObjects().contains(metalakeObject));
Assertions.assertTrue(filteredRole.securableObjects().contains(catalog2Object));
Assertions.assertTrue(filteredRole.securableObjects().contains(schema2Object));
Assertions.assertTrue(filteredRole.securableObjects().contains(table2Object));
}
}

0 comments on commit 2a5838f

Please sign in to comment.